Teach GitGitGadget to map original -> upstream commits, detect when the PR was merged and where#33
Merged
dscho merged 37 commits intogitgitgadget:mastergitgitgadget/gitgitgadget:masterfrom Dec 19, 2018
Merged
Teach GitGitGadget to map original -> upstream commits, detect when the PR was merged and where#33dscho merged 37 commits intogitgitgadget:mastergitgitgadget/gitgitgadget:masterfrom
dscho merged 37 commits intogitgitgadget:mastergitgitgadget/gitgitgadget:masterfrom
Conversation
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Suggested by NPM. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
eef9975 to
263815c
Compare
Of course, there was the `event-stream` problem which needed to be addressed manually: by simply uninstalling the `nodemon` package (we do not need it, actually, it was just meant as a convenience for developing.) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The original idea for keeping GitGitGadget's PRs up to date, despite the fact that commits will *never* make it into git.git unchanged (they will all be sent via mail, then applied with a complete different commit hash), was to use the `amlog` notes that used to provide a mapping from original Message-ID to commit in git.git. During the exploration/development of this part of GitGitGadget, serious gaps and mistakes were discovered in the `amlog` notes, and rather than fixing it, the Git maintainer chose to take away the reverse mapping, and not fix the gaps at all. As a consequence, this developer was forced to re-think the design of this feature. Going back to another exploration contributed as https://public-inbox.org/git/nycvar.QRO.7.76.6.1804041821420.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz/ in the wake of GitMerge 2017, this commit adds a script that takes commits from git.git and tries to match them via their timestamp to the corresponding Message-IDs in https://public-inbox.org/git. As this is not fool-proof, quite a few mappings were added by hand, others were ignored, and all unmatched commits from before 2012 were left as-are, to be fixed later, if necessary. The script now also has an `update` mode which updates the `refs/notes/commit-to-mail` notes in https://github.com/gitgitgadget/git (which are essentially what `amlog` should have been). The new idea is to install a VSTS job that keeps this notes ref up to date with `pu`. For convenience (and also to accommodate the surprising fact that pretty much all Git oldtimes flatly refuse to touch anything that is Typescript, claiming that they are unfamiliar with it, despite the fact that it is *really* close to Javascript, but whatever), this script is a Unix shell script. Another script will be added later that will be installed in the same VSTS job to try to match the new commits to GitGitGadget's PRs' commits, and then update the respective PRs' statuses. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is a companion to `lookup-commit.sh --notes update`: after that command updated refs/notes/commit-to-mail, this here script will update refs/notes/mail-to-commit to provide a reverse mapping: for every Message-ID, one can now find the active git.git commit thusly: hash="$(echo "$MESSAGE_ID" | git hash-object --stdin)" git notes --ref=ref/notes/mail-to-commit show $hash If the command turns up empty-handed, there is no active git.git commit for that mail. Note: in this context, a git.git commit is "active" as long as it is reachable from `pu`. Whenever `pu` is updated and formerly-reachable commits become unreachable, their mapping will be removed from the notes (although you can still see them in the history of that ref). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Due to the way we addressed the changes in 361d073 (Fix "Published-As" footer in the cover letter, 2018-06-28), cSpell now complains about "Fsomebody" and "Fmaster" not to be spelled correctly. Let's fix that, and at the same time make the test script easier to understand. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
It is much better to use proper object orientation than to pass a bag of options around to all functions... While at it, also add a `merge()` method in preparation for future tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Commit notes are appended using the `git notes append` command, which means that either a new commit note is created, or the contents of an existing one are combined with an empty line and then the new note. In our use case, we can safely assume that the notes we append will not contain empty lines, so an easy way to get at the latest note that was appended is to simply get the commit notes and strip everything until the last empty line (if any). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In order to associate original commits with commits in git.git, we will need to keep track of the sent patch mails for which we do not yet know of any such corresponding commit. While at it, also keep track of the open Pull Requests. This will be needed to be able to close Pull Requests when their corresponding patches were merged to `master`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We need to do this in a couple of places, so let's just have *one* static function that does this, and use it from everywhere. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
One part of GitGitGadget is an automated build that maintains the
`commit-to-mail` and `mail-to-commit` notes. These notes can be used to
try to figure out which upstream commits ("git.git commits") correspond
to which mails on the Git mailing list, and vice versa.
We need such a mapping to determine efficiently when/if any GitGitGadget
PR has been integrated into any of the four integration branches of
git.git yet.
For now, this class only has code to use `refs/notes/mail-to-commit` of
https://github.com/gitgitgadget/git.
Later commits will add code to actually use it.
Side note: In https://github.com/gitster/git, Junio Hamano publishes a
notes ref under the name `amlog` that maps some commits to their
corresponding Message-IDs, and some of them are even accurate. Too many
are not, and too many are missing, so we cannot use them here.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The motivation behind this class is that we want to install an automated "Continuous Integration" job to monitor e.g. the `mail-to-commit` notes (see the previous commits for details on that one) and to update the PR on GitHub accordingly. For example, once the patches have been integrated into the `pu` branch, we want to annotate the PR's commits with links to the corresponding commits in the `pu` branch, mark the PR as being integrated into `pu`, etc. We use the Checks API here instead of the Commit Statuses API, despite the user experience of the latter being a bit nicer, because Commit Statuses can never be removed. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
While the normal purpose of "Continuous Integration" jobs is to monitor branches and perform tests whenever they are updated (or Pull Requests are opened), to verify that no regressions were introduced, we can also use the same technology to monitor e.g. the `mail-to-commit` ref and trigger actions based on what we find. For example, we want to know when our patches have been applied and integrated into, say, `pu`, by monitoring and parsing both the `refs/notes/mail-to-commit` ref as well as the `refs/heads/pu` branch. This commit adds the CIHelper class and a script to call its methods, but at the moment it does not do a whole lot of anything (except that CIHelper already has the fun part of determining which commit, if any, merged a branch into another branch). For good measure, we also add some tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Whenever the CIHelper wants to do anything with the MailCommitMapping class, it makes sense to update the notes ref and the branches. We only need to do that at most once per CIHelper instantiation, though (the idea being that the CIHelper will be instantiated via automated builds when any of the upstream branches or the `amlog` notes ref changes). So let's add a method that we can call to ensure that those refs were updated. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When working with Git's code contribution process, it is often necessary to get quickly from the original commit to the corresponding upstream commit (because patches are applied from mails, so the commit name is necessarily different from the original). This commit adds a method by which GitGitGadget can use the `gitgitgadget` and `mail-to-commit` notes to identify the upstream commit, given an original commit. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is a very important thing for GitGitGadget to do: not only will it help users by having a handy link (as a green check mark on the commit in GitHub's commit list) to jump from an originally contributed commit to the commit that finally made it upstream, but it also provides the prerequisite to identify the upstream branches into which the PR was already integrated. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This information requires quite a bit to be detected before that: which original commits were already integrated into git.git and what is the new SHA-1 of the upstream commits. While at it, we can also detect which name Junio Hamano (the git.git maintainer) chose for the patch series, and we can also update the GitHub PR's labels to reflect which upstream branch(es) integrated the patch series already. And we can also close the PR once the patch series hit `master` or `maint`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
There are already a couple of open Pull Requests at https://github.com/gitgitgadget/git/pulls, and we will want to update the metadata (whose "schema" we just expanded) to reflect which PRs are open, and which mails are "active" (read: contain the patches corresponding to open PRs). So we need a tool to fix things up after the fact. This is it. This tool will grow all the functionality we need in that respect. Signed-off-by: Johannes Schindelin <johannes.Schindelin@gmx.de>
When going through that lossy medium called email, the original commit SHA-1 gets lost, and therefore it can be daunting to figure out what the heck happened to ones commits on the way into git.git. When using GitGitGadget, we record the Message-ID corresponding to the mail that was generated and sent from the original commits, and the Git maintainer thankfully maintains a similar log (from Message-ID to commit in `pu`). This commit adds the new sub-command `lookup-upstream-commit` to the `misc-helper` script, to automate looking all this up. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
It is pretty nice to use GitGitGadget. But what if a patch series had already been started without GitGitGadget? Fear not. This script can be used to initialize the PR metadata accordingly. It is still a very manual, error-prone process (the information about the number of patches should be taken from the cover letter, the tip commit should be verified against the latest diff, and the base commit should be determined automatically, just like the iteration). If need be, we can start from here, automating one little piece by one little piece. The primary reason for this was to allow submitting v2 iterations for rebase-in-c-2-basic and rebase-in-c-4-opts: gitgitgadget/git#32 and gitgitgadget/git#33 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The idea is to run the misc-helper whenever `pu` changes (*all* commits seem to enter `pu` first, and if they don't, they will enter `pu` eventually), so that we can have up to date mappings between original commits and upstream commits, presented as "Checks" in GitHub's UI. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The options are stored as noted on the empty blob, and they are stored as a JSON object. With the `get-gitgitgadget-options`, the user can look at them conveniently. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The metadata regarding mails that were sent out by GitGitGadget is stored in the `gitgitgadget` notes, and this new command offers a convenient way to inspect them. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
With this new sub-command, a user can see what GitGitGadget currently knows about a PR (identified by its URL). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is mainly a convenience for debugging, and might come in handy later, too. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is pretty useful information, and it is nice to be able to get to it easily. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
For all open PRs, we want to see whether the commits have counterparts in git.git. This commit adds a one-stop task to do exactly that. It does rely on the GitGitGadget options being kept up to date with the open PRs and the corresponding Message-IDs, and since this patch is developed on a topic branch, the GitGitGadget version running at the time of writing does not do that. For that reason, this task currently requires to be run after the "update-open-prs" task has been run. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is *very* primitive at the moment. Hopefully we will not need it all too often... Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
These tokens are only valid for 1h, intended to be used in CI or in a web app. Exactly what we need. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
derrickstolee
approved these changes
Dec 18, 2018
Contributor
derrickstolee
left a comment
There was a problem hiding this comment.
I tried to read this, but it's a lot of code. Part of the problem is my own: my eyes glaze over at most javascript.
I guess the proof is that it works in production.
Member
Author
:-)
Okay! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is finally active in https://dev.azure.com/git-for-windows/git/_build?definitionId=20&_a=summary. It lets an Azure Pipeline look through all open PRs at https://github.com/gitgitgadget/git/pulls to figure out whether they have been integrated into any upstream branch yet. Based on this information, appropriate labels are added, information is offered in new comments, and if the patches have been integrated into either
masterormaint, the PR is closed.