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

feat: allow installing specific Mesa3D release #22

Merged
merged 8 commits into from
Dec 2, 2024

Conversation

RobPasMue
Copy link
Contributor

@RobPasMue RobPasMue commented Dec 2, 2024

A user might want to install a specific version of Mesa3D on their self-hosted runner or GitHub runner. This change would allow them to do so. Also, it will allow the action, by default, to always pull the latest Mesa3D version available rather than a hard-coded one. See commit b2a733a

Furthermore - we should use the installers that are shipped by Mesa3D in order to avoid DLL incompatibility issues. See commits d6dd86a and adb571e

Added documentation on how to use the input, and modified the action and bash scripts as well. Let me know if there's anything else I can contribute with!

@RobPasMue
Copy link
Contributor Author

Pinging @larsoner @tlambert03 and @banesullivan for feedback and review! Thanks in advance =)

Copy link

@SMoraisAnsys SMoraisAnsys 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 feared that hardcoding values for the systemwidedeploy.cmd file might backfire but this file hasn't changed in two years, see https://github.com/pal1000/mesa-dist-win/blob/master/bin/systemwidedeploy.cmd.

Copy link
Collaborator

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Just one tiny suggestion which I'll apply and merge, this is great, thanks!

action.yml Outdated Show resolved Hide resolved
@RobPasMue
Copy link
Contributor Author

Just one tiny suggestion which I'll apply and merge, this is great, thanks!

Awesome, thanks a lot @larsoner! =) Would it be possible to get a release with this change?

@RobPasMue
Copy link
Contributor Author

RobPasMue commented Dec 2, 2024

Looks like grep -P might not behave properly everywhere. Using sed directly. See https://github.com/pyvista/setup-headless-display-action/actions/runs/12121147123/job/33791452876?pr=22#step:3:17

@RobPasMue
Copy link
Contributor Author

Hmm... for some reason the env var is giving some problems on test (windows-2022) but not on test (windows-latest)...

@larsoner
Copy link
Collaborator

larsoner commented Dec 2, 2024

Still comes up blank for the version 😞

@RobPasMue
Copy link
Contributor Author

Still comes up blank for the version 😞

Yeah saw that... the thing is that it only happens with windows-2022 😮

@larsoner
Copy link
Collaborator

larsoner commented Dec 2, 2024

Maybe instead

$ gh api /repos/pal1000/mesa-dist-win/releases | jq .[0].tag_name
"24.3.0"

?

@RobPasMue
Copy link
Contributor Author

jq...

jq is not available in some Windows Bash terminals 😢

action.yml Outdated Show resolved Hide resolved
@RobPasMue
Copy link
Contributor Author

I added some extra logging to make it easy for users to debug as well

@RobPasMue
Copy link
Contributor Author

It ended up working here: https://github.com/pyvista/setup-headless-display-action/actions/runs/12121505042/job/33793270066

Let's let it re-run again. Might have been some transient issue

@larsoner
Copy link
Collaborator

larsoner commented Dec 2, 2024

Looks like retrieving the version can just be a bit flaky or fail sometimes as 891324c failed but worked once I restarted it. Not sure if there's a good way around that.

@larsoner
Copy link
Collaborator

larsoner commented Dec 2, 2024

@RobPasMue given 1) latest can be problematic/flaky, and 2) can change over time, I am inclined to set the default to 24.3.0, the latest as of today. And then we can bump periodically, probably on major version changes of this library. It seems like a more stable choice for end-users who don't set the variable.

@larsoner larsoner merged commit 83c78fd into pyvista:main Dec 2, 2024
12 checks passed
@larsoner
Copy link
Collaborator

larsoner commented Dec 2, 2024

Thanks @RobPasMue !

@larsoner
Copy link
Collaborator

larsoner commented Dec 2, 2024

Released as v3.1 and updated v3 to point to it

@RobPasMue
Copy link
Contributor Author

@RobPasMue given 1) latest can be problematic/flaky, and 2) can change over time, I am inclined to set the default to 24.3.0, the latest as of today. And then we can bump periodically, probably on major version changes of this library. It seems like a more stable choice for end-users who don't set the variable.

Sounds good! Thanks @larsoner for all your support!

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.

3 participants