Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 191743
Summary: | Review Request: sysprof - a sampling CPU profiler | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Gianluca Sforna <giallu> |
Component: | Package Review | Assignee: | Parag AN(पराग) <panemade> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora, k.georgiou, sandmann |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-10-08 08:51:56 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 191745 | ||
Bug Blocks: | 163779 |
Description
Gianluca Sforna
2006-05-15 15:32:38 UTC
Some Comments:- rpmling gives error as E: sysprof configure-without-libdir-spec I am leaving for a couple of weeks, so I can not look into this until I come back. thanks for your patience... I fixed the rpmlint error by using the %configure macro and also updated the package to version 1.0.3 Spec URL: http://giallu.interfree.it/fedora/sysprof.spec SRPM URL: http://giallu.interfree.it/fedora/sysprof-1.0.3-1.src.rpm The package should own the directory %{_datadir}/sysprof and this line will be sufficient in the files section. Then there is also no need to specify each file explicitly. %{_datadir}/%{name} will be even better, similar package the binary as %{_bindir}/%{name} Thanks. I added a %dir %{_datadir}/sysprof line to the spec, but I prefer to maintain the explicit list of files, since there are really few of them. I am waiting a bit for new comments/requests before updating the .spec and src.rpm files. Forgot to mention this is my first package submission so I am seeking for a sponsor I'm not a reviewer, but I can help: This is shown by rpmlin on the SRPMS... E: sysprof configure-without-libdir-spec A configure script is run without specifying the libdir. configure options must be augmented with something like --libdir=%{_libdir}. New package addressing comment #1 (and comment #8...) http://giallu.homelinux.org/fedora/sysprof.spec http://giallu.homelinux.org/fedora/sysprof-1.0.3-2.src.rpm Please note I have been sponsored, so now it only need an official review from another regular contributor to have this piece of software in Fedora. I suggest you to change SPEC name to sysprof-kmod-common.spec and resubmit package. This is according to kernel module packaging guidelines. Thoug its given that userlan package should Provides: %{name}- kmod-common = %{version} it will be good idea to have similar name given to SPEC also. Anyway i tried with latest uploaded SRPM link. Mock build is failed with error checking for cplus_demangle_opname in -liberty... no configure: error: libiberty is required to compile sysprof error: Bad exit status from /var/tmp/rpm-tmp.63043 (%build) (In reply to comment #10) > I suggest you to change SPEC name to sysprof-kmod-common.spec and resubmit > package. This is according to kernel module packaging guidelines. Nope, see http://fedoraproject.org/wiki/Packaging/KernelModules#head-164fc2703b23579d258b39d675a92643669507e0 In this case such a rename would sound plain wrong to me. Provides: sysprof-kmod-common is lacking a version, though. so i can give any name to userlan package SPEC file with following MUST right? - MUST: The package needs to require the belonging kernel-module with something like 'Requires: %{name}- kmod = %{version}' - MUST: The package needs to provide %{name}-kmod-common with something like 'Provides: %{name}- kmod-common = %{version}' or the name of the package must be %{name}-kmod-common (In reply to comment #13) > so i can give any name to userlan package SPEC file Within the usual package naming guidelines, yes. (I assume you're talking about the userland package name, not only the specfile filename.) > - MUST: The package needs to require the belonging kernel-module with > something like 'Requires: %{name}- kmod = %{version}' ">=" is better than "=" here - the kmod guidelines are in need of some updates. Parag: Seems you are reviewing this package, so I am changing the blocker to FE- REVIEW. If thats not the case you can change it back to FE-NEW and reassign to nobody New package: this one builds correctly in mock (-devel) http://giallu.homelinux.org/fedora/sysprof.spec http://giallu.homelinux.org/fedora/sysprof-1.0.3-3.src.rpm Ville: I am not sure why the Require should read ">=": I assumed kmod- and kmod-common should always be updated in sync, while the ">=" seems to imply I could update only the kmod- retaining the older kmod-common. Am I missing something? Anyway, if you like I could also update the wiki page. The exact details escape me at the moment, but while = would be closer to the intent, >= here IIRC helps in some upgrade scenarios. It's also possible that comment 14 was a brainfart - thl, do you remember better? Anyway, upgrading only the kmod package is prevented by the specfile emitted by kmodtool; it produces a >= dependency to the corresponding kmod-common in the kmod package. (In reply to comment #17) > The exact details escape me at the moment, but while = would be closer to the > intent, >= here IIRC helps in some upgrade scenarios. It's also possible that > comment 14 was a brainfart - thl, do you remember better? /me scratches his head and tries to remember but fails for now, too Anyway: I agree ">= here IIRC helps in some upgrade scenarios" and that should be used. Review: + package builds in mock (development i386). + source files match upstream. 8949fe32a073b84cb2abb7f9d608f755 sysprof-1.0.3.tar.gz + package meets naming and packaging guidelines. + specfile is properly named, is cleanly written + dist tag is present. + build root is correct. + license is open source-compatible. License text included in package. + BuildRequires are proper. + %clean is present. + package installs properly + rpmlint is silent. + Provides: sysprof-kmod-common = 1.0.3 + Requires: kmod-sysprof >= 1.0.3 libatk-1.0.so.0 libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1) libc.so.6(GLIBC_2.2.3) libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) libcairo.so.2 libdl.so.2 libdl.so.2(GLIBC_2.0) libdl.so.2(GLIBC_2.1) libgdk-x11-2.0.so.0 libgdk_pixbuf-2.0.so.0 libglade-2.0.so.0 libglib-2.0.so.0 libgmodule-2.0.so.0 libgobject-2.0.so.0 libgthread-2.0.so.0 libgtk-x11-2.0.so.0 libm.so.6 libpango-1.0.so.0 libpangocairo-1.0.so.0 libpangoft2-1.0.so.0 libpthread.so.0 libpthread.so.0(GLIBC_2.0) libxml2.so.2 libz.so.1 rtld(GNU_HASH) + owns the directories it creates. + doesn't own any directories it shouldn't. + no duplicates in %files. + file permissions are appropriate. + no scriptlets present. + documentation is small, so no -docs subpackage is necessary. + Not a GUI app APPROVED. Don't forget to close this NEXTRELASE when you have imported and built it. (In reply to comment #19) > + Not a GUI app ??? it is actually a GNOME program... and this let me realize I did not add a .desktop file (sorry, this was my first packaging attempt). I will import a fixed spec. Thanks a lot (In reply to comment #21) > (In reply to comment #19) > > + Not a GUI app > > ??? > it is actually a GNOME program... and this let me realize I did not add a > .desktop file (sorry, this was my first packaging attempt). > Yup By mistake i wrote that. in fact that was from my template for review which i just copied but not erased. I in fact check thet GUI APP already. That was because i could not found any .desktop file. > I will import a fixed spec. Thanks a lot Yes .desktop file is not available in this package. Add it and then import your package. Package imported and built |