[Bug 467235] Review Request: globus-callout - Globus Toolkit - Globus Callout Library

[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=467235


Orcan 'oget' Ogetbil <oget.fedora at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+




--- Comment #5 from Orcan 'oget' Ogetbil <oget.fedora at gmail.com>  2009-04-17 14:32:19 EDT ---
(In reply to comment #4)
> (In reply to comment #3)
> > Here are my comments for this package. Please note that the bug that I pointed
> > in bug 453851#c19 needs fixed in order to build this package in koji.
> 
> The file in GPT is needed. The file in globus-common is also needed by a few
> Globus packages, but not by any of the packages I have packaged for Fedora 
> so far - and it looks like it might be possible to work around it. I have 
> prepared an update of globus-common where the file is dropped, I just 
> haven't imported it into CVS yet. If you want to use it in a chain build for > reviewing purposes it is available here:
> 
> http://www.grid.tsl.uu.se/repos/globus/info/new/globus-common-
> 10.2-2.fc10.src.rpm
> 
> (Since this update also backports the fixes based on your comments of this
> package to globus-common I would like your feedback on this package before
> importing it to CVS, since these could possible influence this update as 
> well.)
> 

This looks good to me except the inclusion of the external license file.
Thanks.

> > ? Should the license be ASL 2.0? The source files say they are ASL 2.0 on 
> > their headers.
>
> The licence is Apache-2.0 without any changes, and the specfile already 
> states ASL 2.0 accordingly.

Sorry, I just confused myself :)

> 
> > * Fedora specific compiler flags are not honored in the linking phase. At 
> > the least, "-g -Wall" needs passed.
> 
> They are. The package uses the %configure macro which expands to:
> 

No, I meant during the linking phase: (from build.log)
   libtool-gcc64pthr: link: /usr/bin/gcc -shared  .libs/globus_callout.o
.libs/globus_callout_error.o  -L/usr/lib64 -lglobus_common -ldl -lltdl
-lpthread  -m64 -mtune=generic -m64 -m64   -Wl,-soname
-Wl,libglobus_callout.so.0 -o .libs/libglobus_callout.so.0.0.7

But apparently, there is no general consensus among the packagers about this.
We do need to discuss this in the mailing list. For now, you don't need to
worry about it.

> 
> PS. I have collected the answers to the review comments by you and others in a
> Draft packaging guidelines document for Globus packages. This is available
> here:
> 
> http://fedoraproject.org/wiki/PackagingDrafts/Globus  

Good resource. I will go through it this week and make comments in case.


I think this one is good to go now.

-------------------------------------------------
This package (globus-callout) is APPROVED by oget
-------------------------------------------------

-- 
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.