-
Notifications
You must be signed in to change notification settings - Fork 8
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 Windows Mandrel build for graal/master #380
Conversation
981ba34
to
95ac307
Compare
54c8595
to
34c6bae
Compare
MacOS GHA build fail is tracked here: #382 |
oracle/graal#7557 is the upstream change that makes this change needed for |
Full mandrel CI test run with this for Windows is here: https://github.com/graalvm/mandrel/actions/runs/6931873655 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @jerboaa.
Nice to have: Could you please add an if-exists before the copy to make sure we can build commits before the change as well?
Is this really needed? Not sure what the use-case would be... Would you have an example? |
The only use case is when debugging/git-bisecting, i.e. if we want to build an older version of 24.0-dev. In that case it would be nice if we didn't have to checkout a different mandrel-packaging version. Certainly not a priority or blocker. |
I'll merge as is, if OK. As you can guess I'm not a fan of |
Closes graalvm/mandrel#613
See GHA test in: https://github.com/jerboaa/mandrel-packaging/actions/runs/6931578411/job/18853597211 (or this PR). Windows build passed.