Subject: Re: bug#71024: Update diffoscope to 268

From: Maxim Cournoyer <[email-masked]>
Date: Sat, 25 May 2024 23:05:00 -0400

Hi Vagrant,

Vagrant Cascadian <[email-masked]> writes:

> On 2024-05-21, Maxim Cournoyer wrote:
>> Vagrant Cascadian <[email-masked]> writes:
>>> On 2024-05-20, Vagrant Cascadian wrote:
>>>> On 2024-05-20, Maxim Cournoyer wrote:
>>>>> [email-masked] writes:
>>>>>> From: Vagrant Cascadian <[email-masked]>
>>>> Or fixing diffoscope to work with the older xz version in master
>>>> (5.2.x?) that guix is already using, which, now that I have spelled out
>>>> all of the above, seems possibly a much better idea!
>>>
>>> This was "fixed" in upstream diffoscope git by setting a version
>>> requirement on the test, and I think this was a new test, so not exactly
>>> a regression in test coverage.
>>>
>>>   https://salsa.debian.org/reproducible-builds/diffoscope/-/commit/17c061e767e612540dd0227c3fd1f9cab460a78f
>>>
>>> So we could build diffoscope from that commit instead, or manually apply
>>> the patch, or just wait till the next diffoscope version.
>>
>> Given the xz horror story, waiting a bit more seems a good option to me.
>> Thanks for explaining it in more details; it seems upstream is working
>> on a cleaned up 5.8.0 version, which isn't ready yet.
>
> Well, version 268 was just released, which fixes the xz issue ... but
> introduces a test failure for 7z... which is worked around in the
> attached patch.
>
> The other option might be to package 7zip (which seems more maintained
> than p7zip)... but that seems a bit longer term.
>
> Yet another option would be to revert the patch that broke the test with
> p7zip based 7z.
>
> But for now I propose the workaround to effectively skip this one
> test. :)

[...]

> diff --git a/gnu/packages/diffoscope.scm b/gnu/packages/diffoscope.scm
> index fd2146456d..f7c5f07856 100644
> --- a/gnu/packages/diffoscope.scm
> +++ b/gnu/packages/diffoscope.scm
> @@ -75,7 +75,7 @@ (define-module (gnu packages diffoscope)
>  (define-public diffoscope
>    (package
>      (name "diffoscope")
> -    (version "265")
> +    (version "268")
>      (source
>       (origin
>         (method git-fetch)
> @@ -84,7 +84,7 @@ (define-public diffoscope
>               (commit version)))
>         (file-name (git-file-name name version))
>         (sha256
> -        (base32 "0fdaxihmzz1jf9ay8pwr1z60b2rnihawp4js4nw9l7wv0gij9vpg"))))
> +        (base32 "148r4grla92g9syjcxkyflxca2ydyb3rznb7wrrdl4zgpp4qirrh"))))
>      (build-system python-build-system)
>      (arguments
>       (list
> @@ -114,6 +114,13 @@ (define (bin command)
>                   (string-append "[\"" (bin command) "\","))
>                  (("\\[\"(getfacl)\"," _ command)
>                   (string-append "[\"" (bin command) "\",")))))
> +          (add-after 'unpack 'workaround-sevenz-versioned-test
> +            ;; Detects incorrect 7z version from p7zip:
> +            ;; https://salsa.debian.org/reproducible-builds/diffoscope/-/issues/376
> +            (lambda _
> +              (substitute* "tests/comparators/test_sevenz.py"
> +                ((", skip_unless_tool_is_at_least") ", skip_unless_tool_is_at_least, skip_unless_tool_is_at_most")
> +                (("@skip_unless_tool_is_at_least") "@skip_unless_tool_is_at_most(\"7z\", sevenz_version, \"63\")\n@skip_unless_tool_is_at_least"))))

That would LGTM, if it didn't so severely bust our 80 maximum column
width convention :-).  Would you mind editing it into conformance? You
can use actual newlines in the replacement text, and it's common to have
the substitute form broken on multiple lines like:

(substitute "some-file"
 (("some-pattern")
  "some-replacement"))

-- 
Thanks,
Maxim