Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1720757 - Review Request: pew - Tool to manage multiple virtualenvs written in pure Python
Summary: Review Request: pew - Tool to manage multiple virtualenvs written in pure Python
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-06-14 20:21 UTC by Tadej Janež
Modified: 2019-07-03 02:01 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-06-26 04:06:56 UTC
Type: Bug
Embargoed:
zbyszek: fedora-review+


Attachments (Terms of Use)

Description Tadej Janež 2019-06-14 20:21:16 UTC
Spec URL: https://raw.githubusercontent.com/tjanez/pew-package/58936dffd15627cae0012f667882409ec71a26e1/pew.spec
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/ca9ed68f8ca88ec93b06039f2cc7cf2873af22e3?branch=master.

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

Comment 1 Zbigniew Jędrzejewski-Szmek 2019-06-15 09:33:54 UTC
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.

Comment 2 Tadej Janež 2019-06-18 07:34:05 UTC
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>
> |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-pytest

So using something like:

PATH=%{buildroot}%{_bindir}:$PATH \
PYTHONPATH=%{buildroot}%{python3_sitelib} \
py.test-3 -v tests

should be better.

Comment 3 Miro Hrončok 2019-06-18 08:49:46 UTC
> If I understand pytest's documentation correctly, invoking pytest through python -m pytest will also add the current directory to sys.path

yes

Comment 4 Zbigniew Jędrzejewski-Szmek 2019-06-18 09:27:48 UTC
> 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?!

Comment 5 Miro Hrončok 2019-06-19 11:41:10 UTC
https://apps.fedoraproject.org/koschei/package/pew?collection=f31 already started to fail:

No package found for: python3dist(pythonz-bd) >= 1.10.2

Comment 6 Tadej Janež 2019-06-19 12:04:33 UTC
(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/9d499ce684acd457d1c26fc15a2c3ffca8b12825?branch=master, 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.

Comment 7 Miro Hrončok 2019-06-19 12:19:59 UTC
Right. Koschei uses latest successful build, not git: https://github.com/msimacek/koschei/issues/276

It's back in normal.

Comment 8 Tadej Janež 2019-06-19 12:51:17 UTC
> 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.

Comment 9 Tadej Janež 2019-06-19 12:53:04 UTC
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.

Comment 10 Fedora Update System 2019-06-24 07:53:18 UTC
FEDORA-2019-8dc336f073 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-8dc336f073

Comment 11 Fedora Update System 2019-06-24 07:57:58 UTC
FEDORA-2019-9b1d2a41db has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-9b1d2a41db

Comment 12 Fedora Update System 2019-06-24 22:46:08 UTC
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

Comment 13 Fedora Update System 2019-06-25 02:11:15 UTC
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

Comment 14 Fedora Update System 2019-06-26 04:06:56 UTC
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.

Comment 15 Fedora Update System 2019-07-03 02:01:37 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.