Skip to content

Gate external-repo source dir package boundary checks behind a flag#30019

Closed
fmeum wants to merge 2 commits into
bazelbuild:masterfrom
fmeum:incompatible-check-external-repo-source-dir-package-boundary
Closed

Gate external-repo source dir package boundary checks behind a flag#30019
fmeum wants to merge 2 commits into
bazelbuild:masterfrom
fmeum:incompatible-check-external-repo-source-dir-package-boundary

Conversation

@fmeum

@fmeum fmeum commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Description

#29692 made the package-boundary check for source directories in RecursiveFilesystemTraversalFunction repo-aware. Besides fixing the false positives reported in #29688, it also made the check effective for source directories in external repos, which is a breaking change.

This PR gates that new enforcement behind --incompatible_check_external_repo_source_dir_package_boundary (default off).

When the flag is off, package boundary crossings by source directories in external repositories are skipped entirely rather than reverting to the old main-repo lookup. As a result, #29688 stays fixed regardless of the flag value.

Motivation

#29688 (comment)

Build API Changes

Yes: adds the command-line flag --incompatible_check_external_repo_source_dir_package_boundary.

  1. Has this been discussed in a design doc or issue? N/A
  2. Is the change backward compatible? Yes. The flag defaults to off, preserving the pre-Fix repo collisions when checking package traversals of source dirs #29692 (9.1.x) behavior for source directories in external repositories.
  3. If it's a breaking change, what is the migration plan? Enabling the flag (eventually flipping the default to on in a future major release) restores the strict, repo-aware enforcement.

Checklist

  • I have added tests for the new use cases (if any).
  • I have updated the documentation (if applicable).

Release Notes

RELNOTES: Source directories in external repositories are temporarily allowed to cross package boundaries. This will be disallowed in the future, gated by the new --incompatible_check_external_repo_source_dir_package_boundary flag.

Besides fixing the false positives reported in bazelbuild#29688, this also started
enforcing a genuinely new class of errors: a source directory in an
external repository that crosses a package boundary into a sub-package of
that same external repository. Previously the lookup always resolved
against the main repository, so such crossings inside external
repositories went undetected and the build succeeded.

That is an incompatible change: builds that worked on 9.1.1 now fail (see
the repro in bazelbuild#29688). Gate the new enforcement behind
--incompatible_check_external_repo_source_dir_package_boundary
(default off).

When the flag is off, package boundary crossings by source directories in
external repositories are skipped entirely rather than reverting to the
old main-repo lookup, so bazelbuild#29688 stays fixed regardless of the flag value;
only the new strictness is deferred. Source directories in the main
repository are unaffected.
@fmeum fmeum force-pushed the incompatible-check-external-repo-source-dir-package-boundary branch from 5cd39dd to 1d3f3d3 Compare June 26, 2026 09:36
@fmeum fmeum marked this pull request as ready for review June 26, 2026 09:37
@github-actions github-actions Bot added team-Core Skyframe, bazel query, BEP, options parsing, bazelrc awaiting-review PR is awaiting review from an assigned reviewer team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob labels Jun 26, 2026
@fmeum fmeum requested a review from Wyverald June 26, 2026 09:37
@fmeum

fmeum commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

@bazel-io fork 9.2.0

effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
help =
"If true, a source directory in an external repository fails the build if it crosses a"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just a quick comment: is "source directory" the well-known name for this concept? This sentence is very confusing to me without reading the integration set up in SourceDirectoryIntegrationTest.java ("how can a directory cross package boundaries?").

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the rollout flag was named "track source directories" and this has been the term used for them (unfortunately we say tree artifact instead of generated directory...)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The source directory / package interaction is legitimately confusing. Happy to reword it you think that this should be clarified, I'm just not sure how.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok, thanks for the context. I don't have a much better idea either to be honest, so let's keep it as is for now.

…ectoryIntegrationTest.java

Co-authored-by: Xùdōng Yáng <wyverald@gmail.com>
@fmeum fmeum requested a review from Wyverald June 26, 2026 16:31
@github-actions github-actions Bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Jun 26, 2026
@fmeum fmeum deleted the incompatible-check-external-repo-source-dir-package-boundary branch June 26, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Core Skyframe, bazel query, BEP, options parsing, bazelrc team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants