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

[GR-44872] Normalize env-var keys to upperCase on Windows. #6214

Merged
merged 6 commits into from
Mar 18, 2023
Merged

[GR-44872] Normalize env-var keys to upperCase on Windows. #6214

merged 6 commits into from
Mar 18, 2023

Conversation

graalvmbot
Copy link
Collaborator

  • Rename NATIVE_IMAGE_SLOPPY_BUILDER_SANITATION to NATIVE_IMAGE_DEPRECATED_BUILDER_SANITATION
  • Normalize env-var keys to upperCase on Windows

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Mar 15, 2023
@olpaw olpaw linked an issue Mar 15, 2023 that may be closed by this pull request
[GR-44872] Windows environment variables are case insensitive and result in native build failures #6212
Closed
@olpaw olpaw requested a review from zakkak March 15, 2023 14:52
@olpaw olpaw self-assigned this Mar 15, 2023
@olpaw olpaw added this to the 23.0.0 Release (April 18, 2023) milestone Mar 15, 2023
jerboaa
jerboaa approved these changes Mar 15, 2023
View reviewed changes
Copy link
Collaborator

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

LGTM. I like the name change of the fallback env :-)

zakkak
zakkak approved these changes Mar 15, 2023
View reviewed changes
Copy link
Collaborator

@zakkak zakkak left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM.

I also tested the normalization of the environment keys and it fixes the issue we observed (we now see a different issue due to using the windows-2019 runner).

Test run: https://github.com/zakkak/mandrel/actions/runs/4430035277

Update: Working test with windows-2022 https://github.com/zakkak/mandrel/actions/runs/4430523073/jobs/7772386953

@olpaw
Copy link
Member

olpaw commented Mar 16, 2023

@zakkak unfortunately I had to further refine this PR to ensure env-vars on Windows are looked up in case-insensitive ways but added in their original form. Also for -E<env-key> I had to make the lookup case-insensitive for Windows.

Please re-test to ensure with the new changes the PR still solves the issue you reported originally.

@olpaw olpaw requested a review from zakkak March 16, 2023 15:40
zakkak
zakkak approved these changes Mar 16, 2023
View reviewed changes
Copy link
Collaborator

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

Still good, other than a minor comment.

PR tested in https://github.com/zakkak/mandrel/actions/runs/4440677580

substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java Show resolved Hide resolved
@graalvmbot graalvmbot merged commit b5bd5d5 into master Mar 18, 2023
@graalvmbot graalvmbot deleted the paw/GR-44872 branch March 18, 2023 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zakkak zakkak zakkak approved these changes

@olpaw olpaw olpaw left review comments

@jerboaa jerboaa jerboaa approved these changes

Assignees

@olpaw olpaw

Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Milestone
23.0.0 Release (June 13, 2023)
Development

Successfully merging this pull request may close these issues.

[GR-44872] Windows environment variables are case insensitive and result in native build failures
4 participants
@graalvmbot @olpaw @jerboaa @zakkak