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

add nix flake #261

Merged
merged 18 commits into from Oct 12, 2023
Merged

add nix flake #261

merged 18 commits into from Oct 12, 2023

Conversation

undefined-moe
Copy link
Contributor

@undefined-moe undefined-moe commented Jun 10, 2023

Pull Request Template

Description

This pull request adds flake.nix, to simplify packaging process.
Use nix bundle --bundler github:ralismark/nix-appimage .#ecli-rs, nix bundle --bundler github:ralismark/nix-appimage .#ecli-server to create a static linked AppImage file.

NOTE: this requires ecli/Cargo.lock exists and added into git tree (git add ecli/Cargo.lock -f), but no need to commit it.

P.S: According to Cargo Book, "If you’re building an end product, which are executable like command-line tool or an application, or a system library with crate-type of staticlib or cdylib, check Cargo.lock into git."

I've tested that nix bundle --bundler github:NixOS/bundlers#toDockerImage .#ecli-rs also works well, producing a ~27M docker image.

See also: #258

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

N/A

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! You are to making eunomia-bpf even better.

@oluceps
Copy link
Member

oluceps commented Jun 13, 2023

Hi, Thanks for your contributions!
You might be interested in taking a look at some further plans about nix integration on eunomia-bpf: #158

@yunwei37
Copy link
Member

yunwei37 commented Aug 12, 2023

Any progress on this pr?Is it ready for review?

undefined-moe and others added 11 commits October 12, 2023 01:24
flake: complete eunomia-bpf reproducible build

This commit brings:
- Fix issue mentioned in 158
- Update way of building ecli to adapt code changes
- Add dev env with full eunomia-bpf toolchain

flake: add pre-commit-hooks

This commit adds pre commit check based on nix flake.

chore: apply suggestions from flake check

- Apply format suggestion from black
- Rename file with space-separated names, which
  is not recommend on unix system

flake: add wasm-bpf cli to dev env

flake: devShell subdivision

flake: sync bpftool to wasm-bpftool branch

doc: fix display error in README

flake: fetch vmlinux

chore: update deprecated lock

flake: adapt code changes on master

flake: add missing include

flake: add apps

fix: cargo lock mismatch

Previous version of Cargo.lock show `error: no
matching package named bpf-oci found`.

fix: nix build ecli

flake: fix bpftool version
@oluceps oluceps mentioned this pull request Oct 12, 2023
15 tasks
flake.nix Outdated
Comment on lines 32 to 42
nativeBuildInputs = with pkgs; [
pkg-config
zlib.static
elfutils
zlib
openssl.dev
cargo
rustc
rustPlatform.cargoSetupHook
rustPlatform.bindgenHook
];
Copy link
Member

Choose a reason for hiding this comment

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

openssl, elfutils and zlib should belong to buildInputs.

flake.nix Outdated
Comment on lines 48 to 52
installPhase = ''
mkdir -p $out/bin
install -Dm 755 target/release/ecli-rs $out/bin/
install -Dm 755 target/release/ecli-server $out/bin/
'';
Copy link
Member

Choose a reason for hiding this comment

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

Don't forget the runHook.

flake.nix Outdated
Comment on lines 72 to 75
installPhase = ''
mkdir -p $out/bin
cp $src/bin/ecli-server $out/bin/ecli-server
'';
Copy link
Member

Choose a reason for hiding this comment

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

runHook

flake.nix Outdated
Comment on lines 104 to 113
buildPhase = ''
cd compiler/cmd
export OUT_DIR=.
cargo build --release
'';
installPhase = ''
mkdir -p $out/bin
chmod +x target/release/ecc-rs
cp target/release/ecc-rs $out/bin/ecc
'';
Copy link
Member

Choose a reason for hiding this comment

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

runHook

@ocfox
Copy link
Member

ocfox commented Oct 12, 2023

I clicked approve by mistake, please ignore it. Thanks for your contribution.

@github-actions github-actions bot added the doc label Oct 12, 2023
@oluceps oluceps marked this pull request as ready for review October 12, 2023 03:26
@auto-assign auto-assign bot requested a review from yunwei37 October 12, 2023 03:26
@oluceps oluceps requested a review from ocfox October 12, 2023 03:27
Copy link
Member

@ocfox ocfox left a comment

Choose a reason for hiding this comment

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

LGTM now, wait for ci tests.

@ocfox
Copy link
Member

ocfox commented Oct 12, 2023

Before merging, some conflicts need to be resolved. @oluceps could you help with this?

@ocfox ocfox merged commit c33bd1b into eunomia-bpf:master Oct 12, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants