Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TestWTF tests compilation unit names clash with WTF implementation compilation unit names, causing linker warnings #16858

Merged
merged 1 commit into from Aug 21, 2023
Merged

TestWTF tests compilation unit names clash with WTF implementation compilation unit names, causing linker warnings #16858

merged 1 commit into from Aug 21, 2023

Conversation

emw-apple
Copy link
Contributor

@emw-apple emw-apple commented Aug 19, 2023 •
edited by webkit-early-warning-system

ce7e7da

TestWTF tests compilation unit names clash with WTF implementation compilation unit names, causing linker warnings
https://bugs.webkit.org/show_bug.cgi?id=224860
rdar://problem/77255847

Reviewed by Alexey Proskuryakov.

libWTF.a was being merged into libTestWTF.a, causing a
naming collision that resulted in thousands of warnings like

    Libtool libTestWTF
    /Volumes/.../usr/bin/libtool: warning same member name (AtomString.o) in output file used for input files:

and

    GenerateDSYMFile TestWTF.dSYM
    warning: (x86_64)  could not find object file symbol for symbol __ZNK3WTF10AtomString23convertToASCIILowercaseEv

Instead, move the linkage of libWTF.a down into the TestWTF binary.
Tests in TestWTF can have the same file name as their implementation in
WTF without symbol name collisions, because they are never part of the
same static archive.

* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:

Canonical link: https://commits.webkit.org/267099@main

5d9a883

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@emw-apple emw-apple self-assigned this Aug 19, 2023
@emw-apple emw-apple added the Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases label Aug 19, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 332c3d6)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
  🧪 ios-wk2-wpt   🧪 mac-wk1   🧪 gtk-wk2
  🧪 api-ios   🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@emw-apple emw-apple force-pushed the eng/TestWTF-tests-compilation-unit-names-clash-with-WTF-implementation-compilation-unit-names-causing-linker-warnings branch from 332c3d6 to 5d9a883 Compare August 19, 2023 02:17
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 5d9a883)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@emw-apple
Copy link
Contributor Author

EWS shows the warning count greatly reduced: The macOS-AppleSilicon-Ventura-Debug-Build-EWS build emitted 79 warnings, compared to a recent build on the same queue that emitted 4814.

aproskuryakov
aproskuryakov approved these changes Aug 21, 2023
View reviewed changes
Copy link
Contributor

@aproskuryakov aproskuryakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't meaningfully review the pbxproj change, but the explanation makes good sense, and the XML diff for it would indeed look something like this.

@emw-apple emw-apple added the merge-queue Applied to send a pull request to merge-queue label Aug 21, 2023
@emw-apple
TestWTF tests compilation unit names clash with WTF implementation co…
ce7e7da 
…mpilation unit names, causing linker warnings

https://bugs.webkit.org/show_bug.cgi?id=224860
rdar://problem/77255847

Reviewed by Alexey Proskuryakov.

libWTF.a was being merged into libTestWTF.a, causing a
naming collision that resulted in thousands of warnings like

    Libtool libTestWTF
    /Volumes/.../usr/bin/libtool: warning same member name (AtomString.o) in output file used for input files:

and

    GenerateDSYMFile TestWTF.dSYM
    warning: (x86_64)  could not find object file symbol for symbol __ZNK3WTF10AtomString23convertToASCIILowercaseEv

Instead, move the linkage of libWTF.a down into the TestWTF binary.
Tests in TestWTF can have the same file name as their implementation in
WTF without symbol name collisions, because they are never part of the
same static archive.

* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:

Canonical link: https://commits.webkit.org/267099@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/TestWTF-tests-compilation-unit-names-clash-with-WTF-implementation-compilation-unit-names-causing-linker-warnings branch from 5d9a883 to ce7e7da Compare August 21, 2023 18:28
@webkit-commit-queue
Copy link
Collaborator

Committed 267099@main (ce7e7da): https://commits.webkit.org/267099@main

Reviewed commits have been landed. Closing PR #16858 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit ce7e7da into WebKit:main Aug 21, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aproskuryakov aproskuryakov aproskuryakov approved these changes

@kkinnunen-apple kkinnunen-apple Awaiting requested review from kkinnunen-apple

Assignees

@emw-apple emw-apple

Labels
Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
Milestone
No milestone
5 participants
@emw-apple @webkit-early-warning-system @webkit-commit-queue @aproskuryakov @kkinnunen-apple