[llvm-ar] Support an options string that start with a dash

[mstorsjo]

Summary

Some projects call $AR like "$AR -crs output input1 input2".

This implementation isn't very pretty - any suggestions on a better way of handling it?

[smeenai]

Not sure of a better implementation strategy, but I am a fan of adding support for the dashed variants, since I've hit the issue as well in the past.

[rnk]

lgtm

tools/llvm-ar/llvm-ar.cpp

894

Let's break if argv[i] is "--". That way, users have a way of escaping positional arguments that happen to match this pattern.

[ruiu]

Why don't you just strip the leading dash? I think the first argument is always flags (e.g. ar somesome.a doesn't make sense), so we could always handle ar -something as if ar something.

[mstorsjo]

In D39538#914200, @ruiu wrote:

Why don't you just strip the leading dash? I think the first argument is always flags (e.g. ar somesome.a doesn't make sense), so we could always handle ar -something as if ar something.

The big issue is that llvm-ar has got options of its own that are dash/doubledash-prefixed, so you can have llvm-ar -format gnu [-]crs foo.a obj.o. We don't know what of these are options and what are "all the other args" until we run the command line parser.

[ruiu]

Ah, makes sense. Thank you for explaining.

[mstorsjo]

tools/llvm-ar/llvm-ar.cpp

894

Ok, will do. I'll also break once we strip a dash from some argument, since only one argument should have such a dash.

[mstorsjo]

Break out of the loop on '--' and once an argument with a stripped dash is found.

[ruiu]

tools/llvm-ar/llvm-ar.cpp

892–893

If you are going to use StringRef, then something like this is probably better than using stdlib's string functions.

StringRef S = argv[i];
if (S.startswith("-") && S.find_first_of("dmpqrtxabiosSTucv") == StringRef::npos)

[mstorsjo]

tools/llvm-ar/llvm-ar.cpp

892–893

Ok, I can do something like that. Although I prefer to keep the definition of the chars string where I have it right now (close to the docs listing the option chars).

[smeenai]

tools/llvm-ar/llvm-ar.cpp

892–893

Did you mean find_first_not_of?

[ruiu]

tools/llvm-ar/llvm-ar.cpp

892–893

Ah, yes. Thank you for the correction.

test/tools/llvm-ar/default-add.test

 RUN: yaml2obj %S/Inputs/macho.yaml -o %t-macho.o
 RUN: yaml2obj %S/Inputs/coff.yaml -o %t-coff.o
 
 RUN: rm -f %t.ar
 RUN: llvm-ar crs %t.ar %t-macho.o
 RUN: grep -q __.SYMDEF %t.ar
-RUN: llvm-ar crs %t.ar %t-coff.o
+Test that an option string prefixed by a dash works.
+RUN: llvm-ar -crs %t.ar %t-coff.o
 RUN: grep -q __.SYMDEF %t.ar
 
 RUN: rm -f %t.ar
 RUN: llvm-ar crs %t.ar %t-coff.o
 RUN: not grep -q __.SYMDEF %t.ar
 RUN: llvm-ar crs %t.ar %t-macho.o
 RUN: not grep -q __.SYMDEF %t.ar
 
 Ensure that we select the existing format when updating.
 

tools/llvm-ar/llvm-ar.cpp

static cl::extrahelp MoreHelp(
   "  [S] - do not build a symbol table\n"
   "  [T] - create a thin archive\n"
   "  [u] - update only files newer than archive contents\n"
   "\nMODIFIERS (generic):\n"
   "  [c] - do not warn if the library had to be created\n"
   "  [v] - be verbose about actions taken\n"
 );
 
+static const char OptionChars[] = "dmpqrtxabiosSTucv";
+
 // This enumeration delineates the kinds of operations on an archive
 // that are permitted.
 enum ArchiveOperation {
   Print,            ///< Print the contents of the archive
   Delete,           ///< Delete the specified members
   Move,             ///< Move members to end or as given by {a,b,i} modifiers
   QuickAppend,      ///< Quickly append to end of archive
   ReplaceOrInsert,  ///< Replace or Insert members
int main(int argc, char **argv) {
   StringRef Stem = sys::path::stem(ToolName);
   if (Stem.find("dlltool") != StringRef::npos)
     return dlltoolDriverMain(makeArrayRef(argv, argc));
 
   if (Stem.find("ranlib") == StringRef::npos &&
       Stem.find("lib") != StringRef::npos)
     return libDriverMain(makeArrayRef(argv, argc));
 
+  for (int i = 1; i < argc; i++) {
+    // If an argument starts with a dash and only contains chars
+    // that belong to the options chars set, remove the dash.
+    // We can't handle it after the command line options parsing
+    // is done, since it will error out on an unrecognized string
+    // starting with a dash.
+    // Make sure this doesn't match the actual llvm-ar specific options
+    // that start with a dash.
+    if (argv[i][0] == '-' &&
+        strspn(&argv[i][1], OptionChars) + 1 == strlen(argv[i])) {
+      argv[i]++;
+      break;
+    }
+    if (!strcmp(argv[i], "--"))
+      break;
+  }
+
   // Have the command line options parsed and handle things
   // like --help and --version.
   cl::ParseCommandLineOptions(argc, argv,
     "LLVM Archiver (llvm-ar)\n\n"
     "  This program archives bitcode files into single libraries\n"
   );
 
   if (Stem.find("ranlib") != StringRef::npos)
     return ranlib_main();
   if (Stem.find("ar") != StringRef::npos)
     return ar_main();
   fail("Not ranlib, ar, lib or dlltool!");
 }