brotli: ensure CMAKE_SYSTEM_NAME=Windows on mingw cross builds #46773
Conversation
CMAKE passes the flag -rdynamic otherwise (which is not recognized by mingw). Setting the appropriate flag avoids this problem and allows the build to succeed.
|
For reference, here's the shell.nix file I used for testing: with import <nixpkgs> {};
{
my-env = pkgs.stdenv.mkDerivation {
name = "my-env";
buildInputs = [
pkgs.pkgsCross.mingwW64.brotli
];
};
} |
|
Thanks! I'm glad someone is also working with Mingw. I think we can actually set this up for all CMake builds. Reading the documentation, it looks like by default CMAKE_SYSTEM_NAME is set to CMAKE_HOST_SYSTEM_NAME (which awkwardly is a different kind of "host" compared to the one in Nixpkgs): https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_NAME.html The only thing is keeping track of nixpkgs/pkgs/development/tools/build-managers/cmake/setup-hook.sh Lines 32 to 33 in 92a047a |
|
@GrahamcOfBorg build pkgs.pkgsCross.mingwW64.brotli |
|
Success on x86_64-linux (full log) Attempted: pkgs.pkgsCross.mingwW64.brotli Partial log (click to expand)
|
|
Yeah |
|
Success on aarch64-linux (full log) Attempted: pkgs.pkgsCross.mingwW64.brotli Partial log (click to expand)
|
|
Success on x86_64-darwin (full log) Attempted: pkgs.pkgsCross.mingwW64.brotli Partial log (click to expand)
|
|
Many thanks for the review @matthewbauer @Ericson2314 - I agree it would be ideal if this were to be fixed for all packages at once via the cmake setup hook. I would be happy to try to contribute a patch to that effect. However, I'm not that experienced with setup hooks, so please forgive me for I cannot see immediately how to easily detect within a setup hook that we are running a mingw build (or more generally, that we are running a cross build and what the target then is). I've grep'd all current setup hooks in the repository for inspiration, to no avail. Suggestions? |
There was a problem hiding this comment.
Looks okay to merge right now (not sure if it should be master or staging though). Discussing with @Ericson2314 how we can do this everywhere - it gets a little complicated because now we need some kind of a wrapper to hold what the targeted system is, previously we reused the same thing every time. Anyway, it's used in other places already so should be okay.
|
Maybe I would add this version to 18.09, and do the wrapper on maste/staging. |
|
I am still wanting to explore ways to avoid the wrapping of cmake. The setup hook should have enough information to get the target from the c compiler without a wrapper. Somehow parsing dumpmachine output? Or some other variable |
|
I just don't see how computing the same information every time dynamically is superior to doing it once statically. The wrapper would be a lot simpler than cc-wrapper. It can just be the setup hook and a propagated dependency, in fact. |
|
I agree. Merge this now and generalize after. |
|
It's possible to create the setup hook once, based on the platform, but I don't see a simple nice way immediately. |
Motivation for this change
CMAKE passes the flag -rdynamic otherwise (which is not recognized by mingw). Setting the appropriate flag avoids this problem and allows the build to succeed.
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)I've also tested the resulting executable using wine on fedora (compressing & decompressing a file, checked the difference).
@vcunat @Ericson2314