Closed
Bug 1274655
Opened 6 years ago
Closed 6 years ago
SourceRepository incorrect in application.ini
Categories
(Release Engineering :: Applications: MozharnessCore, defect)
Release Engineering
Applications: MozharnessCore
Tracking
(firefox49 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: gps, Assigned: gps)
References
Details
Attachments
(2 files)
From bug 1274450 comment #18, apparently the SourceRepository in application.ini is incorrect. This is plausible and likely fallout from the work in bug 1270317 to use a single repository store instead of 1 clone/store for each source repo. My guess is the in-tree code for populating SourceRepository is looking at `hg config paths` or something. The value should be coming from mozharness.
Assignee | ||
Comment 1•6 years ago
|
||
Recent changes to mozharness in bug 1270317 started using pooled shared storage for Mercurial repos. This means the "default" path in Mercurial repos is variable depending on which repo was the first to be built on a machine. By default, the Firefox build system resolves the source repository from `hg paths default`. This is now incorrect default behavior in automation. We fix the regression by setting MOZ_SOURCE_REPO in the environment to path to the repository that mozharness is currently building. Review commit: https://reviewboard.mozilla.org/r/54318/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54318/
Attachment #8754977 -
Flags: review?(jlund)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → gps
Status: NEW → ASSIGNED
Updated•6 years ago
|
Attachment #8754977 -
Flags: review?(jlund) → review+
Comment 3•6 years ago
|
||
Comment on attachment 8754977 [details] MozReview Request: Bug 1274655 - Set MOZ_SOURCE_REPO in mozharness to avoid variance from shared repos; r?jlund https://reviewboard.mozilla.org/r/54318/#review51544
Assignee | ||
Comment 6•6 years ago
|
||
Comment on attachment 8754977 [details] MozReview Request: Bug 1274655 - Set MOZ_SOURCE_REPO in mozharness to avoid variance from shared repos; r?jlund Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54318/diff/1-2/
Assignee | ||
Comment 7•6 years ago
|
||
Comment on attachment 8754977 [details] MozReview Request: Bug 1274655 - Set MOZ_SOURCE_REPO in mozharness to avoid variance from shared repos; r?jlund This broke production. Made a minor change. Rerequesting review.
Attachment #8754977 -
Flags: review+ → review?(jlund)
Assignee | ||
Comment 9•6 years ago
|
||
Previously, we required both or none of MOZ_SOURCE_REPO and MOZ_SOURCE_CHANGESET to be defined. This logic was established in 51029f4d82d3 (bug 1247162). There appears to be no good reason why we require MOZ_SOURCE_CHANGESET if MOZ_SOURCE_REPO is defined. After all, if we have a checkout we should be able to resolve the revision. This commit changes the logic to resolve the changeset when not defined. We still error if MOZ_SOURCE_REPO is defined but we can't resolve the changeset. I can't imagine this breaking anything. This change will be necessary to appease TaskCluster tasks once mozharness is changed in a subsequent commit to define MOZ_SOURCE_REPO. Buildbot and TC each have their own way of specifying the source revision. Rather than change mozharness, it feels easier to just have the build system derive things. This decision is further justified by the fact there is a chicken and egg problem in mozharness: the environment variable dict is resolved before source directory population. So, we'd need to teach mozharness about TC's VCS mechanism, which it currently has no knowledge of. I'd rather not do that. Review commit: https://reviewboard.mozilla.org/r/54902/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54902/
Attachment #8755966 -
Flags: review?(mshal)
Assignee | ||
Comment 10•6 years ago
|
||
Comment on attachment 8754977 [details] MozReview Request: Bug 1274655 - Set MOZ_SOURCE_REPO in mozharness to avoid variance from shared repos; r?jlund Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54318/diff/2-3/
Updated•6 years ago
|
Attachment #8754977 -
Flags: review?(jlund) → review+
Comment 11•6 years ago
|
||
Comment on attachment 8754977 [details] MozReview Request: Bug 1274655 - Set MOZ_SOURCE_REPO in mozharness to avoid variance from shared repos; r?jlund https://reviewboard.mozilla.org/r/54318/#review51574 ::: testing/mozharness/mozharness/mozilla/building/buildbase.py:855 (Diff revision 3) > env['MOZ_BUILD_DATE'] = self.query_buildid() > > + # Set the source repository to what we're building from since > + # the default is to query `hg paths` which isn't reliable with pooled > + # storage > + repo_path = self._query_repo() ah, is this because TC doesn't checkout source so we never call _query_repo ? _query_repo() should always return a value if called. this patch seems to suggest you think it might not so just want to sanity check we are on the same page.
Comment 12•6 years ago
|
||
Comment on attachment 8755966 [details] MozReview Request: Bug 1274655 - Resolve changeset when only repo is defined; r?mshal https://reviewboard.mozilla.org/r/54902/#review51596 Looks reasonable to me. I don't see a reason why we would only want both or neither of the variables to be set.
Attachment #8755966 -
Flags: review?(mshal) → review+
Assignee | ||
Comment 13•6 years ago
|
||
Comment on attachment 8754977 [details] MozReview Request: Bug 1274655 - Set MOZ_SOURCE_REPO in mozharness to avoid variance from shared repos; r?jlund Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54318/diff/3-4/
Assignee | ||
Comment 14•6 years ago
|
||
https://reviewboard.mozilla.org/r/54318/#review51574 > ah, is this because TC doesn't checkout source so we never call _query_repo ? > > _query_repo() should always return a value if called. this patch seems to suggest you think it might not so just want to sanity check we are on the same page. Oh, really good catch. I changed the code slightly to assert that _query_repo() always returns a truthy value. I think there are other code paths where repo_path isn't set because we don't call _query_repo until we're checkout out sources.
Comment 15•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/32886f06e0fb https://hg.mozilla.org/integration/mozilla-inbound/rev/1b2ea19553ca
Comment 16•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/32886f06e0fb https://hg.mozilla.org/mozilla-central/rev/1b2ea19553ca
You need to log in
before you can comment on or make changes to this bug.
Description
•