[PATCH] D54726: [libcxx] Support generating linker script for static library

[Louis Dionne via Phabricator]

ldionne added inline comments.


================
Comment at: libcxx/utils/gen_link_script.py:11
 
+import argparse
 import os
----------------
phosek wrote:
> ldionne wrote:
> > Can you confirm that there are no functional changes beyond adding support for a custom output file in this script? This is only a refactoring (thanks!), or did I miss something?
> The original script did not support creating a linker script for static library, so there's a functional change, but I also refactored the code a bit to make it (subjectively) simpler. I'd be fine doing the refactoring as a separate change if you prefer?
No, I'm fine with the refactoring + the functional change, I was just trying to spot what the functional change was. I'm sorry, but I still don't see what prevented the previous script from handling static archives. Can you please point it out?


================
Comment at: libcxx/utils/gen_link_script.py:17
 def main():
-    dryrun, symlink_file, public_libs = parse_args()
-
-    # Check that the given libc++.so file is a valid symlink.
-    if not os.path.islink(symlink_file):
-        print_and_exit("symlink file %s is not a symlink" % symlink_file)
-
-    # Read the symlink so we know what libc++ to link to in the linker script.
-    linked_libcxx = os.readlink(symlink_file)
+    parser = argparse.ArgumentParser()
+    parser.add_argument("--dryrun", help="Don't write any output",
----------------
Please add general description of what the script does, like there used to be.


================
Comment at: libcxx/utils/gen_link_script.py:22
+                        action="store_true", default=False)
+    parser.add_argument("input", help="Path to libc++ library")
+    parser.add_argument("output", help="Path to libc++ linker script")
----------------
Please consider making these named parameters instead of positional parameters. That would have answered some of my questions before I had to ask them.


Repository:
  rCXX libc++

https://reviews.llvm.org/D54726