[Bug 453850] Review Request: globus-openssl - Openssl Library (virtual GPT glue package)

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=453850





--- Comment #10 from Mattias Ellert <mattias.ellert at fysast.uu.se>  2009-04-17 09:43:51 EDT ---
(In reply to comment #7)

> ? Where does the version number come from? and why is the release number 0.x?

The version is the same as for the upstream package (containing the copy of the
openssl sources) this package replaces.

The release was 0.x because I wanted to have a clear upgrade path from my third
party repository containing the packages and the official Fedora version once
the packages are accepted. My plan was to change the release to 1 when
importing it to CVS. My new package that I list below now gives the release
number as 1 already, but I didn't import it to my third party repo.

> * The license seems to be just ASL 2.0. Is there any non-trivial differences?
> If not, please use ASL 2.0:
>    http://fedoraproject.org/wiki/Licensing#Good_Licenses

The licence is Apache-2.0 without any changes, and the specfile already states
ASL 2.0 accordingly.

> ! No need to BR: pkgconfig on the main package. It will be picked up by
> openssl-devel.

Fixed.

> ! No need to Requires: openssl on the progs subpackage since it already
> requires the main package which requires openssl.

Fixed.

> ! No need to Requires: zlib-devel and pkgconfig on the devel subpackage since
> openssl-devel will pull them up.

This Requires was put in there to workaround the broken openssl-devel package
in RHL9 which is missing the Requires on zlib-devel. I have now made the
Requires conditional so that it will only be used in this case and not for
releases where openssl-devel is not broken.

> * The new guidelines suggest that %global should be preferred over %define

Fixed.

> ! Could you collect all your "%global"s at one place?

The globals are now first in the file (which seems to be the custom nowadays).
Except for the global that defines %_name which must come after the Name tag,
since it uses %name in its definition and this is not defined before the Name
tag is parsed. This is also the most logical place for it.

> ! Please make the descriptions span 80 columns

Fixed.

> ? Why are you packaging the .filelist files?

This is part of the package GPT metadata. They are needed by GPT.

> ? Packages must not own files or directories already owned by other packages. 
>    /usr/share/globus and /usr/share/globus/packages is already owned by
> globus-core. Shouldn't you just put globus-core as a requirement to this
> package? Is this package useful without globus-core?  

globus-core is a development only package. globus-openssl and
globus-openssl-progs are runtime packages. Runtime packages must not Require
development packages.

(In reply to comment #8)
> Additionally,
> 
> ! I suggest you to use macros such as %{_sbindir} and %{_datadir}, instead of
> the hardcoded /usr/sbin and /usr/share  

Fixed

(In reply to comment #9)
> One more thing:
> 
> Shouldn't this package be called globus-gsi-openssl for consistency with other
> globus-gsi-openssl-* packages? All the source is in the 
>    source-trees/gsi/openssl*
> directories of the main tarball.

The package names are taken from the GPT source package description file
pkgdata/pkg_data_src.gpt.in (by replacing underscores with hyphens to comply
with the naming guidelines). They are not derived from the directory structure
of the installer tarball.

> Or conversely, shouldn't the other packages be called globus-openssl-* ?  

Same answer as above.

New version is available here:

http://www.grid.tsl.uu.se/repos/globus/info/new/globus-openssl-3.0-1.fc10.src.rpm
http://www.grid.tsl.uu.se/repos/globus/info/new/globus-openssl.spec

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.