@arsenm - could write a bitcode upgrade path to deal with the breakage

The early check for ELF is the problem. I care very little about .linker-options itself as long as the ability to ensure that framework linkage is provided (i.e. -framework ..., and '-L ...` can be passed along as options to the linker).

SVN r357778

Am I mistaken in that this effectively prevents the use of options to handle frameworks? That really is a strong requirement and should be part of this patch.

Took a couple of reads to make sure that nothing was really being changed, but seems so. LGTM.

Please check the error message is printed in the case the YAML is empty.

Please add a comment about verify being only for tests.

Hmm, what exactly does the libclang interfaces not give you or what exactly did you intend to have this be used as. Perhaps with some more details we can find a good solution for the specific case that you have in mind.

LG with the minor changes requested.

@rnk, @zturner, @thakis - why don't we merge this for the time being, and once we have setup lit such that it auto-locates git for windows and sets up the path (removing the dependency on GnuWin32), we can remove this entirely?

Please add a test case for this (flowing from IR -> Obj -> llvm-pdbutil)

CC: @zturner

Would be nice if @scanon would also take a look, but, this seems like a good thing to fix.

SVN r354951

This seems right, the Unix side will provide the same interfaces that do nothing in the case that the target doesn't support the backtrace API, and on Windows we provide the same interfaces using DbgHelp if we have access to it.

Nice find!

Shouldn't there be an accompanying change to readobj to display the values for the object files?

@rnk - okay, that seems reasonable - can you please update the documentation on how to setup the Visual Studio Developer command prompt such that you can use the tools (assuming that Git is setup by the Visual Studio Installer) then?

Probably just a replay of GDB protocol packets connected to a testing server would work. But, I don't know if there is infrastructure for that yet.

Previously we would have emitted Foo [[:space:]] . right?

Would be nice to change the filecheck prefixes to CHECK-... to separate the CHECK and the item being checked.

Actually, I'm not sure if that is a bug in libstackunwind, but rather in LLVM. I think that this is the behaviour that GCC has specifically for the eh_frame section. This constraint is not part of the DWARF spec, but eh_frame even though it is similar to debug_frame, is not part of the DWARF spec, it just uses a similar encoding. The change itself looks good, but, please wait for the discussion to complete before committing this.

Thanks!

@rnk @zturner @ruiu - can we come to a decision here?

SVN r354045

@sgraenitz - not really ... the find_package will load the LLVMConfig.cmake that is generated. We only have what we keep in there.

Add HINTS

@labath - absolutely, that I don't have a problem with. I think that having the additional LLDB specific paths with LLDB_PATH_TO_* is better done by using the standard CMake mechanisms.

Yes, both paths currently work since the LLDB_PATH_TO_* variables are just hints to where to look. It just seems unnecessary to have the custom variables when CMake has a mechanism for directing the build to look for packages in a certain location.

LG with the additional test suggestion

@zturner I think that moving to the integrated lit shell is a great idea, as it really does lower the barrier to entry and makes testing stuff on Windows much easier.

Which version of the patch did you use? L13 in the current version sets LLVM_MAIN_INCLUDE_DIR specifically for that case.

@zturner, to your point, I'd like to add the fact that it took a bit of effort to even figure out that the problem was so nicely tucked away in the corner of lit because the failure scenario is pretty absurd (-ENOENT). IMO, this makes this even more insidious because it will cause a lot of people to get confused or possibly give up. But, perhaps I just am holding on to tightly to my idea that things should just work and the testing infrastructure should encourage people to add tests rather than demotivate them.

@sgraenitz yeah, I passed LLVM_DIR and Clang_DIR, but, this is for a standalone build, so I think that it is pretty reasonable to ask that the user tell us where LLVM and Clang are built. Although, if you install LLVM and Clang to your root (like on Linux), you do not need to specify that because it will search the system by default.

@ruiu - to your point of "it being better in the future" I think is false hope. The backwards API compatibility will prevent Microsoft from changing the behaviour of the APIs without an explicit opt-in. Otherwise, applications need to be rewritten, which has long been the case, you can use the NT style paths all the way back to Windows XP I believe, but applications aren't being rebuilt and redistributed.

@ruiu it is only "removed" if you opt into it by modifying the registry on a per program basis. I don't think that is really a reasonable approach.

@kastiglione - bleh, seems that this got lost. This seems like a good fix that we should get in.

Switch to SHFileOperationW for the recursive removal. Refactor lit.util.mkdir_p to expose lit.utl.mkdir and update that to support long file paths. This makes lit more robust on Windows against creating long path temporary directories.

Okay, seems like the way to handle this is to switch to SHFileOperationW using ctypes. I believe that the win32py support that is needed for this should be part of the default python installation on windows (at least it is part of the Visual Studio python distribution).

Ugh, right ... I think that I had tested it against an empty directory! It was run early in the test. I'll flesh this out with @zturner's idea of extract it into a helper.

@ruiu, no unfortunately, not all the paths can be shortened in the swift test suite since it is such a heavy user of the clang modules, modules cache paths and module naming structure in clang is a problem

comments, error handling, context

That doesn't work with python 2.7 as that uses RemoveDirectoryA, which does not support it.

Fixes

Restore LLVM_MAIN_SRC_DIR cache variable

This doesn't break the build, we check that the path exists, and find_program will not error out. You can still specify LLVM_LIT_EXE to specify the lit.py to use. Is that not sufficient for your needs?

@mgorny, no, you should specify -DLLVM_DIR=/path/to/where/the/config/lives instead.

This actually breaks the compatibility with the existing tools. The quoted string is supposed to be interpreted as a *single* entry. I think that if you want to support something like this, it should be a separate flag for this behaviour.