Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751248AbdGOMtw (ORCPT <rfc822;w@1wt.eu>);
        Sat, 15 Jul 2017 08:49:52 -0400
Received: from ec2-52-27-115-49.us-west-2.compute.amazonaws.com ([52.27.115.49]:48460
        "EHLO osg.samsung.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org
        with ESMTP id S1751187AbdGOMtu (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Sat, 15 Jul 2017 08:49:50 -0400
Date: Sat, 15 Jul 2017 09:49:40 -0300
From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
To: Markus Heiser <markus.heiser@darmarit.de>
Cc: Linux Doc Mailing List <linux-doc@vger.kernel.org>,
        Mauro Carvalho Chehab <mchehab@infradead.org>,
        linux-kernel@vger.kernel.org, Jonathan Corbet <corbet@lwn.net>
Subject: Re: [PATCH RFC] scripts/sphinx-pre-install: add a script to check
 Sphinx install
Message-ID: <20170715094940.2bcd7eab@vento.lan>
In-Reply-To: <1719549A-CDF5-4448-8CD6-28B52802E00A@darmarit.de>
References: <7ab9f1045ad103f1da56cb6b19948ea107fdb272.1500050599.git.mchehab@s-opensource.com>
        <042D234A-6B3D-43D4-84BA-8332C0596AA4@darmarit.de>
        <20170714232140.0608da3c@vento.lan>
        <1719549A-CDF5-4448-8CD6-28B52802E00A@darmarit.de>
Organization: Samsung
X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-redhat-linux-gnu)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 4690
Lines: 132

Em Sat, 15 Jul 2017 11:51:45 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> > Am 15.07.2017 um 04:21 schrieb Mauro Carvalho Chehab <mchehab@s-opensource.com>:
> > 
> > Em Fri, 14 Jul 2017 19:35:59 +0200
> > Markus Heiser <markus.heiser@darmarit.de> escreveu:
> >   
> >>> Am 14.07.2017 um 18:49 schrieb Mauro Carvalho Chehab <mchehab@s-opensource.com>:
> >>> 
> >>> Solving Sphinx dependencies can be painful. Add a script to
> >>> check if everything is ok.    
> >> 
> >> just my 5cent:
> >> 
> >> What we need is a "requirements.txt" file to define a
> >> **reference environment**. E.g. to stick Sphinx 1.4.9 in
> >> such a reference environment::
> >> 
> >> <snip: requirements.txt> ---
> >> Sphinx==1.4.9
> >> sphinx_rtd_theme
> >> <snap> ---------------------
> >> 
> >> The rest is similarly to what you wrote in doc-guide/sphinx.rst ...
> >> 
> >> The ref-environment can be build with virtualenv & pip::
> >> 
> >> $ virtualenv --python=python3 docenv
> >> (doc-env) $ source ./docenv/bin/activate
> >> (doc-env) $ pip install -r requirements.txt
> >> 
> >> From now we can start our build as usual. If not already done,
> >> first activate the environment::
> >> 
> >> $ . ./docenv/bin/activate
> >> (doc-env) $ make htmldocs
> >> 
> >> This (requirements.txt) is the way python packaging goes.  
> > 
> > 
> > The above assumes that the user wants to use virtenv and
> > have python3, virtualenv3 and pip3 already installed.
> > 
> > I agree that a virtual environment works better than using
> > distro-specific packaging, as Sphinx toolchain is really
> > fragile. But we should give an option for the developer to
> > use whatever he wants.  
> 
> The developer is free to choose the way he like. But we are talking
> about what is "best practice".

As I said, the idea is to let the user to decide what it wants.

I focused on the packaging approach first because such logic
is required for other packages. Now that it is working, just
sent a version 5 that will use virtualenv for Sphinx by default.

With such change, it will now do the right thing:

Forcing to use distro-packages:

	$ ./scripts/sphinx-pre-install --no-virtualenv
	Checking if the needed tools for Fedora release 26 (Twenty Six) are available
	ERROR: please install "python-sphinx", otherwise, build won't work.
	Warning: better to also install "sphinx_rtd_theme".
	Warning: better to also install "python-sphinx-latex".
	You should run:

		sudo dnf install -y python3-sphinx python3-sphinx_rtd_theme python-sphinx-latex

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

Default:

	$ ./scripts/sphinx-pre-install
	Checking if the needed tools for Fedora release 26 (Twenty Six) are available
	Warning: better to also install "python-sphinx-latex".
	You should run:

		sudo dnf install -y python-sphinx-latex
		virtualenv sphinx_1.4
		. sphinx_1.4/bin/activate
		pip install 'docutils==0.12'
		pip install 'Sphinx==1.4.9'
		pip install sphinx_rtd_theme

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

There is one problem there on Fedora that I just noticed:
"python-sphinx-latex" actually installs python2-sphinx.

Fixing it is trivial, but will require some time to adjust, as the
script will need to manually check for the packages that are actually
required on Fedora.

Yet, before spending more time on such script, I'd like to have
more feedback if:

	- is this approach acceptable?
	- should it have an optional argument that will make the
	  script to run the needed commands;
	- should it be integrated at the Documentation/Makefile?
	- what's the best name/location for such script?

I guess it could also use kpsewhich to check if the needed
texlive packages are installed. However, the problem with such
approach is that texlive-kpathsea-bin package should be installed
first, in order to provide such command.

So, installing PDF and math dependencies would require two steps.

> I tested sphinx-pre-install and it works fine for me, thats not the
> point. The point is: what do we recommend? E.g. for me it advices me
> to run:
> 
> 	sudo apt-get install python3-sphinx python3-sphinx-rtd-theme
> 
> We should not assume that the developer (better: the build-user) owns the
> privilege to install fine grained OS packages. There is a admin-part and
> a user-part:

That's not relevant. Typically, anyone that is building a Kernel has
admin privileges, otherwise it can't actually test the Kernel that was
built.

Ok, there are exceptions to that, but, on such case, the user should
be able to request the admin to install whatever packages are needed
to build the Kernel.

Thanks,
Mauro