diff mbox series

[bug#60240,v2] gnu: Add python-3.12 and python-next.

Message ID ZeL4e8_uI24-2OGc@noor.fritz.box
State New
Headers show
Series [bug#60240,v2] gnu: Add python-3.12 and python-next. | expand

Commit Message

Lars-Dominik Braun March 2, 2024, 9:59 a.m. UTC
Hey,

> Sorry, but I’m not sure to see what you mean!?
> Outputs are defined for `out`, `tk` and `idle`. And I see phases to move
> files to the outputs: `move-tk-inter` and `move-idle`.
> 
> What do you mean by `referenced from out`? And how would you remove such
> a reference!

so, if you look at `guix size python@3.12`, you can see
it lists tk and tcl. And then a `grep -Ri /gnu/store/path/to/tcl
/gnu/store/path/to/python` reveals it’s being referenced by
lib/python3.12/_sysconfigdata__linux_x86_64-linux-gnu.py and
lib/python3.12/config-3.12-x86_64-linux-gnu/Makefile, which are both
part of the default “out” output. That means TCL and TK are both
part of the closure of the python@3.12 package, i.e. they are downloaded
whenever you install it, making the package “bigger” in size (234.5
MiB in total to be precise). I believe we wanted to avoid that by moving
some parts of Python into the separate “tk” output. But apparently
that does not work (any more).

The attached patch (on top of yours) removes these references and shrinks
the closure of python@3.12 to 180.6 MiB. But I don’t know whether it
has any negative side-effects (i.e. packages not building any more),
because I can’t build libxslt with Python 3.12 due to the module
“imp” having been removed in Python 3.12.

> I searched in the issue list on GH, but couldn’t find anything relevant.
> But there are quite a lot of issues there.
> Do we have to fix this before we merge it?

No, we’d have to dig deeper into which particular test causes this
behavior.

And actually one more nitpick: The name of the package should be
“python-next” not “python”, otherwise `guix install python`
will pick Python 3.12 (it’s based on the name property, not the
variable name).

Cheers,
Lars

Comments

Tanguy LE CARROUR March 6, 2024, 12:49 p.m. UTC | #1
Hi Lars,


Quoting Lars-Dominik Braun (2024-03-02 10:59:23)
> And actually one more nitpick: The name of the package should be
> “python-next” not “python”, otherwise `guix install python`
> will pick Python 3.12 (it’s based on the name property, not the
> variable name).

Oh… thanks! Fixed in v3.
The package name only apply when used I input, right?
I submitted other patches where I did the same. But it’s a different matter
for Python libraries, I guess!? 🤔


> > Sorry, but I’m not sure to see what you mean!?
> > Outputs are defined for `out`, `tk` and `idle`. And I see phases to move
> > files to the outputs: `move-tk-inter` and `move-idle`.
> > 
> > What do you mean by `referenced from out`? And how would you remove such
> > a reference!
> 
> so, if you look at `guix size python@3.12`, you can see
> it lists tk and tcl. And then a `grep -Ri /gnu/store/path/to/tcl
> /gnu/store/path/to/python` reveals it’s being referenced by
> lib/python3.12/_sysconfigdata__linux_x86_64-linux-gnu.py and
> lib/python3.12/config-3.12-x86_64-linux-gnu/Makefile, which are both
> part of the default “out” output. That means TCL and TK are both
> part of the closure of the python@3.12 package, i.e. they are downloaded
> whenever you install it, making the package “bigger” in size (234.5
> MiB in total to be precise). I believe we wanted to avoid that by moving
> some parts of Python into the separate “tk” output. But apparently
> that does not work (any more).
> The attached patch (on top of yours) removes these references and shrinks
> the closure of python@3.12 to 180.6 MiB.

Oh-again! Thanks for taking the time to clarify!
I think I now see what you see:

```console
$ ./pre-inst-env guix size python-next
store item                                                       total    self
/gnu/store/g6w91736w66pszrg0bmcgwx84ahw7frd-python-next-3.12.2     180.6    82.3  45.6%
/gnu/store/gsjczqir1wbz8p770zndrpw4rnppmxi3-glibc-2.35              40.6    38.8  21.5%
/gnu/store/930nwsiysdvy2x5zv1sf6v7ym75z8ayk-gcc-11.3.0-lib          75.3    34.7  19.2%
/gnu/store/69wd3pd1hd3j84xr965jj2fk2qmxn0hl-openssl-3.0.8           83.4     8.1   4.5%
/gnu/store/bcc053jvsbspdjr17gnnd9dg85b3a0gy-ncurses-6.2.20210619    81.2     5.9   3.3%
/gnu/store/4jakqiibsvrkv4jdw1wyl6racrwv9bkh-sqlite-3.39.3           86.0     3.4   1.9%
/gnu/store/zzyywykw7kriln18rxqd82f0k5kidla7-bash-static-5.1.16       1.8     1.8   1.0%
/gnu/store/lxfc2a05ysi7vlaq0m3w5wsfsy0drdlw-readline-8.1.2          82.6     1.4   0.8%
/gnu/store/6k1yys9wqrfn4y41ic1win8gpnimncwj-xz-5.2.8                77.7     1.4   0.8%
/gnu/store/rib9g2ig1xf3kclyl076w28parmncg4k-bash-minimal-5.1.16     41.6     1.0   0.6%
/gnu/store/2w976k6g70gkfih9wwhalqsni209vcqz-gdbm-1.23               75.9     0.6   0.4%
/gnu/store/pl09vk5g3cl8fxfln2hjk996pyahqk8m-bzip2-1.0.8             76.7     0.4   0.2%
/gnu/store/fw1wywd34vh33l4dq182ds5d7jdz45j5-expat-2.5.0             75.7     0.4   0.2%
/gnu/store/slzq3zqwj75lbrg4ly51hfhbv2vhryv5-zlib-1.2.13             75.5     0.2   0.1%
/gnu/store/w8b0l8hk6g0fahj4fvmc4qqm3cvaxnmv-libffi-3.4.4            75.5     0.2   0.1%
total: 180.6 MiB
```

No more Tcl/Tk, thanks! 👍
If you don’t mind, I squashed your modification in v3.


>  But I don’t know whether it has any negative side-effects (i.e.
>  packages not building any more), because I can’t build libxslt with
>  Python 3.12 due to the module “imp” having been removed in Python
>  3.12.

Mmmm… is it something that has to be fixed in `libxslt`?
More likely, it comes from `python-lxml`. Our current version is 4.9.1,
and one test file (`src/lxml/html/tests/test_html5parser.py`) depends on
`imp` for `imp.new_module`.
This has been dropped in 5.1.0.

Regards,
diff mbox series

Patch

diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index 471cf190fb..0b887a2cdb 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -836,7 +836,7 @@  (define-public python-3.12
                                          "json/tests"
                                          "distutils/tests"))))))))
                   (add-after 'remove-tests 'move-tk-inter
-                    (lambda* (#:key outputs #:allow-other-keys)
+                    (lambda* (#:key outputs inputs #:allow-other-keys)
                       ;; When Tkinter support is built move it to a separate output so
                       ;; that the main output doesn't contain a reference to Tcl/Tk.
                       (let ((out (assoc-ref outputs "out"))
@@ -854,7 +854,14 @@  (define-public python-3.12
                                                                         len)
                                                            "/site-packages")))
                                (install-file tkinter.so target)
-                               (delete-file tkinter.so))))))))
+                               (delete-file tkinter.so))))
+                          ;; Remove explicit store path references.
+                          (let ((tcl (assoc-ref inputs "tcl"))
+                                (tk (assoc-ref inputs "tk")))
+                            (substitute* (find-files (string-append out "/lib")
+                                                     "^(_sysconfigdata_.*\\.py|Makefile)$")
+                                         (((string-append "-L" tk "/lib")) "")
+                                         (((string-append "-L" tcl "/lib")) "")))))))
                   (add-after 'move-tk-inter 'move-idle
                     (lambda* (#:key outputs #:allow-other-keys)
                       ;; when idle is built, move it to a separate output to save some