Skip to content

bcachefs: fix subvolume mounting#1058

Merged
Enzime merged 4 commits intonix-community:masterfrom
nothingnesses:nix-community/disko#1045
Dec 14, 2025
Merged

bcachefs: fix subvolume mounting#1058
Enzime merged 4 commits intonix-community:masterfrom
nothingnesses:nix-community/disko#1045

Conversation

@nothingnesses
Copy link
Contributor

This fixes #1045 and updates the bcachefs test to handle the changes made. It also replaces nixpkgs with a version from this PR. This part will need to be reverted, once the changes from that version gets merged upstream.

@Enzime
Copy link
Member

Enzime commented Jun 7, 2025

Please don’t change the Nixpkgs input to your own fork, you’ll need to wait till it gets merged upstream

@Enzime Enzime marked this pull request as draft June 7, 2025 00:01
@VlaDexa
Copy link

VlaDexa commented Oct 25, 2025

That PR got merged 🎉

@Enzime
Copy link
Member

Enzime commented Oct 25, 2025

https://nixpkgs-tracker.ocfox.me/?pr=414391

Currently only on master, not nixpkgs-unstable yet

@VlaDexa
Copy link

VlaDexa commented Oct 30, 2025

It's in every channel now 🎉

@Enzime
Copy link
Member

Enzime commented Nov 4, 2025

@nothingnesses can you rebase this PR?

@nothingnesses nothingnesses force-pushed the nix-community/disko#1045 branch 4 times, most recently from 6fe546d to d6d6c38 Compare November 21, 2025 18:42
@nothingnesses nothingnesses marked this pull request as ready for review November 21, 2025 18:43
Comment on lines +22 to +28
extraModulePackages = [
(config.boot.kernelPackages.callPackage pkgs.bcachefs-tools.kernelModule {})
];
# Can also use the following instead of the above,
# which doesn't necessitate passing in `config`,
# so `{ config, ... }: ` can be removed:
# supportedFilesystems = [ "bcachefs" ];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Enzime @Mic92 Thoughts on this? I'm not sure which one to use.

Copy link
Member

Choose a reason for hiding this comment

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

Using supportedFilesystems sounds good, maybe you can see if this also adds bcachefs-tools to environment.systemPackages

@nothingnesses
Copy link
Contributor Author

Looks like the disko-install test failed when it timed out trying to download this patch. Can someone restart it please?

@Enzime
Copy link
Member

Enzime commented Nov 21, 2025

I ran it again but it's failing to download the patch inside the VM test which means there's a dependency missing from:

dependencies = [
self.nixosConfigurations.testmachine.pkgs.stdenv.drvPath
(self.nixosConfigurations.testmachine.pkgs.closureInfo { rootPaths = [ ]; }).drvPath
# https://github.com/NixOS/nixpkgs/blob/f2fd33a198a58c4f3d53213f01432e4d88474956/nixos/modules/system/activation/top-level.nix#L342
self.nixosConfigurations.testmachine.pkgs.perlPackages.ConfigIniFiles
self.nixosConfigurations.testmachine.pkgs.perlPackages.FileSlurp
self.nixosConfigurations.testmachine.config.system.build.toplevel
self.nixosConfigurations.testmachine.config.system.build.diskoScript
]

@nothingnesses
Copy link
Contributor Author

@Enzime I pushed a commit to add a dependency and it looks like the test passes now.

@nothingnesses
Copy link
Contributor Author

nothingnesses commented Nov 30, 2025

@Enzime Just wondering what the status is on this. Is there anything else I need to do on my end?

@crasm
Copy link
Contributor

crasm commented Dec 3, 2025

I’ve been using this PR successfully with my new config and would like to see this merged.

'';
};
in
pkgs.lib.attrsets.recursiveUpdate diskoTest {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using recursiveUpdate you can pass the relevant NixOS configuration through as an arg to makeDiskoTest using either extraSystemConfig or extraInstallerConfig

# Test that verbose option was set for "/".
machine.succeed("""
findmnt --json \
findmnt -J \
Copy link
Member

Choose a reason for hiding this comment

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

I would leave all of these as --json as it's a lot more clear than -J

Comment on lines +22 to +28
extraModulePackages = [
(config.boot.kernelPackages.callPackage pkgs.bcachefs-tools.kernelModule {})
];
# Can also use the following instead of the above,
# which doesn't necessitate passing in `config`,
# so `{ config, ... }: ` can be removed:
# supportedFilesystems = [ "bcachefs" ];
Copy link
Member

Choose a reason for hiding this comment

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

Using supportedFilesystems sounds good, maybe you can see if this also adds bcachefs-tools to environment.systemPackages

@nothingnesses
Copy link
Contributor Author

nothingnesses commented Dec 10, 2025

@Enzime Thanks for the review! I've pushed more commits to address the points you made. Please let me know if you think there's any other changes needed.

@nothingnesses nothingnesses requested a review from Enzime December 10, 2025 15:22
@Enzime Enzime force-pushed the nix-community/disko#1045 branch from 7bd56a2 to 1e95379 Compare December 14, 2025 04:42
@Enzime Enzime changed the title Fix nix-community/disko#1045 bcachefs: fix subvolume mounting Dec 14, 2025
@Enzime Enzime added this pull request to the merge queue Dec 14, 2025
@Enzime
Copy link
Member

Enzime commented Dec 14, 2025

Thanks :)

Merged via the queue into nix-community:master with commit be1a6b8 Dec 14, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bcachefs subvolume mounting is broken?

4 participants