Skip to content

Update bcachefs, fix build in hydra #97599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Sep 17, 2020
Merged

Update bcachefs, fix build in hydra #97599

merged 3 commits into from
Sep 17, 2020
+18 −16

Conversation

Atemu
Copy link
Member

@Atemu Atemu commented Sep 9, 2020 •
edited
Loading

Motivation for this change

https://hydra.nixos.org/build/126571067

https://hydra.nixos.org/build/126999921

ZHF: #97479

Bcachefs was also very out-of-date

Closes #91749
Closes #96251

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc @NixOS/nixos-release-managers

Sorry, something went wrong.

@Atemu Atemu requested review from a user and eadwu and removed request for a user September 9, 2020 20:51
@eadwu
Copy link
Member

eadwu commented Sep 9, 2020

I don't use bcachefs anymore (or at least for now). Broke my filesystem too many times.

@davidak
Copy link
Member

davidak commented Sep 10, 2020

@eadwu thanks for the feedback. have you reported the issues upstream? they claim it never broke
have you used some crazy setup with many devices or just one partition?

@Atemu
bcachefs-tools: disable tests
90f01b1 
Tests were failing because they require the bcachefs module to be loaded on the builder

Many tests were skipped anyways and Bcachefs is still tested via its nixos test
@eadwu
Copy link
Member

eadwu commented Sep 10, 2020

It was mainly a problem on following upstream too closely since there isn't really any set tags for a "stable" release. I don't think it was really a filesystem issue, but the integration of bcachefs and bcachefs-tools were often not in sync (or at least not when I corrupted some node/journal). And it wasn't possible to downgrade because the tree version was upgraded and thus the previous version was incompatible. The last issue I was dealing with was due to some deadlock under a high cpu load which looks like was fixed some time ago but I stopped using it around that time since I kept crashing to the point where I couldn't compile a kernel.

@Atemu
bcachefs-tools: 2020-04-04 -> 2020-08-25
920441f
@Atemu
linux_testing_bcachefs: 5.3 -> 5.8
3cc3e86 
versioning scheme: major.minor.YYYY.MM.DD -> major.minor.patch-YYYY.MM.DD

Makes a lot more sense imho
@Atemu
Copy link
Member Author

Atemu commented Sep 10, 2020 •
edited
Loading

Eval without aliases should work now.

@ofborg ofborg bot added 8.has: clean-up 8.has: package (new) This PR adds a new package labels Sep 10, 2020
@ofborg ofborg bot requested a review from davidak September 10, 2020 17:59
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 labels Sep 10, 2020
@jonringer
Copy link
Contributor

there should probably be an assertion that the tooling version is compatible.

Whats the best way to implement this? I'm not sure.

@Atemu
Copy link
Member Author

Atemu commented Sep 11, 2020

I don't think there is a way yet. Bcachefs is a very unstable thing still and there is not a single release, much less a stable one. The only known compatible versions are whatever is the newest.

We do have a nixos test you can use to do integration tests, I think expanding that to cover most of bcachefs' features would be the best we can do for now. A topic for a different PR though IMO.

@vcunat vcunat merged commit 638c8cb into NixOS:master Sep 17, 2020
@vcunat
Copy link
Member

vcunat commented Sep 17, 2020 •
edited
Loading

The build wasn't consistently broken but flaky: https://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.bcachefs-tools.x86_64-linux For example, it succeeded for me locally just before merging even though the same failed on Hydra (same for current 20.09), so it seems unclear whether this PR addresses the flakiness.

@vcunat
Copy link
Member

vcunat commented Sep 17, 2020

I don't have bcachefs on that system loaded (lsmod), just bcache 🤔

@Atemu Atemu deleted the update/bcachefs branch September 17, 2020 21:43
@jnsaff
Copy link
Contributor

jnsaff commented Sep 22, 2020

This update breaks my machine, right at mounting at boot. I will try to see whether I can fix it or get more info. Stay tuned.

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

@eadwu eadwu

@davidak davidak

Assignees
No one assigned
Labels
8.has: clean-up 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Atemu @eadwu @davidak @jonringer @vcunat @jnsaff