[PATCH] D54726: [libcxx] Support generating linker script for static library
Louis Dionne via Phabricator
reviews at reviews.llvm.org
Mon Nov 19 16:04:26 PST 2018
ldionne added inline comments.
Comment at: libcxx/utils/gen_link_script.py:11
> 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
- 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.
More information about the libcxx-commits