https://bugzilla.redhat.com/show_bug.cgi?id=2174384
Bug ID: 2174384 Summary: Review Request: contour-terminal - Modern C++ Terminal Emulator Product: Fedora Version: rawhide Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: topazus@outlook.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
SPEC URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhi... SRPM URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhi...
Description: Modern C++ Terminal Emulator
FAS Username: topazus
contour terminal repo: URL: https://github.com/contour-terminal/contour
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #1 from Felix Wang topazus@outlook.com --- updated: SPEC URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhi... SRPM URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhi...
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
Benson Muite benson_muite@emailplus.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? CC| |benson_muite@emailplus.org Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |benson_muite@emailplus.org
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #2 from Felix Wang topazus@outlook.com --- As the dependent bugzilla review request was approved, we may start to move on this.
https://bugzilla.redhat.com/show_bug.cgi?id=2174384 Bug 2174384 depends on bug 2174383, which changed state.
Bug 2174383 Summary: Review Request: libunicode - Modern C++17 Unicode library https://bugzilla.redhat.com/show_bug.cgi?id=2174383
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #3 from Benson Muite benson_muite@emailplus.org --- Desktop files should be validated and have a recommended installation procedure:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_files https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
Please give the spec file the same name as the package, contour-terminal.spec
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #4 from Felix Wang topazus@outlook.com --- I may have corrected as you pointed out. SPEC URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhi... SRPM URL: https://download.copr.fedorainfracloud.org/results/topazus/raku/fedora-rawhi...
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
Jakub Kadlčík jkadlcik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://github.com/contour- | |terminal/contour
--- Comment #5 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5597218 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please make sure the package builds successfully at least for Fedora Rawhide.
- If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field
--- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #6 from Felix Wang topazus@outlook.com --- The failure is because of the missing BuildRequires, I can successfully built on my copr. ref: https://copr.fedorainfracloud.org/coprs/topazus/raku/build/5597199/
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #7 from Benson Muite benson_muite@emailplus.org --- Problems pointed out by fedora-review: a) Check installation procedure for these desktop file icons in another GUI application: Note: Directories without known owners: /usr/share/kservices5/ServiceMenus, /usr/share/icons/hicolor/512x512/apps, /usr/share/terminfo, /usr/share/icons/hicolor/32x32, /usr/share/icons/hicolor/128x128/apps, /usr/share/icons/hicolor/128x128, /usr/share/icons/hicolor, /usr/share/kservices5, /usr/share/icons/hicolor/32x32/apps, /usr/share/icons/hicolor/256x256, /usr/share/icons/hicolor/64x64, /usr/share/terminfo/c, /usr/share/icons/hicolor/512x512, /usr/share/icons/hicolor/256x256/apps, /usr/share/icons/hicolor/64x64/apps Perhaps see if upstream can modify: https://github.com/contour-terminal/contour/blob/master/src/contour/CMakeLis... Maybe https://cmake.org/cmake/help/latest/cpack_gen/rpm.html#cpack_gen:CPack%20RPM... might also be helpful
b) Remove one of these and use a soft link W: files-duplicate /usr/share/terminfo/c/contour-latest /usr/share/terminfo/c/contour
c) Check/modify linking: E: explicit-lib-dependency libunicode Maybe ninja build will not do this?
d) You can run fedora-review in Copr: https://frostyx.cz/posts/running-fedora-review-after-copr-build
e) Consider using Ninja as upstream tests with this: https://github.com/contour-terminal/contour/blob/master/.github/fedora/conto...
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #8 from Felix Wang topazus@outlook.com --- problem a, b an d have been fixed by adding Requires of hicolor-icon-theme, kf5-kservice, kf5-filesystem, ncurses-base. problem c may not be fixed, releated ref: https://bugzilla.redhat.com/show_bug.cgi?id=790869 Thanks for your good advice.
SPEC URL: https://download.copr.fedorainfracloud.org/results/topazus/test-review/fedor... SRPM URL: https://download.copr.fedorainfracloud.org/results/topazus/test-review/fedor...
review: https://download.copr.fedorainfracloud.org/results/topazus/test-review/fedor...
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #9 from Jakub Kadlčík jkadlcik@redhat.com --- Created attachment 1948442 --> https://bugzilla.redhat.com/attachment.cgi?id=1948442&action=edit The .spec file difference from Copr build 5597218 to 5598507
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #10 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5598507 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #11 from Felix Wang topazus@outlook.com --- (In reply to Felix Wang from comment #8)
problem a, b an d have been fixed by adding Requires of hicolor-icon-theme, kf5-kservice, kf5-filesystem, ncurses-base. problem c may not be fixed, releated ref: https://bugzilla.redhat.com/show_bug.cgi?id=790869 Thanks for your good advice.
SPEC URL: https://download.copr.fedorainfracloud.org/results/topazus/test-review/ fedora-rawhide-x86_64/05598415-contour-terminal/contour-terminal.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/topazus/test-review/ fedora-rawhide-x86_64/05598415-contour-terminal/contour-terminal-0.3.11.258- 1.fc39.src.rpm
review: https://download.copr.fedorainfracloud.org/results/topazus/test-review/ fedora-rawhide-x86_64/05598415-contour-terminal/fedora-review/review.txt
Forgive my last comment and Here are detailed corrections with my last comment: a): use rpm -qf to query what package owns the directories. ref: https://stackoverflow.com/questions/30067649/rpmbuild-common-ownership-of-di...
solution: adding Requires of hicolor-icon-theme, kf5-kservice, kf5-filesystem, ncurses-base
b): I have done this. Thanks for your advice.
c): It may not be fixed because there is a package named libunicode in my BuildRequires, maybe the releated ref: https://bugzilla.redhat.com/show_bug.cgi?id=790869
d): I enabled in my Copr. Thanks.
e): I see the upstream tests and modified my .spec file.
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #12 from Benson Muite benson_muite@emailplus.org --- Modified the specfile to remove:
Requires: fontconfig Requires: freetype Requires: harfbuzz Requires: yaml-cpp Requires: libunicode Requires: qt6-qtbase Requires: qt6-qtmultimedia Requires: qt5-qtbase Requires: qt5-qtmultimedia
this fixes problem c. If a library is used in BuildRequires, and it is linked to in the package, it will correctly be included as a dependency, so there is no need to add "Requires: " for it. Can the spec file be updated similarly?
libunicode does not build on s390x or ppc64: https://copr.fedorainfracloud.org/coprs/fed500/contour-terminal/build/560189... get "error: 'intrinsics' has not been declared" Perhaps check with upstream if it can be built without using instrinsics or with intrinsics that are supported on these architectures?
It does build on x86_64 and aarch64: https://copr.fedorainfracloud.org/coprs/fed500/contour-terminal/build/560195... which are most used, but if able to support s390x and ppc64, that would be great as well.
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #13 from Felix Wang topazus@outlook.com --- Thanks a lot. By looking at your comment and reading the Fedora guidelines, I learned a lot. These required packages are automatically added with rpmbuild. ref: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_require...
The upstream maintainer currently can make sure that it is worked on x86_64 and aarch64.
SPEC URL: https://download.copr.fedorainfracloud.org/results/topazus/test-review/fedor... SRPM URL: https://download.copr.fedorainfracloud.org/results/topazus/test-review/fedor... review: https://download.copr.fedorainfracloud.org/results/topazus/test-review/fedor...
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #14 from Jakub Kadlčík jkadlcik@redhat.com --- Created attachment 1948670 --> https://bugzilla.redhat.com/attachment.cgi?id=1948670&action=edit The .spec file difference from Copr build 5598507 to 5602929
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #15 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5602929 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #16 from Benson Muite benson_muite@emailplus.org --- Thanks. Can you exclude other architectures: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_bui... Probably also want to do this in the spec file for libunicode as well
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #17 from Felix Wang topazus@outlook.com --- Oh, appreciated for pointing out for the missing line to deal with architecture failures for contour terminal, though I have already done it in libunicode package. SPEC URL: https://raw.githubusercontent.com/topazus/fedora-src/main/contour-terminal.s...
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
Benson Muite benson_muite@emailplus.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ Status|ASSIGNED |POST
--- Comment #19 from Benson Muite benson_muite@emailplus.org --- Rather than: ExclusiveArch: x86_64 aarch64 It is better to use ExcludeArch: s390x i386 ppc4ie
Please update when importing. Thanks for adding the package.
Approved.
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #20 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/contour-terminal
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
--- Comment #21 from Felix Wang topazus@outlook.com --- Thank you for the review work.
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |MODIFIED
--- Comment #22 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-37b379541b has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-37b379541b
https://bugzilla.redhat.com/show_bug.cgi?id=2174384
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA Last Closed| |2023-03-08 00:52:43
--- Comment #23 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-37b379541b has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.
package-review@lists.fedoraproject.org