pipenv: 2018.11.26 -> 2020.5.28 #89164
Conversation
|
This derivation depends on Upstream's However, they still seem to be indirect dependencies, as they're still listed in upstream's Should (and can) we remove the explicit dependencies from the derivation? |
|
Result of 1 package built:- pipenv |
Pipenv team just changed how they included dev/test deps into Pipfile, instead of repeating setup.py deps in Pipfile they do
The new version can be build without them, while the old version build fails, so I assume it's safe to remove them. |
|
They were in setup_requires https://github.com/pypa/pipenv/blob/v2018.11.26/setup.py#L135, now it's empty https://github.com/pypa/pipenv/blob/v2020.5.28/setup.py#L148 |
|
Added patch that fixes |
kalekseev
force-pushed
the
update/pipenv
branch
2 times, most recently
from
ab4f141
to
947bbb4
Compare
|
Are the issues resolved now? |
| substituteInPlace pipenv/core.py \ | ||
| --replace "vistir.compat.Path(sys.executable).absolute().as_posix()" "vistir.compat.Path('${pythonEnv.interpreter}').absolute().as_posix()" | ||
| --replace "sys.executable" "'${pythonEnv.interpreter}'" |
There was a problem hiding this comment.
👍 for simplifying the substitution while you had to make it wider anyway.
|
Yes but it would be good someone else tested it, I’ve tested on macOS and Ubuntu with my projects
…On Sun, May 31, 2020, at 12:03, Jörg Thalheim wrote:
Are the issues resolved now?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#89164 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACZVG22EGAU7PF24V4ZKKLRUIMM5ANCNFSM4NN7WNXA>.
--
Konstantin Alekseev
|
|
cc @das-g |
|
For 947bbb4d12e on NixOS unstable: Result of 1 package built:- pipenv Tested with some pure-python packages:(Non-pure packages might require a FHS environment with certain libs available.)
👍 I'd say that looks good. |
|
@das-g thanks for suggestions, I was in a hurry today morning so pushed fixed version without checking comment's wording. I have a question, I pass runtimeDeps into clojure instead of using clojure's argument |
Ship it!
I think you mean "closure". Clojure's a programming language. I guess this works because the packages that |
|
One option would be to set the variable to the function instead of the list: das-g/nixpkgs@82b639e |
|
It looks cleaner for me one thing I would change is to use verb |
What would that changed name mean? That the function builds the runtime dependencies? (It doesn't. It just selects some entries from a set and returns them as a list.) Or that it returns buildtime and runtime dependencies? (Is that the case?) |
|
The problem is my limited experience with nix so I'm just trying to mimic things and names that I already saw somewhere in the code, so I think you are right buildRuntimeDeps is not good name for that. Let me know if I should push your original patch and thanks for your help. |
Yeah. I myself thought about renaming the variable now that it holds a function rather than a list, but I'm not aware of any naming convention in Nix to indicate that fact. Some functions seem to be named after what they do, others after what they return.
If you feel that it's an improvement, sure. I'm probably not that much more experienced in Nix than you. Feel free to use my commit das-g@82b639e as-is or to fold it into yours, whichever you prefer. If we need expert judgment, maybe @FRidh or @jonringer (code owners of Python related things in NixPkgs) can chime in? Lines 64 to 71 in 5e898d1 |
Eh, that works, too. 🤷 I'll make a separate PR of das-g/nixpkgs@82b639e then. |
Done: #89293 |
Motivation for this change
https://github.com/pypa/pipenv/releases/tag/v2020.5.28
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)