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

Iterate on workflow to compile binaries for all major OS<>arch combinations #179

Merged
merged 11 commits into from
Feb 13, 2022
Merged

Iterate on workflow to compile binaries for all major OS<>arch combinations #179

merged 11 commits into from
Feb 13, 2022

Conversation

hendrikmaus
Copy link
Contributor

@hendrikmaus hendrikmaus commented Feb 12, 2022

This change-set iterates on the initial version of the workflow #57 and fixes the behavior observed and described in #175 (review).

Instead of compressing the compiled binaries before they are uploaded to GitHub, they are renamed to fj-host-<target>:
image

Furthermore, I adjusted the name of the workflow and some comments in the file.

Copy link
Owner

@hannobraun hannobraun left a comment

Choose a reason for hiding this comment

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

Thank you, @hendrikmaus!

The commits look good, but we have a new problem: The execute permissions don't survive the upload/download process, without an archive to wrap them. (At least for the Linux/musl binary, but I suspect it's true for all the non-Windows ones.)

I'm tempted to just merge, as this pull request contains other improvements. Not sure if missing permissions are any better than the double-archive. Not much of a difference to me personally, but I suspect a less experienced user might be less confused by the double-archive.

Any suggestions?

@hendrikmaus
Copy link
Contributor Author

I tried to explicitly retain the executable flag, but looks like we're out of luck.

On releases, we won't have these issues though. I reckon new users will either download the binary from a release or try to use a package manager to install. Hence, these "intermediary" binaries will only be used by contributors.

I'd stick with the current solution. However, we should write a little piece of documentation.

@hannobraun
Copy link
Owner

hannobraun commented Feb 13, 2022

Makes sense, thanks Hendrik! I'm merging now, and will open an issue about documentation shortly.

@hannobraun
Copy link
Owner

Opened #180.

@hendrikmaus hendrikmaus deleted the #57_do-not-compress-binaries branch February 13, 2022 09:02
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