[Bug 453847] Review Request: grid-packaging-tools - The Grid Packaging Tools (GPT)

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





--- Comment #19 from Mattias Ellert <mattias.ellert at fysast.uu.se>  2009-02-16 10:28:49 EDT ---
(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

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