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: Statically link libiconv on aarch64-darwin; Test static binary in CI #158

Merged
merged 13 commits into from
Aug 2, 2024

Conversation

shivaraj-bh
Copy link
Member

@shivaraj-bh shivaraj-bh commented Jul 17, 2024

Note: macOS system libraries will not be statically linked

resolves #149

Before

❯ otool -L ./result/bin/om
./result/bin/om:
        /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration (compatibility version 1.0.0, current version 1109.60.2)
        /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 59754.60.13)
        /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1770.255.0)
        /nix/store/0rsh55ighfv3fxidf3xz8vl3abiika0i-libiconv-99/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.60.1)

After

❯ otool -L ./result/bin/om
./result/bin/om:
        /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration (compatibility version 1.0.0, current version 1109.60.2)
        /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 59754.60.13)
        /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1770.255.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.60.1)

In After, libiconv is statically linked.

TODO:

@shivaraj-bh
Copy link
Member Author

macOS still has a dylib dependency on libiconv apart from system libraries (see TODO in the description for solution):

❯ otool -L ./result/bin/om
./result/bin/om:
        /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1770.255.0)
        /nix/store/0rsh55ighfv3fxidf3xz8vl3abiika0i-libiconv-99/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.60.1)

@srid
Copy link
Member

srid commented Jul 17, 2024

We should also test in Github Actions CI (public runner, not self-hosted) that the static binary runs (without Nix installed) on both platforms.

@srid
Copy link
Member

srid commented Jul 17, 2024

We should also test in Github Actions CI (public runner, not self-hosted) that the static binary runs (without Nix installed) on both platforms.

Down the line, we may even re-use #160 tests to use the static binary rather than the cargo bin.

@srid
Copy link
Member

srid commented Jul 19, 2024

Just added this to the todo list.

@srid srid mentioned this pull request Jul 19, 2024
4 tasks
@shivaraj-bh
Copy link
Member Author

Just added this to the todo list.

I guess it should be fine if we wrap static nix instead of the default one.

@srid
Copy link
Member

srid commented Jul 22, 2024

There's also nix bundle but I don't know if it (or a variation of it) works on macOS.

@srid
Copy link
Member

srid commented Jul 23, 2024

I think we should gets this PR into main anyway, without considering the nix-flake-schemas dependency:

[ ] Consider how Use flake-schemas #166 will impact this.

This PR will reduce the closure size of nix run ... when retrieved from cachix (#172), and omnix will continue to work as long as nix-flake-schemas is in the closure of omnix.

@srid
Copy link
Member

srid commented Jul 26, 2024

@shivaraj-bh Let's get the Linux static binary part merged to main before we do the same for macOS. This will help with switching GitHub actions from nixci to om ci. Currently, it takes 3 minutes just to install omnix.

@shivaraj-bh shivaraj-bh changed the title feat: Build static binary for omnix CLI on Linux and macOS* feat: Build static binary for omnix CLI on macOS* Jul 28, 2024
@shivaraj-bh shivaraj-bh mentioned this pull request Jul 28, 2024
2 tasks
@shivaraj-bh shivaraj-bh changed the title feat: Build static binary for omnix CLI on macOS* feat: Statically link libiconv on aarch64-darwin; Test static binary in CI Aug 1, 2024
@shivaraj-bh
Copy link
Member Author

image

@shivaraj-bh shivaraj-bh marked this pull request as ready for review August 1, 2024 19:28
@shivaraj-bh
Copy link
Member Author

image

@shivaraj-bh shivaraj-bh requested a review from srid August 1, 2024 19:32
Copy link
Member

@srid srid left a comment

Choose a reason for hiding this comment

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

@shivaraj-bh How should I test the binary on macOS? Currently getting:

image

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
nix/rust.nix Outdated Show resolved Hide resolved
@shivaraj-bh
Copy link
Member Author

@shivaraj-bh How should I test the binary on macOS? Currently getting:

image

I would assume, just copying and running the binary (after providing execute permissions using chmod), like in CI.

@shivaraj-bh
Copy link
Member Author

I believe it’s complete now. In another PR we should also look at releasing a tarball of these static binaries and provide a curl that users can use to download and run om.

@srid
Copy link
Member

srid commented Aug 2, 2024

In another PR we should also look at releasing a tarball of these static binaries and provide a curl that users can use to download and run om.

Yup.

I would assume, just copying and running the binary (after providing execute permissions using chmod), like in CI.

That's what I did. Anyway, let's deal with it (the user-level usage instructions) in the aforementioned another PR.

@srid srid merged commit 88fd9b0 into main Aug 2, 2024
5 checks passed
@srid srid deleted the static-om branch August 2, 2024 19:31
@shivaraj-bh
Copy link
Member Author

@shivaraj-bh How should I test the binary on macOS? Currently getting:

image

I was able to reproduce this. This happens when you download artifacts from a browser or any other app and try to run it. As in such a case Apple requires the binary to be notarized by them. But this is not a problem when you get the binary using curl.
You can try it:

curl -L \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer <YOUR-PAT-TOKEN>" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  https://api.github.com/repos/juspay/omnix/actions/artifacts/1780161532/zip --output om.zip
unzip om.zip
./om

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.

Static binary for omnix-cli
2 participants