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

Subcommand bin_name on Windows contains ".exe" in the middle instead of at the end (or not at all) #992

Closed
Arnavion opened this issue Jun 30, 2017 · 19 comments · Fixed by #3693
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing
Milestone

Comments

@Arnavion
Copy link
Contributor

Arnavion commented Jun 30, 2017

Rust Version

rustc 1.20.0-nightly (f590a44ce 2017-06-27)

Affected Version of clap

2.25.0

Steps to Reproduce the issue

  1. Cargo.toml:

    [package]
    name = "foo"
    version = "0.1.0"
    
    [dependencies]
    clap = "*"
  2. src/main.rs:

    #[macro_use]
    extern crate clap;
    
    fn main() {
        let app = clap_app!(
            @app (app_from_crate!())
            (@subcommand bar => )
        );
        let _matches = app.get_matches();
    }
  3. cargo run -- bar --help (This runs target\debug\foo.exe bar --help. Note the .exe). Alternatively, run .\target\debug\foo.exe bar --help in cmd or PS. Both these will set argv[0] to something ending with foo.exe

    Also, running .\target\debug\foo bar --help in PS (note doesn't have .exe) will still have argv[0] ending with foo.exe because PS adds it automatically when spawning the process. cmd does not have this behavior so .\target\debug\foo bar --help in cmd will have argv[0] ending in foo

Expected Behavior Summary

Output should say foo-bar or foo-bar.exe as the name above the usage.

Actual Behavior Summary

Output says foo.exe-bar as the name above the usage. Note that the usage itself prints foo.exe bar which is fine, just the name printed above the usage is the problem.

foo.exe-bar

USAGE:
    foo.exe bar

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

Debug output

Compile clap with cargo features "debug" such as:

The debug feature does not build on Windows.

Workaround

Provide the bin_name manually instead of letting clap parse it from std::env::args_os().

    let app = clap_app!(
        @app (app_from_crate!()) (bin_name: "foo")
        (@subcommand bar => )
    );
@Arnavion
Copy link
Contributor Author

Arnavion commented Jun 30, 2017

Just stripping .exe from the end of argv[0] if #[cfg(windows)] should be fine, I think.

Of course this will lead to the wrong output from foo.exe --help if someone names their binary foo.exe.exe (it'll strip the .exe and print foo, which won't work), but that is an unlikely situation.

@kbknapp kbknapp added A-help Area: documentation, including docs.rs, readme, examples, etc... C: subcommands C-bug Category: Updating dependencies labels Oct 4, 2017
@kbknapp
Copy link
Member

kbknapp commented Oct 4, 2017

When building the subcommand name, I'd be fine with stripping the .exe only when we do the "concat with subcommands" titling. This would be a very easy fix if someone wants a quick PR.

Relevant function to fix

@pksunkara pksunkara added this to the 3.0 milestone Apr 9, 2020
@pksunkara pksunkara added D: easy E-easy Call for participation: Experience needed to fix: Easy / not much E-medium Call for participation: Experience needed to fix: Medium / intermediate labels Apr 9, 2020
@pksunkara pksunkara modified the milestones: 3.0, 3.1 Apr 23, 2020
@retep998
Copy link

To avoid issues with whether the shell included the .exe in argv[0] you can just get the executable name directly on Windows via GetModuleFileNameW.

@CreepySkeleton
Copy link
Contributor

To avoid issues with whether the shell included the .exe in argv[0] you can just get the executable name directly on Windows via GetModuleFileNameW.

Are there safe bindings to it?

@retep998
Copy link

Actually, it turns out std::env::current_exe() on Windows already does exactly that.

@pksunkara
Copy link
Member

Will you be sending a PR?

@CreepySkeleton
Copy link
Contributor

It's actually more complicated than it sounds.

With current_exe in mind, we have two competing sources for the application name:

  • The first element of the iterator (if NoBinaryName wasn't set)
  • current_exe()

Which one to choose? Do we just discard the element and use current_exe?

@retep998
Copy link

What are the reasons for using argv[0] instead of current_exe()?

@CreepySkeleton
Copy link
Contributor

The iterator clap parser gets may or may not be from env::args(). It can be a user-provided array with it's own first item which is supposed to be used "as if" it was argv[0]. It should probably be prioritized over current_exe.

get_matches must use current_exe and get_matches_from must use iter.nth(0), I think.

@kbknapp
Copy link
Member

kbknapp commented May 26, 2020

The first element of the iterator (if NoBinaryName wasn't set)

The purpose of that setting was to start parsing immediately as arguments, instead of skipping argv[0]. i.e. such as a REPL or other prompt where the consumer code is continually passing an argv direct to clap that doesn't include the binary.

which is supposed to be used "as if" it was argv[0]

That's the incorrect bit 😉 clap assumes the binary name was set manually when NoBinaryName is used, it doesn't re-interpret argv[0] as a new binary name.

@kotx

This comment has been minimized.

@epage
Copy link
Member

epage commented Dec 8, 2021

We've also run into this when testing examples, like the former 08_subcommands[.exe-add

$ 08_subcommands help add
08_subcommands[.exe-add 0.1

Kevin K.

Adds files to myapp

USAGE:
    08_subcommands.exe add <input>

ARGS:
    <input>    the file to add

OPTIONS:
    -h, --help       Print help information
    -V, --version    Print version information

@epage
Copy link
Member

epage commented Dec 9, 2021

Why would we not just strip std::env::consts::EXE_SUFFIX from argv[0] when setting the bin name? Do we really need .exe everywhere else? If get_matches_from is meant to act like get_matches, it seems like it should be fine for us to strip it from there to.

@epage epage added S-waiting-on-decision Status: Waiting on a go/no-go before implementing and removed P4: nice to have E-medium Call for participation: Experience needed to fix: Medium / intermediate labels Dec 9, 2021
@epage epage added S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing and removed S-waiting-on-decision Status: Waiting on a go/no-go before implementing labels Dec 9, 2021
@pksunkara
Copy link
Member

Do we really need .exe everywhere else?

I am under the impression that we do. bin name has always been about the actual binary name and I think it's been used in other places with that assumption which is why Kevin suggested a new variable to hold this name in the first place.

I'd be fine with stripping the .exe only when we do the "concat with subcommands" titling.

If you were to audit the usage of it and decide that it's okay setting bin name without .exe, then I am good with your proposal.

@epage
Copy link
Member

epage commented Dec 10, 2021

If you were to audit the usage of it and decide that it's okay setting bin name without .exe, then I am good with your proposal.

Looking around, it looks like its only used for display purposes, like help and error messages. I'm not seeing anything doing processing based on the name. We don't expose the bin_name from arg_matches. We do mutate the App with the bin name from the last parse, so someone could do

let matches = app.get_matches_mut();
let  bin = app.get_bin_name()

Personally, that feels hacky; imo all parse results should be coming from ArgMatches. In considering this, a naiive implementation of #2911 would make it so people would never be able to look up the bin_name found from parsing.

@spenserblack
Copy link

Do we really need .exe everywhere else?

Are you asking if .exe should be removed everywhere?
I think it's slightly useful in the usage section, since it's possible for command prompt to prioritize my-app.bat over my-app.exe.

@epage epage modified the milestones: 3.x, 3.1, 3.2 Feb 2, 2022
epage added a commit to epage/clap that referenced this issue May 3, 2022
This is a step towards clap-rs#992.  When help renders the application name, it
uses the `bin` template variable which is just the `bin` name with
spaces converted to ` `.  While having `app.exe sub` makes sense,
`app.exe-sub` does not.

To get around needing this for usage, we've created a `display_name`
field that is fairly similar but
- The root name is the `name` and not `bin_name`
- We always join with `-`

This means that the derived `bin_name` will only show up in usage.

For now, the default template has not been updated as that is a minor
compatibility change and should be in a minor release, at least.
epage added a commit to epage/clap that referenced this issue May 3, 2022
This is a step towards clap-rs#992.  When help renders the application name, it
uses the `bin` template variable which is just the `bin` name with
spaces converted to ` `.  While having `app.exe sub` makes sense,
`app.exe-sub` does not.

To get around needing this for usage, we've created a `display_name`
field that is fairly similar but
- The root name is the `name` and not `bin_name`
- We always join with `-`

This means that the derived `bin_name` will only show up in usage.

For now, the default template has not been updated as that is a minor
compatibility change and should be in a minor release, at least.  I was
worried this would be a full breaking change.  The main case I was
worried about was cargo subcommands but our tests show they should just
work.
@epage epage modified the milestones: 3.2, 4.0 May 5, 2022
@epage
Copy link
Member

epage commented May 5, 2022

We're close enough to when we can start working on 4.0, I'm going to play it safe and push it off to then

epage added a commit to epage/clap that referenced this issue May 5, 2022
This will mean we won't have an awkard `.exe` in the middle on Windows

This means users can have a display name for their application rather
than it being dependent on the binary name it was run as

This means users can manually set it to use spaces instead of dashes for
separating things out.

Fixes clap-rs#992
Fixes clap-rs#1474
Fixes clap-rs#1431
epage added a commit to epage/clap that referenced this issue May 5, 2022
This will mean we won't have an awkard `.exe` in the middle on Windows

This means users can have a display name for their application rather
than it being dependent on the binary name it was run as

This means users can manually set it to use spaces instead of dashes for
separating things out.

Fixes clap-rs#992
Fixes clap-rs#1474
Fixes clap-rs#1431
@dbsxdbsx
Copy link

dbsxdbsx commented Dec 26, 2022

Why would we not just strip std::env::consts::EXE_SUFFIX from argv[0] when setting the bin name? Do we really need .exe everywhere else? If get_matches_from is meant to act like get_matches, it seems like it should be fine for us to strip it from there to.

(I am using clap 3.2.14)
I wonder how to eliminate the exe suffix output after running command on windows when using clap under derive.
Currently,
with cargo.toml:

[[bin]]
name = "app_name"
path = "src/main.rs"

and a struct using clap:

#[derive(Parser, Debug, PartialEq, Eq, Deserialize, Default)]
#[clap(version, setting(clap::AppSettings::DeriveDisplayOrder))]
pub struct MyStruct {
...
}

after running cargo run --manifest-path [project_path]/Cargo.toml -- --help > book/src/help.txt
the USAGE section of help.txt is like:

USAGE:
    app_name.exe [OPTIONS] ...

I do can eliminate the exe by changing the derivative to :

#[clap(bin_name = "app_name", version, setting(clap::AppSettings::DeriveDisplayOrder))]

But is there a way without changing the derivative macro? Meanwhile, I also wonder how to output like so:

USAGE:
    app_name[EXE] [OPTIONS] ...

with [EXE] instead of .exe whatever platform I am running.

@epage
Copy link
Member

epage commented Dec 26, 2022

Specifying attributes is how you customize clap via the derive and setting bin_name is exactly the way to resolve this.

[EXE] is in our documentation due to our testing framework for our docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants