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

mark broken package broken #225441

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

Et7f3
Copy link
Contributor

@Et7f3 Et7f3 commented Apr 9, 2023

Found while testing #225044

They are broken on master. So I just add metadata to avoid wasting CPU for other users (PyTorch fail near the end of the build)

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

…dtouch,ocamlPackages.taglib,python3Packages.pytorch: mark broken package broken
@SomeoneSerge
Copy link
Contributor

SomeoneSerge commented Apr 9, 2023

pytorch

I don't have access to Darwin to test, but could it be related to merging #222273?

EDIT: logs at https://hydra.nixos.org/build/215378730/nixlog/1

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Apr 9, 2023
@Et7f3
Copy link
Contributor Author

Et7f3 commented Apr 9, 2023

Will test if reverting this PR help. Thanks for the pointer.

@Et7f3
Copy link
Contributor Author

Et7f3 commented Apr 9, 2023

$ git checkout origin/master
$ git revert -m 1 c5b01a1
$ nix build .#python3Packages.pytorch

and it build.

@SomeoneSerge
Copy link
Contributor

SomeoneSerge commented Apr 9, 2023

I presume that the aligned_alloc in question is this (one of these) function(s) wrapped in a bunch of ifdefs: https://github.com/mreineck/pocketfft/blame/076cb3d2536b7c5d0629093ad886e10ac05f3623/pocketfft_hdronly.h#L156

Updating pytorch -> updating submodules -> maybe some ifdefs have changed? E.g. this is from 2 months ago: mreineck/pocketfft@5fb931a. Although it always had the mingw32 term, it seems. Also cf the related change from October: mreineck/pocketfft@764893f. Also doesn't explain why aarch64 builds fine

@SomeoneSerge
Copy link
Contributor

I'm skeptical but maybe we can try

patches = [
  ...
  (fetchpatch { revert = true; url = "https://github.com/mreineck/pocketfft/commit/764893f3f232d778608025d1256a1c814ce0139e.patch"; hash = ""; })
  (fetchpatch { revert = true; url = "https://github.com/mreineck/pocketfft/commit/5fb931a048d4ed6e2be304bbfb608f655f3da6bf.patch"; hash = ""; })
];

@RaitoBezarius
Copy link
Member

Please can we ping the maintainers of those packages so they can chime in and potentially fix the packages?

@Et7f3
Copy link
Contributor Author

Et7f3 commented Apr 10, 2023

I presume that the aligned_alloc in question is this (one of these) function(s) wrapped in a bunch of ifdefs: https://github.com/mreineck/pocketfft/blame/076cb3d2536b7c5d0629093ad886e10ac05f3623/pocketfft_hdronly.h#L156

Updating pytorch -> updating submodules -> maybe some ifdefs have changed? E.g. this is from 2 months ago: mreineck/pocketfft@5fb931a. Although it always had the mingw32 term, it seems. Also cf the related change from October: mreineck/pocketfft@764893f. Also doesn't explain why aarch64 builds fine

This issue seems to be specific to nixpkgs

I see a fix

] ++ lib.optionals (stdenv.isDarwin && lib.versionOlder stdenv.targetPlatform.darwinSdkVersion "11.0") [
# error: use of undeclared identifier 'aligned_alloc'
"-include mm_malloc.h"
"-Daligned_alloc=_mm_malloc"
]);

Or another is to force taking the false branch that add a fallback.

@Et7f3 Et7f3 marked this pull request as draft April 10, 2023 23:07
@Et7f3
Copy link
Contributor Author

Et7f3 commented Apr 10, 2023

I put in draft because I think I will remove the one I opened PR to fix and will do another round with the help of hydra to spot more failure.

@@ -62,6 +63,7 @@ buildDunePackage rec {
meta = with lib; {
description = "Address Resolution Protocol purely in OCaml";
homepage = "https://github.com/mirage/arp";
broken = stdenv.isDarwin;
Copy link
Member

Choose a reason for hiding this comment

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

missing comment

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in #226657.

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
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 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: ocaml 6.topic: python 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants