[libcxx-commits] [PATCH] D60309: [libcxx] Update gen_link_script.py to support different input and output

[Petr Hosek via Phabricator via libcxx-commits]

phosek added inline comments.


================
Comment at: libcxx/utils/gen_link_script.py:7
+# This file is dual licensed under the MIT and the University of Illinois Open
+# Source Licenses. See LICENSE.TXT for details.
 #
----------------
thakis wrote:
> I think the LHS license is the new license – probably want to keep the new license?
That's unintentional, I just copied the file from the original patch and forgot to update the header.


================
Comment at: libcxx/utils/gen_link_script.py:38
+    elif not os.path.exists(args.input):
+        # Rename the output so we can replace it with the script.
+        libcxx = os.path.relpath(args.input)
----------------
thakis wrote:
> The LHS didn't do this. Is this for Windows?
It's for the static case, where the input would be `libc++static.a` and output would be `libc++.a`, but `libc++.a` is what's actually being produced by the CMake, in that case the script is going to rename `libc++.a` to `libc++static.a` and generate `libc++.a` linker script. The `--input` and `--output` argument names are a bit misleading in that case, maybe something like `--library` and `--link-script` would be better?


================
Comment at: libcxx/utils/gen_link_script.py:62
-    if not os.path.islink(symlink_file):
-        print_and_exit("symlink file %s is not a symlink" % symlink_file)
-
----------------
thakis wrote:
> We're losing this error, is this intentional?
This is intentional because in cases like the static library, there's no symlink.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60309/new/

https://reviews.llvm.org/D60309