Re: Please review: documentation for python build system

[Chris Marusich]

Hi Hartmut!

Thank you very much for the documentation.  It is quite clear, and it
helps clarify the background and intent of the new python build system.
My comments below are primarily grammatical or stylistic.  I hope you
find my feedback helpful.

Hartmut Goebel <address@hidden> writes:

> In Python there are different ways to install packages: distutils,
> setuptools, easy_install and pip.  All of these are sharing the file
> setup.py,

I suggest changing "All of these are sharing the file setup.py" to "All
of these use a file named setup.py".

> introduced with distutils in Python 2.0.  setup.py can be considered

I'm not sure how we feel about starting a sentence with a lower case
character, even if it is the name of a file that is usually lower case.
Perhaps start the sentence with "The setup.py file" to avoid it?

> as a kind of Makefile accepting targets (or commands) like "build" and
> "install".  As of autumn 2016 the recommended way to install Python
> packages is using pip.

I suggest changing "using pip" to "using pip, which is a Python-specific
package manager, since it was not clarified earlier.  Also, should we
capitalize Pip, too?

> For both distutils and setuptools running "python setup.py install" is
> the way to install Python packages. 

Maybe put a comma between "setuptools" and "running" so it looks like
this: "For both distutils and setuptools, running..."

> With distutils the "install" command basically copies all packages
> into <prefix>/lib/pythonX.Y/site-packages.
>
> Some time later "setuptools" have been established to enhance
> distutils.  

I suggest changing "have been" to "was".

> To use setuptools, the developer imports setuptools in setup.py.  When
> importing setuptools, the original "install" command gets overwritten
> by setuptools' "install" command.
>
> easy_install and pip are both command-line tools capable to search and
> download the package source from PyPI (the Python Package Index).  

I suggest rephrasing this as "The command-line tools easy_install and
pip are both capable of finding and downloading the package source..."

> Both of them import setuptools and execute the "setup.py" file under
> their control.  Thus the "setup.py" behaves as if the developer had
> imported setuptools within setup.py - even is still using only
> distutils.
>
> Setuptools' "install" command (to be more precise: the "easy_install"
> command which is called by "install") will put the path of the
> currently installed version of each package and it's dependencies (as
> declared in setup.py) into an "easy-install.pth" file.  In guix 

I suggest changing "guix" to "Guix" or "GNU Guix".  It is my
understanding that we prefer to capitalize Guix unless we're talking
about a command invocation.

> each packages gets it's own "site-packages" directory and thus an
> "easy-install.pth" of it's own.  

Change "it's" to "its" because this is the possessive form.

> To avoid conflicts this file gets renamed to <packagename>.pth in
> phase rename-pth-file.  

I suggest rewording this like so: "To avoid conflicts, the python build
system renames the file to <packagename>.pth in the phase
rename-pth-file.

> To ensure the .pth-file 

I think referring to this as ".pth file", without a hyphen, is more
correct.

> will be process, 

I suggest changing this to "will be processed by Python," or rephrase
the whole sentence to be non-passive, like so: "To ensure that Python
will process the .pth file..."

> easy_install also creates a basic "site.py" in each "site-packages"
> directory. The file is the same for all packages, thus there is no
> need to rename it.

Perhaps either add a sentence to explain what site.py is, or provide a
link to a document that explains it.

> The .pth-files contain the file-system paths (pointing to the store)
> of all dependencies.  So the dependency is hidden in the .pth file but
> is not visible in the file-system.  Now if packages A and B both
> required packages P, but in different versions, guix 

Again, I think this should be "Guix" or "GNU Guix".

> will not detect this when installing both A and B to a profile. (For
> details and example see
> https://lists.gnu.org/archive/html/guix-devel/2016-10/msg01233.html.)
>
> Now pip behaves a bit different: it always executes "setup.py" with
> the option "--single-version-externally-managed" set.  

When you say "now," do you mean that "previously, Pip behaved
differently, but now, it behaves this way", or do you simply mean "Pip
behaves differently than Guix"?  I think you probably mean the latter,
but if there is any ambiguity, perhaps you should omit the word "now".

> This makes setuptools' "install" command to *not* run "easy_install"
> but the original "install" command and thus no .pth-file (and no
> site.py) will be created. 

I suggest changing this to: "This makes setuptools' "install" command
run the original "install" command instead of the "easy_install," so no
.pth file (and no site.py) will be created."

> The "site-packages" directory only contains the package and the
> related .egg-info directory.
>
> This is exactly what we need for guix and this is what we mimic in the
> install phase below.
>
> As a draw back, the magic of the .pth file of linking to the other required
> packages is gone and these packages have now to be declared as
> "propagated-inputs".

I know I haven't reviewed the actual patch series, and I apologize for
not having the time to do so, but it seems unfortunate that we have to
use propagated inputs.  (Of course, it seems like an improvement over
what we had before, so that is good!)  Are there any plans to make it so
that we can use Python packages without propagation?  For example, I
imagine that we can (and probably do) wrap some scripts with a wrapper
that sets the PYTHONPATH correctly.  I am not sure if we could do that
for Python libraries, though.

That said, I have found that in practice Python's packaging system is
very complicated, and .pth files almost always add an order of magnitude
to that complexity.  So I will not be surprised if the answer here is
"there are no plans at this time."

> Note: Importing setuptools also adds two sub-commands:
> "install_egg_info" and "install_scripts".  These sub-commands are
> executed even if "--single-version-externally-managed" is set, thus
> the .egg-info directory and the scripts defined in entry-points will
> always be created.
>
> Note: Even if the "easy-install.pth" is not longer created, we kept this
> phase.  There still may be packages creating an "easy-install.pth" manually
> for some good reason.

Are these notes truly necessary?  Can they perhaps just be comments in
the source code instead?  If they are truly necessary in the manual,
please consider including them as footnotes, so it is easy to tell which
section of text you wanted to associate these notes with.

-- 
Chris

signature.asc
Description: PGP signature