Expand on xtask

[mejrs]

This builds on the work @davidhewitt did earlier 😀

Todo:

• Update pull request template
• Update contributing.md
• Fix llvm_cov error (Error: expected '=' in each line of output from llvm-cov show-env 🤔 )

[mejrs]

I had to do this to stop it from raising OSErrors on windows...not sure what's with that.

[davidhewitt]

Possibly related to #1339?

[adamreichold]

Hhhmmm, I think #2163 (comment) proposed to move all xtasks into a repository-level noxfile instead. Personally, I think using nox is preferable as having the xtasks implemented in Rust is not really beneficial when we require the presence of a Python environment in any case. Especially, the compilation step can be skipped when these are implemented in Python.

[mejrs]

Hmm, I hadn't noticed that.

[davidhewitt]

Thanks! Yes I have gone back-and-forth on xtask (as noted in the above comment). Now that we have this implementation, I'm very content with keeping it! Interestingly, sounds the rust stdlib wants to move away from a Python build system to a Rust one. (https://internals.rust-lang.org/t/cargo-x-for-rustc-dev/16172)

---

nox pros: we require Python install anyway to build PyO3. Python is terser to write short scripts. nox is a more widely known tool than cargo xtask. It's quite handy that it can create Python environments for us (e.g. to run black).

nox cons: I like using cargo test / cargo check to do most of my PyO3 dev work. I have to activate the virtualenv where I've installed nox to run it. (Though I guess I could pip install --user nox.)

---

cargo xtask pros: implemented in Rust 😉. There's also a lot less framework magic going on compared to nox; I'm more sure exactly what's running.

cargo xtask cons: there's a dependency on having nox available anyway for the Python-based testing. If we want to use cargo xtask in CI, we need to support MSRV, which means no clap 3 or other recent features (see e.g. MSRV failure here; I fell for that trap too). Compile times, although they're pretty minimal given xtask is a simple script.

---

Fix llvm_cov error (Error: expected '=' in each line of output from llvm-cov show-env 🤔 )

Probably you need to update your cargo-llvm-cov version. (The error message when the command errors is a bit rubbish, sorry!)

[davidhewitt]

PyPy 3.7 segfault is very interesting. I wonder if that's reproducible or flaky...

[adamreichold]

Interestingly, sounds the rust stdlib wants to move away from a Python build system to a Rust one.

I think they are solving a different problem though, i.e. bootstrapping without a Rust compiler already available (and no fixed requirement on Python for other reasons).

There's also a lot less framework magic going on compared to nox; I'm more sure exactly what's running.

There is also the option of using just a Python script for the xtasks without nox. (This is my personal favourite for projects which require Python for other reasons.)

Compile times, although they're pretty minimal given xtask is a simple script.

It is not a CI runner, but on my machine a fresh build takes 30 seconds. I suspect this is mostly due to using proc-macros, i.e. structopt.

[messense]

I have to activate the virtualenv where I've installed nox to run it. (Though I guess I could pip install --user nox.)

In case you don't know, you should try pipx: pipx install nox.

[mejrs]

PyPy 3.7 segfault is very interesting. I wonder if that's reproducible or flaky...

Seems to be reproducible, other PRs are hitting it too.

It is not a CI runner, but on my machine a fresh build takes 30 seconds.

For me it's about 20 seconds, but only 7 seconds if it can use artefacts from earlier cargo test builds. And my machine is not that good, so it's probably faster for most people.

[mejrs]

no clap 3 or other recent features

Yeah, that is quite annoying. But structopt and clap 2 is still pretty good :)

cargo xtask cons: there's a dependency on having nox available anyway for the Python-based testing.

On the other hand most pull requests don't touch on the python tests, so running python tests is unlikely to be helpful. The parts that tend to fail most (formatting, cargo test etc) don't need nox.

[davidhewitt]

Agreed. I"m happy to proceed with merging this, I don't think there's enough of a difference between nox and cargo xtask that now we have this there's any value in throwing it away. My comment in the other thread was primarily that if I'd gotten around to improving our automations I probably would have used nox.

[mejrs]

Additionally I'd prefer to not (initially) require contributors to install anything to get started. I know installing nox and/or black is pretty easy but it's a speed bump nonetheless.

[davidhewitt]

Looks good to me, thanks! Just a few quick questions, feel free to merge after they're responded to.

Also, to-do list at the top also mentioned Contributing.md, did that still need updating?

[davidhewitt]

How do you feel about having this in doc.rs, and similar for CoverageOpts? I guess arguments can be made to put here or in the respective subcommand files.

[mejrs]

I'd rather have it in cli.rs, it feels like it belongs there imo

[davidhewitt]

Looks great to me! Thanks again for doing this, it's a lovely improvement to our automations and I'll be glad for cargo test to complete without erroring 🙂