Spec URL: http://people.redhat.com/fsimonce/vdsm/vdsm.spec SRPM URL: http://people.redhat.com/fsimonce/vdsm/vdsm-4.9.0-0.192.g69eb727.fc15.src.rpm Description: The VDSM service is required by a Virtualization Manager to manage the Linux hosts. VDSM manages and monitors the host's storage, memory and networks as well as virtual machine creation, other host administration tasks, statistics gathering, and log collection.
Is there a technical reason for the ExclusiveArch: x86_64 ? We (the secondary architectures) prefer to build packages on all possible architectures.
(In reply to comment #1) > Is there a technical reason for the ExclusiveArch: x86_64 ? We (the secondary > architectures) prefer to build packages on all possible architectures. At the moment I am not aware of technical reasons for not building it on secondary architectures. Sadly I cannot guarantee that it will actually work (especially the safelease part) and that its interaction with the other components will be optimal. Moreover it will require libvirt and kvm for most of the tasks so I'm not sure what would be used for on secondary architectures. Anyway if it's just easier and more consistent to build it for all the architectures we could try to fix the build errors (if any).
Howdy, (In reply to comment #2) > components will be optimal. Moreover it will require libvirt and kvm for most > of the tasks so I'm not sure what would be used for on secondary architectures. Could vdsm be installed on another arch just to manage, remotely, a KVM HV? If yes, then it might be interesting having it available on those other architectures, otherwise I guess maintaining the ExclusiveArch: x86_64 makes more sense. Cheers! --aquini
Hello Federico, You should raise the flag fedora-review to get attention from reviewers (I have added now), next time please use it. I have reviewed the spec/package, please check the items marked as WARN (not blocker), blocker items for package approval are marked as FAIL. Thanks Douglas From: http://fedoraproject.org/wiki/Packaging:ReviewGuidelines [OK] MUST: rpmlint must be run on the source rpm and all binary rpms the build produces. The output should be posted in the review $ rpmlint vdsm.spec vdsm.spec: W: invalid-url Source0: vdsm-%{version}.tar.gz 0 packages and 1 specfiles checked; 0 errors, 1 warnings. $ rpmlint vdsm-4.9.0-0.192.g69eb727.fc15.src.rpm vdsm.src: W: invalid-url Source0: vdsm-%{version}.tar.gz 1 packages and 0 specfiles checked; 0 errors, 1 warnings [OK] MUST: The package must be named according to the Package Naming Guidelines . [OK] MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption. [OK] MUST: The package must meet the Packaging Guidelines . [OK] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines . [OK] MUST: The License field in the package spec file must match the actual license. [OK] MUST: 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 must be included in %doc [OK] MUST: The spec file must be written in American English. [OK] MUST: The spec file for the package MUST be legible. [FAIL] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this. From: http://fedoraproject.org/wiki/Packaging/SourceURL Please include a comment into the SPEC how the .tar.gz was build. I was not able to locate any reference (commit #, tag, branch) to "g69eb727" within the pointed upstream git tree -- http://git.fedorahosted.org/git/?p=vdsm.git [OK] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. $ mock -r fedora-16-x86_64 vdsm-4.9.0-0.192.g69eb727.fc15.src.rpm INFO: Done(vdsm-4.9.0-0.192.g69eb727.fc15.src.rpm) Config(fedora-16-x86_64) 1 minutes 3 seconds INFO: Results and/or logs in: /var/lib/mock/fedora-16-x86_64/result State Changed: end $ cat /var/lib/mock/fedora-16-x86_64/result/build.log <snip> Checking for unpackaged file(s): /usr/lib/rpm/check-files /builddir/build/BUILDROOT/vdsm-4.9.0-0.192.g69eb727.fc16.x86_64 Wrote: /builddir/build/RPMS/vdsm-4.9.0-0.192.g69eb727.fc16.x86_64.rpm Wrote: /builddir/build/RPMS/vdsm-debuginfo-4.9.0-0.192.g69eb727.fc16.x86_64.rpm Wrote: /builddir/build/RPMS/vdsm-hook-vhostmd-4.9.0-0.192.g69eb727.fc16.x86_64.rpm Wrote: /builddir/build/RPMS/vdsm-debug-plugin-4.9.0-0.192.g69eb727.fc16.x86_64.rpm Wrote: /builddir/build/RPMS/vdsm-cli-4.9.0-0.192.g69eb727.fc16.x86_64.rpm Wrote: /builddir/build/RPMS/vdsm-bootstrap-4.9.0-0.192.g69eb727.fc16.x86_64.rpm Wrote: /builddir/build/RPMS/vdsm-reg-4.9.0-0.192.g69eb727.fc16.x86_64.rpm Wrote: /builddir/build/RPMS/vdsm-hook-faqemu-4.9.0-0.192.g69eb727.fc16.x86_64.rpm Executing(%clean): /bin/sh -e /var/tmp/rpm-tmp.5wZE89 + umask 022 + cd /builddir/build/BUILD + cd vdsm-4.9.0 + /bin/rm -rf /builddir/build/BUILDROOT/vdsm-4.9.0-0.192.g69eb727.fc16.x86_64 + exit 0 Child returncode was: 0 LEAVE do --> [N/A] MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line. [8] [WARN] MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense. As clean up suggestion only, you can remove redhat-rpm-config as BuildRequires. From: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 Exceptions There is no need to include the following packages or their dependencies as BuildRequires because they would occur too often. These packages are considered the minimum build environment. <snip> redhat-rpm-config </snip> From spec: BuildRequires: python redhat-rpm-config [N/A] MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden. [N/A] MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun [N/A] MUST: Packages must NOT bundle copies of system libraries [N/A] MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker. [OK] MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. [OK] MUST: A Fedora package must not list a file more than once in the spec file's %files listings. (Notable exception: license texts in specific situations) [OK] MUST: Permissions on files must be set properly. Executables should be set with executable permissions. [FAIL] MUST: Each package must consistently use macros Item 1 ============= From spec: %dir %{_localstatedir}/log/core /etc/init.d/vdsmd <----- here please replace /etc/init.d to macro (%{_initrddir}/vdsmd) Item 2 =================== As suggestion, I would recommend to use macro vdsm_name to the Name session and others that call vdsm string directly. <snip> %define vdsm_name vdsm Summary: Virtual Desktop Server Manager Name: vdsm Source: %{vdsm_name}-%{version}.tar.gz </snip> Item 3 ===================== Please decide if you are going use macros/full path to cp/rm/sed commands or not. Select one method and use it. Example: <snip> %post tmp_sudoers=$(mktemp) cp -a /etc/sudoers $tmp_sudoers <--- here </snip> and if outerr=$(/usr/sbin/visudo -c -f $tmp_sudoers 2>&1) ; then /bin/cp -a $tmp_sudoers /etc/sudoers <--- here full path for cp [OK] MUST: The package must contain code, or permissable content [N/A] MUST: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). [OK] MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. [N/A] MUST: Header files must be in a -devel package. [N/A] MUST: Static libraries must be in a -static package. [N/A] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. [N/A] MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name}%{?_isa} = %{version}-%{release} [N/A] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. [N/A] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. [FAIL] MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. <snip> %files hook-faqemu %defattr(-,root,root,-) %doc COPYING %{_bindir}/qemu %{_bindir}/qemu-system-x86_64 qemu and qemu-system-x86_64 are binaries from qemu-system-x86-0.13.0-1.fc14.x86_64 package which are not owned by vdsm. Please includes into hook-faqemu session "Conflicts: qemu-system-x86" [OK] MUST: All filenames in rpm packages must be valid UTF-8. [OK] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. License text included in COPYING file that comes with upstream tarball [N/A] SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available [OK] SHOULD: The reviewer should test that the package builds in mock. [N/A] SHOULD: The package should compile and build into binary rpms on all supported architectures. [OK] SHOULD: If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity. [ [OK] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. [N/A] SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb. [N/A] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. [OK] SHOULD: your package should contain man pages for binaries/scripts. If it doesn't, work with upstream to add them where they make sense.
(In reply to comment #4) > [OK] MUST: rpmlint must be run on the source rpm and all binary rpms the build > produces. The output should be posted in the review > > $ rpmlint vdsm.spec > vdsm.spec: W: invalid-url Source0: vdsm-%{version}.tar.gz > 0 packages and 1 specfiles checked; 0 errors, 1 warnings. > > $ rpmlint vdsm-4.9.0-0.192.g69eb727.fc15.src.rpm > vdsm.src: W: invalid-url Source0: vdsm-%{version}.tar.gz > 1 packages and 0 specfiles checked; 0 errors, 1 warnings At the moment we don't maintain a tarball release. I'll update this as soon as we will have one. > [FAIL] MUST: The sources used to build the package must match the upstream > source, as provided in the spec URL. Reviewers should use md5sum for this task. > If no upstream URL can be specified for this package, please see the Source URL > Guidelines for how to deal with this. > > From: http://fedoraproject.org/wiki/Packaging/SourceURL > > Please include a comment into the SPEC how the .tar.gz was build. > I was not able to locate any reference (commit #, tag, branch) to "g69eb727" > within the pointed upstream git tree -- > http://git.fedorahosted.org/git/?p=vdsm.git The commit g69eb727 is not present in the repository since it's actually the spec modification. When the package will be accepted the hash will appear upstream. > [FAIL] MUST: Packages must not own files or directories already owned by other > packages. The rule of thumb here is that the first package to be installed > should own the files or directories that other packages may rely upon. This > means, for example, that no package in Fedora should ever share ownership with > any of the files or directories owned by the filesystem or man package. If you > feel that you have a good reason to own a file or directory that another > package owns, then please present that at package review time. > > <snip> > %files hook-faqemu > %defattr(-,root,root,-) > %doc COPYING > %{_bindir}/qemu > %{_bindir}/qemu-system-x86_64 > > qemu and qemu-system-x86_64 are binaries from > qemu-system-x86-0.13.0-1.fc14.x86_64 package which are not owned by vdsm. > Please includes into hook-faqemu session > "Conflicts: qemu-system-x86" This was needed only on rhel. I will make this part conditional. Updated spec/srpm: Spec URL: http://people.redhat.com/fsimonce/vdsm/vdsm.spec SRPM URL: http://people.redhat.com/fsimonce/vdsm/vdsm-4.9.0-0.193.g590c86f.fc15.src.rpm
Frederico, In spite of this is being an informal review, I ask you to consider some of the following observations, please. * As I could not find your Fedora Account, I don't know if this is your first RPM package submission or not. If this is you first submission, take a closer look at: - http://fedoraproject.org/wiki/PackageMaintainers/Join * You should write entries to the %changelog section, with review stuff to keep track of modifications made to the package, as reviews usually impose some spec/srpm changing. In order to improve your changelogs, take a look at: - http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs * The release number is how the maintainer marks build revisions, and it should start from 1. Whenever a minor change (spec file changed, patch added/removed) occurs, or a package is rebuilt to use newer headers or libraries, the release number should be incremented; - http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Release_Tag (In reply to comment #5) > The commit g69eb727 is not present in the repository since it's actually the > spec modification. When the package will be accepted the hash will appear > upstream. This in not the correct way to proceed. Code shipped as rpm source must _always_ match upstream. It really doesn't matter if your code is either a downloaded tarball, or a tarball built based on a SCM checkout. Prior to have this work aproved, you should fix this item, as this is a real blocker. In order to dismiss any shadow of doubt, please, check the 10th mandatory item from http://fedoraproject.org/wiki/Packaging:ReviewGuidelines "MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this" Cheers! --aquini
Hi Federico, (In reply to comment #5) > (In reply to comment #4) > > [OK] MUST: rpmlint must be run on the source rpm and all binary rpms the build > > produces. The output should be posted in the review > > > > $ rpmlint vdsm.spec > > vdsm.spec: W: invalid-url Source0: vdsm-%{version}.tar.gz > > 0 packages and 1 specfiles checked; 0 errors, 1 warnings. > > > > $ rpmlint vdsm-4.9.0-0.192.g69eb727.fc15.src.rpm > > vdsm.src: W: invalid-url Source0: vdsm-%{version}.tar.gz > > 1 packages and 0 specfiles checked; 0 errors, 1 warnings > > At the moment we don't maintain a tarball release. > I'll update this as soon as we will have one. Yes, I am aware about that. This item are marked as [OK] > > [FAIL] MUST: The sources used to build the package must match the upstream > > source, as provided in the spec URL. Reviewers should use md5sum for this task. > > If no upstream URL can be specified for this package, please see the Source URL > > Guidelines for how to deal with this. > > > > From: http://fedoraproject.org/wiki/Packaging/SourceURL > > > > Please include a comment into the SPEC how the .tar.gz was build. > > I was not able to locate any reference (commit #, tag, branch) to "g69eb727" > > within the pointed upstream git tree -- > > http://git.fedorahosted.org/git/?p=vdsm.git > > The commit g69eb727 is not present in the repository since it's actually the > spec modification. When the package will be accepted the hash will appear > upstream. > Please see Rafael Aquini comment (https://bugzilla.redhat.com/show_bug.cgi?id=745510#c6) > > [FAIL] MUST: Packages must not own files or directories already owned by other > > packages. The rule of thumb here is that the first package to be installed > > should own the files or directories that other packages may rely upon. This > > means, for example, that no package in Fedora should ever share ownership with > > any of the files or directories owned by the filesystem or man package. If you > > feel that you have a good reason to own a file or directory that another > > package owns, then please present that at package review time. > > > > <snip> > > %files hook-faqemu > > %defattr(-,root,root,-) > > %doc COPYING > > %{_bindir}/qemu > > %{_bindir}/qemu-system-x86_64 > > > > qemu and qemu-system-x86_64 are binaries from > > qemu-system-x86-0.13.0-1.fc14.x86_64 package which are not owned by vdsm. > > Please includes into hook-faqemu session > > "Conflicts: qemu-system-x86" > > This was needed only on rhel. I will make this part conditional. > Ok. Before we go further, you should fix/answer Rafael's comments. https://bugzilla.redhat.com/show_bug.cgi?id=745510#c6 Thanks Douglas
Spec URL: http://people.redhat.com/fsimonce/vdsm/vdsm.spec SRPM URL: http://people.redhat.com/fsimonce/vdsm/vdsm-4.9.0-1.gitb60414c0.fc15.src.rpm
Howdy Frederico, In a glance to your last SPEC modifications, I'm ok with your work now. Since you got your sponsorship from rjones, now you should ask him to formally review this work and approve it, before you can go on and ask for a Fedora SCM repository for this project. Happy Packaging! --aquini
Hello Frederico, From my side, your package is APPROVED as well. However, as Rafael said above, now you should request a formal review from Richard Jones, after that you should go and request the Fedora SCM by raising the fedora-cvs? flag. Good work! Cheers Douglas
Sorry, I meant Federico, instead of Frederico. Thanks Douglas
There's a few problems with the spec file. - '%define' should be '%global' throughout. - What was the reason for Conflicts: selinux-policy-targeted < ...? Could it not be: Requires: selinux-policy-targeted >= ... instead? - What's wrong with just dropping a new file into /etc/sudoers.d/? Anything in this directory is included, and it avoids the ugly %post script rewrite of sudoers. - Why is /etc/pki/vdsm/keys/libvirt_password not just a regular %config file in the %files section? In general anything that would reduce or eliminate those huge %pre/%post/%preun/%postun scripts would be a welcome change and will improve your life because you'll get fewer bug reports from unexpected user customizations.
I am fixing the spec file to build unmodified on SUSE. On SUSE we require that a package either owns its directories or it BuildRequires the packages owning them. vdsm fails here: vdsm-4.9.1-0.4.g1bf3028.i586.rpm: directories not owned by a package: - /etc/pki - /etc/rwtab.d - /etc/sudoers.d - /etc/vdsm - /usr/lib/python2.7/site-packages/sos - /usr/lib/python2.7/site-packages/sos/plugins - /usr/lib/vdsm/hooks - /var/lib/libvirt - /var/lib/libvirt/qemu
(In reply to comment #13) > I am fixing the spec file to build unmodified on SUSE. > > On SUSE we require that a package either owns its directories or it > BuildRequires the packages owning them. vdsm fails here: > > vdsm-4.9.1-0.4.g1bf3028.i586.rpm: directories not owned by a package: > - /etc/pki > - /etc/rwtab.d > - /etc/sudoers.d I'm not sure if I can help you with these, the directories are owned by: filesystem, initscripts and sudo. As far as I remember I read somewhere that it's preferable to not add BuildRequires of packages that are automatically included by mock. Richard, what do you think? > - /etc/vdsm > - /usr/lib/python2.7/site-packages/sos > - /usr/lib/python2.7/site-packages/sos/plugins > - /usr/lib/vdsm/hooks > - /var/lib/libvirt > - /var/lib/libvirt/qemu I've no problem adding these BuildRequires. You can send a patch to vdsm-patches (cc me too).
Spec URL: http://people.redhat.com/fsimonce/vdsm/vdsm.spec SRPM URL: http://people.redhat.com/fsimonce/vdsm/vdsm-4.9.1-0.git31.039976c.fc16.src.rpm
Thanks for correcting all of the problems that I found before. The Source0 file appears to be generated from git. This is fine, but it would be better to have a comment stating how to regenerate this file. I thought this was required by the review guidelines, but I cannot find anything that says that now; therefore this is not a review blocker. Here is the rest of my package review: - rpmlint output vdsm.src: W: invalid-url Source0: vdsm-4.9.1.tar.gz vdsm.x86_64: E: explicit-lib-dependency cyrus-sasl-lib Not quite sure what rpmlint is on about here. The dependency seems OK to me. vdsm.x86_64: W: incoherent-version-in-changelog 4.9.1-0 ['4.9.1-0.git31.039976c.fc16', '4.9.1-0.git31.039976c'] This should be fixed. vdsm.x86_64: E: non-readable /etc/sudoers.d/50_vdsm 0440L vdsm.x86_64: W: non-conffile-in-etc /etc/sudoers.d/50_vdsm Bug in rpmlint probably. vdsm.x86_64: E: non-executable-script /usr/share/vdsm/configNetwork.py 0644L /usr/bin/python Are the permissions on this script correct? vdsm.x86_64: E: non-readable /etc/pki/vdsm/keys/libvirt_password 0600L vdsm.x86_64: E: script-without-shebang /etc/cron.d/vdsm-libvirt-logrotate vdsm.x86_64: E: executable-crontab-file /etc/cron.d/vdsm-libvirt-logrotate vdsm.x86_64: E: non-standard-dir-perm /var/lib/libvirt/qemu/channels 0775L This should probably be 0755 unless you want a group to write to this directory. vdsm.x86_64: E: non-executable-script /usr/share/vdsm/supervdsmServer.py 0644L /usr/bin/python As above. vdsm.x86_64: W: non-ghost-in-var-run /var/run/vdsm vdsm.x86_64: W: non-conffile-in-etc /etc/udev/rules.d/12-vdsm-lvm.rules I believe this is a bug: udev rules files that are installed from RPMs ought to go somewhere under /lib/udev/... vdsm.x86_64: W: non-ghost-in-var-run /var/run/vdsm/pools vdsm.x86_64: W: service-default-enabled /etc/rc.d/init.d/vdsmd vdsm.x86_64: E: incoherent-subsys /etc/rc.d/init.d/vdsmd libvirt-guests Just so you know, all Fedora packages MUST now include systemd configuration. SysV-init is not required and can be removed. vdsm.x86_64: W: service-default-enabled /etc/rc.d/init.d/vdsmd vdsm-hook-vhostmd.noarch: E: non-readable /etc/sudoers.d/50_vdsm_hook_vhostmd 0440L vdsm-hook-vhostmd.noarch: W: non-conffile-in-etc /etc/sudoers.d/50_vdsm_hook_vhostmd vdsm-debug-plugin.noarch: W: no-documentation vdsm-hook-faqemu.noarch: W: spelling-error Summary(en_US) qemu -> emu, q emu vdsm-hook-faqemu.noarch: W: no-manual-page-for-binary vdsm-faqemu vdsm-cli.noarch: E: non-executable-script /usr/share/vdsm/vdsClient.py 0644L /usr/bin/python vdsm-cli.noarch: W: non-conffile-in-etc /etc/bash_completion.d/vdsClient vdsm-bootstrap.noarch: W: spelling-error %description -l en_US bootstap -> bootstrap, boots tap, boots-tap vdsm-bootstrap.noarch: E: non-executable-script /usr/share/vdsm-bootstrap/vds_bootstrap_complete.py 0644L /usr/bin/python vdsm-bootstrap.noarch: E: non-executable-script /usr/share/vdsm-bootstrap/vds_bootstrap.py 0644L /usr/bin/python vdsm-reg.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ovirt_config_setup/rhevm.py 0644L /usr/bin/python vdsm-reg.noarch: W: non-conffile-in-etc /etc/ovirt-commandline.d/vdsm-reg vdsm-reg.noarch: W: service-default-enabled /etc/init.d/vdsm-reg vdsm-reg.noarch: W: no-reload-entry /etc/init.d/vdsm-reg Please run rpmlint yourself and look at these errors and warnings. Some can be ignored, but some look more serious. + package name satisfies the packaging naming guidelines + specfile name matches the package base name ? package should satisfy packaging guidelines Fix the rpmlint problems. + license meets guidelines and is acceptable to Fedora + license matches the actual package license + %doc includes license file + spec file written in American English + spec file is legible + upstream sources match sources in the srpm + package successfully builds on at least one architecture (built on x86-64) n/a ExcludeArch bugs filed + BuildRequires list all build dependencies http://koji.fedoraproject.org/koji/taskinfo?taskID=3518406 n/a %find_lang instead of %{_datadir}/locale/* n/a binary RPM with shared library files must call ldconfig in %post and %postun + does not use Prefix: /usr + package owns all directories it creates + no duplicate files in %files + package must contain code or permissible content n/a large documentation files should go in -doc subpackage + files marked %doc should not affect package n/a header files should be in -devel n/a static libraries should be in -static n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig' n/a libfoo.so must go in -devel n/a -devel must require the fully versioned base n/a packages should not contain libtool .la files n/a packages containing GUI apps must include %{name}.desktop file + packages must not own files or directories owned by other packages + filenames must be valid UTF-8 + use %global instead of %define Optional: n/a if there is no license file, packager should query upstream n/a translations of description and summary for non-English languages, if available + reviewer should build the package in mock http://koji.fedoraproject.org/koji/taskinfo?taskID=3518406 + the package should build into binary RPMs on all supported architectures n/a review should test the package functions as described + scriptlets should be sane n/a pkgconfig files should go in -devel n/a shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin Please take a look at the problems above.
(In reply to comment #16) > The Source0 file appears to be generated from git. This is fine, > but it would be better to have a comment stating how to regenerate > this file. I thought this was required by the review guidelines, > but I cannot find anything that says that now; It's here http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control > therefore this is not a review blocker. sourceurl guidelines do say "may" not "must" but IMHO it should be a review blocker, how would you rebuild and compare source tarball otherwise? > vdsm.x86_64: E: explicit-lib-dependency cyrus-sasl-lib > > Not quite sure what rpmlint is on about here. The dependency seems OK > to me. Seems to be packaging bug that cyrus-sasl-lib contains binaries, maybe better to put explicit: Requires(post): /usr/sbin/sasldblistusers2 Requires(post): /usr/sbin/saslpasswd2 ?
Thanks for tracking that guideline down :-) Agreed with Alan on both points.
Spec URL: http://people.redhat.com/fsimonce/vdsm/vdsm.spec SRPM URL: http://people.redhat.com/fsimonce/vdsm/vdsm-4.9.1-1.git6ee166c.fc16.src.rpm
Source0 hasn't been annotated as required, which is a reject. There are still 8 rpmlint errors and 9 warnings. This is a lot, albeit fewer than before. Please give a summary here of why each rpmlint error and warning is not an actual error in the spec file.
(In reply to comment #20) > Source0 hasn't been annotated as required, which is a reject. Sorry, I didn't notice Alan's comment. Spec URL: http://people.redhat.com/fsimonce/vdsm/vdsm.spec SRPM URL: http://people.redhat.com/fsimonce/vdsm/vdsm-4.9.1-2.git931a43e.fc16.src.rpm > There are still 8 rpmlint errors and 9 warnings. This is a lot, > albeit fewer than before. Please give a summary here of why > each rpmlint error and warning is not an actual error in the > spec file. The error and warning are mostly the rpmlint bugs that you pointed out in comment #16 anyway here's the summary: vdsm.src:290: E: hardcoded-library-path in %{buildroot}/lib/systemd/systemd-vdsmd vdsm.src:293: E: hardcoded-library-path in %{buildroot}/lib/systemd/systemd-vdsm-reg vdsm.src:430: E: hardcoded-library-path in /lib/systemd/systemd-vdsmd vdsm.src:685: E: hardcoded-library-path in /lib/systemd/systemd-vdsm-reg This is a rpmlint bug, it's present also in the systemd package: $ rpmlint systemd-36-3.fc16.src.rpm [...] systemd.src:262: E: hardcoded-library-path in /lib/systemd/systemd-* [...] vdsm.src: W: invalid-url Source0: vdsm-4.9.1-931a43e.tar.gz This was fixed by the vcs comment. vdsm.x86_64: E: non-readable /etc/sudoers.d/50_vdsm 0440L vdsm.x86_64: W: non-conffile-in-etc /etc/sudoers.d/50_vdsm vdsm.x86_64: E: non-readable /etc/pki/vdsm/keys/libvirt_password 0600L This is a rpmlint bug, sudoers shouldn't be readable and it's not configurable. The libvirt password shouldn't be readable. vdsm.x86_64: E: non-standard-dir-perm /var/lib/libvirt/qemu/channels 0775L VDSM needs the group to be able to write here. vdsm.x86_64: W: non-conffile-in-etc /etc/cron.d/vdsm-libvirt-logrotate vdsm-cli.noarch: W: non-conffile-in-etc /etc/bash_completion.d/vdsClient The logrotate and completion file aren't configurable. vdsm-debug-plugin.noarch: W: no-documentation No documentation upstream yet. vdsm-hook-faqemu.noarch: W: spelling-error Summary(en_US) qemu -> emu, q emu False positive spell checking. vdsm-hook-vhostmd.noarch: E: non-readable /etc/sudoers.d/50_vdsm_hook_vhostmd 0440L vdsm-hook-vhostmd.noarch: W: non-conffile-in-etc /etc/sudoers.d/50_vdsm_hook_vhostmd This is correct, see above. vdsm-reg.noarch: W: non-conffile-in-etc /etc/ovirt-commandline.d/vdsm-reg This is not configurable.
This package is APPROVED by rjones.
New Package SCM Request ======================= Package Name: vdsm Short Description: Virtual Desktop Server Manager Owners: fsimonce danken smizrahi Branches: f16 InitialCC: virtmaint
Git done (by process-git-requests). Removed smizrahi, not in Packager group.
Closing per request by Federico Simoncelli.
Package Change Request ====================== Package Name: vdsm New Branches: el6 epel7 Owners: fsimonce danken smizrahi bronhaim dougsland
Git done (by process-git-requests).