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: [Make] Create windows binaries with .exe extension #792

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zalgonoise
Copy link

When fetching Venom binaries, I noticed that Windows releases didn't include an extension. For Windows, it means that the users will face the program-picker prompt due to a lack of extension, and would have to add it manually.

This PR changes the CROSS_COMPILED_BINARIES make target in .build/go.mk to check if the current target OS is windows, and if so, changing the filename to include the .exe extension.

I assume that the point of BINARIES is precisely this, but probably due to some lack of Make syntax knowledge, I couldn't figure out why IS_WINDOWS isn't being recognized :)

That being the case, I am opening a suggestion to leverage the GOOS env var taken from the get_os_from_binary_file call, and changing the filename if the OS is windows.

Any suggestions for changes are welcome, provided that we are able to distribute windows binaries with the proper extension :)

@zalgonoise zalgonoise force-pushed the fix/deploy-windows-binaries-with-exe-extension branch from 5eeac01 to fcaebef Compare May 9, 2024 19:11
Signed-off-by: Mario Salgado <mariozalgo@gmail.com>
Signed-off-by: zalgonoise <mariozalgo@gmail.com>
@zalgonoise zalgonoise force-pushed the fix/deploy-windows-binaries-with-exe-extension branch from fcaebef to d776aec Compare May 9, 2024 19:13
@zalgonoise
Copy link
Author

Note: the force-push was to add the sign-off mark to the commit, in compliance with the DCO :)

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.

None yet

1 participant