Fedora Merge Review: openssl http://cvs.fedora.redhat.com/viewcvs/devel/openssl/ Initial Owner: tmraz
rpmlint on SRPM: openssl.src:115: W: make-check-outside-check-section #patch33 is applied after make test Make check or other automated regression test should be run in %check, as they can be disabled with a rpm macro for short circuiting purposes. Should be fixed. openssl.src:202: E: hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/lib*.so.%{soversion} A library path is hardcoded to one of the following paths: /lib, /usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}. openssl.src:203: E: hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/engines A library path is hardcoded to one of the following paths: /lib, /usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}. openssl.src:206: E: hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/* A library path is hardcoded to one of the following paths: /lib, /usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}. Should be fixed. openssl.src:819: W: macro-in-%changelog _datadir Macros are expanded in %changelog too, which can in unfortunate cases lead to the package not building at all, or other subtle unexpected conditions that affect the build. Even when that doesn't happen, the expansion results in possibly "rewriting history" on subsequent package revisions and generally odd entries eg. in source rpms, which is rarely wanted. Avoid use of macros in %changelog altogether, or use two '%'s to escape them, like '%%foo'. openssl.src: W: mixed-use-of-spaces-and-tabs (spaces: line 269, tab: line 137) The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both. openssl.src: W: patch-not-applied Patch33: openssl-0.9.7f-ca-dir.patch A patch is included in your package but was not applied. Refer to the patches documentation to see what's wrong. These are largely cosmetic, but easy to fix. openssl.src: W: strange-permission make-dummy-cert 0755 A file that you listed to include in your package has strange permissions. Usually, a file should have 0644 permissions. openssl.src: W: strange-permission hobble-openssl 0755 A file that you listed to include in your package has strange permissions. Usually, a file should have 0644 permissions. I assume these are for a good reason? rpmlint on rpms: openssl.i386: E: non-standard-dir-perm /etc/pki/CA/private 0700 A standard directory should have permission set to 0755. If you get this message, it means that you have wrong directory permissions in some dirs included in your package. openssl.i386: E: non-standard-dir-perm /etc/pki/CA 0700 A standard directory should have permission set to 0755. If you get this message, it means that you have wrong directory permissions in some dirs included in your package. Probably for a good reason, but I'd like to be sure. openssl.i386: W: non-conffile-in-etc /etc/pki/tls/certs/Makefile A non-executable file in your package is being installed in /etc, but is not a configuration file. All non-executable files in /etc should be configuration files. Mark the file as %config in the spec file. Not sure about this. openssl.i386: W: file-not-utf8 /usr/share/doc/openssl-0.9.8g/CHANGES The character encoding of this file is not UTF-8. Consider converting it in the specfile for example using iconv(1). Should be fixed. openssl-debuginfo.i386: W: spurious-executable-perm /usr/src/debug/openssl-0.9.8g/crypto/bn/bn_const.c The file is installed with executable permissions, but was identified as one that probably should not be executable. Verify if the executable bits are desired, and remove if not. Should be fixed. There is no URL in the Source: tag. Since I can't find a tarball by that name and no URL to it is provided, I can't md5 it against the tarball in the SRPM. If the tarball included is a modified version of something upstream, please explain in the spec how it is created, or, even better, include a script to convert the upstream tarball to the included one. Other than the above, no blockers.
(In reply to comment #1) > rpmlint on SRPM: > openssl.src:115: W: make-check-outside-check-section #patch33 is applied after > make test > Make check or other automated regression test should be run in %check, as > they can be disabled with a rpm macro for short circuiting purposes. > > Should be fixed. tests are run in the compilation tree not in the installed buildroot before install and the Patch33 must be applied after them and before install. So fixing this would require pretty substantial changes which I don't think are worth it. But feel free to provide a patch for this. > openssl.src:202: E: hardcoded-library-path in > $RPM_BUILD_ROOT/usr/lib/lib*.so.%{soversion} > A library path is hardcoded to one of the following paths: /lib, > /usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}. Note this is the source path of 'mv' command. So this is not a bug. > openssl.src:203: E: hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/engines > A library path is hardcoded to one of the following paths: /lib, > /usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}. > > openssl.src:206: E: hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/* > A library path is hardcoded to one of the following paths: /lib, > /usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}. > > Should be fixed. All of the above are not a bug. We are fixing the results of the upstream make install. > openssl.src:819: W: macro-in-%changelog _datadir > Macros are expanded in %changelog too, which can in unfortunate cases lead > to the package not building at all, or other subtle unexpected conditions that > affect the build. Even when that doesn't happen, the expansion results in > possibly "rewriting history" on subsequent package revisions and generally > odd entries eg. in source rpms, which is rarely wanted. Avoid use of macros > in %changelog altogether, or use two '%'s to escape them, like '%%foo'. Fixed. > openssl.src: W: mixed-use-of-spaces-and-tabs (spaces: line 269, tab: line 137) > The specfile mixes use of spaces and tabs for indentation, which is a > cosmetic annoyance. Use either spaces or tabs for indentation, not both. Fixed. > openssl.src: W: patch-not-applied Patch33: openssl-0.9.7f-ca-dir.patch > A patch is included in your package but was not applied. Refer to the patches > documentation to see what's wrong. > These are largely cosmetic, but easy to fix. Note that it is applied - see the spec for comment. > openssl.src: W: strange-permission make-dummy-cert 0755 > A file that you listed to include in your package has strange > permissions. Usually, a file should have 0644 permissions. > > openssl.src: W: strange-permission hobble-openssl 0755 > A file that you listed to include in your package has strange > permissions. Usually, a file should have 0644 permissions. > > I assume these are for a good reason? They are shell scripts so OK. > rpmlint on rpms: > > openssl.i386: E: non-standard-dir-perm /etc/pki/CA/private 0700 > A standard directory should have permission set to 0755. If you get this > message, it means that you have wrong directory permissions in some dirs > included in your package. > > openssl.i386: E: non-standard-dir-perm /etc/pki/CA 0700 > A standard directory should have permission set to 0755. If you get this > message, it means that you have wrong directory permissions in some dirs > included in your package. > > Probably for a good reason, but I'd like to be sure. Of course for a good reason - private keys and internal CA files are of no bussiness for other users than root. > openssl.i386: W: non-conffile-in-etc /etc/pki/tls/certs/Makefile > A non-executable file in your package is being installed in /etc, but is not > a configuration file. All non-executable files in /etc should be configuration > files. Mark the file as %config in the spec file. > > Not sure about this. Strictly speaking it is not a config file but I'd prefer to leave it there as it is as openssl users expect it to be there. > openssl.i386: W: file-not-utf8 /usr/share/doc/openssl-0.9.8g/CHANGES > The character encoding of this file is not UTF-8. Consider converting it > in the specfile for example using iconv(1). > > Should be fixed. Fixed. > openssl-debuginfo.i386: W: spurious-executable-perm > /usr/src/debug/openssl-0.9.8g/crypto/bn/bn_const.c > The file is installed with executable permissions, but was identified as one > that probably should not be executable. Verify if the executable bits are > desired, and remove if not. > > Should be fixed. Harmless, the permissions are in the source tarball this way. Not worth clogging .spec file with fixes for this. > There is no URL in the Source: tag. Since I can't find a tarball by that name > and no URL to it is provided, I can't md5 it against the tarball in the SRPM. > If the tarball included is a modified version of something upstream, please > explain in the spec how it is created, or, even better, include a script to > convert the upstream tarball to the included one. The script is already there - hobble-openssl. I've added a comment to the spec.
Created attachment 292761 [details] Fixed spec.
Valid explanations, all, which I expected. :) I'd *really* rather have bn_const.c not executable, but I can't justify blocking over it. Looks fine. Commit to CVS. APPROVED.