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 support for cargo configurations. #931

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Alexhuszagh
Copy link
Contributor

@Alexhuszagh Alexhuszagh commented Jul 10, 2022

Also adds support for parsing and reading configuration options/environment variables from cargo, and allows users to ignore config files in the package.

This adds the [build.env.cargo-config] option, which can be set to complete, ignore, or default. If set to complete, cross will have access to every to every cargo config file on the host (if using remote cross, a config file is written to a temporary file which is mounted on the data volume at /.cargo/config.toml). If set to ignore, any .cargo/config.toml files outside of CARGO_HOME are ignored, by mounting anonymous data volumes to hide config files in any .cargo directories. The default behavior uses the backwards-compatible behavior, allowing cross to access any config files in the package and CARGO_HOME directories. If the build is called outside the workspace root or at the workspace root, then we only mount an anonymous volume at $PWD/.cargo.

The alias support includes recursive subcommand detection, and errors before it invokes cargo. A sample error message is [cross] error: alias y has unresolvable recursive definition: x -> y -> z -> a -> y, and therefore can handle non-trivial recursive subcommands.

Closes #562.
Closes #621.

Partially related to #704.

@Alexhuszagh
Copy link
Contributor Author

This still needs to support the cargo environment variables, which luckily shouldn't be too hard.

@Alexhuszagh Alexhuszagh marked this pull request as ready for review July 12, 2022 20:35
@Alexhuszagh Alexhuszagh requested a review from a team as a code owner July 12, 2022 20:35
@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Jul 12, 2022

This now supports environment variables, and the design is similar to the cross configuration files (and much more logical), but avoids parsing variables except for those we know and lazily reads environment variables.

  • Recursive alias improvement

EDIT: This still needs some work handling recursive aliases, since it only handles recursion that is self-referential:

[alias]
recurse = ["recurse"]

This really should be able to handle:

[alias]
x = ["y"]
y = ["z"]
z = ["a"]
a = ["x"]

This would be very doable with a lookup set, which tracks each subcommand that has been seen until an unseen alias is processed. This would output something like:

$ cargo x
error: alias x has unresolvable recursive definition: x -> y -> z -> a -> x

The recursive alias support currently just uses a Vec, since we'd need an IndexMap for better time-complexity, but in practice we're unlikely to have highly recursive aliases. If needed, we could always switch to a hashmap that iterates over by insertion order.

@Alexhuszagh Alexhuszagh marked this pull request as draft July 13, 2022 16:54
@Alexhuszagh Alexhuszagh changed the title Add support for cargo aliases Add support for cargo configurations. Jul 14, 2022
@Alexhuszagh Alexhuszagh marked this pull request as ready for review July 15, 2022 19:56
@Alexhuszagh Alexhuszagh marked this pull request as draft July 16, 2022 23:27
@Alexhuszagh
Copy link
Contributor Author

Temporarily marking as a draft since this might have undergone a bad rebase, and I don't have the time right now to verify that it's correct.

@Alexhuszagh Alexhuszagh marked this pull request as ready for review July 17, 2022 15:11
Also adds support for parsing and reading configuration options/environment variables from cargo, and allows users to ignore config files in the package.

This adds the `[build.env.cargo-config]` option, which can be set to `complete`, `ignore`, or `default`. If set to `complete`, cross will have access to every to every cargo config file on the host (if using remote cross, a config file is written to a temporary file which is mounted on the data volume at `/.cargo/config.toml`). If set to ignore, any `.cargo/config.toml` files outside of `CARGO_HOME` are ignored, by mounting anonymous data volumes to hide config files in any `.cargo` directories. The default behavior uses the backwards-compatible behavior, allowing cross to access any config files in the package and `CARGO_HOME` directories. If the build is called outside the workspace root or at the workspace root, then we only mount an anonymous volume at `$PWD/.cargo`.

The alias support includes recursive subcommand detection, and errors before it invokes cargo. A sample error message is `[cross] error: alias y has unresolvable recursive definition: x -> y -> z -> a -> y`, and therefore can handle non-trivial recursive subcommands.
@Alexhuszagh
Copy link
Contributor Author

This will need substantial rebases and refactoring, which I'll try to do shortly, so I'll revert this to a draft in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cross picking up wrong target options in cargos config.toml cargo alias doesn't work on cross
1 participant