Fix case-insensitive header discovery and validation.#9
Draft
yuzhy8701 wants to merge 1 commit intofmeum:28803-inclusion-checkfrom
Draft
Fix case-insensitive header discovery and validation.#9yuzhy8701 wants to merge 1 commit intofmeum:28803-inclusion-checkfrom
yuzhy8701 wants to merge 1 commit intofmeum:28803-inclusion-checkfrom
Conversation
This change refines how discovered includes are resolved and validated on case-insensitive file systems. Particularly, paths returned by the compiler cannot be trusted to have the correct case. We need to scan action inputs to retrieve the correct artifact. This is important when bazel is running from case-sensitive FS while the the actions run on a case-insensitive FS. Major changes: * `ArtifactFactory`: The `resolveSourceArtifactsAsciiCaseInsensitively` method no longer creates new source artifacts if not found in the cache, as the exact casing is unknown at that point. * `CppCompileAction`: The `validateInclusions` method is refactored to use a map for unvalidated inputs for better efficiency. * `HeaderDiscovery`: The resolution process is rewritten to a multi-stage approach: first checking derived artifacts and the source cache, then action inputs, and finally handling special cases like `cppmap` and tree artifacts. To reduce the memory impact of input scanning, We avoid building map / set from all the inputs - but building from discovered deps instead.
2 tasks
|
Thank you for contributing to the Bazel repository! This pull request has been marked as stale since it has not had any activity in the last 30 days. It will be closed in the next 30 days unless any other activity occurs. If you think this PR is still relevant and should stay open, please post any comment here and the PR will no longer be marked as stale. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 change refines how discovered includes are resolved and validated on case-insensitive file systems. Particularly, paths returned by the compiler cannot be trusted to have the correct case. We need to scan action inputs to retrieve the correct artifact. This is important when bazel is running from case-sensitive FS while the the actions run on a case-insensitive FS.
Major changes:
ArtifactFactory: TheresolveSourceArtifactsAsciiCaseInsensitivelymethod no longer creates new source artifacts if not found in the cache, as the exact casing is unknown at that point.CppCompileAction: ThevalidateInclusionsmethod is refactored to use a map for unvalidated inputs for better efficiency.HeaderDiscovery: The resolution process is rewritten to a multi-stage approach: first checking derived artifacts and the source cache, then action inputs, and finally handling special cases likecppmapand tree artifacts. To reduce the memory impact of input scanning, We avoid building map / set from all the inputs - but building from discovered deps instead.