pypa / installer

A low-level library for installing from a Python wheel distribution.
https://installer.readthedocs.io/
MIT License
124 stars 51 forks source link

FileExistsError on package reinstall/update #121

Closed jameshilliard closed 2 years ago

jameshilliard commented 2 years ago

Some of our development workflows typically expect that one can upgrade or reinstall a package installed in site-packages, however when one tries to do this with installer we hit a FileExistsError error.

For example:

>>> host-python-tomli 2.0.1 Installing to host directory
(cd /home/buildroot/buildroot/output/build/host-python-tomli-2.0.1//; PATH="/home/buildroot/buildroot/output/host/bin:/home/buildroot/buildroot/output/host/sbin:/home/buildroot/bin:/home/buildroot/.local/bin:/home/buildroot/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin" PYTHONNOUSERSITE=1 PATH="/home/buildroot/buildroot/output/host/bin:/home/buildroot/buildroot/output/host/sbin:/home/buildroot/bin:/home/buildroot/.local/bin:/home/buildroot/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin" PKG_CONFIG="/home/buildroot/buildroot/output/host/bin/pkg-config" PKG_CONFIG_SYSROOT_DIR="/" PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 PKG_CONFIG_ALLOW_SYSTEM_LIBS=1 PKG_CONFIG_LIBDIR="/home/buildroot/buildroot/output/host/lib/pkgconfig:/home/buildroot/buildroot/output/host/share/pkgconfig" AR="/usr/bin/ar" AS="/usr/bin/as" LD="/usr/bin/ld" NM="/usr/bin/nm" CC="/usr/bin/gcc" GCC="/usr/bin/gcc" CXX="/usr/bin/g++" CPP="/usr/bin/cpp" OBJCOPY="/usr/bin/objcopy" RANLIB="/usr/bin/ranlib" CPPFLAGS="-I/home/buildroot/buildroot/output/host/include" CFLAGS="-O2 -I/home/buildroot/buildroot/output/host/include" CXXFLAGS="-O2 -I/home/buildroot/buildroot/output/host/include" LDFLAGS="-L/home/buildroot/buildroot/output/host/lib -Wl,-rpath,/home/buildroot/buildroot/output/host/lib" INTLTOOL_PERL=/usr/bin/perl  /home/buildroot/buildroot/output/host/bin/python -m installer dist/* )
Traceback (most recent call last):
  File "/home/buildroot/buildroot/output/host/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/buildroot/buildroot/output/host/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/buildroot/buildroot/output/host/lib/python3.10/site-packages/installer/__main__.py", line 85, in <module>
    _main(sys.argv[1:], "python -m installer")
  File "/home/buildroot/buildroot/output/host/lib/python3.10/site-packages/installer/__main__.py", line 81, in _main
    installer.install(source, destination, {})
  File "/home/buildroot/buildroot/output/host/lib/python3.10/site-packages/installer/_core.py", line 109, in install
    record = destination.write_file(
  File "/home/buildroot/buildroot/output/host/lib/python3.10/site-packages/installer/destinations.py", line 207, in write_file
    return self.write_to_fs(scheme, path_, stream, is_executable)
  File "/home/buildroot/buildroot/output/host/lib/python3.10/site-packages/installer/destinations.py", line 167, in write_to_fs
    raise FileExistsError(message)
FileExistsError: File already exists: /home/buildroot/buildroot/output/host/lib/python3.10/site-packages/tomli/__init__.py

I'm not sure what the correct way to handle this is, I assume the old version should be automatically uninstalled(or there should be a flag to enable this behavior) and then the new version installed if there is an existing installation present?

pradyunsg commented 2 years ago

This library has no logic for handling uninstalls or upgrades. If you need that, you’ll need to use something else that implements uninstallation logic based on reading the RECORD file (eg: pip).

Implementing uninstallation logic is something I’d consider out of scope for a project called “installer”. :)

jameshilliard commented 2 years ago

If you need that, you’ll need to use something else that implements uninstallation logic based on reading the RECORD file (eg: pip).

Hmm, does anything actually implement that which might be usable(we can't use pip due to lack of cross compile compatibility)?

Implementing uninstallation logic is something I’d consider out of scope for a project called “installer”. :)

Would overwrite logic be simpler than full uninstallation logic(maybe bypassing the file existence checks)?

Since installer is part of the early host toolchain bootstrap process I'm also having trouble seeing how this could be implemented externally in a different project in a useable way as installer would be needed to install a package like that which would then create a dependency cycle effectively.

uranusjr commented 2 years ago

Would overwrite logic be simpler than full uninstallation logic (maybe bypassing the file existence checks)?

No because metadata cannot be naively overwritten without consistency issues; a full uninstall-install combination is better (and is how pip does upgrade). An uninstall tool would be best for this, I think.

jameshilliard commented 2 years ago

No because metadata cannot be naively overwritten without consistency issues; a full uninstall-install combination is better (and is how pip does upgrade).

How does setuptools do it when invoked directly using a setup.py install, that's what we've been using for distutils and setuptools packages which seems to work ok?

An uninstall tool would be best for this, I think.

Something like that might need to be vendored in installer since it would presumably depend on installer for installation itself.

uranusjr commented 2 years ago

How does setuptools do it when invoked directly using a setup.py install, that's what we've been using for distutils and setuptools packages which seems to work ok?

If I remember correctly (without checking) it naively overwrites. This works for setuptools because its metadata format is much simpler (and lacks of features) and more likely to be correctly overwritten (still not guaranteed to work, but nice to hear it works for you). The modern metadata format (.dist-info) contains a bit more pieces that might not work as well with this.

pradyunsg commented 2 years ago

Something like that might need to be vendored in installer since it would presumably depend on installer for installation itself.

Any such reinstall and upgrade logic belongs in higher level tools like pip, not a library for unpacking wheel files.

jameshilliard commented 2 years ago

This works for setuptools because its metadata format is much simpler (and lacks of features) and more likely to be correctly overwritten (still not guaranteed to work, but nice to hear it works for you). The modern metadata format (.dist-info) contains a bit more pieces that might not work as well with this.

I mean, as long as it works in most cases I think it's fine as we document this sort of thing in general as something that can break under certain circumstances.

Any such reinstall and upgrade logic belongs in higher level tools like pip, not a library for unpacking wheel files.

I mean, I'm not sure how this would work due to the dependency ordering restrictions, the stage in which we install and/or upgrade installer depends only on flit_core(which can only install itself and not any other packages) being installed. So we effectively have to be able to upgrade installer with nothing other than flit_core or installer itself since no other packages can be installed before installer is installed other than flit_core effectively.

pradyunsg commented 2 years ago

I’m not familiar with your workflow/project, but I can tell you that installer is not going to grow any kind of uninstall, reinstall or upgrade semantics. If you need those, you can get those from pip today.

Beyond that, if you want to avoid using pip, then my only suggestion is to avoid trying to upgrade in-place, and instead use a clean environment to populate with installer. If that doesn’t work, then please feel welcome to write a higher level tool that uses installer’s API to install the wheel and implements the uninstall logic somehow.

pradyunsg commented 2 years ago

Finally, if you want to mimic the “ignore and just overwrite” semantics, please feel welcome to use the API and overwrite the write_to_fs method in a subclass. If things break though, you get to keep all the broken pieces.

I’m gonna go ahead and close this now, since I don’t think any of what’s being requested here is in-scope for this library. I understand that it would be useful to you but this project isn’t meant to be a full fledged package manager — only a library for unpacking wheel files.

jameshilliard commented 2 years ago

If you need those, you can get those from pip today.

AFAIU not possible due to lack of cross compilation support.

my only suggestion is to avoid trying to upgrade in-place, and instead use a clean environment to populate with installer

We have some build modes that sort of do this but they are experimental and we still want to support non-isolated installs.

If that doesn’t work, then please feel welcome to write a higher level tool that uses installer’s API to install the wheel and implements the uninstall logic somehow.

So this would have to vendor installer to be usable I guess? It looks problematic due to dependency cycle issues there as installer and a hypothetical package like this would have a circular dependency on each other otherwise.

Finally, if you want to mimic the “ignore and just overwrite” semantics, please feel welcome to use the API and overwrite the write_to_fs method in a subclass.

Might be good enough.

I understand that it would be useful to you but this project isn’t meant to be a full fledged package manager — only a library for unpacking wheel files.

Def not looking for installer to be a full fledged package manager(those tend to not play nice with our infrastructure), just looking for setuptools style upgrade/overwrite support of some sort. I mean virtually all build systems with installation capability have some sort of basic file overwrite/replace support in general.

greyltc commented 1 year ago

@jameshilliard I made an uninstaller: https://github.com/greyltc-org/uninstaller
you could run that before you run installer to clean things up and prevent file collisions

jameshilliard commented 1 year ago

you could run that before you run installer to clean things up and prevent file collisions

Not really possible since uninstaller indirectly depends on installer for installation itself which would effectively result in a circular package dependency. I just added a clean function to our installer wrapper script which seems to work as we don't treat internal buildroot scripts as package dependencies.

eli-schwartz commented 1 year ago

How does buildroot handle generalized uninstallation of recipes, if at all? Does it use $DESTDIR at all?

jameshilliard commented 1 year ago

How does buildroot handle generalized uninstallation of recipes, if at all?

In general we don't normally support package uninstallation(we aren't a normal distro with stuff like prebuilt binary package management like deb/rpm/opkg).

If using top level parallel which provides additional isolation we can do something similar to an uninstall via make <package>-dirclean which removes the isolated build and install tree for a package(there are cases that aren't handled fully here however).

The clean function however is needed for say a package rebuild/update(ie our make <package>-rebuild) especially when not using top level parallel and to in general avoid errors in cases when trying to install a package that was is already installed. We pretty much just need package overwrite/update support and not generalized package uninstallation support.

greyltc commented 1 year ago

uninstaller indirectly depends on installer

uninstaller does not depend on installer (directly or indirectly) as far as I'm aware (I do use it in tests though). If it does somehow let me know, that's my mistake and I'll fix it.

$ curl https://codeload.github.com/greyltc-org/uninstaller/tar.gz/refs/tags/v0.0.2 | bsdtar -x -
$ PYTHONPATH=uninstaller-0.0.2/src python -m uninstaller --help
usage: python -m uninstaller [-h] [--root path] [--base path] [--scheme scheme] [--not-pure-python] [--ignore-checksums] [--ignore-sizes] [--verbose] package [package ...]

uninstall python packages

positional arguments:
  package               name of the package to uninstall

options:
  -h, --help            show this help message and exit
  --root path, -r path  override package search root
  --base path, -b path  override base path (aka prefix)
  --scheme scheme, -s scheme
                        override the default installation scheme
  --not-pure-python, -n
                        you might need to use this if 'Root-is-Purelib' metadata parameter of the package you want to uninstall is false
  --ignore-checksums, -i
                        use this to skip checksum verification ☠️DANGEROUS☠️
  --ignore-sizes, -z    use this to skip file size verification ☠️DANGEROUS☠️
  --verbose, -v         print every file that's removed to stdout
jameshilliard commented 1 year ago

uninstaller does not depend on installer (directly or indirectly) as far as I'm aware (I do use it in tests though). If it does somehow let me know, that's my mistake and I'll fix it.

To install uninstaller one needs a pep517 toolchain and hatchling(a pep517 build backend), in our case we use installer for installing our pep517 packages and thus any package using pep517 must effectively depend on installer.

PYTHONPATH=uninstaller-0.0.2/src

We really don't want to be doing path hacks like this.

greyltc commented 1 year ago

We really don't want to be doing path hacks like this.

Of course you could get uninstaller into your sys.path by whatever means you're comfortable with. If via installer is the only way you're willing to get it into you your sys.path and that choice causes circular dependency in your workflow, then that's pretty confusing! :-)

Does installer have a circular dependency on itself then?

pradyunsg commented 1 year ago

Well, switching the build-backend to flit in uninstaller should be all that's needed to resolve the concern around bootstrapping that @jameshilliard is raising IIUC (that's the same build-backend as installer).

PS: I love how cleanly the naming works. :)

greyltc commented 1 year ago

Ah ha! Do you have any desire to switch installer to hatchling? installer/uninstaller should definitely have matching backends.

eli-schwartz commented 1 year ago

Why don't you change to flit instead?

greyltc commented 1 year ago

I'll switch uninstaller to build with flit if pradyunsg isn't interested in switching installer to hatchling (or accepting a PR for me to do it). I've got no technical reason for preferring hatchling over flit. The only rational I've got is that the official packaging tutorial uses hatchling by default and I'm getting annoyed by all the different backends/packaging tools flying around lately, so I'm trying to standardize the workflows for my projects on whatever is likely to have the best support going forward.

I think installer & uninstaller having matching build backends is more important than all that though.

eli-schwartz commented 1 year ago

Having matching backends for installer and uninstaller is a pretty low-priority goal, coming after "having installer be really easy to bootstrap from source". To the point: installer depends on flit_core and nothing else, while hatchling directly depends on 6 different packages and indirectly depends on flit_core as well (and that's just the indirect dependencies I know by heart).

What advantages does hatchling confer? Installer chose flit because flit's fundamental goal is to handle the simple cases so well that it's really simple to use it. :) (As evidenced by the fact that it doesn't need a long tail of dependencies.)

Choosing hatchling because it what, came first in an unsorted list? seems like an odd prioritization.

Why is it even bad to have different packaging tools "flying around lately"? Is this different from the C/C++ world where you can choose between autotools, meson, cmake, waf, scons, premake, gyp, gn, bazel, buck, pants, Visual Studio, xmake, qmake, Makefile, Gradle, build2, b2, and more? Different build systems exist because people have different needs. If some of those can be criticized, it's not because of the number that exist, it's because the build system in question just has a bad design.

If your criterion is "whatever is likely to have the best support going forward" then why choose whatever's first on an unsorted list? Maybe you should choose one that is maintained by PyPA. Maybe choose the oldest one around, as clearly setuptools is too important to ever die under any circumstances ever. :)

greyltc commented 1 year ago

You've educated me. I'll switch uninstaller to flit!

pradyunsg commented 1 year ago

Another reason (and why installer won't move away from flit) is that flit has a bootstrapping story for redistributors who want to build-everything-from-source: https://flit.pypa.io/en/stable/bootstrap.html

greyltc commented 1 year ago

I had no idea how much better flit was for bootstrapping. I'm happy to have a good technical reason to switch uninstaller to flit.

jameshilliard commented 1 year ago

If via installer is the only way you're willing to get it into you your sys.path and that choice causes circular dependency in your workflow, then that's pretty confusing! :-)

We have strict dependency ordering requirements that effectively prevent path hacking package dependencies.

Does installer have a circular dependency on itself then?

Kind of, but it's not really an issue in that case as our no circular dependency requirement still allows for packages to depend on themselves, for example a package can path hack itself(for example installer can install itself without violating the no circular dependencies requirement), we can't path hack package dependencies in the way you're suggesting.

Well, switching the build-backend to flit in uninstaller should be all that's needed to resolve the concern around bootstrapping that @jameshilliard is raising IIUC (that's the same build-backend as installer).

PS: I love how cleanly the naming works. :)

Wouldn't really help much since uninstaller would still effectively violate our strict no circular dependencies requirement.

Another reason (and why installer won't move away from flit) is that flit has a bootstrapping story for redistributors who want to build-everything-from-source: https://flit.pypa.io/en/stable/bootstrap.html

Yep, we rely on this for bootstrapping our pep517 toolchain.

In any case I still don't see how I can integrate something like uninstaller into buildroot(without a lot of unmaintainable hacks at least) due to the circular dependency issue.

Maybe you should choose one that is maintained by PyPA.

Well...technically hatchling is also maintained under pypa as well. I recently added support for it to buildroot which was easy enough as it didn't introduce any new toolchain bootstrapping issues. It's not super common from the looks of it but there are a few popular python packages that started using it recently. I'm not familiar with the advantages/disadvantages to it in general but we definitely wouldn't want any packages needed for bootstrapping our pep517 toolchain using hatchling at all as that would make things a lot more complex.