Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: gpt - The Grid Packaging Toolkit https://bugzilla.redhat.com/show_bug.cgi?id=453847 ------- Additional Comments From mattias.ellert@xxxxxxxxxxxx 2008-07-07 04:02 EST ------- (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. -- 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, or are watching someone who is. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review