Bug 1998270 - Review Request: gtksourceview5 - gtk widget for source code
Summary: Review Request: gtksourceview5 - gtk widget for source code
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Kalev Lember
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 2010997 (view as bug list)
Depends On:
Blocks: 2010999
TreeView+ depends on / blocked
 
Reported: 2021-08-26 17:49 UTC by Amanda Graven
Modified: 2022-08-11 22:59 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2022-08-11 22:59:31 UTC
Type: ---
Embargoed:
klember: fedora-review+
amanda: needinfo+


Attachments (Terms of Use)

Description Amanda Graven 2021-08-26 17:49:40 UTC
Spec URL: https://git.nao.sh/amanda/gtksourceview5-rpm/raw/branch/main/gtksourceview5.spec
SRPM URL: https://git.nao.sh/attachments/a7c583a4-993d-4d8c-8565-14c2b22f5ac1
Description: This is my first package, and I am seeking a sponsor. This package packages the latest major release of gtksourceview. Version 4 is generally widely available on most linux distributions, but there did not seem to be packages for any rpm-based distributions.
Fedora Account System Username: agraven

Comment 1 Amanda Graven 2021-08-26 17:55:36 UTC
I put it on COPR as well at https://copr.fedorainfracloud.org/coprs/agraven/gtksourceview5/

Comment 2 Iago Rubio 2021-09-01 14:28:17 UTC
Hello Amanda,

I am not a packager so I can't approve or sponsor you. Just doing the review.

Please re-check each point I make and if necesary ask for another review on devel.

rpmlint - no complains
builds in mock
builds in Copr


Manual Review:

- Package does not contain kernel modules.
- Package contains no static executables.
- Package is licensed with an open-source compatible license
- License field in the package spec file matches the actual license.
! License file installed when any subpackage combination is installed
    Debuginfo does not install license files, nor require the main package that does install license.
- Package does not own files or directories owned by other packages.
- %build honors applicable compiler flags or justifies otherwise. Only defines gtk_doc=true and install_test=true.
- Package contains no bundled libraries without FPC exception
- Changelog in prescribed format
- Sources contain only permissible code or content
- Package contains desktop file if it is a GUI application - not a GUI application.
- Development files must be in a -devel package
- The spec file handles locales properly.
- Package consistently uses macros
- Package is named according to the Package Naming Guidelines
(1) Package does not generate any conflict.
- Package obeys FHS, except libexecdir and /usr/target.
- If the package is a rename of another package ... - is not a rename.
? Requires correct, justified where necessary. vapigen required and not used on any build section. Is this required implicitly?
- spec file is legible and written in American English
- Package contains systemd file(s) if in need - no systemd file needed.
- Useful -debuginfo package or justification otherwise.
- Package is not known to require an ExcludeArch tag.
- Package complies to the Packaging Guidelines


(1) Current Gnome and Rawhide's Gnome uses GTK3 yet you are providing the GTK4 version. I think the name of the package should reflect it's the GTK4 version, and/or ship the GTK3 version to be used in current Gnome. I haven't found on guidelines how to face this issue, but it may conflict with a GTK3 build.

In the SHOULD side: no tests checked, no %check section.

There is a package maintaner for gtksourceview, that currently maintains the gtksourceview4 package.

I am sure you may contact him and ask him for the heads-up/review on this package. That may help to get a good review.

He is @pwalter . I guess you took his .spec as an starting point as they are practically identical, but some meson libs and the initial changelog. 

As I said, take this review with a pinch of salt and wait for an oficial review, unless you check out my findings on the Guidelines and agree with them.

Hope this helps.

Comment 3 Ankur Sinha (FranciscoD) 2021-09-01 15:10:16 UTC
Removing the needinfo on the full packager team, blocking NEEDSPONSOR should be sufficient. Please consider sending an introduction e-mail to the -devel list with info on this review if you haven't already. That's where all the package maintainers (and sponsors to the team) are, so always good to get in touch there.

Comment 4 Link Dupont 2021-10-05 15:48:54 UTC
(In reply to Iago Rubio from comment #2)
> (1) Current Gnome and Rawhide's Gnome uses GTK3 yet you are providing the
> GTK4 version. I think the name of the package should reflect it's the GTK4
> version, and/or ship the GTK3 version to be used in current Gnome. I haven't
> found on guidelines how to face this issue, but it may conflict with a GTK3
> build.

gtksourceview5 appears to be GTK4 only. It's meson.build file requires gtk4 >= 4.2

https://gitlab.gnome.org/GNOME/gtksourceview/-/blob/master/meson.build#L71

Comment 6 Link Dupont 2021-10-05 17:10:25 UTC
- Move gtksourceview5-widget into the -devel subpackage. It's not a particularly useful tool to a user, only developers.
- In %files, there is incorrect (and unnecessary) use of the %dir directive[1]. It is sufficient to claim the files directly. RPM will ensure the parent directories are created correctly. I notice this is the case in the gtksourceview4 spec file too, but I don't believe it is necessary to carry forward. The following %dir directives can be removed.
  - %dir %{_datadir}/gir-1.0
  - %dir %{_datadir}/gtk-doc
  - %dir %{_datadir}/gtk-doc/html
  - %dir %{_datadir}/vala
  - %dir %{_datadir}/vala/vapi
  - %dir %{_libexecdir}/installed-tests
  - %dir %{_datadir}/installed-tests
- The explicit Requires: on pcre2, libxml2 and fribidi are not needed. RPM will generate the requires based on shared libraries automatically.
- A new version of gtksourceview5 is now available. Please update to the new 5.2 version.
- This is a harmless error, so it's safe to proceed and file a bug with upstream to correct the incorrect address noted by rpmlint:
  gtksourceview5.x86_64: E: incorrect-fsf-address /usr/share/gtksourceview-5/language-specs/gdscript.lang
  gtksourceview5.x86_64: E: incorrect-fsf-address /usr/share/gtksourceview-5/language-specs/logtalk.lang
  
1: http://ftp.rpm.org/max-rpm/s1-rpm-inside-files-list-directives.html#S3-RPM-INSIDE-DIR-DIRECTIVE

==============
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- If (and only if) the source package includes the text of the license(s)
  in its own file, then that file, containing the text of the license(s)
  for the package is included in %license.
  Note: License file licenses.snippets is not marked as %license
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/LicensingGuidelines/#_license_text


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "GNU Lesser General Public License,
     Version 2.1", "*No copyright* GNU Lesser General Public License", "*No
     copyright* GNU Lesser General Public License, Version 2.1", "*No
     copyright* GNU Lesser General Public License v2.1 or later", "GNU
     Lesser General Public License v2.1 or later", "GNU Library General
     Public License v2 or later", "GNU Lesser General Public License v2.1
     or later Apache License (v2.0) or MIT license", "GNU Lesser General
     Public License v2.1 or later GNU General Public License v3.0 or
     later", "GNU General Public License v3.0 or later", "*No copyright*
     GNU General Public License". 259 files have unknown license. Detailed
     output of licensecheck in
     /home/link/1998270-gtksourceview5/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
[ ]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/gir-1.0(tracker-
     devel, libinsane-gobject-devel, gtk2-devel, clutter-devel, libsecret-
     devel, malcontent-devel, libgtop2-devel, gstreamer1-rtsp-server-devel,
     geoclue2-devel, libxklavier-devel, libgee-devel, libmodulemd1-devel,
     libmanette-devel, libadwaita-devel, libgepub-devel, json-glib-devel,
     libgxps-devel, gspell-devel, rygel-devel, gplugin-gtk3-devel, libjcat-
     devel, gupnp-devel, gdk-pixbuf2-devel, gfbgraph-devel, gitg-devel,
     gnome-menus-devel, libhandy-devel, vips-devel, colord-gtk-devel,
     gtk3-devel, vte291-devel, webkit2gtk3-devel, libchamplain-devel,
     gtk4-devel, gupnp-dlna-devel, bamf-devel, totem-pl-parser-devel,
     gnome-online-accounts-devel, libmodulemd-devel, libcryptui-devel,
     appstream-devel, atk-devel, libzapojit-devel, glade-devel, libdazzle-
     devel, GConf2-devel, libmypaint-devel, libgee06-devel, libsoup-devel,
     libmash-devel, gupnp-av-devel, librsvg2-devel, gsequencer-devel, at-
     spi2-core-devel, gucharmap-devel, malcontent-ui-devel,
     libgexiv2-devel, libxmlb-devel, gobject-introspection-devel,
     gegl04-devel, goocanvas2-devel, zbar-gi, libgdl-devel, libgnome-
     keyring-devel, fcitx-devel, jsonrpc-glib-devel, gmime30-devel,
     libdmapsharing4-devel, harfbuzz-devel, gcr-devel, graphene-devel,
     gssdp-devel, ibus-devel, gnome-calculator-devel, gsound-devel, babl-
     devel, gdm-devel, libgdata-devel, template-glib-devel, fcitx5-gtk-
     devel, gnome-bluetooth-libs-devel, gplugin-devel, gupnp-igd-devel,
     grilo-devel, webkit2gtk3-jsc-devel, gst-devtools-devel, libpeas-devel,
     vte-devel, gnome-autoar-devel, amtk-devel, gtksourceview4-devel, gocl-
     devel), /usr/share/gtk-doc(libgweather-devel, libinsane-gobject-devel,
     clutter-gst3-devel, parole-devel, gupnp-docs, libsecret-devel,
     libwnck3-devel, libgtop2-devel, geoclue2-devel, libxklavier-devel,
     dbus-doc, cheese-libs-devel, gom-devel, libgnt-doc,
     libmodulemd1-devel, dconf-devel, gtkimageview-devel, libu2f-host-
     devel, libgxps-devel, gnome-desktop3-devel, nautilus-devel,
     ORBit2-devel, rygel-devel, libunity-gtk-parser-devel, libjcat-devel,
     libdbusmenu-doc, exo-devel, libgpod-doc, gfbgraph-devel, anjuta-devel,
     gdk-pixbuf2-xlib-devel, ibus-devel-docs, libhandy-devel, vips-devel,
     libfm-devel-docs, colord-gtk-devel, clutter-doc, libidn2-devel, tomoe-
     devel, vte291-devel, libchamplain-devel, libcloudproviders-devel,
     libxml2-devel, flatpak-devel, bamf-devel, gnome-online-accounts-devel,
     libmodulemd-devel, libsoup-doc, libcryptui-devel, rasqal-devel, power-
     profiles-daemon-docs, gupnp-av-docs, gtranslator, gtk2-devel-docs,
     libwnck-devel, tumbler-devel, polkit-docs, libzapojit-devel, glade-
     devel, libdazzle-devel, libcanberra-devel, libxfce4ui-devel, iio-
     sensor-proxy-docs, gedit-devel, libgda-devel, upower-devel-docs,
     poppler-glib-doc, gimp-devel, cutter-devel, playerctl-docs,
     librsvg2-devel, unique3-docs, low-memory-monitor-doc, libgexiv2-devel,
     glib2-doc, libxmlb-devel, gobject-introspection-devel, yelp-devel,
     libxfce4util-devel, gtk-doc, gtkdatabox-devel, gegl04-devel-docs,
     goocanvas2-devel, anaconda-widgets-devel, libgdl-devel, eog-devel,
     libgnome-keyring-devel, p11-kit-devel, jsonrpc-glib-devel, gnome-
     panel-devel, libgit2-glib-devel, gsequencer-devel-doc, gmime30-devel,
     gtk3-devel-docs, unique-devel, harfbuzz-devel, switcheroo-control-
     docs, gcr-devel, graphene-devel, libgnome-devel, clipsmm-doc, libldm-
     devel, garcon-devel, gsound-devel, raptor2-devel, udisks-devel,
     libgdata-devel, template-glib-devel, gnome-bluetooth-libs-devel,
     webkit2gtk3-doc, gtkspell3-devel, gupnp-igd-devel, libnma-devel,
     nautilus-python-devel, xed-doc, gspell-doc, geocode-glib-devel, gupnp-
     dlna-docs, buildstream-docs, fprintd-devel, libglib-testing-devel,
     gst-devtools-devel, libpeas-devel, redland-devel, gnome-autoar-devel,
     xfconf-devel, amtk-devel, lasem-devel, gtksourceview4-devel, gocl-
     devel), /usr/share/gtk-doc/html(libgweather-devel, libinsane-gobject-
     devel, clutter-gst3-devel, parole-devel, gupnp-docs, libsecret-devel,
     libwnck3-devel, libgtop2-devel, geoclue2-devel, libxklavier-devel,
     dbus-doc, cheese-libs-devel, gom-devel, libgnt-doc,
     libmodulemd1-devel, dconf-devel, gtkimageview-devel, libu2f-host-
     devel, libgxps-devel, gnome-desktop3-devel, nautilus-devel,
     ORBit2-devel, rygel-devel, libunity-gtk-parser-devel, libjcat-devel,
     libdbusmenu-doc, exo-devel, libgpod-doc, gfbgraph-devel, anjuta-devel,
     gdk-pixbuf2-xlib-devel, ibus-devel-docs, libhandy-devel, vips-devel,
     libfm-devel-docs, colord-gtk-devel, clutter-doc, libidn2-devel, tomoe-
     devel, vte291-devel, libchamplain-devel, libcloudproviders-devel,
     libxml2-devel, flatpak-devel, bamf-devel, gnome-online-accounts-devel,
     libmodulemd-devel, libsoup-doc, libcryptui-devel, rasqal-devel, power-
     profiles-daemon-docs, gupnp-av-docs, gtranslator, gtk2-devel-docs,
     libwnck-devel, tumbler-devel, polkit-docs, libzapojit-devel, glade-
     devel, libdazzle-devel, libcanberra-devel, libxfce4ui-devel, iio-
     sensor-proxy-docs, gedit-devel, libgda-devel, poppler-glib-doc, gimp-
     devel, cutter-devel, playerctl-docs, librsvg2-devel, unique3-docs,
     low-memory-monitor-doc, libgexiv2-devel, glib2-doc, libxmlb-devel,
     gobject-introspection-devel, yelp-devel, libxfce4util-devel,
     gtkdatabox-devel, gegl04-devel-docs, goocanvas2-devel, anaconda-
     widgets-devel, libgdl-devel, eog-devel, libgnome-keyring-devel,
     p11-kit-devel, jsonrpc-glib-devel, gnome-panel-devel, libgit2-glib-
     devel, gsequencer-devel-doc, gmime30-devel, gtk3-devel-docs, unique-
     devel, harfbuzz-devel, switcheroo-control-docs, gcr-devel, graphene-
     devel, libgnome-devel, clipsmm-doc, libldm-devel, garcon-devel,
     gsound-devel, raptor2-devel, udisks-devel, libgdata-devel, template-
     glib-devel, gnome-bluetooth-libs-devel, webkit2gtk3-doc,
     gtkspell3-devel, gupnp-igd-devel, libnma-devel, nautilus-python-devel,
     xed-doc, gspell-doc, geocode-glib-devel, gupnp-dlna-docs, buildstream-
     docs, fprintd-devel, libglib-testing-devel, gst-devtools-devel,
     libpeas-devel, redland-devel, gnome-autoar-devel, xfconf-devel, amtk-
     devel, lasem-devel, gtksourceview4-devel, gocl-devel),
     /usr/share/vala(gcr-devel, libgda-devel, libsoup-devel, libgweather-
     devel, gssdp-devel, tracker-devel, vte291-devel, ibus-devel, gnome-
     calculator-devel, gsound-devel, babl-devel, gupnp-av-devel, gupnp-
     dlna-devel, libdmapsharing-devel, librsvg2-devel, libtranslit-devel,
     libsecret-devel, libgdata-devel, bamf-devel, template-glib-devel,
     gnome-online-accounts-devel, gplugin-vala, geoclue2-devel, libosinfo-
     devel, gtksourceview3-devel, libgexiv2-devel, libgee-devel,
     libmanette-devel, pulseaudio-libs-devel, dconf-devel, libgnome-games-
     support-devel, appstream-devel, gegl04-devel, vala, grilo-devel,
     gspell-devel, rygel-devel, libgda-ui-devel, libdazzle-devel, libjcat-
     devel, libcanberra-devel, folks-devel, libgnome-keyring-devel, gupnp-
     devel, jsonrpc-glib-devel, zeitgeist-devel, libgit2-glib-devel,
     libgee06-devel, gitg-devel, gmime30-devel, gnome-autoar-devel,
     libdmapsharing4-devel, gedit-devel, libhandy-devel, caribou-devel,
     gmime-devel, gtksourceview4-devel), /usr/share/vala/vapi(gcr-devel,
     libgda-devel, libsoup-devel, libgweather-devel, gssdp-devel, tracker-
     devel, vte291-devel, ibus-devel, gnome-calculator-devel, gsound-devel,
     babl-devel, gupnp-av-devel, gupnp-dlna-devel, fwupd-devel,
     libdmapsharing-devel, librsvg2-devel, libtranslit-devel, libsecret-
     devel, libgdata-devel, bamf-devel, template-glib-devel, gnome-online-
     accounts-devel, gplugin-vala, geoclue2-devel, libosinfo-devel,
     gtksourceview3-devel, libgexiv2-devel, libgee-devel, libmanette-devel,
     pulseaudio-libs-devel, dconf-devel, libgnome-games-support-devel,
     appstream-devel, gegl04-devel, vala, grilo-devel, gspell-devel, rygel-
     devel, libgda-ui-devel, libdazzle-devel, libjcat-devel, libcanberra-
     devel, folks-devel, libgnome-keyring-devel, gupnp-devel, jsonrpc-glib-
     devel, zeitgeist-devel, libgit2-glib-devel, libgee06-devel, gitg-
     devel, libzeitgeist-devel, gmime30-devel, gnome-autoar-devel,
     libdmapsharing4-devel, gedit-devel, libhandy-devel, caribou-devel,
     gmime-devel, gtksourceview4-devel), /usr/libexec/installed-
     tests(glib2-tests, python-docker-tests, ibus-anthy-tests, json-glib-
     tests, ostree-tests, gdk-pixbuf2-tests, gjs-tests, graphene-tests,
     ibus-tests, flatpak-tests, gtksourceview4-tests, gtk3-tests, gvfs-
     tests, ibus-table-tests, ibus-hangul-tests, mutter3.38-tests, ibus-
     typing-booster-tests), /usr/share/installed-tests(json-glib-tests,
     gjs-tests, flatpak-tests, gnome-photos-tests, gtk3-tests,
     gtksourceview4-tests, gvfs-tests, ibus-hangul-tests, mutter3.38-tests,
     ibus-typing-booster-tests, gdk-pixbuf2-tests, ibus-table-tests,
     clutter-tests, pango-tests, glib2-tests, gnome-desktop3-tests,
     graphene-tests, ibus-tests, glib-networking-tests, cjs-tests, ibus-
     anthy-tests, evolution-data-server-tests, ostree-tests, eog-tests,
     dbus-tests), /usr/lib64/girepository-1.0(malcontent-libs,
     libchamplain, graphene, gcr, grilo, goocanvas2, vips, libcryptui,
     gtk4, libgee, harfbuzz, gstreamer1-rtsp-server, libdmapsharing4,
     clutter, libjcat, librsvg2, libmanette, glade-libs, jsonrpc-glib,
     rygel, libgee06, gsound, libdazzle, appstream, gfbgraph, babl,
     libsecret, libmypaint, gtk2, atk, gucharmap-libs, gupnp-dlna, libmash,
     gitg-libs, libgdl, template-glib, gsequencer, gspell, gssdp, libpeas,
     webkit2gtk3, gnome-autoar, webkit2gtk3-jsc, gmime30, vte291, gplugin-
     libs, gocl, libmodulemd1, gdk-pixbuf2, ibus-libs, at-spi2-core, gupnp,
     gnome-bluetooth-libs, libtracker-sparql, gst-devtools, libgepub,
     libgexiv2, zbar-gi, libmodulemd, gtksourceview4, gtk3, libgnome-
     keyring, libinsane-gobject, vte, json-glib, libgtop2, GConf2,
     malcontent-ui-libs, libgdata, fcitx-libs, libzapojit, libxmlb,
     gobject-introspection, gplugin-gtk3-libs, libhandy, libsoup, gnome-
     online-accounts, gnome-menus, libxklavier, playerctl-libs, gnome-
     calculator, geoclue2-libs, gupnp-av, libgxps, gupnp-igd, amtk)
[ ]: %build honors applicable compiler flags or justifies otherwise.
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package uses nothing in %doc for runtime.
[ ]: The spec file handles locales properly.
[ ]: Package consistently uses macros (instead of hard-coded directory
     names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Useful -debuginfo package or justification otherwise.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[ ]: Avoid bundling fonts in non-fonts packages.
     Note: Package contains font files
[ ]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[ ]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 7557120 bytes in /usr/share
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: gtksourceview5-5.1.0-1.fc36.x86_64.rpm
          gtksourceview5-devel-5.1.0-1.fc36.x86_64.rpm
          gtksourceview5-tests-5.1.0-1.fc36.x86_64.rpm
          gtksourceview5-debuginfo-5.1.0-1.fc36.x86_64.rpm
          gtksourceview5-debugsource-5.1.0-1.fc36.x86_64.rpm
          gtksourceview5-5.1.0-1.fc36.src.rpm
gtksourceview5.x86_64: E: explicit-lib-dependency libxml2
gtksourceview5.x86_64: W: spelling-error %description -l en_US multiline -> multilingual
gtksourceview5.x86_64: W: no-documentation
gtksourceview5.x86_64: E: incorrect-fsf-address /usr/share/gtksourceview-5/language-specs/gdscript.lang
gtksourceview5.x86_64: E: incorrect-fsf-address /usr/share/gtksourceview-5/language-specs/logtalk.lang
gtksourceview5.x86_64: W: no-manual-page-for-binary gtksourceview5-widget
gtksourceview5-tests.x86_64: W: no-documentation
gtksourceview5.src: W: spelling-error %description -l en_US multiline -> multilingual
6 packages and 0 specfiles checked; 3 errors, 5 warnings.




Rpmlint (debuginfo)
-------------------
Checking: gtksourceview5-tests-debuginfo-5.1.0-1.fc36.x86_64.rpm
          gtksourceview5-debuginfo-5.1.0-1.fc36.x86_64.rpm
2 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
Cannot parse rpmlint output:


Source checksums
----------------
https://download.gnome.org/sources/gtksourceview/5.1/gtksourceview-5.1.0.tar.xz :
  CHECKSUM(SHA256) this package     : 1f3f5bbe75d8471add16e58d21d0df9b6fe985dd40461ac89adc04a66ed43680
  CHECKSUM(SHA256) upstream package : 1f3f5bbe75d8471add16e58d21d0df9b6fe985dd40461ac89adc04a66ed43680


Requires
--------
gtksourceview5 (rpmlib, GLIBC filtered):
    fribidi
    glib2(x86-64)
    gtk4(x86-64)
    libc.so.6()(64bit)
    libcairo.so.2()(64bit)
    libfontconfig.so.1()(64bit)
    libfribidi.so.0()(64bit)
    libgdk_pixbuf-2.0.so.0()(64bit)
    libgio-2.0.so.0()(64bit)
    libglib-2.0.so.0()(64bit)
    libgobject-2.0.so.0()(64bit)
    libgtk-4.so.1()(64bit)
    libgtksourceview-5.so.0()(64bit)
    libm.so.6()(64bit)
    libpango-1.0.so.0()(64bit)
    libpangocairo-1.0.so.0()(64bit)
    libpangoft2-1.0.so.0()(64bit)
    libpcre2-8.so.0()(64bit)
    libxml2
    libxml2.so.2()(64bit)
    libxml2.so.2(LIBXML2_2.4.30)(64bit)
    libxml2.so.2(LIBXML2_2.5.0)(64bit)
    libxml2.so.2(LIBXML2_2.5.7)(64bit)
    libxml2.so.2(LIBXML2_2.6.0)(64bit)
    libxml2.so.2(LIBXML2_2.6.6)(64bit)
    pcre2
    rtld(GNU_HASH)

gtksourceview5-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    gtksourceview5(x86-64)
    libgtksourceview-5.so.0()(64bit)
    pkgconfig(fontconfig)
    pkgconfig(fribidi)
    pkgconfig(gio-2.0)
    pkgconfig(glib-2.0)
    pkgconfig(gobject-2.0)
    pkgconfig(gtk4)
    pkgconfig(libpcre2-8)
    pkgconfig(libxml-2.0)
    pkgconfig(pangoft2)

gtksourceview5-tests (rpmlib, GLIBC filtered):
    gtksourceview5(x86-64)
    libc.so.6()(64bit)
    libcairo.so.2()(64bit)
    libfontconfig.so.1()(64bit)
    libfribidi.so.0()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_3.3.1)(64bit)
    libgdk_pixbuf-2.0.so.0()(64bit)
    libgio-2.0.so.0()(64bit)
    libglib-2.0.so.0()(64bit)
    libgobject-2.0.so.0()(64bit)
    libgtk-4.so.1()(64bit)
    libm.so.6()(64bit)
    libpango-1.0.so.0()(64bit)
    libpangocairo-1.0.so.0()(64bit)
    libpangoft2-1.0.so.0()(64bit)
    libpcre2-8.so.0()(64bit)
    libxml2.so.2()(64bit)
    libxml2.so.2(LIBXML2_2.4.30)(64bit)
    libxml2.so.2(LIBXML2_2.5.0)(64bit)
    libxml2.so.2(LIBXML2_2.5.7)(64bit)
    libxml2.so.2(LIBXML2_2.6.0)(64bit)
    libxml2.so.2(LIBXML2_2.6.6)(64bit)
    rtld(GNU_HASH)

gtksourceview5-debuginfo (rpmlib, GLIBC filtered):

gtksourceview5-debugsource (rpmlib, GLIBC filtered):



Provides
--------
gtksourceview5:
    gtksourceview5
    gtksourceview5(x86-64)
    libgtksourceview-5.so.0()(64bit)

gtksourceview5-devel:
    gtksourceview5-devel
    gtksourceview5-devel(x86-64)
    pkgconfig(gtksourceview-5)

gtksourceview5-tests:
    gtksourceview5-tests
    gtksourceview5-tests(x86-64)

gtksourceview5-debuginfo:
    debuginfo(build-id)
    gtksourceview5-debuginfo
    gtksourceview5-debuginfo(x86-64)
    libgtksourceview-5.so.0.0.0-5.1.0-1.fc36.x86_64.debug()(64bit)

gtksourceview5-debugsource:
    gtksourceview5-debugsource
    gtksourceview5-debugsource(x86-64)



Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -b 1998270
Buildroot used: fedora-rawhide-x86_64
Active plugins: Shell-api, Generic, C/C++
Disabled plugins: Haskell, R, SugarActivity, Ocaml, Python, PHP, fonts, Java, Perl
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 7 Michael Catanzaro 2021-10-05 18:50:36 UTC
*** Bug 2010997 has been marked as a duplicate of this bug. ***

Comment 8 Michael Catanzaro 2021-10-26 14:01:47 UTC
This has been needinfo?amanda for a while. Neal Gompa would like to take this over if Amanda doesn't respond soon so that we can unblock gnome-text-editor. His version is here:

Spec URL: https://ngompa.fedorapeople.org/for-review/gtksourceview5.spec
SRPM URL: https://ngompa.fedorapeople.org/for-review/gtksourceview5-5.2.0-1.fc35.src.rpm

Comment 9 Amanda Graven 2021-10-26 19:34:06 UTC
I finally found the time to address the review feedback. I attempted to address feedback from everyone provided it, let me know if there's something I missed.

Spec URL: https://git.graven.dev/amanda/gtksourceview5-rpm/raw/commit/c2ce63e9de019ae1a80795131dfdb6290761a6ee/gtksourceview5.spec
SRPM URL: https://git.graven.dev/attachments/344f641f-4507-4c2f-adb7-521655d79494

Comment 10 Kalev Lember 2021-11-02 15:30:15 UTC
I'll take over the package review from Link Dupont -- he has issues with his internet connection and can't do it right now.

Comment 11 Kalev Lember 2021-11-02 16:20:42 UTC
> Version:        %{majorver}.%{minorver}.%{patchver}

This is not a review blocker, but any chance you could just spell it out as "Version: 5.2.0"? We have some automation scripts that I use for GNOME package mass updates that would break if it's split up like this. I am the one who's been handling the GNOME mass updates recently but amigadave is going to take over for the next cycle.

I would keep majorver as a global though, but maybe rename it to api_ver or something like that, as that would make it easier to bump from gtksourceview 5 to gtksourceview 6 in the future. What do you think?


> %{_libdir}/girepository-1.0/GtkSource-%{majorver}.typelib

The directory ownership here is incorrect. You need to make sure that %{_libdir}/girepository-1.0 directory itself is included in the package's list of files: otherwise when the package is removed it leaves behind the empty directory. You could do it in two ways, either explicitly listing the directory:

%dir %{_libdir}/girepository-1.0
%{_libdir}/girepository-1.0/GtkSource-%{majorver}.typelib

or recursively list both the directory and its contents:

%{_libdir}/girepository-1.0/

Which one of these to use is just a style question.

This and other gir directories might get added to a filesystem package in the future (there's an open FPC ticket about this), but as of right now we need to be explicit to make sure they are listed correctly.


> %{_datadir}/gir-1.0/GtkSource-%{majorver}.gir

Same thing here: Need to make sure %{_datadir}/gir-1.0 directory is listed in the files list.


> %{_datadir}/gtk-doc/html/*

... and here: %{_datadir}/gtk-doc and %{_datadir}/gtk-doc/html directories need to be listed in the files list.


Beyond these nitpicks, it all looks nice and clean to me -- thanks for packaging it up! I don't see any other issues and it's just a new parallel-installable version of the existing gtksourceview4 package so I'd be happy to review+ this if you can fix the nits above.

Do you need a sponsor for the packaging group, by the way? I can help with that if you need it.

Comment 12 Amanda Graven 2021-11-09 17:57:01 UTC
I do believe I need a sponsor yes.

I addressed the nitpicks, though i was somewhat unsure about what you meant with the directory listings, I hope I understood correctly.

Link to spec: https://git.graven.dev/amanda/gtksourceview5-rpm/src/tag/v0.3/gtksourceview5.spec
Link to SRPM: https://git.graven.dev/attachments/b031f9b9-da44-45a5-a474-a1ace9a2fc1e

Comment 13 Kalev Lember 2021-11-09 23:18:00 UTC
Looks good to me! Thanks for the fixes.

Licensing looks good, the spec file is nice and clean and is basically just a new, parallel-installable version of the existing gtksourceview4 package.

APPROVED

Comment 14 Kalev Lember 2021-11-09 23:25:36 UTC
As a next step, can you please log in to https://accounts.fedoraproject.org/ and sign the Fedora Project Contributor Agreement? After that I should be able to sponsor you to the packager group and then we can proceed into importing the package.

Comment 15 Amanda Graven 2021-11-10 12:18:23 UTC
Sure thing. I have signed the agreement.

Comment 16 Kalev Lember 2021-11-10 22:07:07 UTC
Excellent! I went ahead and sponsored you to the packager group. Use your new powers wisely! :)

You may need to log out and in again in https://src.fedoraproject.org/ pagure instance for it to recognize your new access rights.

As your sponsor, I'll be around if you need help with Fedora processes -- feel free to shoot me an email or find me on irc if you need help with anything.

The next step here is to request a new git repo for the new package with 'fedpkg request-repo'. See https://docs.fedoraproject.org/en-US/package-maintainers/New_Package_Process_for_New_Contributors/#add_package_to_source_code_management_scm_system_and_set_owner for more information.

Comment 17 Neal Gompa 2021-11-16 13:29:47 UTC
Can you please request and import the package? My package review depends on this component...

Comment 18 Amanda Graven 2021-11-16 14:41:19 UTC
I requested the package[1] and will import it ASAP.

[1]: https://pagure.io/releng/fedora-scm-requests/issue/37552

Comment 19 Neal Gompa 2021-11-16 16:05:10 UTC
Looks like it failed because your FAS and Bugzilla account email addresses aren't in sync.

Comment 20 Kalev Lember 2021-11-16 16:28:40 UTC
Indeed, amanda vs amanda -- can you fix this please so that the addresses match in the two systems?

Comment 21 Amanda Graven 2021-11-17 07:28:05 UTC
I've corrected the FAS email address. Should I file a new request, or re-open the existing one?

Comment 22 Kalev Lember 2021-11-17 07:55:10 UTC
I think either way should work, but I'd personally just file a new one.

Comment 23 Amanda Graven 2021-11-17 08:00:58 UTC
I filed a new request: https://pagure.io/releng/fedora-scm-requests/issue/37602

Comment 24 Gwyn Ciesla 2021-11-18 16:38:49 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/gtksourceview5

Comment 25 Link Dupont 2021-11-26 03:19:35 UTC
Do you need any help importing the package into dist-git?

Comment 26 Link Dupont 2021-11-26 03:26:17 UTC
When importing the SRPM, I got a warning that the date in the changelog is wrong:

warning: bogus date in %changelog: Tue Oct 25 2021 Amanda Graven <amanda> - 5.2.0-1

Comment 27 Amanda Graven 2021-12-02 12:39:02 UTC
I've imported the package now. Apologies for the delay, I had issues with fedpkg.

Comment 28 Amanda Graven 2021-12-02 13:41:29 UTC
Looking at koji, it seems that the build fails for 32-bit arm and x86, due to an issue with mismatched integer sizes. The build logs are here: https://koji.fedoraproject.org/koji/taskinfo?taskID=79510878. The exact error is:
../gtksourceview/implregex.c: In function ‘impl_regex_new’:
../gtksourceview/implregex.c:239:55: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘gsize’ {aka ‘unsigned int’} [-Werror=format=]
  239 |                 message = g_strdup_printf ("compile=%lx match=%lx pattern=%s",
      |                                                     ~~^
      |                                                       |
      |                                                       long unsigned int
      |                                                     %x
  240 |                                            regex->compile_flags,
      |                                            ~~~~~~~~~~~~~~~~~~~~
      |                                                 |
      |                                                 gsize {aka unsigned int}

At first glance this seems like an upstream bug. I will investigate the issue further and determine how difficult it is to patch.

Comment 29 Michael Catanzaro 2021-12-02 15:23:20 UTC
It's indeed an upstream bug. The correct format would be:

"compile=%zx match=%zx pattern=%s"

l is for printing longs, z is for printing size_t/gsize. Would be a good first merge request for upstream.

Comment 30 Amanda Graven 2021-12-05 17:42:54 UTC
The bug has seemingly been fixed in upstream commit b0b8330, which changes gtksourceview/implregex.c:239

    message = g_strdup_printf ("compile=%"G_GSIZE_MODIFIER"x match=%"G_GSIZE_MODIFIER"x pattern=%s",

Should the fix be backported as a patch, and/or should upstream be contacted about whether making a new patch release soon is possible?

Comment 31 Michael Catanzaro 2021-12-05 18:22:44 UTC
Just backport it. Upstreams rarely do new releases every time somebody has a build fix. It will get included in the next release.


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