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

itstool: fix cross #341556

Closed
wants to merge 5 commits into from
Closed

itstool: fix cross #341556

wants to merge 5 commits into from

Conversation

SFrijters
Copy link
Member

Description of changes

Fixes configure: error: Python module libxml2 is needed to run this package, for e.g. nix build .#pkgsCross.aarch64-multiplatform.itstool.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@SFrijters SFrijters requested a review from jtojnar September 13, 2024 10:37
@ofborg ofborg bot added 6.topic: cross-compilation Building packages on a different platform than they will be used on 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Sep 13, 2024
@SFrijters SFrijters changed the base branch from master to staging September 13, 2024 19:46
@SFrijters
Copy link
Member Author

Retargeted to staging because of the large number of rebuilds.

@SFrijters
Copy link
Member Author

Running the native code still works, and running aarch64 via binfmt works as well.

The program spits out a bunch of warnings - we could consider pulling in itstool/itstool#51 to fix that.

Copy link
Contributor

@bjornfor bjornfor left a comment

Choose a reason for hiding this comment

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

I'd like to see

Fixes configure: error: Python module libxml2 is needed to run this package, for e.g. nix build .#pkgsCross.aarch64-multiplatform.itstool.

in the commit message as well, because that's what goes into git. But it's not a blocker.

Fixes configure step `error: Python module libxml2 is needed to run this package`,
for e.g. nix build .#pkgsCross.aarch64-multiplatform.itstool.
@SFrijters
Copy link
Member Author

SFrijters commented Sep 14, 2024

Amended the commit message, thanks for the review.

Copy link
Contributor

@bjornfor bjornfor left a comment

Choose a reason for hiding this comment

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

Thanks!

@ofborg ofborg bot requested a review from bjornfor September 14, 2024 10:04
@wegank wegank added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Sep 14, 2024
@Artturin
Copy link
Member

Artturin commented Sep 14, 2024

Output depends on the build python

$ nix path-info -r ./result
/nix/store/lw2macnzp3av47m41qv6vcnq2daibgl1-libunistring-1.2
/nix/store/8hq38sx1hjgganlj6dn0fvvba0g7gcpi-libidn2-2.3.7
/nix/store/x45xiwvk2v97bw1yp1vb6rnmvhagpgyz-xgcc-13.3.0-libgcc
/nix/store/3dyw8dzj9ab4m8hv5dpyx7zii8d0w6fi-glibc-2.39-52
/nix/store/1w90l4fm5lzhlybipfilyjij2das6w98-openssl-3.0.14
/nix/store/q0iz2x35ki1aaqpjagjfi8s7q053cmc5-gcc-13.3.0-libgcc
/nix/store/22nxhmsfcv2q2rpkmfvzwg2w5z1l231z-gcc-13.3.0-lib
/nix/store/dvjmqzjcf4y93x4hfxghfrv71rzbh71i-find-xml-catalogs-hook
/nix/store/72ra208yzk1sisi0f9kgav4k4686r2p5-libgcc-aarch64-unknown-linux-gnu-13.3.0
/nix/store/qa51m8r8rjnigk5hf7sxv0hw7qr7l4bc-glibc-aarch64-unknown-linux-gnu-2.39-52
/nix/store/wljxp6bag5nfffy8rv2j46c7x2a86896-libxml2-aarch64-unknown-linux-gnu-2.13.3
/nix/store/xkjsjqp9wi1fss88zlvky0kl3x2bg170-libxml2-aarch64-unknown-linux-gnu-2.13.3-bin
/nix/store/2wzz79ffjy4virjsmzzam4ppfdbwpail-libxml2-aarch64-unknown-linux-gnu-2.13.3-dev
/nix/store/3hxxbbjc8r66nravvjind6ixhz7cpij1-mpdecimal-4.0.0
/nix/store/3zrkasqf3sqr9ff6sv5fddhlbf072a36-mailcap-2.1.54
/nix/store/4w3c2c11j2kmwxrznji2i7is53z5aldi-xz-5.6.2
/nix/store/4ybgqxv43z9sk9lccwq6dgmz6j32syr1-libffi-3.4.6
/nix/store/897xqnq52vw76991r5m80h9j91370vj9-tzdata-2024a
/nix/store/8ad0k7rbn3rr9qiya29464g52asndxqi-gdbm-1.24-lib
/nix/store/f0dmadrh535bpayz2r4q5lslg0m17lnw-expat-2.6.2
/nix/store/fvjy3a1xmykqqx3irhxnni9wb3ngxc16-libxml2-aarch64-unknown-linux-gnu-2.13.3-py
/nix/store/rqs1zrcncqz3966khjndg1183cpdnqxs-zlib-1.3.1
/nix/store/g2i5j8i0bwfih7hg2wbc15k2969bckn3-sqlite-3.46.0
/nix/store/izpf49b74i15pcr9708s3xdwyqs4jxwl-bash-5.2p32
/nix/store/lhfgwjwwdacygiq702yvbahvpqmxivpx-libxcrypt-4.4.36
/nix/store/qhf8n3srrg0vdd44k7q71fb0p5az15b5-bzip2-1.0.8
/nix/store/z7nr6aqlzv51pk5ar8bgzg2alfqvi8fd-ncurses-6.4.20221231
/nix/store/xxcf5gwyn5pldv4b4wa2jw6vqg7v55y6-readline-8.2p10
/nix/store/h3i0acpmr8mrjx07519xxmidv8mpax4y-python3-3.12.5
/nix/store/h1ip4jlifkwfvcdi3vz5k9vgcsj9n86c-itstool-aarch64-unknown-linux-gnu-2.0.7
$ nix why-depends ./result "/nix/store/h3i0acpmr8mrjx07519xxmidv8mpax4y-python3-3.12.5" --all --precise
/nix/store/h1ip4jlifkwfvcdi3vz5k9vgcsj9n86c-itstool-aarch64-unknown-linux-gnu-2.0.7
└───bin/.itstool-wrapped: …#!/nix/store/h3i0acpmr8mrjx07519xxmidv8mpax4y-python3-3.12.5/bin/python -s.#.…
    bin/itstool: …':'}.PATH=${PATH/':''/nix/store/h3i0acpmr8mrjx07519xxmidv8mpax4y-python3-3.12.5/bin'':'/':'}.PAT…
    → /nix/store/h3i0acpmr8mrjx07519xxmidv8mpax4y-python3-3.12.5

In addition to the build script checking the python wrong there's also a bug in wrapPython related to the python splicing which #228139 will fix one day

Simplest fix would be to replace the build python references because the other paths are correct.

@SFrijters
Copy link
Member Author

Sorry, I don't understand - how can I tell that that is the build python (and not the host python) when not cross-compilling? If I build it for aarch64 the script references the aarch64 Python, which is not the build Python, but the host (?) Python?

$ nix build .#pkgsCross.aarch64-multiplatform.itstool -L
$ nix path-info -r ./result
/nix/store/0j9r1vfz5izd8d1kn7gfjlwsnk1xlyad-tzdata-aarch64-unknown-linux-gnu-2024a
/nix/store/72ra208yzk1sisi0f9kgav4k4686r2p5-libgcc-aarch64-unknown-linux-gnu-13.3.0
/nix/store/qa51m8r8rjnigk5hf7sxv0hw7qr7l4bc-glibc-aarch64-unknown-linux-gnu-2.39-52
/nix/store/2rdds31371m6x27dvdrm8pc3hmrg25i6-bash-aarch64-unknown-linux-gnu-5.2p32
/nix/store/dvjmqzjcf4y93x4hfxghfrv71rzbh71i-find-xml-catalogs-hook
/nix/store/wljxp6bag5nfffy8rv2j46c7x2a86896-libxml2-aarch64-unknown-linux-gnu-2.13.3
/nix/store/xkjsjqp9wi1fss88zlvky0kl3x2bg170-libxml2-aarch64-unknown-linux-gnu-2.13.3-bin
/nix/store/2wzz79ffjy4virjsmzzam4ppfdbwpail-libxml2-aarch64-unknown-linux-gnu-2.13.3-dev
/nix/store/5nfb5ir6bjpfi0g3pbas1lz3m0ln4hdr-libxcrypt-aarch64-unknown-linux-gnu-4.4.36
/nix/store/7r4sjncjs4m7ppbb7zk2s8ygrpfwm828-ncurses-aarch64-unknown-linux-gnu-6.4.20221231
/nix/store/7sxq0qszphhcwfrd5qa178qfqp0xr959-gdbm-aarch64-unknown-linux-gnu-1.24-lib
/nix/store/ak1s71zikj63ly0b580r4kvms5av3hm0-aarch64-unknown-linux-gnu-gcc-13.3.0-lib
/nix/store/fvjy3a1xmykqqx3irhxnni9wb3ngxc16-libxml2-aarch64-unknown-linux-gnu-2.13.3-py
/nix/store/h24ricq8f5qv7qlafjyhcmrkm2lqnc1b-mpdecimal-aarch64-unknown-linux-gnu-4.0.0
/nix/store/hy4ivd204zf9f9gh29vkpyw25qia375c-expat-aarch64-unknown-linux-gnu-2.6.2
/nix/store/icklqsfn341r0mgb45gqpkdpwxpkcgf8-zlib-aarch64-unknown-linux-gnu-1.3.1
/nix/store/jns7ynilqnibykg7465hc8ppzwlrz1bs-libffi-aarch64-unknown-linux-gnu-3.4.6
/nix/store/mpn223f5bljipm7ry0zlzvgnf3lkx726-xz-aarch64-unknown-linux-gnu-5.6.2
/nix/store/n1dyczka2rhhiadlaygrcv80sqwk36f2-mailcap-aarch64-unknown-linux-gnu-2.1.54
/nix/store/sgzcv86wwc0z46z8p1g0dchsd9npxzdp-bzip2-aarch64-unknown-linux-gnu-1.0.8
/nix/store/xf2i6hcgvnl2gv3cdyiv9w6ccj5z4608-sqlite-aarch64-unknown-linux-gnu-3.46.0
/nix/store/yxvg4kanwz0q9gdxh6y107gzyxc15shd-readline-aarch64-unknown-linux-gnu-8.2p10
/nix/store/zy5hqwkbiicpxynqsgxmgi68vfqf0axc-openssl-aarch64-unknown-linux-gnu-3.0.14
/nix/store/vc4xlrlswjpwfjcd1f018ykpdd3rla8i-python3-aarch64-unknown-linux-gnu-3.12.5   <--
/nix/store/xyz3fwvc0gn2p5s3a4b43lmqlm7vsprm-itstool-aarch64-unknown-linux-gnu-2.0.7

As far as I can tell the final package is a Python script so we need an interpreter, no?

Comment on lines 5 to 6
buildPackages,
python3,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
buildPackages,
python3,
python3Packages,

python3Packages is spliced so use it instead

@Artturin
Copy link
Member

Artturin commented Sep 15, 2024

Sorry, I don't understand - how can I tell that that is the build python (and not the host python) when not cross-compilling? If I build it for aarch64 the script references the aarch64 Python, which is not the build Python, but the host (?) Python?

You can't since they'll be identical when build == host == target

$ nix build .#pkgsCross.aarch64-multiplatform.itstool -L
$ nix path-info -r ./result
/nix/store/0j9r1vfz5izd8d1kn7gfjlwsnk1xlyad-tzdata-aarch64-unknown-linux-gnu-2024a
/nix/store/72ra208yzk1sisi0f9kgav4k4686r2p5-libgcc-aarch64-unknown-linux-gnu-13.3.0
/nix/store/qa51m8r8rjnigk5hf7sxv0hw7qr7l4bc-glibc-aarch64-unknown-linux-gnu-2.39-52
/nix/store/2rdds31371m6x27dvdrm8pc3hmrg25i6-bash-aarch64-unknown-linux-gnu-5.2p32
/nix/store/dvjmqzjcf4y93x4hfxghfrv71rzbh71i-find-xml-catalogs-hook
/nix/store/wljxp6bag5nfffy8rv2j46c7x2a86896-libxml2-aarch64-unknown-linux-gnu-2.13.3
/nix/store/xkjsjqp9wi1fss88zlvky0kl3x2bg170-libxml2-aarch64-unknown-linux-gnu-2.13.3-bin
/nix/store/2wzz79ffjy4virjsmzzam4ppfdbwpail-libxml2-aarch64-unknown-linux-gnu-2.13.3-dev
/nix/store/5nfb5ir6bjpfi0g3pbas1lz3m0ln4hdr-libxcrypt-aarch64-unknown-linux-gnu-4.4.36
/nix/store/7r4sjncjs4m7ppbb7zk2s8ygrpfwm828-ncurses-aarch64-unknown-linux-gnu-6.4.20221231
/nix/store/7sxq0qszphhcwfrd5qa178qfqp0xr959-gdbm-aarch64-unknown-linux-gnu-1.24-lib
/nix/store/ak1s71zikj63ly0b580r4kvms5av3hm0-aarch64-unknown-linux-gnu-gcc-13.3.0-lib
/nix/store/fvjy3a1xmykqqx3irhxnni9wb3ngxc16-libxml2-aarch64-unknown-linux-gnu-2.13.3-py
/nix/store/h24ricq8f5qv7qlafjyhcmrkm2lqnc1b-mpdecimal-aarch64-unknown-linux-gnu-4.0.0
/nix/store/hy4ivd204zf9f9gh29vkpyw25qia375c-expat-aarch64-unknown-linux-gnu-2.6.2
/nix/store/icklqsfn341r0mgb45gqpkdpwxpkcgf8-zlib-aarch64-unknown-linux-gnu-1.3.1
/nix/store/jns7ynilqnibykg7465hc8ppzwlrz1bs-libffi-aarch64-unknown-linux-gnu-3.4.6
/nix/store/mpn223f5bljipm7ry0zlzvgnf3lkx726-xz-aarch64-unknown-linux-gnu-5.6.2
/nix/store/n1dyczka2rhhiadlaygrcv80sqwk36f2-mailcap-aarch64-unknown-linux-gnu-2.1.54
/nix/store/sgzcv86wwc0z46z8p1g0dchsd9npxzdp-bzip2-aarch64-unknown-linux-gnu-1.0.8
/nix/store/xf2i6hcgvnl2gv3cdyiv9w6ccj5z4608-sqlite-aarch64-unknown-linux-gnu-3.46.0
/nix/store/yxvg4kanwz0q9gdxh6y107gzyxc15shd-readline-aarch64-unknown-linux-gnu-8.2p10
/nix/store/zy5hqwkbiicpxynqsgxmgi68vfqf0axc-openssl-aarch64-unknown-linux-gnu-3.0.14
/nix/store/vc4xlrlswjpwfjcd1f018ykpdd3rla8i-python3-aarch64-unknown-linux-gnu-3.12.5   <--
/nix/store/xyz3fwvc0gn2p5s3a4b43lmqlm7vsprm-itstool-aarch64-unknown-linux-gnu-2.0.7

As far as I can tell the final package is a Python script so we need an interpreter, no?

I applied the changes in the first commit to master and what's in my previous comment is what I see, do you have binfmt enabled? because that can hide cross issues so it shouldn't be used when working on nixpkgs.

@SFrijters
Copy link
Member Author

I thought I had disabled binfmt in my config for just this reason, but it may have required a reboot to apply?
When I build itstool now I do get your list of paths, including the build python :(.

@SFrijters SFrijters marked this pull request as draft September 15, 2024 22:03
Comment on lines 22 to 23
buildPackages.python3
buildPackages.python3Packages.libxml2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
buildPackages.python3
buildPackages.python3Packages.libxml2
python3Packages.python
python3Packages.libxml2

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 10, 2024
@SFrijters SFrijters closed this Dec 29, 2024
@SFrijters SFrijters deleted the itstool-cross branch December 29, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: cross-compilation Building packages on a different platform than they will be used on 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants