Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

__package_pip: explorer/state breaks when --requirement is not package name #45

Open
4nd3r opened this issue Apr 27, 2022 · 3 comments · May be fixed by #46
Open

__package_pip: explorer/state breaks when --requirement is not package name #45

4nd3r opened this issue Apr 27, 2022 · 3 comments · May be fixed by #46
Assignees
@sideeffect42
Labels
bug Something isn't working

Comments

@4nd3r
Copy link
Member

4nd3r commented Apr 27, 2022 •
edited
Loading 

ander@t490s:~$ pip list | grep mkosi
mkosi               12
ander@t490s:~$ echo __package_pip mkosi --requirement git+https://github.com/systemd/mkosi.git | cdist config -i - localhost
INFO: [30773]: localhost: Starting configuration run
INFO: [30773]: localhost: Processing __package_pip/mkosi
INFO: [30773]: localhost: Finished successful run in 6.26 seconds
ander@t490s:~$ cdist-dump localhost | grep mkosi
./messages:__package_pip/mkosi:installed
./object/__package_pip/mkosi/.cdist-akj58qn2/parameter/state:present
./object/__package_pip/mkosi/.cdist-akj58qn2/parameter/requirement:git+https://github.com/systemd/mkosi.git
./object/__package_pip/mkosi/.cdist-akj58qn2/explorer/state:absent
./object/__package_pip/mkosi/.cdist-akj58qn2/explorer/requirement:git+https://github.com/systemd/mkosi.git
./object/__package_pip/mkosi/.cdist-akj58qn2/explorer/pip:/usr/bin/pip3
./object/__package_pip/mkosi/.cdist-akj58qn2/code-remote:/usr/bin/pip3 install -q 'git+https://github.com/systemd/mkosi.git'
./object/__package_pip/mkosi/.cdist-akj58qn2/stderr/code-remote:WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv

probably because of this:

pip_python -c '__import__("pkg_resources").require(__import__("sys").argv[1])' \
    "${requirement}" >/dev/null 2>&1 \
&& echo present \
|| echo absent

somehow it went unnoticed in #37 review 😟
@4nd3r 4nd3r added the bug Something isn't working label Apr 27, 2022
@sideeffect42
Copy link
Member

sideeffect42 commented Apr 27, 2022 •
edited
Loading

Hmm, it should work when the requirement contains the package name (__package_pip mkosi --requirement 'mkosi@git+https://github.com/systemd/mkosi.git').

"Installed" detection is a mess when using URLs.
pip install will also happily reinstall every time when given a URL while it will say "Requirement already satisfied" when given a name.

The pkg_resources check works with mkosi@ but it won't catch if e.g. the @branch has changed in the URL.

Update: I did some more testing and we should probably not feed URLs to pkg_resources.require().
So my suggestion would be to try to detect URLs (how?) and if a URL was given, use the __object_id instead.
Switching branches or doing upgrades by changing Git tags or something won't work this way, though.

Does pip even track the installation URL?

@4nd3r
Copy link
Member Author

4nd3r commented Apr 27, 2022

The pkg_resources check works with mkosi@ but it won't catch if e.g. the @Branch has changed in the URL.

If this technical limitation is documented, then it's fine.

So my suggestion would be to try to detect URLs

Only support __package_pip mkosi --requirement 'mkosi@git+https://github.com/systemd/mkosi.git' format and split it by @ with caveat that this way only first time install is possible.

Or does pip support something like foo[bar,baz]@git+https://github.com/foo/bar?

@sideeffect42
Copy link
Member

Only support __package_pip mkosi --requirement 'mkosi@git+https://github.com/systemd/mkosi.git' format and split it by @ with caveat that this way only first time install is possible.

I did some research and I'm not so sure about the URL parsing anymore.

The PEP508-based pkg@url syntax is only supported by Pip >= 10.0.0b1 (2018-03-31).
e.g. Ubuntu 18.04 ships Pip 9.0.1 as python3-pip, so it wouldn't work there.

Or does pip support something like foo[bar,baz]@git+https://github.com/foo/bar?

Yes, it does.

@sideeffect42 sideeffect42 linked a pull request Apr 28, 2022 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sideeffect42 sideeffect42

Labels
bug Something isn't working
Milestone
No milestone
2 participants
@sideeffect42 @4nd3r