Add deterministic prefix in Library.get_flags() (NFC) #23225
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This factors duplicate routines to add deterministic prefix paths out to `Library.get_cflags()`. Suggested in emscripten-core#23222 (comment).
sbc100
reviewed
Dec 19, 2024
tools/system_libs.py
Outdated
| source_dir = utils.path_from_root() | ||
| if self.batch_inputs: | ||
| relative_source_dir = os.path.relpath(source_dir, build_dir) | ||
| cflags += [f'-ffile-prefix-map={relative_source_dir}={DETERMINISITIC_PREFIX}'] |
There was a problem hiding this comment.
Can you make these two lines unconditional and avoid the need for the new self.batch_inputs?
There was a problem hiding this comment.
Do you have any other comments?
dschuff
approved these changes
Dec 19, 2024
sbc100
approved these changes
Dec 20, 2024
This was referenced Jan 2, 2025
aheejin
added a commit
that referenced
this pull request
Jan 3, 2025
This updates libcxx and libcxxabi to LLVM 19.1.4: /~https://github.com/llvm/llvm-project/releases/tag/llvmorg-19.1.4 The initial update was done using `update_libcxx.py` and `update_libcxxabi.py`, and subsequent fixes were made in indidual commits. The commit history here is kind of messy because of CI testing so not all individual commits are noteworthy. Additional changes: - Build libcxx and libcxxabi with C++23: 8b0bfdf /~https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/src/expected.cpp was added in llvm/llvm-project#87390 and this file assumes C++23 to be compiled. Apparently libc++ sources are always built with C++23 so they don't guard things against it in `src/`: llvm/llvm-project#87390 (comment) This commit also builds libc++abi with C++23 because it doesn't seem there's any downside to it. - Exclude newly added `compiler_rt_shims.cpp`: 5bbcbf0 We have excluded files in /~https://github.com/emscripten-core/emscripten/tree/main/system/lib/libcxx/src/support/win32. This is a new file added in this directory in llvm/llvm-project#83575. - Disable time zone support: a5f2cbe We disabled C++20 time zone support in LLVM 18 update (#21638): df9af64 The list of source files related to time zone support has changed in llvm/llvm-project#74928, so this commit reflects it. - Re-add + update `__assertion_handler` from `default_assertion_handler.in`: 41f8037 This file was added as a part of LLVM 18 update (#21638) in 8d51927 and mistakenly deleted when I ran `update_libcxx.py`. This file was copied from /~https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/vendor/llvm/default_assertion_handler.in, so this also updates the file with the newest `default_assertion_handler.in`. - `_LIBCPP_PSTL_CPU_BACKEND_SERIAL` -> `_LIBCPP_PSTL_BACKEND_SERIAL`: 4b969c3 The name changed in this update, so reflecting it on our `__config_site`. - Directly include `pthread.h` from `emscripten/val.h`: a5a76c3 Due to file rearrangements happened, this was necessary. --- Other things to note: - `std::basic_string<unsigned_char>` is not supported anymore The support for `std::basic_string<unsigned_char>`, which was being used by embind, is removed in this version.#23070 removes the uses from embind. - libcxxabi uses `__FILE__` in more places llvm/llvm-project#80689 started to use `_LIBCXXABI_ASSERT`, which [uses](/~https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxxabi/src/abort_message.h#L22) `__FILE__`, in `private_typeinfo.cpp`. `__FILE__` macro produces different paths depending on the local directory names and how the file is given to clang in the command line, and this file is included in the result of one of our code size tests, `other.test_minimal_runtime_code_size_hello_embind`, which caused the result of the test to vary depending on the CI bots and how the library is built (e.g., whether by embuilder, ninja, or neither). Even though this was brought to surface during this LLVM 19 update, `__FILE__` macro could be a problem for producing reproducible builds anyway. We discussed this problem in #23195, and the fixes landed in #23222, #23225, and #23256.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This factors duplicate routines to add deterministic prefix paths out to
Library.get_cflags(). Suggested in#23222 (comment).