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

mario: init at version 0.0.155 #90695

Closed
wants to merge 2 commits into from
Closed

Conversation

python-mario-bot
Copy link

Motivation for this change

Add Mario CLI app, which runs Python pipelines in the shell.

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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@jonringer
Copy link
Contributor

@python-mario-bot thanks for opening your first PR with nixpkgs :)

@python-mario-bot
Copy link
Author

@jonringer Thanks for the tips. I've applied those changes.

@python-mario-bot python-mario-bot changed the title python3Packages.mario: init at version 0.0.155 mario: init at version 0.0.155 Jul 7, 2020
@jonringer
Copy link
Contributor

cc @FRidh what's the policy on poetry2nix as part of a package?

@jonringer
Copy link
Contributor

As I kind of suspected, this isn't going to be allowed in nixpkgs:

nix-env failed:
warning: ignoring the user-specified setting 'restrict-eval', because it is a restricted setting and you are not a trusted user
error: while evaluating anonymous function at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/.gc-of-borg-outpaths.nix:44:12, called from undefined position:
while evaluating 'g' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/attrsets.nix:276:19, called from undefined position:
while evaluating anonymous function at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/top-level/release-lib.nix:144:38, called from undefined position:
while evaluating 'isDerivation' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/attrsets.nix:305:18, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/top-level/release-lib.nix:146:10:
while evaluating 'callPackageWith' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/customisation.nix:117:35, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/top-level/all-packages.nix:13853:11:
while evaluating 'makeOverridable' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/customisation.nix:67:24, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/customisation.nix:121:8:
while evaluating anonymous function at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/applications/misc/mario/default.nix:1:1, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/customisation.nix:69:16:
while evaluating 'mkPoetryApplication' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/development/tools/poetry2nix/poetry2nix/default.nix:170:5, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/applications/misc/mario/default.nix:3:1:
while evaluating the attribute 'pkgs.buildPythonPackage' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/development/interpreters/python/default.nix:38:9:
while evaluating 'callPackageWith' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/customisation.nix:117:35, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/development/interpreters/python/default.nix:20:24:
while evaluating 'makeOverridable' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/customisation.nix:67:24, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/customisation.nix:121:8:
while evaluating anonymous function at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/top-level/python-packages.nix:9:1, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/customisation.nix:69:16:
while evaluating 'fix'' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/fixed-points.nix:25:10, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/top-level/python-packages.nix:7611:4:
while evaluating 'extends' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/fixed-points.nix:69:24, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/fixed-points.nix:25:21:
while evaluating 'composeExtensions' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/fixed-points.nix:75:17, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/fixed-points.nix:69:67:
while evaluating 'composeExtensions' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/fixed-points.nix:75:17, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/fixed-points.nix:78:22:
while evaluating anonymous function at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/development/tools/poetry2nix/poetry2nix/default.nix:113:20, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/lib/fixed-points.nix:76:22:
while evaluating 'readTOML' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/development/tools/poetry2nix/poetry2nix/lib.nix:70:14, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/development/tools/poetry2nix/poetry2nix/default.nix:40:20:
cannot read '/nix/store/plws5srm0l19swy7pm2jdqkjjwmfdrw1-mario-v0.0.155.tar.gz/poetry.lock', since path '/nix/store/hw1hvl4s701dfnxd9n6d8vys9l62525q-mario-v0.0.155.tar.gz.drv' is not valid, at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-2/pkgs/development/tools/poetry2nix/poetry2nix/lib.nix:70:30

essentially, you're having one derivation create another through poetry2nix. This is fine on your local machine, but nixpkgs has strict evaluation.

Please use normal buildPythonApplication conventions. you can look at awscli for a package which needs to substitute python package versions.

@infinisil
Copy link
Member

Poetry2nix only uses IFD if builtins.fromTOML isn't available, see . And builtins.fromTOML has been available since Nix 2.1. Recently the minimum required Nix version for nixpkgs has been bumped to 2.2, so using poetry2nix should be allowed. I think ofborg just uses an older Nix version for the evaluation (cc @grahamc)

@jonringer
Copy link
Contributor

I'm probably just used to using buildPythonApplication, so much of this seems foreign. If it's able to produce a valid application, and it's not super fragile, then I'm okay with it. Still would like to see what @FRidh thinks

@python-mario-bot
Copy link
Author

Updated in response to @infinisil comments.

@jonringer
Copy link
Contributor

jonringer commented Jul 8, 2020

I'm not much of a fan of the 2100 line lock file, which also requires pulling >200 sources.

I would prefer for this to be tranlated into a normal buildPythonApplication and re-use as much of the existing python-nix packages

@python-mario-bot
Copy link
Author

python-mario-bot commented Jul 8, 2020

I understand the reluctance to have so many new dependency derivations. Smaller packages hog less resources, and re-using the existing nixpkgs would mean eliminating the need to rebuild and store versions that have already been built with buildPythonPackage.

On the other hand, building from Poetry lockfiles makes it easier to create and maintain derivations, since most of the work is automated. Additionally, as poetry2nix usage becomes more common in the wild, poetry2nix nixpkgs will increasingly be able to reuse cached builds. This will have a compounding effect, where each additional poetry2nix package in nixpkgs makes it easier to use poetry2nix (and thus easier to use Nix).

Perhaps mario can be part of the experiment in seeing how well poetry2nix integrates into nixpkgs.

@FRidh
Copy link
Member

FRidh commented Jul 8, 2020

Unless this requires many new (or different versions of) packages that already exist in Nixpkgs I suppose this is the future. I've expressed several times my dislike to the use of these tools in Nixpkgs, but who am I to block it.

This is indeed a large lock file, yet still smaller than e.g. typical yarn lock files.

At the same time, this will take time to build, every time, and add considerably to the S3 storage. So far everything is being paid for, but I wonder when the line will be drawn, because as soon as we have several of these lock files it goes fast.

@python-mario-bot
Copy link
Author

Makes me wonder if eventually it might be worth investing in tooling to try adding constraints/preferences to the solver so that already-built dependency versions are preferred.

@jonringer
Copy link
Contributor

I don't think poetry2nix makes any assumptions about what nixpkgs are available (at least not from looking at the lock file)

@python-mario-bot
Copy link
Author

So far everything is being paid for, but I wonder when the line will be drawn, because as soon as we have several of these lock files it goes fast.

Indeed, if that time comes it may be necessary to course-correct and apply some tooling or manual work to restrict the proliferation of builds. I think the storage and compute resources are worth keeping in mind as nixpkgs evolves, but at this point I'd prefer merging the expression as-is if you're willing.

@python-mario-bot
Copy link
Author

From my perspective this is ready to be merged. If there are no remaining issues, I'd like to proceed.

@Lassulus
Copy link
Member

well we need to find a solution to growing lock files in nixpkgs anyways. there are a lot of them lying around and its getting more, but ignoring open PRs is not a solution so I'm gonna merge this for now

@Lassulus
Copy link
Member

it seems this also needs to be imported in all-packages.nix or somewhere else so it shows up in the package set. Right now nix-review reports no changes when reviewing this

@jonringer
Copy link
Contributor

I think I would be more okay with this if it had hydraPlatforms = [ ]; so at the very least hydra isnt going to have to put an additional ~100MB worth of wheels into the cache per python interpreter, per python derivation change.

python-mario-bot added 2 commits August 24, 2020 19:58
@python-mario-bot
Copy link
Author

@Lassulus I've updated the PR to add the package in all-packages.nix.

@Lassulus
Copy link
Member

@python-mario-bot can you also add the hydraPlatforms = [ ]; which @jonringer suggested? sounds like a good idea

@FRidh
Copy link
Member

FRidh commented Aug 25, 2020

$ nix-instantiate -A mario
error: --- EvalError ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- nix-instantiate
in file: /home/freddy/Code/libraries/nixpkgs_2/pkgs/development/interpreters/python/mk-python-derivation.nix (31:13)

attribute 'pname' missing
(use '--show-trace' to show detailed location information)

@FRidh
Copy link
Member

FRidh commented Sep 21, 2020

I strongly recommend upstream to add a flake with this expression to their repository instead. Flakes-support is not yet released, but I doubt there will be any more changes.

@stale
Copy link

stale bot commented Apr 18, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 18, 2021
@SuperSandro2000
Copy link
Member

Closing due to inactivity from author.

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

8 participants