Closed Bug 1399882 Opened 3 years ago Closed 2 years ago
Wrap cargo invocation in a Python script
59 bytes, text/x-review-board-request
Currently we have a bunch of make and shell logic to invoke cargo with the right set of arguments and environment variables: https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/config/rules.mk#841-1057 The syntax leaves something to be desired, given the available tooling. Additionally, I can't make this work in WSL because environment variables are not persisted to native Windows processes. Given that, I'd like to move this all into a Python script so we can express all these conditionals in Python instead of make+shell, and also give me a more useful place to inject some WSL-specific logic.
Comment on attachment 8908718 [details] bug 1399882 - Wrap cargo invocation in a Python script. https://reviewboard.mozilla.org/r/180350/#review185522 Can't see much else to do with this. This is much nicer than the Makefile-based variant, thank you! ::: python/mozbuild/mozbuild/action/cargo.py:23 (Diff revision 1) > + parser.add_argument('--lib', action='store_true', > + help='Build a library') > + parser.add_argument('--host-lib', action='store_true', > + help='Build a host library') > + parser.add_argument('--bin', nargs='*', > + help='Build a binary') > + parser.add_argument('--host-bin', nargs='*', > + help='Build a host binary') WDYT about trying to ensure that these options are all mutually exclusive? ::: python/mozbuild/mozbuild/action/cargo.py:33 (Diff revision 1) > + parser.add_argument('command', help='cargo subcommand to invoke') > + parser.add_argument('manifest_path', > + help='Cargo manifest to build') > + parser.add_argument('target_dir', > + help='Directory in which to build') Probably doesn't matter greatly here, but do you want to add `metavar=...` arguments for each of these? ::: python/mozbuild/mozbuild/action/cargo.py:130 (Diff revision 1) > + # Cargo needs the same linker flags as the C/C++ compiler, > + # but not the final libraries. Filter those out because they > + # cause problems on macOS 10.7; see bug 1365993 for details. Can you file a followup to see if we can get rid of this now that we don't build on 10.7? I guess we may want to keep the custom linker regardless for cross builds, but we could get rid of the Mac-specific hacks. ::: python/mozbuild/mozbuild/action/cargo.py:155 (Diff revision 1) > + ) > + for k in env.keys(): > + if isinstance(env[k], unicode): > + env[k] = env[k].encode(sys.getfilesystemencoding() or 'mbcs') > + keys = set(env.keys()) - set(os.environ.keys()) > + print('%s %s' % (' '.join('%s=%s' % (k, quote(env[k])) for k in keys), ' '.join(quote(c) for c in cmd))) This is just debugging material? Or do we think this is useful to have in (maybe under `BUILD_VERBOSE_LOG`) so people can see what's going on?
Attachment #8908718 - Flags: review?(nfroyd) → review+
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1522614
You need to log in before you can comment on or make changes to this bug.