labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.

This will break a several use cases we are relying on. More details inline.


================
Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:288
@@ -289,3 +287,3 @@
 #----------------------------------------------------------------------
 ifeq "$(OS)" "Android"
     ifdef PIE
----------------
Please remove the `if android` part and move it to the generic section above.

================
Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:294
@@ -307,1 +293,3 @@
 
+CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \
+                             $(findstring clang,$(CC)), \
----------------
This code will now be under "android-specific options" although it is not. 
Please move it up, above "windows specific options".

================
Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:304
@@ +303,3 @@
+                       $(subst $(3),$(1),$(2)), \
+                       $(if $(findstring ar,$(1)), \
+                            $(if $(findstring gcc,$(3)), \
----------------
I think like we should replace ar with gcc-ar always, and not just when we 
don't have a version suffix present. When I have a gcc installed to a custom 
prefix, I get `$prefix/gcc-ar`, but not `$prefix/ar`. (Previously we would just 
use `ar` unconditionally, which was probably a bug.)

Next, when you specify a clang compiler as `clang-3.5`, you will produce 
`ar-3.5`, which almost certainly does not exist. I think that in case of clang 
we should just strip the version suffix (produce `XXX-ar`) and hope for the 
best (should work for all our current use cases).

Finally, I don't think objcopy is ever versioned with gcc (correct me if I am 
wrong), so I think that in case of objcopy we should strip the version suffix 
unconditionally `XXX-objcopy`.

So, to summarize, these are the transformations I think are wrong:
`foo/gcc` -> `foo/ar` (should be `foo/gcc-ar`)
`clang-3.5` -> `ar-3.5` (should be `ar`)
`gcc-4.8` -> `objcopy-4.8` (should be `objcopy`)

Let me know if these are compatible with your requirements. If we can't find a 
set of rules that work everywhere, we will have to abandon the magic (or maybe 
just leave a simple one), and require the user to specify the paths manually...

================
Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:317
@@ +316,3 @@
+OBJCOPY = $(call replace_cc_with,objcopy)
+AR = $(call replace_cc_with,ar)
+
----------------
This will override the darwin-specific setting of AR on line 130, which it 
probably shouldn't. I guess this should be `ifneq $(OS) darwin`


https://reviews.llvm.org/D20386



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to