[PATCH] D53913: [llvm-objcopy] Support --{enable, disable}-deterministic-archives

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In https://reviews.llvm.org/D53913#1281632, @ruiu wrote:

> I do not see any problem of adding these flags for the sake of compatibility with GNU. What I wondered is whether or not you really have a need to make it nondeterministic. I thought that we might be able to just ignore these flags.


Personally, I think that if the default is to make them deterministic, then I don't see any reason not to allow overriding this default, as the code does not become significantly more complex with this change. However, I could be persuaded that nobody has this use case.



================
Comment at: test/tools/llvm-objcopy/deterministic-archive.test:22-25
+# RUN: llvm-objcopy --enable-deterministic-archives %t.a %t.4D.a
+# RUN: env TZ=GMT llvm-ar tv %t.4D.a | FileCheck %s --check-prefix=CHECK-DETERMINISTIC
+# RUN: llvm-objcopy --disable-deterministic-archives %t.a %t.4U.a
+# RUN: env TZ=GMT llvm-ar tv %t.4U.a | FileCheck %s --check-prefix=CHECK-NONDETERMINISTIC
----------------
Missing llvm-strip equivalents?


================
Comment at: test/tools/llvm-objcopy/deterministic-archive.test:28-29
+# If unspecified, verify that deterministic is the default.
+# RUN: llvm-objcopy %t.a %t.5.a
+# RUN: env TZ=GMT llvm-ar tv %t.5.a | FileCheck %s --check-prefix=CHECK-DETERMINISTIC
+
----------------
Ditto.


================
Comment at: tools/llvm-objcopy/CopyConfig.cpp:348-349
+      InputArgs.hasArg(OBJCOPY_disable_deterministic_archives))
+    error("Specifying both --enable-deterministic-archives and "
+          "--disable-deterministic-archives is ambiguous");
+  if (InputArgs.hasArg(OBJCOPY_enable_deterministic_archives))
----------------
Is this what GNU objcopy does? I'd actually expect this to be a "last one wins" scenario (similar to various --[no-]some-option in LLD etc.


================
Comment at: tools/llvm-objcopy/CopyConfig.cpp:425-426
+      InputArgs.hasArg(STRIP_disable_deterministic_archives))
+    error("Specifying both --enable-deterministic-archives and "
+          "--disable-deterministic-archives is ambiguous");
+  if (InputArgs.hasArg(STRIP_enable_deterministic_archives))
----------------
Same comment as above.


Repository:
  rL LLVM

https://reviews.llvm.org/D53913