Skip to content

Mirage/DNSvizor: improvements#2063

Open
ju1m wants to merge 8 commits intongi-nix:mainfrom
ju1m:dnsvizor
Open

Mirage/DNSvizor: improvements#2063
ju1m wants to merge 8 commits intongi-nix:mainfrom
ju1m:dnsvizor

Conversation

@ju1m
Copy link

@ju1m ju1m commented Feb 10, 2026

A second round on DNSvizor's packaging.

Done

  • Address pending concerns (see commits' descriptions) discussed in DNSvizor package #2018
  • Use a package set dsnvizor.${target} to preserve a good UX
  • Introduce an additional dnsvizor.update package in the set to update src and all targets correctly (ie. updating src only once before materializing all targets). Note that the opam-nix's input is not updated, as it will possibly be used by other packages in the future and that could invalidate their materialization. The correct place to update globally shared inputs is at the beginning of a update-all.
  • Repin upstream opam-nix instead of a ad-hoc fork to support non-IFD builds, now that Materialize buildOpamMonorepo tweag/opam-nix#149 has been merged

Building

$ nix -L build --no-link --print-out-paths -f. dnsvizor
/nix/store/p99xnlb90wv65g3j4w5b9h7iwwd6vx7d-dnsvizor-unix-0-unstable-2026-01-21
/nix/store/9ianik2l59q7j9z5f5ql7yyaajiaji3n-dnsvizor-spt-0-unstable-2026-01-21
/nix/store/vc2hfvl4wg6l9pf333f1ns4j4xxk2cjx-dnsvizor-virtio-0-unstable-2026-01-21
/nix/store/n8swqzy9igdzrca24ykvij0xxi7g4ckh-dnsvizor-xen-0-unstable-2026-01-21
/nix/store/agn8wamxd11gzrk3bz07qlam6lki9749-dnsvizor-qubes-0-unstable-2026-01-21
/nix/store/wzmjmp2mg85984jfrxwr5x0cgwm0mih0-dnsvizor-muen-0-unstable-2026-01-21
/nix/store/d4nqwjg0xsf70njd7gj2c3hdggzvz972-dnsvizor-0-unstable-2026-01-21
/nix/store/wmzhsrn95qz4a7ir5vld6dnihns1qdki-dnsvizor-hvt-0-unstable-2026-01-21

Note the dnsvizor-0-unstable-2026-01-21 out path, corresponding to dnsvizor.update, but the name is set to match what is expected to be read when updating.

Updating

  • Requires:
    • Allow running updateScript of package sets #2154 and update dnsvizor.update (!!! not update dnsvizor !!!)
    • or: nix -L develop --impure --expr 'import <nixpkgs/maintainers/scripts/update.nix> { include-overlays = [ (final: prev: { inherit (import ./. {}) ngipkgs; }) ]; package = "ngipkgs.dnsvizor.update"; }'

Discoverability

$ nix flake show | grep dnsvizor
│   │   ├───"packages/dnsvizor/hvt" omitted (use '--all-systems' to show)
│   │   ├───"packages/dnsvizor/spt" omitted (use '--all-systems' to show)
│   │   ├───"packages/dnsvizor/unix" omitted (use '--all-systems' to show)
│   │   ├───"packages/dnsvizor/update" omitted (use '--all-systems' to show)
│       ├───"packages/dnsvizor/hvt": derivation 'mirage-dnsvizor'
│       ├───"packages/dnsvizor/muen": derivation 'mirage-dnsvizor'
│       ├───"packages/dnsvizor/qubes": derivation 'mirage-dnsvizor'
│       ├───"packages/dnsvizor/spt": derivation 'mirage-dnsvizor'
│       ├───"packages/dnsvizor/unix": derivation 'mirage-dnsvizor'
│       ├───"packages/dnsvizor/update": derivation 'dnsvizor-0-unstable-2026-01-21'
│       ├───"packages/dnsvizor/virtio": derivation 'mirage-dnsvizor'
│       ├───"packages/dnsvizor/xen": derivation 'mirage-dnsvizor'
│   │   ├───"dnsvizor/hvt" omitted (use '--all-systems' to show)
│   │   ├───"dnsvizor/spt" omitted (use '--all-systems' to show)
│   │   ├───"dnsvizor/unix" omitted (use '--all-systems' to show)
│   │   ├───"dnsvizor/update" omitted (use '--all-systems' to show)
│       ├───"dnsvizor/hvt": package 'mirage-dnsvizor'
│       ├───"dnsvizor/muen": package 'mirage-dnsvizor'
│       ├───"dnsvizor/qubes": package 'mirage-dnsvizor'
│       ├───"dnsvizor/spt": package 'mirage-dnsvizor'
│       ├───"dnsvizor/unix": package 'mirage-dnsvizor'
│       ├───"dnsvizor/update": package 'dnsvizor-0-unstable-2026-01-21'
│       ├───"dnsvizor/virtio": package 'mirage-dnsvizor'
│       ├───"dnsvizor/xen": package 'mirage-dnsvizor'

Alternative

@ju1m ju1m force-pushed the dnsvizor branch 2 times, most recently from 4705276 to b6793ef Compare February 10, 2026 16:57
@ju1m ju1m force-pushed the dnsvizor branch 2 times, most recently from f8e9a06 to 8d82c04 Compare February 11, 2026 03:18
@ju1m
Copy link
Author

ju1m commented Feb 11, 2026

Don't know why the CI fails:

$ nix build -L --show-trace --option keep-going true --max-silent-time 1200 --accept-flake-config --out-link result-x86_64-linux.packages/dnsvizor '/nix/store/y905dkcdgyh0fi08g44b59wckgm4zka6-dnsvizor-0-unstable-2026-01-21.drv^*'
 in dir /var/lib/buildbot-worker/worker-004/ngi-nix_ngipkgs_nix-build/build (timeout 10800 secs)
 watching logfiles {}
 argv: [b'nix', b'build', b'-L', b'--show-trace', b'--option', b'keep-going', b'true', b'--max-silent-time', b'1200', b'--accept-flake-config', b'--out-link', b'result-x86_64-linux.packages/dnsvizor', b'/nix/store/y905dkcdgyh0fi08g44b59wckgm4zka6-dnsvizor-0-unstable-2026-01-21.drv^*']
 using PTY: False
this derivation will be built:
  /nix/store/y905dkcdgyh0fi08g44b59wckgm4zka6-dnsvizor-0-unstable-2026-01-21.drv
building '/nix/store/y905dkcdgyh0fi08g44b59wckgm4zka6-dnsvizor-0-unstable-2026-01-21.drv'...
dnsvizor> Running phase: installPhase
error: cannot create symlink '/var/lib/buildbot-worker/worker-004/ngi-nix_ngipkgs_nix-build/build/result-x86_64-linux.packages/dnsvizor'; already exists
program finished with exit code 1
elapsedTime=0.198748

@eljamm
Copy link
Contributor

eljamm commented Feb 11, 2026

Although I haven't looked much into details, dnsvizor is no longer an attribute set after these changes, and instead is just a derivation that produces an empty result.

Before:

nix-repl> dnsvizor
{
  hvt = «derivation /nix/store/9wd8z9v53c1gcpkb0py5if181qxsdxp7-mirage-dnsvizor-hvt-0-unstable-2026-01-21.drv»;
  muen = «derivation /nix/store/46chvcldkab5k429qlkrf9qrdm68jfi8-mirage-dnsvizor-muen-0-unstable-2026-01-21.drv»;
  override = { ... };
  overrideDerivation = «lambda overrideDerivation @ /nix/store/vcd1brksxy9j2gxrsdmzph5hvq649pvr-source/lib/customisation.nix:203:32»;
  qubes = «derivation /nix/store/g0wab6s9lcziazjiskv9qwfapzafi9ly-mirage-dnsvizor-qubes-0-unstable-2026-01-21.drv»;
  recurseForDerivations = true;
  spt = «derivation /nix/store/sy76q5i1knsh9byajdjafc79q0wv07xh-mirage-dnsvizor-spt-0-unstable-2026-01-21.drv»;
  unix = «derivation /nix/store/8413m3bhzjx2z9p0ai9jlkk8y6v3ibff-mirage-dnsvizor-unix-0-unstable-2026-01-21.drv»;
  updateScript = «derivation /nix/store/85wysys65g6vviq87ydmb737z1nxzzni-dnsvizor-update.drv»;
  virtio = «derivation /nix/store/4nw88sf8nvicapn1z6mzd51612d9qybv-mirage-dnsvizor-virtio-0-unstable-2026-01-21.drv»;
  xen = «derivation /nix/store/pbh5jx059xh2921l2ip8zz8xqmym8zq7-mirage-dnsvizor-xen-0-unstable-2026-01-21.drv»;
}

After:

nix-repl> dnsvizor
«derivation /nix/store/y905dkcdgyh0fi08g44b59wckgm4zka6-dnsvizor-0-unstable-2026-01-21.drv»

With:

$ nix-build -A dnsvizor && ls -l ./result
/nix/store/5p0k9anj1wfb00r5hn0nkfcrgqmq378f-dnsvizor-0-unstable-2026-01-21

@jian-lin
Copy link
Contributor

jian-lin commented Feb 11, 2026

Although I haven't looked much into details, dnsvizor is no longer an attribute set after these changes

Are you replying to #2063 (comment)? I CCed you in case you have some suggestion to fix the general issue of updateScript for package sets/scopes.

IIUC, this PR tries to work around the package set updateScript issue by making dnsvizor not a package set. Hopefully, that won't slow down the build.
EDIT: the workaround in this PR breaks package building of CI: CI tries to build checks.x86_64-linux.package/dnsvizor but that is just an "empty" package. The actual packages, such as dnsvizor.hvt and dnsvizor.unix, are not built by CI.

@ju1m
Copy link
Author

ju1m commented Feb 11, 2026

@eljamm, yes, $out being an empty directory was on purpose,
I've now changed $out to include the build results of all the targets yet the CI still fails with the same error.
Apparently something tries to create
/var/lib/buildbot-worker/worker-003/ngi-nix_ngipkgs_nix-build/build/result-x86_64-linux.packages/dnsvizor twice.
Is /var/lib/buildbot-worker/worker-003/ngi-nix_ngipkgs_nix-build/build/result-x86_64-linux.packages/ completely erased between each build of the CI?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not a fan of the unusual treatment of package set. I propose #2174 as an alternative to support updateScript. Reviews are welcome.

Other changes of this PR, such as using extendMkDerivation, looks good to me. Maybe it works better to merge other changes first and deal with updateScript later?

WDYT?

Copy link
Author

Choose a reason for hiding this comment

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

#2174 is not stacked upon this PR, so either I or you will have to resolve the conflicts.
But if it were stacked then you'll have to rebase each time I push a new revision.

I suggest we organize like that:

  1. I continue to focus on mirage.nix
  2. You continue to focus on the underlying improvements in Allow running updateScript of package sets #2154 being merged, and then just ask me to remove the "unusual treatment" (that has the good property to solve problems you raised at a time where Allow running updateScript of package sets #2154 did not exist).

WDYT?

Copy link
Contributor

@jian-lin jian-lin Feb 13, 2026

Choose a reason for hiding this comment

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

If #2154 and #2174 are merged, then I am willing to rebase this PR if you are not.

imincik
imincik previously approved these changes Feb 13, 2026
Copy link
Contributor

Choose a reason for hiding this comment

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

This unusual treatment of package sets has some issues:

  • damage discoverability: for example, nix flake show only shows dnsvizor, but not the actual ones such as dnsvizor.hvt
  • bad UX: the workaround for package build CI, i.e., building all targets when building dnsvizor, can have bad UX

Copy link
Author

@ju1m ju1m Feb 13, 2026

Choose a reason for hiding this comment

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

on your first point I can try to:

  • move the targets back in a package set, with a dummy updateScript not doing anything.
  • add to that package set a dnsvizor-udpate dummy package providing an updateScript updating the targets correctly.

on your second point: if it does not serve any purpose, I'll remove it.

Copy link
Contributor

@jian-lin jian-lin Feb 13, 2026

Choose a reason for hiding this comment

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

add to that package set a dnsvizor-udpate dummy package providing an updateScript updating the targets correctly.

Oh, this sounds much better!

I pushed b1d05e7 implementing it.

@ju1m ju1m marked this pull request as draft February 16, 2026 02:58
@ju1m ju1m marked this pull request as ready for review February 16, 2026 05:02
Julien Moutinho added 4 commits February 23, 2026 22:49
- use multiple outputs instead of multiple derivations
- use a single materializedDir
- use lib.extendMkDerivation
- use camelCase
- use nix eval to get materialization paths
- use stripDebugFlags instead of stripAllList
- use $out/share/mirageos (automatically stripped) instead of $out
- use upstream's letter casing: "DNSvisor" in documentation
Warning: currently this does not update inputs pinned in flake.lock
@ju1m
Copy link
Author

ju1m commented Feb 23, 2026

  • Rebase on main to resolve conflict in flake.lock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants