resources icon indicating copy to clipboard operation
resources copied to clipboard

Fixing build.sh and argument parsing consistency

Open jnm2 opened this issue 6 years ago • 9 comments

My PR https://github.com/cake-build/resources/pull/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 avatar Aug 25 '17 03:08 jnm2

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 avatar Aug 25 '17 03:08 jnm2

PR with just the fix: https://github.com/cake-build/resources/pull/34

jnm2 avatar Aug 25 '17 04:08 jnm2

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.

CharliePoole avatar Sep 08 '17 19:09 CharliePoole

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 avatar Sep 08 '17 19:09 jnm2

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.

jnm2 avatar Sep 08 '17 20:09 jnm2

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

CharliePoole avatar Sep 08 '17 20:09 CharliePoole

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. :-(

jnm2 avatar Sep 08 '17 20:09 jnm2

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. 😕

CharliePoole avatar Sep 08 '17 21:09 CharliePoole

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.)

jnm2 avatar Sep 08 '17 22:09 jnm2