Add correct archiver tool for clang on win32 #3244
Conversation
Signed-off-by: Mats Wichmann <mats@linux.com>
test/Clang/clang_static_library.py
Outdated
| foo_lib = 'foo.lib' | ||
| archiver = 'mslib' | ||
| import SCons.Tool.MSCommon as msc | ||
| if not msc.msvc_exists(): |
There was a problem hiding this comment.
This test will fail if no mslib right?
There was a problem hiding this comment.
I assume so. Is that a problem? As it currently stands, it is failing because there is no 'ar'.
There was a problem hiding this comment.
in that case shouldn't it skip if no msvc_exists? rather than default to ar which won't work with clang?
There was a problem hiding this comment.
Oh, I see. Possibly: you tell me. I don't know if there's any "other path" on windows. I'm not sure how much it makes sense for msvc to have to exist either (as I said, that stanza is copied from another test)... what if people try to install clang for building instead of msvc? Unfortunately I'm just guessing here. It's not really a good place to be developing from, better if one had specific use cases and wasn't guessing what folks might do.
There was a problem hiding this comment.
It was my understanding that you need msvc tools to build viable dlls, libs, and exe's with clang? (doesn't it call msvc linker under the hood?)
There was a problem hiding this comment.
If you use the choco-packaged clang, which is the llvm-built-for-the-Microsoft-way-of-doing-things model of windows. But there are other options too. See for example here: https://github.com/boostorg/hana/wiki/Setting-up-Clang-on-Windows which cites four possible combinations, two of which helpfully don't appear to work :)
This is why I mentioned use-cases: not sure if the expectation is clang will be used both msvc-style and mingw-style.
There was a problem hiding this comment.
It's probably the case that we need a separate clang-cl tools (-cl aims to process msvc style command line flags instead of "normal/posix" type compiler flags)
There was a problem hiding this comment.
I saw that... maybe use the github project tab for logging those kinds of work items?
There was a problem hiding this comment.
might be overkill. just add a tag to the issue maybe. for now until someone asks for it, there's many other issues to work on.
test/Clang/clang_static_library.py
Outdated
| @@ -32,11 +34,23 @@ | |||
| if not test.where_is('clang'): | |||
| test.skip_test("Could not find 'clang', skipping test.\n") | |||
|
|
|||
| if sys.platform == 'win32': | |||
There was a problem hiding this comment.
You can use:
test.IS_WINDOWS
There was a problem hiding this comment.
Sure, no problem, but grep tells me, in test/, 200-ish direct checks for win32, two uses of IS_WINDOWS. If you want to pursue a strategy of "if you touch a test, convert it" that's fine - let's just write it down somewhere.
There was a problem hiding this comment.
Yes. If you touch it, make it better is the general policy.. improve docstrings, remove redundant code,etc..
Signed-off-by: Mats Wichmann <mats@linux.com>
Do some work to figure out which tool to initialize with so that StaticLibrary works out for clang.
Contributor Checklist:
master/src/CHANGES.txtdirectory (and read theREADME.txtin that directory)