[Bug 1720757] New: Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

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: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  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

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |POST
                 CC|                            |[e-mail]
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review+

--- Comment #1 from Zbigniew Jędrzejewski-Szmek  ---
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.

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #2 from Tadej Janež  ---
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?= 
> |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.

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

Miro Hrončok  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #3 from Miro Hrončok  ---
> If I understand pytest's documentation correctly, invoking pytest through python -m pytest will also add the current directory to sys.path

yes

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #4 from Zbigniew Jędrzejewski-Szmek  ---
> 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?!

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #5 from Miro Hrončok  ---
https://apps.fedoraproject.org/koschei/package/pew?collection=f31 already
started to fail:

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

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #6 from Tadej Janež  ---
(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.

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #7 from Miro Hrončok  ---
Right. Koschei uses latest successful build, not git:
https://github.com/msimacek/koschei/issues/276

It's back in normal.

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #8 from Tadej Janež  ---
> 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.

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #9 from Tadej Janež  ---
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.

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|POST                        |MODIFIED

--- Comment #10 from Fedora Update System  ---
FEDORA-2019-8dc336f073 has been submitted as an update to Fedora 30.
https://bodhi.fedoraproject.org/updates/FEDORA-2019-8dc336f073

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #11 from Fedora Update System  ---
FEDORA-2019-9b1d2a41db has been submitted as an update to Fedora 29.
https://bodhi.fedoraproject.org/updates/FEDORA-2019-9b1d2a41db

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #12 from Fedora Update System  ---
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

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #13 from Fedora Update System  ---
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

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA
        Last Closed|                            |2019-06-26 04:06:56

--- Comment #14 from Fedora Update System  ---
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.

[Bug 1720757] Review Request: pew - Tool to manage multiple virtualenvs written in pure Python

https://bugzilla.redhat.com/show_bug.cgi?id=1720757

--- Comment #15 from Fedora Update System  ---
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.