Bug 2174383 - Review Request: libunicode - Modern C++17 Unicode library
Summary: Review Request: libunicode - Modern C++17 Unicode library
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
medium
unspecified
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/contour-terminal/l...
Whiteboard:
Depends On:
Blocks: 2174384
TreeView+ depends on / blocked
 
Reported: 2023-03-01 12:32 UTC by Felix Wang
Modified: 2023-03-06 01:10 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-03-06 01:10:36 UTC
Type: Bug
Embargoed:
zbyszek: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 5582423 to 5592162 (249 bytes, patch)
2023-03-04 12:13 UTC, Jakub Kadlčík
no flags Details | Diff
The .spec file difference from Copr build 5592162 to 5593347 (1.17 KB, patch)
2023-03-04 23:49 UTC, Jakub Kadlčík
no flags Details | Diff

Description Felix Wang 2023-03-01 12:32:54 UTC
SPEC URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhide-x86_64/05581922-libunicode/libunicode.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhide-x86_64/05581922-libunicode/libunicode-0.3.0-1.fc39.src.rpm

Description: Modern C++17 Unicode library

FAS Username: topazus

This library is dependency of contour terminal package which I also will create in another review request of new package.
libunicode repo URL: https://github.com/contour-terminal/libunicode
contour terminal repo: URL: https://github.com/contour-terminal/contour

Comment 1 Felix Wang 2023-03-01 12:37:05 UTC
I noticed that there are some changes about Catch2 v2 and v3 package. https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/U2FLBYI3VSLKKT35TVEMZ3DKLXWVGTFT/. The BuildRequires of catch-devel or catch2-devel worked for now, it need to be modified after pushing stable update about catch2 package.

Comment 2 Jakub Kadlčík 2023-03-01 12:46:30 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5582423
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2174383-libunicode/fedora-rawhide-x86_64/05582423-libunicode/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 4 Jakub Kadlčík 2023-03-04 12:13:43 UTC
Created attachment 1947841 [details]
The .spec file difference from Copr build 5582423 to 5592162

Comment 5 Jakub Kadlčík 2023-03-04 12:13:46 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5592162
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2174383-libunicode/fedora-rawhide-x86_64/05592162-libunicode/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 6 Zbigniew Jędrzejewski-Szmek 2023-03-04 12:58:06 UTC
> BuildRequires:  gcc-c++ cmake unzip
> BuildRequires:  range-v3-devel fmt-devel
One per line, please.

> pushd _ucd/ucd-%{ucd_version}
>    unzip %{SOURCE1}
> popd
'unzip -d _ucd/ucd-%{ucd_version} %{SOURCE1}' ?

Hmm, we already have the UCD available as unicode-ucd? Why not use that instead.
Please provide some explanation what %SOURCE1 is used for.

> %{_libdir}/%{name}*.so.*
This glob is too generic. See
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_listing_shared_library_files

> libunicode-tools.x86_64: W: no-manual-page-for-binary unicode-query
This would be nice-to-have, esp. that unicode-query just treats all arguments
as stuff to query.

> libunicode.x86_64: W: library-not-linked-against-libc /usr/lib64/libunicode_ucd.so.0.2.0
>  6 packages and 0 specfiles checked; 0 errors, 2 warnings, 0 badness; has taken 0.7 s 

Hmm.
$ ldd /usr/lib64/libunicode_ucd.so.0.2.0 | rg libc
	libc.so.6 => /lib64/libc.so.6 (0x00007f16f5e21000)
I think rpmlint got confused somehow.

Comment 7 Felix Wang 2023-03-04 23:37:51 UTC
As for UCD, I asked project maintainer about it. I may set custom LIBUNICODE_UCD_DIR to cmake (/usr/share/unicode/ucd, where ucd files provided by unicode-ucd package) with a patch. ref: https://github.com/contour-terminal/libunicode/issues/68
I modified the file as you suggested.
SPEC URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhide-x86_64/05593342-libunicode/libunicode.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhide-x86_64/05593342-libunicode/libunicode-0.3.0-1.fc39.src.rpm

Comment 8 Jakub Kadlčík 2023-03-04 23:49:14 UTC
Created attachment 1947989 [details]
The .spec file difference from Copr build 5592162 to 5593347

Comment 9 Jakub Kadlčík 2023-03-04 23:49:16 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5593347
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2174383-libunicode/fedora-rawhide-x86_64/05593347-libunicode/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 10 Zbigniew Jędrzejewski-Szmek 2023-03-05 18:41:13 UTC
Using unicode-ucd is nice. We avoid having two independent version
of the same dataset. I would suggest that you subscribe to notifications
about unicode-ucd so that you know when to rebuild this package.

+ package name is OK
+ latest version
+ license is acceptable for Fedora (Apache-2.0)
+ license is specified correctly (SPDX)
+ builds and installs OK
+ BR/R/P look OK
+ rpmlint and fedora-review don't find any major issues, see below.

rpmlint:
> libunicode.x86_64: W: library-not-linked-against-libc /usr/lib64/libunicode_ucd.so.0.2.0
Seems to be a false positive.
> libunicode-tools.x86_64: W: no-manual-page-for-binary unicode-query
As mentioned above, this is just nice-to-have.
> 6 packages and 0 specfiles checked; 0 errors, 2 warnings, 0 badness; has taken 1.1 s 

Package is APPROVED.

Comment 11 Fedora Admin user for bugzilla script actions 2023-03-06 00:19:54 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/libunicode

Comment 12 Felix Wang 2023-03-06 00:25:32 UTC
Very appreciated for your reviewing work.

Comment 13 Fedora Update System 2023-03-06 01:09:58 UTC
FEDORA-2023-3a16d3bec7 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-3a16d3bec7

Comment 14 Fedora Update System 2023-03-06 01:10:36 UTC
FEDORA-2023-3a16d3bec7 has been pushed to the Fedora 39 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.