Bug 226220 - Merge Review: openssl
Merge Review: openssl
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Gwyn Ciesla
Fedora Package Reviews List
:
Depends On:
Blocks: F9MergeReviewTarget
  Show dependency treegraph
 
Reported: 2007-01-31 15:19 EST by Nobody's working on this, feel free to take it
Modified: 2008-01-25 10:19 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-25 10:19:46 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑review+


Attachments (Terms of Use)
Fixed spec. (37.57 KB, text/plain)
2008-01-24 03:58 EST, Tomas Mraz
no flags Details

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 15:19:12 EST
Fedora Merge Review: openssl

http://cvs.fedora.redhat.com/viewcvs/devel/openssl/
Initial Owner: tmraz@redhat.com
Comment 1 Gwyn Ciesla 2008-01-23 11:41:49 EST
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.
Comment 2 Tomas Mraz 2008-01-24 03:58:14 EST
(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.
Comment 3 Tomas Mraz 2008-01-24 03:58:50 EST
Created attachment 292761 [details]
Fixed spec.
Comment 4 Gwyn Ciesla 2008-01-25 10:19:46 EST
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.

Note You need to log in before you can comment on or make changes to this bug.