Closed Bug 1240667 Opened 9 years ago Closed 9 years ago

Artifact builds don't work with anything else than fx-team

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: glandium, Assigned: chmanchester)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

While the code makes it possible to specify the tree to mach artifact install, the code in config_status.py doesn't allow to set it, so even if you do, say, mach artifact install --tree=mozilla-inbound, your build subsequently fails because config_status.py re-runs mach artifact install without --tree=mozilla-inbound, which then tries to find the build for your changeset on fx-team.
And same with mach_commands.py (which is why I wanted _run_mach_artifact_install to be used from config_status.py).
Pretty much. I think the correct way to address this is to grow a ~/.machrc or similar to configure artifacts out of band. As we move towards caching objects and libraries in the regular build system, it won't be feasible to specify the tree with every build invocation.
I've wanted a global mach configuration file for the longest time. There is even crude support for mach config files in mach itself.
I think we can reasonably try to find what the tree is, instead of leaving that to the user config, which would have to change if they switch trees and some such.
(In reply to Mike Hommey [:glandium] from comment #4) > I think we can reasonably try to find what the tree is, instead of leaving > that to the user config, which would have to change if they switch trees and > some such. I didn't know how to do this when I thought about it. On merge commits, central, fx-team, and mozilla-inbound may refer to the same commit, and I can't really think of a way to disambiguate fx-team and mozilla-inbound. Granted, such commits are relatively rare, and in this situation, it doesn't really matter which tree we fetch from.
mozext installs some template keywords to identify which tree(s) a changeset is known to be in. $ hg log -T '{node|short} {join(trees, ", ")}\n' 66bce0b7c669 inbound c12b2d098ae0 inbound 9c48dbddeeb6 inbound 67c94e29bcac inbound b67316254602 central, inbound 20fdd2a2344e inbound, central 2007ddbc08c9 inbound, central 77ce3012d481 inbound, central 5b8303030d9c inbound, central ffac58123eb0 inbound, central c96da719d455 inbound, central 154d22a2a34c inbound, central f89869a36b16 inbound, central There is also {firstpushtree} that identifies the repo a changeset was first pushed to chronologically. You could use this to break ties to maximize your chances for finding artifacts. See also `hg help -e mozext` for the full list of template keywords. There are a lot. I think this will help solve your problem.
The thing that tripped me here is that autoland only works with inbound right now so I started using that as my development tree not realising I was hosing myself with artifact.
Assignee: nobody → cmanchester
Currenlty --enable-artifact builds will take artifacts from fx-team regardless of the state of the current working directory. This can lead to broken builds if someone updates to a tree other than fx-team. This commit changes the default behavior from tracking fx-team to tracking a tree associated with a recent ancestor of the working parent. The first tree a commit appeared in for the first commit that appeared in a tree is taken, but "try" is ignored as a candidate tree. This will generally select the tree last updated to. This also fixes a mis-match between tree names according to mozext and branch names in the taskcluster index preventing artifact download from common integration branches. Review commit: https://reviewboard.mozilla.org/r/32213/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/32213/
Attachment #8711497 - Flags: review?(nalexander)
Comment on attachment 8711497 [details] MozReview Request: Bug 1240667 - Detect a tree to use for artifact builds based on recent changesets. r=nalexander https://reviewboard.mozilla.org/r/32213/#review28831 I'd like to think deeper about what we want to have happen here. Can we do this in a single `hg log`? There's an oddity here in that a tree that gets infrequent pushes, like `cedar`, might "steal" the install chain. That is, if we have sha1 "Pushed to cedar" sha2 "Pushed to fx-team" ... sha1000 "Pushed to fx-team" and the cedar sha1 doesn't have the right artifacts built, then we'll ignore all the candidate fx-team pushes that *do* have the right artifacts built. If we instead folded the trees calculation into the existing log, we might just take the newest artifacts, regardless of what tree they were built on. I thought about doing this when I first wrote this code, and I think it makes more sense. So, in the case where a tree isn't specified, look for anything with a push to "fx-team, inbound, central" and use that. What say you? ::: python/mozbuild/mozbuild/artifacts.py:565 (Diff revision 1) > + tree = self._tree_replacements.get(tree, tree) nit: `.get(tree, tree)` took my a second to parse -- consider `.get(tree) or tree`. ::: python/mozbuild/mozbuild/artifacts.py:675 (Diff revision 1) > + 'Retrieving artifacts based on tree: {tree}') "based on"? Why not "from tree"? Is there some subtlety we're trying to convey? ::: python/mozbuild/mozbuild/mach_commands.py:1471 (Diff revision 1) > - def _compute_defaults(self, tree=None, job=None): > + def _find_tree(self, hg): I'd like to move this functionality into `artifacts.py`, so that potential non-`mach` consumers can access it. Also, consider unifying some of the error handling with what's already there, perhaps by adding exception types for the two situations, and then catching those exceptions and providing the fine-grained feedback we want in each case. ::: python/mozbuild/mozbuild/mach_commands.py:1484 (Diff revision 1) > + [hg, 'log', '-f', '--template', '{join(trees, ",")}\n', This dramatically changes the execution profile of `mach artifact install`: we've doubled the number of `hg` invocations, and this one isn't even cached! This needs more discussion.
Attachment #8711497 - Flags: review?(nalexander)
https://reviewboard.mozilla.org/r/32213/#review28831 I see the oddity, and I think this could impact people working from central as well. To do this right we'll need build in an idea of how to check for artifacts across multiple trees, which I think will require re-structuring much of the current interaction with version control, although that might not be too bad. > This dramatically changes the execution profile of `mach artifact install`: we've doubled the number of `hg` invocations, and this one isn't even cached! This needs more discussion. We can't necessarily cache the trees a revision might be associated with. Maybe that isn't going to end up being a problem, but let's revisit this after the modifications above.
Attachment #8711497 - Flags: review?(nalexander)
Comment on attachment 8711497 [details] MozReview Request: Bug 1240667 - Detect a tree to use for artifact builds based on recent changesets. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32213/diff/1-2/
Comment on attachment 8711497 [details] MozReview Request: Bug 1240667 - Detect a tree to use for artifact builds based on recent changesets. r=nalexander https://reviewboard.mozilla.org/r/32213/#review29623 This breaks --tree. I'd like to either repair it, if it's easy, or remove that functionality (parameter and `_tree` member field, as appropriate) entirely. (We can add it back in follow-up if it's desireable.) Otherwise, rock on! ::: python/mozbuild/mozbuild/artifacts.py:510 (Diff revision 2) > - pushheads = subprocess.check_output([self._hg, 'log', > + result = self._index.listNamespaces(rev_ns, {"limit": 10}) Is this safe? I feel there are more than 10 namespaces in an "average" push. Is there a reason to not be flexible here, and take maybe 100? ::: python/mozbuild/mozbuild/artifacts.py:658 (Diff revision 2) > + # an exact match. Can you be more precise, like: ``` # an exact match. For example, |mach artifact install --tree=central| should map to |hg log -r 'pushhead(central)|', which corresponds to the Task Cluster namespaces "mozilla-central". ``` Or whatever is appropriate. ::: python/mozbuild/mozbuild/artifacts.py:693 (Diff revision 2) > + rev_info = line.split(',') Explain why you can't have "sha," here, which results in `rev_trees['sha'] = ('', )`, which seems wrong. Is it because `trees` is non-empty if `pushhead()` is true? That seems reasonable, but I'd still like to be defensive here. ::: python/mozbuild/mozbuild/artifacts.py:704 (Diff revision 2) > + def find_pushead_artifacts(self, task_cache, tree_cache, job, pushhead, trees): nit: "pushhead", with two 'h' characters.
Attachment #8711497 - Flags: review?(nalexander) → review+
https://reviewboard.mozilla.org/r/32213/#review29623 Will repair. Thank you for noticing that. > Is this safe? I feel there are more than 10 namespaces in an "average" push. Is there a reason to not be flexible here, and take maybe 100? I think this is safe. The namespaces under buildbot.revision.{rev} are trees -- finding multiple entries there equates to a revision being a pushhead on multiple trees. It seems like more than one would be quite rare. > Explain why you can't have "sha," here, which results in `rev_trees['sha'] = ('', )`, which seems wrong. > > Is it because `trees` is non-empty if `pushhead()` is true? That seems reasonable, but I'd still like to be defensive here. Yes, it looks like trees will be non-empty if pushhead() is true, but I see no reason not to be defensive.
Comment on attachment 8711497 [details] MozReview Request: Bug 1240667 - Detect a tree to use for artifact builds based on recent changesets. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32213/diff/2-3/
Comment on attachment 8711497 [details] MozReview Request: Bug 1240667 - Detect a tree to use for artifact builds based on recent changesets. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32213/diff/3-4/
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: