diff --git a/.github/workflows/missing-commits.yml b/.github/workflows/missing-commits.yml new file mode 100644 index 00000000000..cc6a7faa369 --- /dev/null +++ b/.github/workflows/missing-commits.yml @@ -0,0 +1,60 @@ +name: missing-commits + +on: + push: + branches: + # Only check that the branches are up to date when updating the + # relevant branches. + - develop + - release + +jobs: + check: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Check for missing commits + id: commits + env: + SUGGESTION: | + + If you are reading this, then the commits indicated above are + missing from "develop" and/or "release". Do a reverse-merge + as soon as possible. See CONTRIBUTING.md for instructions. + run: | + set -o pipefail + # Branches ordered by how "canonical" they are. Every commit in + # one branch should be in all the branches behind it + order=( master release develop ) + branches=() + for branch in "${order[@]}" + do + # Check that the branches exist so that this job will work on + # forked repos, which don't necessarily have master and + # release branches. + if git ls-remote --exit-code --heads origin \ + refs/heads/${branch} > /dev/null + then + branches+=( origin/${branch} ) + fi + done + + prior=() + for branch in "${branches[@]}" + do + if [[ ${#prior[@]} -ne 0 ]] + then + echo "Checking ${prior[@]} for commits missing from ${branch}" + git log --oneline --no-merges "${prior[@]}" \ + ^$branch | tee -a "missing-commits.txt" + echo + fi + prior+=( "${branch}" ) + done + if [[ $( cat missing-commits.txt | wc -l ) -ne 0 ]] + then + echo "${SUGGESTION}" + exit 1 + fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3bfce52c09b..ca83017d275 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,15 +5,12 @@ XRPL. # Contributing We assume you are familiar with the general practice of [making -contributions on GitHub][1]. This file includes only special +contributions on GitHub][contrib]. This file includes only special instructions specific to this project. ## Before you start -In general, contributions should be developed in your personal -[fork](https://github.com/XRPLF/rippled/fork). - The following branches exist in the main project repository: - `develop`: The latest set of unreleased features, and the most common @@ -26,9 +23,20 @@ The tip of each branch must be signed. In order for GitHub to sign a squashed commit that it builds from your pull request, GitHub must know your verifying key. Please set up [signature verification][signing]. -[rippled]: https://github.com/XRPLF/rippled -[signing]: - https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification +In general, external contributions should be developed in your personal +[fork][forking]. Contributions from developers with write permissions +should be done in [the main repository][rippled] in a branch with +a permitted prefix. Permitted prefixes are: +* XLS-[a-zA-Z0-9]+/.+ + * e.g. XLS-0033d/mpt-clarify-STEitherAmount +* [GitHub username]/.+ + * e.g. JoelKatz/fix-rpc-webhook-queue +* [Organization name]/.+ + * e.g. ripple/antithesis + +Regardless of where the branch is created, please open a *draft* pull +request as soon as possible after pushing the branch to Github, to +increase visibility, and ease feedback during the development process. ## Major contributions @@ -49,6 +57,7 @@ author delegates that responsibility to others. ## Before making a pull request +(Or marking a draft pull request as ready.) Changes that alter transaction processing must be guarded by an [Amendment](https://xrpl.org/amendments.html). @@ -57,18 +66,19 @@ Amendment. Ensure that your code compiles according to the build instructions in [`BUILD.md`](./BUILD.md). -If you create new source files, they must go under `src/ripple`. -You will need to add them to one of the -[source lists](./Builds/CMake/RippledCore.cmake) in CMake. Please write tests for your code. -If you create new test source files, they must go under `src/test`. -You will need to add them to one of the -[source lists](./Builds/CMake/RippledCore.cmake) in CMake. If your test can be run offline, in under 60 seconds, then it can be an automatic test run by `rippled --unittest`. Otherwise, it must be a manual test. +If you create new source files, they must be organized as follows: +* If the files are in any of the `libxrpl` modules, the headers (`.h`) must go + under `include/xrpl`, and source (`.cpp`) files must go under + `src/libxrpl`. +* All other non-test files must go under `src/xrpld`. +* All test source files must go under `src/test`. + The source must be formatted according to the style guide below. Header includes must be [levelized](./Builds/levelization). @@ -119,7 +129,10 @@ unit tests for Feature X (#1234)`. ## Pull requests In general, pull requests use `develop` as the base branch. -(Hotfixes are an exception.) +The exceptions are +* Fixes and improvements to a release candidate use `release` as the + base. +* Hotfixes use `master` as the base. If your changes are not quite ready, but you want to make it easily available for preliminary examination or review, you can create a "Draft" pull request. @@ -142,14 +155,12 @@ before it can be considered for merge by a Maintainer. Maintainers retain discretion to require more approvals if they feel the credibility of the existing approvals is insufficient. -Pull requests must be merged by [squash-and-merge][2] +Pull requests must be merged by [squash-and-merge][squash] to preserve a linear history for the `develop` branch. -### When and how to merge pull requests - -#### "Passed" +### "Ready to merge" -A pull request should only have the "Passed" label added when it +A pull request should only have the "Ready to merge" label added when it meets a few criteria: 1. It must have two approving reviews [as described @@ -166,142 +177,17 @@ meets a few criteria: merge, they should also ensure the commit message(s) are updated as well. 4. The PR branch must be up to date with the base branch (usually - `develop`). This is usually accomplised by merging the base branch + `develop`). This is usually accomplished by merging the base branch into the feature branch, but if the other criteria are met, the changes can be squashed and rebased on top of the base branch. 5. Finally, and most importantly, the author of the PR must positively indicate that the PR is ready to merge. That can be - accomplished by adding the "Passed" label if their role allows, - or by leaving a comment to the effect that the PR is ready to + accomplished by adding the "Ready to merge" label if their role + allows, or by leaving a comment to the effect that the PR is ready to merge. -Once the "Passed" label is added, a maintainer may merge the PR at -any time, so don't use it lightly. - -#### Instructions for maintainers - -The maintainer should double-check that the PR has met all the -necessary criteria, and can request additional information from the -owner, or additional reviews, and can always feel free to remove the -"Passed" label if appropriate. The maintainer has final say on -whether a PR gets merged, and are encouraged to communicate and -issues or concerns to other maintainers. - -##### Most pull requests: "Squash and merge" - -Most pull requests don't need special handling, and can simply be -merged using the "Squash and merge" button on the Github UI. Update -the suggested commit message if necessary. - -##### Slightly more complicated pull requests - -Some pull requests need to be pushed to `develop` as more than one -commit. There are multiple ways to accomplish this. If the author -describes a process, and it is reasonable, follow it. Otherwise, do -a fast forward only merge (`--ff-only`) on the command line and push. - -Either way, check that: -* The commits are based on the current tip of `develop`. -* The commits are clean: No merge commits (except when reverse - merging), no "[FOLD]" or "fixup!" messages. -* All commits are signed. If the commits are not signed by the author, use - `git commit --amend -S` to sign them yourself. -* At least one (but preferably all) of the commits has the PR number - in the commit message. - -**Never use the "Create a merge commit" or "Rebase and merge" - functions!** - -##### Releases, release candidates, and betas - -All releases, including release candidates and betas, are handled -differently from typical PRs. Most importantly, never use -the Github UI to merge a release. - -1. There are two possible conditions that the `develop` branch will - be in when preparing a release. - 1. Ready or almost ready to go: There may be one or two PRs that - need to be merged, but otherwise, the only change needed is to - update the version number in `BuildInfo.cpp`. In this case, - merge those PRs as appropriate, updating the second one, and - waiting for CI to finish in between. Then update - `BuildInfo.cpp`. - 2. Several pending PRs: In this case, do not use the Github UI, - because the delays waiting for CI in between each merge will be - unnecessarily onerous. Instead, create a working branch (e.g. - `develop-next`) based off of `develop`. Squash the changes - from each PR onto the branch, one commit each (unless - more are needed), being sure to sign each commit and update - the commit message to include the PR number. You may be able - to use a fast-forward merge for the first PR. The workflow may - look something like: -``` -git fetch upstream -git checkout upstream/develop -git checkout -b develop-next -# Use -S on the ff-only merge if prbranch1 isn't signed. -# Or do another branch first. -git merge --ff-only user1/prbranch1 -git merge --squash user2/prbranch2 -git commit -S -git merge --squash user3/prbranch3 -git commit -S -[...] -git push --set-upstream origin develop-next - -``` -2. Create the Pull Request with `release` as the base branch. If any - of the included PRs are still open, - [use closing keywords](https://docs.github.com/articles/closing-issues-using-keywords) - in the description to ensure they are closed when the code is - released. e.g. "Closes #1234" -3. Instead of the default template, reuse and update the message from - the previous release. Include the following verbiage somewhere in - the description: -``` -The base branch is release. All releases (including betas) go in -release. This PR will be merged with --ff-only (not squashed or -rebased, and not using the GitHub UI) to both release and develop. -``` -4. Sign-offs for the three platforms usually occur offline, but at - least one approval will be needed on the PR. -5. Once everything is ready to go, open a terminal, and do the - fast-forward merges manually. Do not push any branches until you - verify that all of them update correctly. -``` -git fetch upstream -git checkout -b upstream--develop -t upstream/develop || git checkout upstream--develop -git reset --hard upstream/develop -# develop-next must be signed already! -git merge --ff-only origin/develop-next -git checkout -b upstream--release -t upstream/release || git checkout upstream--release -git reset --hard upstream/release -git merge --ff-only origin/develop-next -# Only do these 3 steps if pushing a release. No betas or RCs -git checkout -b upstream--master -t upstream/master || git checkout upstream--master -git reset --hard upstream/master -git merge --ff-only origin/develop-next -# Check that all of the branches are updated -git log -1 --oneline -# The output should look like: -# 02ec8b7962 (HEAD -> upstream--master, origin/develop-next, upstream--release, upstream--develop, develop-next) Set version to 2.2.0-rc1 -# Note that all of the upstream--develop/release/master are on this commit. -# (Master will be missing for betas, etc.) -# Just to be safe, do a dry run first: -git push --dry-run upstream-push HEAD:develop -git push --dry-run upstream-push HEAD:release -# git push --dry-run upstream-push HEAD:master -# Now push -git push upstream-push HEAD:develop -git push upstream-push HEAD:release -# git push upstream-push HEAD:master -# Don't forget to tag the release, too. -git tag -git push upstream-push -``` -6. Finally -[create a new release on Github](https://github.com/XRPLF/rippled/releases). - +Once the "Ready to merge" label is added, a maintainer may merge the PR +at any time, so don't use it lightly. # Style guide @@ -312,7 +198,7 @@ coherent rather than a set of _thou shalt not_ commandments. ## Formatting -All code must conform to `clang-format` version 10, +All code must conform to `clang-format` version 18, according to the settings in [`.clang-format`](./.clang-format), unless the result would be unreasonably difficult to read or maintain. To demarcate lines that should be left as-is, surround them with comments like @@ -477,17 +363,22 @@ existing maintainer without a vote. ## Current Maintainers -Maintainers are users with admin access to the repo. Maintainers do not typically approve or deny pull requests. +Maintainers are users with maintain or admin access to the repo. +* [bthomee](https://github.com/bthomee) (Ripple) * [intelliot](https://github.com/intelliot) (Ripple) * [JoelKatz](https://github.com/JoelKatz) (Ripple) * [nixer89](https://github.com/nixer89) (XRP Ledger Foundation) +* [RichardAH](https://github.com/RichardAH) (XRP Ledger Foundation) * [Silkjaer](https://github.com/Silkjaer) (XRP Ledger Foundation) * [WietseWind](https://github.com/WietseWind) (XRPL Labs + XRP Ledger Foundation) +* [ximinez](https://github.com/ximinez) (Ripple) + ## Current Code Reviewers -Code Reviewers are developers who have the ability to review and approve source code changes. +Code Reviewers are developers who have the ability to review, approve, and +in some cases merge source code changes. * [HowardHinnant](https://github.com/HowardHinnant) (Ripple) * [scottschurr](https://github.com/scottschurr) (Ripple) @@ -511,6 +402,607 @@ Code Reviewers are developers who have the ability to review and approve source * [RichardAH](https://github.com/RichardAH) (XRPL Labs + XRP Ledger Foundation) * [dangell7](https://github.com/dangell7) (XRPL Labs) +Developers not on this list are able and encouraged to submit feedback +on pending code changes (open pull requests). + +## Instructions for maintainers + +These instructions assume you have your git upstream remotes configured +to avoid accidental pushes to the main repo, and a remote group +specifying both of them. e.g. +``` +$ git remote -v | grep upstream +upstream https://github.com/XRPLF/rippled.git (fetch) +upstream https://github.com/XRPLF/rippled.git (push) +upstream-push git@github.com:XRPLF/rippled.git (fetch) +upstream-push git@github.com:XRPLF/rippled.git (push) + +$ git config remotes.upstreams +upstream upstream-push +``` + +You can use the [setup-upstreams] script to set this up. + +It also assumes you have a default gpg signing key set up in git. e.g. +``` +$ git config user.signingkey +968479A1AFF927E37D1A566BB5690EEEBB952194 +# (This is github's key. Use your own.) +``` + +### When and how to merge pull requests + +The maintainer should double-check that the PR has met all the +necessary criteria, and can request additional information from the +owner, or additional reviews, and can always feel free to remove the +"Ready to merge" label if appropriate. The maintainer has final say on +whether a PR gets merged, and are encouraged to communicate and issues +or concerns to other maintainers. + +#### Most pull requests: "Squash and merge" + +Most pull requests don't need special handling, and can simply be +merged using the "Squash and merge" button on the Github UI. Update +the suggested commit message, or modify it as needed. + +#### Slightly more complicated pull requests + +Some pull requests need to be pushed to `develop` as more than one +commit. A PR author may *request* to merge as separate commits. They +must *justify* why separate commits are needed, and *specify* how they +would like the commits to be merged. If you disagree with the author, +discuss it with them directly. + +If the process is reasonable, follow it. The simplest option is to do a +fast forward only merge (`--ff-only`) on the command line and push to +`develop`. + +Some examples of when separate commits are worthwhile are: +1. PRs where source files are reorganized in multiple steps. +2. PRs where the commits are mostly independent and *could* be separate + PRs, but are pulled together into one PR under a commit theme or + issue. +3. PRs that are complicated enough that `git bisect` would not be much + help if it determined this PR introduced a problem. + +Either way, check that: +* The commits are based on the current tip of `develop`. +* The commits are clean: No merge commits (except when reverse + merging), no "[FOLD]" or "fixup!" messages. +* All commits are signed. If the commits are not signed by the author, use + `git commit --amend -S` to sign them yourself. +* At least one (but preferably all) of the commits has the PR number + in the commit message. + +The "Create a merge commit" and "Rebase and merge" options should be +disabled in the Github UI, but if you ever find them available **Do not +use them!** + +### Releases + +All releases, including release candidates and betas, are handled +differently from typical PRs. Most importantly, never use +the Github UI to merge a release. + +Rippled uses a linear workflow model that can be summarized as: + +1. In between releases, developers work against the `develop` branch. +2. Periodically, a maintainer will build and tag a beta version from + `develop`, which is pushed to `release`. + * Betas are usually released every two to three weeks, though that + schedule can vary depending on progress, availability, and other + factors. +3. When the changes in `develop` are considered stable and mature enough + to be ready to release, a release candidate (RC) is built and tagged + from `develop`, and merged to `release`. + * Further development for that release (primarily fixes) then + continues against `release`, while other development continues on + `develop`. Effectively, `release` is forked from `develop`. Changes + to `release` must be reverse merged to `develop`. +4. When the candidate has passed testing and is ready for release, the + final release is merged to `master`. +5. If any issues are found post-release, a hotfix / point release may be + created, which is merged to `master`, and then reverse merged to + `develop`. + +#### Betas, and the first release candidate + +##### Preparing the `develop` branch + +1. Optimally, the `develop` branch will be ready to go, with all + relevant PRs already merged. +2. If there are any PRs pending, merge them **BEFORE** preparing the beta. + 1. If only one or two PRs need to be merged, merge those PRs [as + normal](#when-and-how-to-merge-pull-requests), updating the second + one, and waiting for CI to finish in between. + 2. If there are several pending PRs, do not use the Github UI, + because the delays waiting for CI in between each merge will be + unnecessarily onerous. (Incidentally, this process can also be + used to merge if the Github UI has issues.) Merge each PR branch + directly to a `release-next` on your local machine and create a single + PR, then push your branch to `develop`. + 1. Squash the changes from each PR, one commit each (unless more + are needed), being sure to sign each commit and update the + commit message to include the PR number. You may be able to use + a fast-forward merge for the first PR. + 2. Push your branch. + 3. Continue to [Making the release](#making-the-release) to update + the version number, etc. + + The workflow may look something like: +``` +git fetch --multiple upstreams user1 user2 user3 [...] +git checkout -B release-next --no-track upstream/develop + +# Only do an ff-only merge if prbranch1 is either already +# squashed, or needs to be merged with separate commits, +# and has no merge commits. +# Use -S on the ff-only merge if prbranch1 isn't signed. +git merge [-S] --ff-only user1/prbranch1 + +git merge --squash user2/prbranch2 +git commit -S # Use the commit message provided on the PR + +git merge --squash user3/prbranch3 +git commit -S # Use the commit message provided on the PR + +[...] + +# Make sure the commits look right +git log --show-signature "upstream/develop..HEAD" + +git push --set-upstream origin + +# Continue to "Making the release" to update the version number, so +# everything can be done in one PR. +``` + +You can also use the [squash-branches] script. + +You may also need to manually close the open PRs after the changes are +merged to `develop`. Be sure to include the commit ID. + +##### Making the release + +This includes, betas, and the first release candidate (RC). + +1. If you didn't create one [preparing the `develop` + branch](#preparing-the-develop-branch), Ensure there is no old + `release-next` branch hanging around. Then make a `release-next` + branch that only changes the version number. e.g. +``` +git fetch upstreams + +git checkout --no-track -B release-next upstream/develop + +v="A.B.C-bD" +build=$( find -name BuildInfo.cpp ) +sed 's/\(^.*versionString =\).*$/\1 "'${v}'"/' ${build} > version.cpp && mv -vi version.cpp ${build} + +git diff + +git add ${build} + +git commit -S -m "Set version to ${v}" + +# You could use your "origin" repo, but some CI tests work better on upstream. +git push upstream-push +git fetch upstreams +git branch --set-upstream-to=upstream/release-next +``` + You can also use the [update-version] script. +2. Create a Pull Request for `release-next` with **`develop`** as + the base branch. + 1. Use the title "[TRIVIAL] Set version to X.X.X-bX". + 2. Instead of the default description template, use the following: +``` +## High Level Overview of Change -[1]: https://docs.github.com/en/get-started/quickstart/contributing-to-projects -[2]: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#squash-and-merge-your-commits +This PR only changes the version number. It will be merged as +soon as Github CI actions successfully complete. +``` +3. Wait for CI to successfully complete, and get someone to approve + the PR. (It is safe to ignore known CI issues.) +4. Push the updated `develop` branch using your `release-next` + branch. **Do not use the Github UI. It's important to preserve + commit IDs.** +``` +git push upstream-push release-next:develop +``` +5. In the unlikely event that the push fails because someone has merged + something else in the meantime, rebase your branch onto the updated + `develop` branch, push again, and go back to step 3. +6. Ensure that your PR against `develop` is closed. Github should do it + automatically. +7. Once this is done, forward progress on `develop` can continue + (other PRs may be merged). +8. Now create a Pull Request for `release-next` with **`release`** as + the base branch. Instead of the default template, reuse and update + the message from the previous release. Include the following verbiage + somewhere in the description: +``` +The base branch is `release`. [All releases (including +betas)](https://github.com/XRPLF/rippled/blob/develop/CONTRIBUTING.md#before-you-start) +go in `release`. This PR branch will be pushed directly to `release` (not +squashed or rebased, and not using the GitHub UI). +``` +7. Sign-offs for the three platforms (Linux, Mac, Windows) usually occur + offline, but at least one approval will be needed on the PR. + * If issues are discovered during testing, simply abandon the + release. It's easy to start a new release, it should be easy to + abandon one. **DO NOT REUSE THE VERSION NUMBER.** e.g. If you + abandon 2.4.0-b1, the next attempt will be 2.4.0-b2. +8. Once everything is ready to go, push to `release`. +``` +git fetch upstreams + +# Just to be safe, do a dry run first: +git push --dry-run upstream-push release-next:release + +# If everything looks right, push the branch +git push upstream-push release-next:release + +# Check that all of the branches are updated +git fetch upstreams +git log -1 --oneline +# The output should look like: +# 0123456789 (HEAD -> upstream/release-next, upstream/release, +# upstream/develop) Set version to 2.4.0-b1 +# Note that upstream/develop may not be on this commit, but +# upstream/release must be. +# Other branches, including some from upstream-push, may also be +# present. +``` +9. Tag the release, too. +``` +git tag +git push upstream-push +``` +10. Delete the `release-next` branch on the repo. Use the Github UI or: +``` +git push --delete upstream-push release-next +``` +11. Finally [create a new release on + Github](https://github.com/XRPLF/rippled/releases). + +#### Release candidates after the first + +Once the first release candidate is [merged into +release](#making-the-release), then `release` and `develop` *are allowed +to diverge*. + +If a bug or issue is discovered in a version that has a release +candidate being tested, any fix and new version will need to be applied +against `release`, then reverse-merged to `develop`. This helps keep git +history as linear as possible. + +A `release-next` branch will be created from `release`, and any further +work for that release must be based on `release-next`. Specifically, +PRs must use `release-next` as the base, and those PRs will be merged +directly to `release-next` when approved. Changes should be restricted +to bug fixes, but other changes may be necessary from time to time. + +1. Open any PRs for the pending release using `release-next` as the base, + so they can be merged directly in to it. Unlike `develop`, though, + `release-next` can be thrown away and recreated if necessary. +2. Once a new release candidate is ready, create a version commit as in + step 1 [above](#making-the-release) on `release-next`. You can use + the [update-version] script for this, too. +3. Jump to step 8 ("Now create a Pull Request for `release-next` with + **`release`** as the base") from the process + [above](#making-the-release) to merge `release-next` into `release`. + +##### Follow up: reverse merge + +Once the RC is merged and tagged, it needs to be reverse merged into +`develop` as soon as possible. + +1. Create a branch, based on `upstream/develop`. + The branch name is not important, but could include "mergeNNNrcN". + E.g. For release A.B.C-rcD, use `mergeABCrcD`. +``` +git fetch upstreams + +git checkout --no-track -b mergeABCrcD upstream/develop +``` +2. Merge `release` into your branch. +``` +# I like the "--edit --log --verbose" parameters, but they are +# not required. +git merge upstream/release +``` +3. `BuildInfo.cpp` will have a conflict with the version number. + Resolve it with the version from `develop` - the higher version. +4. Push your branch to your repo (or `upstream` if you have permission), + and open a normal PR against `develop`. The "High level overview" can + simply indicate that this is a merge of the RC. The "Context" should + summarize the changes from the RC. Include the following text + prominently: +``` +This PR must be merged manually using a push. Do not use the Github UI. +``` +5. Depending on the complexity of the changes, and/or merge conflicts, + the PR may need a thorough review, or just a sign-off that the + merge was done correctly. +6. If `develop` is updated before this PR is merged, do not merge + `develop` back into your branch. Instead rebase preserving merges, + or do the merge again. (See also the `rerere` git config setting.) +``` +git rebase --rebase-merges upstream/develop +# OR +git reset --hard upstream/develop +git merge upstream/release +``` +7. When the PR is ready, push it to `develop`. +``` +git fetch upstreams + +# Make sure the commits look right +git log --show-signature "upstream/develop^..HEAD" + +git push upstream-push mergeABCrcD:develop + +git fetch upstreams +``` +Development on `develop` can proceed as normal. + + +#### Final releases + +A final release is any release that is not a beta or RC, such as 2.2.0. + +Only code that has already been tested and vetted across all three +platforms should be included in a final release. Most of the time, that +means that the commit immediately preceding the commit setting the +version number will be an RC. Occasionally, there may be last-minute bug +fixes included as well. If so, those bug fixes must have been tested +internally as if they were RCs (at minimum, ensuring unit tests pass, +and the app starts, syncs, and stops cleanly across all three +platforms.) + +*If in doubt, make an RC first.* + +The process for building a final release is very similar to [the process +for building a beta](#making-the-release), except the code will be +moving from `release` to `master` instead of from `develop` to +`release`, and both branches will be pushed at the same time. + +1. Ensure there is no old `master-next` branch hanging around. + Then make a `master-next` branch that only changes the version + number. As above, or using the + [update-version] script. +2. Create a Pull Request for `master-next` with **`master`** as + the base branch. Instead of the default template, reuse and update + the message from the previous final release. Include the following verbiage + somewhere in the description: +``` +The base branch is `master`. This PR branch will be pushed directly to +`release` and `master` (not squashed or rebased, and not using the +GitHub UI). +``` +7. Sign-offs for the three platforms (Linux, Mac, Windows) usually occur + offline, but at least one approval will be needed on the PR. + * If issues are discovered during testing, close the PR, delete + `master-next`, and move development back to `release`, [issuing + more RCs as necessary](#release-candidates-after-the-first) +8. Once everything is ready to go, push to `release` and `master`. +``` +git fetch upstreams + +# Just to be safe, do dry runs first: +git push --dry-run upstream-push master-next:release +git push --dry-run upstream-push master-next:master + +# If everything looks right, push the branch +git push upstream-push master-next:release +git push upstream-push master-next:master + +# Check that all of the branches are updated +git fetch upstreams +git log -1 --oneline +# The output should look like: +# 0123456789 (HEAD -> upstream/master-next, upstream/master, +# upstream/release) Set version to A.B.0 +# Note that both upstream/release and upstream/master must be on this +# commit. +# Other branches, including some from upstream-push, may also be +# present. +``` +9. Tag the release, too. +``` +git tag +git push upstream-push +``` +10. Delete the `master-next` branch on the repo. Use the Github UI or: +``` +git push --delete upstream-push master-next +``` +11. [Create a new release on + Github](https://github.com/XRPLF/rippled/releases). Be sure that + "Set as the latest release" is checked. +12. Finally [reverse merge the release into `develop`](#follow-up-reverse-merge). + +#### Special cases: point releases, hotfixes, etc. + +On occassion, a bug or issue is discovered in a version that already +had a final release. Most of the time, development will have started +on the next version, and will usually have changes in `develop` +and often in `release`. + +Because git history is kept as linear as possible, any fix and new +version will need to be applied against `master`. + +The process for building a hotfix release is very similar to [the +process for building release candidates after the +first](#release-candidates-after-the-first) and [for building a final +release](#final-releases), except the changes will be done against +`master` instead of `release`. + +If there is only a single issue for the hotfix, the work can be done in +any branch. When it's ready to merge, jump to step 3 using your branch +instead of `master-next`. + +1. Create a `master-next` branch from `master`. +``` +git checkout --no-track -b master-next upstream/master +git push upstream-push +git fetch upstreams +``` +2. Open any PRs for the pending hotfix using `master-next` as the base, + so they can be merged directly in to it. Unlike `develop`, though, + `master-next` can be thrown away and recreated if necessary. +3. Once the hotfix is ready, create a version commit using the same + steps as above, or use the + [update-version] script. +4. Create a Pull Request for `master-next` with **`master`** as + the base branch. Instead of the default template, reuse and update + the message from the previous final release. Include the following verbiage + somewhere in the description: +``` +The base branch is `master`. This PR branch will be pushed directly to +`master` (not squashed or rebased, and not using the GitHub UI). +``` +7. Sign-offs for the three platforms (Linux, Mac, Windows) usually occur + offline, but at least one approval will be needed on the PR. + * If issues are discovered during testing, update `master-next` as + needed, but ensure that the changes are properly squashed, and the + version setting commit remains last +8. Once everything is ready to go, push to `master` **only**. +``` +git fetch upstreams + +# Just to be safe, do a dry run first: +git push --dry-run upstream-push master-next:master + +# If everything looks right, push the branch +git push upstream-push master-next:master + +# Check that all of the branches are updated +git fetch upstreams +git log -1 --oneline +# The output should look like: +# 0123456789 (HEAD -> upstream/master-next, upstream/master) Set version +# to 2.4.1 +# Note that upstream/master must be on this commit. upstream/release and +# upstream/develop should not. +# Other branches, including some from upstream-push, may also be +# present. +``` +9. Tag the release, too. +``` +git tag +git push upstream-push +``` +9. Delete the `master-next` branch on the repo. +``` +git push --delete upstream-push master-next +``` +10. [Create a new release on + Github](https://github.com/XRPLF/rippled/releases). Be sure that + "Set as the latest release" is checked. + +Once the hotfix is released, it needs to be reverse merged into +`develop` as soon as possible. It may also need to be merged into +`release` if a release candidate is under development. + +1. Create a branch in your own repo, based on `upstream/develop`. + The branch name is not important, but could include "mergeNNN". + E.g. For release 2.2.3, use `merge223`. +``` +git fetch upstreams + +git checkout --no-track -b merge223 upstream/develop +``` +2. Merge master into your branch. +``` +# I like the "--edit --log --verbose" parameters, but they are +# not required. +git merge upstream/master +``` +3. `BuildInfo.cpp` will have a conflict with the version number. + Resolve it with the version from `develop` - the higher version. +4. Push your branch to your repo, and open a normal PR against + `develop`. The "High level overview" can simply indicate that this + is a merge of the hotfix version. The "Context" should summarize + the changes from the hotfix. Include the following text + prominently: +``` +This PR must be merged manually using a --ff-only merge. Do not use the Github UI. +``` +5. Depending on the complexity of the hotfix, and/or merge conflicts, + the PR may need a thorough review, or just a sign-off that the + merge was done correctly. +6. If `develop` is updated before this PR is merged, do not merge + `develop` back into your branch. Instead rebase preserving merges, + or do the merge again. (See also the `rerere` git config setting.) +``` +git rebase --rebase-merges upstream/develop +# OR +git reset --hard upstream/develop +git merge upstream/master +``` +7. When the PR is ready, push it to `develop`. +``` +git fetch upstreams + +# Make sure the commits look right +git log --show-signature "upstream/develop..HEAD" + +git push upstream-push HEAD:develop +``` +Development on `develop` can proceed as normal. It is recommended to +create a beta (or RC) immediately to ensure that everything worked as +expected. + +##### An even rarer scenario: A hotfix on an old release + +Historically, once a final release is tagged and packages are released, +versions older than the latest final release are no longer supported. +However, there is a possibility that a very high severity bug may occur +in a non-amendment blocked version that is still being run by +a significant fraction of users, which would necessitate a hotfix / point +release to that version as well as any later versions. + +This scenario would follow the same basic procedure as above, +except that *none* of `develop`, `release`, or `master` +would be touched during the release process. + +In this example, consider if version 2.1.1 needed to be patched. + +1. Create two branches in the main (`upstream`) repo. +``` +git fetch upstreams + +# Create a base branch off the tag +git checkout --no-track -b master-2.1.2 2.1.1 +git push upstream-push + +# Create a working branch +git checkout --no-track -b master212-next master-2.1.2 +git push upstream-push + +git fetch upstreams +``` +2. Work continues as above, except using `master-2.1.2`as + the base branch for any merging, packaging, etc. +3. After the release is tagged and packages are built, you could + potentially delete both branches, e.g. `master-2.1.2` and + `master212-next`. However, it may be useful to keep `master-2.1.2` + around indefinitely for reference. +4. Assuming that a hotfix is also released for the latest + version in parallel with this one, or if the issue is + already fixed in the latest version, do no do any + reverse merges. However, if it is not, it probably makes + sense to reverse merge `master-2.1.2` into `master`, + release a hotfix for _that_ version, then reverse merge + from `master` to `develop`. (Please don't do this unless absolutely + necessary.) + +[contrib]: https://docs.github.com/en/get-started/quickstart/contributing-to-projects +[squash]: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#squash-and-merge-your-commits +[forking]: https://github.com/XRPLF/rippled/fork +[rippled]: https://github.com/XRPLF/rippled +[signing]: https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification +[setup-upstreams]: ./bin/git/setup-upstreams.sh +[squash-branches]: ./bin/git/squash-branches.sh +[update-version]: ./bin/git/update-version.sh diff --git a/bin/git/setup-upstreams.sh b/bin/git/setup-upstreams.sh new file mode 100755 index 00000000000..cdf3f37f37e --- /dev/null +++ b/bin/git/setup-upstreams.sh @@ -0,0 +1,86 @@ +#!/bin/bash + +if [[ $# -ne 1 || "$1" == "--help" || "$1" == "-h" ]] +then + name=$( basename $0 ) + cat <<- USAGE + Usage: $name + + Where is the Github username of the upstream repo. e.g. XRPLF +USAGE + exit 0 +fi + +# Create upstream remotes based on origin +shift +user="$1" +# Get the origin URL. Expect it be an SSH-style URL +origin=$( git remote get-url origin ) +if [[ "${origin}" == "" ]] +then + echo Invalid origin remote >&2 + exit 1 +fi +# echo "Origin: ${origin}" +# Parse the origin +ifs_orig="${IFS}" +IFS=':' read remote originpath <<< "${origin}" +# echo "Remote: ${remote}, Originpath: ${originpath}" +IFS='@' read sshuser server <<< "${remote}" +# echo "SSHUser: ${sshuser}, Server: ${server}" +IFS='/' read originuser repo <<< "${originpath}" +# echo "Originuser: ${originuser}, Repo: ${repo}" +if [[ "${sshuser}" == "" || "${server}" == "" || "${originuser}" == "" + || "${repo}" == "" ]] +then + echo "Can't parse origin URL: ${origin}" >&2 + exit 1 +fi +upstream="https://${server}/${user}/${repo}" +upstreampush="${remote}:${user}/${repo}" +upstreamgroup="upstream upstream-push" +current=$( git remote get-url upstream 2>/dev/null ) +currentpush=$( git remote get-url upstream-push 2>/dev/null ) +currentgroup=$( git config remotes.upstreams ) +if [[ "${current}" == "${upstream}" ]] +then + echo "Upstream already set up correctly. Skip" +elif [[ -n "${current}" && "${current}" != "${upstream}" && + "${current}" != "${upstreampush}" ]] +then + echo "Upstream already set up as: ${current}. Skip" +else + if [[ "${current}" == "${upstreampush}" ]] + then + echo "Upstream set to dangerous push URL. Update." + _run git remote rename upstream upstream-push || \ + _run git remote remove upstream + currentpush=$( git remote get-url upstream-push 2>/dev/null ) + fi + _run git remote add upstream "${upstream}" +fi + +if [[ "${currentpush}" == "${upstreampush}" ]] +then + echo "upstream-push already set up correctly. Skip" +elif [[ -n "${currentpush}" && "${currentpush}" != "${upstreampush}" ]] +then + echo "upstream-push already set up as: ${currentpush}. Skip" +else + _run git remote add upstream-push "${upstreampush}" +fi + +if [[ "${currentgroup}" == "${upstreamgroup}" ]] +then + echo "Upstreams group already set up correctly. Skip" +elif [[ -n "${currentgroup}" && "${currentgroup}" != "${upstreamgroup}" ]] +then + echo "Upstreams group already set up as: ${currentgroup}. Skip" +else + _run git config --add remotes.upstreams "${upstreamgroup}" +fi + +_run git fetch --jobs=$(nproc) upstreams + +exit 0 + diff --git a/bin/git/squash-branches.sh b/bin/git/squash-branches.sh new file mode 100755 index 00000000000..66f1a2d715c --- /dev/null +++ b/bin/git/squash-branches.sh @@ -0,0 +1,69 @@ +#!/bin/bash + +if [[ $# -lt 3 || "$1" == "--help" || "$1" = "-h" ]] +then + name=$( basename $0 ) + cat <<- USAGE + Usage: $name workbranch base/branch user/branch [user/branch [...]] + + * workbranch will be created locally from base/branch + * base/branch and user/branch may be specified as user:branch to allow + easy copying from Github PRs + * Remotes for each user must already be set up +USAGE +exit 0 +fi + +work="$1" +shift + +branches=( $( echo "${@}" | sed "s/:/\//" ) ) +base="${branches[0]}" +unset branches[0] + +set -e + +users=() +for b in "${branches[@]}" +do + users+=( $( echo $b | cut -d/ -f1 ) ) +done + +users=( $( printf '%s\n' "${users[@]}" | sort -u ) ) + +git fetch --multiple upstreams "${users[@]}" +git checkout -B "$work" --no-track "$base" + +for b in "${branches[@]}" +do + git merge --squash "${b}" + git commit -S # Use the commit message provided on the PR +done + +# Make sure the commits look right +git log --show-signature "$base..HEAD" + +parts=( $( echo $base | sed "s/\// /" ) ) +repo="${parts[0]}" +b="${parts[1]}" +push=$repo +if [[ "$push" == "upstream" ]] +then + push="upstream-push" +fi +if [[ "$repo" == "upstream" ]] +then + repo="upstreams" +fi +cat << PUSH + +------------------------------------------------------------------- +This script will not push. Verify everything is correct, then push +to your repo, and create a PR if necessary. Once the PR is approved, +run: + +git push $push HEAD:$b +git fetch $repo +------------------------------------------------------------------- +PUSH + diff --git a/bin/git/update-version.sh b/bin/git/update-version.sh new file mode 100755 index 00000000000..c901a29e6a7 --- /dev/null +++ b/bin/git/update-version.sh @@ -0,0 +1,58 @@ +#!/bin/bash + +if [[ $# -ne 3 || "$1" == "--help" || "$1" = "-h" ]] +then + name=$( basename $0 ) + cat <<- USAGE + Usage: $name workbranch base/branch version + + * workbranch will be created locally from base/branch. If it exists, + it will be reused, so make sure you don't overwrite any work. + * base/branch may be specified as user:branch to allow easy copying + from Github PRs. +USAGE +exit 0 +fi + +work="$1" +shift + +base=$( echo "$1" | sed "s/:/\//" ) +shift + +version=$1 +shift + +set -e + +git fetch upstreams + +git checkout -B "${work}" --no-track "${base}" + +push=$( git rev-parse --abbrev-ref --symbolic-full-name '@{push}' \ + 2>/dev/null ) || true +if [[ "${push}" != "" ]] +then + echo "Warning: ${push} may already exist." +fi + +build=$( find -name BuildInfo.cpp ) +sed 's/\(^.*versionString =\).*$/\1 "'${version}'"/' ${build} > version.cpp && \ +diff "${build}" version.cpp && exit 1 || \ +mv -vi version.cpp ${build} + +git diff + +git add ${build} + +git commit -S -m "Set version to ${version}" + +git log --oneline --first-parent ${base}^.. + +cat << PUSH + +------------------------------------------------------------------- +This script will not push. Verify everything is correct, then push +to your repo, and create a PR as described in CONTRIBUTING.md. +------------------------------------------------------------------- +PUSH diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 8e28eb98cdd..9352518dbac 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -3171,7 +3171,7 @@ class LedgerRPC_test : public beast::unit_test::suite params[jss::ledger_index] = jss::validated; params[jss::permissioned_domain] = "NotAHexString"; auto const jrr = env.rpc("json", "ledger_entry", to_string(params)); - checkErrorValue(jrr[jss::result], "malformedObjectId", ""); + checkErrorValue(jrr[jss::result], "malformedRequest", ""); } { @@ -3180,7 +3180,7 @@ class LedgerRPC_test : public beast::unit_test::suite params[jss::ledger_index] = jss::validated; params[jss::permissioned_domain] = 10; auto const jrr = env.rpc("json", "ledger_entry", to_string(params)); - checkErrorValue(jrr[jss::result], "malformedObject", ""); + checkErrorValue(jrr[jss::result], "malformedRequest", ""); } { @@ -3190,7 +3190,7 @@ class LedgerRPC_test : public beast::unit_test::suite params[jss::permissioned_domain][jss::account] = 1; params[jss::permissioned_domain][jss::seq] = seq; auto const jrr = env.rpc("json", "ledger_entry", to_string(params)); - checkErrorValue(jrr[jss::result], "malformedAccount", ""); + checkErrorValue(jrr[jss::result], "malformedRequest", ""); } { @@ -3200,7 +3200,7 @@ class LedgerRPC_test : public beast::unit_test::suite params[jss::permissioned_domain][jss::account] = ""; params[jss::permissioned_domain][jss::seq] = seq; auto const jrr = env.rpc("json", "ledger_entry", to_string(params)); - checkErrorValue(jrr[jss::result], "malformedAccount", ""); + checkErrorValue(jrr[jss::result], "malformedAddress", ""); } { @@ -3210,7 +3210,7 @@ class LedgerRPC_test : public beast::unit_test::suite params[jss::permissioned_domain][jss::account] = alice.human(); params[jss::permissioned_domain][jss::seq] = "12g"; auto const jrr = env.rpc("json", "ledger_entry", to_string(params)); - checkErrorValue(jrr[jss::result], "malformedSequence", ""); + checkErrorValue(jrr[jss::result], "malformedRequest", ""); } } diff --git a/src/xrpld/rpc/handlers/LedgerEntry.cpp b/src/xrpld/rpc/handlers/LedgerEntry.cpp index daf46ab1b38..7298ee290d8 100644 --- a/src/xrpld/rpc/handlers/LedgerEntry.cpp +++ b/src/xrpld/rpc/handlers/LedgerEntry.cpp @@ -811,22 +811,19 @@ parsePermissionedDomains(Json::Value const& pd, Json::Value& jvResult) { if (pd.isString()) { - Json::Value result; - auto const index = parseIndex(pd, result); - if (!index) - jvResult[jss::error] = "malformedObjectId"; + auto const index = parseIndex(pd, jvResult); return index; } if (!pd.isObject()) { - jvResult[jss::error] = "malformedObject"; + jvResult[jss::error] = "malformedRequest"; return std::nullopt; } if (!pd.isMember(jss::account) || !pd[jss::account].isString()) { - jvResult[jss::error] = "malformedAccount"; + jvResult[jss::error] = "malformedRequest"; return std::nullopt; } @@ -834,14 +831,14 @@ parsePermissionedDomains(Json::Value const& pd, Json::Value& jvResult) (pd[jss::seq].isInt() && pd[jss::seq].asInt() < 0) || (!pd[jss::seq].isInt() && !pd[jss::seq].isUInt())) { - jvResult[jss::error] = "malformedSequence"; + jvResult[jss::error] = "malformedRequest"; return std::nullopt; } auto const account = parseBase58(pd[jss::account].asString()); if (!account) { - jvResult[jss::error] = "malformedAccount"; + jvResult[jss::error] = "malformedAddress"; return std::nullopt; }