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

Do not match java_binary deploy.jar with wildcard target patterns #17115

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jan 3, 2023

With the native implementation of java_binary, bazel build //... would not end up building deploy jars. This is achieved for the Starlark implementation by adding the manual tag to the deploy jar rule.

With the native implementation of `java_binary`, `bazel build //...`
would not end up building deploy jars. This is achieved for the Starlark
implementation by adding the `manual` tag to the deploy jar rule.
@fmeum fmeum changed the title Do not build java_binary deploy.jar with wildcard target patterns Do not match java_binary deploy.jar with wildcard target patterns Jan 3, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented Jan 3, 2023

@hvadehra Could you take a look? I noticed this while debugging an unrelated issue at HEAD.

@fmeum fmeum marked this pull request as ready for review January 3, 2023 09:15
@sgowroji sgowroji added team-Rules-Java Issues for Java rules awaiting-review PR is awaiting review from an assigned reviewer labels Jan 3, 2023
hvadehra
hvadehra previously approved these changes Jan 18, 2023
Copy link
Member

@hvadehra hvadehra left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@hvadehra hvadehra removed the awaiting-review PR is awaiting review from an assigned reviewer label Jan 18, 2023
@sgowroji
Copy link
Member

Hi @hvadehra, Is the Above PR is Awaiting to merge?

@hvadehra hvadehra dismissed their stale review January 18, 2023 11:27

breaks internally

@hvadehra
Copy link
Member

So, this will not work as is, since the tags list could be frozen and we can't append. We'll need to make a copy. I'll going to run a benchmark to ensure the performance penalty isn't too huge.

@fmeum fmeum deleted the manual-deploy-jar branch January 19, 2023 08:40
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
With the native implementation of `java_binary`, `bazel build //...` would not end up building deploy jars. This is achieved for the Starlark implementation by adding the `manual` tag to the deploy jar rule. Since the list may already be frozen, we need to make a copy.

Closes #17115.

PiperOrigin-RevId: 503071603
Change-Id: I99afdac8c040bf0aecd5a1bf7d4acf529058ac22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-Java Issues for Java rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants