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

Fix OMR_ENV_OPENXL Usage #20928

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

midronij
Copy link
Contributor

@midronij midronij commented Jan 13, 2025

Clean up usage of OMR_ENV_OPENXL make/config files
Replace all usages of OMR_ENV_OPENXL with the already existing env variable, OMR_TOOLCHAIN

Depends on eclipse-omr/omr#7605

@midronij midronij changed the title Clean up usage of OMR_ENV_OPENXL make/config files Fix OMR_ENV_OPENXL Usage Jan 13, 2025
@keithc-ca keithc-ca self-requested a review January 14, 2025 15:28
@keithc-ca
Copy link
Contributor

Moving to draft state until consensus is reached with respect to the configuration pattern.

@keithc-ca keithc-ca marked this pull request as draft January 14, 2025 15:29
@midronij midronij force-pushed the openxl-config branch 2 times, most recently from 70d8ec7 to 71c8aaa Compare January 14, 2025 20:12
The environment variable OMR_ENV_OPENXL was created in a previous
contribution to indicate if the C/C++ compiler being used was OpenXL.
However, this can be done in a way that remains consistent to other
platforms by using the already existing OMR_TOOLCHAIN variable instead.

Signed-off-by: midronij <[email protected]>
@midronij
Copy link
Contributor Author

midronij commented Feb 4, 2025

@keithc-ca since the OMR side contribution is just about finalized, I'll unmark this as a draft. If you have any further feedback to give, then please feel free to review my changes again. Otherwise, I think we can get the CI tests started.

@midronij midronij marked this pull request as ready for review February 4, 2025 18:16
Copy link
Contributor

@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

This looks good, but will need to wait until the OMR change promotes to the openj9 branch.

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