1410475 - Add support for detecting whether c-c is topdir in mozconfigs.

Status: RESOLVED FIXED

(Thunderbird :: Build Config, enhancement)

Description

[Tom Prince [:tomprince]]

We are transitioning comm-central (in particular, Thunderbird) from building as a parent directory of mozilla-central to as a subdirectory of it.

In order to have mozconfig files that support building in either configuration during the transition, without duplicating the logic in every mozconfig file, there needs to exist a file that exists at the same path in mozilla-central and comm-central.

Comment 1

[Tom Prince [:tomprince]]

Attachment: Bug 1410475: Add support for detecting whether c-c is topdir in mozconfigs;

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

Comment 2

[Tom Prince [:tomprince]]

Attachment: Bug 1410475: Add support for detecting whether c-c is topdir in mozconfigs;

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

Comment 3

[Tom Prince [:tomprince]]

Comment on attachment 8920656 [details]
Bug 1410475: Add support for detecting whether c-c is topdir in mozconfigs;

https://reviewboard.mozilla.org/r/191630/#review196838

::: mail/config/mozconfigs/linux64/nightly:1
(Diff revision 1)
> +. "$topsrcdir/build/mozconfig.comm-support"

I've only changed this one mozconfig since it is the only one I've got setup to test building with m-c as  topdir. I'll follow this up with patches that refactor our mozconfigs.

Comment 4

[Michael Shal [:mshal]]

Comment on attachment 8920655 [details]
Bug 1410475: Add support for detecting whether c-c is topdir in mozconfigs;

https://reviewboard.mozilla.org/r/191636/#review197828

::: build/mozconfig.comm-support:22
(Diff revision 1)
> +# mozconfig.
> +
> +
> +# Note that the top-level mozconfig file is in $2.
> +
> +if [ "$(dirname "$2")" == "$topsrcdir" ]; then

The extra quoting here around "$2" is odd - just "$(dirname $2)" should suffice.

::: build/mozconfig.comm-support:22
(Diff revision 1)
> +# mozconfig.
> +
> +
> +# Note that the top-level mozconfig file is in $2.
> +
> +if [ "$(dirname "$2")" == "$topsrcdir" ]; then

We should probably use = instead of == since the latter is a bash extension.

::: build/mozconfig.comm-support:36
(Diff revision 1)
> +    . "$topsrcdir/comm/build/mozconfig.comm-support"
> +  else
> +    echo "ERROR: Unknown build directory layout."
> +    exit 1
> +  fi
> +elif [ "$(dirname "$2")" == "$(dirname "$topsrcdir")" ]; then

Similarly here, just have a single set of quotes like "$(dirname $2)" and "$(dirname $topsrcdir)"

::: build/mozconfig.comm-support:36
(Diff revision 1)
> +    . "$topsrcdir/comm/build/mozconfig.comm-support"
> +  else
> +    echo "ERROR: Unknown build directory layout."
> +    exit 1
> +  fi
> +elif [ "$(dirname "$2")" == "$(dirname "$topsrcdir")" ]; then

And replace == with = here.

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 8920656 [details]
Bug 1410475: Add support for detecting whether c-c is topdir in mozconfigs;

https://reviewboard.mozilla.org/r/191632/#review197836

Comment 6

[Tom Prince [:tomprince]]

Comment on attachment 8920655 [details]
Bug 1410475: Add support for detecting whether c-c is topdir in mozconfigs;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/191636/diff/1-2/

Comment 7

[Tom Prince [:tomprince]]

Comment on attachment 8920655 [details]
Bug 1410475: Add support for detecting whether c-c is topdir in mozconfigs;

https://reviewboard.mozilla.org/r/191636/#review197828

> The extra quoting here around "$2" is odd - just "$(dirname $2)" should suffice.

I put the extra quotes in because [shellcheck](https://github.com/koalaman/shellcheck) was complaining about it. It could be (but hopefully usually isn't) that `$2` has spaces in it, in which case the quoting is correct. I imagine lots of other stuff might break, though.

Comment 8

[Michael Shal [:mshal]]

(In reply to Tom Prince [:tomprince] from comment #7)
> > The extra quoting here around "$2" is odd - just "$(dirname $2)" should suffice.
> 
> I put the extra quotes in because
> [shellcheck](https://github.com/koalaman/shellcheck) was complaining about
> it. It could be (but hopefully usually isn't) that `$2` has spaces in it, in
> which case the quoting is correct. I imagine lots of other stuff might
> break, though.

Interesting, so it does. Feel free to keep it as you had it then!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7ae1c6f2a49
Add support for detecting whether c-c is topdir in mozconfigs; r=mshal

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c7ae1c6f2a49

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/539567e1f723
Add support for detecting whether c-c is topdir in mozconfigs; r=mshal

Comment 12

[Jorg K (CEST = GMT+2)]

Tom, just out of interest, why are there changes in mail/config/mozconfigs/linux64/nightly and no other config?

Comment 13

[Tom Prince [:tomprince]]

> I've only changed this one mozconfig since it is the only one I've got setup to test building with m-c as  topdir. I'll follow this up with patches that refactor our mozconfigs.

Comment 14

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8920655 [details]
Bug 1410475: Add support for detecting whether c-c is topdir in mozconfigs;

Looks good to me, thanks for working on it!

Comment 15

[Jorg K (CEST = GMT+2)]

Backed out comm part from C-B at Tom's request:
https://hg.mozilla.org/releases/comm-beta/rev/00904da56d345538e62da27458c308c275b10bde

Comment 16

[Jorg K (CEST = GMT+2)]

Relanded most of the patch, apart from the changes to mail/config/mozconfigs/linux64/nightly:
https://hg.mozilla.org/releases/comm-beta/rev/c43f5499c52482498c955966200c02507e0b50b1