Skip to content
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

ceph: 9.2.0 -> 12.2.2 #33894

Merged
merged 1 commit into from Feb 21, 2018
Merged

ceph: 9.2.0 -> 12.2.2 #33894

merged 1 commit into from Feb 21, 2018

Conversation

adevress
Copy link
Contributor

@adevress adevress commented Jan 15, 2018 •
edited

  • Update Ceph to last version
  • Solve security vulnerability of the old version
  • Provide shared version of snappy, required for Ceph
Motivation for this change
  • Make Ceph usable again
  • Provide shared version for snappy
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@andir
Copy link
Member

andir commented Jan 15, 2018

Thank you for working on this. You must also update the sha256 of ceph otherwise you will build the old version with a new name ;)

Mic92
Mic92 reviewed Jan 15, 2018
View reviewed changes
pkgs/development/libraries/snappy/default.nix Outdated
@@ -1,4 +1,4 @@
{ stdenv, fetchFromGitHub, cmake }:
{ stdenv, fetchFromGitHub, cmake, sharedLibrary ? false }:
Copy link
Member

@Mic92 Mic92 Jan 15, 2018 •
edited

Choose a reason for hiding this comment

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

I would rather build shared libraries by default and remove this option. We prefer shared libraries in nixpkgs.

Copy link
Member

Choose a reason for hiding this comment

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

This variable does not have an effect on this derivation, was this added by mistake?

@adevress
Copy link
Contributor Author

Hi @Mic92,

Ok this patch comes from an old nixpkgs branch we have in our side (BBP).

I just realized it diverged way more I expected, and that the recommended way to build Ceph is cmake nowadays.

This would imply way more modifications, let me have a look to that and I come back to you.

@adevress adevress force-pushed the master branch 2 times, most recently from f05258d to 50b95db Compare February 6, 2018 14:10
@adevress adevress requested a review from peti as a code owner February 6, 2018 14:10
@adevress
Copy link
Contributor Author

adevress commented Feb 6, 2018

ping @vcunat @Mic92

Ok guys, problems fixed.

  • The build reciep for Ceph has been entirely switched to CMake. It greatly simplified the build process.
  • External dependencies to boost and RocksDB are enforced.
  • This pull request requires rocksdb: 5.1.2 to 5.10.2 #34662 and lttng-ust: refactor #34661 to be merged
  • Ceph is now built with RocksDB as default backend
  • Ceph now has its own python environment, avoiding ugly hack to get its python tool / doc generation working.
  • Every output is separated correctly

@peti peti removed their request for review February 6, 2018 16:09
@adevress
ceph: 9.2.0 -> 12.2.2
6beb4da 
- Update Ceph to last version
- Solve vulernatibility problem in the old version
- Patch rocksdb ceph rocksdb API
- Simplify a lot ceph build by switching to CMake

This Pull request require NixOS#34662 and NixOS#34661 to be effective
@vcunat
Copy link
Member

vcunat commented Feb 6, 2018

/cc maintainer @alexanderkjeldaas. BTW, it's good to keep lib.maintainers.* aligned with github user names.

@srhb
Copy link
Contributor

srhb commented Feb 7, 2018 •
edited

@lejonet Might be interesting to test this as the base for your ceph module :)

@lejonet
Copy link
Contributor

lejonet commented Feb 12, 2018

@srhb Yeah, I will do that, seems like @adevress did a thorough work through of the build, I only fixed what I needed to get it to compile at all, to develop the ceph module.

Will fork and apply my module ontop and try it out

@lejonet
Copy link
Contributor

lejonet commented Feb 13, 2018 •
edited

@adevress I've tried your branch on a nixos system, with sandbox enabled and all binaries compile and run fine
I just had to make a tiny patch for the ceph source code and your package that you can see here: lejonet@a782a50

Whoever thought it was a good idea with absolute paths in includes, should be shot...

@joachifm
Copy link
Contributor

@GrahamcOfBorg build ceph bareos

@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Partial log (click to expand)

Package ‘lttng-ust-2.10.0’ in /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/lnl7-mac/pkgs/development/tools/misc/lttng-ust/default.nix:32 is not supported on ‘x86_64-darwin’, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Partial log (click to expand)

remote: Total 257 (delta 43), reused 82 (delta 10), pack-reused 0
Receiving objects: 100% (257/257), 697.92 KiB | 11.26 MiB/s, done.
Resolving deltas: 100% (43/43), done.
From https://github.com/facebook/zstd
 * branch            refs/pull/484/head -> FETCH_HEAD
Switched to a new branch 'fetchgit'
removing `.git'...
cannot build derivation '/nix/store/7a9c0pkgx7sxrfnbjlpkrw0q28ry8d9z-ceph-12.2.2.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/4iyilmysdm0wjy7p1d54bzflg9jql709-bareos-15.2.4.drv': 2 dependencies couldn't be built
error: build of '/nix/store/4iyilmysdm0wjy7p1d54bzflg9jql709-bareos-15.2.4.drv', '/nix/store/7a9c0pkgx7sxrfnbjlpkrw0q28ry8d9z-ceph-12.2.2.drv' failed

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Partial log (click to expand)

[ 25%] Building CXX object src/mds/CMakeFiles/mds.dir/Capability.cc.o
[ 25%] Building CXX object src/mds/CMakeFiles/mds.dir/MDSDaemon.cc.o
[ 25%] Building CXX object src/mds/CMakeFiles/mds.dir/MDSRank.cc.o
[ 25%] Building CXX object src/mds/CMakeFiles/mds.dir/Beacon.cc.o
[ 25%] Building CXX object src/mds/CMakeFiles/mds.dir/flock.cc.o
[ 25%] Building C object src/mds/CMakeFiles/mds.dir/locks.c.o
[ 25%] Building CXX object src/mds/CMakeFiles/mds.dir/journal.cc.o
building of ‘/nix/store/hyjyagxn8y7wwjgk20h1hsx4agh1v00b-ceph-12.2.2.drv’ timed out after 3600 seconds
cannot build derivation ‘/nix/store/78a2lv0pqwkx1gk2bz5z5ypak4jn50lv-bareos-15.2.4.drv’: 1 dependencies couldn't be built
error: build of ‘/nix/store/78a2lv0pqwkx1gk2bz5z5ypak4jn50lv-bareos-15.2.4.drv’, ‘/nix/store/hyjyagxn8y7wwjgk20h1hsx4agh1v00b-ceph-12.2.2.drv’ failed

@Mic92 Mic92 merged commit f0466c4 into NixOS:master Feb 21, 2018
@Mic92
Copy link
Member

Mic92 commented Feb 21, 2018

Thanks!

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

@Mic92 Mic92 Mic92 left review comments

Assignees
No one assigned
Labels
1.severity: security 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@adevress @andir @vcunat @srhb @lejonet @joachifm @GrahamcOfBorg @Mic92