[bug#46848] [PATCHES] [core-updates] PEP 517 python-build-system

[Lars-Dominik Braun <lars@6xq.net>]

Hi Maxim,

> Here's a review of patches 1 to 6.

thanks for the review. Unfortunately this is not the most recent
proposal and I have no way to retract the previous patches. I pushed v3
to the wip-python-pep517 branch, because of the sheer number of patches
and so the CI could build it (since it requires a rebuild of the entire
rust bootstrap chain).

> > +        ;; Prefer pytest
> > +        ;; XXX: support nose
> 
> You can remove this; nose is stale/deprecated.

So it’s preferred to replace 'check in cases where python-nose is
still in use?

> 
> > +        (cond
> > +          (pytest
> > +            (begin
> > +              (format #t "using pytest~%")
> > +              (invoke pytest "-vv"))) ; XXX: support skipping tests based on name/extra arguments?
> 
> We could have a #:test-command argument to specify an arbitrary command
> as a list of strings, such as used by the emacs-build-system; that'd
> allow us to avoid overriding the phase just to add a '-k "not
> this-test"' or similar.

I added #:test-flags in my v3 proposal.

> > +          ;; But fall back to setup.py, which should work for most
> > +          ;; packages. XXX: would be nice not to depend on setup.py here? fails
> > +          ;; more often than not to find any tests at all. Maybe we can run
> > +          ;; `python -m unittest`?
> > +          (have-setup-py
> > +            (begin
> > +              (format #t "using setup.py~%")
> > +                (invoke "python" "setup.py" "test" "-v")))
> 
> As Marius noted, falling back to 'python setup.py test' is not
> desirable; it's scheduled to be removed already.

Sure, but using `python -m unittest` instead requires some investigation.

> > +    (define (move-script source destination)
> > +      "Move executable script file from .data/scripts to out/bin and replace
> > +temporary hashbang"
> > +	  (move-data source destination)
> > +      ;; ZIP does not save/restore permissions, make executable
> > +      ;; XXX: might not be a file, but directory with subdirectories
> > +      (chmod destination #o755)
> > +      (substitute* destination (("#!python") python-hashbang)))
> 
> It seems the directory case should be handled?  Otherwise the
> substitute* call would error out upon encountering it.

I have not seen anyone using subdirectories in bin/ yet. Is that
supported anywhere?

> So, IIUC, this complicated install phase is because we no longer take
> 'pip' for granted and is only later available, built from this very
> build system, right?  Otherwise installing a wheel with pip would be
> trivial (c.f. python-isort).

If we want to bootstrap these two packages easily (and possibly start
unvendoring their vendored dependencies later), they cannot be part of
this build system and thus we need to implement building/installing
ourselves. I tried using pypa-build in an earlier version, but the
bootstrap chain is unmaintainable.

There also is a project called installer[1], but it does not have a
CLI yet.

Cheers,
Lars

[1] https://github.com/pradyunsg/installer