DNSvizor package#2018
Conversation
|
Please clean up the history and update the branch to the latest changes. |
bad: eval failure by IFD or build failure
My patches has been upstreamed.
My patches are applied using nix.
By default, buildbot-nix looks at "checks", which consists of "checks.x86_64-linux" and "checks.aarch64-linux". Our buildbot-nix CI runs on a x86_64-linux[1] machine so buildbot-nix errors out for IFDs needing to build on aarch64-linux systems. This patch fixes that error by letting buildbot-nix only look at "checks.x86_64-linux", which was made possible by [2]. Compared to the previous state, the only disadvantage is that we do not catch eval errors on aarch64-linux in the buildbot-nix CI any more. There are 3 possible alternative fixes: 1. ban IFD in ngipkgs 2. exclude "aarch64-linux" from "checks" 3. emulate aarch64-linux on a x86_64-linux machine using boot.binfmt.emulatedSystems = [ "aarch64-linux" ] The 1st alternative fix usually needs extra work to implement and usually means we have to commit generated lock files to ngipkgs repo. The 2nd alternative fix affects more than just buildbot-nix CI, such as "nix flake check", which may not be desirable. The 3rd alternative fix will slow down the buildbox-nix CI since emulating another system is slow. [1]: https://github.com/ngi-nix/ngipkgs/blob/dfab738d4a1d00f6c1b958be29163d672badf05f/infra/makemake/default.nix#L3 [2]: nix-community/buildbot-nix#318
ju1m
force-pushed
the
dnsvizor-package
branch
from
58bab27 to
837fc34
Compare
|
Just to point out this PR has the same runtime error as #1907. Hope it makes the state of this PR a little bit clearer. |
There was a problem hiding this comment.
Currently, the closure sizes are too large. For example, for hvt target:
$ nix path-info -Ssh ./result
/nix/store/0z8mf2p5496n5cjyhvz68rnxb6hrc7kp-mirage-dnsvizor-hvt-0-unstable-2025-12-17 16.5 MiB 964.1 MiBWe can shrink the closure to 17M by using doNixSupport (and maybe also removeOcamlReferences) of opam-nix.
There was a problem hiding this comment.
doNixSupport on dnsvizor.hvt
On dnsvizor.hvt (not any dependency):
-
doNixSupport=true && removeOcamlReferences=false(opam-nix's default):
/nix/store/vq0qajvv4x4sy1cyyjcanqr9kl7d8jml-mirage-dnsvizor-hvt-0-unstable-2025-12-17 16.6 MiB 964.5 MiB -
doNixSupport=false && removeOcamlReferences=false:
/nix/store/4gh7s6z0vjsw3wi724kqaw7lqfia8zqk-mirage-dnsvizor-hvt-0-unstable-2025-12-17 16.6 MiB 838.3 MiB -
doNixSupport=false && removeOcamlReferences=true:
/nix/store/4gh7s6z0vjsw3wi724kqaw7lqfia8zqk-mirage-dnsvizor-hvt-0-unstable-2025-12-17 16.6 MiB 838.3 MiB
So doNixSupport=false on dnsvizor.hvt reduces the full closure by 126.2 MiB (-13%).
doNixSupport=false on ocaml-solo5
Reduces the full closure of dnsvizor.hvt from 964.5 MiB to 938.4 MiB.
Combined with doNixSupport=false on dnsvizor.hvt the closures gets down to 808.9 MiB.
removeOcamlReferences on ocaml-solo5
Does nothing because it looks binaries in $out/bin.
Note that ocaml-solo5 contains its own OCaml compilers in $out/solo5-sysroot/bin/.
Custom calls to remove-references-to to remove some references
Could possibly help. I've not investigated much.
Custom outputs.
Might help. I've not investigated.
doNixSupport=false on other OCaml dependencies
Does not work, $/nix-support/propagated-native-build-inputs is required for building reverse dependencies.
removeOcamlReferences on other OCaml dependencies
Unlikely to help, for instance dnsvizor.hvt.passthru.packages-unmaterialized.pecu depends on dnsvizor.hvt.passthru.packages-unmaterialized.ocaml-base-compiler but has no $out/bin/
Static build
If this size is a matter for NGIpkgs' owners, a static build can be investigated, but this has a strong potential to be a worktime sink because simply using pkgsStatic and opam-nix.staticOverlay currently fails with:
opam2json-static-x86_64-unknown-linux-musl> File "src/opam2json.ml", line 1:
opam2json-static-x86_64-unknown-linux-musl> Error: I/O error: as -o 'src/opam2json.o' '/build/camlasmdba6bc.s'
opam2json-static-x86_64-unknown-linux-musl> make: *** [Makefile:10: opam2json] Error 2
error: Cannot build '/nix/store/ynrk85x09ipkfhdmpwncm9lb5yzq736n-opam2json-static-x86_64-unknown-linux-musl-0.4.drv'.
And so does:
$ nix -L build --allow-import-from-derivation github:tweag/opam-nix#checks.x86_64-linux.opam2json-static
[…]
> ocamlfind ocamlopt -package cmdliner -package opam-file-format -package yojson -linkpkg src/opam2json.ml -o opam2json
> File "src/opam2json.ml", line 120, characters 8-12:
> 120 | Term.(pure run $ arg_files)
> ^^^^
> Error: Unbound value pureThere was a problem hiding this comment.
Thanks for the info that doNixSupport and removeOcamlReferences do not very well to reduce closure size of MirageOS unikernel!
I guess we now know why there are some "unusual" behaviors in hillingar. Another reason why fragmented ecosystem is bad.
About static build: isn't MirageOS unikernels (excluding unix target`) already static-built?
To reduce nix closure size of MirageOS unikernel, I think just deleting $out/nix-support of the built unikernels is enough. Maybe we can contribute that (deleting $out/nix-support) to a config option of opam-nix.
I would consider this a blocking comment: there is really no reason to pull 1GB things when deploying dnsvizor if all we need is just 17MB dnsvizor.hvt.
There was a problem hiding this comment.
My opinion: let's merge this first and think about optimizing the size later. No need to block this if it works.
There was a problem hiding this comment.
I think just deleting $out/nix-support of the built unikernels is enough
Turns out deleting $out/nix-support is not enough. Now I agree that we can merge first and deal with closure size later.
FWIW, one selling point of MirageOS unikernel is that its deployment size (closure) is small.
There was a problem hiding this comment.
OK, I already have a working patch for the closure size. Will test it against #1944 and PR later.
There was a problem hiding this comment.
Here is the patch. It passes tests in #1944.
Feel free to include it in this PR and remove these two commits:
- perf/weight(DNSvizor): remove unneeded runtime closure items
- build/static(DNSvizor): add investigation notes
From 530c121157ac4fa3fc961390d90ccd0e0f908127 Mon Sep 17 00:00:00 2001
From: Lin Jian <me@linj.tech>
Date: Fri, 6 Feb 2026 21:13:07 +0800
Subject: [PATCH] mirage: remove unneeded references from closure
This patch reduces the closure size of DNSvizor from 1GB to 17MB.
---
pkgs/by-name/dnsvizor/mirage.nix | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/pkgs/by-name/dnsvizor/mirage.nix b/pkgs/by-name/dnsvizor/mirage.nix
index b0d8ab9..e79ca87 100644
--- a/pkgs/by-name/dnsvizor/mirage.nix
+++ b/pkgs/by-name/dnsvizor/mirage.nix
@@ -6,6 +6,7 @@
opam-nix,
stdenv,
writeShellApplication,
+ removeReferencesTo,
...
}:
@@ -84,6 +85,7 @@ rec {
__intentionallyOverridingVersion = true;
# ToDo: pick depexts of deps in monorepo?
buildInputs = previousAttrs.buildInputs ++ depexts;
+ nativeBuildInputs = previousAttrs.nativeBuildInputs or [ ] ++ [ removeReferencesTo ];
env =
previousAttrs.env or { }
// lib.optionalAttrs (finalOpam ? ocaml-solo5) {
@@ -109,6 +111,21 @@ rec {
cp -L ${mirageDir}/dist/${pname}* $out/
runHook postInstall
'';
+ # reduce closure size
+ postFixup =
+ let
+ unneededReferences' = [
+ finalOpam.ocaml-solo5
+ finalOpam.solo5
+ ]
+ ++ depexts;
+ unneededReferences = unneededReferences' ++ map lib.getDev unneededReferences';
+ in
+ ''
+ ${previousAttrs.postFixup or ""}
+ rm -vrf $out/nix-support
+ remove-references-to -t ${lib.concatStringsSep " -t " unneededReferences} $out/*
+ '';
});
}
);
--
2.52.0There was a problem hiding this comment.
Upstream-built dnsvizor.hvt is only 9MB. However, ours is 17MB.
It turns out that we do not strip!
not-stripped -> 17MB
strip debug symbols (`-S`) -> 12MB
strip all symbols (`-s`) -> 9.1MB
After stripping all symbols, we reduce the difference to only 0.1MB.
There was a problem hiding this comment.
Instead of vendoring a MirageOS unikernel build helper (the builds function in this file) in Ngipkgs, I think it is better to contribute improvements to hillingar. Reasons are as follows:
- The diff between this and hillingar seems not large.
- It good for the ecosystem to be less fragmented. We can automatically benefit from other potential improvements of hillingar in the future.
There was a problem hiding this comment.
That was my first attempt as can be seen in the old commit history (preserved in the long commit description because I was asked to clean it), until the changes became so substantial both breaking the interfaces (using attrs, passthru, proper phases, env, …) and the implementation logic (no src filtering, materialization, updates), that discussing all of this would become a worktime sink.
On 1., I am not of this opinion. In fact, the rewrite is so complete and the resulting logic so different that I don't see how any copyright claim could even hold at this point.
On 2. I would argue that hillingar is the fragmentation to begin with, that contributing to it would comfort that, whereas having it in NGIpkgs, which is a financed effort often acting as a testing ground for Nixpkgs, is more likely to defragment the ecosystem.
Also this is assuming future changes in hillingar are actual improvements and not bugs or regressions.
This being said, if NGIpkgs' owners prefer to have it move to hillingar (provided it becomes Open Source) or Nixpkgs, I can try.
There was a problem hiding this comment.
Some words about 2:
When users trying to use nix to build MirageOS unikernel, if that unikernel is not packaged in Ngipkgs, they probably will find hillingar instead of Ngipkgs. Presumably, more adoption and contribution will happen there.
ju1m
force-pushed
the
dnsvizor-package
branch
from
8a8ea84 to
7aa8f3a
Compare
There was a problem hiding this comment.
Discussed today in the review meeting with the conclusion that we should merge this since it's working and make improvements in followups. As such, I've created an issue for the closure size and I invite you to do the same for similar things.
|
My gut feeling is that the last commit |
Sure, I included my notes to be sure any future will quickly be aware of my best guess at attacking the closure problem. |
ju1m
force-pushed
the
dnsvizor-package
branch
from
7aa8f3a to
461777f
Compare
jian-lin
left a comment
jian-lin
left a comment
There was a problem hiding this comment.
I did a quick review and it looks good!
…e IFD flex/adapt(mirage): rewrite in a more idiomatic way flex/adapt(hillingar): reproduce the previous state with a vendored-in version maint/format(mirage): hillingar is not using nixfmt-rfc-style maint/update(mirage): `overrideScope'` no longer exist in opam-nix feat/role(mirage): remove `src`-filtering from build helper maint/clean(mirage): remove unused dependencies feat/role(mirage): remove `phases`-filtering from build helper compat/standardize(mirage): use `package.nix`
This reverts commit 4b6ad3f.
eljamm
force-pushed
the
dnsvizor-package
branch
from
461777f to
6420761
Compare
eljamm
left a comment
eljamm
left a comment
There was a problem hiding this comment.
Fixed formatting. I'm merging this as previously mentioned, so please open new PRs for any followup changes.
| call = self.newScope { | ||
| nixdoc-to-github = self.callPackage sources.nixdoc-to-github { }; | ||
| dream2nix = sources.dream2nix; | ||
| hillingar = sources.hillingar.lib.${system}; |
There was a problem hiding this comment.
Why don't replace hillingar with opam-nix here? Why is opam-nix added via pkgs/by-name/default.nix?
There was a problem hiding this comment.
Because it is the call/callPackage used to import pkgs/by-name/default.nix but not the same used to import pkgs/by-name/**/package.nix.
At least when I try it raises:
error: evaluation aborted with the following error message: 'lib.customisation.callPackageWith: Function called without required argument "opam-nix" at /home/julm/src/nix/NGIpkgs/pkgs/by-name/dnsvizor/package.nix:7'
So one can either
- Add
opam-nixinpkgs/lib.nix, and pass it down inpkgs/by-name/default.nix'scallPackage. - Or add
opam-nixdirectly inpkgs/by-name/default.nix'scallPackage(currently done) - Or add
opam-nixinpkgs/lib.nix, and inheritcallPackage, removing that seemingly uselesscallPackageredefinition. (probably clearer)
| inputs.devshell.inputs.nixpkgs.follows = "nixpkgs"; | ||
| inputs.devshell.url = "github:numtide/devshell"; | ||
|
|
||
| inputs.nix-filter.url = "github:numtide/nix-filter/3e1fff9"; |
There was a problem hiding this comment.
It seems nix-filter is not used anywhere?
|
|
||
| # FixMe(maint/upstream): merge this branch upstream | ||
| #inputs.opam-nix.url = "github:tweag/opam-nix"; | ||
| inputs.opam-nix.url = "github:ju1m/opam-nix/materialize-monorepo"; |
There was a problem hiding this comment.
So I have been thinking about removing opam-nix for a long time. Just checked the missing ocaml deps and turns out there are only 8. Sounds promising!
The motivation of doing so is that
- we can upstream dnsvizor to Nixpkgs
- we can remove the logic of opam-nix and its generated big json files
3 participants
Build
Update
Done
hillingarwithlib/pkgs/mirage.nix--allow-import-from-derivationmain.Solo5: trap: type=#PFdue to Thread-Local StorageRelations
buildOpamMonorepotweag/opam-nix#149 to support removing IFD, hence my fork being used inflake.nixfor now.opam-nix#2016