[PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[tbird20d]

From: Tim Bird <tim.bird@sony.com>

With Ubuntu 16.04 (and presumably Debian distros of the same age),
the instructions for setting up a python virtual environment should
do so with the python 3 interpreter.  On these older distros, the
default python (and virtualenv command) might be python2 based.

Some of the packages that sphinx relies on are now only available
for python3.  If you don't specify the python3 interpreter for
the virtualenv, you get errors when doing the pip installs for
various packages

Fix this by adding '-p python3' to the virtualenv recommendation
line.

Signed-off-by: Tim Bird <tim.bird@sony.com>
---
 scripts/sphinx-pre-install | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/scripts/sphinx-pre-install b/scripts/sphinx-pre-install
index a8f0c00..fa3fb05 100755
--- a/scripts/sphinx-pre-install
+++ b/scripts/sphinx-pre-install
@@ -701,11 +701,26 @@ sub check_needs()
 		} else {
 			my $rec_activate = "$virtenv_dir/bin/activate";
 			my $virtualenv = findprog("virtualenv-3");
+			my $rec_python3 = "";
 			$virtualenv = findprog("virtualenv-3.5") if (!$virtualenv);
 			$virtualenv = findprog("virtualenv") if (!$virtualenv);
 			$virtualenv = "virtualenv" if (!$virtualenv);
 
-			printf "\t$virtualenv $virtenv_dir\n";
+			my $rel = "";
+			if (index($system_release, "Ubuntu") != -1) {
+				$rel = $1 if ($system_release =~ /Ubuntu\s+(\d+)[.]/);
+				if ($rel && $rel >= 16) {
+					$rec_python3 = " -p python3";
+				}
+			}
+			if (index($system_release, "Debian") != -1) {
+				$rel = $1 if ($system_release =~ /Debian\s+(\d+)/);
+				if ($rel && $rel >= 7) {
+					$rec_python3 = " -p python3";
+				}
+			}
+
+			printf "\t$virtualenv$rec_python3 $virtenv_dir\n";
 			printf "\t. $rec_activate\n";
 			printf "\tpip install -r $requirement_file\n";
 			deactivate_help();
-- 
2.1.4

Re: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Jonathan Corbet]

On Mon, 24 Feb 2020 18:34:41 -0700
tbird20d@gmail.com wrote:

> With Ubuntu 16.04 (and presumably Debian distros of the same age),
> the instructions for setting up a python virtual environment should
> do so with the python 3 interpreter.  On these older distros, the
> default python (and virtualenv command) might be python2 based.
> 
> Some of the packages that sphinx relies on are now only available
> for python3.  If you don't specify the python3 interpreter for
> the virtualenv, you get errors when doing the pip installs for
> various packages
> 
> Fix this by adding '-p python3' to the virtualenv recommendation
> line.
> 
> Signed-off-by: Tim Bird <tim.bird@sony.com>

I've applied this, even though it feels a bit fragile to me.  But Python
stuff can be a bit that way, sometimes, I guess.

Thanks,

jon

RE: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Bird, Tim]

> -----Original Message-----
> From: Jonathan Corbet <corbet@lwn.net>
> 
> On Mon, 24 Feb 2020 18:34:41 -0700
> tbird20d@gmail.com wrote:
> 
> > With Ubuntu 16.04 (and presumably Debian distros of the same age),
> > the instructions for setting up a python virtual environment should
> > do so with the python 3 interpreter.  On these older distros, the
> > default python (and virtualenv command) might be python2 based.
> >
> > Some of the packages that sphinx relies on are now only available
> > for python3.  If you don't specify the python3 interpreter for
> > the virtualenv, you get errors when doing the pip installs for
> > various packages
> >
> > Fix this by adding '-p python3' to the virtualenv recommendation
> > line.
> >
> > Signed-off-by: Tim Bird <tim.bird@sony.com>
> 
> I've applied this, even though it feels a bit fragile to me.  But Python
> stuff can be a bit that way, sometimes, I guess.

I agree it seems a bit wonky.
The less fragile approach would have been to just
always add the '-p python3' option to the virtualenv setup hint,
but Mauro seemed to want something more fine-tuned.
As far as the string parsing goes, I think that the format of strings
returned by lsb-release (and the predecesors that sphinx_pre_install
checks) is unlikely to change.

Thanks for applying it.
 -- Tim

Re: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Jonathan Corbet]

On Tue, 3 Mar 2020 17:07:48 +0000
"Bird, Tim" <Tim.Bird@sony.com> wrote:

> The less fragile approach would have been to just
> always add the '-p python3' option to the virtualenv setup hint,
> but Mauro seemed to want something more fine-tuned.

At some point I think we'll want to say that Python 2 just isn't supported
anymore.  After all, the language itself isn't supported anymore.  Perhaps
it's time to add a warning somewhere.

Thanks,

jon

Re: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Mauro Carvalho Chehab]

Em Tue, 3 Mar 2020 17:07:48 +0000
"Bird, Tim" <Tim.Bird@sony.com> escreveu:

> > -----Original Message-----
> > From: Jonathan Corbet <corbet@lwn.net>
> > 
> > On Mon, 24 Feb 2020 18:34:41 -0700
> > tbird20d@gmail.com wrote:
> >   
> > > With Ubuntu 16.04 (and presumably Debian distros of the same age),
> > > the instructions for setting up a python virtual environment should
> > > do so with the python 3 interpreter.  On these older distros, the
> > > default python (and virtualenv command) might be python2 based.
> > >
> > > Some of the packages that sphinx relies on are now only available
> > > for python3.  If you don't specify the python3 interpreter for
> > > the virtualenv, you get errors when doing the pip installs for
> > > various packages
> > >
> > > Fix this by adding '-p python3' to the virtualenv recommendation
> > > line.
> > >
> > > Signed-off-by: Tim Bird <tim.bird@sony.com>  
> > 
> > I've applied this, even though it feels a bit fragile to me.  But Python
> > stuff can be a bit that way, sometimes, I guess.  
> 
> I agree it seems a bit wonky.

Well, we could, instead, add some code that would be checking python and pip
versions, but still distros could be doing some backports with could
cause side-effects. So, checking for distro versions as done in this patch
seems a lot safer.

> The less fragile approach would have been to just
> always add the '-p python3' option to the virtualenv setup hint,
> but Mauro seemed to want something more fine-tuned.

Yeah, I asked for a more fine-tuned version.

Depending on python/pip version, adding a -p python3 seems to cause
troubles (at least I found some bug reports about that). I may be
wrong (it was a long time ago), but, before adding the logic that checks
for "python3" I guess I tried first add -p python3, but, back then,
I found some troubles (probably with some old Fedora version).

So, better to use this syntax only on distros we know it will
work as expected.

> As far as the string parsing goes, I think that the format of strings
> returned by lsb-release (and the predecesors that sphinx_pre_install
> checks) is unlikely to change.

Since when we added this script, we didn't have any troubles yet with
the part of the code with checks the distribution version. So, I guess
that the lsb-release related checks are pretty much reliable.

> 
> Thanks for applying it.
>  -- Tim
> 
> 

Re: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Markus Heiser]

Am 04.03.20 um 06:42 schrieb Mauro Carvalho Chehab:
> Em Tue, 3 Mar 2020 17:07:48 +0000
> "Bird, Tim" <Tim.Bird@sony.com> escreveu:
> 
>>> -----Original Message-----
>>> From: Jonathan Corbet <corbet@lwn.net>
>>>
>>> On Mon, 24 Feb 2020 18:34:41 -0700
>>> tbird20d@gmail.com wrote:
>>>    
>>>> With Ubuntu 16.04 (and presumably Debian distros of the same age),
>>>> the instructions for setting up a python virtual environment should
>>>> do so with the python 3 interpreter.  On these older distros, the
>>>> default python (and virtualenv command) might be python2 based.
>>>>
>>>> Some of the packages that sphinx relies on are now only available
>>>> for python3.  If you don't specify the python3 interpreter for
>>>> the virtualenv, you get errors when doing the pip installs for
>>>> various packages
>>>>
>>>> Fix this by adding '-p python3' to the virtualenv recommendation
>>>> line.
>>>>
>>>> Signed-off-by: Tim Bird <tim.bird@sony.com>
>>>
>>> I've applied this, even though it feels a bit fragile to me.  But Python
>>> stuff can be a bit that way, sometimes, I guess.
>>
>> I agree it seems a bit wonky.
> 
> Well, we could, instead, add some code that would be checking python and pip
> versions, but still distros could be doing some backports with could
> cause side-effects. So, checking for distro versions as done in this patch
> seems a lot safer.
> 
>> The less fragile approach would have been to just
>> always add the '-p python3' option to the virtualenv setup hint,
>> but Mauro seemed to want something more fine-tuned.
> 
> Yeah, I asked for a more fine-tuned version.
> 
> Depending on python/pip version, adding a -p python3 seems to cause
> troubles (at least I found some bug reports about that). I may be
> wrong (it was a long time ago), but, before adding the logic that checks
> for "python3" I guess I tried first add -p python3, but, back then,
> I found some troubles (probably with some old Fedora version).
> 
> So, better to use this syntax only on distros we know it will
> work as expected.
> 
>> As far as the string parsing goes, I think that the format of strings
>> returned by lsb-release (and the predecesors that sphinx_pre_install
>> checks) is unlikely to change.
> 
> Since when we added this script, we didn't have any troubles yet with
> the part of the code with checks the distribution version. So, I guess
> that the lsb-release related checks are pretty much reliable.
> 

With py3 the recommended way to install virtual environments is::

   python3 -m venv sphinx-env

This (python3) is what worked for me on RHEL/CentOS (dnf),
archlinux and debian/ubuntu (tested from 16.04 up to 20.04).

I am not familiar with the sphinx-pre-install script but may be
one of you is able to apply such a patch?


  -- Markus --

Re: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Mauro Carvalho Chehab]

Em Wed, 4 Mar 2020 07:20:48 +0100
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> Am 04.03.20 um 06:42 schrieb Mauro Carvalho Chehab:
> > Em Tue, 3 Mar 2020 17:07:48 +0000
> > "Bird, Tim" <Tim.Bird@sony.com> escreveu:
> >   
> >>> -----Original Message-----
> >>> From: Jonathan Corbet <corbet@lwn.net>
> >>>
> >>> On Mon, 24 Feb 2020 18:34:41 -0700
> >>> tbird20d@gmail.com wrote:
> >>>      
> >>>> With Ubuntu 16.04 (and presumably Debian distros of the same age),
> >>>> the instructions for setting up a python virtual environment should
> >>>> do so with the python 3 interpreter.  On these older distros, the
> >>>> default python (and virtualenv command) might be python2 based.
> >>>>
> >>>> Some of the packages that sphinx relies on are now only available
> >>>> for python3.  If you don't specify the python3 interpreter for
> >>>> the virtualenv, you get errors when doing the pip installs for
> >>>> various packages
> >>>>
> >>>> Fix this by adding '-p python3' to the virtualenv recommendation
> >>>> line.
> >>>>
> >>>> Signed-off-by: Tim Bird <tim.bird@sony.com>  
> >>>
> >>> I've applied this, even though it feels a bit fragile to me.  But Python
> >>> stuff can be a bit that way, sometimes, I guess.  
> >>
> >> I agree it seems a bit wonky.  
> > 
> > Well, we could, instead, add some code that would be checking python and pip
> > versions, but still distros could be doing some backports with could
> > cause side-effects. So, checking for distro versions as done in this patch
> > seems a lot safer.
> >   
> >> The less fragile approach would have been to just
> >> always add the '-p python3' option to the virtualenv setup hint,
> >> but Mauro seemed to want something more fine-tuned.  
> > 
> > Yeah, I asked for a more fine-tuned version.
> > 
> > Depending on python/pip version, adding a -p python3 seems to cause
> > troubles (at least I found some bug reports about that). I may be
> > wrong (it was a long time ago), but, before adding the logic that checks
> > for "python3" I guess I tried first add -p python3, but, back then,
> > I found some troubles (probably with some old Fedora version).
> > 
> > So, better to use this syntax only on distros we know it will
> > work as expected.
> >   
> >> As far as the string parsing goes, I think that the format of strings
> >> returned by lsb-release (and the predecesors that sphinx_pre_install
> >> checks) is unlikely to change.  
> > 
> > Since when we added this script, we didn't have any troubles yet with
> > the part of the code with checks the distribution version. So, I guess
> > that the lsb-release related checks are pretty much reliable.
> >   
> 
> With py3 the recommended way to install virtual environments is::
> 
>    python3 -m venv sphinx-env
> 
> This (python3) is what worked for me on RHEL/CentOS (dnf),
> archlinux and debian/ubuntu (tested from 16.04 up to 20.04).

Hmm... from:

	https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/

This works since Python version 3.3. It sounds doable to use it.

Yet, if we'll be switching to this method, the script should check if
the version is 3.3 or newer. The logic inside get_sphinx_fname() would 
also require some changes, as it won't need to install anymore the 
virtualenv program for Python >= 3.3.

> 
> I am not familiar with the sphinx-pre-install script but may be
> one of you is able to apply such a patch?
> 
> 
>   -- Markus --


Thanks,
Mauro

Re: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Markus Heiser]

Am 04.03.20 um 09:31 schrieb Mauro Carvalho Chehab:
> Em Wed, 4 Mar 2020 07:20:48 +0100
> Markus Heiser <markus.heiser@darmarit.de> escreveu:
>> With py3 the recommended way to install virtual environments is::
>>
>>     python3 -m venv sphinx-env
>>
>> This (python3) is what worked for me on RHEL/CentOS (dnf),
>> archlinux and debian/ubuntu (tested from 16.04 up to 20.04).
> 
> Hmm... from:
> 
> 	https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/
> 
> This works since Python version 3.3. It sounds doable to use it.
> 
> Yet, if we'll be switching to this method, the script should check if
> the version is 3.3 or newer. The logic inside get_sphinx_fname() would
> also require some changes, as it won't need to install anymore the
> virtualenv program for Python >= 3.3.

I guess you can ignore 3.2 and downwards

   https://en.wikipedia.org/wiki/History_of_Python#Table_of_versions

Support for py2.7 and >=py3.3 should match nearly all use cases / distributions 
we support.

BTW: starting scripts with:

-m <module-name>
     Searches sys.path for the named module and runs the
     corresponding .py file as a script.

is mostly more robust.  The option exists also in py2.  From py3.3 on
a subset of virtualenv is built-in, so you can run '-m venv' ot of the
box.

   -- Markus --

RE: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Bird, Tim]

> -----Original Message-----
> From: Jonathan Corbet <corbet@lwn.net>
> 
> On Tue, 3 Mar 2020 17:07:48 +0000
> "Bird, Tim" <Tim.Bird@sony.com> wrote:
> 
> > The less fragile approach would have been to just
> > always add the '-p python3' option to the virtualenv setup hint,
> > but Mauro seemed to want something more fine-tuned.
> 
> At some point I think we'll want to say that Python 2 just isn't supported
> anymore.  After all, the language itself isn't supported anymore.  Perhaps
> it's time to add a warning somewhere.

Probably.  IMHO always adding the 'p python3' would have been the
first vestiges of such a hint, but maybe it should be more explicit.
A more explicit statement of "watch out if your default python
interpreter is python2" would have probably shortened some of 
my setup time.

Ubuntu 16.04 and Ubuntu 18.04 both include python3, but have
/usr/bin/python default to python2.  So, while the package recommendations
from the script were good (and sufficient), the virtualenv hint was somewhat
lacking.

And, just for full disclosure, I wish there was a way to mark this type of fix
with a obsolescence  flag so it could be removed later.  This is the type of
thing that gets put into a script as a workaround for a transition period, but
keeps being run 10 years later when it's no longer relevant.  Heck, the whole
virtualenv recommendation might be irrelevant in the next version of Ubuntu
(but maybe the script already figures that out.)
 -- Tim

Re: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Mauro Carvalho Chehab]

Em Wed, 4 Mar 2020 17:25:41 +0000
"Bird, Tim" <Tim.Bird@sony.com> escreveu:

> > -----Original Message-----
> > From: Jonathan Corbet <corbet@lwn.net>
> > 
> > On Tue, 3 Mar 2020 17:07:48 +0000
> > "Bird, Tim" <Tim.Bird@sony.com> wrote:
> >   
> > > The less fragile approach would have been to just
> > > always add the '-p python3' option to the virtualenv setup hint,
> > > but Mauro seemed to want something more fine-tuned.  
> > 
> > At some point I think we'll want to say that Python 2 just isn't supported
> > anymore.  After all, the language itself isn't supported anymore.  Perhaps
> > it's time to add a warning somewhere.  
> 
> Probably.  IMHO always adding the 'p python3' would have been the
> first vestiges of such a hint, but maybe it should be more explicit.
> A more explicit statement of "watch out if your default python
> interpreter is python2" would have probably shortened some of 
> my setup time.

A warning like that makes sense to me.

> Ubuntu 16.04 and Ubuntu 18.04 both include python3, but have
> /usr/bin/python default to python2.  So, while the package recommendations
> from the script were good (and sufficient), the virtualenv hint was somewhat
> lacking.

And if you try to change the Ubuntu default to python3, some things break 
there (I did that last month using alternates on Ubuntu 19.10 - had to 
revert back to python2).

> And, just for full disclosure, I wish there was a way to mark this type of fix
> with a obsolescence  flag so it could be removed later.  This is the type of
> thing that gets put into a script as a workaround for a transition period, but
> keeps being run 10 years later when it's no longer relevant.  Heck, the whole
> virtualenv recommendation might be irrelevant in the next version of Ubuntu
> (but maybe the script already figures that out.)

Actually, virtualenv support was added later ;-)

The script supports both virtualenv and distro package install, although
the current default is to use virtualenv. You can force it to recommend the
distro package with:

	$ ./scripts/sphinx-pre-install --no-virtualenv
	Detected OS: Fedora release 30 (Thirty).

	ERROR: please install "python-sphinx", otherwise, build won't work.
	Warning: better to also install "sphinx_rtd_theme".
	You should run:

		sudo dnf install -y python3-sphinx python3-sphinx_rtd_theme

	Can't build as 1 mandatory dependency is missing at ./scripts/sphinx-pre-install line 721.

The main reason why virtualenv is the default is that we need to
ensure a certain minimal version (nowadays, it is recommended at least 1.7.9). 
If one is using a LTS distro, the distro-package could be older than that.

With virtualenv, it is easier to fulfill the minimal version (and the
recommended one). You may even have multiple Sphinx versions, with is
quite useful when trying to identify if the html and/or pdf output
is doing the right thing with different toolchain versions.

Thanks,
Mauro

Re: [PATCH] scripts/sphinx-pre-install: add '-p python3' to virtualenv

[Mauro Carvalho Chehab]

Em Wed, 4 Mar 2020 10:20:34 +0100
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> 
> Am 04.03.20 um 09:31 schrieb Mauro Carvalho Chehab:
> > Em Wed, 4 Mar 2020 07:20:48 +0100
> > Markus Heiser <markus.heiser@darmarit.de> escreveu:
> >> With py3 the recommended way to install virtual environments is::
> >>
> >>     python3 -m venv sphinx-env
> >>
> >> This (python3) is what worked for me on RHEL/CentOS (dnf),
> >> archlinux and debian/ubuntu (tested from 16.04 up to 20.04).
> > 
> > Hmm... from:
> > 
> > 	https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/
> > 
> > This works since Python version 3.3. It sounds doable to use it.
> > 
> > Yet, if we'll be switching to this method, the script should check if
> > the version is 3.3 or newer. The logic inside get_sphinx_fname() would
> > also require some changes, as it won't need to install anymore the
> > virtualenv program for Python >= 3.3.
> 
> I guess you can ignore 3.2 and downwards
> 
>    https://en.wikipedia.org/wiki/History_of_Python#Table_of_versions
> 
> Support for py2.7 and >=py3.3 should match nearly all use cases / distributions 
> we support.
> 
> BTW: starting scripts with:
> 
> -m <module-name>
>      Searches sys.path for the named module and runs the
>      corresponding .py file as a script.
> 
> is mostly more robust.  The option exists also in py2.  From py3.3 on
> a subset of virtualenv is built-in, so you can run '-m venv' ot of the
> box.

I did some tests... as everything with python, it is not so simple...
The thing is that "-mvenv" requires a python module called "ensurepip".

On Fedora, openSuse and archlinux, this is installed together with
python3, but Debian maintainers had a different idea about how to package it.
There, ensurepip is inside a python3-venv-3.x (where x is 5, 6 or 7 -
depending on the Ubuntu/Debian version, and if backports repository is
been used or not).

There is a package python3-venv too, with installs the right package,
together with some unneeded stuff (pyvenv, with is a deprecated script). 

Yet, installing python3-venv seems to be a reliable way to install
the proper package without having to deal with more fragile heuristics.

I'm working on some patches that should hopefully add support for
using "python3 -mvenv", but testing it is not trivial, as I want
to ensure that it won't cause troubles on other distros. So, I'm
installing a myriad of distros with lxc, in order to test how the
script will actually work with some different environments.

Thanks,
Mauro