[Bug 749132] Review Request: dpm-dsi - Disk Pool Manager (DPM) plugin to GridFTP

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

--- Comment #10 from Ricardo Rocha <rocha.porto at gmail.com> 2011-11-04 11:23:35 EDT ---
(In reply to comment #9)
> Ricardo, 
> If possible can you at least take the  newer file from globus.org since 
> is after globus switched to the apache license.
> 
> Otherwise this package becomes 'ASL2.0 and Globus' where globus
> is now the old defunct Globus license which is today not fedora
> approved. There would be no problem getting it approved I'm sure
> if that's the only option.

I had to also include an additional header - globus_gsi_gss_constants.h - which
was commented in the copied file of dpm-dsi (and not in the original).

I searched for it first and couldn't find a globus package providing. Guess
i'll need to ask for this way to come with globus too.

> Mattias's suggestion to request globus exposes this header makes sense,
> could you request this so it may get fixed one day.

Bug in the fedora tracker or globus?

> New items or things I missed first time:
> 
> (i) Can you parallelize the make?
> make %{?_smp_mflags}

Done.

> (ii) Source code does not match.
> Your instructions say to use 
> tar -czvf dpm-dsi-1.8.2.tar.gz dpm-dsi-1.8.2
> however your .src.rpm contains a misnamed tar file only
> $ file ../SOURCES/dpm-dsi-1.8.2.tar.gz 
> ../SOURCES/dpm-dsi-1.8.2.tar.gz: POSIX tar archive (GNU)
> 
> Moreover when I compare what is checkout vs what is in the tar ball they
> are different.
> $ diff --brief -r dpm-dsi-1.8.2 ../SOURCES/dpm-dsi-1.8.2
> Only in ../SOURCES/dpm-dsi-1.8.2: config.status
> Only in ../SOURCES/dpm-dsi-1.8.2: Makefile
> i.e these files are only in the .src.rpm and not in the checkout.

The contents of the tarball it's my bad, i've fixed it.

Regarding the commands... i didn't get it, how is it misnamed?

> (iii) Redundant files
> %doc LICENSE RELEASE-NOTES
> are not needed in devel since it can't be installed without the main package.

Fixed.

> (iv) dpm-dsi-devel should probably Require 

It Requires dpm-dsi, which Require do you mean?

> (v) Reading the init.d script
> if [ `uname -m` = "x86_64" ]; then
>     LD_LIBRARY_PATH=/opt/glite/lib64:/opt/lcg/lib64:$GLOBUS_LOCATION/lib
> else
>     LD_LIBRARY_PATH=/opt/glite/lib:/opt/lcg/lib:$GLOBUS_LOCATION/lib
> fi
> export LD_LIBRARY_PATH
> 
> The /opt directories have no place on FHS system,
> would better to junk it or at least case it so does not get used.

Should i put a patch for this one in Fedora? It will stay upstream given the
same package is used for the gLite installations.

> 
> Everything else from comment #2 is good.
> 
> Other wise looking good.

I'll wait for your comments on the items above, and will provide a new version
just after.

Thanks.

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