Should our QT `mkDerivation` be deprecated?

AndersonTorres · July 10, 2022, 9:44pm

Hello, people!

Given that stdenv.mkDerivation now supports brand new overlay style overridable rec-less recursive attributes since

github.com/NixOS/nixpkgs

stdenv.mkDerivation: overlay style overridable recursive attributes

NixOS:master ← hercules-ci:mkDerivation-overridable-recursive-attributes

opened 11:53AM - 20 Apr 21 UTC

roberth

+220 -10

###### Motivation for this change - Allows to get rid of confusing `rec { }` in `mkDerivation` calls. Overriding an attribute in this style causes derived attributes to be overridden as expected. - Allows `passthru` attributes to reference the outputs - Fixes #119407, the problem where if you use `overrideAttrs`, the tests in `passthru` still use the old package. - Should allow cleanup of python and rust packaging functions by using "overlay" layers on a prototype derivation, rather than the current workarounds. See updated manual for usage explanation. Update: this problem was also brought up in this discussion: https://github.com/NixOS/nixpkgs/pull/119731#pullrequestreview-638309038 Update: "overlay" part minus `public` was also discovered and implemented by lilyball https://github.com/NixOS/nixpkgs/pull/94198 # :rotating_light: Please prioritize reviews and fixes for the release first! ### Consider that your reviewer is already busy with stabilization work and other reviews, so now is not really a time for refactoring, unless you need the change to solve a significant problem. Thank you. ###### Things done - [ ] Tested using sandboxing **not applicable:** changes are evaluation-only ([nix.useSandbox](https://nixos.org/nixos/manual/options.html#opt-nix.useSandbox) on NixOS, or option `sandbox` in [`nix.conf`](https://nixos.org/nix/manual/#sec-conf-file) 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](https://github.com/NixOS/nixpkgs/blob/master/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) - [x] Ensured that relevant documentation is up to date - [x] Fits [CONTRIBUTING.md](https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md).

I myself am rewriting and updating some expressions to this format.

When rewriting bibletime, I found a “bug”: libsForQt5.mkDerivation has no such support. So I am using stdenv.mkDerivation and reported this to @roberth, and he filled this issue below:

github.com/NixOS/nixpkgs

Qt `mkDerivation` is slightly broken, redundant and should be deprecated

opened 11:00AM - 09 Jul 22 UTC

roberth

0.kind: bug 6.topic: qt/kde 6.topic: hygiene

### Describe the bug a. The Qt `mkDerivation` function provided by `qt` does …not support overlay-style package definitions. b. The Qt `mkDerivation` function is over-engineered because it only adds `wrapQtAppsHook` to `nativeBuildInputs`. A "hierarchy" of `mkDerivation` functions does not compose. ### Steps To Reproduce #### a. 1. Start a package definition with `mkDerivation (finalAttrs:` where `mkDerivation` isn't `stdenv.mkDerivation` but the one provided by the qt expressions. 2. Eval error #### b. 1. Write a Haskell or Python package that uses qt 2. Can't use qt's `mkDerivation` as we need to use a language specific `mkDerivation`. ### Expected behavior Just call `stdenv.mkDerivation` and add `wrapQtAppsHook`. Many packages already do this anyway (resulting in duplicate hooks; apparently harmless?). Deprecate `qt` `mkDerivation`. ### Screenshots ### Additional context Example - https://github.com/NixOS/nixpkgs/pull/180606 ### Notify maintainers  @NixOS/qt-kde ### Metadata Please run `nix-shell -p nix-info --run "nix-info -m"` and paste the result. ```console [user@system:~]$ nix-shell -p nix-info --run "nix-info -m" output here ```

That being exposed, should we deprecate and replace such mkDerivation specialization?

roberth · July 11, 2022, 6:52am

Alternatively, support for a function argument can be added to the Qt mkDerivation. I forgot to include that in the issue description, mentioning it only in the linked comment. I do not think that is the optimal solution though.