TestWTF tests compilation unit names clash with WTF implementation compilation unit names, causing linker warnings defaults write org.webkit.BuildConfiguration BuildTranscriptVerbosity normal make debug .... CompileC WorkQueue.o CompileC WTFString.o CompileC WTFStringUtilities.o Libtool libTestWTF /Volumes/.../usr/bin/libtool: warning same member name (AtomString.o) in output file used for input files: /Users/kkinnunen/Build/TestWebKitAPI.build/Debug/TestWTFLibrary.build/Objects-normal/x86_64/AtomString.o and: /Users/kkinnunen/Build/Debug/libWTF.a(AtomString.o) due to use of basename, truncation and blank padding ... GenerateDSYMFile TestWTF.dSYM warning: (x86_64) could not find object file symbol for symbol __ZNK3WTF10AtomString23convertToASCIILowercaseEv warning: (x86_64) could not find object file symbol for symbol __ZNK3WTF10AtomString16convertASCIICaseILNS0_15CaseConvertTypeE1EEES0_v warning: (x86_64) could not find object file symbol for symbol ... Causes over 4000 warnings, hiding warnings that might be real bugs..
This looks like a weird thing to warn about. What’s wrong with using the same file name in different directories?
https://github.com/godotengine/godot/issues/24419 I think the issue is that the file format is packing flat files without directories. Libtool sources apparently says it’s not really a problem, but the warning is there since user might extract the archive with at and then the duplicate files would be overwritten with the last file. However, this fixes a lot of “symbol not found” warnings in dSYM phase, probably of something that has real world value
Well, actually ”file not found for symbol”
<rdar://problem/77255847>
Alexey, how about this change? I don't think the master object relink can be used with WTF. Typically one avoids having same file names in the project, so it's easier to open files. E.g. now if you try to open WTFString.cpp, you might open either the test or the implementation. The fix here (which fixes about 4000 ld warnings) has the side-effect of moving the tests to XXXTest.cpp. Using the "Test" prefix is also what David suggested me for a new test.
I think that you are ahead of me in understanding the issue, and if there is still no path to other solution in sight, then let's do this. One thing I'd suggest is starting a discussion on webkit-dev, because people need to be aware that we are enforcing limitations on naming files, notably across multiple projects. We'd probably also need a way to make it so that it doesn't regress in the future.
> One thing I'd suggest is starting a discussion on webkit-dev, because people need to be aware that we are enforcing limitations on naming files, notably across multiple projects. We'd probably also need a way to make it so that it doesn't regress in the future. Something we could do in the Xcode build is have a build rule that changes of the names of TestWTF sources before they would be handed to the compiler. i.e., the Xcode equivalent of: ``` %_test.cpp : % cp $< $@ ``` This would make the suffix be automatic, albeit not guaranteed to never collide. Something I don't quite understand is -- why does libTestWTF.a need to merge itself with libWTF.a? I would think that these libraries can be linked in to the TestWTF binary separately, and would never share a static archive to have name collisions in.
(In reply to Elliott Williams from comment #7) > > One thing I'd suggest is starting a discussion on webkit-dev, because people need to be aware that we are enforcing limitations on naming files, notably across multiple projects. We'd probably also need a way to make it so that it doesn't regress in the future. Yeah, I think check-webkit-style check or turning on warning-as-error for the link problem would fix it. > Something we could do in the Xcode build is have a build rule that changes > of the names of TestWTF sources before they would be handed to the compiler. > i.e., the Xcode equivalent of: > > ``` > %_test.cpp : % > cp $< $@ This sounds like a convoluted fix for a problem. Generated sources always slow the build down and make it more hard to understand the dependencies than needed. There's also always issues with fringe use-cases. Like, debugging a massive template or preprocessing failure, I myself run the compile commands manually and modify the source file. Copy generated source case, this would fail as the edits would go to the original file, but the manual cut-pasted compile mantra would compile the copied file. Generated dummy file that does #include "OriginalFile.cpp" would work, but probably has problems with include dirs. > ``` > Something I don't quite understand is -- why does libTestWTF.a need to merge > itself with libWTF.a? I would think that these libraries can be linked in to > the TestWTF binary separately, and would never share a static archive to > have name collisions in. You're the compile expert, so I hesitate to speculate, but.. I don't understand the the thought. The binary production link is atomic, nothing that can be done separately. E.g. TestWTF = libTestWTF.a + libWTF.a. That is the merge step. If the question is that "why tests are compiled into libTestWTF.a instead of linking to the TestWTF directly?" (TestWTF = TestXXX.o + TestABC.o + ... + libWTF.a), that question I understand. I don't remember definitively, but it might have something to do with different compile flags for the binary and the libTestWTF targets last time I tried to understand why it was done. Note: there were name clashes between WTF, JSC, bmalloc last time I looked, so solving it for the libTestWTF isn't still a good solution to get warning-free link. Requiring unique names within binaries seems like a livable requirement for me. I'd imagine this could be implemented by hardcoding an estimate of directories -> binaries in check-webkit-style.
FWIW: XCode seems to understand the problem, since in some conditions it will rename the .o target automatically. This is an example warning that reveals that: ld: warning: cannot export hidden symbol webrtc::RtpSenderInterface::dtls_transport() const from /Users/kkinnunen/Build/libwebrtc.build/Debug/libwebrtc.build/Objects-normal/arm64e/rtp_sender-146c1e31017971145ecf09b9c6cb398e.o (We should fix the warning, btw) I think it's because the duplicate rtp_sender.cc compilations are part of the same project, so the object file will be produced with hash of .. something. Unfortunately I don't see a way to enforce this manually. That being said, if the WTF tests would exist in the WTF project, WebCore tests in WebCore project, like in normal projects, Xcode would be successful.
> I don't understand the the thought. The binary production link is atomic, nothing that can be done separately. E.g. TestWTF = libTestWTF.a + libWTF.a. That is the merge step. This issue we have now is that TestWTF is *not* the merge step, libTestWTF.a is. The linkage occurs like this. libTestWTF.a = AtomString.o + ... + WorkQueue.o + libWTF.a TestWTF = mainMac.o + libTestWTF.a I think that we can restructure it to work like you're describing, where TestWTF = mainMac.o + libTestWTF.a + libWTF.a and that will fix the issue because TestWTF and WTF objects will never coexist in the same static archive. > XCode seems to understand the problem, since in some conditions it will rename the .o target automatically. Unfortunately for us, it only uniques source file names per *target*, not per project. Which makes sense because, for instance, Xcode writes all of a target's object files to the same directory.
Pull request: https://github.com/WebKit/WebKit/pull/16858
Committed 267099@main (ce7e7da9c4dc): <https://commits.webkit.org/267099@main> Reviewed commits have been landed. Closing PR #16858 and removing active labels.