1274655 - SourceRepository incorrect in application.ini

Status: RESOLVED FIXED

Description

[Gregory Szorc [:gps]]

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.

Comment 1

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1274655 - Set MOZ_SOURCE_REPO in mozharness to avoid variance from shared repos; r?jlund

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/

Comment 3

[Jordan Lund (:jlund)]

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

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2a4c06d9736

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d6e4c44790d2

Comment 6

[Gregory Szorc [:gps]]

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/

Comment 7

[Gregory Szorc [:gps]]

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.

Comment 8

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff8388f0333f

Comment 9

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1274655 - Resolve changeset when only repo is defined; r?mshal

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/

Comment 10

[Gregory Szorc [:gps]]

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/

Comment 11

[Jordan Lund (:jlund)]

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

[Michael Shal [:mshal]]

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.

Comment 13

[Gregory Szorc [:gps]]

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/

Comment 14

[Gregory Szorc [:gps]]

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

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/32886f06e0fb
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b2ea19553ca

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/32886f06e0fb
https://hg.mozilla.org/mozilla-central/rev/1b2ea19553ca