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.

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

rocksdb: Build with MinGW #73316

Merged
merged 8 commits into from Nov 23, 2019
Merged

rocksdb: Build with MinGW #73316

merged 8 commits into from Nov 23, 2019

Conversation

Ericson2314
Copy link
Member

Motivation for this change

@angerman nerd sniped me. Got all the debs building, now just need to make rocksdb itself work without localtime_r.

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 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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@Ericson2314
Copy link
Member Author

;

doCheck = false; # tests take a very long time
checkTarget = "test";

# TODO remove
postInstall = stdenv.lib.optionalString (!enableStatic) "rm $out/lib/*.a";
# TODO(@Ericson2314): Make resusable setup hook for this issue on Windows.
Copy link
Member

Choose a reason for hiding this comment

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

It looks like lz4 intentionally puts it in bindir. Perhaps the setup hook should instead add $out/bin to the -L lookups:

https://github.com/lz4/lz4/blob/49c20c498b80e70bae3ae604ef468921d79d2121/lib/Makefile#L182-L187

Copy link
Contributor

Choose a reason for hiding this comment

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

This is something slightly awkward about windows, shared lirbaries (should) end up in bin. There are a few cases in nixpkgs as well I believe, where we specifically place shared libraries into bin for windows builds. That way (because the bin folders would end up on PATH, it usually just works on windows.

I keep traversing all dependencies, and collect .dlls from lib and bin and place them alongside the final executable, as that seems to be the most reliable way to build some form of distributable for windows.

Copy link
Member Author

Choose a reason for hiding this comment

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

So the way I think of it is that the libraries primarily should live in lib, so that you don't pick up extra exe dependencies in the wrong multiple outputs, but each exe needs it's dlls symlinked in its bin directory since there is no RUNPATH or RPATH by which it could find them.

lib is still the cannonical location, but the bin symlinks ensure the exes run.

Copy link
Member Author

Choose a reason for hiding this comment

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

Err, why use pre-compiled anything? We are entirely building from source here (except for linux native boostrapping tools).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think my thing with the TODO is fine for now.

@Ericson2314
Copy link
Member Author

OK this now builds RocksDB, but references a PR commit that I'll probably have to rebase away because facebook/rocksdb#6027 doesn't pass their CI. Should I just vendor the patch then?

mb and others added 4 commits November 23, 2019 01:25
THREAD_MSG := ==> building with threading support
THREAD_CPP := -DZSTD_MULTITHREAD
-THREAD_LD := -pthread
+THREAD_LD :=
Copy link
Member Author

Choose a reason for hiding this comment

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

@Ericson2314 Ericson2314 changed the title WIP: rocksdb: Build with MinGW rocksdb: Build with MinGW Nov 23, 2019
@Ericson2314 Ericson2314 merged commit e241c14 into NixOS:master Nov 23, 2019
@Ericson2314 Ericson2314 deleted the mingw-rocksdb branch November 23, 2019 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants