[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

Debbugs page

david larsson wrote 4 years ago
(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)
Marius Bakke wrote 4 years ago
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-----

david larsson wrote 4 years ago
(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
Marius Bakke wrote 4 years ago
(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-----

david larsson wrote 4 years ago
(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)
Marius Bakke wrote 4 years ago
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-----

david larsson wrote 4 years ago
(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
david larsson wrote 4 years ago
(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)
Marius Bakke wrote 4 years ago
(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
?
Your comment

This issue is archived.

To comment on this conversation send an email to 47898@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 47898
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help