Bug 453847 - (gpt) Review Request: grid-packaging-tools - The Grid Packaging Tools (GPT)
Review Request: grid-packaging-tools - The Grid Packaging Tools (GPT)
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Extras Quality Assurance
:
Depends On:
Blocks: globus 453848 453849 453850 453851 453852 453853 453854 453855 453856 453857 453858 453861 453862 453865 467235 467237 467239 478917 478918 478919 478920 478921 478922 478923 478925 478926 478927 478928 478929 478930 478931
  Show dependency treegraph
 
Reported: 2008-07-02 18:07 EDT by Mattias Ellert
Modified: 2009-05-22 12:52 EDT (History)
9 users (show)

See Also:
Fixed In Version: 3.2-16.fc9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-18 14:54:26 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
hdegoede: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Mattias Ellert 2008-07-02 18:07:08 EDT
Spec URL: http://www.grid.tsl.uu.se/repos/globus/fedora/9/info/gpt.spec
SRPM URL: http://www.grid.tsl.uu.se/repos/globus/fedora/9/src/SRPMS/gpt-3.2-8.fc9.src.rpm
Description :
GPT is a collection of packaging tools built around an XML based
packaging data format. This format provides a straight forward way to
define complex dependency and compatibility relationships between
packages. The tools provide a means for developers to easily define the
packaging data and include it as part of their source code distribution.
Binary packages can be automatically generated from this data. The
packages defined by GPT are compatible with other packages and can be
easily converted.

This is a build requirement for the Globus Toolkit
Comment 1 Patrice Dumas 2008-07-04 06:33:46 EDT
I'll have a look at all this globus stuff (as time permits...).
It is a possible root dependency which has been submitted
recently.
Comment 2 Patrice Dumas 2008-07-04 09:45:16 EDT
It seems strange to me to need gpt for globus in fedora, on pure
rpm system it should not be useful. That being said, it doesn't mean
that gpt cannot be in fedora.

I think that the name is too short, it would be better to have something
longer.

You have a fair amount of patches. Is upstream aware of these?

Why do you change _ in - in script file names?

Reading the /usr/share/gpt/lib/perl/Grid/GPT/LocalEnv.pm
file it seems that gtar/gzip is used, so a Requires should be needed,
in stead of the perl(Archive::Tar) Requires. Also rpm and rpmbuild
seems to be used so maybe some Requires are missing.

Why aren't the dtds in some dtd directory?

There are some automake duplicated files, this deserves a comment.
Also gpt-bootstrap.sh seems to require all the autotools and it is not
very clear where it is documented.

the globus_core-src.tar.gz file seems also dubious to me.

Do you really need to rerun the autotools? It doesn't seems clear
to me based on the patches.

Instead of populating %{docdir} yourseld, I wuold suggest using an 
in-source directory and %doc. This allows to better keep timestamps 
without much work. Like:

rm -rf __dist_docs
mkdir __dist_docs
mv $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/dtd __dist_docs/
mv $RPM_BUILD_ROOT%{_datadir}/%{name}/gpt_rpm.spec __dist_docs/
iconv -f iso-8859-1 -t utf-8 LICENSE > LICENSE.utf8
touch -c -r LICENSE LICENSE.utf8
mv LICENSE.utf8 LICENSE

and in %files:
%doc __dist_docs/* CHANGES INSTALL README

For patch, 2 suggestions, use
%patch0 -p1 -b .globus
to be able to use gendiff more easily, and name patch like
Patch0:         %{name}-3.2-globus.patch
to know in which version the patch was introduced.
Comment 3 Mattias Ellert 2008-07-07 04:02:45 EDT
(In reply to comment #2)
Thank you for taking the time to review this.

> It seems strange to me to need gpt for globus in fedora, on pure
> rpm system it should not be useful. That being said, it doesn't mean
> that gpt cannot be in fedora.

You are right, gpt is quite useless for most things. Except for building globus.
The globus build (makefiles and configure files) makes heavy use of gpt macros
and gpt package dependency description files. Building globus without gpt would
be possible, but would require a large amount of work to rewrite the build
instructions.

> I think that the name is too short, it would be better to have something
> longer.

I have made a new package where I use the full name, grid-packaging-tools:

http://www.grid.tsl.uu.se/repos/globus/fedora/9/src/SRPMS/grid-packaging-tools-3.2-9.fc9.src.rpm

http://www.grid.tsl.uu.se/repos/globus/fedora/9/info/grid-packaging-tools.spec

It is still OK to use /usr/share/gpt as the install location, right? Or must I
change that to /usr/share/grid-packaging-tools?

> You have a fair amount of patches. Is upstream aware of these?

The patch with the fixes from globus I consider to be upstream. The patches that
adapt to the HFS file system layout used in Fedora are not really bug fixes but
packaging adaptations, so not really appropriate to submit to upstream. The
remaining patches are mainly one and two liners, that probably should be
committed upstream. NCSA:s bugzilla is quite closed - you need to have an
account to submit, and you need to apply for an account via e-mail - no online
registration. I will submit the relevant patches upstream when I get a reply on
my bugzilla account request (provided it is accepted).

> Why do you change _ in - in script file names?

The upstream code is inconsistent in its naming. It has 20 script names that
have - and 8 that uses _, for no apparent good reason. Making the naming of the
scripts consistent is more userfriendly.

> Reading the /usr/share/gpt/lib/perl/Grid/GPT/LocalEnv.pm
> file it seems that gtar/gzip is used, so a Requires should be needed,
> in stead of the perl(Archive::Tar) Requires. Also rpm and rpmbuild
> seems to be used so maybe some Requires are missing.

I have added Requires for tar and gzip. (I did not do that originally since
those were listed as exceptions that were not needed to be listed as build
requirements. But thinking about it that is not quite the same thing.) The
perl(Archive::Tar) requirement is automatically picked up by rpm, and not
mentioned in the spec file. And I think it should be there.

I did not add any requires for rpm and rpmbuild since their use inside gpt is a
rather exotic use of gpt. Using gpt to produce rpms that way will create bad
rpms, with no proper sources.

> Why aren't the dtds in some dtd directory?

In the new package I have moved them to /usr/share/gpt/dtd which seems
consistent with how other Fedora packages install dtd files.

> There are some automake duplicated files, this deserves a comment.

The files in /usr/share/gpt/amdir are not simple copies of the automake
versions. They are extended gpt versions with additional build instructions in them.

> Also gpt-bootstrap.sh seems to require all the autotools and it is not
> very clear where it is documented.

Yes bootstrapping requires the autotools, but that is normally the case, so I
can't see that it requires any special documentation. Please clarify what you
meant by this comment, since I don't get what you are suggesting.

> the globus_core-src.tar.gz file seems also dubious to me.

Removed in the new package.

> Do you really need to rerun the autotools? It doesn't seems clear
> to me based on the patches.

Patches change configure.in and Makefile.am, so yes.

> Instead of populating %{docdir} yourseld, I wuold suggest using an 
> in-source directory and %doc. This allows to better keep timestamps 
> without much work. Like:
> 
> rm -rf __dist_docs
> mkdir __dist_docs
> mv $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/dtd __dist_docs/
> mv $RPM_BUILD_ROOT%{_datadir}/%{name}/gpt_rpm.spec __dist_docs/
> iconv -f iso-8859-1 -t utf-8 LICENSE > LICENSE.utf8
> touch -c -r LICENSE LICENSE.utf8
> mv LICENSE.utf8 LICENSE

Done. (It was even simpler since I moved the dtds to its own directory).

> and in %files:
> %doc __dist_docs/* CHANGES INSTALL README
> 
> For patch, 2 suggestions, use
> %patch0 -p1 -b .globus
> to be able to use gendiff more easily, and name patch like
> Patch0:         %{name}-3.2-globus.patch
> to know in which version the patch was introduced.

I have added -b options to the patch macros.
Comment 4 Mattias Ellert 2008-07-09 02:18:14 EDT
(In reply to comment #3)
> I will submit the relevant patches upstream when I get a reply on
> my bugzilla account request (provided it is accepted).

The relevant patches have now been submitted to upstream bugzilla:

grid-packaging-tools-globus-package-dtd.patch:
http://bugzilla.ncsa.uiuc.edu/show_bug.cgi?id=377

grid-packaging-tools-bootstrap-shebang.patch:
http://bugzilla.ncsa.uiuc.edu/show_bug.cgi?id=378

grid-packaging-tools-flavored-headers.patch:
http://bugzilla.ncsa.uiuc.edu/show_bug.cgi?id=379

grid-packaging-tools-colocate-bugfix.patch:
http://bugzilla.ncsa.uiuc.edu/show_bug.cgi?id=380

grid-packaging-tools-age-version.patch:
http://bugzilla.ncsa.uiuc.edu/show_bug.cgi?id=381

grid-packaging-tools-xml.patch:
http://bugzilla.ncsa.uiuc.edu/show_bug.cgi?id=382
Comment 5 Patrice Dumas 2008-07-09 03:12:24 EDT
(In reply to comment #3)

> You are right, gpt is quite useless for most things. Except for building globus.
> The globus build (makefiles and configure files) makes heavy use of gpt macros
> and gpt package dependency description files. Building globus without gpt would
> be possible, but would require a large amount of work to rewrite the build
> instructions.

Ok.


>
http://www.grid.tsl.uu.se/repos/globus/fedora/9/src/SRPMS/grid-packaging-tools-3.2-9.fc9.src.rpm

Looks good.

> http://www.grid.tsl.uu.se/repos/globus/fedora/9/info/grid-packaging-tools.spec
> 
> It is still OK to use /usr/share/gpt as the install location, right? Or must I
> change that to /usr/share/grid-packaging-tools?

You'll need to change it only if another package really needs to use that
name.


> The patches that
> adapt to the HFS file system layout used in Fedora are not really bug fixes but
> packaging adaptations, so not really appropriate to submit to upstream.

(It is FHS, not HFS). This patch too should be made acceptable by 
upstream. It uses configure, so the standard paths could be passed to
the application. They should be used unless the environment variable
is set, in which case the environment variable should be used.

> > Why do you change _ in - in script file names?
> 
> The upstream code is inconsistent in its naming. It has 20 script names that
> have - and 8 that uses _, for no apparent good reason. Making the naming of the
> scripts consistent is more userfriendly.

Maybe, but this is something that should be changed upstream and not in 
fedora.

> > Reading the /usr/share/gpt/lib/perl/Grid/GPT/LocalEnv.pm
> > file it seems that gtar/gzip is used, so a Requires should be needed,
> > in stead of the perl(Archive::Tar) Requires. Also rpm and rpmbuild
> > seems to be used so maybe some Requires are missing.
> 
> I have added Requires for tar and gzip. (I did not do that originally since
> those were listed as exceptions that were not needed to be listed as build
> requirements. But thinking about it that is not quite the same thing.) The
> perl(Archive::Tar) requirement is automatically picked up by rpm, and not
> mentioned in the spec file. And I think it should be there.

I don't think that perl(Archive::Tar) is used when gtar/gzip is used, so
it shouldn't be required.
 
> I did not add any requires for rpm and rpmbuild since their use inside gpt is a
> rather exotic use of gpt. Using gpt to produce rpms that way will create bad
> rpms, with no proper sources.

I don't think it is an issue. If it is what gpt user are used to.

> > Why aren't the dtds in some dtd directory?
> 
> In the new package I have moved them to /usr/share/gpt/dtd which seems
> consistent with how other Fedora packages install dtd files.

Ok.

> > There are some automake duplicated files, this deserves a comment.
> 
> The files in /usr/share/gpt/amdir are not simple copies of the automake
> versions. They are extended gpt versions with additional build instructions in
them.

Ok.

> > Also gpt-bootstrap.sh seems to require all the autotools and it is not
> > very clear where it is documented.
> 
> Yes bootstrapping requires the autotools, but that is normally the case, so I
> can't see that it requires any special documentation. Please clarify what you
> meant by this comment, since I don't get what you are suggesting.

If gpt-bootstrap.sh is meant to be used by gpt users, Requires in gpt
are needed for the autotools.

> > the globus_core-src.tar.gz file seems also dubious to me.
> 
> Removed in the new package.
> 
> > Do you really need to rerun the autotools? It doesn't seems clear
> > to me based on the patches.
> 
> Patches change configure.in and Makefile.am, so yes.

Ah, I missed them. Looking at them they seem simple enough that patching
configure and Makefile.in additionally would allow not to rerun the
autotools.

Comment 6 Patrice Dumas 2008-07-12 05:24:35 EDT
In the next iterations, it could be nice to have in the spec file the 
the links to the bugzilla you put in Comment #4 in comment near the 
patch.

I have 2 additional comments regarding the changelog section
* you should remove %{?dist} from the changelog
* I personally prefer when there is a blank line between 2 changelog
  entries, but this is a amatter of preference.


I also like to add a trailing / to directories in %files to mark 
visually that they are directories, once again it is a personal
preference you may ignore. It would lead to this entry in %files:
%{_datadir}/gpt/
Comment 7 Patrice Dumas 2008-07-13 19:58:36 EDT
In fact the renaming of the package cannot be done only in fedora,
the naming guidelines mandate that the package is called gpt in
that case. So upstream should be convinced to change the package name
first. If it cannot be done it is not easy to know what to do,
since gpt doesn't show up that high on a google search. I have 
put some thoughts about that issue on:

https://fedoraproject.org/wiki/PackagingTricks#Use_of_common_namespace

Comment 8 Mattias Ellert 2008-07-16 03:37:36 EDT
Sorry for the delay of this iteration. Globus made a new release of the globus
toolkit, so I had to update my packages accordingly.

After communicating with the upstream GPT, I have been informed that the
maintenance of GPT has been transferred from NCSA to the Globus Alliance, so I
have based the new GPT SRPM on the latest tarfile from globus.org. (This
transfer is not indicated anywhere on the www.gridpackagingtools.org website
though.)

(In reply to comment #7)

> In fact the renaming of the package cannot be done only in fedora,
> the naming guidelines mandate that the package is called gpt in
> that case.

So the name is back to gpt, new version is here:

SRPM:
http://www.grid.tsl.uu.se/repos/globus/fedora/9/src/SRPMS/gpt-3.2-10.fc9.src.rpm
SPEC: http://www.grid.tsl.uu.se/repos/globus/fedora/9/info/gpt.spec

(In reply to comment #5)

> (It is FHS, not HFS). This patch too should be made acceptable by 
> upstream. It uses configure, so the standard paths could be passed to
> the application. They should be used unless the environment variable
> is set, in which case the environment variable should be used.

I have split the large FHS patch to several smaller ones and submitted them
upstream.

> > > Why do you change _ in - in script file names?
> > 
> > The upstream code is inconsistent in its naming. It has 20 script names that
> > have - and 8 that uses _, for no apparent good reason. Making the naming of the
> > scripts consistent is more userfriendly.
> 
> Maybe, but this is something that should be changed upstream and not in 
> fedora.

OK, I have removed this.

> > > Reading the /usr/share/gpt/lib/perl/Grid/GPT/LocalEnv.pm
> > > file it seems that gtar/gzip is used, so a Requires should be needed,
> > > in stead of the perl(Archive::Tar) Requires. Also rpm and rpmbuild
> > > seems to be used so maybe some Requires are missing.
> > 
> > I have added Requires for tar and gzip. (I did not do that originally since
> > those were listed as exceptions that were not needed to be listed as build
> > requirements. But thinking about it that is not quite the same thing.) The
> > perl(Archive::Tar) requirement is automatically picked up by rpm, and not
> > mentioned in the spec file. And I think it should be there.
> 
> I don't think that perl(Archive::Tar) is used when gtar/gzip is used, so
> it shouldn't be required.

The system tar/gzip is only used for packaging/unpackaging. The Archive::Tar
perl module is still used for extracting metadata about tarfiles, like the
number of files in the archive. (Yes, I did try to filter out the requirement
and the globus build then failed).

> > I did not add any requires for rpm and rpmbuild since their use inside gpt is a
> > rather exotic use of gpt. Using gpt to produce rpms that way will create bad
> > rpms, with no proper sources.
> 
> I don't think it is an issue. If it is what gpt user are used to.

I have added requires on rpm and rpm-build.

> > > Also gpt-bootstrap.sh seems to require all the autotools and it is not
> > > very clear where it is documented.
> > 
> > Yes bootstrapping requires the autotools, but that is normally the case, so I
> > can't see that it requires any special documentation. Please clarify what you
> > meant by this comment, since I don't get what you are suggesting.
> 
> If gpt-bootstrap.sh is meant to be used by gpt users, Requires in gpt
> are needed for the autotools.

I have added requires on autotools (automake, autoconf, libtool). (I hope you
won't reject the package now due to rpmlint complaining about the libtool
requirement.)

> > > Do you really need to rerun the autotools? It doesn't seems clear
> > > to me based on the patches.
> > 
> > Patches change configure.in and Makefile.am, so yes.
> 
> Ah, I missed them. Looking at them they seem simple enough that patching
> configure and Makefile.in additionally would allow not to rerun the
> autotools.

OK, The patches grew quite a bit in order to propagate the changes, but it was
not impossible, so I have done it this way in the new package.

(In reply to comment #6)

> In the next iterations, it could be nice to have in the spec file the 
> the links to the bugzilla you put in Comment #4 in comment near the 
> patch.

Due to the change of maintenance to the Globus Alliance I was asked to resubmit
the patches to the globus bugzilla by the current maintainer. I have put links
to these new bugzilla reports in the spec file.

> I have 2 additional comments regarding the changelog section
> * you should remove %{?dist} from the changelog
> * I personally prefer when there is a blank line between 2 changelog
>   entries, but this is a matter of preference.

OK, done in the new package.
Comment 9 Patrice Dumas 2008-08-08 10:55:48 EDT
Sorry for being so long, but I am pretty busy right now. I'll
try to progress during the week-end. Also I don't have internet at
home anymore which is also slowing me.
Comment 10 Javier Palacios 2008-08-13 14:41:49 EDT
The INSTALL file should not be included at %doc section
Comment 11 Patrice Dumas 2008-09-03 07:27:54 EDT
I still haven't progressed a bit, but hopefully I'll be less busy in the next days...
Comment 12 Patrice Dumas 2008-09-04 09:49:02 EDT
The package seems right from a strict packaging point of view,
now regarding the patches, I find them a bit too important to
have them in fedora and not upstream. And they are not perfect
either since some hardcoded paths should be set from ./configure.
But these are certainly already better than without them, and
it would be much better if upstream already acked them as is before 
improving them.

In the general case I would have tend to leave this review open
and wait for upstream to adapt the software to be easier to
package, with your help and patches, but this blocks many
other packagaes, so I am not so sure about what to do.

What's your opinion?
Comment 13 Mattias Ellert 2008-09-22 02:48:35 EDT
I have received the following message during my e-mail correspondence with the maintainer:

"As you comment in one of the reports, we are not particularly active developers of GPT. We basically took control of it only because we depended on it and their funding ended. However, I think we will probably wind up applying some if not all of these patches, so I am glad you sent them to us."

So there is acknowledgement of the patches from the maintainer, but no upstream integration yet.
Comment 14 Charles Bacon 2008-09-22 11:35:35 EDT
Hi, this is the upstream maintainer.

We'll need to verify that the patched source works for us; if it does, we'll incorporate these patches into the mainline.  I believe we'll require the hardcoded paths to be configurable, but (without looking yet) I can't imagine that will be too hard.
Comment 15 Mattias Ellert 2008-10-13 10:17:27 EDT
I have created yet another version

http://www.grid.tsl.uu.se/repos/globus/fedora/9/src/SRPMS/gpt-3.2-11.fc9.src.rpm

based on the feedback from the upstream maintainer. The details for the reasons for the changes are discussed in the upstream bugzilla:

http://bugzilla.globus.org/bugzilla/show_bug.cgi?id=6229
Comment 16 Mattias Ellert 2008-12-09 04:07:20 EST
I have updated the package due to a compatibility issue with older versions of automake.

SRPM:
http://www.grid.tsl.uu.se/repos/globus/fedora/9/src/SRPMS/gpt-3.2-12.fc9.src.rpm

SPEC:
http://www.grid.tsl.uu.se/repos/globus/fedora/9/info/gpt.spec

The new additional patch is reported to upstream bugzilla:
http://bugzilla.globus.org/bugzilla/show_bug.cgi?id=6564
Comment 17 Mattias Ellert 2009-01-05 19:42:23 EST
Due to a naming conflict with an existing package in the Ubuntu distribution the package has been renamed from gpt to grid-packaging-tools. The new package name was discussed in an e-mail exchange with the upstream maintainer. Quoting the upstream maintainer:

"I think grid-packaging-tools is fine if that name is not considered too long.  Grid Packaging Tools is the official name of the software, it just goes by GPT because it is a more convenient acronym."

SRPM: http://www.grid.tsl.uu.se/repos/globus/fedora/10/src/SRPMS/grid-packaging-tools-3.2-13.fc10.src.rpm

SPEC: http://www.grid.tsl.uu.se/repos/globus/info/grid-packaging-tools.spec
Comment 18 Hans de Goede 2009-02-16 04:46:11 EST
As discussed by mail, after I've reviewed 3 of your submission and deemed them approvable (the actual approving will happen when you get sponsored) I'll sponsor you.

Full review done, looks good, I only have a few small nitpicks:

MUST FIX
--------
* You are missing: BuildRequires: perl, note that Requires will not get installed
  during the build, only BuildRequires (and visa versa during install)
* Source code changes / conversions should be done in %prep, so please move these 
  3 lines:
iconv -f iso-8859-1 -t utf-8 LICENSE > LICENSE.utf8
touch -c -r LICENSE LICENSE.utf8
mv LICENSE.utf8 LICENSE
  to %prep

SHOULD FIX
----------
* Please add a comment as to why you are removing these files:
rm $RPM_BUILD_ROOT%{_sbindir}/gpt-perl-version
rm $RPM_BUILD_ROOT%{_datadir}/globus/globus_core-src.tar.gz
rm $RPM_BUILD_ROOT%{_datadir}/globus/gpt_scripts_list

* It is sort of standard to put %doc in %files the line below %defattr, instead 
  of at the end

* These 2 rpmlint messages:
grid-packaging-tools.noarch: E: non-executable-script /usr/share/globus/aclocal/bootstrap.frg 0644
grid-packaging-tools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 115, tab: line 1)

  Simple chmod 755 the bootstrap.frg file, and in your specfile either uses
  tabs everywhere to indent or spaces, do not mix the 2 note that the Fedora
  standard is sort of to use spaces. But you're free to use tabs if you prefer.
Comment 19 Mattias Ellert 2009-02-16 10:28:49 EST
(In reply to comment #18)
> 
> MUST FIX
> --------
> * You are missing: BuildRequires: perl, note that Requires will not get
> installed
>   during the build, only BuildRequires (and visa versa during install)

I know the difference between Requires and BuildRequires, however perl always got dragged in for me in the default build environment by e.g. rpm-build or redhat-rpm-config. But an explicit BuildRequires on perl doesn't hurt - added.

> * Source code changes / conversions should be done in %prep, so please move
> these 
>   3 lines:
> iconv -f iso-8859-1 -t utf-8 LICENSE > LICENSE.utf8
> touch -c -r LICENSE LICENSE.utf8
> mv LICENSE.utf8 LICENSE
>   to %prep

Changed accordingly.

> SHOULD FIX
> ----------
> * Please add a comment as to why you are removing these files:
> rm $RPM_BUILD_ROOT%{_sbindir}/gpt-perl-version
> rm $RPM_BUILD_ROOT%{_datadir}/globus/globus_core-src.tar.gz
> rm $RPM_BUILD_ROOT%{_datadir}/globus/gpt_scripts_list

Done.

> * It is sort of standard to put %doc in %files the line below %defattr, instead 
>   of at the end

Done.

> * These 2 rpmlint messages:
> grid-packaging-tools.noarch: E: non-executable-script
> /usr/share/globus/aclocal/bootstrap.frg 0644
> grid-packaging-tools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 115,
> tab: line 1)

The /usr/share/globus/aclocal/bootstrap.frg file is a script fragment - it is not intended to be executable standalone. It does have a shebang so that when put together with other pieces to create a complete script that script will be executable. Making the fragment executable would silence rpmlint, but it would contradict the intended usage of the file.

The specfile uses tabs everywhere for indentation. However it is not possible to enter 1.375 tabs. For this you have do 1 tab + 3 spaces. Line 115 (now 120) starts with a tab, so I don't see why rpmlint complains about it using spaces for indentation. If there at some place in the file was a tab immediately following a space, or a set of 2 or more consecutive spaces crossing or ending at an even 8 column boundary then the warning would make sense. This is not the case. I consider this warning a "false positive" - at least for my understanding of what mixed-use-of-spaces-and-tabs means.

New version available here:

http://www.grid.tsl.uu.se/repos/globus/info/grid-packaging-tools.spec
http://www.grid.tsl.uu.se/repos/globus/fedora/10/src/SRPMS/grid-packaging-tools-3.2-14.fc10.src.rpm
Comment 20 Hans de Goede 2009-02-23 08:34:16 EST
(In reply to comment #19)
> (In reply to comment #18)
> > 

<snip>

> > * These 2 rpmlint messages:
> > grid-packaging-tools.noarch: E: non-executable-script
> > /usr/share/globus/aclocal/bootstrap.frg 0644
> > grid-packaging-tools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 115,
> > tab: line 1)
> 
> The /usr/share/globus/aclocal/bootstrap.frg file is a script fragment - it is
> not intended to be executable standalone. It does have a shebang so that when
> put together with other pieces to create a complete script that script will be
> executable. Making the fragment executable would silence rpmlint, but it would
> contradict the intended usage of the file.
> 

Ok, I'm fine with ignoring the rpmlint output then

> The specfile uses tabs everywhere for indentation. However it is not possible
> to enter 1.375 tabs. For this you have do 1 tab + 3 spaces. Line 115 (now 120)
> starts with a tab, so I don't see why rpmlint complains about it using spaces
> for indentation. If there at some place in the file was a tab immediately
> following a space, or a set of 2 or more consecutive spaces crossing or ending
> at an even 8 column boundary then the warning would make sense. This is not the
> case. I consider this warning a "false positive" - at least for my
> understanding of what mixed-use-of-spaces-and-tabs means.
> 

I agree, one could even call it a bug, so ignore this one too.

> New version available here:
> 
> http://www.grid.tsl.uu.se/repos/globus/info/grid-packaging-tools.spec
> http://www.grid.tsl.uu.se/repos/globus/fedora/10/src/SRPMS/grid-packaging-tools-3.2-14.fc10.src.rpm

Looks good! Once 2 more packages are in good shape I'll sponsor you and approve all 3. I'm not doing the approving right now, as some people run scripts in the flags that sets, checking all approved packages have been imported in to CVS, etc.
Comment 21 Hans de Goede 2009-03-16 06:10:44 EDT
I'm going to sponsor Mattias now, so I'm approving this.
Comment 22 Mattias Ellert 2009-03-17 06:04:19 EDT
New Package CVS Request
=======================
Package Name: grid-packaging-tools
Short Description: Grid Packaging Tools (GPT)
Owners: ellert
Branches: F-9 F-10 EL-4 EL-5
InitialCC:
Comment 23 Kevin Fenzi 2009-03-17 23:45:01 EDT
cvs done.
Comment 24 Fedora Update System 2009-03-18 07:39:01 EDT
grid-packaging-tools-3.2-16.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/grid-packaging-tools-3.2-16.fc10
Comment 25 Fedora Update System 2009-03-18 07:39:11 EDT
grid-packaging-tools-3.2-16.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/grid-packaging-tools-3.2-16.fc9
Comment 26 Fedora Update System 2009-03-18 14:54:20 EDT
grid-packaging-tools-3.2-16.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 27 Fedora Update System 2009-03-18 15:13:28 EDT
grid-packaging-tools-3.2-16.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

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