search

graalvm / mandrel-packaging

6 stars 8 forks source link

Fix Windows Mandrel build for graal/master #380

Closed jerboaa closed 9 months ago

jerboaa commented 10 months ago

Closes https://github.com/graalvm/mandrel/issues/613

See GHA test in: https://github.com/jerboaa/mandrel-packaging/actions/runs/6931578411/job/18853597211 (or this PR). Windows build passed.

jerboaa commented 10 months ago

MacOS GHA build fail is tracked here: https://github.com/graalvm/mandrel-packaging/issues/382

jerboaa commented 10 months ago

https://github.com/oracle/graal/pull/7557 is the upstream change that makes this change needed for graal/master.

jerboaa commented 10 months ago

Full mandrel CI test run with this for Windows is here: https://github.com/graalvm/mandrel/actions/runs/6931873655

jerboaa commented 9 months ago

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? mandrel-packaging in the master config builds 24.0 currently. We are going to branch for 24.1 and have separate branches for older releases. What am I missing?

zakkak commented 9 months ago

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.

jerboaa commented 9 months ago

Certainly not a priority or blocker.

I'll merge as is, if OK. As you can guess I'm not a fan of first-check-if-exists-then-do-something flow. It's expected to be there and the chance of doing bisect on Windows are minimal? Hope that's fine.