Skip to content

rocksdb: 5.11.3 -> 6.1.2#58834

Merged
Lassulus merged 2 commits intoNixOS:masterfrom
magenbluten:master
Jun 19, 2019
Merged

rocksdb: 5.11.3 -> 6.1.2#58834
Lassulus merged 2 commits intoNixOS:masterfrom
magenbluten:master

Conversation

@magenbluten
Copy link
Copy Markdown
Contributor

@magenbluten magenbluten commented Apr 2, 2019

Motivation for this change

rocksdb was out of date. also removed stuff that seemed broken.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg GrahamcOfBorg added 8.has: clean-up This PR removes packages or removes other cruft 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Apr 2, 2019
@ryantm
Copy link
Copy Markdown
Member

ryantm commented Apr 4, 2019

cc @adevress

ryantm
ryantm requested changes Apr 4, 2019
View reviewed changes
Comment thread pkgs/development/libraries/rocksdb/default.nix Outdated
Comment thread pkgs/development/libraries/rocksdb/default.nix Outdated
Comment thread pkgs/development/libraries/rocksdb/default.nix Outdated
Comment thread pkgs/development/libraries/rocksdb/default.nix Outdated
@ryantm
Copy link
Copy Markdown
Member

ryantm commented Apr 4, 2019

@GrahamcOfBorg build rocksdb

@infinisil infinisil added the 8.has: package (update) This PR updates a package to a newer version label Apr 4, 2019
ryantm
ryantm requested changes Apr 5, 2019
View reviewed changes
Comment thread lib/licenses.nix Outdated
@magenbluten magenbluten changed the title rocksdb: 5.11.3 -> 5.18.3 rocksdb: 5.11.3 -> 6.0.1 Apr 17, 2019
Mic92
Mic92 reviewed Apr 17, 2019
View reviewed changes
Comment thread lib/licenses.nix Outdated
Mic92
Mic92 approved these changes Apr 17, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

Looks good to me otherwise.

Mic92
Mic92 reviewed Apr 17, 2019
View reviewed changes
Comment thread pkgs/development/libraries/rocksdb/default.nix Outdated
Copy link
Copy Markdown
Member

@Mic92 Mic92 Apr 17, 2019

Choose a reason for hiding this comment

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

Could this break somewhere?

@ofborg ofborg Bot requested review from Ma27 and cstrahan April 29, 2019 06:59
Ma27
Ma27 reviewed Apr 29, 2019
View reviewed changes
Comment thread pkgs/tools/system/osquery/default.nix Outdated
@magenbluten magenbluten changed the title rocksdb: 5.11.3 -> 6.0.1 rocksdb: 5.11.3 -> 6.0.2 Apr 30, 2019
Lassulus
Lassulus reviewed Apr 30, 2019
View reviewed changes
Comment thread pkgs/top-level/all-packages.nix Outdated
@Lassulus
Copy link
Copy Markdown
Member

Lassulus commented May 7, 2019

@GrahamcOfBorg build ceph

@Lassulus
Copy link
Copy Markdown
Member

Lassulus commented May 7, 2019

this also breaks ceph. and everything that depends on it. But ceph seems to be an old version? @adevress @alexanderkjeldaas

@Lassulus
Copy link
Copy Markdown
Member

Lassulus commented May 21, 2019

since no one responded for the ceph breakage, we marked ceph as broken.

@ofborg ofborg Bot requested a review from Ma27 May 21, 2019 21:34
flokli
flokli reviewed May 22, 2019
View reviewed changes
Comment thread pkgs/development/libraries/rocksdb/default.nix Outdated
flokli
flokli reviewed May 22, 2019
View reviewed changes
Comment thread pkgs/development/libraries/rocksdb/default.nix Outdated
flokli
flokli reviewed May 22, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@flokli flokli left a comment

Choose a reason for hiding this comment

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

some nitpicks.

@magenbluten magenbluten changed the title rocksdb: 5.11.3 -> 6.0.2 rocksdb: 5.11.3 -> 6.1.1 Jun 5, 2019
@magenbluten magenbluten changed the title rocksdb: 5.11.3 -> 6.1.1 rocksdb: 5.11.3 -> 6.1.2 Jun 18, 2019
@magenbluten
Copy link
Copy Markdown
Contributor Author

magenbluten commented Jun 18, 2019 •
edited
Loading

i tried to unbreak osquery with no luck. it still fails when linking against rocksdb(-lite). see https://gist.github.com/magenbluten/997142eaa828ccd177c0dd06cc8261b4

@Lassulus
Copy link
Copy Markdown
Member

Lassulus commented Jun 18, 2019

@Ma27 any points against merging this? since you recently updated osquery and this PR would break it.

@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Jun 18, 2019

No. While working on osquery quite recently I realized that they currently have a hard time staying up-to-date with new software versions and I don't consider osquery important enough to actually block this PR any longer.

Therefore, it's IMHO ok to merge this now, I'll take care of osquery on one of the next weekends :)

rocksdb: 5.11.3 -> 6.1.2
e369007 
- mark osquery as broken
- mark ceph as broken

both osquery and ceph packages are outdated. furthermore, ceph has its
own inline rocksdb source tree which isn't use in the current nixpkg.
this needs to be fixed.
@Lassulus Lassulus merged commit b54b5f9 into NixOS:master Jun 19, 2019
@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Jun 29, 2019

@magenbluten @Lassulus I managed to get osquery building locally (didn't test ceph as a bunch of python2 deps appear to be broken on master atm) and it works perfectly fine when using the old build system (no cmake). I suspect this is because the cmake build doesn't create statically linked libraries.

According to the install section it's recommended to simply use make {shared,static}_lib depending on what's needed.

I'd simply go back to the old build approach for rocksdb (and keep 6.1.2), however I'm not sure if I'm missing a reason why cmake is used now (and I probably just didn't manage to get the static lib building with cmake ^^).

Do you think using the old make-based approach makes sense or am I missing something? :)

@magenbluten
Copy link
Copy Markdown
Contributor Author

magenbluten commented Jun 30, 2019

i guess the install section you mention is outdated (dez. 2018).

the "correct" solution would be to figure out how to make cmake make static libs ;)

@magenbluten
Copy link
Copy Markdown
Contributor Author

magenbluten commented Jun 30, 2019 •
edited
Loading

git grep -i static_lib:

CMakeLists.txt:set(ROCKSDB_STATIC_LIB rocksdb${ARTIFACT_SUFFIX})

seems like it should work with cmake.

@infinisil infinisil mentioned this pull request Nov 24, 2019
Merged
10 tasks
@Janik-Haag Janik-Haag added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mic92 Mic92 Mic92 approved these changes

@flokli flokli flokli left review comments

@Lassulus Lassulus Lassulus left review comments

@ryantm ryantm ryantm requested changes

@cstrahan cstrahan Awaiting requested review from cstrahan

@Ma27 Ma27 Awaiting requested review from Ma27

Assignees

No one assigned

Labels

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: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@magenbluten @ryantm @Lassulus @Ma27 @Mic92 @flokli @infinisil @GrahamcOfBorg @Janik-Haag