Skip to content

Use Args#add_all to add a directory in Java bootstrap action #18159

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
Closed
fmeum wants to merge 1 commit into from
Closed

Use Args#add_all to add a directory in Java bootstrap action #18159

fmeum wants to merge 1 commit into from

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Apr 20, 2023

This allows StarlarkCustomCommandLine to recognize the argument as an Artifact and map its path according to --experimental_output_paths.

Work towards #6526

@fmeum
Use Args#add_all to add a directory in Java bootstrap action
8b9512e 
This allows `StarlarkCustomCommandLine` to recognize the argument as an
`Artifact` and map its path according to `--experimental_output_paths`.

Work towards bazelbuild#6526
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Rules-Java Issues for Java rules labels Apr 20, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 20, 2023

@cushon Could you review this PR? I had a similar one a while back that was partially reverted, this just restores that behavior. Tests that prevent such "regressions" will be coming soon, they just happened to be delayed.

@fmeum fmeum requested a review from cushon April 20, 2023 15:03
@cushon cushon added the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Apr 20, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 20, 2023

@cushon Since it's related to my work on path mapping: Is support for multiplexed sandboxing something that has been considered for the Javac worker? I would potentially look into implementing it.

@cushon
Copy link
Contributor

cushon commented Apr 20, 2023

@cushon Since it's related to my work on path mapping: Is support for multiplexed sandboxing something that has been considered for the Javac worker? I would potentially look into implementing it.

I do not know. @larsrc-google do you know if that's something that has been considered?

@larsrc-google
Copy link
Contributor

larsrc-google commented Apr 20, 2023

It's not just considered, it's mostly there. Unfortunately I hit some absolute path problems in plugins and then got derailed.

@fmeum
Copy link
Collaborator Author

fmeum commented Apr 20, 2023

That's great to hear. I can use sandboxed singleplex workers in the meantime, but sandboxed multiplex Javac would likely help reduce the overhead of path stripping for local builds.

@fmeum fmeum mentioned this pull request Apr 20, 2023
Closed
@fmeum fmeum deleted the bootstrap-path-mapping branch April 21, 2023 19:02
@sgowroji sgowroji removed awaiting-review PR is awaiting review from an assigned reviewer awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally labels Apr 24, 2023
fweikert pushed a commit to fweikert/bazel that referenced this pull request May 25, 2023
@fmeum @fweikert
Use Args#add_all to add a directory in Java bootstrap action
ac96873 
This allows `StarlarkCustomCommandLine` to recognize the argument as an `Artifact` and map its path according to `--experimental_output_paths`.

Work towards bazelbuild#6526

Closes bazelbuild#18159.

PiperOrigin-RevId: 526060134
Change-Id: I18fb9c12317a5089908821b52ce019720aa03ae3
@fmeum fmeum mentioned this pull request Oct 4, 2023
Closed
@gregestren gregestren mentioned this pull request Oct 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cushon cushon cushon approved these changes

Assignees

No one assigned

Labels

team-Rules-Java Issues for Java rules

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@fmeum @cushon @larsrc-google @sgowroji