CLI to install to Python running installer

[takluyver]

This starts from #66, and moves the added functionality for destdir & bytecode compilation into the Python API (SchemeDictionaryDestination), so the CLI is a thinner wrapper over the Python API. It also drops the compatibility checks, so installer once again has no dependencies.

Alternative to #92, for consideration.
Closes #93

[pradyunsg]

Two major concerns:

• Making the compilation opt-out on the CLI.
• Ensuring that the destdir="/" works on Windows. It might be a good idea to have it default to None instead.

[takluyver]

I agree, I don't think / would do the right thing on Windows, so I've made None the default.

For bytecode compilation, you can technically tell it not to by passing --optimize with no levels. Obviously that's a pretty back-to-front interface, though.

Do you want bytecode generation to be on by default, with separate arguments to disable it (--no-compile-bytecode?) and to control the optimisation levels? Or off by default, and you specify e.g. --compile-bytecode 0 1 to enable it? Maybe allowing a plain --compile-bytecode with no value to enable it for level 0?

[pradyunsg]

How about:

• no-args: compile with the default level (i.e. optimize=-1).
• --no-compile-bytecode: don't compile.
• --compile-bytecode 0: compile with level 0 only.
• --compile-bytecode 1 --compile-bytecode 2: compile with level 1 and level 2.
• --compile-bytecode 0 --compile-bytecode 1 --compile-bytecode 2: compile with level 0, level 1 and level 2.

Have the metavar be level, mapping to "optimisation level". Only take 0, 1, 2 as valid values.

[takluyver]

I think that should implement what you suggested. I still need to get to writing some tests.

[pradyunsg]

Apparently I have review comments that I haven't posted?

[pradyunsg]

I like your tweak better! :)

[FRidh]

I gave this a try in Nixpkgs, and it seems the path to the interpreter is always prefixed.

What we get

/nix/store/9pi9l6c3g8hmmgv656zjfkh7hwwxwd5m-python3.9-bootstrapped-build-installer/nix/store/dn4fwp0yx6nsa85cr20cwvdmg64xwmcy-python3-3.9.9/lib/python3.9/site-packages/flit_core/buildapi.py

What we should have

/nix/store/9pi9l6c3g8hmmgv656zjfkh7hwwxwd5m-python3.9-bootstrapped-build-installer/lib/python3.9/site-packages/flit_core/buildapi.py

Building wheels is done with

${python.pythonForBuild.interpreter} -m build --wheel "$pkg" --no-isolation --skip-dependency-check

and installing using

${python.pythonForBuild.interpreter} -m installer "$whl" --destdir $out --compile-bytecode "0"

(NixOS/nixpkgs#105714)

[eli-schwartz]

My understanding is that NixOS specifically does not use DESTDIR because every package already has its own unique PREFIX. No idea how you handle sysconfig paths though (which contain the PREFIX).

[FRidh]

My understanding is that NixOS specifically does not use DESTDIR because every package already has its own unique PREFIX. No idea how you handle sysconfig paths though (which contain the PREFIX).

Correct, every package has a prefix. Without setting --destdir it wants to install to

/nix/store/dn4fwp0yx6nsa85cr20cwvdmg64xwmcy-python3-3.9.9/bin/pyproject-build

Here it indeed takes the prefix corresponding to the interpreter, and that is not OK for us.

With pip we use

${python.pythonForBuild.interpreter} -m pip install --no-build-isolation --no-index --prefix=$out  --ignore-installed --no-dependencies --no-cache .

What we need is like other build systems the ability to set a --prefix.

[takluyver]

To be 100% clear, --destdir is equivalent to the --root option in pip, not --prefix. The behaviour you show looks correct for that option (though I understand it's not the behaviour you want).

I floated the idea of a prefix option on #66 (the predecessor to this PR), but @FFY00 and @eli-schwartz argued strongly against it. I get the sense they've spent more time than me thinking about these things, so I deferred to them. But I can't see a way to achieve what you want without a prefix option, short of messing around with sysconfig (which seems like a workaround to me).

One other way to approach this could be through the lower-level 'give us explicit target paths' interface (as in #93), and leave it to you to construct paths like $out/lib/python3.10/site-packages.

@pradyunsg any preferences?

[FRidh]

To be 100% clear, --destdir is equivalent to the --root option in pip, not --prefix. The behaviour you show looks correct for that option (though I understand it's not the behaviour you want).

Right.

I floated the idea of a prefix option on #66 (the predecessor to this PR), but @FFY00 and @eli-schwartz argued strongly against it. I get the sense they've spent more time than me thinking about these things, so I deferred to them. But I can't see a way to achieve what you want without a prefix option, short of messing around with sysconfig (which seems like a workaround to me).

My understanding is that @FFY00 indeed suggests adding a sysconfig scheme:

Which demonstrates the limitations of overwriting the prefix, and would be require a different tool like the one I described above. But if spack is the one shipping the Python interpreter, I would recommend adding a sysconfig scheme instead sweat_smile

Adding a scheme might be a possibility. We now use the posix_prefix scheme as is. If we would make a custom scheme, this scheme would be the posix_prefix minus the path to the interpreter that ours has. E.g., we now get when running from a "Nixpkgs buildenv" environment (sort of like a venv but then not limited to just Python):

{'stdlib': '/nix/store/dn4fwp0yx6nsa85cr20cwvdmg64xwmcy-python3-3.9.9/lib/python3.9',
 'platstdlib': '/nix/store/hidc6byr4gdxpxckkc4a8s9fbvjl05jy-python3-3.9.9-env/lib/python3.9',
 'purelib': '/nix/store/hidc6byr4gdxpxckkc4a8s9fbvjl05jy-python3-3.9.9-env/lib/python3.9/site-packages',
 'platlib': '/nix/store/hidc6byr4gdxpxckkc4a8s9fbvjl05jy-python3-3.9.9-env/lib/python3.9/site-packages',
 'include': '/nix/store/dn4fwp0yx6nsa85cr20cwvdmg64xwmcy-python3-3.9.9/include/python3.9',
 'platinclude': '/nix/store/dn4fwp0yx6nsa85cr20cwvdmg64xwmcy-python3-3.9.9/include/python3.9',
 'scripts': '/nix/store/hidc6byr4gdxpxckkc4a8s9fbvjl05jy-python3-3.9.9-env/bin',
 'data': '/nix/store/hidc6byr4gdxpxckkc4a8s9fbvjl05jy-python3-3.9.9-env'}

Just to compare, a virtualenv, constructed from that env, looks like

>>> pprint.pprint(sysconfig.get_paths())
{'data': '/tmp/venv/env',
 'include': '/nix/store/dn4fwp0yx6nsa85cr20cwvdmg64xwmcy-python3-3.9.9/include/python3.9',
 'platinclude': '/nix/store/dn4fwp0yx6nsa85cr20cwvdmg64xwmcy-python3-3.9.9/include/python3.9',
 'platlib': '/tmp/venv/env/lib/python3.9/site-packages',
 'platstdlib': '/tmp/venv/env/lib/python3.9',
 'purelib': '/tmp/venv/env/lib/python3.9/site-packages',
 'scripts': '/tmp/venv/env/bin',
 'stdlib': '/nix/store/dn4fwp0yx6nsa85cr20cwvdmg64xwmcy-python3-3.9.9/lib/python3.9'}
>>>

I don't know how the interpreter would behave though if we would make this custom prefix the default prefix. It would run with something looking like

{'stdlib': 'lib/python3.9',
 'platstdlib': 'lib/python3.9',
 'purelib': 'python3.9/site-packages',
 'platlib': 'site-packages',
 'include': 'include/python3.9',
 'platinclude': 'include/python3.9',
 'scripts': 'bin',
 'data': ''}

but that would then have to use --destdir which has a different purpose and I am quite sure will result in breakage.

[jameshilliard]

I floated the idea of a prefix option on #66 (the predecessor to this PR), but @FFY00 and @eli-schwartz argued strongly against it. I get the sense they've spent more time than me thinking about these things, so I deferred to them.

Since installer needs to be able to install itself then it can't depend on any packages that would have installer in their own dependency tree, otherwise you would have a dependency cycle.

So IMO installer really should provide enough cli functionality to install itself into the desired locations without any external dependencies(other than flit_core which would be able to install itself in #511).

[mgorny]

I've just gotten a report that the install doesn't make scripts (as in normal scripts, not entry points) executable. I'm not sure if this is something to tackle as part of this PR or separately (#90 might be relevant).

Example wheel: https://files.pythonhosted.org/packages/91/8b/fa0a7295cfb6c81586a2ec0d57ef3e8dc8c425f11726f351f13bcf523cda/pkgcore-0.12.9-py3-none-any.whl

[takluyver]

It should install the script files to the correct location, but I don't think it treats them specially at all - rewriting shebangs, setting the execute bit for Unix, or making exe wrappers for Windows. Entry points became the de facto standard for installing executable commands, so the old mechanism to include script files doesn't get much love (see e.g. pypa/pip#2596 ).

The wheel spec is silent, as far as I can see, on whether it's the responsibility of the code building the wheel to set the executable bit, or whether the code installing it should make anything in the scripts directory executable.

In any case, any special handling for script files is orthogonal to adding a CLI (the existing Python API has the same limitation), so I think it should be addressed separately.

[takluyver]

(My mistake, I see it does have special handling for setting the shebang. In that case, I suspect #90 will fix the problem you describe)

[pradyunsg]

#90 is the right thing for that. :)

[mgorny]

@takluyver, could you update your patch following the changes in main? Not sure if rebasing or merging is the right thing for this repo.

[takluyver]

I've merged master in for now - Github makes that easy. @pradyunsg I can rebase it if you prefer.

[mgorny]

Thank you. I suppose I'll just switch Gentoo to your fork rather than trying to apply this PR as a patch ;-).

[pradyunsg]

I'm gonna squash-merge this one, since it's decently atomic on the whole and I don't find it particularly interesting to have the entire history of the evolution of this PR, recorded on main.

[pradyunsg]

Thanks @takluyver for taking this over the line, @FFY00 for starting us down this path and everyone else for engaging in this discussion! ^>^

[takluyver]

Thanks! Just to link it here, I see @FRidh has opened #98 for further discussion about the idea of a prefix option.