diff mbox series

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

Message ID YJ+VD+Xcb7tCU6FF@noor.fritz.box
State New
Headers show
Series [bug#46848,PATCHES,core-updates] PEP 517 python-build-system | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Lars-Dominik Braun May 15, 2021, 9:31 a.m. UTC
Hi,

I rebased my changes on top of the current core-updates HEAD.

Cheers,
Lars

Comments

Lars-Dominik Braun Dec. 13, 2021, 8:10 p.m. UTC | #1
Hi,

I’m working on version 3 of this patchset, removing the dependency
on python-pypa-build, which simplifies bootstrapping. A rough version
is available at

https://github.com/PromyLOPh/guix/commits/work-python-build-pep517

Cheers,
Lars
Lars-Dominik Braun Jan. 5, 2022, 2:51 p.m. UTC | #2
Hi everyone,

I pushed the 3rd version of my new PEP 517-based python-build-system
into wip-python-pep517. I removed the dependency on python-pypa-build
for building packages and instead implemented building with a single
python invokation. This simplifies bootstrapping Python. The 'install
phase also learned how to create scripts in bin/ from an entry points
specification.

Currently 600 of 2212 packages using python-build-system are failing to
build. That’s alot, but most failures so far were related to testsuites,
which were not enabled correctly previously because 'check was not
replaced manually. And some fail because upstream does not separate
sources and tests and a name clash occurs. This also happens when C
extensions are build and there does not seem to be a workaround.

This number does not include Python 2 packages, which will probably
break with this new build system. Since Python 2 is EOL anyway I suggest
to entirely remove Python 2 support before merging this changeset. After
merging we should upgrade the entire Python ecosystem, because alot of
packages are already years old.

Cheers,
Lars

PS: Maxim: This is your ping for a review (via #52269). Thanks :)
Marius Bakke Jan. 20, 2022, 3:41 p.m. UTC | #3
Lars-Dominik Braun <lars@6xq.net> skriver:

> I pushed the 3rd version of my new PEP 517-based python-build-system
> into wip-python-pep517. I removed the dependency on python-pypa-build
> for building packages and instead implemented building with a single
> python invokation. This simplifies bootstrapping Python. The 'install
> phase also learned how to create scripts in bin/ from an entry points
> specification.

This is great, thank you.

> Currently 600 of 2212 packages using python-build-system are failing to
> build. That’s alot, but most failures so far were related to testsuites,
> which were not enabled correctly previously because 'check was not
> replaced manually. And some fail because upstream does not separate
> sources and tests and a name clash occurs. This also happens when C
> extensions are build and there does not seem to be a workaround.

Can you elaborate on the name clash issue?

> This number does not include Python 2 packages, which will probably
> break with this new build system. Since Python 2 is EOL anyway I suggest
> to entirely remove Python 2 support before merging this changeset. After
> merging we should upgrade the entire Python ecosystem, because alot of
> packages are already years old.

Unfortunately we need Python 2 for some time still.  It is used to
bootstrap old versions of GHC, Node, and probably other things.

Do you think it is feasible to provide this as a new build system, say
pep517-build-system, during a transitional period?  Then we can a) start
using it right away for the increasing amount of packages that lack a
setup.py; and b) flesh out most bugs before eventually merging it back
(possibly piecemeal) into python-build-system.

I've had a cursory look and it looks good overall.  A few comments:

* Zipping a wheel just to unpack it afterwards is weird, but there seems
  to be no way around it.
* I also think trying "python setup.py test" is unnecessary.
* It would be nice to support the "no tests" case without having to add
  explicit #:tests? everywhere.  Perhaps along the lines of...

  (match use-test-backend
   ('pytest (apply invoke ...))
   [...]
   (_ (match (find-files "." "^test_.*\\.py$")
        (() (format #t "no tests found~%"))
        (_ (apply invoke `("python" "-m" "unittest" ,@test-flags))))))

WDYT?

Great work, and apologies for not chiming in earlier!
Lars-Dominik Braun Jan. 20, 2022, 6:43 p.m. UTC | #4
Hi Marius,

> Can you elaborate on the name clash issue?
the problem seems to be that most packages do not put the source code
into a subdirectory in the sdist. I.e. package foobar will have a foobar/
directory, instead of src/foobar. Thus Python tries to load the module
foobar.barbaz from that directory, instead of the store. Thus it’ll
also look there for C extensions and fail. I don’t know why it does
that, even when *not* using `python -m` – according to the documentation
it should not. If anyone has any pointers I’d like to have a look again.

> Unfortunately we need Python 2 for some time still.  It is used to
> bootstrap old versions of GHC, Node, and probably other things.
But as far as I see these specific examples do not use
python-build-system. Only if some package using `#:python python-2` is
required during bootstrapping, then we’d need a way to support Python
2, right?

> Do you think it is feasible to provide this as a new build system, say
> pep517-build-system, during a transitional period?  Then we can a) start
> using it right away for the increasing amount of packages that lack a
> setup.py; and b) flesh out most bugs before eventually merging it back
> (possibly piecemeal) into python-build-system.
Actually, I think that’s a good idea. I tried, but I cannot fix all
packages broken by this change on my own, so smoothing the transition
could help and – as you said – we could catch bugs early on.

I could prepare a patch, if there is interest in this path.

> * Zipping a wheel just to unpack it afterwards is weird, but there seems
>   to be no way around it.
Indeed, that’s how PEP 517 works, which always builds a wheel
(i.e. ZIP file).

> * I also think trying "python setup.py test" is unnecessary.
It still works quite often, although its usefulness will decrease in
the future I guess. Another problem I see is that this command will not
fail if there are no tests.

> * It would be nice to support the "no tests" case without having to add
>   explicit #:tests? everywhere.  Perhaps along the lines of...
My idea was to force packagers to make and explain this decision
explicitly. If you don’t run tests, you have to add `#:tests? #f`
and leave a comment why they are disabled. I do see this could become
a hassle with packages that simply don’t have any tests. But my hope
is that it increases package quality.
 
>         (_ (apply invoke `("python" "-m" "unittest" ,@test-flags))))))
If I remember correctly I tried this and it did not work for some
reason. I’ll have a look again.

Cheers,
Lars
Marius Bakke Jan. 20, 2022, 8:43 p.m. UTC | #5
Lars-Dominik Braun <lars@6xq.net> skriver:

> Hi Marius,
>
>> Can you elaborate on the name clash issue?
> the problem seems to be that most packages do not put the source code
> into a subdirectory in the sdist. I.e. package foobar will have a foobar/
> directory, instead of src/foobar. Thus Python tries to load the module
> foobar.barbaz from that directory, instead of the store. Thus it’ll
> also look there for C extensions and fail. I don’t know why it does
> that, even when *not* using `python -m` – according to the documentation
> it should not. If anyone has any pointers I’d like to have a look again.

Perhaps it could be worked around by building in a separate directory,
i.e. ./build/lib, like setuptools does?  And ensure that the original
source directory does not end up on {,GUIX_}PYTHONPATH.

(I don't really understand the problem, just throwing ideas around.)

>> Unfortunately we need Python 2 for some time still.  It is used to
>> bootstrap old versions of GHC, Node, and probably other things.
> But as far as I see these specific examples do not use
> python-build-system. Only if some package using `#:python python-2` is
> required during bootstrapping, then we’d need a way to support Python
> 2, right?

Right.  If we don't need any Python 2 modules it should be fine to
remove support from the build system.

>> Do you think it is feasible to provide this as a new build system, say
>> pep517-build-system, during a transitional period?  Then we can a) start
>> using it right away for the increasing amount of packages that lack a
>> setup.py; and b) flesh out most bugs before eventually merging it back
>> (possibly piecemeal) into python-build-system.
> Actually, I think that’s a good idea. I tried, but I cannot fix all
> packages broken by this change on my own, so smoothing the transition
> could help and – as you said – we could catch bugs early on.
>
> I could prepare a patch, if there is interest in this path.

It would lower the bar significantly for testing and contributing, so I
would appreciate it.  :-)

>> * I also think trying "python setup.py test" is unnecessary.
> It still works quite often, although its usefulness will decrease in
> the future I guess. Another problem I see is that this command will not
> fail if there are no tests.
>
>> * It would be nice to support the "no tests" case without having to add
>>   explicit #:tests? everywhere.  Perhaps along the lines of...
> My idea was to force packagers to make and explain this decision
> explicitly. If you don’t run tests, you have to add `#:tests? #f`
> and leave a comment why they are disabled. I do see this could become
> a hassle with packages that simply don’t have any tests. But my hope
> is that it increases package quality.

My main concern is end-user experience when they just want to get some
random library working on their local channel.  But it's arguably
something that can be solved on the importer level and not a strong
opinion.

Thinking about importers, perhaps they could start emitting git origins
when possible, as there is a trend to strip tests before uploading to
PyPI.

>>         (_ (apply invoke `("python" "-m" "unittest" ,@test-flags))))))
> If I remember correctly I tried this and it did not work for some
> reason. I’ll have a look again.

Eh, it should be (apply invoke "python" "-m" "unittest" test-flags), but
you probably got that.  :-P

Thanks!
Maxim Cournoyer Feb. 26, 2022, 2:10 p.m. UTC | #6
Hi Lars-Dominik,

Lars-Dominik Braun <lars@6xq.net> writes:

When you judge the branch ready to merge, could you please send a subset
of the patches (at least the ones touching the python-build-system
directly) to this issue (marked as v2 -- git send-email -v2) so that
they can be more easily commented?

Thanks,

Maxim
Lars-Dominik Braun Feb. 28, 2022, 7:25 p.m. UTC | #7
Hi Maxim,

> When you judge the branch ready to merge, could you please send a subset
> of the patches (at least the ones touching the python-build-system
> directly) to this issue (marked as v2 -- git send-email -v2) so that
> they can be more easily commented?

as soon as time permits I can do that. Which route do we want to
take? Replace python-build-system entirely in one big merge or add a
new one and slowly migrate?

Cheers,
Lars
Maxim Cournoyer Feb. 28, 2022, 10:32 p.m. UTC | #8
Hi Lars,

Lars-Dominik Braun <lars@6xq.net> writes:

> Hi Maxim,
>
>> When you judge the branch ready to merge, could you please send a subset
>> of the patches (at least the ones touching the python-build-system
>> directly) to this issue (marked as v2 -- git send-email -v2) so that
>> they can be more easily commented?
>
> as soon as time permits I can do that. Which route do we want to
> take? Replace python-build-system entirely in one big merge or add a
> new one and slowly migrate?

After the Berlin can be rebooted onto its new file system, I expect it
should allow us to keep the build farm much busier than in the past give
us the headroom to experiment with a PEP 517-focused world rebuilding
branch.

I'd aim to have the build system completely replaced; unless your
experience suggests it's going to take multiple weeks to fix iron out
the kinks?

Thanks,

Maxim
Maxim Cournoyer Jan. 11, 2023, 3:41 p.m. UTC | #9
Hello!

With the pyproject build system now in master, can we close this issue?
Lars-Dominik Braun Feb. 10, 2023, 10:13 a.m. UTC | #10
Hi,

> With the pyproject build system now in master, can we close this issue?
yes, I think so.

Cheers,
Lars
diff mbox series

Patch

From ec72099a90b0fc7f6538f578ab3e28411391dacb Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Mon, 1 Mar 2021 17:21:35 +0100
Subject: [PATCH 14/14] gnu: scons: Remove obsolete argument.

* gnu/packages/python-xyz.scm (scons) [arguments]: Remove #:use-setuptools?
argument.
---
 gnu/packages/python-xyz.scm | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index fdc9bd85b7..a61451f8ea 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -2641,8 +2641,7 @@  and is not compatible with JSON.")
                "1xy8jrwz87y589ihcld4hv7wn122sjbz914xn8h50ww77wbhk8hn"))))
     (build-system python-build-system)
     (arguments
-     `(#:use-setuptools? #f                ; still relies on distutils
-       #:tests? #f                         ; no 'python setup.py test' command
+     `(#:tests? #f                         ; no 'python setup.py test' command
        #:phases
        (modify-phases %standard-phases
          (add-before 'build 'bootstrap
-- 
2.26.3