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

How to check whether a given task exists or not? #10

Open
kornelski opened this issue Apr 27, 2020 · 7 comments
Open

How to check whether a given task exists or not? #10

kornelski opened this issue Apr 27, 2020 · 7 comments

Comments

@kornelski
Copy link
Collaborator

I'm concerned that the design of xtask as a single all-tasks binary with custom opaque command-line parsing doesn't allow external tools to integrate with the tasks. This limits xtask to UI interaction with humans only, and doesn't allow graceful integration with packagers, CI or editors.

For example, when adding a new project in a CI service, the service could search the repository to decide how to build it, and create a build template automatically. For Cargo projects that's something like cargo test --all. But if xtask was machine-readable, the CI could also check if xtask ci exists, and use it instead of the default cargo test. There's similar story for deployments like Heroku or packaging like cargo-deb. They could replace Rusty default assumptions with a specific tasks, if they could know that the task is implemented by the xtask binary.

However, this is currently not possible, because mere presence of the xtask crate is not enough to know that xtask ci works. Running xtask commands to see if they work is not reliable (e.g. xtask ci may fail because tests failed, not because the subcommand is not implemented. Conversely, xtask ci can succeed because a custom command-line parsing library interprets it as some other argument or fails to use appropriate exit code).

Running xtask --help might work sometimes, but it's not a machine readable output, and in any case running arbitrary code to check what it supports is not so great. Especially when the thing that inspects the repo is not prepared to sandbox arbitrary code (the configuration-detection part of a system is usually separate from the part that runs the configured project sandboxed).

@kornelski
Copy link
Collaborator Author

kornelski commented Apr 27, 2020

Potential solutions that come to my mind:

  • Require xtask binary to only use a predefined, specific xtask-provided command-line parser, and teach this command-line parser to have something like --help --output-format=json for tools to describe what the binary can do.

  • Require xtask to use a manifest file, or add Cargo metadata section describing what commands are implemented. This would be tough to keep in sync with custom command-parser though.

  • Switch to multi-binary crate, with one binary per command. This way tools could run cargo metadata on xtask crate, and know which subcommands are implemented by looking at the targets. Or xtask could require to rely on Cargo's "autobins" behavior, so that tools merely need to check for existence of ./xtask/src/bin/ci.rs.

  • Switch to multiple xtasks crates, so that tools can check for ./xtask/ci/Cargo.toml existence.

@kornelski
Copy link
Collaborator Author

kornelski commented Nov 12, 2020

I'll bump this.

I'd really like to use xtask from cargo-deb, but the current black-box design ("parse whatever args you want however you want") is fundamentally incompatible with cargo-deb.

@matklad
Copy link
Owner

matklad commented Nov 12, 2020

Yeah, these are all good questions....

I am personally not familiar with listed use-cases that much, so can't give guidance here. Process wise, @kornelski, can you just draft some design which you think works for this, we'll merge this into the repo and see how it works in practice? We'll need more scrunity before we set it in stone and recommend folks to use it, but having something in the meantime would be better than analysis-paralisis.

My straw man proposal is:

Add [package.metadata.xtask] section to the Cargo.toml manifest in some fixed format, which lists some specifc commands, like this:

[package.metadata.xtask]
dist = true

or like this:

[package.metadata.xtask]
dist = "cargo -p xtask run -- --dist my-custom-flag"

The benefit here is that the consuming tool knows statically what commands are available, and it's easy enough to add some additional meta later. The drawback is that a human would need to manually sync Cargo.toml and whatever xtask commands they want to publicly expose.

@kornelski
Copy link
Collaborator Author

kornelski commented Nov 13, 2020

I'd rather avoid putting shell scripts in Cargo.toml. String escaping and portability of that are dubious. It's not great that Cargo already does that in config.

Maybe .cargo/config [alias] could be reused?

[alias]
xtask = "run --package xtask --"
xtask-dist = "run --whatever --dist"

@matklad
Copy link
Owner

matklad commented Nov 13, 2020

@kornelski those are not intended to be shell scripts, those are arguments to std::process::Command. And we probably should remove the leading cargo, to have syntax exactly like aliases.

And yeah, given that, I really like your idea of using .cargo/config. It may be not the most obvious one, but its way simpler than any alternative, because it just re-uses existing things. I think it makes sense to punt on "proper" way to do things until we add first-class Cargo support for this.

Could you send a PR which documents "API* commands which are recognized by 3rd party tooling and have specific interface?

Something along the lines of:

The following commands are "public API" commands. They may be invoked by 3rd-party tools, like distribution-specific packagers, and are expected to have a specific intefcace. These commands must be specified in `.carg/config` in the `[alias]` section -- this is how the tools learn that they exist

**dist:**
whatever behavior is convenient for `cargo deb`

It's not great that Cargo already does that in config.

I believe cargo never invokes system's shell.

@kornelski
Copy link
Collaborator Author

kornelski commented Nov 13, 2020

yikes, Cargo just does split_whitespace on the alias command to get the args. So it avoids non-portable shell escaping, because there's no support for string escaping :D

@matklad
Copy link
Owner

matklad commented Nov 13, 2020

Still better than invoking the shell though :-) But yeah, having something like python shelx in Rust would be lovely for such use-cases.

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

No branches or pull requests

2 participants