python3Packages.relaxBuildSystemRequiresHook: init #331315
Conversation
ofborg
bot
added
8.has: package (new)
| relaxBuildSystemRequiresHook = callPackage ({ makePythonHook, packaging, tomlkit }: | ||
| makePythonHook { | ||
| name = "relax-build-system-requires-hook"; | ||
| propagatedBuildInputs = [ packaging tomlkit ]; |
There was a problem hiding this comment.
FTR, tomlkit in turn depends on pyyaml, pytest and poetry-core, which now risks being load-bearing for the majority of python3Packages. This is far better than introducing tomlq or dasel. 👍
There was a problem hiding this comment.
Could we maybe use the inbuilt tomli and tomli-w? THen we only need poetry-core. For pytest we could disable tests.
There was a problem hiding this comment.
I feel it is way more safe to use the tool that preserves whitespace and comments
There was a problem hiding this comment.
tomlkit is recommended for editing existing toml.
Also, poetry-core is the only dependency when tests are disabled.
The TOML Kit package is a style-preserving TOML library with both read and write capability. It is a recommended replacement for this module for editing already existing TOML files.
| In general you should always use `relaxBuildSystem`, because `removeBuildSystem` | ||
| will cause silent build errors. However `removeBuildSystem` may | ||
| still be useful in exceptional cases, and also to remove dependencies wrongly | ||
| declared by upstream (for example, declaring `pytest` as a build dependency | ||
| instead of a test dependency). |
There was a problem hiding this comment.
I think also mentioning numpy_2 here is a good idea, since they explicitly reccomend that you add numpy>=2 to requires even when compatible with v1
|
This should also be backported to stable, since fixes that make use of this is also bound to get backported |
|
Result of 17 packages built:
|
|
The ofborg darwin failure is already present on hydra https://hydra.nixos.org/build/267900625 |
Do we actually care from where the dependency is? Can't we combine those by default? |
|
I have no strong opinion about adding new attributes. |
|
Regarding |
A |
I'm in favor of the separation. It's cleaner, easier to understand and document, and might afford maintainers to navigate more packaging edge cases |
natsukium
force-pushed
the
relax-build-system-requires-hook
branch
from
96aa7f8
to
91451e0
Compare
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
| @@ -489,6 +489,7 @@ are used in [`buildPythonPackage`](#buildpythonpackage-function). | |||
| - `pythonRelaxDepsHook` will relax Python dependencies restrictions for the package. | |||
| See [example usage](#using-pythonrelaxdepshook). | |||
| - `pythonRemoveBinBytecode` to remove bytecode from the `/bin` folder. | |||
| - `relaxBuildSystemRequiresHook` to relax build-system restrictions defined in PEP518 for the package. | |||
There was a problem hiding this comment.
Hmm most people are not familiar with the different PEPs, so how about:
| - `relaxBuildSystemRequiresHook` to relax build-system restrictions defined in PEP518 for the package. | |
| - `relaxBuildSystemRequiresHook` to relax build-system restrictions defined in projects' `pyproject.toml`. |
| @@ -1376,8 +1377,53 @@ work with any of the [existing hooks](#setup-hooks). | |||
|
|
|||
| The `pythonRelaxDepsHook` has no effect on build time dependencies, such as | |||
| those specified in `build-system`. If a package requires incompatible build | |||
| time dependencies, they should be removed in `postPatch` through | |||
| `substituteInPlace` or similar. | |||
| time dependencies, consider using `relaxBuildSystem` or `removeBuildSystem`. | |||
There was a problem hiding this comment.
The Deps suffix missing is confusing IMO, how about naming these attributes:
| time dependencies, consider using `relaxBuildSystem` or `removeBuildSystem`. | |
| time dependencies, consider using `pythonRelaxBuildDeps` or `pythonRemoveBuildDeps`. |
|
|
||
| #### Using relaxBuildSystemRequiresHook {#using-relaxbuildsystemrequireshook} | ||
|
|
||
| `relaxBuildSystemRequiresHook` has the same function as `pythonRelaxDepsHook`, |
There was a problem hiding this comment.
Similarly, I'd suggest a different name, analogue to the existing attributes:
| `relaxBuildSystemRequiresHook` has the same function as `pythonRelaxDepsHook`, | |
| `pythonRelaxBuildDepsHook` has the same function as `pythonRelaxDepsHook`, |
| @@ -2008,7 +2054,8 @@ The following rules are desired to be respected: | |||
| * Only unversioned attributes (e.g. `pydantic`, but not `pypdantic_1`) can be included in `dependencies`, | |||
| since due to `PYTHONPATH` limitations we can only ever support a single version for libraries | |||
| without running into duplicate module name conflicts. | |||
| * The version restrictions of `dependencies` can be relaxed by [`pythonRelaxDepsHook`](#using-pythonrelaxdepshook). | |||
| * The version restrictions of `dependencies` can be relaxed by [`pythonRelaxDepsHook`](#using-pythonrelaxdepshook) | |||
| and of `build-system` can be relaxed by [`relaxBuildSystemRequiresHook`](#using-relaxbuildsystemrequireshook). | |||
There was a problem hiding this comment.
| and of `build-system` can be relaxed by [`relaxBuildSystemRequiresHook`](#using-relaxbuildsystemrequireshook). | |
| and of `build-system` can be relaxed by [`pythonRelaxBuildDepsHook`](#using-pythonRelaxBuildDepsHook). |
Description of changes
relaxBuildSystemRequiresHookhas the same function aspythonRelaxDepsHook, but it affectsbuild-systeminstead ofdependencies.This hook is executed in the preBuild phase and modifies pyproject.toml using
relaxBuildSystemandremoveBuildSystem.This hook avoids manual patching and reduces build failures due to old patches when updating.
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.