| Message ID | 87bk4vp913.fsf@wireframe |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#71024] Update diffoscope to 268 | expand |
Hi Vagrant, Vagrant Cascadian <vagrant@reproducible-builds.org> writes: > 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. :) [...] > 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"))
On 2024-05-25, Maxim Cournoyer wrote: > Vagrant Cascadian <vagrant@reproducible-builds.org> writes: >> 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! ... >> 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. ... >> + (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? Oops. must not have guix linted that one! Well, again, upstream has fixed it even better: https://salsa.debian.org/reproducible-builds/diffoscope/-/commit/2a361d7dff135d3e832161f587a55a62fcbec9f2 Tested fine for me building against that commit. So ... wait again till another release (and maybe find new breakage in the new release), or build against an untagged commit, or pull in a patch from the upstream commit? :) live well, vagrant
Hi Vagrant, Vagrant Cascadian <vagrant@reproducible-builds.org> writes: > On 2024-05-25, Maxim Cournoyer wrote: >> Vagrant Cascadian <vagrant@reproducible-builds.org> writes: >>> 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! > ... >>> 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. > ... >>> + (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? > > Oops. must not have guix linted that one! > > > Well, again, upstream has fixed it even better: > > https://salsa.debian.org/reproducible-builds/diffoscope/-/commit/2a361d7dff135d3e832161f587a55a62fcbec9f2 > > Tested fine for me building against that commit. > > So ... wait again till another release (and maybe find new breakage in > the new release), or build against an untagged commit, or pull in a > patch from the upstream commit? :) If you want to get done with this, I'd recommend pulling a patch from the upstream commit. Then when we get to upgrading to the next release we can drop such patch. Hope that helps,
On 2024-05-29, Maxim Cournoyer wrote: > Vagrant Cascadian <vagrant@reproducible-builds.org> writes: >> Well, again, upstream has fixed it even better: >> >> https://salsa.debian.org/reproducible-builds/diffoscope/-/commit/2a361d7dff135d3e832161f587a55a62fcbec9f2 >> >> Tested fine for me building against that commit. >> >> So ... wait again till another release (and maybe find new breakage in >> the new release), or build against an untagged commit, or pull in a >> patch from the upstream commit? :) > > If you want to get done with this, I'd recommend pulling a patch from > the upstream commit. Then when we get to upgrading to the next release > we can drop such patch. Apparently waiting was the thing to do... 269 was released with only that change! Pushed as c7888f5361fbdbe5182e7dbe90ccc12e2d95d3c3. live well, vagrant
From 31c4b7ce41eb36aaf58913b3cc17021643e5fefe Mon Sep 17 00:00:00 2001 From: Vagrant Cascadian <vagrant@reproducible-builds.org> Date: Fri, 24 May 2024 04:16:32 -0700 Subject: [PATCH] gnu: diffoscope: Update to 268. * gnu/packages/diffoscope.scm (diffoscope): Update to 268. [phases] Add 'workaround-sevenz-versioned-test. --- gnu/packages/diffoscope.scm | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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")))) (add-after 'build 'build-man-page (lambda _ (invoke "make" "-C" "doc"))) base-commit: c5e63e19ac672f9e63fc8ee98fa9a16f978ce19c -- 2.39.2