Skip to content

Conversation

@Prince213
Copy link
Member

@Prince213 Prince213 commented Mar 25, 2025

Resolves #391350.

This PR removes sources.nix entirely by using fetchSubmodules.
It also moves the package to by-name (RFC 0140).

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Also other sources were updated:
- cbqn-bytecode: 0-unstable-2024-09-15 -> 0-unstable-2025-03-16
- replxx: didn't change
- singeli: 0-unstable-2024-09-29 -> 0-unstable-2025-03-13
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 25, 2025
@Prince213 Prince213 force-pushed the pkgs/cbqn-0.9.0 branch 2 times, most recently from 0fa9747 to 4700084 Compare March 25, 2025 14:21
@Prince213 Prince213 added 8.has: package (update) This PR updates a package to a newer version 8.has: clean-up This PR removes packages or removes other cruft labels Apr 12, 2025
@sternenseemann sternenseemann mentioned this pull request Apr 21, 2025
13 tasks
Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

I've created a PR with just the update (#400662) and will merge that as soon as CI passes, thanks. I think that change should be uncontroversial.

I don't really have an opinion on using fetchSubmodules, though actually using git for fetching can be slow and brittle – not sure. Maybe @AndersonTorres can describe why they implemented it as they did and whether it should or should not be changed?

I don't like moving cbqn to by-name, it doesn't really make sense for this sort of thing and makes the bootstrapping wiring-up in all-packages.nix even more confusing.

''
mkdir -p build/singeliLocal/
cp -r ${sources.singeli.src}/* build/singeliLocal/
cp -r build/singeliSubmodule/* build/singeliLocal/
Copy link
Member

Choose a reason for hiding this comment

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

If you remove the mkdir before, it can become

Suggested change
cp -r build/singeliSubmodule/* build/singeliLocal/
mv build/singeliSubmodule/ build/singeliLocal

Should also work for the other instances of this!

@AndersonTorres
Copy link
Member

I don't really have an opinion on using fetchSubmodules, though actually using git for fetching can be slow and brittle – not sure. Maybe @AndersonTorres can describe why they implemented it as they did and whether it should or should not be changed?

Obscure things about reproducibility. I remember some specific, illustrative examples like the yuzu corpse.

  • The issue of vendoring dependencies is an argument I employed against using fetchSubmodules.

  • Further, there are caveats (somewhere around comments in Nixpkgs) about git not following certain assumptions, so that .git is a source of impurities. This is another reason to not rely on git metadata at all.

  • Because of the above, I manually tracked these submodules and put them in their places, emulating the fetchSubmodules behavior.

@Prince213
Copy link
Member Author

Thanks for the input.

The thoughts I have on fetchSubmodules is that we don't have any patch at the moment and that it seems to be unhappy with newer version of submodules (it will not build). Also we don't really use much git metadata and we have fixed input with hash as well, so probably no problems with reproducibility.

What do you think? @sternenseemann @AndersonTorres

@AndersonTorres
Copy link
Member

I have no feelings on this.
Do it as you feel more comfortable.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 23, 2025
@wegank
Copy link
Member

wegank commented Jun 18, 2025

cbqn is at 0.9.0 on master, please rebase.

@wegank wegank marked this pull request as draft June 18, 2025 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (update) This PR updates a package to a newer version 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Request: cbqn 0.8.0 → 0.9.0

4 participants