marker icon indicating copy to clipboard operation
marker copied to clipboard

Simplify lint crates build to a single `cargo build` invocation

Open Veetaha opened this issue 2 years ago • 5 comments

cargo build can be specified to build the dependent crates only. It means we can use the build workspace to both fetch and build the lint crate dlls with a single cargo build command.

So instead of

cargo fetch
metadata=$(cargo metadata)
cargo build --manifest-path /path/to/lint-crate/a/Cargo.toml -Z unstable-options --out-dir path
cargo build --manifest-path /path/to/lint-crate/b/Cargo.toml -Z unstable-options --out-dir path
# ...

We have just

artifacts=$(cargo build --message-format json -p a -p b ...)
metadata=$(cargo metadata)

And then we pick the DLLs from the generated artifacts metadata.

This also removes the usage of the unstable --out-dir parameter for cargo build.

Draft

I need to fix the integration with --message-format json to discover the compiled artifacts. The problem space here is to match the package ID of the lint crates with the package ID of the compiler artifacts reports

Veetaha avatar Sep 19 '23 23:09 Veetaha

I need to fix the integration with --message-format json to discover the compiled artifacts. The problem space here is to match the package ID of the lint crates with the package ID of the compiler artifacts reports

It's a shame, that Cargo's metadata doesn't include the build artifacts and their names.

Here is another idea, which might be worth checking out:

Cargo has a --config parameter which can be used to override any field for the execution of a command. We could set the lib.name field of lint crates to get the file names we want. I've done some testing, just by editing marker_lints/Cargo.toml. The following worked for me:

Set lib.name field in marker_lints/Cargo.toml:

  [lib]
+ name = "cool"
  crate-type = ["cdylib"]

Run cargo build -p marker_lints. This creates the following .so artifacts:

  • target/debug/libcool.so
  • target/debug/deps/libcool.so

Parsing --message-format json might be the better approach, but I wanted to mention this option as well :)

xFrednet avatar Sep 22 '23 07:09 xFrednet

I think using --message-format json is better, because this way we can build everything with a single cargo build, which should in theory be faster. I suppose your suggestion for overriding the --config implies multiple cargo builds, right?

But.. I suppose we will actually need multi-cargo build behavior if we want to implement --locked where we use the lock file from the lint crates themselves (https://github.com/rust-marker/marker/issues/251#issuecomment-1731174620).

Veetaha avatar Sep 22 '23 10:09 Veetaha

I suppose your suggestion for overriding the --config implies multiple cargo builds, right?

Yes, I came up with the idea in that context, but the flag might be usable for dependencies as well. I assume that it can modify everything cargo-metadata provides. But this is just a guess. It was also a general FYI, that this option exists. :)

xFrednet avatar Sep 22 '23 10:09 xFrednet

There is a problem with the --message-format approach. Error messages are emitted in JSON as well and cargo marker would need to forward them. This complicates things even more. Need to think about this 🤔

Veetaha avatar Oct 01 '23 21:10 Veetaha

Yeah, the ability to request this information is sadly quite limited :confused: I might have two ideas:

  1. Check if cargo maybe supports outputting two formats at once.
  2. The very hacky one: First run cargo build and forward all warnings etc to the user. If that succeeds, run it again with --message-format to get the artifacts. This might work with incremental compilation + caching.

xFrednet avatar Oct 02 '23 11:10 xFrednet