Skip to content

libvpx: fix cross-compilation#126912

Merged
SuperSandro2000 merged 1 commit intoNixOS:stagingfrom
Mindavi:libvpx/cross-compilation
Jun 17, 2021
Merged

libvpx: fix cross-compilation#126912
SuperSandro2000 merged 1 commit intoNixOS:stagingfrom
Mindavi:libvpx/cross-compilation

Conversation

@Mindavi
Copy link
Contributor

@Mindavi Mindavi commented Jun 15, 2021

This is done by removing some warnings that are not supported by the
cross-compiler.

We also need to disable the --enable-external-build flag, which breaks
the build completely.
This PR: #100210 added the flag,
but it doesn't explain why it was added. It also doesn't show any
attempt at trying to cross-compile for either darwin or aarch64. So
let's remove it again, since it 'just' seems to break the build anyway.

Motivation for this change

Support cross-compilation for libvpx, which is a dependency of ffmpeg.

I've tested building this with nix build .#pkgsCross.aarch64-multiplatform.libvpx, which works. With #123915 applied, ffmpeg now also cross-compiles for aarch64-multipllatform :).

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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 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.

This reverts a change made in #118783. @TredwellGit do you have any idea why the --enable-external-build flag was added there? It doesn't seem like you attempted a cross-build (looking at the PR at least), but maybe you did?

@Mindavi Mindavi added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Jun 15, 2021
@ofborg ofborg bot requested a review from codyopel June 15, 2021 06:28
@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. labels Jun 15, 2021
@TredwellGit
Copy link
Member

I added it because 1859b5a did. Please remove it along with the other commented out flags.

@Mindavi
Copy link
Contributor Author

Mindavi commented Jun 16, 2021

Thanks for the review. I've updated the PR to remove the commented out flags and to be more specific on which files need patchShebangs. Let me know if there's anything I can improve.

@jonringer
Copy link
Contributor

Please follow CONTRIBUTING.md and manual#submitting-changes-making-patches and squash the fix-up commits.

git rebase -i is a powerful command which achieves this, I created a small video demonstrating it's use here. A more indepth text tutorial can be found here

This is done by removing some warnings that are not supported by the
cross-compiler.

We also need to disable the --enable-external-build flag, which breaks
the build completely.
This PR: NixOS#100210 added the flag,
but it doesn't explain why it was added. It also doesn't show any
attempt at trying to cross-compile for either darwin or aarch64. So
let's remove it again, since it 'just' seems to break the build anyway.

- drop commented out configure flags
- be (more) explicit in patchShebangs
- libvpx_1_8: be explicit about patchShebangs, remove commented flags
@Mindavi Mindavi force-pushed the libvpx/cross-compilation branch from a88265c to 7907718 Compare June 16, 2021 20:32
@Mindavi
Copy link
Contributor Author

Mindavi commented Jun 16, 2021

Squashed the commits together. I thought the commits I made were separate enough to warrant a commit each, but this is ok for me too, so let's do it.

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

diff LGTM

still need to test

@Mindavi
Copy link
Contributor Author

Mindavi commented Jun 17, 2021

Note that libvpx_1_8 still doesn't cross-compile, but only firefox depends on it, so I think it's less of a priority.

That's also the reason why I didn't remove the --enable-external-build flag, it doesn't help anyway.

@SuperSandro2000
Copy link
Member

nix-build -A pkgsCross.aarch64-multiplatform.libvpx build for me.

@SuperSandro2000 SuperSandro2000 merged commit 3a70a0a into NixOS:staging Jun 17, 2021
@Mindavi Mindavi deleted the libvpx/cross-compilation branch June 17, 2021 15:47
@NickCao NickCao mentioned this pull request Dec 19, 2021
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: cross-compilation Building packages on a different platform than they will be used on 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants