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

Fixing build.sh and argument parsing consistency #33

Open
jnm2 opened this issue Aug 25, 2017 · 9 comments
Open

Fixing build.sh and argument parsing consistency #33

jnm2 opened this issue Aug 25, 2017 · 9 comments

Comments

@jnm2
Copy link
Contributor

jnm2 commented Aug 25, 2017

My PR #30 caused build.sh to stop accepting certain argument formattings that it used to accept. I thought cake.exe understood all these forms but it turns out it only understands --target=foo.

Current (breaks some people):

cake.exe build.ps1 build.sh
-t foo ✔️
-t=foo
-t:foo ✔️
-target foo ✔️
-target:foo ✔️
--target foo
--target=foo ✔️ ✔️ ✔️

What we need:

cake.exe build.ps1 build.sh
-t foo ✔️ ✔️
-t=foo
-t:foo ✔️
-target foo ✔️
-target:foo ✔️
--target foo ✔️
--target=foo ✔️ ✔️ ✔️

This is what we used to have. Not having --target Foo broke an NUnit build. The fix (--target=Foo) is easy and IMO preferable but I assume you don't want the break in the first place. Assuming you don't,

  • I need to get a bootstrapper fix in for this.

  • We need to have tests in this repo for this.


What would be cool:

After the immediate fix is taken care of, I propose changing cake.exe so that it understands at least -t foo, -t=foo, and --target foo in addition to --target=foo so that build.sh can go back to not having a special understanding of any arguments beyond --script:

cake.exe build.ps1 build.sh
-t foo ✔️ ✔️ ✔️
-t=foo ✔️ ✔️ ✔️
-t:foo ✔️
-target foo ✔️
-target:foo ✔️
--target foo ✔️ ✔️ ✔️
--target=foo ✔️ ✔️ ✔️
@jnm2
Copy link
Contributor Author

jnm2 commented Aug 25, 2017

Actually, I think I'm wrong about -t=foo ever working in build.sh. It's just -t foo and --target foo that we're concerned with. Updated.

@jnm2
Copy link
Contributor Author

jnm2 commented Aug 25, 2017

PR with just the fix: #34

@CharliePoole
Copy link

Typically, whether you can use =, : or space between an option and its value is a feature of the particular OS command-line in use, not of the program, although some programs actually try to handle this themselves.

For consistency with other programs, a shell script should never IMO accept a single hyphen for a multi-character option. This doesn't create inconsistency between operating systems, it merely acknowledges the inconsistency that's already there and makes the script consistent with other scripts on the same OS.

@jnm2
Copy link
Contributor Author

jnm2 commented Sep 8, 2017

For consistency with other programs, a shell script should never IMO accept a single hyphen for a multi-character option. This doesn't create inconsistency between operating systems, it merely acknowledges the inconsistency that's already there and makes the script consistent with other scripts on the same OS.

I agree 100% with that, as listed above. PowerShell is the oddity there and that's why it's the only one in the three charts that allows a single dash with multiple characters.

Ideally the build.ps1 column shows only the formats Cake understands + what PowerShell scripts commonly understand, and the build.sh column shows only the formats Cake understands + what shell scripts commonly understand.

To that end, can you point out if I got the conventions wrong for build.sh (or build.ps1)? I'm not overly familiar with common shell script argument styles.

@jnm2
Copy link
Contributor Author

jnm2 commented Sep 8, 2017

Typically, whether you can use =, : or space between an option and its value is a feature of the particular OS command-line in use, not of the program, although some programs actually try to handle this themselves.

In the case of scripts, the script host parses the arguments, but in the case of an .exe, the OS and command line don't do anything special for you as far as I know- whatever you type in gets sent as an unparsed verbatim string to the exe. That either puts cake.exe on the hook for understanding all OS conventions, or else it puts build.ps1 and build.sh on the hook to "translate" parsed shell arguments from within the respective scripting environment to a raw OS-independent string for cake.exe to serialize in an OS-independent fashion.

@CharliePoole
Copy link

If Cake is one of those programs that parses it's own command-line, then nothing I said applies to it. 😄

@jnm2
Copy link
Contributor Author

jnm2 commented Sep 8, 2017

I'm just saying, there is no .exe that doesn't parse its own command line. It's standard for every .exe (Windows at least) to parse using the C++ argv convention. (spec) But even once argv gives you a string array, it's still always been up to the individual .exe whether to look for:

  • args[0] == "-t" && args[1] == "value"
  • args[0] == "-t=value"
  • args[0] == "/t" && args[1] == "value"
  • args[0] == "/t:value"

etc.

There's no standard I've found. Even Windows utilities differ. :-(

@CharliePoole
Copy link

Issue of semantics... I took "parse the command line" to mean getting a long string and figuring out the boundaries between the args. That's done, but rarely.

Also, I wasn't aware Cake.exe was a C++ program. 😕

@jnm2
Copy link
Contributor Author

jnm2 commented Sep 8, 2017

Gotcha. Cake is C#, but every runtime library follows C's argv convention regardless of language and (I think) OS. CommandLineToArgvW in Windows, which the CLR uses when it calls your Main method.

(Correcting myself, I think it's the C runtime, not C++, whose convention every library follows now.)

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