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

Support native compilation on non-GNU Unix systems #5237

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

neduard
Copy link
Contributor

@neduard neduard commented Jul 17, 2024

Overview

Currently only tested on OpenBSD, but the hope is to more easily support non-GNU systems. Two main changes:

  1. Use libb2 for all blake2 related functions. This is because blake2 is not supported at the moment by LibreSSL
  2. Use the more portable sh in jit-tests.sh. This ensures it can run on systems where bash isn't installed by default.

(cc @dolio as I think you've been quite involved in the native compilation? Curious to hear your thoughts on this!)

Interesting/controversial decisions

One could argue that the vast majority of users will be on Windows/Linux/Mac. However, I think it's important in general to strive for portable software. The more systems software can run on, the less chances of system/implementation-specific issues cropping up.

Test coverage

Relying mostly on CI . Manually ran jit-tests.sh locally on Mac.

Loose ends

I've also updated the README for the scheme-libs here #5238

Tests are currently failing because of outdated scripts. That is fixed in #5236

neduard added 3 commits July 17, 2024 23:00
required if compiling on systems that use eg libressl
this is more portable and ensures we can compile on systems without bash (eg *BSD)
@neduard neduard marked this pull request as ready for review July 17, 2024 23:31
@sellout
Copy link
Contributor

sellout commented Jul 18, 2024

I haven’t tried this out, but the changes look good to me in principle, and I agree with the motivations.

@dolio dolio self-requested a review July 18, 2024 15:33
@dolio dolio self-assigned this Jul 18, 2024
Copy link
Contributor

@dolio dolio left a comment

Choose a reason for hiding this comment

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

Cool.

Seems like simple enough changes to make things more portable. Those two hashes appear to be in the tests, so it seems like it should still be working on the usual platforms.

@dolio dolio added the ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved label Jul 18, 2024
@mergify mergify bot merged commit 42ebc76 into unisonweb:trunk Jul 18, 2024
20 checks passed
@mergify mergify bot removed the ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved label Jul 18, 2024
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