https://bugzilla.redhat.com/show_bug.cgi?id=1720757
Bug ID: 1720757 Summary: Review Request: pew - Tool to manage multiple virtualenvs written in pure Python Product: Fedora Version: rawhide Status: NEW Component: Package Review Assignee: nobody@fedoraproject.org Reporter: tadej.j@nez.si QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://raw.githubusercontent.com/tjanez/pew-package/58936dffd15627cae0012f6... SRPM URL: https://tadej.fedorapeople.org/pew-1.2.0-1.fc31.src.rpm Description: Python Env Wrapper is a set of commands to manage multiple virtual environments. Pew can create, delete and copy your environments, using a single command to switch to them wherever you are, while keeping them in a single (configurable) location. Fedora Account System Username: tadej
This is a re-review request to unretire this package. pew was originally retired because it was orphaned for more than 6 weeks on Dec 24, 2018: https://src.fedoraproject.org/rpms/pew/c/ca9ed68f8ca88ec93b06039f2cc7cf2873a....
Besides un-retiring, I've also did the following: - Update to 1.2.0 release - Drop the tests-connection-marker-fix patch since it has been upstreamed - Remove Python version management functionality in Fedora 30+ - Use automatic Python dependency generator
The whole diff can be seen here: https://github.com/tjanez/pew-package/compare/a019779...review
Scratch builds: - rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=35544669 - f30: https://koji.fedoraproject.org/koji/taskinfo?taskID=35544665 - f29: https://koji.fedoraproject.org/koji/taskinfo?taskID=35544667
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |POST CC| |zbyszek@in.waw.pl Assignee|nobody@fedoraproject.org |zbyszek@in.waw.pl Flags| |fedora-review+
--- Comment #1 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- I don't think you need -S git and BR:git-core. Patches formatted by git apply without trouble by default.
export PYTHONPATH=$PYTHONPATH:%{buildroot}%{python3_sitelib}
That's wrong, surprisingly. This will evaluate to PTYHONPATH=:/path/to/buildroot/usr/lib/python3.7/site-packages, and an empty component in path has special meaning of cwd. I think it's better/safer to write this as:
PATH=%{buildroot}%{_bindir}:$PATH \ PYTHONPATH=%{buildroot}%{python3_sitelib} \ %__python3 -m pytest -v tests
Checklist: + package name is OK (it's an application) + latest version + builds and installs OK + license is acceptable for Fedora (MIT) + license is specified correctly + BR/R/P look OK + %python_provide macro is used + rpmlint shows nothing important
Package is APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #2 from Tadej Janež tadej.j@nez.si --- Zbigniew,
thanks for a quick review!
I don't think you need -S git and BR:git-core. Patches formatted by git apply without trouble by default.
I tried using %autosetup without -S git as you suggested but it didn't work:
- /usr/bin/cat /builddir/build/SOURCES/0001-Remove-Python-version-management-on-Fedora.patch
- /usr/bin/patch -s --fuzz=0 --no-backup-if-mismatch
BUILDSTDERR: error: Bad exit status from /var/tmp/rpm-tmp.3WUx03 (%prep) The text leading up to this was:
|From e43b1f4a04e3b5ce841a0dbb125bc87fc330bc13 Mon Sep 17 00:00:00 2001 |From: =?UTF-8?q?Tadej=20Jane=C5=BE?= tadej.j@nez.si |Date: Wed, 12 Jun 2019 23:28:13 +0200 |Subject: [PATCH] Remove Python version management on Fedora | |This removes the pythonz-bd dependency which is not available in Fedora |anymore. |Furthermore, there is strong support upstream to either remove Pew's |Python version management or replace it with pyenv: |https://github.com/berdario/pew/issues/195. |--- | pew/pew.py | 22 +++------------------- | pew/shell_config/complete.fish | 9 --------- | pew/shell_config/complete.zsh | 3 --- | tests/test_install.py | 29 ----------------------------- | 4 files changed, 3 insertions(+), 60 deletions(-) | delete mode 100644 tests/test_install.py | |diff --git a/pew/pew.py b/pew/pew.py |index c588a2e..2ffea2f 100644 |--- a/pew/pew.py |+++ b/pew/pew.py
File to patch: Skip this patch? [y] 1 out of 1 hunk ignored The text leading up to this was:
|diff --git a/pew/shell_config/complete.fish b/pew/shell_config/complete.fish |index af9f6d2..5dd0195 100644 |--- a/pew/shell_config/complete.fish |+++ b/pew/shell_config/complete.fish
File to patch: Skip this patch? [y] 1 out of 1 hunk ignored The text leading up to this was:
|diff --git a/pew/shell_config/complete.zsh b/pew/shell_config/complete.zsh |index 623fbff..e3a9aa5 100644 |--- a/pew/shell_config/complete.zsh |+++ b/pew/shell_config/complete.zsh
File to patch: Skip this patch? [y] 1 out of 1 hunk ignored The next patch would delete the file test_install.py, which does not exist! Assume -R? [n] Apply anyway? [n] 1 out of 1 hunk ignored RPM build errors: BUILDSTDERR: Bad exit status from /var/tmp/rpm-tmp.3WUx03 (%prep)
This will evaluate to PTYHONPATH=:/path/to/buildroot/usr/lib/python3.7/site-packages, and an empty component in path has special meaning of cwd.
Auch, thanks for pointing that out!
I think it's better/safer to write this as:
PATH=%{buildroot}%{_bindir}:$PATH \ PYTHONPATH=%{buildroot}%{python3_sitelib} \ %__python3 -m pytest -v tests
If I understand pytest's documentation correctly, invoking pytest through python -m pytest will also add the current directory to sys.path which is the exact thing we want to avoid? https://docs.pytest.org/en/latest/usage.html#calling-pytest-through-python-m...
So using something like:
PATH=%{buildroot}%{_bindir}:$PATH \ PYTHONPATH=%{buildroot}%{python3_sitelib} \ py.test-3 -v tests
should be better.
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
Miro Hrončok mhroncok@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mhroncok@redhat.com
--- Comment #3 from Miro Hrončok mhroncok@redhat.com ---
If I understand pytest's documentation correctly, invoking pytest through python -m pytest will also add the current directory to sys.path
yes
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #4 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl ---
If I understand pytest's documentation correctly, invoking pytest through python -m pytest will also add the current directory to sys.path
Oh, python, why do you do this to me?!
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #5 from Miro Hrončok mhroncok@redhat.com --- https://apps.fedoraproject.org/koschei/package/pew?collection=f31 already started to fail:
No package found for: python3dist(pythonz-bd) >= 1.10.2
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #6 from Tadej Janež tadej.j@nez.si --- (In reply to Miro Hrončok from comment #5)
https://apps.fedoraproject.org/koschei/package/pew?collection=f31 already started to fail:
No package found for: python3dist(pythonz-bd) >= 1.10.2
Yes, I also noticed this.
This was because I enabled Koschei tracking before I pushed my latest changes to git. Namely, Koschei took the then latest commit: https://src.fedoraproject.org/rpms/pew/c/9d499ce684acd457d1c26fc15a2c3ffca8b..., which still has the dependency on pythonz-bd.
I've pushed my changes to git and the rawhide build was successful: https://koji.fedoraproject.org/koji/buildinfo?buildID=1290046
I don't know how to update Koschei manually or when it will pick up the new code in master.
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #7 from Miro Hrončok mhroncok@redhat.com --- Right. Koschei uses latest successful build, not git: https://github.com/msimacek/koschei/issues/276
It's back in normal.
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #8 from Tadej Janež tadej.j@nez.si ---
Right. Koschei uses latest successful build, not git: https://github.com/msimacek/koschei/issues/276
Huh, that's a bit counter-intuitive. Thanks for pointing that out!
It's back in normal.
Ok, great.
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #9 from Tadej Janež tadej.j@nez.si --- And BTW, I couldn't build the F30 package due to it being blocked:
BuildError: package pew is blocked for tag f30-updates-candidate
I've posted to https://pagure.io/releng/issue/8448#comment-577000.
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |MODIFIED
--- Comment #10 from Fedora Update System updates@fedoraproject.org --- FEDORA-2019-8dc336f073 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-8dc336f073
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #11 from Fedora Update System updates@fedoraproject.org --- FEDORA-2019-9b1d2a41db has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-9b1d2a41db
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #12 from Fedora Update System updates@fedoraproject.org --- pew-1.2.0-1.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-9b1d2a41db
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- pew-1.2.0-1.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-8dc336f073
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2019-06-26 04:06:56
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- pew-1.2.0-1.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1720757
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- pew-1.2.0-1.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.
package-review@lists.fedoraproject.org