ruff: Build as a standalone without Python #358029
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 tasks
1facbb3 to
1670796
Compare
baloo
requested changes
Nov 22, 2024
There was a problem hiding this comment.
Just change your development shell expression to use python312.pkgs.ruff (or whichever version of python you might want) and not just ruff?
I'm not sure I follow the problem here.
The upstream project intends for ruff to be built using maturin. Here you're duplicating the build to make another one that won't use maturin.
|
If the purpose is to strip off propagation, you can do so in your nix-shell: let
pkgs = import <nixpkgs> {};
ruff = pkgs.runCommandNoCC "ruff" {} ''
mkdir -p $out/bin
ln -s ${pkgs.ruff}/bin/ruff $out/bin
'';
in
pkgs.mkShell {
buildInputs = [ ruff ];
} |
But this should be the default behaviour, don't you think? |
Then you have to fix the behavior 1042 other packages. |
cc5ea3d to
e1cd562
Compare
|
@baloo Well, you have to start somewhere. Starting with popular packages that matter to you seems an apt strategy? |
|
Well, the problem can be solved by using |
|
@adisbladis are you willing to finish this ? |
|
#361878 has been merged into |
e1cd562 to
8f71cfe
Compare
76afa0b to
67d9958
Compare
|
drupol
approved these changes
Dec 23, 2024
|
Most darwin failures are not related and tackled in #367756. |
|
ambroisie
reviewed
Dec 24, 2024
pkgs/by-name/ru/ruff/package.nix
Outdated
| --fish <($bin/bin/ruff generate-shell-completion fish) \ | ||
| --zsh <($bin/bin/ruff generate-shell-completion zsh) | ||
| ''; | ||
| postInstall = lib.optionalString (stdenv.buildPlatform.canExecute stdenv.hostPlatform) '' |
There was a problem hiding this comment.
Nit: could use stdenv.hostPlatform.emulator buildPackages instead.
There was a problem hiding this comment.
Thanks for the tip ! Have I got it right ?
There was a problem hiding this comment.
Almost, you don't need stdenv.buildPlatform.canExecute as a condition for it. You can look at how other packages do it.
There was a problem hiding this comment.
Oh cool ! I removed the optionalString.
d4873da to
5b27fe1
Compare
|
5b27fe1 to
70e50f5
Compare
drupol
approved these changes
Dec 24, 2024
baloo
reviewed
Dec 24, 2024
| @@ -0,0 +1,28 @@ | |||
| { | |||
| buildPythonPackage, | |||
| pkgs, | |||
There was a problem hiding this comment.
This effectively breaks cross-compilation.
baloo
previously requested changes
Dec 24, 2024
There was a problem hiding this comment.
This breaks python3Packages.pytest-ruff which currently builds on master.
I think it requires the following patch:
diff --git a/pkgs/by-name/ru/ruff/package.nix b/pkgs/by-name/ru/ruff/package.nix
index 646ef93836f5..662b7409ec8e 100644
--- a/pkgs/by-name/ru/ruff/package.nix
+++ b/pkgs/by-name/ru/ruff/package.nix
@@ -26,14 +26,6 @@ rustPlatform.buildRustPackage rec {
hash = "sha256-c5d2XaoEjCHWMdjTLD6CnwP8rpSXTUrmKSs0QWQ6UG0=";
};
- # Do not rely on path lookup at runtime to find the ruff binary
- postPatch = ''
- substituteInPlace python/ruff/__main__.py \
- --replace-fail \
- 'ruff_exe = "ruff" + sysconfig.get_config_var("EXE")' \
- 'return "${placeholder "bin"}/bin/ruff"'
- '';
-
useFetchCargoVendor = true;
cargoHash = "sha256-jbUjsIJRpkKYc+qHN8tkcZrcjPTFJfdCsatezzdX4Ss=";
diff --git a/pkgs/development/python-modules/ruff/default.nix b/pkgs/development/python-modules/ruff/default.nix
index 8d79eee66f4b..8f5efa9a240b 100644
--- a/pkgs/development/python-modules/ruff/default.nix
+++ b/pkgs/development/python-modules/ruff/default.nix
@@ -10,12 +10,19 @@ buildPythonPackage {
pname
version
src
- postPatch
cargoDeps
postInstall
meta
;
+ # Do not rely on path lookup at runtime to find the ruff binary
+ postPatch = ''
+ substituteInPlace python/ruff/__main__.py \
+ --replace-fail \
+ 'ruff_exe = "ruff" + sysconfig.get_config_var("EXE")' \
+ 'return "${placeholder "out"}/bin/ruff"'
+ '';
+
pyproject = true;
nativeBuildInputs = [
(the python derivation does not carry a bin output)
70e50f5 to
99b21b8
Compare
Good catch, thanks |
Using `buildPythonPackage` make `ruff` propagate the Python used for the build, which might not be the same Python as you want in your development shell. NixOS#350654 changed the ruff top-level attribute to be built with Python modules. This doesn't actually make much sense, we have versioned Python sets, and including just the one Python in the top-level package isn't helpful, and causes issues downstream related to PATH & PYTHONPATH. See my related [uv PR](NixOS#357113 (comment)).
99b21b8 to
d2d7c7e
Compare
|
drupol
approved these changes
Dec 25, 2024
|
@baloo is cross-compilation still broken ? |
|
|
Merging as it seems to work fine. |
dotlambda
reviewed
Jan 7, 2025
| @@ -0,0 +1,28 @@ | |||
| { | |||
| buildPythonPackage, | |||
| pkgs, | |||
| }: | ||
|
|
||
| buildPythonPackage { | ||
| inherit (pkgs.ruff) |
There was a problem hiding this comment.
Suggested change
| inherit (pkgs.ruff) | |
| inherit (ruff) |
| ruff = toPythonModule (pkgs.ruff.override { | ||
| python3Packages = self; | ||
| }); | ||
| ruff = callPackage ../development/python-modules/ruff { }; |
There was a problem hiding this comment.
Suggested change
| ruff = callPackage ../development/python-modules/ruff { }; | |
| ruff = callPackage ../development/python-modules/ruff { | |
| inherit (pkgs) ruff; | |
| }; |
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using
buildPythonPackagemakeruffpropagate the Python used for the build, which might not be the same Python as you want in your development shell.#350654 changed the ruff top-level attribute to be built with Python modules. This doesn't actually make much sense, we have versioned Python sets, and including just the one Python in the top-level package isn't helpful, and causes issues downstream related to PATH & PYTHONPATH.
See my related uv PR.
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.