Skip to content

Forward --host_jvmopt to the exec configuration #26346

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

Closed
wants to merge 2 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jun 20, 2025

This ports #15978, which never got merged, to Starlark.

@fmeum fmeum requested a review from gregestren June 20, 2025 05:24
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Jun 20, 2025
@fmeum fmeum requested a review from aranguyen June 20, 2025 06:41
@gregestren
Copy link
Contributor

LGTM but did any recent user-facing behavior motivate this?

#15978 was from before the --experimental_output_directory_naming_scheme=diff_against_dynamic_baseline default.

And from #15978 (comment):

Could there be a chain of transitions that results in different JVM options (host or target) due to "losing" the host options when applying the exec transition?

I don't think this kind of change is harmful either way. Just want to understand the context.

@fmeum
Copy link
Collaborator Author

fmeum commented Jun 20, 2025

You are right, this is no longer motivated by action conflicts. I still think it's needed for correctness in an exec -> jvmopts -> exec transition scenario, however unlikely that may be in practice.

@gregestren gregestren added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jun 20, 2025
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jun 30, 2025
@fmeum fmeum deleted the patch-22 branch July 1, 2025 05:41
@fmeum
Copy link
Collaborator Author

fmeum commented Jul 1, 2025

@bazel-io fork 8.4.0

bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Jul 1, 2025
This ports bazelbuild#15978, which never got merged, to Starlark.

Closes bazelbuild#26346.

PiperOrigin-RevId: 777732082
Change-Id: I699c8fc248d18e1e6db168725f49fc051829b45e
github-merge-queue bot pushed a commit that referenced this pull request Jul 2, 2025
This ports #15978, which never
got merged, to Starlark.

Closes #26346.

PiperOrigin-RevId: 777732082
Change-Id: I699c8fc248d18e1e6db168725f49fc051829b45e

Commit
7c40bbe

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants