1357597 - Support overlaying ad-hoc Git refs into Mercurial repos

Status: ASSIGNED

(Developer Services :: General, task)

Description

[Gregory Szorc [:gps]]

We want to support integrating GitHub pull requests into Mercurial repos. This process involves:

* Optionally rewriting commits in the PR to conform with various standards. This includes possibly squashing commits together.

* Converting Git commits to Mercurial changesets

* Overlaying changes into a sub-directory of the Firefox monorepo

Aspects of these already exist. But we'll need some modifications to support certain properties of pull requests, notably the ad-hoc nature of pull requests versus the persistent nature of conversions that current vcs sync code performs.

In this bug, I'll implement functionality for incorporating ad-hoc Git refs into Mercurial repos. I may or may not scope bloat it to cover the actual GitHub pull request integration. But the code is being written with pull requests in mind, so I'll design it accordingly.

Comment 1

[Gregory Szorc [:gps]]

Attachment: vcssync: extract code for updating refs into own function (bug 1357597);

We'll soon introduce more code that wants to update refs/reflogs in a
similar manner. Let's extract the code to a reusable function to
facilitate that.

This introduces the gitutil module for holding generic, lower-level
Git code. I have additional plans for this module.

Review commit: https://reviewboard.mozilla.org/r/131422/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131422/

Comment 2

[Gregory Szorc [:gps]]

Attachment: vcssync: API and command to squash Git commits (bug 1357597)

Many pull requests on GitHub don't practice the Mozilla-preferred
style of linear, commit-level bisectable history. Furthermore,
getting people to change is hard, especially when most GitHub
projects seem to practice an "anything goes" commit authoring
convention. Rather than fight that battle, let's concede defeat
and accept that many GitHub pull requests that we'll "import"
into the Firefox monorepo will need to be heavily normalized
before they are imported.

This commit implements a brute normalization hammer in the form
of commit squashing. Given two Git refs, it computes a merge
base between the two, the set of commits in the source ref, and
produces a new, single commit representing the "squashed" result
of all the indivudal commits.

The added CLI command likely won't be used in production. Its main
purpose at this junction is to serve as an interface so tests can
be written. The important parts of this commit are the new generic
Python APIs for finding merge bases and performing a squash.
Subsequent commits will build on top of these primitives to
implement more advanced functionality.

Review commit: https://reviewboard.mozilla.org/r/131424/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131424/

Comment 3

[Gregory Szorc [:gps]]

Attachment: vcssync: extract code for testing for conversion presence (bug 1357597);

The near future will introduce another consumer that will want to test
whether the `hg convert` revision map has knowledge of specific source
commits. So we extract this functionality to a standalone function.

As part of the extraction, behavior changed slightly. We now accept
an iterable of commits whose presence to test for, not just a single
one. We also maintain a mapping of source to destination commit.
File reading is still lazy and stops as soon as all matches are
found. Both these features are currently unused but will be used in
an upcoming commit.

Review commit: https://reviewboard.mozilla.org/r/131432/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131432/

Comment 4

[Gregory Szorc [:gps]]

Attachment: vcssync: extract code for running `hg convert` to reusable function (bug 1357597);

A future commit will introduce a new consumer that wants to call
`hg convert`. Since the code is a bit involved, let's factor the
functionality to a standalone function.

This also has the benefit of making linearize_git_repo_to_hg()
significantly more readable.

Review commit: https://reviewboard.mozilla.org/r/131434/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131434/

Comment 5

[Gregory Szorc [:gps]]

Attachment: vcssync: eliminate shamap_path variable (bug 1357597);

The value is redundant with rev_map and not necessary. Given the
state of mind I was in when I wrote this code (I was rushing to
get this feature deployed), I'm not surprised this slipped through.

Review commit: https://reviewboard.mozilla.org/r/131436/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131436/

Comment 6

[Gregory Szorc [:gps]]

Attachment: vcssync: implement a context manager for monitoring an hg repo (bug 1357597);

Previously, linearize_git_repo_to_hg() had a bunch of low-level code
to grab state from a Mercurial repo before and after an operation.
This type of probing is generic and can be extracted to its own
function. Furthermore, this pattern of performing actions before
and after an event is something that context managers can do for
us.

So this commit extracts the code to a utility function that is
used as a context manager.

At this point, linearize_git_repo_to_hg() is much easier to read
and most of its generic logic is in standalone functions,
facilitating reuse.

Review commit: https://reviewboard.mozilla.org/r/131438/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131438/

Comment 7

[Gregory Szorc [:gps]]

There's still some more patches coming. Likely tomorrow. I still have yet to implement the "convert and overlay to Mercurial" bits that are specific to transient PRs. That's where the fun should be.

Comment 8

[Gregory Szorc [:gps]]

Attachment: vcssync: functionality for preparing a Git ref for integration (bug 1357597)

Before importing a GitHub pull request into a Mercurial repository, we
first need to fetch the commit data so we can operate on it. We also
may need to "normalize" the commits so they conform to Mozilla
standards. For example, we don't like merges or "fixup" commits in
the final history of the Firefox repo, so if we see a pull request
with either of these, we probably just want to squash it down to a
single commit. Or more practically, we may wish to prune certain
files or directories that aren't part of the destination repository.
You could also conceive of more advanced functionality, such as
automatically filing a bug and adding a reference to it in the
commit message.

Because "pull requests" are a specific implementation of a generic
concept that is "incorporate [Git] changes from X into Y," we invent
a generic term for the work we're doing that doesn't overload the
"pull request" terminology: integration.

This pre-import "integration" work constitutes sufficient complexity
to justify having it exist as a standalone function. This commit
establishes that function.

Building on top of the work to identify a merge base and to squash
commits, we implement a function that fetches Git commits needed to
process a request to incorporate changes between 2 refs. It then
rewrites the "incoming" commits as appropriate for the (eventual)
destination repository. The rewriting is driven by 2 callbacks: 1
chooses the "shape" of the commits (currently either "preserve" or
"squash") and the other allows rewriting of commits themselves. The
latter can be used to change the commit message, etc.

After returning successfully, the function ensures that a local Git
repository contains commits for the originally requested "base" and
"head" refs and a ref for the (possibly rewritten) "integration"
commit(s). A future consumer of this function will take the
"integration" commit(s) and import them to Mercurial.

While the code is intended to be used for GitHub pull requests, it
is intentionally sufficiently generic to allow uses outside of
GitHub. All you need are 2 Git refs and some optional callbacks
to control rewriting and you can prepare any set of Git commits
for whatever is ahead.

Review commit: https://reviewboard.mozilla.org/r/131798/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131798/

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8859396 [details]
vcssync: extract code for updating refs into own function (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131422/diff/1-2/

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8859408 [details]
vcssync: extract code for testing for conversion presence (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131432/diff/1-2/

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8859409 [details]
vcssync: extract code for running `hg convert` to reusable function (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131434/diff/1-2/

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8859410 [details]
vcssync: eliminate shamap_path variable (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131436/diff/1-2/

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8859411 [details]
vcssync: implement a context manager for monitoring an hg repo (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131438/diff/1-2/

Comment 14

[Gregory Szorc [:gps]]

Comment on attachment 8859397 [details]
vcssync: API and command to squash Git commits (bug 1357597)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131424/diff/1-2/

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 8859396 [details]
vcssync: extract code for updating refs into own function (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131422/diff/2-3/

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8859408 [details]
vcssync: extract code for testing for conversion presence (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131432/diff/2-3/

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 8859409 [details]
vcssync: extract code for running `hg convert` to reusable function (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131434/diff/2-3/

Comment 18

[Gregory Szorc [:gps]]

Comment on attachment 8859410 [details]
vcssync: eliminate shamap_path variable (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131436/diff/2-3/

Comment 19

[Gregory Szorc [:gps]]

Comment on attachment 8859411 [details]
vcssync: implement a context manager for monitoring an hg repo (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131438/diff/2-3/

Comment 20

[Gregory Szorc [:gps]]

Comment on attachment 8859397 [details]
vcssync: API and command to squash Git commits (bug 1357597)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131424/diff/2-3/

Comment 21

[Gregory Szorc [:gps]]

Comment on attachment 8859810 [details]
vcssync: functionality for preparing a Git ref for integration (bug 1357597)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131798/diff/1-2/

Comment 22

[Gregory Szorc [:gps]]

Attachment: vcssync: move console scripts declaration to a list;

This will allow us to conditionally add entries depending on
flags.

Review commit: https://reviewboard.mozilla.org/r/132588/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/132588/

Comment 23

[Gregory Szorc [:gps]]

Attachment: vcssync: install nose in virtualenv;

./run-tests uses nose to run Python-based unit tests. So install it so
we can start writing them.

Review commit: https://reviewboard.mozilla.org/r/132590/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/132590/

Comment 24

[Gregory Szorc [:gps]]

Attachment: vcssync: add function for searching for original commit (bug 1357597);

When rewriting Git commits, we sometimes add an annotation recording
the original commit ID. This commit introduces functionality for
parsing that annotation to find the rewritten version of a commit.

Review commit: https://reviewboard.mozilla.org/r/132592/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/132592/

Comment 25

[Gregory Szorc [:gps]]

Attachment: INCOMPLETE vcssync: support integrating Git refs into a Mercurial repo

Review commit: https://reviewboard.mozilla.org/r/132594/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/132594/

Comment 26

[Gregory Szorc [:gps]]

Comment on attachment 8859396 [details]
vcssync: extract code for updating refs into own function (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131422/diff/3-4/

Comment 27

[Gregory Szorc [:gps]]

Comment on attachment 8859408 [details]
vcssync: extract code for testing for conversion presence (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131432/diff/3-4/

Comment 28

[Gregory Szorc [:gps]]

Comment on attachment 8859409 [details]
vcssync: extract code for running `hg convert` to reusable function (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131434/diff/3-4/

Comment 29

[Gregory Szorc [:gps]]

Comment on attachment 8859410 [details]
vcssync: eliminate shamap_path variable (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131436/diff/3-4/

Comment 30

[Gregory Szorc [:gps]]

Comment on attachment 8859411 [details]
vcssync: implement a context manager for monitoring an hg repo (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131438/diff/3-4/

Comment 31

[Gregory Szorc [:gps]]

Comment on attachment 8859397 [details]
vcssync: API and command to squash Git commits (bug 1357597)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131424/diff/3-4/

Comment 32

[Gregory Szorc [:gps]]

Comment on attachment 8859810 [details]
vcssync: functionality for preparing a Git ref for integration (bug 1357597)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131798/diff/2-3/

Comment 33

[:glob ✱]

Comment on attachment 8859396 [details]
vcssync: extract code for updating refs into own function (bug 1357597);

https://reviewboard.mozilla.org/r/131422/#review138018

Comment 34

[:glob ✱]

Comment on attachment 8859408 [details]
vcssync: extract code for testing for conversion presence (bug 1357597);

https://reviewboard.mozilla.org/r/131432/#review138020

Comment 35

[:glob ✱]

Comment on attachment 8859409 [details]
vcssync: extract code for running `hg convert` to reusable function (bug 1357597);

https://reviewboard.mozilla.org/r/131434/#review138022

Comment 36

[:glob ✱]

Comment on attachment 8859410 [details]
vcssync: eliminate shamap_path variable (bug 1357597);

https://reviewboard.mozilla.org/r/131436/#review138024

Comment 37

[:glob ✱]

Comment on attachment 8859411 [details]
vcssync: implement a context manager for monitoring an hg repo (bug 1357597);

https://reviewboard.mozilla.org/r/131438/#review138026

Comment 38

[:glob ✱]

Comment on attachment 8860576 [details]
vcssync: move console scripts declaration to a list;

https://reviewboard.mozilla.org/r/132588/#review138028

Comment 39

[:glob ✱]

Comment on attachment 8859397 [details]
vcssync: API and command to squash Git commits (bug 1357597)

clearing reviews as parts of this are WIP.
re-request when it's all good to go.

Comment 40

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

This is likely useful for wpt-sync as well, so adding Bug 1364561.

Comment 41

[Gregory Szorc [:gps]]

I've got a refactor in a good state locally. But it isn't in shape for review yet.

I'm heads down the next few days polishing up a Firefox developer survey so we can conduct that before the SF all hands. I hope to have something up for review on this bug by the end of the week.

Comment 42

[Gregory Szorc [:gps]]

Attachment: vcssync: support for squashing a Git ref into a Mercurial repo (bug 1357597);

It is common to want to "import a GitHub pull request" into the Firefox
monorepo where the pull request comes from a Git repo that is tracking
a sub-directory of the Firefox monorepo.

This commit implements a primitive that will simulate a `git merge` and
then copy the end state to form a Mercurial changeset.

While some of this logic could be abstracted out, per discussion with
glob, we've decided to implement something basic first. Once we know
other variations that we need to build, we can refactor the code and
abstract things as necessary.

Review commit: https://reviewboard.mozilla.org/r/146384/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146384/

Comment 43

[Gregory Szorc [:gps]]

Attachment: vcssync: context manager for temporarily mutating Git refs (bug 1357597);

We often perform operations that want to temporarily mutate the state
of a repo then restore the repo to a pristine state.

Let's create a context manager to facilitate restoring refs on a Git
repo.

To implement this as robustly as possible, we add a "force-update"
action to our update_git_refs() helper. We shouldn't need this because
we have the old value available. But there is a very narrow race
condition where things could fail. Given the nature of the code that
is running, it seems prudent to eliminate as many failure scenarios
as possible.

Review commit: https://reviewboard.mozilla.org/r/180088/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180088/

Comment 44

[Gregory Szorc [:gps]]

Attachment: vcssync: context manager to create a temporary Git checkout (bug 1357597);

This seems like another useful primitive to establish.

Review commit: https://reviewboard.mozilla.org/r/180090/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180090/

Comment 45

[Gregory Szorc [:gps]]

Attachment: vcssync: helper function to ensure a Git commit present locally (bug 1357597);

It allows us to perform a fetch without using remotes. This isn't a
common practice in Git land and the syntax for `git fetch` is a bit
wonky. So it is best to abstract it away.

Plus, we can avoid network I/O if the given commit is present
locally.

Review commit: https://reviewboard.mozilla.org/r/180092/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180092/

Comment 46

[Gregory Szorc [:gps]]

Attachment: vcssync: utility function to copy between working directories (bug 1357597);

Synchronizing the contents of working directories is not trivial because
of ignored and untracked files. Common tools like rsync aren't aware of
VCS state and will likely copy files it shouldn't.

So, we implement our own recursive directory synchronization primitive
that is VCS aware. We only support Git to Mercurial at the moment. But
the other direction is trivial to implement.

Review commit: https://reviewboard.mozilla.org/r/180094/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180094/

Comment 47

[Gregory Szorc [:gps]]

Comment on attachment 8875030 [details]
vcssync: support for squashing a Git ref into a Mercurial repo (bug 1357597);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/146384/diff/1-2/

Comment 48

[:glob ✱]

Comment on attachment 8875030 [details]
vcssync: support for squashing a Git ref into a Mercurial repo (bug 1357597);

clearing reviews as per discussion

Comment 49

[:glob ✱]

the sync service is no longer running/needed.