Makefile: installer respecting --prefix breaks assumptions in existing packages

[MattSturgeon]

In #343 I "fixed" the caller to python -m installer to respect --prefix and --destdir when set.

This fixed some issues we had packaging umu-launcher in nixpkgs, however it has also changed the makefile's behaviour breaking assumptions for (all?) other packaging efforts.

I'm not familiar enough with umu or python to know exactly what this installation step is doing, how PREFIX, DESTDIR, and PYTHONDIR fit into the picture, etc.

Nixpkgs can be a bit confusing when compared to other packaging systems. The package's files should be installed into the prefix $out instead of the typical /usr. This allows every package to have its own isolated prefix.

Note that $out is the actual prefix installed to on the system. It is not a DESTDIR that should be treated as a fs-root.

The in-tree nix package (/packaging/nix/) seems to have several workarounds to move files into the correct location. The nixpkgs package however attempted to use PREFIX and PYTHONDIR to tell the makefile to install to the correct location without any additional steps.

https://github.com/NixOS/nixpkgs/blob/9e1c4ffd677cdd1bfed9a84a80948f6c751b70ac/pkgs/by-name/um/umu-launcher-unwrapped/package.nix#L49-L55

makeFlags = [

   # This line:
  "PYTHONDIR=$(PREFIX)/${python3Packages.python.sitePackages}"

  "PYTHON_INTERPRETER=${lib.getExe python3Packages.python}"
  # Override RELEASEDIR to avoid running `git describe`
  "RELEASEDIR=${pname}-${version}"
  "SHELL_INTERPRETER=${lib.getExe bash}"
];

Is nesting PYTHONDIR below PREFIX normal or abnormal when packaging python apps?

Originally reported as #317 (comment)
Some additional discussion in that thread

cc @R1kaB3rN @beh-10257 @Hexa @natsukium

[MattSturgeon]

@nullcub3

since the PREFIX variable is already set(?), then with the conditions added in #343 it will always do INSTALLER_ARGS += --prefix=$(PREFIX), which wasn't the behavior before

You're right. This makes me think the old behaviour was probably wrong, but all packaging efforts must have been working around the behaviour.

On the other hand, maybe this installation step is targeting PYTHONDIR which is assumed to be outside of the PREFIX?

I'm not familiar enough with python or this project to say confidently 😅

[MattSturgeon]

For the in-tree nix package specifically, this can be handled by #347. IDK if this also affects other packaging efforts though.

[R1kaB3rN]

To clarify, where exactly are you seeing the behaviour change and assumptions breaking as result of your pull request that directly affect other packaging efforts? Because I can confirm that neither the Ubuntu, Debian, nor RPM artifacts are affected from your change. Unless I am missing something, they are functional and the directory structure within the Python site packages directory are still what's expected.

Also, nesting PYTHONDIR below PREFIX is abnormal and the Makefile in the repo does not do that.

[MattSturgeon]

To clarify, where exactly are you seeing the behaviour change and assumptions breaking as result of your pull request that directly affect other packaging efforts?

I've only observed it in the in-tree flake package. Whether other packages are affected is pure speculation and uncertainty on my part.

Because I can confirm that neither the Ubuntu, Debian, nor RPM artifacts are affected from your change.

That's reassuring, thanks for checking!

Also, nesting PYTHONDIR below PREFIX is abnormal and the Makefile in the repo does not do that.

Thanks for the context. Due to the nature of how nix packages are installed, everything must be prefixed in this package's nix-store path ($out), which will look something like /nix/store/somehash-umu-launcher-1.1.4/

We can achieve that by setting the make-variable PYTHONDIR=$(PREFIX)/${sitePackages}, so long as the makefile can support this without other packages having issues.

The issues with the in-tree flake package turned out to be trivial install path collisions, resolved by #347. If no other packages are affected, then that pr fixes this issue.

[R1kaB3rN]

Closing as #347 was merged that fixes the install path collisions.