| Summary: | Review request: xsensors - An X11 interface to lm_sensors | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Jeremy Newton <alexjnewt> |
| Component: | Package Review | Assignee: | Christopher Meng <i> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | cedric.olivier, i, package-review |
| Target Milestone: | --- | Flags: | i:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | xsensors-0.72-1.fc19 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2014-01-07 03:30:58 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
|
Description
Jeremy Newton
2012-02-25 00:16:14 UTC
I retract what I said about my sponsor and it seems I do need a sponsor for Fedora after all. There is a problem when trying to build it with mock on fedora-16-i386 :
checking for suffix of object files... configure: error: in `/builddir/build/BUILD/xsensors-0.70':
configure: error: cannot compute suffix of object files: cannot compile
See `config.log' for more details.
erreur: Mauvais status de sortie pour /var/tmp/rpm-tmp.ARt9UR (%build)
Mauvais status de sortie pour /var/tmp/rpm-tmp.ARt9UR (%build)
Erreur de construction de RPM:
Child return code was: 1
EXCEPTION: Command failed. See logs for output.
when looking in config.log :
configure:2688: gcc -V >&5
gcc: error: unrecognized command line option '-V'
gcc: fatal error: no input files
compilation terminated.
(In reply to comment #2) > There is a problem when trying to build it with mock on fedora-16-i386 : > > checking for suffix of object files... configure: error: in > `/builddir/build/BUILD/xsensors-0.70': > configure: error: cannot compute suffix of object files: cannot compile > See `config.log' for more details. > erreur: Mauvais status de sortie pour /var/tmp/rpm-tmp.ARt9UR (%build) > Mauvais status de sortie pour /var/tmp/rpm-tmp.ARt9UR (%build) > Erreur de construction de RPM: > Child return code was: 1 > EXCEPTION: Command failed. See logs for output. > > when looking in config.log : > > configure:2688: gcc -V >&5 > gcc: error: unrecognized command line option '-V' > gcc: fatal error: no input files > compilation terminated. Interesting, building failed for me but in the %install step due to a missing dependency, rather than your error, which is in the %build step. I added the missing dependency, removed the build flags, and it built fine for me in mock (fc16-i386). Can you give it another shot? SPEC: http://dl.dropbox.com/u/42480493/xsensors.spec SRPM: http://dl.dropbox.com/u/42480493/xsensors-0.70-2.fc16.src.rpm You can find here an informal review because I am not a sponsor :
[+] mock build OK
[+] source files match upstream
4f8fb83cfd03c0cc34967a73c6021531
[+] package name according to the Package Naming Guidelines
[+] specfile is properly named, is cleanly written and uses macros
consistently.
[+] dist tag is present.
[+] license field matches the actual license.
[+] license is open source-compatible.
GPLv2+
[+] license text included in package.
[+] latest version is being packaged.
[+] BuildRequires are proper.
[+] compiler flags are appropriate.
[NA] handle locales properly
[+] package installs properly
[+] debuginfo package looks complete.
[1] rpmlint is silent.
[+] no shared libraries are added to the regular linker search paths.
[+] owns the directories it creates.
[+] doesn't own any directories it shouldn't.
[+] no duplicates in %files.
[+] file permissions are appropriate.
[+] scriptlets are present and sane.
[+] documentation is small, so no -docs subpackage is necessary.
[+] GUI applications must include a %{name}.desktop
[+] contain man pages for binaries/scripts
(1) E: incorrect-fsf-address : non blocking, could you create a new bug for upstream.
(In reply to comment #4) > You can find here an informal review because I am not a sponsor : Thanks much appreciated :) > [+] mock build OK > [+] source files match upstream > 4f8fb83cfd03c0cc34967a73c6021531 > [+] package name according to the Package Naming Guidelines > [+] specfile is properly named, is cleanly written and uses macros > consistently. > [+] dist tag is present. > [+] license field matches the actual license. > [+] license is open source-compatible. > GPLv2+ > [+] license text included in package. > [+] latest version is being packaged. > [+] BuildRequires are proper. > [+] compiler flags are appropriate. > [NA] handle locales properly > [+] package installs properly > [+] debuginfo package looks complete. > [1] rpmlint is silent. > [+] no shared libraries are added to the regular linker search paths. > [+] owns the directories it creates. > [+] doesn't own any directories it shouldn't. > [+] no duplicates in %files. > [+] file permissions are appropriate. > [+] scriptlets are present and sane. > [+] documentation is small, so no -docs subpackage is necessary. > [+] GUI applications must include a %{name}.desktop > [+] contain man pages for binaries/scripts > > (1) E: incorrect-fsf-address : non blocking, could you create a new bug for > upstream. As I said in comment #0 (Description), upstream is not active and is currently maintained by debian in the meantime. I can submit a patch or a bug report with debian; is this what you had in mind? If this is only the debian project that keeps this project for a long time, it might well be asking them to formally regain control of xsensors. Packaging a death project is for me a problem. Usually every patch must be submitted to upstream. For me if debian isn't upstream for this project, they can't change copying file. Licence choice is made by upstream and nobody can alter its content, even to update a link. (In reply to comment #6) > If this is only the debian project that keeps this project for a long time, it > might well be asking them to formally regain control of xsensors. > > Packaging a death project is for me a problem. Usually every patch must be > submitted to upstream. > > For me if debian isn't upstream for this project, they can't change copying > file. Licence choice is made by upstream and nobody can alter its content, even > to update a link. Apologies for the delay. Due to inactivity upstream, I've decided to maintain it myself as some sort of fork for personal interest. I don't see a reason why this would be a problem as the project is GPL and I didn't do anything that violates such (or at least not on purpose), but please let me know if there is so I can fix it. I updated the addresses but the licenses are intact. Anyway, I've also updated the files to reflect this new upstream SPEC: http://dl.dropbox.com/u/42480493/xsensors.spec SRPM: http://dl.dropbox.com/u/42480493/xsensors-0.71-1.fc16.src.rpm Only the spelling RPMLINT warnings are left. Thanks! Also I have been recently sponsored :)
Could you add %{?_smp_mflags} in build section ? It is used for controls how many simultaneous jobs make starts.
Oh yes, of course; it seemed to have slipped my mind. SPEC: http://dl.dropbox.com/u/42480493/xsensors.spec SRPM: http://dl.dropbox.com/u/42480493/xsensors-0.71-2.fc16.src.rpm This message is a reminder that Fedora 16 is nearing its end of life. Approximately 4 (four) weeks from now Fedora will stop maintaining and issuing updates for Fedora 16. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as WONTFIX if it remains open with a Fedora 'version' of '16'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version prior to Fedora 16's end of life. Bug Reporter: Thank you for reporting this issue and we are sorry that we may not be able to fix it before Fedora 16 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora, you are encouraged to click on "Clone This Bug" and open it against that version of Fedora. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping This message is a reminder that Fedora 18 is nearing its end of life. Approximately 4 (four) weeks from now Fedora will stop maintaining and issuing updates for Fedora 18. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as WONTFIX if it remains open with a Fedora 'version' of '18'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version prior to Fedora 18's end of life. Thank you for reporting this issue and we are sorry that we may not be able to fix it before Fedora 18 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora, you are encouraged change the 'version' to a later Fedora version prior to Fedora 18's end of life. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. You'd better finish this one ASAP. fedora-review flag is not set either I need someone to review this. I set the review flag to ?. Please fix this if it's the incorrect to put for this. Thanks (In reply to Jeremy Newton from comment #15) > I need someone to review this. I set the review flag to ?. Please fix this > if it's the incorrect to put for this. > Thanks This flag need to be set by the reviewer but not the reporter. Please also tell me your FAS account name. Thanks, I was a little confused by Michaels comment. My FAS username is mystro256 No need to be confused. Just ask. ;-) This bugzilla ticket is _assigned to_ "Cédric OLIVIER" (the reviewer), but the reviewer also needs to set the fedora-review flag to '?': https://fedoraproject.org/wiki/Package_Review_Process#Reviewer (In reply to Jeremy Newton from comment #17) > Thanks, I was a little confused by Michaels comment. My FAS username is > mystro256 Oh I remember now, I just left a comment at love package FTBFS bug, hope we can work together ;) I will take this review. I don't know how you got 0.71 version? I can't find this version anywhere. Hi, sorry it's been a while since I looked at this. I should do a test build because I'm not sure if it builds in rawhide anymore. 0.71 was my own fork, which collaborates various patches I've found, as the project is no longer maintained. I have it hosted on git hub: https://github.com/Mystro256/xsensors Let me make sure it still builds It seems everything is working fine. I noticed there was a couple date mistakes, so I fixed them. Here's the current files: SPEC: http://dl.dropbox.com/u/42480493/xsensors.spec SRPM: http://dl.dropbox.com/u/42480493/xsensors-0.71-2.fc20.src.rpm Package Review
==============
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
===== MUST items =====
C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
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 (v2 or later)", "Unknown or generated". 2 files have unknown
license. Detailed output of licensecheck:
GPL (v2 or later)
-----------------
xsensors/src/chips.c
xsensors/src/chips.h
xsensors/src/gui.c
xsensors/src/gui.h
xsensors/src/main.c
xsensors/src/main.h
Unknown or generated
--------------------
xsensors/acconfig.h
xsensors/autogen.sh
[!]: Package requires other packages for directories it uses.
Note: No known owner of /usr/share/pixmaps/xsensors
[!]: Package must own all directories that it creates.
Note: Directories without known owners: /usr/share/pixmaps/xsensors
[!]: %build honors applicable compiler flags or justifies otherwise.
--> Please remove --prefix=/usr as %configure has included it.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[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.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: 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]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
(~1MB) or number of files.
Note: Documentation size is 30720 bytes in 4 files.
[!]: Package complies to the Packaging Guidelines
--> %post* sections are wrong. Please remove them.
[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 %doc.
[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]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or desktop-
file-validate if there is such a file.
[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 do not use a name that already exist
[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:
[x]: 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]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make %{?_smp_mflags} macro.
[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).
[x]: Large data in /usr/share should live in a noarch subpackage if package is
arched.
Rpmlint
-------
Checking: xsensors-0.71-2.fc21.i686.rpm
xsensors-0.71-2.fc21.src.rpm
xsensors.i686: W: spelling-error Summary(en_US) lm -> ml, km, l
xsensors.i686: W: spelling-error %description -l en_US lm -> ml, km, l
xsensors.src: W: spelling-error Summary(en_US) lm -> ml, km, l
xsensors.src: W: spelling-error %description -l en_US lm -> ml, km, l
2 packages and 0 specfiles checked; 0 errors, 4 warnings.
Rpmlint (installed packages)
----------------------------
# rpmlint xsensors
xsensors.i686: W: spelling-error Summary(en_US) lm -> ml, km, l
xsensors.i686: W: spelling-error %description -l en_US lm -> ml, km, l
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
# echo 'rpmlint-done:'
Requires
--------
xsensors (rpmlib, GLIBC filtered):
/bin/sh
libatk-1.0.so.0
libc.so.6
libcairo.so.2
libfontconfig.so.1
libfreetype.so.6
libgdk-x11-2.0.so.0
libgdk_pixbuf-2.0.so.0
libgio-2.0.so.0
libglib-2.0.so.0
libgobject-2.0.so.0
libgtk-x11-2.0.so.0
libpango-1.0.so.0
libpangocairo-1.0.so.0
libpangoft2-1.0.so.0
libsensors.so.4
rtld(GNU_HASH)
Provides
--------
xsensors:
application()
application(xsensors.desktop)
xsensors
xsensors(x86-32)
Source checksums
----------------
https://github.com/downloads/Mystro256/xsensors/xsensors-0.71.tar.gz :
CHECKSUM(SHA256) this package : c1924d6e3930eaeabfa32503508b7a4f153807646fa94998bf6c5782bcba7568
CHECKSUM(SHA256) upstream package : c1924d6e3930eaeabfa32503508b7a4f153807646fa94998bf6c5782bcba7568
Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13
Command line :/bin/fedora-review -rvn xsensors-0.71-2.fc20.src.rpm
Buildroot used: fedora-rawhide-i386
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: BATCH, EPEL5, EXARCH, DISTTAG
-----------------
I think you should change URL tag to your GitHub page since it's a fork.
Not approved.
Doesn't filesystem own pixmaps? Also what would you advise to fix this issue? I take this is the only real issue for approval? (In reply to Jeremy Newton from comment #24) > Doesn't filesystem own pixmaps? Also what would you advise to fix this issue? > > I take this is the only real issue for approval? It owns pixmaps dir only itself but not subdir underneath it. I'm sorry, I'm a little confused.
How would I add ownership of the xsensors directory?
Are you implying I need to add
%{_datadir}/pixmaps/%{name}
to the file list?
Let's see:
1. %{_datadir}/pixmaps/%{name}/default.xpm
So, you put the icon underneath dir %{_datadir}/pixmaps/%{name}/.
Now you just wrote %{_datadir}/pixmaps/%{name}/default.xpm, so RPM will own this icon only, and the parent directory %{_datadir}/pixmaps/%{name} is un-owned.
Solution:
%{_datadir}/pixmaps/%{name}/default.xpm
-->
%{_datadir}/pixmaps/%{name}/
2. BUT!
Since you are the upstream of this fork, I hope you don't put this icon here(why we need an extra dir???), just directly put it at %{_datadir}/pixmaps and name it %{name}.xpm so you can just write this in RPM:
%{_datadir}/pixmaps/%{name}.xpm
And don't forget to modify the desktopfile entry:
Icon=/usr/share/pixmaps/xsensors/default.xpm
-->
Icon=xsensors.xpm
Oh, very good point :) Here you go: SPEC: http://dl.dropbox.com/u/42480493/xsensors.spec SRPM: http://dl.dropbox.com/u/42480493/xsensors-0.72-1.fc20.src.rpm Let me know if I missed something Thanks! PACKAGE APPROVED. New Package SCM Request ======================= Package Name: xsensors Short Description: An X11 interface to lm_sensors Owners: mystro256 Branches: f19 f20 InitialCC: Git done (by process-git-requests). xsensors-0.72-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/xsensors-0.72-1.fc19 xsensors-0.72-1.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/xsensors-0.72-1.fc20 xsensors-0.72-1.fc20 has been pushed to the Fedora 20 stable repository. xsensors-0.72-1.fc19 has been pushed to the Fedora 19 stable repository. Package Change Request ====================== Package Name: xsensors New Branches: el6 epel7 Owners: mystro256 I wish to extend this to epel 6 and onwards, thanks! Git done (by process-git-requests). |