[llvm-ar] Enforce one-dash form for long options

[MaskRay]

Summary

binutils ar does not support one-dash long options (they conflict with
operation code and modifier flags).

% ar -help
ar: invalid option -- 'e'
...
% ar -version
ar: invalid option -- 'e'
...

% ar x --plugin=xx x.a  # ok
% ar x -plugin=xx x.a
ar: two different operation options specified
% ar -plugin=/usr/lib/gcc/x86_64-linux-gnu/10/liblto_plugin.so x x.a
ar: x: No such file or directory

Drop one-dash long options to simplify code and match the usual practice for
command line utilities.

[jhenderson]

I'm mostly fine with this.

llvm/test/tools/llvm-ar/plugin.test

1

Seems tangential to this patch? Test is fine, but probably should be committed separately.

Any particular reason you don't test the single-dash version here, but you do in the --format and --version options?

[MaskRay]

llvm/test/tools/llvm-ar/plugin.test

1

Yes, I plan to commit this separately.

Any particular reason you don't test the single-dash version here, but you do in the --format and --version options?

Just thought it seems excessive to test every long option. --format and --version have good coverage.

[gbreynoo]

LGTM but I will hold of accepting this revision until James has seen the responses.

[jhenderson]

LGTM.

[MaskRay]

llvm/test/tools/llvm-ar/plugin.test

1

Pushed separately in f8476fd47b539a14d6a8994e7a5e6401f9ed2a98

[sbc100]

This change broke an emscripten test:

https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8825145660107382369/overview
https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8825145660107382369/+/u/Emscripten_testsuite__upstream__other_/stdout

The test in question was using -version with a single dash:

https://github.com/emscripten-core/emscripten/blob/a246105a147e4a08981c4bdbe4e10f4103410604/tests/test_other.py#L570-L576

[sbc100]

In D116977#3240895, @sbc100 wrote:

This change broke an emscripten test:

https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8825145660107382369/overview
https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8825145660107382369/+/u/Emscripten_testsuite__upstream__other_/stdout

The test in question was using -version with a single dash:

https://github.com/emscripten-core/emscripten/blob/a246105a147e4a08981c4bdbe4e10f4103410604/tests/test_other.py#L570-L576

Converted test so use the double-dash form, but I guess there could be other folks out there who get broken too?:

https://github.com/emscripten-core/emscripten/pull/16007

[MaskRay]

In D116977#3240912, @sbc100 wrote:

In D116977#3240895, @sbc100 wrote:

This change broke an emscripten test:

https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8825145660107382369/overview
https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8825145660107382369/+/u/Emscripten_testsuite__upstream__other_/stdout

The test in question was using -version with a single dash:

https://github.com/emscripten-core/emscripten/blob/a246105a147e4a08981c4bdbe4e10f4103410604/tests/test_other.py#L570-L576

Converted test so use the double-dash form, but I guess there could be other folks out there who get broken too?:

https://github.com/emscripten-core/emscripten/pull/16007

If there is a justifiable issue, we can temporarily add -version, otherwise we don't need to take more defensive stance.

llvm/test/Object/archive-format.test

 see that there is an empty symbol table.
 RUN: mkdir -p %t
 RUN: yaml2obj %S/Inputs/solaris-nosymbols.yaml -o %t/foo.o
 RUN: llvm-ar rs %t/foo.a %t/foo.o
 RUN: cat -v %t/foo.a | FileCheck -strict-whitespace --check-prefix=SOLARIS %s
 SOLARIS:      !<arch>
 SOLARIS-NEXT: /               0           0     0     0       8         `
 SOLARIS-NEXT: ^@^@^@^@^@^@^@^@foo.o/
+
+RUN: not llvm-ar -format=gnu rc %t.a 0123456789abcde 2>&1 | FileCheck --check-prefix=ONE-DASH %s
+ONE-DASH: error: unknown option f

llvm/test/tools/llvm-ar/plugin.test

+## Test that --plugin is ignored.
+
+# RUN: rm -f %t.a
+# RUN: touch %t.txt
+# RUN: llvm-ar rc %t.a %t.txt
+# RUN: llvm-ar --plugin ignore t %t.a | FileCheck %s
+# RUN: llvm-ar t --plugin=ignore %t.a | FileCheck %s
+
+# CHECK: {{.*}}.txt

llvm/test/tools/llvm-ar/response.test

 # RUN:   FileCheck  %s --check-prefix=ERROR
 # ERROR: Invalid response file quoting style foobar
 
 # RUN: echo -e 'rc %/t.a blah\\foo' > %t-rsp.txt
 # RUN: not llvm-ar --rsp-quoting=windows @%t-rsp.txt 2>&1 | \
 # RUN:   FileCheck -DMSG=%errc_ENOENT %s --check-prefix=WIN
 # WIN: error: blah\foo: [[MSG]]
 
-# RUN: not llvm-ar -rsp-quoting posix @%t-rsp.txt 2>&1 | \
+# RUN: not llvm-ar --rsp-quoting posix @%t-rsp.txt 2>&1 | \
 # RUN:   FileCheck -DMSG=%errc_ENOENT %s --check-prefix=POSIX
 # POSIX: error: blahfoo: [[MSG]]

llvm/test/tools/llvm-ar/version.test

 ## Show that the version for llvm-ar can be printed with either the long flag
 ## -version or with the V modifier.
 
 RUN: llvm-ar V | FileCheck %s
 RUN: llvm-ar xV | FileCheck %s
 RUN: llvm-ar -V | FileCheck %s
 RUN: llvm-ar -xV | FileCheck %s
-RUN: llvm-ar -version | FileCheck %s
 RUN: llvm-ar --version | FileCheck %s
 
 CHECK: version
+
+RUN: not llvm-ar -version 2>&1 | FileCheck %s --check-prefix=ONE-DASH
+
+ONE-DASH: error: unknown option e

llvm/tools/llvm-ar/llvm-ar.cpp

static void runMRIScript() {
 
   // Nothing to do if not saved.
   if (Saved)
     performOperation(ReplaceOrInsert, &NewMembers);
   exit(0);
 }
 
 static bool handleGenericOption(StringRef arg) {
-  if (arg == "-help" || arg == "--help" || arg == "-h") {
+  if (arg == "--help" || arg == "-h") {
     printHelpMessage();
     return true;
   }
-  if (arg == "-version" || arg == "--version") {
+  if (arg == "--version") {
     cl::PrintVersionMessage();
     return true;
   }
   return false;
 }
 
 static const char *matchFlagWithArg(StringRef Expected,
                                     ArrayRef<const char *>::iterator &ArgIt,
                                     ArrayRef<const char *> Args) {
   StringRef Arg = *ArgIt;
 
   if (Arg.startswith("--"))
     Arg = Arg.substr(2);
-  else if (Arg.startswith("-"))
-    Arg = Arg.substr(1);
 
   size_t len = Expected.size();
   if (Arg == Expected) {
     if (++ArgIt == Args.end())
       fail(std::string(Expected) + " requires an argument");
 
     return *ArgIt;
   }