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: install script on windows #1168

Merged
merged 2 commits into from
Feb 8, 2023
Merged

Conversation

hdeadman
Copy link
Contributor

@hdeadman hdeadman commented Oct 9, 2022

What

Fix install.sh script for windows (mingw/msys2 bash).

Why

The install script wasn't working on windows b/c the released binary for windows has a .exe extension on it which made the script error with a 404. Also, the USE_SUDO variable should be set to false by default on windows (otherwise piping script to bash won't work).

Implications

An alternative fix would be to change the releases to not include the .exe extension on the windows binary but maybe that was done for use outside of mingw where the install.sh script wouldn't be used. The install script still saves k3d to /usr/local/bin/k3d without the .exe extension and it works without the .exe extension (in msys2/mingw). It would work the same way if it was saved as k3d.exe.

@hdeadman
Copy link
Contributor Author

hdeadman commented Oct 9, 2022

When I initially tested this PR locally with MSYS2, it worked, but then I added a github action workflow to run install.sh on ubuntu, mac and windows and it failed on windows. Github's windows image uses the bash that comes with Git for Windows and that is different in some ways from MSYS2. It doesn't include /usr/local/bin, but they do set ~ to the user's windows home directory (/c/Users/xyz) and they put ~/bin first in the path so it's sort of the equivalent of /usr/local/bin. The ~/bin folder doesn't exist by default, despite being in the path, so this change creates it if it doesn't exist and puts k3d in it (only if /usr/local/bin not available and only on windows).

Copy link
Member

@iwilltry42 iwilltry42 left a comment

Choose a reason for hiding this comment

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

LGTM

@iwilltry42
Copy link
Member

I'm personally not testing too much on Windows, so this is a nice fix, thank you very much @hdeadman! 👍

@iwilltry42 iwilltry42 changed the title fix install script on windows fix: install script on windows Feb 8, 2023
@iwilltry42 iwilltry42 merged commit 68588b0 into k3d-io:main Feb 8, 2023
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