https://bugzilla.redhat.com/show_bug.cgi?id=974714
Bug ID: 974714 Summary: Review Request: thermal_daemon - A close loop thermal monitoring and control daemon Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: unspecified Assignee: nobody@fedoraproject.org Reporter: srinivas.pandruvada@linux.intel.com QA Contact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org
Spec URL: http://spandruvada.fedorapeople.org/thermal_daemon.spec SRPM URL: http://spandruvada.fedorapeople.org/thermal_daemon-1.02-1.fc19.src.rpm koji_build_results: http://spandruvada.fedorapeople.org/koji_build_results
Description: Thermal Daemon is a Linux daemon for monitoring and controlling platform temperatures. Once the system temperature reaches a certain threshold, the Linux daemon activates various cooling methods to try to cool the system.
More details can be obtained from: https://01.org/linux-thermal-daemon/documentation/introduction-thermal-daemo... http://www.linux.com/news/featured-blogs/200-libby-clark/721494-linux-therma... http://www.phoronix.com/scan.php?page=news_item&px=MTM2OTk
It operates in two modes: Daemon acts to cool the system under the following two modes : 1. Zero Configuration Mode 2. User defined configuration mode
1. Zero Configuration Mode For most users, this should be enough to bring the CPU temperature of the system under control. This uses DTS temperature sensor and uses Intel P state driver, Power clamp driver, Running Average Power Limit control and cpufreq as cooling methods. 2. User defined configuration mode This allows ACPI style configuration in a thermal XML configuration file. This can be used to fix the buggy ACPI configuration or fine tune by adding more sensors and cooling devices. This is a first step in implementing a close loop thermal control in user mode and can be enhanced based on community feedback and suggestions.
This is my first package and seeking a sponsorship.
Fedora Account System Username: spandruvada
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #1 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- *** Bug 974715 has been marked as a duplicate of this bug. ***
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Marcelo Barbosa firemanxbr@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |firemanxbr@fedoraproject.or | |g
--- Comment #2 from Marcelo Barbosa firemanxbr@fedoraproject.org --- I'm not officially Fedora packager yet, here my informal review:
1) Remove "BuildRequires: gcc-c++", this not require this it essential part for minimum build system.
2) Will your package build from EPEL5 ? if no, remove "%defattr(-,root,root)" in %files, this option only EPEL5.
3) Adjust your %changelog in format like: from: "* Wed May 8 2013 Base version 1.0-1" To something like: "* Wed May 8 2013 Srinivas Pandruvada srinivas.pandruvada@linux.intel.com 1.0-1 - Initial package"
In next adjusts increment your release this package, example 1.0-2.
Best regards.
Marcelo Barbosa Fedora Project Ambassador firemanxbr@fedoraproject.org
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Antonio Trande anto.trande@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |anto.trande@gmail.com
--- Comment #3 from Antonio Trande anto.trande@gmail.com --- Hi Srinivas.
- Remove "BuildRequires: gcc-c++", this not require this it essential part
for minimum build system.
- Will your package build from EPEL5 ?
if no, remove "%defattr(-,root,root)" in %files, this option only EPEL5.
- Adjust your %changelog in format like:
from: "* Wed May 8 2013 Base version 1.0-1" To something like: "* Wed May 8 2013 Srinivas Pandruvada srinivas.pandruvada@linux.intel.com 1.0-1
- Initial package"
In next adjusts increment your release this package, example 1.0-2.
Also,
- URL/Source* refer to project's website and source files' location respectively. - Your package provides two .service files; please, refer to http://fedoraproject.org/wiki/Packaging:Systemd#Packaging guidelines
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Marcelo Barbosa mr.marcelo.barbosa@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |mr.marcelo.barbosa@gmail.co | |m
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Marcelo Barbosa mr.marcelo.barbosa@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Marcelo Barbosa mr.marcelo.barbosa@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #4 from Marcelo Barbosa mr.marcelo.barbosa@gmail.com --- Hi Srinivas Pandruvada,
Now i'm packager official. I go review your package, follow this comments in this ticket, questions: i'm available.
Marcelo Barbosa mr.marcelo.barbosa@gmail.com
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #5 from Marcelo Barbosa mr.marcelo.barbosa@gmail.com --- Hi Srinivas Pandruvada,
Anything new? i can review this package, but some errors where description in other comments, please adjust your package.
Thank you.
Marcelo Barbosa Fedora Project Packager Fedora Project Ambassador firemanxbr@fedoraproject.org
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #6 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Hi Marcelo,
I am working on your comments. Sorry, it is taking time as I am new to this package building and submission.
Thanks, Srinivas
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #7 from Marcelo Barbosa mr.marcelo.barbosa@gmail.com --- Srinivas,
No problem, i can help you, questions send for me.
Thank you and good working.
Marcelo Barbosa Fedora Project Packager Fedora Project Ambassador firemanxbr@fedoraproject.org
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #8 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Updated: http://spandruvada.fedorapeople.org/thermal_daemon.spec http://spandruvada.fedorapeople.org/thermal_daemon-1.02-2.fc19.src.rpm
============== diff --git a/rpms/thermal_daemon.spec b/rpms/thermal_daemon.spec index f81a1b7..a55f2d5 100644 --- a/rpms/thermal_daemon.spec +++ b/rpms/thermal_daemon.spec @@ -1,26 +1,30 @@ Name: thermal_daemon Version: 1.02 -Release: 1%{?dist} +Release: 2%{?dist} Summary: The "Linux Thermal Daemon" program from 01.org
License: GPLv2+ -URL: http://spandruvada.fedorapeople.org/ -Source0: http://spandruvada.fedorapeople.org/%%7Bname%7D-%%7Bversion%7D.tar.gz +URL: https://github.com/01org/thermal_daemon +%global commit 78b9dd1343bf7350740eb76e3bf87dc207078d9d +%global shortcommit %(c=%{commit}; echo ${c:0:7}) +Source0: https://github.com/01org/thermal_daemon/archive/%%7Bcommit%7D/%%7Bname%7D-v%...
+BuildRequires: automake +BuildRequires: autoconf BuildRequires: glib-devel BuildRequires: dbus-glib-devel BuildRequires: libxml2-devel -BuildRequires: automake -BuildRequires: autoconf -BuildRequires: gcc-c++ +BuildRequires: systemd Requires: libxml2 +Requires(post): systemd-units +Requires(preun): systemd-units +Requires(postun): systemd-units
%description Thermal Daemon monitors and controls platform temperature.
%prep -%setup -q - +%setup -qn %{name}-%{version}
%build autoreconf -f -i @@ -32,9 +36,16 @@ make %{?_smp_mflags} rm -rf $RPM_BUILD_ROOT %make_install DESTDIR=$RPM_BUILD_ROOT
+%post +%systemd_post thermald.service + +%preun +%systemd_preun thermald.service + +%postun +%systemd_postun_with_restart thermald.service
%files -%defattr(-,root,root) %{_bindir}/thermald %config(noreplace) %{_sysconfdir}/dbus-1/system.d/org.freedesktop.thermald.conf %{_datadir}/dbus-1/system-services/org.freedesktop.thermald.service @@ -45,6 +56,7 @@ rm -rf $RPM_BUILD_ROOT %{_unitdir}/thermald.service
%changelog -* Wed May 8 2013 Base version 1.0-1 -- Base version -- updated to vesrion 1.02 +* Tue Jun 18 2013 Srinivas Pandruvada srinivas.pandruvada@linux.intel.com 1.02-2 +- Update spec file after first review +* Fri Jun 14 2013 Srinivas Pandruvada srinivas.pandruvada@linux.intel.com 1.02-1 +- Initial package
=============
Question: Is "Documentation=" field in the thermald.service is mandatory?
I hope this is better than the previous.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #9 from Marcelo Barbosa mr.marcelo.barbosa@gmail.com --- Hello Srinivas,
Your release is much better for this last version, but necessary see this it:
Issues: =======
1) Sources used to build the package match the upstream source, as provided in the spec URL.
Note: Upstream MD5sum check error, diff is in
Only in /home/marcelo.barbosa/rpmbuild/SOURCES/reviews/974714-thermal_daemon/srpm-unpacked/thermal_daemon-v1.02.tar.gz-extract: thermal_daemon-1.02 Only in /home/marcelo.barbosa/rpmbuild/SOURCES/reviews/974714-thermal_daemon/upstream-unpacked/Source0: thermal_daemon-78b9dd1343bf7350740eb76e3bf87dc207078d9d
Source checksums ----------------
https://github.com/01org/thermal_daemon/archive/78b9dd1343bf7350740eb76e3bf8... : CHECKSUM(SHA256) this package : d870160e9d0518e2cc0e0fdc948e9feb28112638de70f696f393683a913893c2 CHECKSUM(SHA256) upstream package : 4c0beba121cb2769a6a423e2d861bbf1210d717c631d57d21e9001ad12205cd2 diff -r also reports differences
Please see: http://fedoraproject.org/wiki/Packaging/SourceURL
PS: You not use variable shortcommit in this line: Source0: https://github.com/01org/thermal_daemon/archive/%%7Bcommit%7D/%%7Bname%7D-v%...
2) Please remove this line, is not necessary:
Requires: libxml2
ERROR: thermal_daemon.x86_64: E: explicit-lib-dependency libxml2
3) About this question is mandatory, please see: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd
Thank you and good working.
Marcelo Barbosa Fedora Project Packager Fedora Project Ambassador firemanxbr@fedoraproject.org
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #10 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Hi Marcelo,
I have uploaded new spec file and source rpm. I did a diff -r on the tar file from what is generated and what is obtained from gitweb. There is no difference, but checksum doesn't match.
I see at https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL a comment: " Keep in mind that github tarballs are generated on-demand, so their modification dates will vary and cause checksum tests to fail. Reviewers will need to use diff -r to verify the tarballs. "
Also added documentation field in the systemd service file.
Thanks, Srinivas
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cickumqt@gmail.com
--- Comment #11 from Christopher Meng cickumqt@gmail.com --- Some suggestions:
1. In %build section,
prefix=/usr, please change it to %{_prefix}
2. In %install section,
Please remove "rm -rf $RPM_BUILD_ROOT"
3. And, please replace $RPM_BUILD_ROOT by %{buildroot}
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #12 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Thanks. Did the suggested changed and submitted new spec file source rpm. Let me know if there are any other suggestions.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #13 from Marcelo Barbosa mr.marcelo.barbosa@gmail.com --- Srinivas,
Please post in commentary your new updates:
INFO: Processing bugzilla bug: 974714 INFO: Getting .spec and .srpm Urls from : 974714 INFO: --> SRPM url: http://spandruvada.fedorapeople.org/thermal_daemon-1.02-2.fc19.src.rpm INFO: --> Spec url: http://spandruvada.fedorapeople.org/thermal_daemon.spec INFO: Using review directory: /home/marcelo.barbosa/rpmbuild/SOURCES/reviews/974714-thermal_daemon INFO: Downloading .spec and .srpm files ERROR: 'Error 404 downloading http://spandruvada.fedorapeople.org/thermal_daemon-1.02-2.fc19.src.rpm'
Example:
url with .spec file url with src rpm file
PS: not necessary diff command.
Marcelo Barbosa
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #14 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Hi Marcelo,
Spec URL: http://spandruvada.fedorapeople.org/thermal_daemon.spec SRPM URL: http://spandruvada.fedorapeople.org/thermal_daemon-1.02-4.fc19.src.rpm
I didn't understand "post in commentary". I see you are firing some build from bugzilla, how do you do that?
I can't attach Koji results as it will not work behind our firewall at work.
Thanks for your patience.
Srinivas
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #15 from Marcelo Barbosa mr.marcelo.barbosa@gmail.com --- Srinivas,
You execute new changes in your .spec file, do you have generate your packages again:
$ rpmlint -bs .spec.file
And:
$ rpmlint -ba .spec.file
Look this error again:
OUTUPUT:
Rpmlint ------- Checking: thermal_daemon-1.02-4.fc18.x86_64.rpm thermal_daemon.x86_64: W: only-non-binary-in-usr-lib 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
Rpmlint (installed packages) ---------------------------- # rpmlint thermal_daemon thermal_daemon.x86_64: E: explicit-lib-dependency libxml2 thermal_daemon.x86_64: W: only-non-binary-in-usr-lib 1 packages and 0 specfiles checked; 1 errors, 1 warnings. # echo 'rpmlint-done:'
Look this output cacater "E" = ERROR, but "W" = WARNING, in this package your "W" is false positive.
PS: Other commentary this output from fedora-review tool.
Marcelo Barbosa
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #16 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Hi Marcelo, I did regenerate and upload. I don't see this error. This is what I did to test the uploaded SRPM: - Emptied my ~/rpmbuild sub-folder contents. So no RPM or SPEC file, tar files exists - wget http://spandruvada.fedorapeople.org/thermal_daemon-1.02-4.fc19.src.rpm - rpm -ivh thermal_daemon-1.02-4.fc19.src.rpm - This installed source in ~/rpmbuild/SOURCES and spec file in ~/rpmbuild/SPECS - rpmbuild -bb /home/spandruvada/rpmbuild/SPECS/thermal_daemon.spec - It generated thermal_daemon-1.02-4.fc19.x86_64.rpm and thermal_daemon-debuginfo-1.02-4.fc19.x86_64.rpm - rpm -qa | grep thermal , shows the installed package - rpmlint /home/spandruvada/rpmbuild/SPECS/thermal_daemon.spec thermal_daemon-1.02-4.fc19.src.rpm thermal_daemon.src: W: file-size-mismatch thermal_daemon-v1.02-121414e.tar.gz = 760874, https://github.com/01org/thermal_daemon/archive/121414ee4aef9cfea68179687807... = 758973 1 packages and 1 specfiles checked; 0 errors, 1 warnings.
(As expected)
- rpmlint thermal_daemon thermal_daemon.x86_64: W: only-non-binary-in-usr-lib 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
I don't see libxml2 error.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Marcelo Barbosa mr.marcelo.barbosa@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |echevemaster@gmail.com
--- Comment #17 from Marcelo Barbosa mr.marcelo.barbosa@gmail.com --- Srinivas,
Please add in this ticket one new block with: FE-NEEDSPONSOR With new block you get one sponsor for grant your for packager. Your package is almost finish, only two adjustments, see below this items with [!]:
==================================================================== - the rpmlint is false positive in: W: only-non-binary-in-usr-lib
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
===== 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]: %build honors applicable compiler flags or justifies otherwise. [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 [-]: Package requires other packages for directories it uses. [x]: Package uses nothing in %doc for runtime. [x]: Package is not known to require ExcludeArch. [x]: Package complies to the 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", "GPL (v2)", "Unknown or generated". 3 files have unknown license. Detailed output of licensecheck in
/home/marcelo.barbosa/rpmbuild/SOURCES/reviews/974714-thermal_daemon/licensecheck.txt [x]: Package consistently uses macro is (instead of hard-coded directory names).
[!]: Package is named according to the Package Naming Guidelines. please see: https://fedoraproject.org/wiki/Packaging:NamingGuidelines?rd=Packaging/Namin...
[x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [x]: Package contains systemd file(s) if in need. [-]: Useful -debuginfo package or justification otherwise. [-]: Large documentation must go in a -doc subpackage. Note: Documentation size is 30720 bytes in 2 files. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: %config files are marked noreplace or the reason is justified. [x]: Each %files section contains %defattr if rpm < 4.4 [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Fully versioned dependency in subpackages, if present. [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 use %makeinstall only when make install' ' DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: No %config files under /usr. [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 [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).
===== 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. test_pref.sh [marcelo.barbosa@firelap test]$ pwd /home/marcelo.barbosa/rpmbuild/SOURCES/reviews/974714-thermal_daemon/srpm-unpacked/thermal_daemon-v1.02-121414e.tar.gz-extract/thermal_daemon-121414ee4aef9cfea6817968780701531ebf814e/test
[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. [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Uses parallel make. [x]: SourceX tarball generation or download is documented. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define.
===== EXTRA items =====
Generic: [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [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: thermal_daemon-1.02-4.fc18.x86_64.rpm thermal_daemon.x86_64: W: only-non-binary-in-usr-lib 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
Rpmlint (installed packages) ---------------------------- # rpmlint thermal_daemon thermal_daemon.x86_64: W: only-non-binary-in-usr-lib 1 packages and 0 specfiles checked; 0 errors, 1 warnings. # echo 'rpmlint-done:'
Requires -------- thermal_daemon (rpmlib, GLIBC filtered): /bin/sh config(thermal_daemon) libc.so.6()(64bit) libdbus-1.so.3()(64bit) libdbus-glib-1.so.2()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgio-2.0.so.0()(64bit) libglib-2.0.so.0()(64bit) libgmodule-2.0.so.0()(64bit) libgobject-2.0.so.0()(64bit) libgomp.so.1()(64bit) libm.so.6()(64bit) libpthread.so.0()(64bit) librt.so.1()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libxml2.so.2()(64bit) libxml2.so.2(LIBXML2_2.4.30)(64bit) libxml2.so.2(LIBXML2_2.6.0)(64bit) rtld(GNU_HASH) systemd-units
Provides -------- thermal_daemon: config(thermal_daemon) thermal_daemon thermal_daemon(x86-64)
Source checksums ---------------- https://github.com/01org/thermal_daemon/archive/121414ee4aef9cfea68179687807... : CHECKSUM(SHA256) this package : 7be4249f07e151ab8d4e3140cbc09360456fada5455b30488baa589827398cac CHECKSUM(SHA256) upstream package : 73d8c2744f8d5fb653010b666489863b488cb49777b0a41e36d669c27241e061 However, diff -r shows no differences
Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29 Buildroot used: fedora-18-x86_64 Command line :/usr/bin/fedora-review -b 974714
====================================================================
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #18 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Hi Marcelo, What is the failure "[!]: %check is present and all tests pass." means?
Thanks, Srinivas
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #19 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Also For the package naming guidlines:
- Common Character Set for Package Naming : Meets this requirment
When naming a package, the name should match the upstream tarball or project: Meets this requirment
When naming packages for Fedora, the maintainer must use the dash '-' as the delimiter for name parts. The maintainer must NOT use an underscore '_', a plus '+', or a period '.' as a delimiter. Version numbers used in compat libraries do not need to omit the dot '.' or change it into a dash '-':
This seems to be issue. The problem is that if remove underscore here, then it will not match the upstream tar ball name. What can be done about this?
Thanks for your help and patience.
Thanks, Srinivas
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Srinivas Pandruvada srinivas.pandruvada@linux.intel.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #20 from Christopher Meng cickumqt@gmail.com --- IMO If I package this program, I'll name it to thermals.
And I found the service file is also named thermald.
Underscore is bad, yes.
So just change them, and define a new macro like:
%global pkgname thermal_daemon
Then use this new macro in SourceURL and other places.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #21 from Marcelo Barbosa mr.marcelo.barbosa@gmail.com --- Srinivas,
Besides the name also confer the other problem(%check) your source have dir "/test" but your spec file no.
Marcelo Barbosa
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #22 from Michael Schwendt mschwendt@gmail.com --- Re: comment 19
Take a look at
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Separators
and notice what it says towards the bottom of that section:
| There are a few exceptions to the no underscore '_' rule. | | [...] | * packages where the upstream name naturally contains an underscore | are excluded from this. Examples of these packages include: | | [...] | | If in doubt, ask on fedora-devel-list.
# rpm -qa --qf %{n}\n *_*|sort libart_lgpl libcom_err libini_config libipa_hbac libnetfilter_conntrack libpath_utils libref_array libsss_idmap microcode_ctl pam_krb5 pam_pkcs11 python-backports-ssl_match_hostname sg3_utils-libs tcp_wrappers tcp_wrappers-libs usb_modeswitch usb_modeswitch-data wpa_supplicant
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Yohan Graterol yohangraterol92@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW CC| |yohangraterol92@gmail.com Assignee|mr.marcelo.barbosa@gmail.co |nobody@fedoraproject.org |m |
--- Comment #23 from Yohan Graterol yohangraterol92@gmail.com --- Hi Marcelo,
Good job! But, you can not assigned this package. Only a sponsor can do formal review.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #24 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- David Woodhouse (dwmw2@infradead.org) is willing to sponsor me. So I will update spec and srpm files after I pass all fedora-review tests.
Thanks to Marcelo and Christopher to spend time and help me to learn submission process.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #25 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Spec URL: http://spandruvada.fedorapeople.org/thermal-daemon.spec SRPM URL: http://spandruvada.fedorapeople.org/thermal-daemon-1.02-5.fc19.src.rpm
Removed underscore in package name. Added comment for %check as there is no self test exist in the upstream repo.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Mario Blättermann mario.blaettermann@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |mario.blaettermann@gmail.co | |m Assignee|nobody@fedoraproject.org |mr.marcelo.barbosa@gmail.co | |m
https://bugzilla.redhat.com/show_bug.cgi?id=974714
David Woodhouse dwmw2@infradead.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dwmw2@infradead.org Assignee|mr.marcelo.barbosa@gmail.co |dwmw2@infradead.org |m |
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #26 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Updated to thermal daemon version 1.03. Spec URL: http://spandruvada.fedorapeople.org/thermal-daemon.spec SRPM URL: http://spandruvada.fedorapeople.org/thermal-daemon-1.03-1.fc19.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC|cickumqt@gmail.com | Flags|fedora-review? |
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #27 from Piruthiviraj Natarajan piruthiviraj@gmail.com ---
@srinivas
I see updated package release from upstream. Do you mind submitting an updated version here?
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Yohan Graterol yohangraterol92@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC|yohangraterol92@gmail.com |
Eric Griffith EGriffith92@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |EGriffith92@gmail.com
--- Comment #28 from Eric Griffith EGriffith92@gmail.com --- Hey guys, im tempted to try and take over / reviving packaging of thermal_daemon. It'd be my first attempt at packaging though so would anyone be willing to be kind of be my guide in the matter?
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #29 from Srinivas Pandruvada srinivas.pandruvada@linux.intel.com --- Eric Griffith : Feel free to take over.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #30 from Eric Griffith EGriffith92@gmail.com --- Question for anyone keeping an eye on this. Srinivas I've got your spec file on hand as a reference but I've got a bit of a general question that I'd love to have answered beyond just this one .spec file.
Currently it says:
Source0: https://github.com/01org/thermal_daemon/archive/v%%7Bversion%7D.tar.gz
But when I do "rpmbuild -ba thermal_daemon.spec" (following Fedora Documentation http://docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_... ) then rpmbuild complains about:
error: File /home/egriffith/rpmbuild/SOURCES/v1.30.tar.gz: No such file or directory
presumably because its just looking at "v%{version}.tar.gz". What is the appropriate fix here? The upstream source is in fact:
https://github.com/01org/thermal_daemon/archive/v%%7Bversion%7D.tar.gz (in this case v1.30.tar.gz)
but the download link gives you "thermal_daemon-1.3.tar.gz" which is in my SOURCES directory. So do I rename thermal_daemon-1.3.tar.gz to be "v1.30.tar.gz" or do I modify "Source0" to not be 100% faithful to the actual URL? Or is there an override I can use to specify that the downloaded files have a different name than the upstream source URL?
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #31 from Eric Griffith EGriffith92@gmail.com --- Created attachment 991607 --> https://bugzilla.redhat.com/attachment.cgi?id=991607&action=edit New spec file, please review.
Disclaimer: First time writing a spec file, tried to follow all available guidelines. This builds, installs and works fine so i know this isn't broken just want to make it accurate as per guidelines & assumptions.
A handful of questions in regards to the spec file format:
1) rpmlint complains about source0 not being a valid URL, but it rpmbuild can't find the source if I have it correct... see my above post in regards to that situation.
2) %build section. The original spec file had 'autoreconf' the compilation instructions had ./autogen.sh. Which is the correct one to use? Current spec uses autogen.sh
3) Is a "BuildRequires: systemd" necessary or is it just redundant?
4) Can someone link or explain the Requires(post)/(preun)/(postun), same with %post, %preun, %postun
5) %files does every single file need to be listed here or just 'important ones? I left out /usr/share/src, /usr/lib and a couple others.
6) Upstream name is "thermal_daemon" but I see it packaged elsewhere as "thermald" and the binary / .service is thermald. Preference goes to...?
https://bugzilla.redhat.com/show_bug.cgi?id=974714
jeremy9856@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jeremy9856@gmail.com
--- Comment #32 from jeremy9856@gmail.com --- Is there still something that block the package to be add in the repo ?
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Srinivas Pandruvada srinivas.pandruvada@intel.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |srinivas.pandruvada@intel.c | |om
--- Comment #33 from Srinivas Pandruvada srinivas.pandruvada@intel.com --- Someone needs to be maintainer who can gets a sponsor or already a maintainer.
I tried 2 and half years back, but I can't be maintainer for this package for Fedora anymore because of my schedule.
So anybody wants to own it, please feel free to take over.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
--- Comment #34 from jeremy9856@gmail.com --- I'm surprised that nobody want to take care of this package because it's a fairly important one.
https://bugzilla.redhat.com/show_bug.cgi?id=974714
Jens Lody fedora@jenslody.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@jenslody.de
--- Comment #35 from Jens Lody fedora@jenslody.de --- https://bugzilla.redhat.com/show_bug.cgi?id=1440406
--- Comment #36 from jeremy9856@gmail.com --- Well, it seem someone took care of it :) I guess that can be closed now.
Thanks !
package-review@lists.fedoraproject.org