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.

Sign up for GitHub

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

Jump to bottom

@wkspc shorthand label syntax works as a dependency but not on command line #4385

Closed
alexeagle opened this issue Jan 3, 2018 · 5 comments
Closed

@wkspc shorthand label syntax works as a dependency but not on command line #4385

alexeagle opened this issue Jan 3, 2018 · 5 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug

Comments

@alexeagle
Copy link
Contributor

Bazel 0.8.1
The current treatment of a label expressed as just @wkspc is inconsistent.

I can have deps=["@rxjs"] as a shorthand for deps=["@rxjs//:rxjs"]

But if I bazel build @rxjs I get

INFO: Analysed 0 targets (4 packages loaded).
INFO: Found 0 targets...

and if I try to abbreviate bazel run @yarn//:yarn as bazel run @yarn I get ERROR: No targets found to run

Reproduction in this repo:
https://github.com/alexeagle/angular-bazel-example
@hlopko hlopko added category: misc > misc P3 We're not considering working on this, but happy to review a PR. (No assignee) type: bug labels Jan 4, 2018
@alexeagle
Copy link
Contributor Author

fyi @dslomov we might want to take a different solution to this as part of allowing @ in package names

@aiuto aiuto added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. and removed category: misc > misc labels Dec 10, 2018
@baskus
Copy link

baskus commented May 14, 2019

I'd love for this to be resolved. :)

@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
@philwo philwo removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Nov 29, 2021
@fmeum fmeum mentioned this issue Mar 28, 2023
Closed
copybara-service bot pushed a commit that referenced this issue Mar 30, 2023
@Wyverald @copybara-github
Tests for TargetPattern parsing, and some sanity fixes
03266a8 
- TargetPatternTest isn't testing much right now. This CL changes it to actually assert the result of parsing is as expected, and also test a few different parser setups (in main repo, with a relative directory, in a non-main repo).
  - This necessitated adding #toString methods to all TargetPattern subclasses.
  - Also rearranged the parameters in the constructors of all subclasses so that `originalPattern` is always the first parameter.
- Removed the always-true `checkWildcardConflict` parameter in `TargetsInPackage`.
- Removed the weak check before `interpretPathAsTarget` as it was not only weak, but wrong. It made parsing `Bar\\java` a failure at the repo root but a success in a subdirectory.

Work towards #4385

PiperOrigin-RevId: 520630430
Change-Id: Icee98f38134fe274ba800d2e949c83f0ae18c247
@Wyverald
Copy link
Member

@bazel-io fork 6.2.0

@bazel-io bazel-io mentioned this issue Mar 30, 2023
Closed
Wyverald added a commit that referenced this issue Mar 31, 2023
@Wyverald
Tests for TargetPattern parsing, and some sanity fixes
0770518 
- TargetPatternTest isn't testing much right now. This CL changes it to actually assert the result of parsing is as expected, and also test a few different parser setups (in main repo, with a relative directory, in a non-main repo).
  - This necessitated adding #toString methods to all TargetPattern subclasses.
  - Also rearranged the parameters in the constructors of all subclasses so that `originalPattern` is always the first parameter.
- Removed the always-true `checkWildcardConflict` parameter in `TargetsInPackage`.
- Removed the weak check before `interpretPathAsTarget` as it was not only weak, but wrong. It made parsing `Bar\\java` a failure at the repo root but a success in a subdirectory.

Work towards #4385

PiperOrigin-RevId: 520630430
Change-Id: Icee98f38134fe274ba800d2e949c83f0ae18c247
Wyverald added a commit that referenced this issue Mar 31, 2023
@Wyverald
Switch TargetPattern.Parser to use LabelParser
c8a4619 
- Target patterns and labels have very similar syntax, yet are parsed completely separately. Furthermore, the code for target pattern parsing wasn't written with repos in mind, causing some corner cases such as #4385.
- This CL augments LabelParser to deal with some syntax specific to target patterns (mostly the '...' thing), and switches TargetPattern.Parser to use LabelParser instead.
- This fixes some inconsistencies between the two and eliminates the bug linked above.
- Added LabelParserTest to make sure that the table in the javadoc of LabelParser.Parts#parse is actually accurate.
  - Switched LabelParser.Parts to use AutoValue instead to enable testing.
- Some more cleanup in TargetPattern.Parser; removing outdated fields/methods/etc.

Fixes #4385.

RELNOTES: `@foo` labels can now be used on the command line as the top-level target (that is, `bazel build @foo` now works). Double-dot syntax is now forbidden (`bazel build ../foo` will no longer work).
PiperOrigin-RevId: 520902549
Change-Id: I38d6400381a3c4ac4c370360578702d5dd870909
@hvadehra hvadehra reopened this Apr 3, 2023
@hvadehra
Copy link
Member

hvadehra commented Apr 3, 2023

24f6fe8 is being reverted due to internal breakages.

copybara-service bot pushed a commit that referenced this issue Apr 3, 2023
@hvadehra @copybara-github
Automated rollback of commit 24f6fe8.
842d23e 
*** Reason for rollback ***

Breakages in depot[1]

*** Original change description ***

Switch TargetPattern.Parser to use LabelParser

- Target patterns and labels have very similar syntax, yet are parsed completely separately. Furthermore, the code for target pattern parsing wasn't written with repos in mind, causing some corner cases such as #4385.
- This CL augments LabelParser to deal with some syntax specific to target patterns (mostly the '...' thing), and switches TargetPattern.Parser to use LabelParser instead.
- This fixes some in...

***

PiperOrigin-RevId: 521412117
Change-Id: I34f182cc9567678b07e45db82fbdafeab17e67a3
copybara-service bot pushed a commit that referenced this issue Apr 4, 2023
@Wyverald @copybara-github
Automated rollback of commit 842d23e.
a46b473 
*** Reason for rollback ***

Fixed depot breakage: unknown commit

*** Original change description ***

Automated rollback of commit 24f6fe8.

*** Reason for rollback ***

Breakages in depot[1]

*** Original change description ***

Switch TargetPattern.Parser to use LabelParser

- Target patterns and labels have very similar syntax, yet are parsed completely separately. Furthermore, the code for target pattern parsing wasn't written with repos in mind, causing some corner cases such as #4385.
- This CL augments LabelParser to deal with some syntax...

***

PiperOrigin-RevId: 521714380
Change-Id: I9786fb38b4848c4ebe34a228e9fc4f62e40f97d6
@Wyverald
Copy link
Member

Wyverald commented Apr 4, 2023

Re-submitted.

@Wyverald Wyverald closed this as completed Apr 4, 2023
Wyverald added a commit that referenced this issue Apr 4, 2023
@Wyverald
[6.2.0] TargetPattern parsing fixes (#17945)
fb4a0c2 
* Tests for TargetPattern parsing, and some sanity fixes

- TargetPatternTest isn't testing much right now. This CL changes it to actually assert the result of parsing is as expected, and also test a few different parser setups (in main repo, with a relative directory, in a non-main repo).
  - This necessitated adding #toString methods to all TargetPattern subclasses.
  - Also rearranged the parameters in the constructors of all subclasses so that `originalPattern` is always the first parameter.
- Removed the always-true `checkWildcardConflict` parameter in `TargetsInPackage`.
- Removed the weak check before `interpretPathAsTarget` as it was not only weak, but wrong. It made parsing `Bar\\java` a failure at the repo root but a success in a subdirectory.

Work towards #4385

PiperOrigin-RevId: 520630430
Change-Id: Icee98f38134fe274ba800d2e949c83f0ae18c247

* Switch TargetPattern.Parser to use LabelParser

- Target patterns and labels have very similar syntax, yet are parsed completely separately. Furthermore, the code for target pattern parsing wasn't written with repos in mind, causing some corner cases such as #4385.
- This CL augments LabelParser to deal with some syntax specific to target patterns (mostly the '...' thing), and switches TargetPattern.Parser to use LabelParser instead.
- This fixes some inconsistencies between the two and eliminates the bug linked above.
- Added LabelParserTest to make sure that the table in the javadoc of LabelParser.Parts#parse is actually accurate.
  - Switched LabelParser.Parts to use AutoValue instead to enable testing.
- Some more cleanup in TargetPattern.Parser; removing outdated fields/methods/etc.

Fixes #4385.

RELNOTES: `@foo` labels can now be used on the command line as the top-level target (that is, `bazel build @foo` now works). Double-dot syntax is now forbidden (`bazel build ../foo` will no longer work).
PiperOrigin-RevId: 520902549
Change-Id: I38d6400381a3c4ac4c370360578702d5dd870909
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
@Wyverald @fweikert
Tests for TargetPattern parsing, and some sanity fixes
ef2134a 
- TargetPatternTest isn't testing much right now. This CL changes it to actually assert the result of parsing is as expected, and also test a few different parser setups (in main repo, with a relative directory, in a non-main repo).
  - This necessitated adding #toString methods to all TargetPattern subclasses.
  - Also rearranged the parameters in the constructors of all subclasses so that `originalPattern` is always the first parameter.
- Removed the always-true `checkWildcardConflict` parameter in `TargetsInPackage`.
- Removed the weak check before `interpretPathAsTarget` as it was not only weak, but wrong. It made parsing `Bar\\java` a failure at the repo root but a success in a subdirectory.

Work towards bazelbuild#4385

PiperOrigin-RevId: 520630430
Change-Id: Icee98f38134fe274ba800d2e949c83f0ae18c247
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
@Wyverald @fweikert
Switch TargetPattern.Parser to use LabelParser
43dd8b3 
- Target patterns and labels have very similar syntax, yet are parsed completely separately. Furthermore, the code for target pattern parsing wasn't written with repos in mind, causing some corner cases such as bazelbuild#4385.
- This CL augments LabelParser to deal with some syntax specific to target patterns (mostly the '...' thing), and switches TargetPattern.Parser to use LabelParser instead.
- This fixes some inconsistencies between the two and eliminates the bug linked above.
- Added LabelParserTest to make sure that the table in the javadoc of LabelParser.Parts#parse is actually accurate.
  - Switched LabelParser.Parts to use AutoValue instead to enable testing.
- Some more cleanup in TargetPattern.Parser; removing outdated fields/methods/etc.

Fixes bazelbuild#4385.

RELNOTES: `@foo` labels can now be used on the command line as the top-level target (that is, `bazel build @foo` now works). Double-dot syntax is now forbidden (`bazel build ../foo` will no longer work).
PiperOrigin-RevId: 520902549
Change-Id: I38d6400381a3c4ac4c370360578702d5dd870909
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
@hvadehra @fweikert
Automated rollback of commit 24f6fe8.
e9cbbdb 
*** Reason for rollback ***

Breakages in depot[1]

*** Original change description ***

Switch TargetPattern.Parser to use LabelParser

- Target patterns and labels have very similar syntax, yet are parsed completely separately. Furthermore, the code for target pattern parsing wasn't written with repos in mind, causing some corner cases such as bazelbuild#4385.
- This CL augments LabelParser to deal with some syntax specific to target patterns (mostly the '...' thing), and switches TargetPattern.Parser to use LabelParser instead.
- This fixes some in...

***

PiperOrigin-RevId: 521412117
Change-Id: I34f182cc9567678b07e45db82fbdafeab17e67a3
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
@Wyverald @fweikert
Automated rollback of commit 842d23e.
17e7407 
*** Reason for rollback ***

Fixed depot breakage: unknown commit

*** Original change description ***

Automated rollback of commit 24f6fe8.

*** Reason for rollback ***

Breakages in depot[1]

*** Original change description ***

Switch TargetPattern.Parser to use LabelParser

- Target patterns and labels have very similar syntax, yet are parsed completely separately. Furthermore, the code for target pattern parsing wasn't written with repos in mind, causing some corner cases such as bazelbuild#4385.
- This CL augments LabelParser to deal with some syntax...

***

PiperOrigin-RevId: 521714380
Change-Id: I9786fb38b4848c4ebe34a228e9fc4f62e40f97d6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for @repo target patterns fmeum/bazel
8 participants
@alexeagle @Wyverald @philwo @hlopko @baskus @hvadehra @aiuto @dslomov