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

incompatible_disallow_load_labels_to_cross_package_boundaries: make 'load' statement respect package boundaries #6408

Closed
haxorz opened this issue Oct 16, 2018 · 7 comments
Assignees
Labels
incompatible-change Incompatible/breaking change P1 I'll work on this now. (Assignee required)

Comments

@haxorz
Copy link
Contributor

haxorz commented Oct 16, 2018

Description of the problem / feature request:

load('//a:b/c.bzl', 'doesntmatter') currently checks that '//a' is a package but doesn't check that '//a/b' is not.

Feature requests: what underlying problem are you trying to solve with this feature?

we currently have a "label crosses subpackage boundary check" elsewhere (e.g. for target names inside of a package). so we should add this for the sake of consistency.

Any other information, logs, or outputs that you want to share?

we should piggyback/unify the existing mechanism for the existing "label crosses subpackage boundary check" in PackageFunction.java.

@haxorz haxorz self-assigned this Oct 16, 2018
@laurentlb laurentlb added the P1 I'll work on this now. (Assignee required) label Oct 16, 2018
bazel-io pushed a commit that referenced this issue Oct 16, 2018
…ndary. This behavior is guarded by a flag (default value is "don't error out"), and will eventually become the only behavior.

This implements the feature in #6408.

RELNOTES: None
PiperOrigin-RevId: 217402217
@dslomov dslomov added the incompatible-change Incompatible/breaking change label Oct 17, 2018
@dslomov
Copy link
Contributor

dslomov commented Oct 17, 2018

This is a breaking change, right? Which flag guards it? How do users migrate?

@laurentlb
Copy link
Contributor

Yes, it's a breaking change.
The commit above adds an incompatible flag: https://docs.bazel.build/versions/master/skylark/backward-compatibility.html#load-label-cannot-cross-package-boundaries

We need tooling to help the migration (cc @vladmos).

@laurentlb laurentlb assigned laurentlb and unassigned haxorz Oct 17, 2018
@dslomov dslomov changed the title Starlark: make 'load' statement respect package boundaries incompatible_disallow_load_labels_to_cross_package_boundaries: make 'load' statement respect package boundaries Oct 31, 2018
@dslomov
Copy link
Contributor

dslomov commented Oct 31, 2018

What is missing for migration: migration docs, length of migration window. After these are done, please add "migration-ready" label.

@laurentlb
Copy link
Contributor

Migration notes

This will fail when a label crosses packages, i.e. when the position of the colon is not correct in a label. The error message will tell you what is the correct label to use instead.

Read more about the label syntax here: https://docs.bazel.build/versions/master/build-ref.html#labels

@laurentlb
Copy link
Contributor

Downstream projects that fail with this flag (https://buildkite.com/bazel/bazel-at-release-plus-incompatible-flags/builds/56):

  • rules_k8s
  • rules_typescript
  • TensorFlow

@philwo
Copy link
Member

philwo commented Feb 6, 2019

This flag was not flipped in time for the Bazel 0.23.0 release and will thus be postponed to Bazel 0.24.0.

@katre
Copy link
Member

katre commented Mar 1, 2019

This issue was tagged as "breaking-change-0.24" but does not appear ready to be flipped in the 0.24.0 release. If this is incorrect please comment on that issue and discuss with me.

lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Sep 1, 2019
After flipping of
--incompatible_disallow_load_labels_to_cross_package_boundaries bit in
Bazel: [1], currently documented way to import external dependencies
from plugins that implement standalone build mode doesn't work any more.

This change documents an alternative approach how to make it work,
without abandoning of standalone build mode.

[1] bazelbuild/bazel#6408

Bug: Issue 10885
Change-Id: I7b1c4e9fa4f6ba8966efcde4cdee7d76d6f59d1c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible-change Incompatible/breaking change P1 I'll work on this now. (Assignee required)
Projects
None yet
Development

No branches or pull requests

6 participants