Python eggs may be another option which might be an easier way forward.

At least for the from-source distribution, where the destination of the image is the build host, Python_EXECUTABLE is definitely preferable (e.g. #!@Python_EXECUTABLE@). Furthermore, there is no reason to assume that /usr/bin/env python3 will give you the same python 3 as what was given to cmake (you can control that with -DPython_EXECUTABLE=...). I think that requiring the user to specify the tool is possibly the best option - that is completely remove the shebang as that ensures that shifts the burden of selection to the user. Another option may be to use something like p2exe and create self-contained tools.

Ugh, this is still not the most structured handling of the arguments. But, yeah, this seems like it should be equivalent. Fine by me if @aaron.ballman has no more comments.

Tests?

Seems like it should be equivalent.

Hmm, yes, it is for use from the build tree. I don't see how INTERFACE_INCLUDE_DIRECTORIES can do a -include though. It has to be INTERFACE_COMPILE_OPTIONS or INTERFACE_COMPILE_DEFINITIONS.

@lhames - I like the logAllUnhandledErrors!

@gribozavr I think that this usage here is actually useful because it
a) tests the actual behaviour
b) provides example code for other users

-ldl doesn't work on all platforms (e.g. android, FreeBSD, etc). -lpthread is wrong - if you want to add that, I think that we need to improve the -thread-model flag in clang first (it currently just always passes posix, which is ignored; but would identify the threading model). -lpthread is wrong - consider building on Solaris with Solaris threads rather than POSIX threads, or on Windows with the Win32 threading. This really is inline with the work that needs to be finished up with having library link dependencies for static libraries (i.e. #pragma comment(lib, …)).

BTW, I think that we should add a test case to ensure that we see the error in the case that the inotify fds are exhausted. We should be able to create a process and set the limit for that process to 0/1 and use that to trigger the failure.

Rather than silently ignoring tests when the DirectoryWatcher isn't created, can you please print an error message and exit with an error code to indicate the test failed?

SVN r367042

This looks good to me generally. I don't fully understand the reason for u being kept, is that something you intend to clean up in a subsequent patch?

I'm pretty sure that this doesn't work for Visual Studio (where you do not have a cross-compiler).

That is indeed a typo. I somehow ended up copying the driver flag instead of the feature flag. Thanks for fixing this!

Yeah, that makes sense, the common path uses {{.*}} as the value itself is uninteresting.

Thats what I get for not having coffee before responding. objdump is used quite a bit in tests as well (objdump -t | grep ..., objdump -h | grep ..., etc).

objcopy is often used in autotools to run tests of what the compiler is doing. I'm afraid that changing that behaviour will break the autoconf tests which rely on the behaviour (a.out is what the compiler generates by default, and that is why the tool defaults to that filename). So, if you have something like the following:

Yeah, mutations of the hashtable during iteration would be bad. LGTM.

This breaks compatibility with objdump. Please ensure that the original behavior is preserved when invoked as objdump rather than llvm-objdump.

Seems that all the comments have been addressed and this is purely code motion. LGTM

I don't see any cases where [[clang::required]] is tested, am I missing something?

SVN r365562

@ldionne - that was exactly the motivation for this change - it always takes me a couple of reads to figure out what we are trying to do here.

I'm happy to see this finally happening, it makes much more sense.

SVN r365154

Generally, it is nicer to have some more context in the uploaded diff, although, in this case, it doesn't really matter.

The explicit list I think is way better for readability, this is a nice starting point for cleaning this up.

I don't understand the change for the PRIVATE to PUBLIC. That doesn't make a different to the import library handling, which is baked right into CMake, so that will always be handled properly irrespective of the link dependency's visibility. I think the only thing that may be needed is the Core dependency on the TestPlugin.

SVN r364352

@craig.topper, hmm, what happens in terms of CG when LAHF/SAHF are not available? I assume its just worse CG as you could spill AH onto the stack and do a load/store. This actually results in library calls which may not be possible to fulfill.

Move test case around

add additional context and test case

@lebedev.ri - sure, I will add a driver test to ensure that the feature is set on the command line when invoked from the driver, however, I don't think that there is really much in terms of testing that you can do for this type of stuff other than throw a large corpus at it.

Could you please add a test to ensure that Darwin defaults to the old behaviour?