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

buildPython*: store dist (wheel/sdist) in dist output #190487

Merged
merged 1 commit into from Sep 12, 2022
Merged

Conversation

FRidh
Copy link
Member

@FRidh FRidh commented Sep 9, 2022

Store the intermediate artifacts. In time, we should build, install and test in separate derivations as that reduces circular dependencies, avoids rebuilds when fixing tests, and makes it possible to use the wheels for creating say virtualenv's.

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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@FRidh
Copy link
Member Author

FRidh commented Sep 9, 2022

Note this will roughly double the amount of data in the cache.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Sep 9, 2022

There is often an error log in the output like for py:

Check whether the following modules can be imported: iniconfig
pythonOutputDistPhase
Executing pythonOutputDistPhase
Finished executing pythonOutputDistPhase
pythonRemoveBinBytecodePhase
pythonImportsCheckPhase
Executing pythonImportsCheckPhase
pythonOutputDistPhase
Executing pythonOutputDistPhase
Finished executing pythonOutputDistPhase
patching script interpreter paths in /nix/store/kk2xcy6y89586r8qwy80x6snbs41x05z-python3.10-py-1.11.0
checking for references to /build/ in /nix/store/kk2xcy6y89586r8qwy80x6snbs41x05z-python3.10-py-1.11.0...
find: ‘/nix/store/hp1fdmx4ff9irrysasl2q8phxiz9w897-python3.10-py-1.11.0-dist’: No such file or directory
strip is /nix/store/n95cd4q1dqzdvsiy1hmrkx9shwi3n4sh-gcc-wrapper-11.3.0/bin/strip
Executing pythonRemoveTestsDir
Finished executing pythonRemoveTestsDir
pythonCatchConflictsPhase
pythonRemoveBinBytecodePhase
pythonImportsCheckPhase
Executing pythonImportsCheckPhase
Check whether the following modules can be imported: py
pythonOutputDistPhase
Executing pythonOutputDistPhase
Finished executing pythonOutputDistPhase

and the build fails because two of the ofborg evaluators run out of inodes:

error: creating file '/nix/store/b9l6z3w0rnlfgs8adc5imgg6vwdbz4rl-source/tls/s2n_client_cert.c': No space left on device

@ofbrog eval

Store the intermediate artifacts. In time, we should build, install and
test in separate derivations as that reduces circular dependencies,
avoids rebuilds when fixing tests, and makes it possible to use the
wheels for creating say virtualenv's.
@FRidh
Copy link
Member Author

FRidh commented Sep 9, 2022

Yes something expects the output to be built before our phase is executed. Moved to preFixupPhases.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-build-python-package-for-use-in-non-nix-environments/21587/2

@FRidh FRidh changed the base branch from master to staging September 12, 2022 17:01
@FRidh FRidh changed the base branch from staging to staging-next September 12, 2022 17:51
@FRidh FRidh changed the base branch from staging-next to staging September 12, 2022 17:51
@FRidh FRidh merged commit adbc59c into NixOS:staging Sep 12, 2022
@FRidh FRidh deleted the dist branch September 12, 2022 20:15
@jonringer
Copy link
Contributor

I'm late, but I think this is a good first step in getting a source -> dist -> package workflow going.

@trofi
Copy link
Contributor

trofi commented Sep 13, 2022

Bisect claims that adbc59c buildPython*: store dist (wheel/sdist) in dist output dendered cvise and python3Packages.scipy to fail builds in staging as:

@nix { "action": "setPhase", "phase": "installCheckPhase" }
running install tests
/nix/store/hlihc9dxb0rk56ln3k7f2394h6jb3ayv-stdenv-linux/setup: line 1399: pushd: dist: No such file or directory

@FRidh
Copy link
Member Author

FRidh commented Sep 13, 2022

yeah we should pushd to another folder in that package

@FRidh
Copy link
Member Author

FRidh commented Sep 15, 2022

Fix in #191327

@trofi
Copy link
Contributor

trofi commented Sep 15, 2022

I still see the same cvise build failure on staging and on python-updates:

$ nix build --no-link -f. cvise
...
@nix { "action": "setPhase", "phase": "pythonOutputDistPhase" }
pythonOutputDistPhase
Executing pythonOutputDistPhase
mv: cannot stat 'dist': No such file or directory

Not sure if I'm looking at the wrong branch or the failure is expected.

@FRidh
Copy link
Member Author

FRidh commented Sep 16, 2022

cvise needs fixing, probably needs format="other"; and all these dont* removed

@trofi
Copy link
Contributor

trofi commented Sep 16, 2022

Thank you! Proposed possible fix as #191461

@trofi
Copy link
Contributor

trofi commented Sep 16, 2022

cvise was already fixed in 370cce6

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/why-is-buildpythonpackage-passthru-not-in-my-result-folder/21852/4

markuskowa added a commit to Nix-QChem/NixOS-QChem that referenced this pull request Oct 3, 2022
markuskowa added a commit to Nix-QChem/NixOS-QChem that referenced this pull request Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants