Skip to content

Bonfire: 1.0.1-beta.11 -> 1.0.2-alpha.23#2155

Draft
ju1m wants to merge 2 commits intongi-nix:mainfrom
ju1m:bonfire-update
Draft

Bonfire: 1.0.1-beta.11 -> 1.0.2-alpha.23#2155
ju1m wants to merge 2 commits intongi-nix:mainfrom
ju1m:bonfire-update

Conversation

@ju1m
Copy link

@ju1m ju1m commented Feb 12, 2026

Done

  • Update Bonfire to latest version.
  • Move the package set of flavours into the passthru of a derivation building all Bonfire flavours.
    Which makes it compatible with nix-update --flake --use-update-script bonfire.
  • Fix breaking changes introduced by deps_nix: generated deps.nix no longer ask for pkgs but to be used with callPackage.
  • Fix BEAM help builders due to callPackage now used on mixNixDeps.

Checked

  • nom build -f. projects.Bonfire.tests.basic

Copy link
Contributor

@jian-lin jian-lin Feb 12, 2026

Choose a reason for hiding this comment

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

Move the package set of flavours into the passthru of a derivation building all Bonfire flavours.
Which makes it compatible with nix-update --flake --use-update-script bonfire.

This is a very unusual treatment for package sets.

If #2154 is merged, this change is not needed.

In addition, in case you do not know, the building package CI can handle package sets well.

Copy link
Author

@ju1m ju1m Feb 12, 2026

Choose a reason for hiding this comment

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

  1. This PR is still a draft, that I push to at the end of each working day to get permalinks for my work log.
  2. I've been working on this draft for a day before you announced Allow running updateScript of package sets #2154 and at a time where you were retracting comments announcing you would be working on Allow running updateScript of package sets #2154 ; letting me believe nothing would change in the foreseeable future.
  3. I'll gladly rebase on top of your work and use it whenever available in main, unless a package set sharing a same src causes troubles. That I don't know yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

unless a package set sharing a same src causes troubles

There won't be troubles now since we update one package at a time. Maybe there will be some race condition if we update multiple packages at the same time in the future.

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately updating one package at a time would not remove the race condition.
Because if the second package updates the src, then the first package will not have the correct src anymore.
This happened to me when writing this PR between bonfire-1.0.2-alpha.22 and bonfire-1.0.2-alpha.23.
Now with the current updateScript, src is updated only once before updating the flavours, so this would not happen anymore.

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.

I am not familiar with bonfire, but I guess it is similar to dnsvizor and you are familiar with dnsvizor, so maybe #2174, that is updating src and deps of all packages in lockstep, is one way to avoid that issue?

Due to my limited knowledge of bonfire, I will refrain from reviewing this PR. Sorry for the noise.

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.

Yes, like DNSvizor (or all MirageOS packages for that matter) Bonfire is composed of multiple packages all sharing a single src.

But, AFAICS #2174 does not prevent the problem because each target update tries to unstableGitUpdater the shared src right? (and the same likely holds for the global flake inputs, which also impacts all other packages using them).
AFAICS, the update of src and flake inputs is there done per-target but AFAIU:

  • for src it should be once for all targets when src is shared,
  • and for flake inputs it should be at the beginning of a global update, not per-package nor per-target.

The "unusual treatment" introducing a top derivation in charge of updating src and then each target enables this using a top updateScript. Maybe #2154 can provide the same, but maybe it's so uncommon that it's better to keep the "unusual treatment" for the few packages that share a src.

Another solution is to no longer share any src or flake inputs.

So, IMHO, if #2154 cannot or does not support src-sharing between packages of a package set, then both Bonfire's flavours and DNSvizor's targets need an "unusual treatment" like the one there to avoid having one flavour/target's materialization corresponding to a more recent src while another's corresponding to another more ancient src.
Which happens when upstream tags a new release between updating two flavours or targets
(which happened to me with Bonfire and is likely to happen again due to the ~1h long update per flavour).

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.

I see your points. To sum up, your unusual treatment is more correct (for updateScript only, it has a few other issues) but unusual 😅; #2174 is more common but slight less correct (considering that its updateScript only takes 40s to run).

For slow updates, your unusual treatment is a bit more justified. However, I do not think so for fast updates like mirage.


Here is my defence for #2174.

Basically, all mirage targets share the same updateScirpt. In that updateScript, src is updated and then related flake inputs and deps of all targets are updated.

each target update tries to unstableGitUpdater the shared src right

Yes. What's the problem?

for src it should be once for all targets when src is shared

It does not hurt to do it multiple times on all targets though. Note that updateScirpt takes only about 40 seconds. It is almost impossible that src is updated when we are running the same updateScript for all targets sequentially. After the first run, src update is almost a no-op in the following runs.

and for flake inputs it should be at the beginning of a global update, not per-package not per-target

Similar to src, updating flake inputs multiple times does not hurt.

both Bonfire's flavours and DNSvizor's targets need an "unusual treatment" like the one there to avoid having one flavour/target's materialization corresponding to a more recent src while another's corresponding to another more ancient src.

Almost impossible for dnsvizor.


Another solution is to no longer share any src or flake inputs.

How should that be implemented for dnsvizor? This is a genuine question.

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.

2 participants

Comments