Hi Vagrant,
Vagrant Cascadian <vagrant@reproducible-builds.org> writes:
Toggle quote (36 lines)
> On 2024-05-21, Maxim Cournoyer wrote:
>> Vagrant Cascadian <vagrant@reproducible-builds.org> writes:
>>> On 2024-05-20, Vagrant Cascadian wrote:
>>>> On 2024-05-20, Maxim Cournoyer wrote:
>>>>> vagrant@reproducible-builds.org writes:
>>>>>> From: Vagrant Cascadian <vagrant@reproducible-builds.org>
>>>> 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. :)
Toggle quote (34 lines)
> 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