-
Notifications
You must be signed in to change notification settings - Fork 0
Nix packaging #1
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
Conversation
nix/bext-di.nix
Outdated
version = "v1.3.2"; | ||
|
||
src = builtins.fetchTarball { | ||
url = "https://github.com/boost-ext/di/archive/refs/tags/${version}.tar.gz"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why isn't this a flake input? We can keep it that way, but when we already use flakes..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented the fetch this way because I saw the same pattern many times in derivations in nix packages. I have only used flake inputs to retrieve other flakes, so the idea did not even arise.
I like the suggestion but am unsure about some details. With the current implementation, I can update the version, and the tarball URL is automatically adapted. With flake inputs, would you recommend specifying a branch or a tag? My considerations and questions:
- branch (e.g., main, stable)
- flake update also updates to newest revision on the branch, can update to newer version
- tag (e.g., v1.3.2)
- flake input always points to same version, perhaps always to same commit
- version upgrade requires specifying another tag
In both cases, how do you handle the version field in the derivation? If you specified a branch, the version might change on flake update, but you have to manually check this and update the version field in the derivation. With a tag, you have to be aware of new releases and update flake input and version field. I don't like that this requires changes in two separate files (flake.nix, bext-di.nix), and I found that parsing the version from the flake input URL is possible. Is this approach good practice according to your experience?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In both cases, how do you handle the version field in the derivation?
As you rely on a versioned tarball currently, you would point to a stable tag. You are right, this requires keeping the version in the input and the derivation in sync on updates. You can probably build convenience around that, but I have commonly seen it done manually before.
vmicore/Readme.md
Outdated
@@ -4,6 +4,7 @@ | |||
# VMICore | |||
|
|||
_VMICore_ is a VM Introsepction tool capable of dynamic malware analysis. | |||
If you prefer to build with Nix, see [the Nix Readme](../nix/Readme.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This remark should move under the "How to Build" headline, maybe as a note/tip at the top (see https://github.blog/changelog/2023-12-14-new-markdown-extension-alerts-provide-distinctive-styling-for-significant-content/).
If you prefer to build with Nix, see [the Nix Readme](../nix/Readme.md). | |
[!TIP] | |
If you prefer to build with Nix, see [the Nix Readme](../nix/Readme.md). |
e580964
to
eb5c0cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Left two minor wording suggestions that you can address or ignore.
vmicore/Readme.md
Outdated
[!NOTE] | ||
If you prefer to build with Nix, see [the Nix Readme](../nix/Readme.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what this does, I think [!TIP]
is a better semantic fit. But a note is fine as well.
Tip
Helpful advice for doing things better or more easily.
vmicore/rust_src/.gitignore
Outdated
# For reproducible builds with Nix, we must check the file in, although it is a | ||
# library. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a "library". Maybe better wording:
# For reproducible builds with Nix, we must check the file in, although it is a | |
# library. | |
# For reproducible builds with Nix, we must check the file in, although it is auto-generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I get it now. This refers to the old guidance on committing the lock file for applications but not libraries. FYI: the guidance was updated a while back to "do what is best for your project", see https://blog.rust-lang.org/2023/08/29/committing-lockfiles/
Corrosion setup breaks due to new rustup toolchain list format since v1.28.0
the default behavior (retrieving libvmi from github:GDATASoftwareAG/libvmi) is not changed, but this change allows vcpkg or Nix to supply libvmi instead.
the default behavior (retrieving Corrosion from github:corrosion-rs/corrosion) is not changed, but this change allows vcpkg or Nix to supply Corrosion instead.
This change is necessary to enable offline compilation of the cxxbridge-cmd with Cargo. The offline compilation is a requirement to build in the Nix sandbox.
To enable reproducible builds (with Nix), Cargo.lock needs to be tracked in version control.
The fetchContent way of retrieving libvmi specifies the exact branch to retrieve; with find_package, it is possible that an incorrect version of libvmi is passed from vcpkg/nix/etc. This commit adds a check that ensure the functions GDATA added to their libvmi fork are present. The build would fail later, but this explicit error clarifies the issue.
FetchContent_Declare( | ||
Corrosion | ||
GIT_REPOSITORY https://github.com/corrosion-rs/corrosion.git | ||
GIT_TAG stable/v0.5 # Newer versions require cmake 3.22 which is not available on Debian 11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the comment is outdated. If you bump the version here, it means Debian 11 is no longer supported as a build platform?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the bump was done upstream, it seems without updating the comment.
This PR was just for internal review before the upstream PR (GDATASoftwareAG#154) |
This PR adds Nix packaging for smartvmi and all smartvmi's dependencies that are not yet packaged in nixpkgs.
The Nix-based build locks down all dependencies and prevents online access during the build process, resulting in reproducible builds and avoiding build behavior that is based on the configuration of the system the build runs on.
NixOS is not required to utilize smartvmi built by Nix—installing the Nix package manager, e.g., on Debian, is sufficient.