Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add compile mode for cargo check #107

Closed
ebkalderon opened this issue Mar 26, 2020 · 5 comments
Closed

Add compile mode for cargo check #107

ebkalderon opened this issue Mar 26, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@ebkalderon
Copy link
Member

As mentioned in #92 (comment), we should add an option for compileMode = "check", which would correspond to cargo check. This would be useful for supporting other tools like Clippy in Nix expressions run in CI environments, for example.

To find what artifacts should be copied to the Nix store in a cargo check compile mode, we first generate a new crate with cargo new --bin foo and then check the contents of the target directory after each command (with a cargo clean in between each run).

cargo check

target
└── debug
    ├── build
    ├── deps
    │   ├── foo-3c902f3ce96f8337.d
    │   └── libfoo-3c902f3ce96f8337.rmeta
    ├── examples
    ├── incremental
    │   └── foo-2hoi7zn3kcata
    │       ├── s-flx0c62kz4-1uiqlpc-qmasrgjks254
    │       │   ├── dep-graph.bin
    │       │   ├── query-cache.bin
    │       │   └── work-products.bin
    │       └── s-flx0c62kz4-1uiqlpc.lock
    └── native

cargo check --release

target
└── release
    ├── build
    ├── deps
    │   ├── foo-67a189b0d138f374.d
    │   └── libfoo-67a189b0d138f374.rmeta
    ├── examples
    ├── incremental
    └── native

From this, it seems that the .d and .rmeta artifacts are of the most importance and should definitely be copied into the Nix store. Running cargo clippy and cargo clippy --release also produced identical results, which confirms that both check and clippy share the same cache and that the latter doesn't produce any additional artifacts.

The real question is how to expose the artifacts from the Nix store into the current build. For all other compile modes, we achieve this using a rustc wrapper which silently forwards vendored artifacts from the Nix store to the compiler. However, these artifacts aren't rlibs, so I assume this wrapper won't be able to help us here.

CC @trha, do you have any ideas?

Brief diversion about rmeta artifacts

According to this comment on rust-lang/cargo, it seems that both cargo build and cargo check produce .rmeta artifacts, but each contains slightly different data and aren't interchangeable at the time of writing.

We currently don't retain these.rmeta artifacts in derivations built with compileMode = "build" in cargo2nix. Crates seem to compile perfectly fine without them, so presumably either cargo or rustc is generating them on the fly if it has access to the source directory. Should we keep these .rmeta files in compileMode = "build" as well? It would seem a bit inconsistent for us to retain them in check mode but not in build mode.

@ebkalderon
Copy link
Member Author

This task could be made easier with #118, given that --message-format=json happens to include a list of filenames (in this case, .rmeta artifacts when using cargo check) belonging to the current crate. We could leverage this flag to glean what artifacts need to be copied in the Nix store on cargo check or cargo clippy.

Note that the .d files aren't listed with --message-format=json, and deleting them doesn't seem to invalidate the cache of cargo check nor affect the final result, indicating that they are probably safe to ignore.

@ebkalderon
Copy link
Member Author

I think this feature would require a sizeable refactor of overlay/mkcrate.nix because cargo check is so different from cargo {build,test}. In the latter, we only care about .{rlib,a,so,dylib} files, while in the former we apparently only care about .rmeta files. This means that we can't reuse the linking logic in overlay/utils.sh; we'll need to write it nearly from scratch specifically for cargo check.

AFAIK, we can't wrap rustc and use command-line flags to point to Nix store paths of .rmeta files, so we may need to instead create a target/deps directory, symlink-farm all the dependencies' .rmeta files into it, and then pick out all the non-symlink files created and copy them to $out somewhere. This is so radically different from the other two compile modes, in fact, that I'm not sure how to approach this task without turning mkcrate.nix into a tangled mess.

@psionic-k
Copy link
Member

I ran the check --release on RustAnalyzer. Significantly more diverse outputs in the artifacts than just .d and .rmeta

Aside, since rust check doesn't need to link or emit binary, is it even benefiting from cargo2nix's native deps? I would like to constrain the role so that cargo2nix is doing maximum utility but with only strictly necessary second-system overhead

@ebkalderon
Copy link
Member Author

@psionic-k It depends on the particular crate. If a crate provides a build.rs that performs codegen, it might run at cargo check time. As such, we have to be prepared for the possibility that the build.rs might call out to a native dep at any time, even during cargo check. Apparently, there is a desire upstream to indicate to build scripts whether a crate is "building" or "checking" and adjust the work done as a result, but this doesn't seem to have landed yet (rust-lang/cargo#4001).

@psionic-k
Copy link
Member

See reasoning in #92 for not blocking, but also not pursuing more support for non-output Nix derivations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants