Bug 905255
Summary: | Review Request: open-vm-tools - Open Virtual Machine Tools | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Sankar Tanguturi <stanguturi> | ||||||||
Component: | Package Review | Assignee: | Simone Caronni <negativo17> | ||||||||
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | unspecified | ||||||||||
Version: | rawhide | CC: | akataria, bsarathy, clintdw, dtor, fedora, jsavanyo, mfojtik, mrunge, negativo17, notting, ravindrakumar, rjones, vmware-gos-qa | ||||||||
Target Milestone: | --- | Flags: | negativo17:
fedora-review+
|
||||||||
Target Release: | --- | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
See Also: | https://fedorahosted.org/fesco/ticket/1107 | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2013-05-06 04:24:07 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: | 950748, 1119255 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Sankar Tanguturi
2013-01-29 00:41:28 UTC
Hello, (In reply to comment #0) > Check redhat bug 883614 for more details. We are planning to upstream > 'open-vm-tools'. "Access denied" :( Just an informal review for the moment. I tested your package on Fedora 18. I see your SRPM package is labelled "f16", be careful this version won't be maintained anymore in a few days. 1) The following dependencies are missing from the BuildRequires: - libXtst-devel Without them, I just can't build your package, the configure step fails. Some others are detected as unavailable during configure, although it doesn't block the build: - fuse-devel (probably to enable sharing support) - libSM-devel - libICE-devel I suggest you to build your packages using mock, to be sure not to forget any BR: http://fedoraproject.org/wiki/Projects/Mock 2) I have issues with procps which can't be found, although procps-ng-devel is installed: configure: error: libproc not found. Please configure without procps (using --without-procps) or install procps - http://procps.sourceforge.net It looks like libproc.so is detected by default by configure, whereas Fedora provides libprocps.so. To fix this issue, I had to: - add procps-ng-devel as BuildRequires - override the default procps library name detected using "export CUSTOM_PROCPS_NAME=procps" before %configure 3) I don't thing glib is really required by openvm-tools. I think you mean glib2 (according to configure.ac, only glib2 is needed). As a result, replace glib by glib2 in BuildRequires. 4) gtk2 is probably not the BuildRequires you want to use. You want gtk2-devel instead. gtk2-devel contains the headers and .so files needed to develop with GTK+2, and required by openvm-tools. 5) BuildRoot tag, BuildRoot cleaning and %clean sections are deprecated: http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag http://fedoraproject.org/wiki/Packaging:Guidelines#.25clean Please drop all these things. %defattr is useless too in your package: http://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions Please drop it from the %files section. 6) SysV init scripts are deprecated in favor of systemd unit files: http://fedoraproject.org/wiki/Packaging:Systemd You can't provide anymore such service file, you must convert it to Systemd. 7) Although all the fixes made above, the package build fails with the following error: fileLogger.c: In function 'FileLoggerLog': fileLogger.c:257:4: error: 'g_static_rw_lock_reader_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:210): Use 'g_rw_lock_reader_lock' instead [-Werror=deprecated-declarations] fileLogger.c:268:7: error: 'g_static_rw_lock_reader_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:216): Use 'g_rw_lock_reader_unlock' instead [-Werror=deprecated-declarations] fileLogger.c:269:7: error: 'g_static_rw_lock_writer_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:219): Use 'g_rw_lock_writer_lock' instead [-Werror=deprecated-declarations] fileLogger.c:273:7: error: 'g_static_rw_lock_writer_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:225): Use 'g_rw_lock_writer_unlock' instead [-Werror=deprecated-declarations] fileLogger.c:274:7: error: 'g_static_rw_lock_reader_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:210): Use 'g_rw_lock_reader_lock' instead [-Werror=deprecated-declarations] fileLogger.c:291:13: error: 'g_static_rw_lock_reader_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:216): Use 'g_rw_lock_reader_unlock' instead [-Werror=deprecated-declarations] fileLogger.c:292:13: error: 'g_static_rw_lock_writer_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:219): Use 'g_rw_lock_writer_lock' instead [-Werror=deprecated-declarations] fileLogger.c:298:13: error: 'g_static_rw_lock_writer_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:225): Use 'g_rw_lock_writer_unlock' instead [-Werror=deprecated-declarations] fileLogger.c:299:13: error: 'g_static_rw_lock_reader_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:210): Use 'g_rw_lock_reader_lock' instead [-Werror=deprecated-declarations] fileLogger.c:309:4: error: 'g_static_rw_lock_reader_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:216): Use 'g_rw_lock_reader_unlock' instead [-Werror=deprecated-declarations] fileLogger.c: In function 'FileLoggerDestroy': fileLogger.c:331:4: error: 'g_static_rw_lock_free' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:228): Use 'g_rw_lock_free' instead [-Werror=deprecated-declarations] fileLogger.c: In function 'GlibUtils_CreateFileLogger': fileLogger.c:378:4: error: 'g_static_rw_lock_init' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:207): Use 'g_rw_lock_init' instead [-Werror=deprecated-declarations] Could you check it with your devteam? 8) Why do you disable debug package generation(see %global debug_package %{nil}). This is something unacceptable for the Fedora Project, unless you really have a good reason to do it. Could you explain such a choice? > 3) I don't thing glib is really required by openvm-tools. I think you mean
> glib2 (according to configure.ac, only glib2 is needed). As a result,
> replace glib by glib2 in BuildRequires.
Same issue than for gtk2: glib2-devel would be more appropriate than glib2 as BuildRequires.
Thanks for the review comments. I will address the review comments and update the spec files. I had the same 2 issues as Mohamed 2 and 7, although I had to make a symlink to libproc.so --> libprocps.so to get past 2, since export CUSTOM_PROCPS_NAME=procps didn't work. Both of these are bugs that should be addressed upstream by open-vm-tools, if it is even being maintained anymore; there hasn't been a new release in a while. here's my make traceback: fileLogger.c: In function 'FileLoggerLog': fileLogger.c:257:4: error: 'g_static_rw_lock_reader_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:210): Use 'g_rw_lock_reader_lock' instead [-Werror=deprecated-declarations] fileLogger.c:268:7: error: 'g_static_rw_lock_reader_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:216): Use 'g_rw_lock_reader_unlock' instead [-Werror=deprecated-declarations] fileLogger.c:269:7: error: 'g_static_rw_lock_writer_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:219): Use 'g_rw_lock_writer_lock' instead [-Werror=deprecated-declarations] fileLogger.c:273:7: error: 'g_static_rw_lock_writer_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:225): Use 'g_rw_lock_writer_unlock' instead [-Werror=deprecated-declarations] fileLogger.c:274:7: error: 'g_static_rw_lock_reader_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:210): Use 'g_rw_lock_reader_lock' instead [-Werror=deprecated-declarations] fileLogger.c:291:13: error: 'g_static_rw_lock_reader_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:216): Use 'g_rw_lock_reader_unlock' instead [-Werror=deprecated-declarations] fileLogger.c:292:13: error: 'g_static_rw_lock_writer_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:219): Use 'g_rw_lock_writer_lock' instead [-Werror=deprecated-declarations] fileLogger.c:298:13: error: 'g_static_rw_lock_writer_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:225): Use 'g_rw_lock_writer_unlock' instead [-Werror=deprecated-declarations] fileLogger.c:299:13: error: 'g_static_rw_lock_reader_lock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:210): Use 'g_rw_lock_reader_lock' instead [-Werror=deprecated-declarations] fileLogger.c:309:4: error: 'g_static_rw_lock_reader_unlock' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:216): Use 'g_rw_lock_reader_unlock' instead [-Werror=deprecated-declarations] fileLogger.c: In function 'FileLoggerDestroy': fileLogger.c:331:4: error: 'g_static_rw_lock_free' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:228): Use 'g_rw_lock_free' instead [-Werror=deprecated-declarations] fileLogger.c: In function 'GlibUtils_CreateFileLogger': fileLogger.c:378:4: error: 'g_static_rw_lock_init' is deprecated (declared at /usr/include/glib-2.0/glib/deprecated/gthread.h:207): Use 'g_rw_lock_init' instead [-Werror=deprecated-declarations] cc1: all warnings being treated as errors make[2]: *** [libGlibUtils_la-fileLogger.lo] Error 1 make[2]: Leaving directory `/home/whoami/Downloads/open-vm-tools-9.2.2-893683/lib/glibUtils' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/whoami/Downloads/open-vm-tools-9.2.2-893683/lib' make: *** [all-recursive] Error 1 use: sudo ./configure CFLAGS=-Wno-deprecated-declarations to remove deprecation warnings treated as errors isntalls fine as long as you also link to libprocps.so Of course this is a short term fix. I tried looking into replacing the depracated functions g_static_rw_lock_reader_lock --> g_rw_lock_reader_lock g_static_rw_lock_reader_unlock --> g_rw_lock_reader_unlock g_static_rw_lock_writer_lock --> g_rw_lock_writer_lock g_static_rw_lock_writer_unlock --> g_rw_lock_writer_unlock g_static_rw_lock_free --> g_rw_lock_free g_static_rw_lock_init --> g_rw_lock_init but the function signatures are not the same, so it would take an investment I took up Sankar's original submission and made following changes: - Modified SPEC to follow the conventions and guidelines - Addressed review comments from Mohamed El Morabity - Added systemd script - Verified and built the RPMS for Fedora 18 - Fixed rpmlint warnings - Split the UX components in a separate package for desktops - Split the help files in a separate package for help - Split the guestlib headers in a separate devel package Following are the updated SPEC file and SRPM: Spec URL: https://www.dropbox.com/s/96qpmdrl3fzsw80/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/2ndmgtgodbra59t/open-vm-tools-9.2.2-1.fc18.src.rpm Please take another look and let me know if I need to make any other changes. My Fedora Account System Username: ravindrakumar This is my first package submission to Fedora. Therefore, I need an sponsor, marking FE-NEEDSPONSOR. I'm taking this for review as I'm actually a user of Fedora on VMWare 5.x. I'm not a sponsor and I'm not sure I should request it as my time to work on Fedora it's not always consistent. Despite this, the review can proceed :) I have a few notes on things to fix in the spec file, a message will follow in a while. Thanks Simone. I hope you are ok with doing review under this bug. In case you want me to file a new bug and mark this one as duplicate, let me know. FYI, I would like to push this package in Fedora 19. So, I would really appreciate your review keeping Fedora 19 deadlines. Hello, no problem for the Fedora 19 deadlines, I think we can make it and we can also add the package to Fedora 17/18 and EPEL 5/6. I think it would be very beneficial for all the virtualisation needs. And I'm offering myself as co-mantainer as now currently I use official VMWare packages for all the current RHEL systems (5, 6) and no tools at all for the Fedora systems. In the next post there are "some" things to fix; I'm attaching also a patch to your spec file as this is your first package and the list of changes is quite big. Please understand the patch contents and then we will proceed with the other items remaining (systemd, spec file differences between EPEL / Fedora, etc.). 1) Please use tabs or spaces consistently in the SPEC file; use tabs all over or spaces. For example, all the "Group" lines use spaces and not tabs like the rest of the file. 2) Please reduce the number of variables at the beginning of the SPEC file, for example there's no reason for "toolspkgname" to exist, you could already use "%{name}" where required. 3) Please add a %doc directive to the base package, there are at least the files: %doc AUTHORS ChangeLog COPYING INSTALL NEWS README that can be added to the release. According to the packaging guidelines, you should have at least the license file installed in any combination of the packages/subpackages, so that is required at least in the base package. 4) File ownership... The base package should own the %{_sysconfdir}/vmware-tools/ directory entirely and not only the files therein. The same goes for the directory %{_libdir}/%{name}/; is not currently owned by any package. There's a floating alone "%doc" line at the end of the file before the %changelog section, probably is there by mistake. 5) The Group "Application/Documentation" is not valid, please use "Documentation". 6) There's a check command that can be performed in the build, so please add a section before %post: %check make check 7) During a review, the %changelog should be upgraded and updated as you did for the comment part but also the revision number should change. 8) The install command should preserve timestamps (-p) and the systemd service files need not to be executable. 9) There's currently a subpackage defined as "help" but nobody is creating the specified folder listed in the files section and nobody is installing any document in it, so please: - Add a Build requirement for Doxygen. - Build the docs under the "help" directory, there's a Makefile. According to the configure script, all the documents should be built if doxygen is installed. - If they are very large, you can add them into a doc subpackage as it is now; but only by marking them in the %doc section. - If they are not, add them to the appropriate package as %doc lines. It appears they are quite big (~130kb); so a separate package is fine. 10) Please sort BuildRequires for easier reading. 11) The following directories should be owned by the base package, not only the files contained therein: %{_libdir}/%{name}/ %{_libdir}/%{name}/plugins %{_libdir}/%{name}/plugins/common/ %{_libdir}/%{name}/plugins/vmsvc/ This one should be owned by the desktop subpackage: %{_libdir}/%{name}/plugins/vmusr/ 12) The static archives (*.la) and unversioned shared objects (*.so) should be included in the devel subpackage. I think the static archives could be removed entirely. 13) The directory /sbin/ should be excluded entirely, not just its contents. 14) It's good practice that the description of packages/subpackages stay the closest to 80 columns (no more) after macros have been expanded. Created attachment 733289 [details]
Review fixes for the first part
Please make sure to understand the patch contents and apply as is or integrate with your amendments. I also made a mistake in the patch, the changelog version should be 9.2 not 1.0. Please update the changelog and bump the release. Simone, thanks for the review and providing a fixes patch. I have incorporated your patch as is and made a few corrections in the files section for base and devel packages. Please find the updated files below. SPEC File URL: https://www.dropbox.com/s/bfv10xe5hnxsvor/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/nyga3zs6xilazhl/open-vm-tools-9.2.2-3.fc18.src.rpm And, I can swear I did not mix tabs and spaces :-) I have defined tab space=8 in my vimrc and I think you would have defined it as 4, because your alignment in patch is not working in my VIM until I set tab space to 4. Please let me know if space is preferred over tabs in general. I personally prefer space because of tab weirdness, but I'd like to follow Fedora convention for this submission. I think it would be great if we can get this package in FC 18 as well, please let me know how to make that happen? Looking forward for the next set of comments! (In reply to comment #15) > And, I can swear I did not mix tabs and spaces :-) > > I have defined tab space=8 in my vimrc and I think you would have defined it > as 4, because your alignment in patch is not working in my VIM until I set > tab space to 4. Please let me know if space is preferred over tabs in > general. I personally prefer space because of tab weirdness, but I'd like to > follow Fedora convention for this submission. I have downloaded the file again and it contains tabs again, maybe it's a Dropbox "feature"? :D I prefer spaces as well, and there's no general guidelines for this, only to be consistent on it all over the spec file. > I think it would be great if we can get this package in FC 18 as well, > please let me know how to make that happen? We can make it happen (and I'm very interested in) for both Fedora and EPEL by making the spec file compatible with both distributions / releases. This involves a few things: - Systemd specific BuildRequires/Requires/%post for Fedora 17/18 - Systemd specific BuildRequires/Requires/%post for Fedora 19 and RHEL/CentOS 7+ - SysV init specific BuildRequires/Requires/%post for RHEL/CentOS 5/6 - SysV init script for RHEL/CentOS 5/6 Your spec file already contains a bit of everything, in particular, the systemd policies are for Fedora 17/18, the Group directive is for RHEL/CentOS 5 only. Next set of comments in a few minutes. 1) Service enablement According to the Fedora policies no service should be started upon installation for the exception of those listed in this page: https://fedoraproject.org/wiki/Starting_services_by_default I agree that the guestd daemon should be started upon installation as this is installed only in VMWare guests. After the review I will file a ticket to FESCO as required, to see if we can enable it: https://fedorahosted.org/fesco/ So for now _all_ services should be installed but disabled by default. 2) Systemd conditionals To enable Systemd for Fedora 17 and 18+, we need to apply the packaging policies as defined here: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd As you see, there are differences between those 2 blocks (18+ and 17) so an "%if" between distributions should be applied When f17 will be EOL, the block relevant to it can be removed. 3) RHEL/CentOS 5 building RHEL/CentOS 5 has rpm 4.5 and needs some more settings for packages. A BuildRoot, and a %clean section. To generate template spec files for the various distributions, use the "rpmdev-newspec" command. For example, the following 3 commands: rpmdev-newspec -m -r 4.5 -o 45.spec rpmdev-newspec -m -r 4.8 -o 48.spec rpmdev-newspec -m -o 410.spec 4) SysV init script conditionals SysV init scripts for RHEL/CentOS systems is defined in these policies: https://fedoraproject.org/wiki/Packaging:SysVInitScript Again we need to "%if" the distribution from Fedora. You can create a SysV init script template with the "rpmdev-newinit open-vm-tools-guestd" command from the "rpmdevtools" package. Please create it, modify appropriately for RHEL and add it to the source package as "open-vm-tools-guestd.init"; it's the commented line with "Source2". All the information you need is in the above link. The init script should be enabled by default on runlevels 2-5 when "chkconfig --enable" is run; as the tools can be handy also in single user mode. will generate rpm spec file templates for RHEL 5 (rpm 4.5), RHEL 6 (rpm 4.8) and Fedora 18+. You can tie the version to the generation, for example Fedora 19 uses rpm 4.11 (no changes to the template). 5) open-vm-tools-desktop The file "/etc/xdg/autostart/vmware-user.desktop" contains a note about KDE, can you check that this is still valid? 6) open-vm-tools-guestd.service Please see the following URL: http://fedoraproject.org/wiki/Packaging:Systemd - The Documentation directive is missing - The lines "RestartSec=5" and "TimeoutStopSec=2" are probably not needed, as the process is contained in a cgroup - Daemons should not be forking if possible, they should "stay" in the foreground as systemd takes care of that. Can the "Type=forking" line be removed? - Probably you can make the description like "Service for virtual machines hosted on VMware" and remove the package name from it, but this is not required. Attached is a patch to your spec file with the changes for points 1-4. As before, I have mixed tabs/spaces, so they do not match with yours, please apply / update as you see fit. Created attachment 733357 [details]
Review fixes for the second part
Sorry, I made a mistake (damn touchpad). Just to make it clearer: (In reply to comment #17) > rpmdev-newspec -m -r 4.5 -o 45.spec > rpmdev-newspec -m -r 4.8 -o 48.spec > rpmdev-newspec -m -o 410.spec > will generate rpm spec file templates for RHEL 5 (rpm 4.5), RHEL 6 (rpm 4.8) > and Fedora 18+. You can tie the version to the generation, for example > Fedora 19 uses rpm 4.11 (no changes to the template). The second sentence at the end of point 4 was supposed to go at the end of point 3. > The init script should be enabled by default on runlevels 2-5 when "chkconfig > --enable" is run; as the tools can be handy also in single user mode. Well, runlevel 2 is multi user with no network, not single user mode. Regards, --Simone (In reply to comment #17) > I agree that the guestd daemon should be started upon installation as this > is installed only in VMWare guests. Yes. > After the review I will file a ticket to FESCO as required, to see if we can > enable it: > > https://fedorahosted.org/fesco/ > > So for now _all_ services should be installed but disabled by default. Sure. Could you please CC me on the ticket? > 2) Systemd conditionals > > To enable Systemd for Fedora 17 and 18+, we need to apply the packaging > policies as defined here: > > http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd > > As you see, there are differences between those 2 blocks (18+ and 17) so an > "%if" between distributions should be applied > When f17 will be EOL, the block relevant to it can be removed. > > 3) RHEL/CentOS 5 building > > RHEL/CentOS 5 has rpm 4.5 and needs some more settings for packages. ... > > 4) SysV init script conditionals > > SysV init scripts for RHEL/CentOS systems is defined in these policies: > > https://fedoraproject.org/wiki/Packaging:SysVInitScript > > Again we need to "%if" the distribution from Fedora. Thanks for making all the changes and providing the patch. Given all these complexities and testing effort, I'm inclined to focus on Fedora 18+ and RHEL 7+, because those are almost the same from systemd perspective. That will help me keep the SPEC file simple and testing is easy for me. Hope you would agree with me? > 5) open-vm-tools-desktop > > The file "/etc/xdg/autostart/vmware-user.desktop" contains a note about KDE, > can you check that this is still valid? I looked at the bug https://bugs.kde.org/show_bug.cgi?id=190522 and it seems to have been fixed in a way that it is not reproducible. However, I will not be able to fix/test the note right now, because this file is coming from open-vm-tools source and we will need to fix the source code as sourceforge. I think we will need to raise a ticket for open-vm-tools code. > 6) open-vm-tools-guestd.service > > Please see the following URL: > > http://fedoraproject.org/wiki/Packaging:Systemd > > - The Documentation directive is missing I don't see any man pages generated from open-vm-tools build, so I have just added a link to http://open-vm-tools.sourceforge.net/about.php. > - The lines "RestartSec=5" and "TimeoutStopSec=2" are probably not needed, > as the process is contained in a cgroup Removed RestartSec. TimeoutStopSec default is 90s which is way too long from service stop and guest shutdown perspectives. Please note that tools service does not handle SIGTERM nicely, so systemd ends up timing out and issues SIGKILL ultimately to kill this service. I believe this also needs to be fixed in open-vm-tools source code. > - Daemons should not be forking if possible, they should "stay" in the > foreground as systemd takes care of that. Can the "Type=forking" line be > removed? I had started with "simple" service and had some issues. So, I set it to forking, I will try again with "simple". > - Probably you can make the description like "Service for virtual machines > hosted on VMware" and remove the package name from it, but this is not > required. Changed. I will share the updated the SPEC file and SRPM in a few minutes. (In reply to comment #16) > I have downloaded the file again and it contains tabs again, maybe it's a > Dropbox "feature"? :D > > I prefer spaces as well, and there's no general guidelines for this, only to > be consistent on it all over the spec file. I did not mean I did not use tabs, I meant I did not mix tabs and spaces :-) Please find the updated SPEC file and SRPM below. I have replaced tabs with spaces now. SPEC File URL: https://www.dropbox.com/s/279c1xbqv49ulro/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/zfc8bx0q41bzvn1/open-vm-tools-9.2.2-4.fc18.src.rpm Some comments: - It looks like the help subpackage contains developement documentation. You'd better drop it and include it in the devel subpackage. - Static libraries are not recommanded to be provided: http://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries_2 Unless you have a good reason to provide them, please drop them. You can use the --disable-static option in %configure. Remove also all *.la files. - Be careful not to own directories already owned by other packages: the desktop subpackage provides some desktop files in %{_sysconfdir}/xdg/autostart/, please refer to them in « %files desktop » section as below: %files desktop ... %{_sysconfdir}/xdg/autostart/*.desktop ... Also in devel subpackage: %files devel ... %{_includedir}/vmGuestLib/ - Using %defattr(..) macro, after each %files section, is deprecated. Please drop all of them. - Doc. files are usually installed in %{_docdir}/%{name}-%{version}. You can probably use the --docdir=%{_defaultdocdir}/%{name}-%{version}/ option in %configure, instead of installing API documentation « manually » through %doc. Using %exclude is also discouraged to drop files from packages. You should remove them instead in %install section. - The summary may be more explicit about the roles of the package: something like « Open VMware Tools for virtual machines hosted on VMware » or whatever may be more explicit for the main package. - Last but not least, open-vm-tools are not usable without specific kernel modules, unless I'm wrong. Since Fedora doesn't allow inclusion of external kernel modules, are they plans from VMware to merge them in the vanilla kernel? Is there a schedule or a kernel version target? (In reply to comment #21) > Sure. Could you please CC me on the ticket? Sure. > Thanks for making all the changes and providing the patch. Given all these > complexities and testing effort, I'm inclined to focus on Fedora 18+ and > RHEL 7+, because those are almost the same from systemd perspective. That > will help me keep the SPEC file simple and testing is easy for me. Hope you > would agree with me? I can test and I'm offering myself as co-mantainer for the package, but I would like to have epel 5 and 6 available. Is that ok for you? I'm writing the SysV init script for RHEL now. > I looked at the bug https://bugs.kde.org/show_bug.cgi?id=190522 and it seems > to have been fixed in a way that it is not reproducible. However, I will not > be able to fix/test the note right now, because this file is coming from > open-vm-tools source and we will need to fix the source code as sourceforge. > I think we will need to raise a ticket for open-vm-tools code. In the mean time you can provide a patch for it. I would rather not apply a fix for a problem we don't know it does exist. People can always file a bug later if they have some problems running kde with the guest agent starting. From the bug report, the issue does not seem to happen since KDE 4.8. > Removed RestartSec. TimeoutStopSec default is 90s which is way too long from > service stop and guest shutdown perspectives. Please note that tools service > does not handle SIGTERM nicely, so systemd ends up timing out and issues > SIGKILL ultimately to kill this service. I believe this also needs to be > fixed in open-vm-tools source code. > I had started with "simple" service and had some issues. So, I set it to > forking, I will try again with "simple". Again, you can still patch the code and push it upstream. (In reply to comment #23) > - It looks like the help subpackage contains developement documentation. > You'd better drop it and include it in the devel subpackage. Agreed, this is api documentation and the help package is fairly small (100 kb). > - Static libraries are not recommanded to be provided: > > Unless you have a good reason to provide them, please drop them. You can use > the --disable-static option in %configure. Remove also all *.la files. Yes, please delete them. > - Be careful not to own directories already owned by other packages: the > desktop subpackage provides some desktop files in > %{_sysconfdir}/xdg/autostart/, please refer to them in « %files desktop » > section as below: > %files desktop > ... > %{_sysconfdir}/xdg/autostart/*.desktop Yes, I did not notice this, please fix it as well. > Also in devel subpackage: > %files devel > ... > %{_includedir}/vmGuestLib/ The one which is now in place works fine as well as at the moment of packaging there are no extra files in /usr/include/ except those from the package, so the folder is included. You can change as suggested here, if you prefer. > - Using %defattr(..) macro, after each %files section, is deprecated. Please > drop all of them. Isn't this needed for EPEL 5? > - Doc. files are usually installed in %{_docdir}/%{name}-%{version}. You can > probably use the --docdir=%{_defaultdocdir}/%{name}-%{version}/ option in > %configure, instead of installing API documentation « manually » through > %doc. If you can, yes, please do. > Using %exclude is also discouraged to drop files from packages. You should > remove them instead in %install section. I prefer this too, as while doing the actual building you can have a folder containing only what's needed in the package. > - The summary may be more explicit about the roles of the package: something > like « Open VMware Tools for virtual machines hosted on VMware » or whatever > may be more explicit for the main package. Yep. > - Last but not least, open-vm-tools are not usable without specific kernel > modules, unless I'm wrong. Since Fedora doesn't allow inclusion of external > kernel modules, are they plans from VMware to merge them in the vanilla > kernel? Is there a schedule or a kernel version target? So far, as far as I know, all the required drivers except for the hgfs driver for filesystem mounting are included in RHEL 6.4+ and Fedora 17+. Do not know for RHEL 5. Any info on this? Thanks Mohamed for your comments. > > - Last but not least, open-vm-tools are not usable without specific kernel
> > modules, unless I'm wrong. Since Fedora doesn't allow inclusion of external
> > kernel modules, are they plans from VMware to merge them in the vanilla
> > kernel? Is there a schedule or a kernel version target?
>
> So far, as far as I know, all the required drivers except for the hgfs
> driver for filesystem mounting are included in RHEL 6.4+ and Fedora 17+. Do
> not know for RHEL 5. Any info on this?
open-vm-tools provides also the following kernel modules:
- vmblock
- vmci
- vmhgfs
- vmsync
- vmxnet
- vsock
Their build is disabled here, for the reasons given above.
Only vsock is already available in vanilla kernel (>2.6.32, so not in RHEL6). vmxnet is not available, whereas vmxnet3 is in the kernel for a long time. Those two drivers don't support anyway the same virtual net devices.
vmblock is required to handle drag-and-drop file copy in WorkStation and Player.
open-vm-tools could integrate "as if" the Fedora repos, but its support would be very limited.
(In reply to comment #25) > open-vm-tools provides also the following kernel modules: > - vmblock > - vmci > - vmhgfs > - vmsync > - vmxnet > - vsock I confirm what Mohamed says, I have only the following drivers "natively" in my Fedora and RHEL systems, as also noted on the RHEL 6.4 Technical notes [1]: - vmxnet3 - vmw_pvscsi - vmware_balloon - vmmouse_drv - vmware_drv > Only vsock is already available in vanilla kernel (>2.6.32, so not in > RHEL6). Yes, it has been first included in 3.9rc1 [2]. So it will be in Fedora 19 first and backported to Fedora 18 3.9.x kernels. It might be that eventually it will make its way into RHEL. > open-vm-tools could integrate "as if" the Fedora repos, but its support > would be very limited. I agree, but I'm also convinced that this should go in the repositories. When new kernel modules will be integrated, the functionality will start working. I'm installing it right now the latest spec file on some Fedora guests in our environment, will let you know what can I do with it. If at least it can fill the guest information in the VMware console like ip address, shutdown, etc. I think it's a big win. Both for Fedora and RHEL. What do you think? Will let you have some feedback in a few minutes. [1] https://access.redhat.com/site/documentation/en-US/Red_Hat_Enterprise_Linux/6/html-single/6.4_Release_Notes/index.html#vmware [2] https://www.kernel.org/pub/linux/kernel/projects/backports/stable/v3.9-rc1/ChangeLog-3.9-rc1-3 (In reply to comment #26) > (In reply to comment #25) > > open-vm-tools provides also the following kernel modules: > > - vmblock > > - vmci > > - vmhgfs > > - vmsync > > - vmxnet > > - vsock > > I confirm what Mohamed says, I have only the following drivers "natively" in > my Fedora and RHEL systems, as also noted on the RHEL 6.4 Technical notes > [1]: > > - vmxnet3 > - vmw_pvscsi > - vmware_balloon > - vmmouse_drv > - vmware_drv I forgot to add vmwgfx in Fedora. I installed the tools in some Fedora 17 and 18 guests, I have IP information reporting, Guest OS (it is named Fedora! Yay!) and I can power off / restart the guest cleanly with the vCenter console. I think it's a great addition even with an incomplete set of drivers. One could always opt to use the source tarball in VMware ESX or the VMware official packages (http://www.vmware.com/download/packages.html) Adding RHEL is not much effort and I'm offering myself as co-mantainer. Here is an updated package along with a patch to your previous file. Included is: - The SysV init script - RHEL conditionals for 5 and 6 - A text file with the description of the drivers needed / used - Renamed the service and Sysv init script from "open-vm-tools-guestd" to "vmtoolsd", the command is really too long and not intuitive - %defattr is still there as I don't know if it's needed for RHEL 5. SPEC File URL: http://slaanesh.fedorapeople.org/open-vm-tools.spec SRPM URL: http://slaanesh.fedorapeople.org/open-vm-tools-9.2.2-5.fc18.src.rpm patch from previous spec file: http://slaanesh.fedorapeople.org/open-vm-tools.spec.patch text file with driver status: http://slaanesh.fedorapeople.org/open-vm-tools-drivers.txt Can you give me some feedback? Thanks, --Simone Thanks for the review Mohamed, and thanks Simone for testing things and providing patch for Mohamed's comments. > - Last but not least, open-vm-tools are not usable without specific kernel > modules, unless I'm wrong. This is not entirely accurate. open-vm-tools provide lots of administrative and customization capabilities of the VM from the VM management interface. The confusion comes from open-vm-tools code having drivers code as well, but actually most of the drivers are required for specific devices and not for the open-vm-tools operation itself. The open-vm-tools user space components that we are packaging in this bug are dependent on only two drivers vmhgfs and vmblock. vmhgfs is needed for "Shared Folder" functionality in Workstation and vmblock driver is needed for "Drag and Drop" feature. For vmblock driver, the alternative is to use Fuse and that is what we are doing in this package, so we are fine with "Drag and Drop". The only missing piece of the puzzle is vmhgfs driver, which will impact the "Shared Folder" functionality of Workstation. And, with no upstreaming planned for this driver, users will have to either use VMware Tools provided by VMware or build and install this driver from open-vm-tools source code. > Since Fedora doesn't allow inclusion of external kernel modules, are they > plans from VMware to merge them in the vanilla kernel? > Is there a schedule or a kernel version target? Most of the kernel modules except vmhgfs are already part of Linux Kernel 3.6 and even older ones like 2.6.32+. > So far, as far as I know, all the required drivers except for the hgfs driver > for filesystem mounting are included in RHEL 6.4+ and Fedora 17+. Do not know > for RHEL 5. Any info on this? RHEL 5 uses Linux Kernel 2.6.18 and it is missing most of the drivers. May be all of them. Most of the upstreaming has been done in 2.6.32 and later kernels. > I can test and I'm offering myself as co-mantainer for the package, but I would like to have epel 5 and 6 available. Is that ok for you? I will integrate your changes with a condition that we will be testing and supporting only Fedora 19+ and RHEL 7+. All older releases will have to be tested and verified by you or someone who is going to use the RPM on those distros. > I'm installing it right now the latest spec file on some Fedora guests in our environment, will let you know what can I do with it. If at least it can fill the guest information in the VMware console like ip address, shutdown, etc. I think it's a big win. Both for Fedora and RHEL. > What do you think? That should work normally for most of the administrative operations. I'm integrating your changes and will publish the files soon with my changes. Simone, I have two questions. 1. Why do we need systemd-sysv for these releases? 47 %if 0%{?fedora} >= 18 || 0%{?rhel} >= 7 48 Requires(post): systemd-sysv 2. I'm going to drop drivers.txt file (explanation in more previous comment). Are you ok with that? Created attachment 733937 [details]
Modified open-vm-tools.init
Patch to Simone's init script.
The patch I have provided for init script is for following reason. I did not find the status check before starting the service, so I have also modified the init script with status checks and missing exist $? at the end. Now it is more like the template provided here: https://fedoraproject.org/wiki/Packaging:SysVInitScript#Initscript_template. I will upload the spec file and srpm in a short while. > - Doc. files are usually installed in %{_docdir}/%{name}-%{version}. You can probably use the --docdir=%{_defaultdocdir}/%{name}-%{version}/ option in %configure, instead of installing API documentation « manually » through %doc.
This is causing inclusion of help files in base package as well as in devel
package. I will have to revert this change to avoid the issue.
(In reply to comment #29) > > I can test and I'm offering myself as co-mantainer for the package, but I would like to have epel 5 and 6 available. Is that ok for you? > > I will integrate your changes with a condition that we will be testing and > supporting only Fedora 19+ and RHEL 7+. All older releases will have to be > tested and verified by you or someone who is going to use the RPM on those > distros. To me is fine, you can add me as co-mantainer or make me directly the owner of the el5, el6, f17, f18 branches. (In reply to comment #30) > 1. Why do we need systemd-sysv for these releases? > > 47 %if 0%{?fedora} >= 18 || 0%{?rhel} >= 7 > 48 Requires(post): systemd-sysv Sorry, it was a leftover from another spec file from which I did copy the text. It will probably be needed at rhel 7 release time to make the upgrade from RHEL 6 (SysV) to the RHEL 7 (systemd) package. I will add it to the RHEL 6 branch when it will be time. > 2. I'm going to drop drivers.txt file (explanation in more previous comment). > Are you ok with that? Well, I would prefer that a text file goes along with the package, I think it would be better to leave that in. I'm pretty sure someone will open a bug saying that the required modules are missing or another discussion will rise. It's clear for you (and now all the people here in CC) but not for people looking for a way to install the tools in Fedora et al. Maybe you can expand it adding the explanation you did on comment #29. That would be good information, along the kernel drivers information. (In reply to comment #32) > The patch I have provided for init script is for following reason. > > I did not find the status check before starting the service, so I have also > modified the init script with status checks and missing exist $? at the end. > Now it is more like the template provided here: > https://fedoraproject.org/wiki/Packaging:SysVInitScript#Initscript_template. Yes, please integrate, probably rpmdev-newspec is not on par with the Packaging Guidelines. (In reply to comment #33) > > - Doc. files are usually installed in %{_docdir}/%{name}-%{version}. You can probably use the --docdir=%{_defaultdocdir}/%{name}-%{version}/ option in %configure, instead of installing API documentation « manually » through %doc. > > This is causing inclusion of help files in base package as well as in devel > package. I will have to revert this change to avoid the issue. Yes, but please add api docs only to the devel subpackage and not in a separate help subpackage. Thanks, --Simone Ravindra, I've created the FESco ticket for the boot enabled service. I've put you in CC and referenced it here. I have sponsored Ravindra Kumar into the packager group. I've discovered a bug in RHEL 6 official packaging, referencing it here. It prevents building by using "procps-devel". I've put it as a blocker, but this is only relevant to RHEL 6. > I have sponsored Ravindra Kumar into the packager group. Thanks Richard. > Sorry, it was a leftover from another spec file from which I did copy the text. > It will probably be needed at rhel 7 release time to make the upgrade from > RHEL 6 (SysV) to the RHEL 7 (systemd) package. > I will add it to the RHEL 6 branch when it will be time. Sure. > Well, I would prefer that a text file goes along with the package, I think it > would be better to leave that in. How about fixing it in the README file from open-vm-tools source? That way it will stay more accurate. > Yes, but please add api docs only to the devel subpackage and not in a separate > help subpackage. Sure. I'm keeping most of your changes. I was not right --docdir is not causing double packaging of help, it seems to be something else. I'm still trying to figure it out. > I've created the FESco ticket for the boot enabled service. Thanks for creating this ticket. > I've discovered a bug in RHEL 6 official packaging, referencing it here. It > prevents building by using "procps-devel". In your bug, you have mentioned that you are able to work around it using yum. Is that so? (In reply to comment #38) > How about fixing it in the README file from open-vm-tools source? That way it > will stay more accurate. Sure, no problem; as long as the information is available in the package I'm fine. Are you patching the README file? > Sure. I'm keeping most of your changes. I was not right --docdir is not > causing double packaging of help, it seems to be something else. I'm > still trying to figure it out. Ok. > In your bug, you have mentioned that you are able to work around it using > yum. > Is that so? Not exactly. If you install on a system both procps and procps-devel through yum, you don't have the error as ldconfig recreates the symlink. Mock and Koji are more picky and refuse to install both packages specifying that the files conflict. Koji is required for building release packages. The only way to build with koji or mock is to change momentarily the BuildRequires to procps. That's not a problem, the RHEL 6 build can follow later. > Sure. I'm keeping most of your changes. I was not right --docdir is not > causing double packaging of help, it seems to be something else. I'm > still trying to figure it out. In order to make progress with this review. I've uploaded the latest SPEC file and SRPM. I will continue to debug the double inclusion of documentation files and fix it once I find it out. SPEC File URL: https://www.dropbox.com/s/68yvjzfbttl4gt9/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/lguwpdfldxpagca/open-vm-tools-9.2.2-6.fc18.src.rpm > Are you patching the README file? Not right away. I would like to get this package in with the current open-vm-tools. Subsequently, I will make patches for open-vm-tools changes. Then we can push updates to Fedora again. I also looked over the spec file and it seems sane to me. There are no obvious problems. My personal preference would be to remove the conditions on different versions of Fedora/RHEL and just have a separate spec file for each branch, which would also allow the spec file to be modernized for Fedora. However this is entirely up to the preferences of the packager/maintainer. docdir handling seems a bit unothodox, but if it works then it's fine. (In reply to comment #41) > docdir handling seems a bit unothodox, but if it works then > it's fine. Me too, I don't really like it, and it doesn't work for the devel package, even if you change the directory where --docdir points to. Seems to work fine if you include the docs in the main package. Please apply the following patch, it simplifies the docs packaging and removes another %exclude from the file lists. --- open-vm-tools.spec.old 2013-04-11 13:24:04.988138217 +0200 +++ open-vm-tools.spec 2013-04-11 13:24:18.796410870 +0200 @@ -99,8 +99,7 @@ %configure \ --without-kernel-modules \ --disable-static \ - --enable-docs \ - --docdir=%{_defaultdocdir}/%{name}-devel-%{version}/ + --enable-docs make %{?_smp_mflags} %install @@ -108,7 +107,8 @@ make install DESTDIR=%{buildroot} # Remove unnecessary .la files -rm %{buildroot}/%{_libdir}/*.la +find %{buildroot} -name '*.la' -delete +rm -fr %{buildroot}%{_defaultdocdir} %if 0%{?fedora} || 0%{?rhel} >= 7 @@ -200,7 +200,6 @@ %dir %{_libdir}/%{name}/plugins %dir %{_libdir}/%{name}/plugins/common %{_libdir}/%{name}/plugins/common/*.so -%exclude %{_libdir}/%{name}/plugins/common/*.la %dir %{_libdir}/%{name}/plugins/vmsvc %{_libdir}/%{name}/plugins/vmsvc/*.so %{_sbindir}/mount.vmhgfs @@ -221,7 +220,7 @@ %files devel %defattr(-,root,root,-) -%doc +%doc docs/api/build/* %{_includedir}/vmGuestLib/ %{_libdir}/pkgconfig/*.pc %{_libdir}/libguestlib.so > My personal preference would be to remove the conditions on > different versions of Fedora/RHEL and just have a separate spec > file for each branch, which would also allow the spec file to > be modernized for Fedora. However this is entirely up to the > preferences of the packager/maintainer. I have the exact opposite feeling regarding this, if I mantain more branches and there's no reason that they should not be kept in sync I prefer to have them merged with master so I can apply/push the same fix. The Fedora 17 blocks will go away with the release of Fedora 19 in a few months. But since I will probably be the one mantaining the other branches (el5, el6, f17, f18) if it's ok for everybody we can review this and at the time of initial committing to the repositories Ravindra can push a clean spec for Fedora 19 and I will push the other one in the other branches. Can I proceed with the formal review once Ravindra applies the aforementioned patch? Erhm, just discovered a font packaged with the docs: /usr/share/doc/open-vm-tools-devel-9.2.2/html/FreeSans.ttf This should be removed as well. Updated patch: --- open-vm-tools.spec.old 2013-04-11 13:24:04.988138217 +0200 +++ open-vm-tools.spec 2013-04-11 13:36:06.518425057 +0200 @@ -99,8 +99,7 @@ %configure \ --without-kernel-modules \ --disable-static \ - --enable-docs \ - --docdir=%{_defaultdocdir}/%{name}-devel-%{version}/ + --enable-docs make %{?_smp_mflags} %install @@ -108,7 +107,9 @@ make install DESTDIR=%{buildroot} # Remove unnecessary .la files -rm %{buildroot}/%{_libdir}/*.la +find %{buildroot} -name '*.la' -delete +find %{buildroot} -name '*.ttf' -delete +rm -fr %{buildroot}%{_defaultdocdir} %if 0%{?fedora} || 0%{?rhel} >= 7 @@ -200,7 +201,6 @@ %dir %{_libdir}/%{name}/plugins %dir %{_libdir}/%{name}/plugins/common %{_libdir}/%{name}/plugins/common/*.so -%exclude %{_libdir}/%{name}/plugins/common/*.la %dir %{_libdir}/%{name}/plugins/vmsvc %{_libdir}/%{name}/plugins/vmsvc/*.so %{_sbindir}/mount.vmhgfs @@ -221,7 +221,7 @@ %files devel %defattr(-,root,root,-) -%doc +%doc docs/api/build/* %{_includedir}/vmGuestLib/ %{_libdir}/pkgconfig/*.pc %{_libdir}/libguestlib.so (In reply to comment #42) > Can I proceed with the formal review once Ravindra applies the > aforementioned patch? Please do. > But since I will probably be the one mantaining the other branches (el5, el6, > f17, f18) if it's ok for everybody we can review this and at the time of > initial committing to the repositories Ravindra can push a clean spec for > Fedora 19 and I will push the other one in the other branches. This sounds like a good plan. > Erhm, just discovered a font packaged with the docs: > /usr/share/doc/open-vm-tools-devel-9.2.2/html/FreeSans.ttf > +find %{buildroot} -name '*.ttf' -delete Your fix does not seem to address it because we are packaging the doc files directly from the docs/api/build and not from buildroot. I think we will have to remove this file from open-vm-tools source itself. Simone, your following patch solved the issue of inclusion of help files in the base package: > +rm -fr %{buildroot}%{_defaultdocdir} Thanks for fixing it for me :-) Here are the patched and updated files: SPEC File URL: https://www.dropbox.com/s/0hkmz9smo13f46k/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/c7cfb6m16pul5gi/open-vm-tools-9.2.2-7.fc18.src.rpm For the font, you need also to delete it from the current release; when you'll be updating the package upstream and thus generate a new package, you can remove the line. --- open-vm-tools.spec.old 2013-04-12 08:46:27.422858942 +0200 +++ open-vm-tools.spec 2013-04-12 08:59:51.521904690 +0200 @@ -85,6 +85,7 @@ %prep %setup -q -n %{name}-%{version}-%{toolsbuild} +rm -f docs/api/build/FreeSans.ttf %build # open-vm-tools source has some warnings generated because Let's wait a moment on the FESCo decision on whether to start it at boot or not in Fedora. If such is the case; here is the patch for Fedora 17: --- open-vm-tools.spec.old 2013-04-12 08:46:27.422858942 +0200 +++ open-vm-tools.spec 2013-04-12 08:47:09.976766695 +0200 @@ -129,7 +129,7 @@ /usr/sbin/ldconfig if [ $1 -eq 1 ] ; then # Initial installation - /bin/systemctl daemon-reload >/dev/null 2>&1 || : + /bin/systemctl enable %{toolsdaemon}.service >/dev/null 2>&1 || : fi %preun For Fedora 18 the systemd macro does not change and vmtoolsd needs to be added to the presets (already asked on FESCo ticket). (In reply to comment #45) > I think we will have to remove this file from open-vm-tools source itself. The main page of the project gives the idea that is totally abandoned, showing releases and VMWare events from 2010. While on the contrary releases have been frequent, with the one currently reviewed here being only 4 months old: http://sourceforge.net/projects/open-vm-tools/files/open-vm-tools/stable-9.2.x/ When creating the new source release for the removal of the ttf file and adding the kernel modules information to the README file, can I ask you to update the website? :D Thanks. FYI: The ttf file is created by doxygen. It's not part of open-vm-tools, but copied from /usr/share/fonts/gnu-free from the existing system (package 'gnu-free-sans-fonts' on Fedora). Furthermore the doxygen.css file uses this font (although it has fallbacks). Oops, you're right, the rm line should be in the %build or %install section. --- open-vm-tools.spec.old 2013-04-12 08:46:27.422858942 +0200 +++ open-vm-tools.spec 2013-04-12 10:29:41.633744622 +0200 @@ -107,6 +107,7 @@ # Remove unnecessary files from packaging find %{buildroot}/%{_libdir} -name '*.la' -delete rm -fr %{buildroot}/%{_defaultdocdir} +rm -f docs/api/build/FreeSans.ttf %if 0%{?fedora} || 0%{?rhel} >= 7 @@ -129,7 +130,7 @@ /usr/sbin/ldconfig if [ $1 -eq 1 ] ; then # Initial installation - /bin/systemctl daemon-reload >/dev/null 2>&1 || : + /bin/systemctl enable %{toolsdaemon}.service >/dev/null 2>&1 || : fi %preun btw Fedora already has that font by default in package gnu-free-sans-fonts. Should we add gnu-free-sans-fonts as BuildRequires? Koji does not give any error when building the current package. If Koji doesn't give an error, then don't worry about it. Off Topic: Ravindra, I don't have any problem on the systems I tested onto; and the code seems in good shape, but is anybody taking seriously the sourceforge pages and website? There are 170 bugs reported for the tools and no one is assigned: http://sourceforge.net/tracker/?group_id=204462&atid=989708 I had noticed that web site has gone bit old, but the main reason for that is there are not many new exciting things there to update. Once we package it in Fedora and RHEL, we will have things to update on the web site. Regarding bugs, all 170 are not open bugs. If you filter by "open" status, you would find much less of them. Richard, if doxygen is copying the font, do we really need to remove the font file? I would prefer to leave it there if doxygen wants it. The font file seems fine. I would not remove it. (In reply to comment #53) > The font file seems fine. I would not remove it. Really? Are you sure? I've never seen a package copying a file arbitrarily from the system and including it. I don't think this is allowed. And if you remove the font file there's no harm as well, as the font is already included in the distribution. I tried removing the file and there's no change, the browser uses the system installed one. It's not clear to me. I asked on the packagers list: http://lists.fedoraproject.org/pipermail/packaging/2013-April/009046.html (In reply to comment #54) > (In reply to comment #53) > > The font file seems fine. I would not remove it. > > Really? Are you sure? I've never seen a package copying a file arbitrarily > from the system and including it. I don't think this is allowed. From: https://fedoraproject.org/wiki/Packaging:FontsPolicy 1. You're not allowed to package fonts in the same (sub)package as other things > Packagers MUST package each font family in a separate (noarch.rpm) > (sub)package 2. You're not allowed to select the font file location yourself > Creating font packages or subpackages in Fedora is done using the > fontpackages-devel package (where font location is determined by a macro precisely to avoid people putting fonts everywhere on the FS) If you apply those mandatory rules in our packaging guidelines you end up with a freesans rpm clone. At which point (that the guidelines do not state explicitely, because it should be obvious) you're better of using the freesans rpm directly In practical terms, those rules means that any non-font (sub)package if Fedora is only allowed to contain references or symlinks to the contents of a font (sub)package containing the font files they use Based on the discussion above, I have removed the font file from the packaging. I could not remove it by inserting rm command in the %install and %build sections. I had to insert the rm command in %check section to achieve this. Here are the updated SPEC file and SRPM: SPEC File URL: https://www.dropbox.com/s/158ss3zzpckaifp/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/9fmzh8jkjoazrer/open-vm-tools-9.2.2-8.fc18.src.rpm You mustn't do this in the %check section. The reason you couldn't remove it in %install is likely because %doc is copied after %install. However you can still remove it by doing (in %install): rm docs/api/build/FreeSans.ttf Sorry, I'll be in a meeting all day, I'll start the official review later today or tomorrow. > However you can still remove it by doing (in %install):
> rm docs/api/build/FreeSans.ttf
Richard, I had this earlier from Simone's patch and it did not work because 'make
install' places it under 'docs/api/build/html/FreeSans.ttf' dir. I corrected the
path and rpm got generated but still with the font file!
Apparently, there is something after 'make install' that is copying the font
file again. I guessed it to be 'make check' and therefore I debugged it and
confirmed it is 'make check' that regenerates this file. I think following
are few possible ways to fix it:
1. Place 'rm docs/api/build/html/FreeSans.ttf' right after 'make check'
2. Copy selective doc files and exclude this font file under %files
3. Fix the open-vm-tools 'make check' target [not sure if this is feasible]
Your analysis must be wrong somewhere. %check runs after the RPM has been built, so it shouldn't matter if %check runs doxygen again. It's quite possible that 'make install' runs doxygen (it would be an upstream bug). However you can just remove the file after 'make install', but not in %check. You have replaced the spec file on dropbox and not in the src.rpm file, please revert it as fedora-review parses this bug entry for downloading files. This spec file does not even build a src.rpm as you used the %build, %check etc. tags in the comments; if you want to use those in the %changelog prepend them with another %; like %%build or %%check. Apart from this, building your last src.rpm file on a Fedora 18 x86_64 and a RHEL 6 x86_64 mock chroot does not include the font file; so it seems it's fine as it is; maybe it's just some confusion :) Building on Fedora 19 x86_64 produces this error and the build fails: DEBUG: /builddir/build/BUILD/open-vm-tools-9.2.2-893683/lib/include/backdoor_types.h:125:164: error: typedef 'AssertOnCompileFailed' locally defined but not used [-Werror=unused-local-typedefs] For me, the src.rpm file you produced is ok as is. > Your analysis must be wrong somewhere. %check runs after > the RPM has been built, so it shouldn't matter if %check runs > doxygen again. I have verified this with 'rpmbuild --nocheck' as well. I have done following analysis with the SPEC file from dropbox (please note that this SPEC file has a step to remove font file in %install section as well as in %check section): 1. I remove the step to remove font file 'rm docs/api/build/html/FreeSans.ttf' from %check section, the font file gets included in the devel rpm. 2. I inserted debug checks to see existence of this file before and after 'make check' and found that file was not present before 'make check', but it was present after 'make check'. 3. Running rpmbuild with '--nocheck' does not include the font file in the package. > It's quite possible that 'make install' runs doxygen (it would > be an upstream bug). However you can just remove the file > after 'make install', but not in %check. I have done it already in the SPEC file present on dropbox as explained above. > You have replaced the spec file on dropbox and not in the src.rpm file, please > revert it as fedora-review parses this bug entry for downloading files. My bad, I corrected it. While copying files between machines, I ended up with wrong SPEC file there. Now, I have uploaded the right SPEC file, which was used to generate the SRPM. > Apart from this, building your last src.rpm file on a Fedora 18 x86_64 and a > RHEL 6 x86_64 mock chroot does not include the font file; so it seems it's > fine as it is; maybe it's just some confusion :) I believe this is because I have 'rm docs/api/build/html/FreeSans.ttf' in %check section. You can see it in the SPEC file inside SRPM. > This spec file does not even build a src.rpm as you used the %build, %check > etc. tags in the comments; if you want to use those in the %changelog > prepend them with another %; like %%build or %%check. Yeah, it is because I uploaded the wrong SPEC file. > Building on Fedora 19 x86_64 produces this error and the build fails: I will try Fedora 19 build today. The %check section is a nice addition but is not mandatory, if this cause more problems than anything we can just get rid of it. Richard what you think? TBH I'm confused about which version I'm supposed to be looking at. Also I don't think dropbox is a very good service -- the MIME type it uses doesn't let me simply look at the file in the browser. Can you [Ravinda] please post the latest spec file. Current version is 8. As I updated in my yesterday's update, these are the URLs: SPEC File URL: https://www.dropbox.com/s/158ss3zzpckaifp/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/9fmzh8jkjoazrer/open-vm-tools-9.2.2-8.fc18.src.rpm FYI, I have fixed the wrong file issue what Simone mentioned in the same link. So, the same links are good to look at. > Also I don't think dropbox is a very good service -- the > MIME type it uses doesn't let me simply look at the file in the > browser. I will see if I can fix it in dropbox. Otherwise, I might need to request for space on fedoraproject. > I will see if I can fix it in dropbox.
I can't find a way to specify mime-type property for the file in dropbox :-(
Please let me know if having storage space on fedoraproject will help in
reviewing spec file.
I built the latest spec file, first removing the 'rm' command in the %check section, and there is no ttf file in the result. So remove the rm in the %check section. Apart from that, the spec file looks fine to me. Sa(In reply to comment #68) > I built the latest spec file, first removing the 'rm' command > in the %check section, and there is no ttf file in the result. > > So remove the rm in the %check section. > > Apart from that, the spec file looks fine to me. Exactly same behaviour here. Starting the review now, FESCo has not yet approved the exception though. Please be patient as I have another meeting :) Thanks Richard and Simone. I will do a totally fresh and new build and verify this. Also, I will wait for your other review comments before I make this change in my SPEC file, so that I can merge all the changes in one single update. I've put spec and src.rpm on fedorapeople.org, as dropbox does not work with Fedora review as it opens the download form and it bombs out. If you are sponsored I think you can have already access to your fedorapeople.org space, btw. %changelog * Wed Apr 17 2013 Simone Caronni <negativo17> - 9.2.2-9 - Removed rm command in %%check section. - Remove blank character at the beginning of each changelog line. SPEC File URL: http://slaanesh.fedorapeople.org/open-vm-tools.spec SRPM URL: http://slaanesh.fedorapeople.org/open-vm-tools-9.2.2-9.fc18.src.rpm Package Review ============== Key: [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]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: Header files in -devel subpackage, if present. [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]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf %{buildroot} present but not required [x]: Sources contain only permissible code or content. [x]: Each %files section contains %defattr if rpm < 4.4 Note: %defattr present but not needed [x]: Development files must be in a -devel package [x]: Package requires other packages for directories it uses. [x]: Package uses nothing in %doc for runtime. [x]: Package is not known to require ExcludeArch. [!]: Fully versioned dependency in subpackages, if present. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in open-vm- tools-desktop , open-vm-tools-devel [x]: Package complies to the Packaging Guidelines [!]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "GPL", "CDDL", "LGPL", "CDDL BSD (3 clause) LGPL", "Unknown or generated", "BSD (3 clause) GPL", "MIT/X11 (BSD like)", "ISC", "CDDL LGPL", "CDDL GPL", "BSD (3 clause)", "GPL (v2 or later)", "GPL (v2)". 13 files have unknown license. Detailed output of licensecheck in /home/slaanesh/Documents/fedora/905255-open-vm-tools/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: Package consistently uses macro is (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. [-]: 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. [x]: Useful -debuginfo package or justification otherwise. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: %config files are marked noreplace or the reason is justified. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [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: [!]: Spec use %global instead of %define. Note: %define majorversion 9.2 %define minorversion 2 %define toolsbuild 893683 %define toolsversion %{majorversion}.%{minorversion} %define toolsdaemon vmtoolsd [x]: Buildroot is not present Note: Buildroot: present but not needed [!]: Avoid bundling fonts in non-fonts packages. Note: Package contains font files [-]: 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. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [!]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Sources can be downloaded from URI in Source: tag [x]: Reviewer should test that the package builds in mock. [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]: The placement of pkgconfig(.pc) files are correct. [x]: SourceX tarball generation or download is documented. [x]: SourceX is a working URL. ===== EXTRA items ===== Generic: [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 1372160 bytes in /usr/share 1116160 open-vm-tools-devel-9.2.2-9.fc18.x86_64.rpm 256000 open-vm- tools-9.2.2-9.fc18.x86_64.rpm [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: open-vm-tools-9.2.2-9.fc18.x86_64.rpm open-vm-tools-desktop-9.2.2-9.fc18.x86_64.rpm open-vm-tools-devel-9.2.2-9.fc18.x86_64.rpm open-vm-tools.x86_64: W: spelling-error %description -l en_US virtualization -> visualization, actualization, vitalization open-vm-tools.x86_64: E: postin-without-ldconfig /usr/lib64/libvmtools.so.0.0.0 open-vm-tools.x86_64: E: postun-without-ldconfig /usr/lib64/libvmtools.so.0.0.0 open-vm-tools.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/open-vm-tools-9.2.2/README open-vm-tools.x86_64: E: postin-without-ldconfig /usr/lib64/libguestlib.so.0.0.0 open-vm-tools.x86_64: E: postun-without-ldconfig /usr/lib64/libguestlib.so.0.0.0 open-vm-tools.x86_64: E: executable-marked-as-config-file /etc/pam.d/vmtoolsd open-vm-tools.x86_64: E: script-without-shebang /etc/pam.d/vmtoolsd open-vm-tools.x86_64: E: postin-without-ldconfig /usr/lib64/libhgfs.so.0.0.0 open-vm-tools.x86_64: E: postun-without-ldconfig /usr/lib64/libhgfs.so.0.0.0 open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-xferlogs open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-checkvm open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-toolbox-cmd open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-hgfsclient open-vm-tools.x86_64: W: no-manual-page-for-binary vmtoolsd open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-rpctool open-vm-tools.x86_64: W: no-manual-page-for-binary mount.vmhgfs open-vm-tools.x86_64: W: install-file-in-docs /usr/share/doc/open-vm-tools-9.2.2/INSTALL open-vm-tools-desktop.x86_64: E: summary-too-long C User experience components for Open VMware Tools for virtual machines hosted on VMware open-vm-tools-desktop.x86_64: W: no-documentation open-vm-tools-desktop.x86_64: W: non-conffile-in-etc /etc/xdg/autostart/vmware-user.desktop open-vm-tools-desktop.x86_64: W: no-manual-page-for-binary vmware-vmblock-fuse open-vm-tools-desktop.x86_64: W: no-manual-page-for-binary vmware-user-suid-wrapper 3 packages and 0 specfiles checked; 9 errors, 14 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint open-vm-tools open-vm-tools-devel open-vm-tools-desktop open-vm-tools.x86_64: W: spelling-error %description -l en_US virtualization -> visualization, actualization, vitalization open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libvmtools.so.0.0.0 /lib64/libcrypt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libvmtools.so.0.0.0 /lib64/libicudata.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libvmtools.so.0.0.0 /lib64/libm.so.6 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/librt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libcrypt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libicui18n.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libicuuc.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libicudata.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libpthread.so.0 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libdl.so.2 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libm.so.6 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libglib-2.0.so.0 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libgthread-2.0.so.0 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/librt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libcrypt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libicui18n.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libicuuc.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libicudata.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libdl.so.2 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libm.so.6 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libglib-2.0.so.0 open-vm-tools.x86_64: E: postin-without-ldconfig /usr/lib64/libvmtools.so.0.0.0 open-vm-tools.x86_64: E: postun-without-ldconfig /usr/lib64/libvmtools.so.0.0.0 open-vm-tools.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/open-vm-tools-9.2.2/README open-vm-tools.x86_64: E: postin-without-ldconfig /usr/lib64/libguestlib.so.0.0.0 open-vm-tools.x86_64: E: postun-without-ldconfig /usr/lib64/libguestlib.so.0.0.0 open-vm-tools.x86_64: E: executable-marked-as-config-file /etc/pam.d/vmtoolsd open-vm-tools.x86_64: E: script-without-shebang /etc/pam.d/vmtoolsd open-vm-tools.x86_64: E: postin-without-ldconfig /usr/lib64/libhgfs.so.0.0.0 open-vm-tools.x86_64: E: postun-without-ldconfig /usr/lib64/libhgfs.so.0.0.0 open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-xferlogs open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-checkvm open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-toolbox-cmd open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-hgfsclient open-vm-tools.x86_64: W: no-manual-page-for-binary vmtoolsd open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-rpctool open-vm-tools.x86_64: W: no-manual-page-for-binary mount.vmhgfs open-vm-tools.x86_64: W: install-file-in-docs /usr/share/doc/open-vm-tools-9.2.2/INSTALL open-vm-tools-desktop.x86_64: E: summary-too-long C User experience components for Open VMware Tools for virtual machines hosted on VMware open-vm-tools-desktop.x86_64: W: no-documentation open-vm-tools-desktop.x86_64: W: non-conffile-in-etc /etc/xdg/autostart/vmware-user.desktop open-vm-tools-desktop.x86_64: W: no-manual-page-for-binary vmware-vmblock-fuse open-vm-tools-desktop.x86_64: W: no-manual-page-for-binary vmware-user-suid-wrapper 3 packages and 0 specfiles checked; 9 errors, 35 warnings. # echo 'rpmlint-done:' Requires -------- open-vm-tools (rpmlib, GLIBC filtered): /bin/sh config(open-vm-tools) libc.so.6()(64bit) libcrypt.so.1()(64bit) libdl.so.2()(64bit) libdnet.so.1()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libglib-2.0.so.0()(64bit) libgmodule-2.0.so.0()(64bit) libgobject-2.0.so.0()(64bit) libgthread-2.0.so.0()(64bit) libguestlib.so.0()(64bit) libhgfs.so.0()(64bit) libicudata.so.49()(64bit) libicui18n.so.49()(64bit) libicuuc.so.49()(64bit) libm.so.6()(64bit) libprocps.so.0()(64bit) libprocps.so.0(LIBPROCPS_0)(64bit) libpthread.so.0()(64bit) librt.so.1()(64bit) libstdc++.so.6()(64bit) libvmtools.so.0()(64bit) rtld(GNU_HASH) systemd open-vm-tools-devel (rpmlib, GLIBC filtered): /usr/bin/pkg-config libguestlib.so.0()(64bit) libhgfs.so.0()(64bit) libvmtools.so.0()(64bit) open-vm-tools open-vm-tools-desktop (rpmlib, GLIBC filtered): libX11.so.6()(64bit) libXext.so.6()(64bit) libXi.so.6()(64bit) libXinerama.so.1()(64bit) libXrandr.so.2()(64bit) libXrender.so.1()(64bit) libXtst.so.6()(64bit) libatk-1.0.so.0()(64bit) libatkmm-1.6.so.1()(64bit) libc.so.6()(64bit) libcairo.so.2()(64bit) libcairomm-1.0.so.1()(64bit) libcrypt.so.1()(64bit) libdl.so.2()(64bit) libfontconfig.so.1()(64bit) libfreetype.so.6()(64bit) libfuse.so.2()(64bit) libfuse.so.2(FUSE_2.6)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgdk-x11-2.0.so.0()(64bit) libgdk_pixbuf-2.0.so.0()(64bit) libgdkmm-2.4.so.1()(64bit) libgio-2.0.so.0()(64bit) libgiomm-2.4.so.1()(64bit) libglib-2.0.so.0()(64bit) libglibmm-2.4.so.1()(64bit) libgobject-2.0.so.0()(64bit) libgthread-2.0.so.0()(64bit) libgtk-x11-2.0.so.0()(64bit) libgtkmm-2.4.so.1()(64bit) libhgfs.so.0()(64bit) libicudata.so.49()(64bit) libicui18n.so.49()(64bit) libicuuc.so.49()(64bit) libm.so.6()(64bit) libpango-1.0.so.0()(64bit) libpangocairo-1.0.so.0()(64bit) libpangoft2-1.0.so.0()(64bit) libpangomm-1.4.so.1()(64bit) libpthread.so.0()(64bit) librt.so.1()(64bit) libsigc-2.0.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libvmtools.so.0()(64bit) open-vm-tools rtld(GNU_HASH) Provides -------- open-vm-tools: config(open-vm-tools) libguestInfo.so()(64bit) libguestlib.so.0()(64bit) libhgfs.so.0()(64bit) libhgfsServer.so()(64bit) libpowerOps.so()(64bit) libtimeSync.so()(64bit) libvix.so()(64bit) libvmbackup.so()(64bit) libvmtools.so.0()(64bit) open-vm-tools open-vm-tools(x86-64) open-vm-tools-devel: open-vm-tools-devel open-vm-tools-devel(x86-64) pkgconfig(vmguestlib) open-vm-tools-desktop: libdesktopEvents.so()(64bit) libdndcp.so()(64bit) libresolutionSet.so()(64bit) open-vm-tools-desktop open-vm-tools-desktop(x86-64) Unversioned so-files -------------------- open-vm-tools: /usr/lib64/open-vm-tools/plugins/common/libhgfsServer.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/common/libvix.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/vmsvc/libguestInfo.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/vmsvc/libpowerOps.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/vmsvc/libtimeSync.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/vmsvc/libvmbackup.so open-vm-tools-desktop: /usr/lib64/open-vm-tools/plugins/vmusr/libdesktopEvents.so open-vm-tools-desktop: /usr/lib64/open-vm-tools/plugins/vmusr/libdndcp.so open-vm-tools-desktop: /usr/lib64/open-vm-tools/plugins/vmusr/libresolutionSet.so Not applicable issues: ====================== Unversioned so-files -------------------- open-vm-tools: /usr/lib64/open-vm-tools/plugins/common/libhgfsServer.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/common/libvix.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/vmsvc/libguestInfo.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/vmsvc/libpowerOps.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/vmsvc/libtimeSync.so open-vm-tools: /usr/lib64/open-vm-tools/plugins/vmsvc/libvmbackup.so open-vm-tools-desktop: /usr/lib64/open-vm-tools/plugins/vmusr/libdesktopEvents.so open-vm-tools-desktop: /usr/lib64/open-vm-tools/plugins/vmusr/libdndcp.so open-vm-tools-desktop: /usr/lib64/open-vm-tools/plugins/vmusr/libresolutionSet.so Ignore this, they are plugins and hidden in a subfolder without ldconfig configuration. - update-desktop-database is invoked when required Note: desktop file(s) in open-vm-tools-desktop See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache - Package installs a %{name}.desktop using desktop-file-install if there is such a file. See: http://fedoraproject.org/wiki/Packaging/Guidelines#desktop Ignore, this is not a real issue, as the desktop file is not an icon for the menu as it's an autostart. - Spec file lacks Packager, Vendor, PreReq tags. Note: Found : Packager: Simone Caronni <negativo17> See: (this test has no URL) Ignore, these should not be added. - Large documentation must go in a -doc subpackage. Note: Documentation size is 1228800 bytes in 119 files. See: http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation This is not applicable here, as the guidelines say the api documentation should go along with the -devel subpackage and it's not large. Rpmlint (installed packages) ---------------------------- # rpmlint open-vm-tools open-vm-tools-devel open-vm-tools-desktop open-vm-tools.x86_64: W: spelling-error %description -l en_US virtualization -> visualization, actualization, vitalization open-vm-tools.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/open-vm-tools-9.2.2/README open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-xferlogs open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-checkvm open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-toolbox-cmd open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-hgfsclient open-vm-tools.x86_64: W: no-manual-page-for-binary vmtoolsd open-vm-tools.x86_64: W: no-manual-page-for-binary vmware-rpctool open-vm-tools.x86_64: W: no-manual-page-for-binary mount.vmhgfs open-vm-tools-desktop.x86_64: W: no-documentation open-vm-tools-desktop.x86_64: W: non-conffile-in-etc /etc/xdg/autostart/vmware-user.desktop open-vm-tools-desktop.x86_64: W: no-manual-page-for-binary vmware-vmblock-fuse open-vm-tools-desktop.x86_64: W: no-manual-page-for-binary vmware-user-suid-wrapper Ignore. Issues: ======= - ldconfig called in %post and %postun if required. Note: /sbin/ldconfig not called in open-vm-tools See: http://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries Please use /sbin/ldconfig as suggested so it works also with Epel. [!]: Spec use %global instead of %define. Note: %define majorversion 9.2 %define minorversion 2 %define toolsbuild 893683 %define toolsversion %{majorversion}.%{minorversion} %define toolsdaemon vmtoolsd Please check this and apply where appropriate: http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define [!]: Avoid bundling fonts in non-fonts packages. Note: Package contains font files This happens to me again on the review! I've checked the Makefile in the docs folder; the check is rebuilding the docs. So let's remove the %check section as it's not mandatory and causes more trouble than anything else. [!]: Package should compile and build into binary rpms on all supported architectures. Does not build on Fedora 19 with the error explained in comment #62. open-vm-tools.x86_64: E: postin-without-ldconfig /usr/lib64/libvmtools.so.0.0.0 open-vm-tools.x86_64: E: postun-without-ldconfig /usr/lib64/libvmtools.so.0.0.0 open-vm-tools.x86_64: E: postin-without-ldconfig /usr/lib64/libguestlib.so.0.0.0 open-vm-tools.x86_64: E: postun-without-ldconfig /usr/lib64/libguestlib.so.0.0.0 open-vm-tools.x86_64: E: postin-without-ldconfig /usr/lib64/libhgfs.so.0.0.0 open-vm-tools.x86_64: E: postun-without-ldconfig /usr/lib64/libhgfs.so.0.0.0 Plase use /sbin/ldconfig as described above. open-vm-tools.x86_64: E: executable-marked-as-config-file /etc/pam.d/vmtoolsd open-vm-tools.x86_64: E: script-without-shebang /etc/pam.d/vmtoolsd This should not be executable. open-vm-tools.x86_64: W: install-file-in-docs /usr/share/doc/open-vm-tools-9.2.2/INSTALL Please remove from %doc line. open-vm-tools-desktop.x86_64: E: summary-too-long C User experience components for Open VMware Tools for virtual machines hosted on VMware Please shorten (21 charachters maximum if I remember correctly). open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libvmtools.so.0.0.0 /lib64/libcrypt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libvmtools.so.0.0.0 /lib64/libicudata.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libvmtools.so.0.0.0 /lib64/libm.so.6 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/librt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libcrypt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libicui18n.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libicuuc.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libicudata.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libpthread.so.0 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libdl.so.2 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libm.so.6 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libguestlib.so.0.0.0 /lib64/libglib-2.0.so.0 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libgthread-2.0.so.0 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/librt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libcrypt.so.1 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libicui18n.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libicuuc.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libicudata.so.49 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libdl.so.2 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libm.so.6 open-vm-tools.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhgfs.so.0.0.0 /lib64/libglib-2.0.so.0 Please see: https://fedoraproject.org/wiki/Common_Rpmlint_issues#unused-direct-shlib-dependency License issues: =============== I think we need some review here... [!]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "GPL", "CDDL", "LGPL", "CDDL BSD (3 clause) LGPL", "Unknown or generated", "BSD (3 clause) GPL", "MIT/X11 (BSD like)", "ISC", "CDDL LGPL", "CDDL GPL", "BSD (3 clause)", "GPL (v2 or later)", "GPL (v2)". 13 files have unknown license. Detailed output of licensecheck in /home/slaanesh/Documents/fedora/905255-open-vm-tools/licensecheck.txt GPL --- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/modules/linux/vmci/linux/vmciKernelIf.c CDDL ---- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/modules/solaris/vmhgfs/filesystem.h LGPL ---- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/tests/testVmblock/manual-blocker.c CDDL BSD (3 clause) LGPL ------------------------ /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/modules/shared/vmmemctl/backdoor_balloon.h Unknown or generated -------------------- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/services/plugins/dndcp/dndGuest/dndCPTransportGuestRpc.hpp BSD (3 clause) GPL ------------------ /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/modules/freebsd/shared/compat_mount.h MIT/X11 (BSD like) ------------------ /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/services/plugins/resolutionSet/vmwarectrlproto.h ISC --- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/lib/misc/base64.c CDDL LGPL --------- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/lib/rpcOut/rpcout.c CDDL GPL -------- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/modules/shared/vmxnet/vmnet_def.h BSD (3 clause) -------------- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/modules/freebsd/vmblock/vfsops.c GPL (v2 or later) ----------------- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/config/ltmain.sh GPL (v2) -------- /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2-893683/modules/linux/dkms.sh Thanks Simone for the detailed review. I'm working on these. I'm wondering if I should remove old releases related bits from spec file before I submit it again for next round of review? Or, should I do that right before final submission? Well, if it's going as in comment #42 that I will be the mantainer of the other branches, feel free to remove all the tags, I will readd them later. Btw, FESCo has approved the exception for starting, I've asked to update the preset file for Fedora 18+. I will add the correct systemd line to the Fedora 17 spec file, Simone, I have addressed all of your comments except license issue. Here are the updated files. SPEC File URL: https://www.dropbox.com/s/9f689yzcrnqf1vx/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/leo2d05e9ln2cvd/open-vm-tools-9.2.2-10.fc18.src.rpm I'm not able to test this using koji because I'm having trouble accessing koji through a proxy. Please let me know how to resolve the license issue. > Well, if it's going as in comment #42 that I will be the mantainer of the other > branches, feel free to remove all the tags, I will readd them later. Koji scratch build also has some issues with %fedora usage, https://fedoraproject.org/wiki/Using_the_Koji_build_system#Scratch_Builds May be this is one more reason to split these files. But, I'm fine to keep the spec file as it is till review is going on. > Btw, FESCo has approved the exception for starting, I've asked to update the > preset file for Fedora 18+. Thanks for taking care of this. (In reply to comment #78) > Koji scratch build also has some issues with %fedora usage, > https://fedoraproject.org/wiki/Using_the_Koji_build_system#Scratch_Builds > May be this is one more reason to split these files. But, I'm fine to keep > the spec file as it is till review is going on. Yes, I usually build locally with mock as it's faster, and then submit the package. If the package fails to build you don't need necessarily to bump the release for a rebuild as if the package was built correctly, so it's pretty safe. All the points have been fixed, so for me the package is good to go; Good job :) We have only to solve the licensing issues. Have you had a change to read this? https://fedoraproject.org/wiki/Licensing:FAQ It lists common cases with multiple licensing scenarios. I think the best thing you could do is make another upstream release with the following: - leverage licenses across the files - fix the make check target - fix the compilation problems - add the driver notes to the README file as your comment #38 Bump the release and it will get approved. Otherwise since all the licenses seem compatible with each other please find a way to write it down in the License field in the spec file using the above link. (In reply to comment #79) > > We have only to solve the licensing issues. Have you had a change to read > this? > > https://fedoraproject.org/wiki/Licensing:FAQ > > It lists common cases with multiple licensing scenarios. I think the best > thing you could do is make another upstream release with the following: > > - leverage licenses across the files Simone, At this time we are not considering changing the licenses on components of open-vm-tools project. The userspace and Linux kernel drivers are GPL, Freebsd kernel drivers are either GPL or BSD, and Solaris kernel drivers are CDDL. As this particular package is for Linux userspace only the overall license on the package should be GPL (v2.1 only). Thanks, Dmitry The source code release contains all those licenses though. Ignoring the kernel module files; this is the list of contained licenses: > LGPL > /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2- > 893683/tests/testVmblock/manual-blocker.c > MIT/X11 (BSD like) > /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2- > 893683/services/plugins/resolutionSet/vmwarectrlproto.h > ISC > /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2- > 893683/lib/misc/base64.c > CDDL LGPL > /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2- > 893683/lib/rpcOut/rpcout.c > GPL (v2 or later) > /var/lib/mock/fedora-18-x86_64/root/builddir/build/BUILD/open-vm-tools-9.2.2- > 893683/config/ltmain.sh Well, from what I see here: https://fedoraproject.org/wiki/Licensing:FAQ#What_is_.22effective_license.22_and_do_I_need_to_know_that_for_the_License:_tag.3F and here: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios it seems you can only leave GPLv2 as the License. Quoting the Packaging guidelines: "The source code contains some .c files which are GPLv2+ and some other .c files which are BSD. They're compiled together to form an executable. Since some of the files are licensed as GPL, the resulting executable is also GPL. The License tag should read: License: GPLv2+ Note that you do NOT need to list BSD in the License tag, the License tag reflects the resulting, packaged, items in the binary RPM." So, for my understanding your point seems valid and the License is valid as the BSD/MIT files are compatible with the GPL. I'm not a licensing expert, though. Anyone has anything to say regarding this? Otherwise the package is good to go. Thanks, --Simone Note: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#License:_field that the License: field applies to the *binary*, not to the source. So if there are BSD / Solaris / whatever drivers in the open-vm-tools source package, then the license of those has no bearing on the License: field (provided AIUI that those drivers aren't being compiled into the binary RPM). Perfect, thanks Richard. Package approved! Ravindra, please keep in touch when doing the inital pushing. Thanks to everybody! I'll be off for the weekend and come back on monday (CET). Thanks Simone for reviewing this and working on this with me. Assuming this review is complete from feedback perspective, I'm planning to remove old branch checks and going to request only for f19 and future branches. I believe Simone would be requesting for all other old branches, mainly f17 and f18 etc. Richard, do you have anything for me to look at before I start the SCM process? New Package SCM Request ======================= Package Name: open-vm-tools Short Description: Open VMware Tools for virtual machines hosted on VMware Owners: ravindrakumar Branches: f19 InitialCC: rjones slaanesh (In reply to comment #85) > Richard, do you have anything for me to look at before I start the SCM > process? No. Git done (by process-git-requests). I have removed the steps related to old versions of Fedora and RHEL. Please take a look at the final version of the SPEC file for Fedora 19 I would like to checkin: SPEC File URL: https://www.dropbox.com/s/hzs0lk8l1reyaix/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/ilp61bz2ql64oa6/open-vm-tools-9.2.2-11.fc18.src.rpm Finally, I have green results from koji for the SRPM link I provided above in my last update: http://koji.fedoraproject.org/koji/taskinfo?taskID=5294959 (f19) http://koji.fedoraproject.org/koji/taskinfo?taskID=5294970 (f20) Checked in the package to rawhide and f19 branches. Here are the links for fedpkg builds: http://koji.fedoraproject.org/koji/taskinfo?taskID=5294998 (rawhide) http://koji.fedoraproject.org/koji/taskinfo?taskID=5295014 (f19-candidate) open-vm-tools-9.2.2-11.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.2-11.fc19 Package Change Request ====================== Package Name: open-vm-tools New Branches: el5 el6 f17 f18 Owners: slaanesh rjones ravindrakumar http://autoqa.fedoraproject.org/report/rm4b AutoQA is complaining about a dependency on /usr/sbin/ldconfig, which I don't understand. It looks like it's a false positive to me. (In reply to comment #94) > http://autoqa.fedoraproject.org/report/rm4b > > AutoQA is complaining about a dependency on /usr/sbin/ldconfig, > which I don't understand. It looks like it's a false positive to me. As in comment #74, Ravindra used /usr/sbin/ldconfig instead of the guidelines' /sbin/ldconfig. I will push /sbin/ldconfig for all the other Fedora and RHEL branches. (In reply to comment #89) > I have removed the steps related to old versions of Fedora and RHEL. Please > take a look at the final version of the SPEC file for Fedora 19 I would like > to checkin: > > SPEC File URL: https://www.dropbox.com/s/hzs0lk8l1reyaix/open-vm-tools.spec > SRPM URL: > https://www.dropbox.com/s/ilp61bz2ql64oa6/open-vm-tools-9.2.2-11.fc18.src.rpm Sorry, I've not been able to connect and respond to the comment. I've asked for the commit acl in pkgdb and added the request for the additional branches. In the spec file you can delete all the %defattr, Group and BuildRoot tags. Those are obsolete for Fedora. If you approve the acls I can remove the tags and rebuild. Regards, --Simone Git done (by process-git-requests). I've built now the other branches, I've started from version 1 so upgrades to f19 will work even if I make some adjustment to the older branches. I did apply the /sbin/ldconfig change and ExclusiveArch: %{ix86} x86_64 so it doesn't get built on ppc/ppc64 (RHEL/CentOS). Unsetting flag. open-vm-tools-9.2.2-1.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.2-1.fc18 open-vm-tools-9.2.2-1.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.2-1.fc17 FYI: a new stable open-vm-tools (open-vm-tools-9.2.3-1031360) has been released today 2013-04-24: http://sourceforge.net/projects/open-vm-tools/files/open-vm-tools/stable-9.2.x/open-vm-tools-9.2.3-1031360.tar.gz A pseudo list of fixes is on Dmitry Torokhov sf.net page: https://sourceforge.net/users/dtor/ open-vm-tools-9.2.2-11.fc19 has been pushed to the Fedora 19 testing repository. (In reply to comment #101) > FYI: a new stable open-vm-tools (open-vm-tools-9.2.3-1031360) has been > released today 2013-04-24: > > http://sourceforge.net/projects/open-vm-tools/files/open-vm-tools/stable-9.2. > x/open-vm-tools-9.2.3-1031360.tar.gz > > A pseudo list of fixes is on Dmitry Torokhov sf.net page: > > https://sourceforge.net/users/dtor/ Thanks for the info. I will wait on updating the Fedora 17 and 18 builds until Ravindra has updated the Fedora 19 one or granted me permissions on the branch. In the tarball there's a Changelog and NEWS file with a list of the changes. > AutoQA is complaining about a dependency on /usr/sbin/ldconfig, > which I don't understand. It looks like it's a false positive to me. I too thought so, because ldconfig comes from glibc package. > In the spec file you can delete all the %defattr, Group and BuildRoot tags. > Those are obsolete for Fedora. If you approve the acls I can remove the tags > and rebuild. Simone, I have addressed all these comments except 'Group' tag. It seems to be optional, do you really want me to remove it? Here are the updated files: SPEC File URL: https://www.dropbox.com/s/hxc3uh0hbzt54we/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/h3rwukk85qc3eac/open-vm-tools-9.2.2-12.fc18.src.rpm I will push the new source code after current push to Fedora 19 is successful. Simone, thanks for pushing it to f17 and f18. I have granted you commit permissions on f19 and devel branches. Thanks for the review, I have made the changes you have asked above. open-vm-tools-9.2.2-12.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.2-12.fc19 I have modified the spec file to pick new upstream version. The new version has fixed some things that no longer require few configure options, I have removed those now. Following are the updated files, I'm going to checkin. Please review. SPEC File URL: https://www.dropbox.com/s/c1m1ez3btmj4y6j/open-vm-tools.spec SRPM URL: https://www.dropbox.com/s/2gsya9bbex5xg9q/open-vm-tools-9.2.3-1.fc18.src.rpm open-vm-tools-9.2.3-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-1.fc19 Sorry for the delay, I was quite busy. Spec file looks fine. Still has the Group tag which was considered optional [1] around Fedora 10 if I remember correctly. I think we can stop using this bug for discussing packaging changes; please address me directly! :D Thanks, --Simone [1] https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag open-vm-tools-9.2.3-1.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-1.fc18 open-vm-tools-9.2.3-1.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-1.fc17 open-vm-tools-9.2.3-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-1.el6 open-vm-tools-9.2.3-3.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-3.fc19 open-vm-tools-9.2.3-4.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-4.fc19 open-vm-tools-9.2.3-4.fc19 has been pushed to the Fedora 19 stable repository. open-vm-tools-9.2.3-5.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-5.fc19 open-vm-tools-9.2.3-5.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-5.fc17 open-vm-tools-9.2.3-5.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-5.fc18 open-vm-tools-9.2.3-5.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/open-vm-tools-9.2.3-5.el6 open-vm-tools-9.2.3-5.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report. open-vm-tools-9.2.3-5.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report. open-vm-tools-9.2.3-5.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report. *** Bug 294321 has been marked as a duplicate of this bug. *** *** Bug 789965 has been marked as a duplicate of this bug. *** |