-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
Allow postgresql to cross compile #44083
Allow postgresql to cross compile #44083
Conversation
patches = [ ./shtool.patch ]; | ||
|
||
preConfigure = '' | ||
export ac_cv_va_copy=yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think conventionally in Nixpkgs, this sort of stuff is not exported but added to configureFlags
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats the problem. This configure
script does not allow you to fix the test results on va_copy
. Please advise a better approach, if any.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dezgeg means passing the foo=var
directly to configure, which does work for autoconf-based configure scripts.
@@ -14,8 +14,15 @@ let | |||
outputs = [ "out" "lib" "doc" "man" ]; | |||
setOutputFlags = false; # $out retains configureFlags :-/ | |||
|
|||
combinedLibXML2 = symlinkJoin { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, why is this needed? Why doesn`t
nativeBuildInputs = [ libxml2 ];
work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly, but I just realised that the upstream broke cross-compilation capability, possibly due to #44068 by @Ericson2314. I will close this PR for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dezgeg I tried. However, the configure
of postgresql
expects the libxml2
path to contain both its headers and its static libraries.
We would either patch the configure
, or just supply the combined libxml2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you pass it as a buildInput
, its dev outputs will be passed with the headers, and the -L
to the libs added by cc-wrapper will probably leave the configure script unable to notice the difference. If you make it just a nativeBuildInput you will not get the -L
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ericson2314 I just confirmed that if libxml2
is added to buildInputs
, the static library is visible to the cross compilers. However, the headers are not picked up by the configure
script.
Sorry that I am new to this cross compiling business. Should we modify CFLAGS
to point to the headers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently libxml2
export headers in the include/libxml2
folder in the Nix store, but the CFLAGS
is pointing to include
only. When configure
is resolving #include <libxml/parser.h>
, it is looking at the include
folder only and this fails the configure
checks.
The build works if I point the headers to include/libxml2
. The question now is whether libxml2
Nix expression evaluating correctly.
Cross compilation is broken. Close for now and will come back later. |
Cross compilation is fixed again. |
@@ -22,14 +22,18 @@ let | |||
|
|||
makeFlags = [ "world" ]; | |||
|
|||
preConfigure = '' | |||
export CFLAGS="$CFLAGS -I${libxml2.dev}/include/libxml2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we usually prefer to do this like:
NIX_CFLAGS_COMPILE = "-I${libxml2.dev}/include/libxml2";
Mostly this is because not all build systems recognize CFLAGS.
++ lib.optional (!stdenv.isDarwin) "--with-ossp-uuid"; | ||
++ lib.optional stdenv.isDarwin "--with-uuid=e2fs" | ||
++ lib.optional (!stdenv.isDarwin) "--with-ossp-uuid" | ||
++ lib.optional stdenv.isCross "--with-system-tzdata=${tzdata}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one you can probably do unconditionally.
@@ -22,14 +22,18 @@ let | |||
|
|||
makeFlags = [ "world" ]; | |||
|
|||
preConfigure = '' | |||
export NIX_CFLAGS_COMPILE="$NIX_CFLAGS_COMPILE -I${libxml2.dev}/include/libxml2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does postgresql also picks this up, when pkgconfig
is included in nativeBuildInputs
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I see why this fails:
https://github.com/postgres/postgres/blob/master/configure.in#L896
It needs xml2-config
compiled for the build platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of preConfigure, this can be exported in nix instead:
# needed for cross-compiling, since xml2-config is not compiled for the build platform
NIX_CFLAGS_COMPILE = [ "-I${libxml2.dev}/include/libxml2" ];
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The long-term solution would be pkg-config
: However this would been an upstream fix: libarchive/libarchive#407
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I will try to raise this issue in postgresql
mailing list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually xml2-config
is built correctly but it doesn't get into the PATH
since we don't add binaries from dev
output to PATH
. This is something we should do in stdenv since many other packages will benefit from that as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I do it in a separate pull request? Or this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for now this is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I agree it can be done later.
Let's however conditionalize it on cross, so that someone doesn't break it by noticing it native-compiles fine with it.
@GrahamcOfBorg build pkgsCross.raspberryPi.postgresql |
Success on x86_64-linux (full log) Attempted: pkgsCross.raspberryPi.postgresql Partial log (click to expand)
|
Failure on x86_64-darwin (full log) Attempted: pkgsCross.raspberryPi.postgresql Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: pkgsCross.raspberryPi.postgresql Partial log (click to expand)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
No attempt on aarch64-linux (full log) The following builds were skipped because they don't evaluate on aarch64-linux: pkgsCross.raspberryPi.postgresql Partial log (click to expand)
|
No attempt on x86_64-linux (full log) The following builds were skipped because they don't evaluate on x86_64-linux: pkgsCross.raspberryPi.postgresql Partial log (click to expand)
|
patches = [ ./shtool.patch ]; | ||
|
||
configureFlags = [ | ||
"ac_cv_va_copy=yes" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ericson2314 Configure flags is also above
ccc2357
to
f539c30
Compare
@GrahamcOfBorg build pkgsCross.raspberryPi.postgresql |
retroactively testing oops (I missed that above, but I also figured at least native builds would continue to work). |
Success on x86_64-linux (full log) Attempted: pkgsCross.raspberryPi.postgresql The following builds were skipped because they don't evaluate on x86_64-linux: retroactively... Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: pkgsCross.raspberryPi.postgresql Partial log (click to expand)
|
Cool :) |
Failure on x86_64-darwin (full log) Attempted: pkgsCross.raspberryPi.postgresql The following builds were skipped because they don't evaluate on x86_64-darwin: retroactively... Partial log (click to expand)
|
Failure on x86_64-darwin (full log) Attempted: pkgsCross.raspberryPi.postgresql Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: pkgsCross.raspberryPi.postgresql The following builds were skipped because they don't evaluate on aarch64-linux: retroactively... Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: pkgsCross.raspberryPi.postgresql Partial log (click to expand)
|
@Ericson2314 this was a mass-rebuild and should have gone to staging. |
Sorry @FRidh. I missed that completely. |
This was introduced in NixOS#44083 to fix cross building, where xml2-config wouldn't run on the host platform. This was fixed upstream two years later [1], so that from v13 on pkg-config is used before xml2-config is. Once v12 is EOL, we can remove this entirely. [1]: postgres/postgres@0bc8ceb
Motivation for this change
Currently,
PostgreSQL
does not cross compile due to not listinglibXML2
headers as a native build dependency properly, andlibossp-uuid
as a dependency ofPostgreSQL
does not cross compile because it does not respectstrip
command setting made byconfigure
.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)