diff mbox series

[bug#63044] gnu: python-setuptools: Disable date checking in bdist_egg.py

Message ID 878re4q4z3.fsf@psyduck.jhoto.kublai.com
State New
Headers show
Series [bug#63044] gnu: python-setuptools: Disable date checking in bdist_egg.py | expand

Commit Message

Brian Cully May 4, 2023, 12:37 a.m. UTC 
Brian Cully <bjc@spork.org> writes:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> The way to do that is to add (guix build python-build-system) 
>> to
>> #:imported-modules and #:modules in those packages, and then to 
>> use
>> ‘modify-phases’ to add that ‘ensure-no-mtimes-pre-1980’ phase 
>> to
>> those
>> of ‘gnu-build-system’.
>>
>> How does that sound, Brian?
>
> That sounds reasonable to me. I'll update the ticket in a bit 
> with
> these changes.

This turns out to be trickier than it first appears:

For one thing, ‘ensure-no-mtimes-pre-1980’ isn't exported from 
(guix build python-build-system). I can use ‘@@’ to reference it, 
but that seems less-than-ideal.

A larger problem is that ‘ensure-no-mtimes-pre-1980’ has a bug: it 
blindly tries to change the utime of files even if they're 
symlinks, which raises an “operation not permitted” 
exception. It's easy enough to fix this, but since it changes 
‘python-build-system’, it causes a tremendous amount of 
rebuilding.

I'm not sure how to proceed. The bug with the build system should 
certainly be fixed, but it'll be disruptive. Until then, at least 
‘criu’ will remain broken — though ‘sssd’ is now building 
correctly with the new phase.

This is the patch:

--8<---------------cut here---------------start------------->8---
--8<---------------cut here---------------end--------------->8---


-bjc

Comments

Ludovic Courtès May 10, 2023, 3:18 p.m. UTC | #1 
Hi,

Brian Cully <bjc@spork.org> skribis:

> For one thing, ‘ensure-no-mtimes-pre-1980’ isn't exported from 
> (guix build python-build-system). I can use ‘@@’ to reference it, 
> but that seems less-than-ideal.
>
> A larger problem is that ‘ensure-no-mtimes-pre-1980’ has a bug: it 
> blindly tries to change the utime of files even if they're 
> symlinks, which raises an “operation not permitted” 
> exception. It's easy enough to fix this, but since it changes 
> ‘python-build-system’, it causes a tremendous amount of 
> rebuilding.

Yes.  I’m surprised we didn’t hit a problem before, despite having
thousands of Python packages.  Can we work around it for the one package
that hits that bug?  (Until a fix like you propose lands.)

> I'm not sure how to proceed. The bug with the build system should 
> certainly be fixed, but it'll be disruptive. Until then, at least 
> ‘criu’ will remain broken — though ‘sssd’ is now building 
> correctly with the new phase.

If you can use @@ to get at ‘ensure-no-mtimes-pre-1980’, I’d still
suggest to do that for ‘criu’.

Does that make sense?  Or am I overlooking something?

Thanks,
Ludo’.
Brian Cully May 12, 2023, 10:37 p.m. UTC | #2 
Ludovic Courtès <ludo@gnu.org> writes:

> Yes.  I’m surprised we didn’t hit a problem before, despite 
> having
> thousands of Python packages.  Can we work around it for the one 
> package
> that hits that bug?  (Until a fix like you propose lands.)

Yeah, I've given up trying to do this the “right” way. See #63149 
for an updated patch that just copies in the ensure-mtime 
procedure with a change to check for symlinks.

> If you can use @@ to get at ‘ensure-no-mtimes-pre-1980’, I’d 
> still
> suggest to do that for ‘criu’.

This is what I've done for ‘sssd’, see #63091. For ‘criu’, see 
above.

-bjc
Lars-Dominik Braun May 13, 2023, 7:11 a.m. UTC | #3 
Hi,

> --8<---------------cut here---------------start------------->8---
> diff --git a/guix/build/python-build-system.scm 
> b/guix/build/python-build-system.scm
> index aa04664b25..1236ea62cd 100644
> --- a/guix/build/python-build-system.scm
> +++ b/guix/build/python-build-system.scm
> @@ -270,7 +270,8 @@ (define* (ensure-no-mtimes-pre-1980 #:rest _)
>    ;; timestamps before 1980.
>    (let ((early-1980 315619200))  ; 1980-01-02 UTC
>      (ftw "." (lambda (file stat flag)
> -               (unless (<= early-1980 (stat:mtime stat))
> +               (unless (or (<= early-1980 (stat:mtime stat))
> +                           (eq? (stat:type stat) 'symlink))
>                   (utime file early-1980 early-1980))
>                 #t))))
>  
> --8<---------------cut here---------------end--------------->8---

I’m going to include this patch in the python-team branch. We’re
looking at a massive rebuild due to #63139 anyways, as you said. In
theory there’s nothing wrong with patching setuptools like this, but
I’d like to avoid it, because that substitute* can just break silently
in the future.

Cheers,
Lars
diff mbox series

Patch

diff --git a/guix/build/python-build-system.scm 
b/guix/build/python-build-system.scm
index aa04664b25..1236ea62cd 100644
--- a/guix/build/python-build-system.scm
+++ b/guix/build/python-build-system.scm
@@ -270,7 +270,8 @@  (define* (ensure-no-mtimes-pre-1980 #:rest _)
   ;; timestamps before 1980.
   (let ((early-1980 315619200))  ; 1980-01-02 UTC
     (ftw "." (lambda (file stat flag)
-               (unless (<= early-1980 (stat:mtime stat))
+               (unless (or (<= early-1980 (stat:mtime stat))
+                           (eq? (stat:type stat) 'symlink))
                  (utime file early-1980 early-1980))
                #t))))