Bug 1252130 - Review Request: gnome-shell-extension-drop-down-terminal - Drop Down Terminal
Review Request: gnome-shell-extension-drop-down-terminal - Drop Down Terminal
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Andrew Toskin
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-08-10 14:56 EDT by Mosaab Alzoubi
Modified: 2018-03-06 18:35 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2018-03-06 18:35:04 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
spec-file diff (1.34 KB, patch)
2015-08-16 17:22 EDT, Jens Lody
no flags Details | Diff
spec-file diff (1.69 KB, patch)
2015-08-17 18:41 EDT, Jens Lody
no flags Details | Diff

  None (edit)
Description Mosaab Alzoubi 2015-08-10 14:56:36 EDT
Spec URL: http://ojuba.org/test/gnome-shell-extension-drop-down-terminal.spec
SRPM URL: http://ojuba.org/test/gnome-shell-extension-drop-down-terminal-17-1.oj35.src.rpm
Description: Drop Down Terminal extension for the Gnome Shell
Fedora Account System Username: moceap
Comment 1 Jens Lody 2015-08-16 17:22:43 EDT
Created attachment 1063569 [details]
spec-file diff

"fedora-review" fails without this patch:
BuildArch must be noarch, otherwise mock tries to generate a debuginfo-package which fails because of an empty filelist (no executables or libs).
"glib-compile-schemas" must be run in %postun and %posttrans.
The gnome-shell-extensions-common and glib2 are required (they are the owner of the used directories).
I'm not absolutely sure about glib2, because it's in the dependency-chain of gnome-shell-extensions-common, but it silents the warning about unowned directories and is needed anyway.
Comment 2 Jens Lody 2015-08-16 17:28:05 EDT
By the way: this is an informal review.

I'm unhappy with having gschemas.compiled in the extensions directory, but the upstream cnvenience.js searches for it there and not in the system-wide %{_datadir}/glib-2.0/schemas/.
I'm not sure if that's allowed in FHS and Packaging Guidelines (I did not found anything about it), so I leave two points open:

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

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



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

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "GPL (v3 or later)". Detailed output of licensecheck in
     /home/jens/gnome-shell-extension-drop-down-terminal/licensecheck.txt
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: 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
[x]: Package uses nothing in %doc for runtime.
[x]: glib-compile-schemas is run in %postun and %posttrans if package has
     *.gschema.xml files.
     Note: gschema file(s) in gnome-shell-extension-drop-down-terminal
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: 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.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 3 files.
[ ]: 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]: 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.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[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 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]: Packages must not store files under /srv, /opt or /usr/local

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

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[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]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).


Rpmlint
-------
Checking: gnome-shell-extension-drop-down-terminal-17-1.fc24.noarch.rpm
          gnome-shell-extension-drop-down-terminal-17-1.fc24.src.rpm
gnome-shell-extension-drop-down-terminal.noarch: E: explicit-lib-dependency glib2
gnome-shell-extension-drop-down-terminal.noarch: E: zero-length /usr/share/gnome-shell/extensions/drop-down-terminal@gs-extensions.zzrough.org/stylesheet.css
2 packages and 0 specfiles checked; 2 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: Datei oder Verzeichnis nicht gefunden
gnome-shell-extension-drop-down-terminal.noarch: E: explicit-lib-dependency glib2
gnome-shell-extension-drop-down-terminal.noarch: E: zero-length /usr/share/gnome-shell/extensions/drop-down-terminal@gs-extensions.zzrough.org/stylesheet.css
1 packages and 0 specfiles checked; 2 errors, 0 warnings.



Requires
--------
gnome-shell-extension-drop-down-terminal (rpmlib, GLIBC filtered):
    /bin/sh
    glib2
    gnome-shell-extension-common



Provides
--------
gnome-shell-extension-drop-down-terminal:
    gnome-shell-extension-drop-down-terminal



Source checksums
----------------
https://github.com/zzrough/gs-extensions-drop-down-terminal/archive/18ffa318a71f26f5bffe0f7c6c2936bf03a039e0/gs-extensions-drop-down-terminal-18ffa318a71f26f5bffe0f7c6c2936bf03a039e0.tar.gz :
  CHECKSUM(SHA256) this package     : 52873c1abb5dcea7c8b5888e6dbea2dfc388ec058db84449f4f80aa5a2a7be4d
  CHECKSUM(SHA256) upstream package : 52873c1abb5dcea7c8b5888e6dbea2dfc388ec058db84449f4f80aa5a2a7be4d


Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review --rpm-spec -m fedora-rawhide-x86_64 -n /home/jens/rpmbuild/SRPMS/gnome-shell-extension-drop-down-terminal-17-1.fc22.src.rpm
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api
Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Comment 3 Jens Lody 2015-08-17 18:41:55 EDT
Created attachment 1064088 [details]
spec-file diff

I just see, that the glib2 dependency seems to be an error in a noarch-package, the same is with the empty stylesheet.
This will lead to a warning about an unowned directory.
I'm not sure how to fix this , the easiest way would be to add the directory %{_datadir}/glib-2.0/ to the %files, that would include the schemas directoy and the included xml-file also, but your package would be an owner of the folders.
But it might also be possible to leave it as it is.
Comment 4 Jens Lody 2015-08-18 07:52:19 EDT
Okay, finally (but hopefully correct): the package should require gnome-shell >= x.xx and own the extensions directory (add %{_datadir}/gnome-shell/extensions/ to the files sections and remove everything below it, because it will automatically included).
This makes the package an owner of the shell-extensions directory, but this is correct (as I understand the packaging guidelines), because it can be the only shell-extension installed and removing it as last would lead to orphaned directories.
Sorry for the former misleading advices.
Comment 5 Jens Lody 2015-08-21 04:04:43 EDT
It's a bit more tricky, and you probably need to distinguish between fc24 and older.
Since recent fc24 the extension-dir is owned by gnome-shell itself and this is the needed requirement, for older releases gnome-shell-extension-common was the owner of the directory and therefore the required package.
What's more since fc24 file-triggers exist, so there is no need for triggering the schema-compile manually in %postun and %posttrans any more (even if fedora-review spits out an error about this).
See:
https://bugzilla.redhat.com/show_bug.cgi?id=1246903#c8
https://lists.fedoraproject.org/pipermail/desktop/2015-August/012685.html
https://fedoraproject.org/wiki/Packaging:DistTag#Conditionals

In general the comments on (one of) my own shell-extension(s) might be of interest:
https://bugzilla.redhat.com/show_bug.cgi?id=1246903
Comment 6 Andrew Toskin 2017-03-06 19:33:12 EST
I don't suppose OP is still interested in working on this package anymore?
Comment 7 Mosaab Alzoubi 2017-03-07 20:33:29 EST
Do you want to make package review of gnome-shell-extension-drop-down-terminal ?
Comment 8 Andrew Toskin 2017-03-08 17:45:12 EST
If you're still willing to work on this, then sure, I'll review it with you and help you get it into Fedora.

It's been a while, so I assume you at least need to bump the spec to use the latest upstream release. I'll take a look once you post the update.
Comment 9 Andrew Toskin 2017-03-16 00:44:59 EDT
...Looks up stream is on version 23. So if you want to resume working on this package, we'll need an updated spec and SRPM before we get started with a review.
Comment 10 Mosaab Alzoubi 2017-03-18 05:43:54 EDT
Ok, Ill do it this week.
Comment 11 Andrew Toskin 2017-09-05 18:15:47 EDT
Hey, Mosaab, I'm just reviewing some of my open Bugzilla threads. Are you still interested in working on this package? :)
Comment 12 Andrew Toskin 2017-11-21 21:03:15 EST
I'll ask again: Mosaab, are you still interested in working on this package?

If yes, no problem, let's keep this issue open and keep working on it as we find the time. If not, no problem, let's close this issue and move on.

It's up to you, no pressure. But please do let me know what your plans are here.
Comment 13 Andrew Toskin 2018-03-06 18:35:04 EST
I'm gonna go ahead and close this. Mosaab, if you decide you want to keep working on this package after all, feel free to reopen this ticket.

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