Right now, artifacts.py requires hg with the `pushlog` extension: https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/artifacts.py#364. This ticket tracks making this work with git. The hard parts here are likely to be maintaining the upstream pushlog locally, and converting upstream push SHAs (which are hg) into local git commits SHAs. I believe there's a mapping service endpoint for this, but I don't know the API or how well it will perform in practice.

https://api.pub.build.mozilla.org/mapper/gecko-dev/mapfile/full contains the SHA-1 map for the gecko-dev Git repo. See https://wiki.mozilla.org/ReleaseEngineering/Applications/Mapper for the full API, which supports querying individual SHA-1s. FWIW, I've considered exposing these Git SHA-1s (complete with URLs to git.mo and/or GitHub) on hg.mo. If that makes things easier for you, let me know. We could potentially expose them through the pushlog on hg.mo as well. Of course, git-cinnabar has its own set of SHA-1s and there is a different mechanism to do the SHA-1 lookup there. So when you say "work with git" there are 2 "gits" that need to be implemented.

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

Hotel WiFi inspires many things. Review commit: https://reviewboard.mozilla.org/r/36553/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/36553/

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

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

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

Just a little clean-up and simplification. Review commit: https://reviewboard.mozilla.org/r/36561/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/36561/

ahunt: testing of this feature in a git repo would be appreciated. It should work more or less exactly as it does in an hg repo.

FYI, I just applied these patches in my local tree and whereas I couldn't get the pushlog stuff to work, this worked great. Fastest build ever. Nice work.

Sorry, this was mercurial, not git.

Comment on attachment 8723437 [details] MozReview Request: Bug 1234913 - Pre: Allow to --skip-cache when running |mach artifact install|. r?chmanchester https://reviewboard.mozilla.org/r/36551/#review33191 I will use this.

Comment on attachment 8723438 [details] MozReview Request: Bug 1234913 - Pre: Show download progress during |mach artifact install|. r?chmanchester https://reviewboard.mozilla.org/r/36553/#review33193 ::: python/mozbuild/mozbuild/artifacts.py:643 (Diff revision 1) > + # Python doesn't make it easy to capture a variable outside of the > + # function scope. last[0] is our last printed value. > + last = [-1] This is probably the most objectionable thing about python I've seen so far. Making this a property on self would be a little easier on the eyes later on.

Comment on attachment 8723439 [details] MozReview Request: Bug 1234913 - Part 1: Clarify things that are hg-specific; make iteration lazy. r?chmanchester https://reviewboard.mozilla.org/r/36555/#review33203 This is an improvement independent of later revisions.

Comment on attachment 8723440 [details] MozReview Request: Bug 1234913 - Part 2: Support git in |mach artifact install|. r?chmanchester https://reviewboard.mozilla.org/r/36557/#review33205 As discussed irl this exposes a hazard when querying many revisions that don't have artifacts yet, so we're going to re-think this.

Comment on attachment 8723441 [details] MozReview Request: Bug 1234913 - Post: Stop using local pushlog in |mach artifact install|. r?chmanchester https://reviewboard.mozilla.org/r/36559/#review33207

Comment on attachment 8723442 [details] MozReview Request: Bug 1234913 - Post: Unify trees and known_trees; remove explicit trees. r?chmanchester https://reviewboard.mozilla.org/r/36561/#review33209

https://reviewboard.mozilla.org/r/36557/#review33225 ::: python/mozbuild/mozbuild/artifacts.py:784 (Diff revision 1) > + def _find_git_pushheads(self, rev): There is a mozversioncontrol Python package under python/. I'd like to see the version control interaction code live there to prevent duplication across the tree. ::: python/mozbuild/mozbuild/artifacts.py:799 (Diff revision 1) > + self._git, 'rev-list', '--ancestry-path', You want `--topo-order` here. ::: python/mozbuild/mozbuild/mach_commands.py:1477 (Diff revision 1) > + self.virtualenv_manager.install_pip_package('mercurial==3.7.1') Oy. This could cause pain. git-cinnabar already has hg installed. Why is this needed?

https://reviewboard.mozilla.org/r/36559/#review33229 ::: python/mozbuild/mozbuild/artifacts.py:767 (Diff revision 1) > - yield rev_info[0], tuple(rev_info[1:]) > + yield (hg_hash, trees) Nit: don't need () when returning tuples.

https://reviewboard.mozilla.org/r/36557/#review33225 > Oy. This could cause pain. git-cinnabar already has hg installed. Why is this needed? I'm about to file a bug because `install_pip_package` appears broken on multiple machines, so please explain more. The only Python that it makes sense to use to run cinnabar is the virtualenv Python; and it definitely does *not* have mercurial installed, so what do you propose?

(In reply to Nick Alexander :nalexander from comment #19) > The only Python that it makes sense to use to run cinnabar is the virtualenv Python; and it definitely > does *not* have mercurial installed, so what do you propose? Huh? If you run "git cinnabar" it's *very* unlikely to be using the virtualenv python.

I volunteered to see this through.

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

Hotel WiFi inspires many things. Review commit: https://reviewboard.mozilla.org/r/37261/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/37261/

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

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

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

Comment on attachment 8725026 [details] MozReview Request: Bug 1234913 - Pre: Allow to --skip-cache when running |mach artifact install|. r=chmanchester https://reviewboard.mozilla.org/r/37259/#review33845 This is Nick's code, already reviewed.

Comment on attachment 8725028 [details] MozReview Request: Bug 1234913 - Clarify things that are hg-specific; make iteration lazy. r=chmanchester https://reviewboard.mozilla.org/r/37263/#review33847 This is Nick's code, already reviewed.

Comment on attachment 8725027 [details] MozReview Request: Bug 1234913 - Pre: Show download progress during |mach artifact install|. r=nalexander https://reviewboard.mozilla.org/r/37261/#review33853 Sure, this was your review comment. Right?

https://reviewboard.mozilla.org/r/37261/#review33853 Yep. I also noticed we need to guard dl.set_progress in case dl is None.

Comment on attachment 8725029 [details] MozReview Request: Bug 1234913 - Stop using local pushlog in |mach artifact install|. r=nalexander https://reviewboard.mozilla.org/r/37265/#review33855 Nice work! I was always trying to do this in one changeset-based query, and couldn't get it to work. Did you try `from=HASH1&to=HASH2` or whatever the arguments are, and find it didn't work? I have one larger request: could you log as you're making each request and iterating each result so we can see more of the work? (Could be DEBUG level logging, if you see fit.) ::: python/mozbuild/mozbuild/artifacts.py:563 (Diff revision 1) > + if req.status_code >= 400: Why not outside of `[200, 299]`? ::: python/mozbuild/mozbuild/artifacts.py:573 (Diff revision 1) > + end = pushid Having the constant baked in here makes the function much less clear. It doesn't appear to impact the cache, so can you just put `start, end` in the parameters? ::: python/mozbuild/mozbuild/artifacts.py:768 (Diff revision 1) > - def _find_hg_pushheads(self): > + def _pushheads_from_rev(self, rev, count): The comment is a little unclear. The "contains a push introducing `rev`" is per tree? And then you return that pushhead, and `count - 1` before that? ::: python/mozbuild/mozbuild/artifacts.py:769 (Diff revision 1) > - """Return an iterator of (hg_hash, {tree-set}) associating hg revision > + """Queries the hg.m.o's json-pushlog for the last `count` pushhead revisions nit: no sense abbreviating -- might as well help somebody who's searching. Although I guess they'll see the actual URLs... Also, drop "the". (Or "hg.mo.o's".) ::: python/mozbuild/mozbuild/artifacts.py:809 (Diff revision 1) > + last_revs = self._get_recent_revisions() Let's be clear that `_get_recent_revisions` is *public* or *remote* revisions.

Comment on attachment 8725030 [details] MozReview Request: Bug 1234913 - Support git in |mach artifact install|. r=nalexander https://reviewboard.mozilla.org/r/37267/#review33863 Not much of substance here -- just some nits and tidying up. Usually I would stamp ship it, but I'm trying to break my habit of doing so to prepare for a more security conscious me; and because I want to test this particular implementation locally. Great work! ::: python/mozbuild/mozbuild/artifacts.py:809 (Diff revision 1) > + cinnabar, 'git2hg' This approach *definitely* didn't work for me locally; I will test now. ::: python/mozbuild/mozbuild/artifacts.py:968 (Diff revision 1) > def install_from_hg_revset(self, revset, distdir): This name is now misleading, in that it doesn't install from revsets if we're using git. Can you reconsider the callers and either split the function or weaken the name? ::: python/mozbuild/mozbuild/artifacts.py:975 (Diff revision 1) > + if len(revset) != 40: I feel like we should be able to git-ify the underlying idea, allowing named references, etc. `git rev-parse` would do it, wouldn't?

https://reviewboard.mozilla.org/r/37267/#review33863 Local testing in hg and git went well. I uncovered a weirdness around importing using cinnabar that exposes a weakness in our approach (namely, cinnabar doesn't accurately reflect hg's published state; we're using all 0s as a proxy) but it's not something we can or should addres.

https://reviewboard.mozilla.org/r/37265/#review33855 Thanks for the super speedy review! I didn't try the fromchange/tochange arguments, because of an earlier version of this where I wasn't assuming anything about how many revisions might be in a push. Now that I'm using a fixed range I could, but the result would be about the same.

https://reviewboard.mozilla.org/r/37267/#review33863 Can you say a bit more about this? In this implementation we're strongly assuming that the first changeset we have locally that's known to hg will also be known to automation. If this is a bad assumtion under some circumstances we need to accomodate that.

Comment on attachment 8725026 [details] MozReview Request: Bug 1234913 - Pre: Allow to --skip-cache when running |mach artifact install|. r=chmanchester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37259/diff/1-2/

Comment on attachment 8725027 [details] MozReview Request: Bug 1234913 - Pre: Show download progress during |mach artifact install|. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37261/diff/1-2/

Comment on attachment 8725028 [details] MozReview Request: Bug 1234913 - Clarify things that are hg-specific; make iteration lazy. r=chmanchester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37263/diff/1-2/

Comment on attachment 8725029 [details] MozReview Request: Bug 1234913 - Stop using local pushlog in |mach artifact install|. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37265/diff/1-2/

Comment on attachment 8725030 [details] MozReview Request: Bug 1234913 - Support git in |mach artifact install|. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37267/diff/1-2/

https://reviewboard.mozilla.org/r/37267/#review33863 Sure. My local hg clone, `gecko`, confuses cinnabar. Therefore I have a second `gecko-clean` that has no local changes. To apply from RB, I pull into `gecko-clean` and then use cinnabar to pull into `gecko.git`. (Convoluted, I know -- but I don't know how to pull a specific RB reference in cinnabar. It is possible, I am told.) The commits in `gecko-clean` that came from RB are not public, since RB is not publishing. But cinnabar seems them as coming from an external repository, hence has an hg hash for them; and that's the bit that we switch on. Of course, such a commit has never been pushed to a known repository. (Well, until we generalize to try, which is dicy for other reasons.) It's possible cinnabar knows about hg's phase state, and we could do better, but that is *definitely* followup :)

https://reviewboard.mozilla.org/r/37265/#review33995 ::: python/mozbuild/mozbuild/artifacts.py:810 (Diff revisions 1 - 2) > + to be known to mozilla automation. nit: capital M. (Through-out.) ::: python/mozbuild/mozbuild/artifacts.py:839 (Diff revisions 1 - 2) > + 'Search started with {rev}, which must be known to mozilla automation.\n\n' nit: capital M.

Comment on attachment 8725030 [details] MozReview Request: Bug 1234913 - Support git in |mach artifact install|. r=nalexander https://reviewboard.mozilla.org/r/37267/#review33997 Nice! Let's get a tester or two to try this before we post to dev.*. Great work! ::: python/mozbuild/mozbuild/artifacts.py:995 (Diff revisions 1 - 2) > - revision = revset > + if len(revision.split('\n')) != 1: I feel it's worth throwing if `git2hg` gives all 0s here, since it'll be common.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f1dc3fbce25d https://hg.mozilla.org/integration/mozilla-inbound/rev/1b7bcd5eefb3 https://hg.mozilla.org/integration/mozilla-inbound/rev/205f1459dcb8 https://hg.mozilla.org/integration/mozilla-inbound/rev/ac1fff91cf4d https://hg.mozilla.org/integration/mozilla-inbound/rev/643e7face8c8

https://hg.mozilla.org/mozilla-central/rev/f1dc3fbce25d https://hg.mozilla.org/mozilla-central/rev/1b7bcd5eefb3 https://hg.mozilla.org/mozilla-central/rev/205f1459dcb8 https://hg.mozilla.org/mozilla-central/rev/ac1fff91cf4d https://hg.mozilla.org/mozilla-central/rev/643e7face8c8

(In reply to Nick Alexander :nalexander from comment #43) > Comment on attachment 8725030 [details] > MozReview Request: Bug 1234913 - Support git in |mach artifact install|. > r=nalexander > > https://reviewboard.mozilla.org/r/37267/#review33997 > > Nice! > > Let's get a tester or two to try this before we post to dev.*. Great work! > It worked for me on a fresh pull of fx-team, building fennec. (This was on a build where nalexander manually fixed my virtualenv last week, I've not tried a completely from scratch build yet.)

So what's necessary to get this running with git-cinnabar? For me it still fails with the latest code on inbound in artifact install. Getting https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds updated would be nice too.