Red Hat Bugzilla – Bug 1252130
Review Request: gnome-shell-extension-drop-down-terminal - Drop Down Terminal
Last modified: 2018-03-06 18:35:04 EST
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
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.
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
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.
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.
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
I don't suppose OP is still interested in working on this package anymore?
Do you want to make package review of gnome-shell-extension-drop-down-terminal ?
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.
...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.
Ok, Ill do it this week.
Hey, Mosaab, I'm just reviewing some of my open Bugzilla threads. Are you still interested in working on this package? :)
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.
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.