python312Packages.tree-sitter_0_21: remove from propagated dependencies#332747
Conversation
github-actions
bot
added
the
6.topic: python
ofborg
bot
added
8.has: clean-up
ofborg
bot
added
10.rebuild-darwin: 11-100
pkgs/development/python-modules/tree-sitter-languages/default.nix
Outdated
Show resolved
Hide resolved
natsukium
force-pushed
the
dont-propagate-versioned-tree-sitter
branch
from
3efc201 to
6351301
Compare
|
Result of 4 packages marked as broken and skipped:
2 packages failed to build:
27 packages built successfully:
Result of 4 packages marked as broken and skipped:
15 packages failed to build:
13 packages built successfully:
|
doronbehar
left a comment
doronbehar
left a comment
There was a problem hiding this comment.
Changes look good overall, I wonder how come so many packages got dependent on tree-sitter_0_21... I'm only a bit confused about the changes to python31{1,2}Packages.textual:
The optional-dependencies.syntax attribute is confusing to me - why not change there the tree-sitter dependency to tree-sitter_0_21? If the last commit will be included eventually, the optional-dependencies.syntax attribute will almost never be evaluated, but it is still confusing, especially since this PR is meant to avoid mixing different versions of the same python package in the dependency tree of packages.
Another thing that bothers me, is the fact that without the last commit, python312Packages.textual builds fine on Hydra. However, python311Packages.textual doesn't evaluate because
tree-sitter-languages is marked as broken on Darwin. Although the lib.optionals condition should not add it to optional-dependencies.syntax. Do you know how to explain that please @natsukium ?
Also, if the goal of the last commit is to fix the python311Packages.textual, the commit message should indicate that.
I take these words back... I iterated once more the commits and I do understand why these packages dependent on I would approve a PR that would change only |
|
Did you notice that We can't control the downstream packages or the user environment. |
natsukium
force-pushed
the
dont-propagate-versioned-tree-sitter
branch
from
a930fa8 to
4ea27c0
Compare
|
I found that |
…ee-sitter Python packages must not propagate such versioned packages due to PYTHONPATH conflicts.
Python packages must not propagate such versioned packages due to PYTHONPATH conflicts.
Don't depend on tree-sitter-languages for testing, as it is marked as broken. Fixes also: NixOS#331260
Yea indeed it is a weird thing to do.
No.. (perhaps I should have added my self to the maintainers.)
OK, that is convincing.
That's good. I took the liberty to rebase the PR onto latest |
doronbehar
force-pushed
the
dont-propagate-versioned-tree-sitter
branch
from
4ea27c0 to
1e61017
Compare
|
Thanks for understanding and adding context. |
| tree-sitter = callPackage ../development/python-modules/tree-sitter { }; | ||
|
|
||
| tree-sitter0_21 = callPackage ../development/python-modules/tree-sitter0_21 { }; | ||
| tree-sitter_0_21 = callPackage ../development/python-modules/tree-sitter/0_21.nix { }; |
There was a problem hiding this comment.
Would have been ideal to add an entry in python-aliases for the rename.
There was a problem hiding this comment.
Certainly, but it's an internal package and I didn't want to pollute the namespace too much.
There was a problem hiding this comment.
Would have been ideal to add an entry in python-aliases for the rename.
Certainly, but it's an internal package and I didn't want to pollute the namespace too much.
Also, that attribute was there for a ~month or less, and it is an outdated version, so I barely think anyone had started using it externally.
4 participants
Description of changes
Python packages must not propagate such versioned packages due to PYTHONPATH conflicts.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.