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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #2 from Tadej Janež <tadej.j@xxxxxx> ---
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@xxxxxx>
> |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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux