From 115f622a6e656711767d8215cb85be2fbc5721a5 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Fri, 25 Sep 2020 00:06:20 -0700 Subject: [PATCH 1/3] Proposals/ideas for elements tracking of upstream bitcoin git branches --- doc/elements-upstream-tracking.md | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 doc/elements-upstream-tracking.md diff --git a/doc/elements-upstream-tracking.md b/doc/elements-upstream-tracking.md new file mode 100644 index 0000000000..27e669808b --- /dev/null +++ b/doc/elements-upstream-tracking.md @@ -0,0 +1,47 @@ +NOTE: I'm not currently sure whether the proposed new process (see below) would actually be a good idea, but here it is for consideration. + + +### Current process for elements tracking upstream bitcoin changes: + +* At various points (ideally after each major Bitcoin release, optionally after minor releases), someone performs a merge from the Bitcoin release, or some nearby commit on the Bitcoin release branch, into the Elements `master` branch. + * This usually seems to go through a third, temporary branch, onto which fixes can be applied to make sure that the result builds and passes tests, before merging it back into Elements `master`. + * This requires quite a lot of work from whoever is performing the merge, and it's generally all done at once, in a single big-bang merge commit. + * As a result of this process, the Elements master branch not only gets commits from Bitcoin master, it also gets commits from Bitcoin release branches, including version numbering changes, various release-related cleanup, and rebased copies of some future commits from Bitcoin master. Some of these things are desirable; some are annoying. + * A modified version of this process is possible, in which the 'merge manager' still does all the merging at once, but brings the Bitcoin side over one pull request at a time, rather than all at once, resolving merge conflicts as they go. This might give an easier time. + + +### Proposed new process for elements tracking upstream bitcoin changes: + +* Upstream Bitcoin branches/tags (outside our control): + * `master` (main development) + * `0.XX` -- release branches (one for each major version) + * [tags] `v0.XX.Y` -- one for each release +* Our branches: + * `master` (main elements development) + * (proposed) `merged-master` (contains a merged combination of all commits from _both_ elements `master` _and_ bitcoin `master`) + * (proposed) `merged-0.XX` (contains a merged combination of all commits from both elements `master` and bitcoin's `0.XX` release branch) + * We should only bother with this for the latest bitcoin major release, and retire the older release branches. If there are bugfix releases for those older versions, better for us to just say that we don't support them, and people should upgrade, to reduce the overhead of this process. + +* Some automated process constantly (for each new commit) merges each bitcoin branch into the corresponding `merged-` branch, and constantly (for each new commit) merges elements `master` into _every_ `merged-` branch. + * This will generally happen at the granularity of pull-requests / merge-requests, because the branches on each side are made up almost exclusively of merge commits. + * If the automated process fails, an issue is created, and someone has to do the merge manually, and fix the conflicts. (This is the same work someone would have to do in the current process, but without batching it all up to the very end like we do now.) + * (There was a proposal to only test the merge from the bitcoin side, and then throw it away if it's clean; and only keep it if it's nontrivial and requires human intervention. But while that reduces the total number of merges, it also means that Elements-side changes are not getting tested against the current bitcoin master immediately.) + +* When Bitcoin Core branches off for a major release, creating a new `0.XX` release branch, we branch off `merged-0.XX` to follow it. + * We _wait_ until Core finalizes the `0.XX.0` release, then we prepare a corresponding `0.XX` Elements release based on the `merged-0.XX` branch. + * (This next part is the trickiest part of the whole thing, and I'm not totally clear on how it should work.) + * We then take the _last_ commit _before_ Bitcoin 0.XX.0 branched off of Bitcoin master, and merge _that_ into the Elements master branch for further development. + * We will need to apply human judgement on what to do with the additional commits on the 0.XX release branch. Historically when merging in a Bitcoin release, we've pulled those in as well, as described above. However, some of those commits will be cherry-picked/rebased from the master branch, which means we are going to end up getting them twice. (Git seems to handle that well, as long as it can see that the two changes are the same.) + * This same issue will apply for each Bitcoin 0.XX.1, .2, etc. release. Those seem to also be made by cherry-picking / rebasing commits from the Bitcoin master branch, so the same issue exists. + + +### Proposed additional changes to the Elements development process + +* The biggest issue for this whole process, whether we do it the old way or the new way, seems to be around refactoring on the Bitcoin side. Movement of code is very confusing to `git diff` and `git merge`. (I strongly recommend using the `patience` mode, which can help somewhat.) + * A way of reducing the impact of this dramatically, is to move as much of our code as possible to separate files. E.g. instead of putting Elements wallet RPCs into `rpcwallet.cpp`, put them into `rpcwallet-elements.cpp` or similar. + * In cases where git's textual diffs get confused by code movement, this should prevent spurious conflicts completely. + * In cases where upstream changes things like function signatures, this will turn a merge conflict into a compile error, which is probably easier to fix. + * We already do some of this by separating our code within files, but git's diff algorithm is still somewhat lousy at dealing with changes _near_ other changes, or wholesale moves of files where code also changes. + * We will still need changes in upstream files, but we should try to keep them as small as practical. + +* Another annoying issue is the rename from `bitcoin` to `elements`, which conflicts with any refactoring in the build system files, for example. If possible, we may consider trying to automate this process, so that we can avoid having to merge a rename commit, and instead regenerate it by running a script against the new Bitcoin release. From 7207226d9c8d740656e0d53db8e36d126d408d74 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Tue, 20 Oct 2020 14:43:14 -0700 Subject: [PATCH 2/3] Update elements branching proposal based on chat with apoelstra --- doc/elements-upstream-tracking.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/doc/elements-upstream-tracking.md b/doc/elements-upstream-tracking.md index 27e669808b..a832d2f311 100644 --- a/doc/elements-upstream-tracking.md +++ b/doc/elements-upstream-tracking.md @@ -19,21 +19,21 @@ NOTE: I'm not currently sure whether the proposed new process (see below) would * Our branches: * `master` (main elements development) * (proposed) `merged-master` (contains a merged combination of all commits from _both_ elements `master` _and_ bitcoin `master`) - * (proposed) `merged-0.XX` (contains a merged combination of all commits from both elements `master` and bitcoin's `0.XX` release branch) - * We should only bother with this for the latest bitcoin major release, and retire the older release branches. If there are bugfix releases for those older versions, better for us to just say that we don't support them, and people should upgrade, to reduce the overhead of this process. + * `0.XX` / `0.XX.Y` -- release branches + * Each of these is based on `master`. We do not directly pull in any commits from the upstream release branches; for each cherry-pick into the upstream release branches, we cherry-pick from our `merged-master` branch instead. For administrivia (bumping version numbers, updating changelogs, etc.), we do this ourselves, to reduce merge conflicts. -* Some automated process constantly (for each new commit) merges each bitcoin branch into the corresponding `merged-` branch, and constantly (for each new commit) merges elements `master` into _every_ `merged-` branch. - * This will generally happen at the granularity of pull-requests / merge-requests, because the branches on each side are made up almost exclusively of merge commits. +* Some automated process merges each merged upstream pull request, and each merged Elements merge request, into the `merged-master` branch. * If the automated process fails, an issue is created, and someone has to do the merge manually, and fix the conflicts. (This is the same work someone would have to do in the current process, but without batching it all up to the very end like we do now.) - * (There was a proposal to only test the merge from the bitcoin side, and then throw it away if it's clean; and only keep it if it's nontrivial and requires human intervention. But while that reduces the total number of merges, it also means that Elements-side changes are not getting tested against the current bitcoin master immediately.) -* When Bitcoin Core branches off for a major release, creating a new `0.XX` release branch, we branch off `merged-0.XX` to follow it. - * We _wait_ until Core finalizes the `0.XX.0` release, then we prepare a corresponding `0.XX` Elements release based on the `merged-0.XX` branch. - * (This next part is the trickiest part of the whole thing, and I'm not totally clear on how it should work.) - * We then take the _last_ commit _before_ Bitcoin 0.XX.0 branched off of Bitcoin master, and merge _that_ into the Elements master branch for further development. - * We will need to apply human judgement on what to do with the additional commits on the 0.XX release branch. Historically when merging in a Bitcoin release, we've pulled those in as well, as described above. However, some of those commits will be cherry-picked/rebased from the master branch, which means we are going to end up getting them twice. (Git seems to handle that well, as long as it can see that the two changes are the same.) - * This same issue will apply for each Bitcoin 0.XX.1, .2, etc. release. Those seem to also be made by cherry-picking / rebasing commits from the Bitcoin master branch, so the same issue exists. +* When Bitcoin Core branches off for a major release, creating a new `0.XX` release branch, we take the last commit _before_ the upstream release branch -- which has already been merged into our `merged-master` branch -- and merge that into our local `master` elements development branch. + * This means we will always be developing based on the latest major release from Core. + * (If anything is too broken at this point on the upstream `master` branch to even be able to develop, we may have to cherry-pick fixes from the upstream release branch.) + * We _wait_ until Core finalizes the `0.XX.0` release, then we prepare a corresponding `0.XX` Elements release. + * We create our own parallel release branch, and make the same cherry-picks that Core does on their release branch, but we do it from our `merged-master` branch (where any merge conflicts between Core's work and ours have already been resolved.) For any administrative changes, such as version bumps and changelog edits, we do those ourselves on our own release branch. +* When we want to make a minor release, we will repeat the release process, branching from the Elements development `master` branch. + * We will need to cherry-pick any changes from the Core release branch again, since they are not in our dev branch. + * If Core has made any minor releases, it will be a matter of judgement to decide what exactly we cherry-pick for our minor release; most likely we will want to take everything from the Core release branch, corresponding to the latest Core minor release. ### Proposed additional changes to the Elements development process From 2f40d80b01032788234c4f01e1a430b2e96c1df5 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Fri, 26 Feb 2021 01:00:16 -0800 Subject: [PATCH 3/3] Further revisions in response to comments --- doc/elements-upstream-tracking.md | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/doc/elements-upstream-tracking.md b/doc/elements-upstream-tracking.md index a832d2f311..e7d687c763 100644 --- a/doc/elements-upstream-tracking.md +++ b/doc/elements-upstream-tracking.md @@ -1,16 +1,13 @@ -NOTE: I'm not currently sure whether the proposed new process (see below) would actually be a good idea, but here it is for consideration. +### Old process for elements tracking upstream bitcoin changes: - -### Current process for elements tracking upstream bitcoin changes: - -* At various points (ideally after each major Bitcoin release, optionally after minor releases), someone performs a merge from the Bitcoin release, or some nearby commit on the Bitcoin release branch, into the Elements `master` branch. +* At various points (ideally after each major Bitcoin release, optionally after minor releases), someone performs a merge from the Bitcoin release, or some nearby commit, into the Elements `master` branch. * This usually seems to go through a third, temporary branch, onto which fixes can be applied to make sure that the result builds and passes tests, before merging it back into Elements `master`. * This requires quite a lot of work from whoever is performing the merge, and it's generally all done at once, in a single big-bang merge commit. - * As a result of this process, the Elements master branch not only gets commits from Bitcoin master, it also gets commits from Bitcoin release branches, including version numbering changes, various release-related cleanup, and rebased copies of some future commits from Bitcoin master. Some of these things are desirable; some are annoying. + * In theory, this process only gets commits from Bitcoin `master`; sometimes it also seems to have gotten commits from Bitcoin release branches. It's unclear why this happened, but may be related to irregularities in how Core itself has used release branches in the past. * A modified version of this process is possible, in which the 'merge manager' still does all the merging at once, but brings the Bitcoin side over one pull request at a time, rather than all at once, resolving merge conflicts as they go. This might give an easier time. -### Proposed new process for elements tracking upstream bitcoin changes: +### New process for elements tracking upstream bitcoin changes: * Upstream Bitcoin branches/tags (outside our control): * `master` (main development) @@ -18,12 +15,13 @@ NOTE: I'm not currently sure whether the proposed new process (see below) would * [tags] `v0.XX.Y` -- one for each release * Our branches: * `master` (main elements development) - * (proposed) `merged-master` (contains a merged combination of all commits from _both_ elements `master` _and_ bitcoin `master`) + * `merged-master` (contains a merged combination of all commits from _both_ elements `master` _and_ bitcoin `master`) * `0.XX` / `0.XX.Y` -- release branches * Each of these is based on `master`. We do not directly pull in any commits from the upstream release branches; for each cherry-pick into the upstream release branches, we cherry-pick from our `merged-master` branch instead. For administrivia (bumping version numbers, updating changelogs, etc.), we do this ourselves, to reduce merge conflicts. * Some automated process merges each merged upstream pull request, and each merged Elements merge request, into the `merged-master` branch. * If the automated process fails, an issue is created, and someone has to do the merge manually, and fix the conflicts. (This is the same work someone would have to do in the current process, but without batching it all up to the very end like we do now.) + * Both the merging and the reviewing, are assisted by scripts checked in under `contrib`. * When Bitcoin Core branches off for a major release, creating a new `0.XX` release branch, we take the last commit _before_ the upstream release branch -- which has already been merged into our `merged-master` branch -- and merge that into our local `master` elements development branch. * This means we will always be developing based on the latest major release from Core. @@ -33,11 +31,11 @@ NOTE: I'm not currently sure whether the proposed new process (see below) would * When we want to make a minor release, we will repeat the release process, branching from the Elements development `master` branch. * We will need to cherry-pick any changes from the Core release branch again, since they are not in our dev branch. - * If Core has made any minor releases, it will be a matter of judgement to decide what exactly we cherry-pick for our minor release; most likely we will want to take everything from the Core release branch, corresponding to the latest Core minor release. + * If Core has made any minor releases, it will be a matter of judgement to decide what exactly we cherry-pick for our minor release; most likely we will want to take everything from the Core release branch corresponding to the latest Core minor release. ### Proposed additional changes to the Elements development process -* The biggest issue for this whole process, whether we do it the old way or the new way, seems to be around refactoring on the Bitcoin side. Movement of code is very confusing to `git diff` and `git merge`. (I strongly recommend using the `patience` mode, which can help somewhat.) +* The biggest issue for this whole process, whether we do it the old way or the new way, seems to be around refactoring on the Bitcoin side. Movement of code is very confusing to `git diff` and `git merge`. (I strongly recommend using advanced options to `git diff` found in the man page. The `patience` or `histogram` modes can help somewhat, and the various options around colorizing code movement are also very useful`.) * A way of reducing the impact of this dramatically, is to move as much of our code as possible to separate files. E.g. instead of putting Elements wallet RPCs into `rpcwallet.cpp`, put them into `rpcwallet-elements.cpp` or similar. * In cases where git's textual diffs get confused by code movement, this should prevent spurious conflicts completely. * In cases where upstream changes things like function signatures, this will turn a merge conflict into a compile error, which is probably easier to fix.