SourceRepository incorrect in application.ini

RESOLVED FIXED

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

3 years ago
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

3 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

3 years ago
Assignee: nobody → gps
Status: NEW → ASSIGNED
Assignee

Updated

3 years ago
Duplicate of this bug: 1275047
Attachment #8754977 - Flags: review?(jlund) → review+
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

3 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

3 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

3 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

3 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/
Attachment #8754977 - Flags: review?(jlund) → review+
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 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

3 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

3 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 16

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/32886f06e0fb
https://hg.mozilla.org/mozilla-central/rev/1b2ea19553ca
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.