[Bug 453851] Review Request: globus-common - Globus Toolkit - Common 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=453851





--- Comment #11 from Mattias Ellert <mattias.ellert at fysast.uu.se>  2009-04-06 17:06:13 EDT ---
(In reply to comment #10)
> (In reply to comment #9)
> 
> > The %post script generates the two files labelled as %ghost in the filelist:
> > %ghost %{_datadir}/globus/setup/globus-sh-tools-vars.sh
> > %ghost %{_datadir}/globus/globus-sh-tools-vars.sh
> 
> And what are these files used for?

These files are sourced by many scripts in this and other globus packages in
order to find the location of common tools like cat, chmod, sed and so on.
Quoting a few lines from the file as an example:

GLOBUS_SH_AWK="/bin/gawk"
GLOBUS_SH_AUTOCONF="/usr/bin/autoconf"
GLOBUS_SH_BASENAME="/bin/basename"
GLOBUS_SH_CAT="/bin/cat"
GLOBUS_SH_CHGRP="/bin/chgrp"
GLOBUS_SH_CHMOD="/bin/chmod"
GLOBUS_SH_CHOWN="/bin/chown"
GLOBUS_SH_CLEAR="/usr/bin/clear"

It doesn't really matter if all tools are found at installation time when the
file is created or not, since the scripts, after having sourced this file, uses
constructions like

${GLOBUS_SH_SED-sed}

so if e.g. GLOBUS_SH_SED is not defined in the file it will use sed from the
path. This means there is no need to have Requires(post) conditions requiring
every tool the %post script tries to find to ensure that all entries are
filled. The important thing is that if the file is not there the scripts will
fail when they try to source the file, so it must be there. I agree that doing
things this way is a bit silly, but it is the way upstream does it.

> > I think a script is an overkill here, since the instructions are quite simple
> > (extract a complete subdirectory) and can be stated in a comment. The reviewer
> 
> I don't think "overkill" is the right description for a 3 line script.
> Everytime there is a globus update, you need to: unpack the original archive,
> pack the two new archives. Now if you write these 3 commands into a script
> file, I'm happy.
> 
> Furthermore, Fedora has guidelines for what to do if one has to repack the
> source archive. They are there for a reason. There has to be a good reason for
> not following them, and IMO calling a 3 line script "overkill" is not a good
> enough reason...

I was obviously not suggesting I should not follow the guidelines. I was trying
to argue that what I had done follows the guidelines. The guidelines says:

"There are several cases where upstream is not providing the source to you in
an upstream tarball. In these cases you must document how to generate the
tarball used in the rpm either through a spec file comment OR a script included
as a separate SourceX" (my capitalisation).

I was arguing that using a separate script file in this simple case is an
overkill and that using a spec file comment is sufficient. The guideline page
quoted above uses a subversion checkout as an example of a simple case where a
specfile comment is used. Extracting a directory from a tarball is an operation
of similar complexity. The example where a separate script is used is much more
elaborate and involves removing specific files from a tarball as well as
modifying other files using sed expressions. Extracting a directory from a
tarball is no way nearly as complicated as that. The operations needed to
extract a directory from a tarball are in my opinion simple enough not to
warrant creating a separate script to explain them, nor the additional burden
of maintaining such a script.

If on the other hand your objection really is that the comment was to terse to
allow a reviewer or anyone else interested to reproduce the source tarball that
is an other matter. I have created an updated specfile where I have made the
comment more explicit. (The new comment is similar to the subversion checkout
example - which I find more relevant in this case.) I hope this will be to your
satisfaction.

http://www.grid.tsl.uu.se/repos/globus/info/new/globus-common.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.