Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

Implementation: Fix references #14

Closed
infinisil opened this issue Nov 10, 2022 · 5 comments
Closed

Implementation: Fix references #14

infinisil opened this issue Nov 10, 2022 · 5 comments

Comments

@infinisil
Copy link
Member

In https://github.com/nixpkgs-architecture/nixpkgs/tree/auto-calling I implemented a rough version of this RFC draft using a hacky bash script (see commit message). The main problem with it is that evaluation is broken due to references to moved files:

$ nix-build -A hello
error (ignored): error: end of string reached
error: getting status of '/home/tweagysil/na/nixpkgs/build-support/setup-hooks/role.bash': No such file or directory
(use '--show-trace' to show detailed location information)

The fix for this particular issue is

diff --git a/pkgs/auto/gettext/default.nix b/pkgs/auto/gettext/default.nix
index a1270af259c..de6c7394d30 100644
--- a/pkgs/auto/gettext/default.nix
+++ b/pkgs/auto/gettext/default.nix
@@ -57,7 +57,7 @@ stdenv.mkDerivation rec {
     ++ lib.optionals (!stdenv.isLinux && !stdenv.hostPlatform.isCygwin) [ libiconv ];
 
   setupHooks = [
-    ../../../build-support/setup-hooks/role.bash
+    ../../build-support/setup-hooks/role.bash
     ./gettext-setup-hook.sh
   ];
   gettextNeedsLdflags = stdenv.hostPlatform.libc != "glibc" && !stdenv.hostPlatform.isMusl;

But there might be hundreds or even more of these evaluation failures.

Ideally the script to do the transition would automatically fix such references, but that's not very trivial without parsing Nix. I and @roberth had the idea of using rnix to write a tool that could do renames of files while updating references to those files automatically: nix-community/rnix-parser#151

In the absence of such a tool, we need to either manually do these fixes, or write a hacky tool and manually fix the problems.

@infinisil
Copy link
Member Author

infinisil commented Nov 11, 2022

Taking a closer look at these failures, there's at least these two types:

Failures caused by packages referencing other packages files (the gettext example above).

While these references ideally wouldn't exist, that's a separate issue, and fixing those references is fairly easy by just using the new relative location

Failures caused by packages having multiple versions of themselves, but referencing common files.

An example of this is readline, which is defined in all-packages.nix like this:

{
  readline = readline6;
  readline6 = readline63;
  readline63 = callPackage ../development/libraries/readline/6.3.nix { };
  readline70 = callPackage ../development/libraries/readline/7.0.nix { };
  readline81 = callPackage ../development/libraries/readline/8.1.nix { };
}

Meanwhile the development/libraries/readline directory has these files:

6.3.nix
7.0.nix
8.1.nix
android.patch
link-against-ncurses.patch
no-arch_only-6.3.patch
readline-6.3-patches.nix
readline-7.0-patches.nix
readline-8.1-patches.nix
update-patch-set.sh -> ../../../shells/bash/update-patch-set.sh

The script to do the transition is not very smart about this: It creates pkgs/auto/{readline63,readline70,readline81}/default.nix files and moves the {6.3,7.0,8.1}.nix files there.

Each of these files contains references to patchfiles in the readline directory though, here's 6.3.nix:

{
  # ...
  patches =
    [ ./link-against-ncurses.patch
      ./no-arch_only-6.3.patch
    ] ++ lib.optional stdenv.hostPlatform.useAndroidPrebuilt ./android.patch
    ++
    (let
       patch = nr: sha256:
         fetchurl {
           url = "mirror://gnu/readline/readline-6.3-patches/readline63-${nr}";
           inherit sha256;
         };
     in
       import ./readline-6.3-patches.nix patch);
  # ...
}

There's two solutions which don't really work well:

  • It's inconvenient for them to stay behind in pkgs/development/libraries/readline. This requires the files in pkgs/auto to use a long reference like ../../development/libraries/readline. Also the ad-hoc categorization is not removed like that
  • It doesn't make sense to move these patchfiles into any of pkgs/auto/{readline63,readline70,readline81}, because these are shared files between them

Instead I believe the best solution for this problem is to create pkgs/auto/readline without a default.nix file and move the patchfiles there. While auto-calling that directory would fail, it won't be evaluated because we have readline = readline6 in all-packages.nix which overrides the auto-called version. This then allows the references to these files to just use e.g. ../readline/link-against-ncurses.patch

@infinisil
Copy link
Member Author

Catch: This only works when the generic attribute (here readline) exists as an attribute already. I just discovered the docbook_xml_dtd_* attributes, which don't have any docbook_xml_dtd alias:

{
  docbook_xml_dtd_412 = callPackage ../data/sgml+xml/schemas/xml-dtd/docbook/4.1.2.nix { };
  docbook_xml_dtd_42 = callPackage ../data/sgml+xml/schemas/xml-dtd/docbook/4.2.nix { };
  docbook_xml_dtd_43 = callPackage ../data/sgml+xml/schemas/xml-dtd/docbook/4.3.nix { };
  docbook_xml_dtd_44 = callPackage ../data/sgml+xml/schemas/xml-dtd/docbook/4.4.nix { };
  docbook_xml_dtd_45 = callPackage ../data/sgml+xml/schemas/xml-dtd/docbook/4.5.nix { };
}

And they have a single generic shared file between them, pkgs/data/sgml+xml/schemas/xml-dtd/docbook/generic.nix.

Not yet sure how to resolve this. Maybe one of the non-ideal solutions mentioned earlier would be okay here

@blaggacao
Copy link
Contributor

@infinisil have you thought about a possible broader structured approach to version matrices that can be generalized?

It maybe, though, something for a later iteration of the NAT.

@infinisil
Copy link
Member Author

@blaggacao Yeah, that's for a future iteration but definitely in scope :)

@infinisil
Copy link
Member Author

In the last meeting we decided to just not move any files that require updating file references: #20 (review), so I think this can be closed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants