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

Enable Invoking Custom Subcommands with Cross #716

Open
4 tasks
Alexhuszagh opened this issue May 25, 2022 · 10 comments
Open
4 tasks

Enable Invoking Custom Subcommands with Cross #716

Alexhuszagh opened this issue May 25, 2022 · 10 comments

Comments

@Alexhuszagh
Copy link
Contributor

Alexhuszagh commented May 25, 2022

It's often useful to run custom subcommands for another target, such as asm, deb, or others. Since we can't necessarily enumerate every subcommand, there's a few solutions I see:

  1. Create an environment variable, forcing the command to run in the image, such as CROSS_FORCE_RUN_IN_IMAGE=1. This name isn't ideal, so we'd probably modify it to something else.
  2. If pre-build hooks are a thing and we're fine not supporting custom images, we could write a config file (just a TOML list of known extra subcommands) and parse this if the subcommand isn't known.
  3. Get the output of cross --list if the subcommand isn't known, parse it, and if the subcommand is known, run it in the image.

We need to support the following, at the very least:

  • cargo-valgrind
  • cargo-asm
  • cargo-deb
  • cargo-auditable

Related to #207, #210, #715.

@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Jul 16, 2022

Just because of the lack of updates: I've added a very rough base design for this, found on my subcommands fork.

The general idea is as follows:

  1. We have an allowlist of subcommands we recognize. This would be relatively small, but definitely include cargo-asm and cargo-deb. Ones like cargo-valgrind which rely on external dependencies would not be included, since they require pre-build hooks to get to work.
  2. If you use the subcommand and it is not installed but is part of the allowlist, it will be installed automatically. This will be installed in a local data directory, so the subcommand will be native for the target architecture (required for these subcommands to work).
  3. If the subcommand is not recognized, you can install it via cross-util. This will install it to the same directory as mentioned above. So for example, ~/.local/share/cross-rs/aarch64-unknown-linux-gnu/bin.
  4. If the subcommand is used, then this install point will be mounted and the subcommand invoked directly (such as cargo-asm rather than using cargo asm). There might be a better design here, but it might require some wrappers to get it to work with calling cargo directly.
  5. Subcommands can also be uninstalled via cross-util.

Now, the specifics. Installed subcommands will be registered in a JSON file in a general data directory (~/.local/share/cross-rs/subcommands.json). The general layout is quite simple:

{
    "aarch64-unknown-linux-gnu": [
        "asm",
        "deb"
    ]
}

This allows us to quickly look up the subcommand and if it's registered easily. We can also have cross-util list all registered subcommands, including for a given target. We can also likely add this to cross --list.

Thoughts? I've tested the general idea with cargo-asm, cargo-deb, and both work well.

@Emilgardis
Copy link
Member

There might be a better design here, but it might require some wrappers to get it to work with calling cargo directly.

this is what cargo does, it doesn't set any extra env, only invokes the binary with cargo-tool tool <..args>

  1. If you use the subcommand and it is not installed but is part of the allowlist,

I think the automatic installation should be replaced with a suggestion to install the tool with cross-util

@Alexhuszagh
Copy link
Contributor Author

I think the automatic installation should be replaced with a suggestion to install the tool with cross-util

It would be handled by the same code as cross-util, and only invoked from a small list of known subcommands without external dependencies. Everything else would have to be handled independently.

@Emilgardis
Copy link
Member

What i'm thinking is that we should probably preserve the same behaviour as cargo, cargo doesn't install automatically

@DASPRiD
Copy link

DASPRiD commented Nov 28, 2022

I assume there was no further progress made on this? Is there maybe a workaround at the moment which would allow running cargo-deb in cross without any direct cross support? Assuming using a custom docker image.

@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Nov 28, 2022

I assume there was no further progress made on this? Is there maybe a workaround at the moment which would allow running cargo-deb in cross without any direct cross support? Assuming using a custom docker image.

There's a mostly working PR for this based off of #931 at https://github.com/Alexhuszagh/cross/tree/subcommands. I've still got notes on the implementation status, I'm just waiting for some dust to settle before rebasing (it touches too many things so I'll have to do it entirely manually), and it also requires cargo configs and aliases to be resolved (hence, #931). So there is a decent amount of progress, there's substantial effort right now in ensuring I have the time to ensure it is fully comprehensive and correct, that no more changes are made to the config settings in the meantime (which they currently are), and that another maintainer has time to review a massive PR.

That branch should be almost entirely working IIRC. It only works if runners for the target are available, such as Qemu, which in this case it is, however, it requires a few tricks for pre-installing the commands.

@Shnatsel
Copy link

this is what cargo does, it doesn't set any extra env, only invokes the binary with cargo-tool tool <..args>

This is not entirely true - Cargo does set one environment variable, CARGO.

@NobodyXu
Copy link

I would also like cargo-nextest to be supported, it speeds up the testing by creating multiple processes to run unit/integration tests.

@Emilgardis
Copy link
Member

Not sure it's compatible with qemu, since apparently there's threading issues in some virtualization, maybe the story has changed.

See the readme:

cross test runs units tests sequentially because QEMU gets upset when you spawn multiple threads. This means that, if one of your unit tests spawns threads, then it's more likely to fail or, worst, never terminate.

I remember some discussion in rust-lang/rust about it, I'll try to find it and update this comment

@NobodyXu
Copy link

Not sure it's compatible with qemu, since apparently there's threading issues in some virtualization, maybe the story has changed.

nextest runs tests in parallel by spawning multiple processes, so its idea is sound and compatible with qemu.

The only problem with nextest is that it uses tokio multi-thread rt internally, but this can be helped by setting TOKIO_WORKER_THREADS, which should work unless nextest manually override it.

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

No branches or pull requests

5 participants