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.

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

Ignore convenience symlinks in glob expansion #22128

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

illicitonion
Copy link
Contributor

@illicitonion illicitonion commented Apr 25, 2024

It's slightly unfortunate that this require moving symlink-related options to an options scope that applies to query (because query also needs to know to ignore the symlinks when expanding globs).

It's also slightly unfortunate that the behaviour of the symlink-related flags is really about changing the existence of symlinks - the existence of the "ignore" value is somewhat problematic as it means that evaluation of globs may vary depending on the outcomes of previous builds' options, but this better (and more niche) than the current state which is that a first build and subsequent build with the same options may give different results (because the first build may produce files in bazel-bin which are then picked up as inputs by the second build).

Fixes #11875

@illicitonion illicitonion requested a review from a team as a code owner April 25, 2024 16:25
@illicitonion illicitonion requested review from katre and removed request for a team April 25, 2024 16:25
@github-actions github-actions bot added team-Configurability Issues for Configurability team awaiting-review PR is awaiting review from an assigned reviewer labels Apr 25, 2024
@illicitonion illicitonion force-pushed the globs-ignore-convenience-symlinks branch from 2fe8a50 to a473312 Compare April 25, 2024 16:51
@illicitonion illicitonion changed the title Glob expansion ignores convenience symlinks Ignore convenience symlinks in glob expansion Apr 26, 2024
@illicitonion illicitonion force-pushed the globs-ignore-convenience-symlinks branch from a473312 to e44f32a Compare April 26, 2024 09:47
@illicitonion illicitonion marked this pull request as draft April 26, 2024 10:07
@illicitonion illicitonion force-pushed the globs-ignore-convenience-symlinks branch 4 times, most recently from 6f48362 to 39853d4 Compare April 26, 2024 13:11
@illicitonion illicitonion marked this pull request as ready for review April 26, 2024 13:53
@github-actions github-actions bot added team-Performance Issues for Performance teams team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. labels Apr 26, 2024
@illicitonion
Copy link
Contributor Author

illicitonion commented Apr 26, 2024

Note that this PR depends on #22142 - the first two commits are that PR, and the third is new content here. The dependent PR merged 🎉

It's slightly unfortunate that this require moving symlink-related
options to an options scope that applies to query (because query _also_
needs to know to ignore the symlinks when expanding globs).

It's also slightly unfortunate that the behaviour of the symlink-related
flags is really about _changing_ the existence of symlinks - the
existence of the "ignore" value is somewhat problematic as it means that
evaluation of globs may vary depending on the outcomes of previous
builds' options, but this better (and more niche) than the current state
which is that a first build and subsequent build with the _same_ options
may give different results (because the first build may produce files in
bazel-bin which are then picked up as inputs by the second build).
@illicitonion illicitonion force-pushed the globs-ignore-convenience-symlinks branch from 39853d4 to 586f3e5 Compare April 29, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Configurability Issues for Configurability team team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Performance Issues for Performance teams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

recursive glob in top-level package follows "bazel-*" symlinks
1 participant