[PATCH] [core-updates] Add option --xpath0 to xmllint from libxml2

  • Done
  • quality assurance status badge
Details
2 participants
  • david larsson
  • Marius Bakke
Owner
unassigned
Submitted by
david larsson
Severity
normal
D
D
david larsson wrote on 19 Apr 2021 20:48
(address . guix-patches@gnu.org)
20aec8da67d18b52e5a166f45cab40b2@selfhosted.xyz
Hi!
This patch adds the option to separate xpath results from xmllint by a
null delimiter when using the --xpath0 option-flag. It is something
that's been asked for for a long time by users on stackoverflow and
merge-attempts have been made to upstream without success.

Examples:

Best regards,
David
Attachment: 0001-gnu-libxml2-Add-patch-for-xpath0-option-to-xmllint.patch (.01 MiB)
M
M
Marius Bakke wrote on 13 May 2021 14:41
87eeeaiz5h.fsf@gnu.org
david larsson <david.larsson@selfhosted.xyz> skriver:

Toggle quote (10 lines)
> Hi!
> This patch adds the option to separate xpath results from xmllint by a
> null delimiter when using the --xpath0 option-flag. It is something
> that's been asked for for a long time by users on stackoverflow and
> merge-attempts have been made to upstream without success.
>
> Examples:
> - https://gitlab.gnome.org/GNOME/libxml2/-/issues/227
> - https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/8

I'm reluctant to take a patch that has been rejected upstream,
especially for a core package such as libxml2. Porting can be
time-consuming which delays security updates, and we'll have situations
where something "works on Guix" but not elsewhere (or vice versa).

Will adding it as a separate package be sufficient? We could call it
"xmllint-xpath0" or similar and make it clear that it's just libxml2
with a patched xmllint in the description.

Thoughts?
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYJ0eig8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHfApQEAk6EMXLEkfBWhoRiU/ww+Z3pXlzmdBJArQ/4u
wBuSbOAA/iTqT8W3lfj+VaEHqzIWCOI3bqAkvacO8l58uxExK6UO
=Myx0
-----END PGP SIGNATURE-----

D
D
david larsson wrote on 13 May 2021 15:21
(name . Marius Bakke)(address . marius@gnu.org)(address . 47898@debbugs.gnu.org)
48c1517d3239d55826324aa8f2bc9d4d@selfhosted.xyz
On 2021-05-13 14:41, Marius Bakke wrote:
Toggle quote (17 lines)
> david larsson <david.larsson@selfhosted.xyz> skriver:
>
>> Hi!
>> This patch adds the option to separate xpath results from xmllint by a
>> null delimiter when using the --xpath0 option-flag. It is something
>> that's been asked for for a long time by users on stackoverflow and
>> merge-attempts have been made to upstream without success.
>>
>> Examples:
>> - https://gitlab.gnome.org/GNOME/libxml2/-/issues/227
>> - https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/8
>
> I'm reluctant to take a patch that has been rejected upstream,
> especially for a core package such as libxml2. Porting can be
> time-consuming which delays security updates, and we'll have situations
> where something "works on Guix" but not elsewhere (or vice versa).

I understand.

Toggle quote (7 lines)
>
> Will adding it as a separate package be sufficient? We could call it
> "xmllint-xpath0" or similar and make it clear that it's just libxml2
> with a patched xmllint in the description.
>
> Thoughts?

Yes, that would be nice. How about calling it libxml2-xpath0 instead?
That would maintain libxml2 in the name, letting users know it's just a
version of the original package and should show up when searching for
libxml2. If calling it xmllint-xpath0, I feel that the package should
remove other outputs than just the xmllint binary file, which is perhaps
a good idea though?

Either option is fine for me.

Best regards,
David
M
M
Marius Bakke wrote on 13 May 2021 15:29
(name . david larsson)(address . david.larsson@selfhosted.xyz)(address . 47898@debbugs.gnu.org)
87bl9eiwx5.fsf@gnu.org
david larsson <david.larsson@selfhosted.xyz> skriver:

Toggle quote (16 lines)
> On 2021-05-13 14:41, Marius Bakke wrote:
>> david larsson <david.larsson@selfhosted.xyz> skriver:
>>
>> Will adding it as a separate package be sufficient? We could call it
>> "xmllint-xpath0" or similar and make it clear that it's just libxml2
>> with a patched xmllint in the description.
>>
>> Thoughts?
>
> Yes, that would be nice. How about calling it libxml2-xpath0 instead?
> That would maintain libxml2 in the name, letting users know it's just a
> version of the original package and should show up when searching for
> libxml2. If calling it xmllint-xpath0, I feel that the package should
> remove other outputs than just the xmllint binary file, which is perhaps
> a good idea though?

libxml2-xpath0 is probably better indeed, I don't have a strong opinion.
Can you send an updated patch? :-)

Thanks,
Marius
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYJ0p1g8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHccigD/cEYdjZGWr2L/DXNkuIXuUld779u+qN2GyBaf
tZUX1GsA/jJcoa4P6iFdo8rfa2cATHavlOtUzVA5sDUc+a7/6LoO
=xEHg
-----END PGP SIGNATURE-----

D
D
david larsson wrote on 15 May 2021 14:53
(address . 47898@debbugs.gnu.org)
dd8d7dea34d4305f27637802f836ef8a@selfhosted.xyz
Toggle quote (7 lines)
> libxml2-xpath0 is probably better indeed, I don't have a strong
> opinion.
> Can you send an updated patch? :-)
>
> Thanks,
> Marius

Updated patch attached!

Best regards,
David
Attachment: 0001-gnu-libxml2-xpath0-New-package-with-patch-for-xpath0.patch (.01 MiB)
M
M
Marius Bakke wrote on 15 May 2021 18:12
875yzkht7g.fsf@gnu.org
david larsson <david.larsson@selfhosted.xyz> skriver:

Toggle quote (9 lines)
>> libxml2-xpath0 is probably better indeed, I don't have a strong
>> opinion.
>> Can you send an updated patch? :-)
>>
>> Thanks,
>> Marius
>
> Updated patch attached!

Thanks!

[...]

Toggle quote (8 lines)
> gnu/packages/patches/libxml2-Add-option-xpath0.patch: New file...
> gnu/packages/xml.scm (libxml2-xpath0) [source]: ...apply it.
> ---
> .../patches/libxml2-Add-option-xpath0.patch | 139 ++++++++++++++++++
> gnu/packages/xml.scm | 52 +++++++
> 2 files changed, 191 insertions(+)
> create mode 100644 gnu/packages/patches/libxml2-Add-option-xpath0.patch

Please also register this patch in gnu/local.mk. Can you also add your
copyright at the top of xml.scm?

[...]

Toggle quote (13 lines)
> +(define-public libxml2-xpath0
> + (package
> + (name "libxml2-xpath0")
> + (version "2.9.10")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append "ftp://xmlsoft.org/libxml2/libxml2-"
> + version ".tar.gz"))
> + (sha256
> + (base32
> + "07xynh8hcxb2yb1fs051xrgszjvj37wnxvxgsj10rzmqzy9y3zma"))
> + (patches (list (search-patch "libxml2-Add-option-xpath0.patch")))))

You can inherit another record in Scheme to avoid duplicating all the
fields. Then the package can be shortened to:

(define-public libxml2-xpath0
(package/inherit libxml2
(name "libxml2-xpath0")
(source (origin
(inherit (package-source libxml2))
(patches (append (search-patches "libxml2-Add-option-xpath0.patch")
(origin-patches (package-source libxml2))))))
(description
"...")))

We should fill out that description to mention how it differs from the
regular libxml2. Can you give it a try? I can make the other changes
on your behalf, but not sure what to write.

Thanks,
Marius
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYJ/y4w8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHeAcAEA9arySw4EJ9t8tNYXWlwJFLwk/8YVzaVECJ00
PPTQwysA/iSv/GRtJeicerQ9AUh+uiSP1uWs8FH52NkrgwPZHe8H
=ycNQ
-----END PGP SIGNATURE-----

D
D
david larsson wrote on 15 May 2021 19:12
(name . Marius Bakke)(address . marius@gnu.org)(address . 47898@debbugs.gnu.org)
350a1a3c73fb0040aadbba0fd34c6518@selfhosted.xyz
On 2021-05-15 18:12, Marius Bakke wrote:
Toggle quote (67 lines)
> david larsson <david.larsson@selfhosted.xyz> skriver:
>
>>> libxml2-xpath0 is probably better indeed, I don't have a strong
>>> opinion.
>>> Can you send an updated patch? :-)
>>>
>>> Thanks,
>>> Marius
>>
>> Updated patch attached!
>
> Thanks!
>
> [...]
>
>> gnu/packages/patches/libxml2-Add-option-xpath0.patch: New file...
>> gnu/packages/xml.scm (libxml2-xpath0) [source]: ...apply it.
>> ---
>> .../patches/libxml2-Add-option-xpath0.patch | 139
>> ++++++++++++++++++
>> gnu/packages/xml.scm | 52 +++++++
>> 2 files changed, 191 insertions(+)
>> create mode 100644
>> gnu/packages/patches/libxml2-Add-option-xpath0.patch
>
> Please also register this patch in gnu/local.mk. Can you also add your
> copyright at the top of xml.scm?
>
> [...]
>
>> +(define-public libxml2-xpath0
>> + (package
>> + (name "libxml2-xpath0")
>> + (version "2.9.10")
>> + (source (origin
>> + (method url-fetch)
>> + (uri (string-append "ftp://xmlsoft.org/libxml2/libxml2-"
>> + version ".tar.gz"))
>> + (sha256
>> + (base32
>> +
>> "07xynh8hcxb2yb1fs051xrgszjvj37wnxvxgsj10rzmqzy9y3zma"))
>> + (patches (list (search-patch
>> "libxml2-Add-option-xpath0.patch")))))
>
> You can inherit another record in Scheme to avoid duplicating all the
> fields. Then the package can be shortened to:
>
> (define-public libxml2-xpath0
> (package/inherit libxml2
> (name "libxml2-xpath0")
> (source (origin
> (inherit (package-source libxml2))
> (patches (append (search-patches
> "libxml2-Add-option-xpath0.patch")
> (origin-patches (package-source
> libxml2))))))
> (description
> "...")))
>
> We should fill out that description to mention how it differs from the
> regular libxml2. Can you give it a try? I can make the other changes
> on your behalf, but not sure what to write.
>
> Thanks,
> Marius

I can fix all of it and send an updated patch again.

Best regards,
David
D
D
david larsson wrote on 15 May 2021 21:21
(name . Marius Bakke)(address . marius@gnu.org)(address . 47898@debbugs.gnu.org)
2f7a02b8290c9728644a9d2c2caad0b9@selfhosted.xyz
Toggle quote (3 lines)
> Please also register this patch in gnu/local.mk. Can you also add your
> copyright at the top of xml.scm?

Done!

Toggle quote (40 lines)
>
> [...]
>
>> +(define-public libxml2-xpath0
>> + (package
>> + (name "libxml2-xpath0")
>> + (version "2.9.10")
>> + (source (origin
>> + (method url-fetch)
>> + (uri (string-append "ftp://xmlsoft.org/libxml2/libxml2-"
>> + version ".tar.gz"))
>> + (sha256
>> + (base32
>> +
>> "07xynh8hcxb2yb1fs051xrgszjvj37wnxvxgsj10rzmqzy9y3zma"))
>> + (patches (list (search-patch
>> "libxml2-Add-option-xpath0.patch")))))
>
> You can inherit another record in Scheme to avoid duplicating all the
> fields. Then the package can be shortened to:
>
> (define-public libxml2-xpath0
> (package/inherit libxml2
> (name "libxml2-xpath0")
> (source (origin
> (inherit (package-source libxml2))
> (patches (append (search-patches
> "libxml2-Add-option-xpath0.patch")
> (origin-patches (package-source
> libxml2))))))
> (description
> "...")))
>
> We should fill out that description to mention how it differs from the
> regular libxml2. Can you give it a try? I can make the other changes
> on your behalf, but not sure what to write.
>
> Thanks,
> Marius

Hi, new patch attached. There's a lint warning though:
"libxml2-xpath0@2.9.10: no updater for libxml2-xpath0". I don't know
what it means, maybe it should be fixed before committing?

Best regards,
David
Attachment: 0001-gnu-libxml2-xpath0-New-package-with-patch-for-xpath0.patch (.01 MiB)
M
M
Marius Bakke wrote on 18 May 2021 22:25
(name . david larsson)(address . david.larsson@selfhosted.xyz)(address . 47898-done@debbugs.gnu.org)
87im3fbxh1.fsf@gnu.org
david larsson <david.larsson@selfhosted.xyz> skriver:

Toggle quote (4 lines)
> Hi, new patch attached. There's a lint warning though:
> "libxml2-xpath0@2.9.10: no updater for libxml2-xpath0". I don't know
> what it means, maybe it should be fixed before committing?

That warning is harmless and affects a lot of packages. I've applied
the patch with a couple minor tweaks:

Toggle quote (10 lines)
> +From e1df743329bdfd94fbfdea18303c5c6c6fe13403 Mon Sep 17 00:00:00 2001
> +From: methuselah-0 <david.larsson@selfhosted.xyz>
> +Date: Thu, 1 Apr 2021 08:33:56 +0200
> +Subject: [PATCH] Add option --xpath0
> +
> +---
> + doc/xmllint.xml | 16 ++++++++++++++++
> + xmllint.c | 32 +++++++++++++++++++++++---------
> + 2 files changed, 39 insertions(+), 9 deletions(-)

I replaced this git-style patch header with a short description and an
URL to the upstream issue, like we usually do.

Toggle quote (13 lines)
> +(define-public libxml2-xpath0
> + (package/inherit libxml2
> + (name "libxml2-xpath0")
> + (source (origin
> + (inherit (package-source libxml2))
> + (patches (append (search-patches
> + "libxml2-xpath0-Add-option-xpath0.patch")
> + (origin-patches (package-source libxml2))))))
> + (description
> + "Libxml2-xpath0 is like libxml2 but with a patch applied that
> +provides an --xpath0 option to xmllint that enables it to output xpath
> +results with a null delimiter")))

...and sprinkled some markup and punctuation into the description.

Pushed as b58efbc6611550ad9234163e198ff71ace5306ea, thank you!
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYKQiyg8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHcVzQD/c0IWBYUxvY42oYIeknVrHC1QIvpkbVSyvxyU
PVYhLEkBAK54ZKGSAYL7yjrmYvjXGrHw/P5diRMkxLbTiF9D8RUI
=YxIh
-----END PGP SIGNATURE-----

Closed
?