1253203 - Move parts of configure.py into sandboxed moz.configure

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Configure.py currently handles reading the mozconfig to find autoconf, run autoconf, and execute the old configure.
Most of this should move into sandboxed moz.configure.

Comment 1

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1253203 - Normalize path separators in MozbuildObject

The upcoming move of the configure.py initialization to sandboxed
moz.configure changes the path separators for topsrcdir and topobjdir
from native to always use forward-slashes, which confuses the hell out
of the test_base.py test. Settle the issue by declaring that
MozbuildObject will always use forward-slashed paths for topsrcdir
and topobjdir.

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

Comment 2

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1253203 - Use mozpath functions for sandboxed os.path in configure.py

Generally speaking, the configuration needs forward-slashes in paths.
We might as well make it hard(er) to set configuration items with
backslash separators on Windows by exposing mozpath.* functions in place
of os.path functions. The downside is that functions explicitly
importing os will still get the real os.path functions.

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

Comment 3

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1253203 - Remove everything COMM_BUILD

comm-central doesn't set it since bug 1040009.

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

Comment 4

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1253203 - Move parts of configure.py into sandboxed moz.configure

This moves all the reading mozconfig, finding autoconf, refreshing the
old configure, and running the old configure into sandboxed
moz.configure. This effectively bootstraps the sandboxed python configure.

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

Comment 5

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8726622 [details]
MozReview Request: Bug 1253203 - Use mozpath functions for sandboxed os.path in configure.py

https://reviewboard.mozilla.org/r/38109/#review34867

Comment 6

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8726623 [details]
MozReview Request: Bug 1253203 - Remove everything COMM_BUILD

https://reviewboard.mozilla.org/r/38111/#review34869

Comment 7

[Chris Manchester (limited bugmail, email directly)]

https://reviewboard.mozilla.org/r/38107/#review34875

::: python/mozbuild/mozbuild/test/test_base.py:63
(Diff revision 1)
> -            self.assertTrue(base.topobjdir.startswith(topsrcdir))
> +            print(base.topobjdir, base.topsrcdir)

Left over print?

Comment 8

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8726622 [details]
MozReview Request: Bug 1253203 - Use mozpath functions for sandboxed os.path in configure.py

https://reviewboard.mozilla.org/r/38109/#review34877

Comment 9

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8726623 [details]
MozReview Request: Bug 1253203 - Remove everything COMM_BUILD

https://reviewboard.mozilla.org/r/38111/#review34879

Comment 10

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8726624 [details]
MozReview Request: Bug 1253203 - Move parts of configure.py into sandboxed moz.configure

https://reviewboard.mozilla.org/r/38113/#review34873

This is a mind-bending patch.

::: build/moz.configure/init.configure:71
(Diff revision 1)
> +def mozconfig(current_project, mozconfig,  build_env, help):

Why do you depend on, and then not use, `help`?

::: configure.py:32
(Diff revision 1)
> -        for k, v in raw_config['substs']
> +        if k not in ('DEFINES', 'non_global_defines', 'TOPSRCDIR', 'TOPOBJDIR')

nit: indentation should be improved here.

Comment 11

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8726624 [details]
MozReview Request: Bug 1253203 - Move parts of configure.py into sandboxed moz.configure

https://reviewboard.mozilla.org/r/38113/#review35111

::: build/moz.configure/init.configure:89
(Diff revision 1)
> +@advanced
> +def command_line_helper():

Why does this need @advanced?

::: build/moz.configure/init.configure:100
(Diff revision 1)
> +        helper = command_line_helper()

Can we inline this?

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8726621 [details]
MozReview Request: Bug 1253203 - Normalize path separators in MozbuildObject

https://reviewboard.mozilla.org/r/38107/#review35179

Looks good aside from the print() chmanchester also found.

Comment 13

[Mike Hommey [:glandium]]

https://reviewboard.mozilla.org/r/38113/#review35111

> Why does this need @advanced?

without @advanced, this fails with: RuntimeError: restricted attribute

> Can we inline this?

Unfortunately no, because @depends functions don't expose depends, or any other function that would allow the same kind of trick to access the CommandLineHelper.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f39bfd0598db
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6f2dc4b7709
https://hg.mozilla.org/integration/mozilla-inbound/rev/671a3f345959
https://hg.mozilla.org/integration/mozilla-inbound/rev/e654e71b61b5

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f39bfd0598db
https://hg.mozilla.org/mozilla-central/rev/b6f2dc4b7709
https://hg.mozilla.org/mozilla-central/rev/671a3f345959
https://hg.mozilla.org/mozilla-central/rev/e654e71b61b5

Comment 16

[Blake Winton (:bwinton) (:☕️)]

This seems to have broken builds on Mac OS X with homebrew's autoconf213.
I specify:
  mk_add_options AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213
in my .mozconfig, and get the error:
  js/src> Could not find autoconf 2.13

There's also a typo on https://hg.mozilla.org/mozilla-central/rev/e654e71b61b5#l4.53
  if find:
should be
  if fink:

Comment 17

[Blake Winton (:bwinton) (:☕️)]

Maybe we can integrate something like https://bug1250294.bmoattachments.org/attachment.cgi?id=8724127 so that all the homebrew users won't need to specify AUTOCONF in our .mozconfigs, and old.configure will just find it automatically?  Please?

Comment 18

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Please file a followup?