1553230 - Teach moz.configure to auto-update certain tools and toolchains (e.g. clang and cbindgen)

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement, P3)

Description

[Christian Holler (:decoder)]

Attachment: rust-cbindgen-update.patch

Me and other developers had it many times now that we started a build and left for a while, only to find that the build didn't start because some dependency, e.g. rustc or cbindgen, needed an update. While the build system literally outputs the command you can run to fix the problem, it would be even nicer if we had an option like AUTOUPDATE (similar to AUTOCLOBBER) to perform this task automatically and then move on with the build.

I have made a WIP patch that does this for rustc and cbindgen (however without the AUTOUPDATE variable guarding it). That patch might be a good starting point for a general discussion on how to implement this.

Comment 1

[Christian Holler (:decoder)]

Also, with the attached patch I got the following backtrace after a successful cbindgen update (but only after a while):

2:24.84 Traceback (most recent call last):
2:24.86   File "/usr/lib/python2.7/runpy.py", line 174, in _run_module_as_main
2:24.86     "__main__", fname, loader, pkg_name)
2:24.86   File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
2:24.87     exec code in run_globals
2:24.87   File "/srv/repos/mozilla-central-2/python/mozbuild/mozbuild/action/file_generate.py", line 120, in <module>
2:24.87     sys.exit(main(sys.argv[1:]))
2:24.87   File "/srv/repos/mozilla-central-2/python/mozbuild/mozbuild/action/file_generate.py", line 71, in main
2:24.87     ret = module.__dict__[method](output, *args.additional_arguments, **kwargs)
2:24.87   File "/srv/repos/mozilla-central-2/layout/style/RunCbindgen.py", line 27, in generate
2:24.87     buildconfig.substs['CBINDGEN'],
2:24.87   File "/srv/repos/mozilla-central-2/python/mozbuild/mozbuild/backend/configenvironment.py", line 293, in __getitem__
2:24.87     raise KeyError("'%s'" % key)
2:24.87 KeyError: "'CBINDGEN'"
2:24.88 backend.mk:71: recipe for target '.deps/ServoStyleConsts.h.stub' failed
2:24.91 make[4]: *** [.deps/ServoStyleConsts.h.stub] Error 1

Comment 2

[Nick Alexander :nalexander [he/him]]

Generally I think this is a great idea that I would like to see happen. Technically, I have two thoughts:

1. it would be nice for moz.configure and mach bootstrap to share snippets for these installations. This would support the general desire to make mach bootstrap more library like; I'm sure there are tickets but I can't see one right this moment.

2. it feels like there's a template pattern here just waiting to be defined, a way to declare a check and a "fix" step that moz.configure knows how to deal with. That is, moz.configure can learn to look for a tool, try to install it if AUTOUPDATE is set, and then look for it again. I'm hoping glandium has thought about this already :)

Comment 3

[:Gijs (he/him)]

Would it be possible to get something prioritized in this bug? Like :decoder, and Mats in bug 1540533, and mconley ( https://twitter.com/mike_conley/status/1146434012975652864 ) and I suspect many others, this is a repeated annoyance for me. Especially when both rustc and cbindgen are updated in short succession so you get not 1 but 2 false starts and only the third build attempt actually works.

Comment 4

[Kris Maglione [:kmag]]

(In reply to :Gijs (he/him) from comment #3)

Especially when both rustc and cbindgen are updated in short succession so you get not 1 but 2 false starts and only the third build attempt actually works.

Two false starts that you don't find out until the very end of a very long configure script, no less.

Comment 5

[Chris Manchester (limited bugmail, email directly)]

glandium suggested he may be able to look into this.

Comment 7

[Mike Hommey [:glandium]]

Note to self, so as to not forget some: this covers:

• rustc/cargo (rust.configure, uses check_prog and post-fact validations)
• cbindgen (bindgen.configure, uses custom code around find_program)
• clang (toolchain.configure, uses check_prog and post-fact validations)
• node.js (node.configure, uses find_node_executable from mozbuild.nodeutil ; not great because that means it relies on different mechanisms to find the executable path)
• nasm (toolchain.configure, uses check_prog and post-fact validations)

rustc/cargo is the only we don't get from toolchain artifacts, although we do invite people to install cbindgen with cargo install, but that's not what mach bootstrap does.

node.js complicates things, but we have much less churn about it, so I'm tempted to leave that for a followup. All the others can be handled in a similar way.

Another followup would be to make it such that if the user is using a .mozbuild executable, we don't use the same version check, because we'd rather people use newer versions than base versions. (which is not true if they use system executables). I might actually address that in this bug, because future-proofing for that might get me far enough that there wouldn't be much left, but we'll see how it goes.

Comment 8

[Mike Hommey [:glandium]]

Also to add to the list: Android SDK and NDK.

Comment 9

[:Gijs (he/him)]

This bug appears to have stalled a bit. Is there a path forward here, and/or could we perhaps do some of this incrementally (ie add generic code + support for some of these items and then "plug in" the others in separate bugs), and if not, is there something quicker we can do at least about the dual rustup/cbindgen step (comment #3) ?

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 9066464 [details] [diff] [review]
rust-cbindgen-update.patch

PoC covering everything from comment 7 (but not comment 8) incoming.

Comment 11

[Mike Hommey [:glandium]]

Attachment: Bug 1553230 - Allow to opt-in to automatically update some bootstrapped toolchains.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c69826ee93e9
Allow to opt-in to automatically update some bootstrapped toolchains. r=firefox-build-system-reviewers,nalexander,mhentges

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f829723621f
Fix mbu failures on configure/lint.py. r=glandium

Comment 14

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/c69826ee93e9
https://hg.mozilla.org/mozilla-central/rev/9f829723621f

Comment 15

[Rob Wu [:robwu]]

Can documentation be added for the new --enable-bootstrap flag to improve its discoverability?

Comment 16

[Mike Hommey [:glandium]]

I've left it out of documentation intentionally because it's at the experimentation stage. I'll be sending a message to dev-platform later to get people to test it.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Mike, is there any strong reason this option is only available with developer_options? It's a bit weird having to remember to remove it if you're doing e.g. an --enable-release build for profiling.

Comment 18

[Emilio Cobos Álvarez (:emilio)]

(see above)

Comment 19

[Mike Hommey [:glandium]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)

Mike, is there any strong reason this option is only available with developer_options? It's a bit weird having to remember to remove it if you're doing e.g. an --enable-release build for profiling.

The fact that !developer_options currently means that the bootstrap paths are not looked up for would make it non-obvious why --enable-bootstrap updates stuff but doesn't pick them up in that configuration. I'd rather people not use --enable-release for profiling purposes. I made rust default to opt-level=2 in #1689284, please report anything else you're using --enable-release for that cannot currently be enabled by hand.