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

Use main binary if nothing else is specified #39

Closed
therealprof opened this issue Nov 17, 2018 · 6 comments
Closed

Use main binary if nothing else is specified #39

therealprof opened this issue Nov 17, 2018 · 6 comments

Comments

@therealprof
Copy link
Contributor

Usually a cargo command will automatically use the main binary if no binary or example is specified. This does not work for cargo-binutils and instead requires to specify the binary name which for a single binary crate is the project name and thus a bit clunky.

@japaric
Copy link
Member

japaric commented Dec 18, 2018

Thing is cargo objdump -- -d some/path works today and has the semantics of llvm-objdump -d some/path. Presumably if we want cargo objdump -- -d to also Just Work we would need to parse the llvm tool flags to see if the user passed a path to a binary or not (to know if we need to build the crate and append the path to the output artifact to the llvm tool args), which can get tricky since some tools take more than one path (e.g. llvm-objcopy), others take flag arguments (e.g. llvm-objcopy -O binary (..); is binary a path or a the argument of -O?). To me that sounds like we would have to copy some of llvm-$tool argument parsing logic into cargo-$tool and I'd prefer to avoid that.

A simpler alternative may be to ship rust-$tool with the cargo-binutils package. These rust-$tool would be just sugar for path/to/llvm-$tool. Then we can modify cargo-$tool to always build the package and invoke llvm-$tool (..) path/to/cargo/artifact. But this would be a breaking change because e.g. cargo objdump -- -d some/path would then pass two paths to llvm-objdump and error out (which is not the case today).

@japaric
Copy link
Member

japaric commented Jun 21, 2019

Was thinking about this issue today and realized that this statement:

Usually a cargo command will automatically use the main binary if no binary or example is specified.

is not true. What cargo build and cargo check do is build all the artifacts in the Cargo project that are not examples. For example:

$ tree
.
├── Cargo.toml
├── examples
│   └── baz.rs
└── src
    ├── bin
    │   └── bar.rs
    ├── lib.rs
    └── main.rs

$ cargo build -v
   Compiling foo v0.1.0 (/tmp/foo)
     Running `rustc --edition=2018 --crate-name foo src/lib.rs --color always --crate-type lib --emit=dep-info,metadata,link -C debuginfo=2 -C metadata=71740c4f2a453aee -C extra-filename=-71740c4f2a453aee --out-dir /tmp/foo/target/debug/deps -C incremental=/tmp/foo/target/debug/incremental -L dependency=/tmp/foo/target/debug/deps`
     Running `rustc --edition=2018 --crate-name foo src/main.rs --color always --crate-type bin --emit=dep-info,link -C debuginfo=2 -C metadata=5d315b79c4d3a0f2 -C extra-filename=-5d315b79c4d3a0f2 --out-dir /tmp/foo/target/debug/deps -C incremental=/tmp/foo/target/debug/incremental -L dependency=/tmp/foo/target/debug/deps --extern foo=/tmp/foo/target/debug/deps/libfoo-71740c4f2a453aee.rlib`
     Running `rustc --edition=2018 --crate-name bar src/bin/bar.rs --color always --crate-type bin --emit=dep-info,link -C debuginfo=2 -C metadata=22dbb2ea0ec0271e -C extra-filename=-22dbb2ea0ec0271e --out-dir /tmp/foo/target/debug/deps -C incremental=/tmp/foo/target/debug/incremental -L dependency=/tmp/foo/target/debug/deps --extern foo=/tmp/foo/target/debug/deps/libfoo-71740c4f2a453aee.rlib`
    Finished dev [unoptimized + debuginfo] target(s) in 0.31s

$ cargo check -v
cargo check -v
    Checking foo v0.1.0 (/tmp/foo)
     Running `rustc --edition=2018 --crate-name foo src/lib.rs --color always --crate-type lib --emit=dep-info,metadata -C debuginfo=2 -C metadata=4af807af49468713 -C extra-filename=-4af807af49468713 --out-dir /tmp/foo/target/debug/deps -C incremental=/tmp/foo/target/debug/incremental -L dependency=/tmp/foo/target/debug/deps`
     Running `rustc --edition=2018 --crate-name foo src/main.rs --color always --crate-type bin --emit=dep-info,metadata -C debuginfo=2 -C metadata=3fc606d28351772e -C extra-filename=-3fc606d28351772e --out-dir /tmp/foo/target/debug/deps -C incremental=/tmp/foo/target/debug/incremental -L dependency=/tmp/foo/target/debug/deps --extern foo=/tmp/foo/target/debug/deps/libfoo-4af807af49468713.rmeta`
     Running `rustc --edition=2018 --crate-name bar src/bin/bar.rs --color always --crate-type bin --emit=dep-info,metadata -C debuginfo=2 -C metadata=09a17f670d00aa41 -C extra-filename=-09a17f670d00aa41 --out-dir /tmp/foo/target/debug/deps -C incremental=/tmp/foo/target/debug/incremental -L dependency=/tmp/foo/target/debug/deps --extern foo=/tmp/foo/target/debug/deps/libfoo-4af807af49468713.rmeta`
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s

So before this can be implemented it needs to be defined what a Cargo subcommand should do on a Cargo project like the one shown above. Run the llvm tool on all the artifacts? With the exact same user arguments? That wouldn't make much sense for subcommands like cargo strip and cargo objcopy as that may override the output file specified by the user .

@therealprof
Copy link
Contributor Author

Fair enough, but isn't the interesting question what cargo run would do? IMHO that's the behaviour which should be replicated.

@japaric
Copy link
Member

japaric commented Jun 21, 2019

isn't the interesting question what cargo run would do?

Is it? In my example, it throws an error:

$ cargo run -v
error: `cargo run` could not determine which binary to run. Use the `--bin` option to specify a binary, or (on nightly) the `default-run` manifest key.
available binaries: foo, bar

The foo mentioned is src/main.rs. Odd that it doesn't list baz even though cargo run --example baz certainly works, but sure we can replicate that behavior.

@therealprof
Copy link
Contributor Author

Is it? In my example, it throws an error:

Yes, that's exactly what we want. If there's no choice, do the obvious thing; if there is a choice, explain the options.

@therealprof
Copy link
Contributor Author

This has been fixed in release 0.2.0.

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