Bug 449135 (GForge) - Review Request: gforge - GForge Collaborative Development Environment
Summary: Review Request: gforge - GForge Collaborative Development Environment
Keywords:
Status: CLOSED NOTABUG
Alias: GForge
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-05-30 18:12 UTC by Javier Palacios
Modified: 2009-05-22 17:03 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-04-14 16:02:53 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Javier Palacios 2008-05-30 18:12:03 UTC
Spec URL: http://www.freewebs.com/javiplx/Fedora/gforge.spec
SRPM URL: http://www.freewebs.com/javiplx/Fedora/gforge-4.5-4.fc9.src.rpm
Description:
GForge is a web-based Collaborative Development Environment offering
easy access to CVS, mailing lists, bug tracking, message
boards/forums, task management, permanent file archival, and total
web-based administration.

Comment 1 Andrea Musuruane 2008-06-20 15:28:26 UTC
The spec file doesn't seem to follow many Fedora packaging guidelines. Please
read them again at:
http://fedoraproject.org/wiki/Packaging/Guidelines

BTW the latest stable GForge version is 4.5.19 released on 10/11/2007. You
should definitely package this version. 

You should also split it in different sub-packages, like Debian and Mandriva do.

You may also want to check other distribution's packages for patches and ideas.

Debian:
http://packages.debian.org/etch/gforge

Mandriva:
http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/gforge/current/



Comment 2 Javier Palacios 2008-06-21 14:00:59 UTC
Event after rereading the packaging guidelines, I don't see the spec missing
many of them, but maybe only some. Could you be more specific?

Packaging old version is somewhat intentional, partly because that is the
version I initially packaged at work, and partly to be able to test a proper
package upgrade.

About subpackages, I'm not sure about which ones might be useful, and I don't
see the package mature enough for them.

Comment 3 Andrea Musuruane 2008-06-21 14:15:49 UTC
(In reply to comment #2)
> Event after rereading the packaging guidelines, I don't see the spec missing
> many of them, but maybe only some. Could you be more specific?

If you don't follow each guideline your package cannot be approved. Therefore
start correcting the one you noticed.

Then run rpmlint and correct the errors you'll get.

After that scrap all the ugly non-fedora bits like: 
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT

I suggest you to read other spec files that are in Fedora to improve your knowledge.

> Packaging old version is somewhat intentional, partly because that is the
> version I initially packaged at work, and partly to be able to test a proper
> package upgrade.

It is not a good choice to present a package review for an outdated version.
Just think about the security problem version 4.5 probably had and that have
been corrected in 4.5.19.

> About subpackages, I'm not sure about which ones might be useful, and I don't
> see the package mature enough for them.

You really should do subpackages. Thus the user can choose to install only the
subpackages he needs.


Comment 4 Javier Palacios 2008-06-23 19:17:17 UTC
Actually I didn't see any missing guideline. Just say 'some' as a midpoint from
none to many. Whichever blocking guideline you see that I didn't follow, please
tell me, and I'll fix.

rpmlint only gives a warning about indentation mixing of tabs and spaces, but
that's actually a rpmlint problem parsing a HERE document.

And regarding the non-fedora bits, I might remove the check for build root not
being root directory, but the cleaning of buildroot _is_ a fedora bit.

Regarding the version, I have upgraded (and uploaded) the package to the latest
stable version (4.5.19)

Comment 5 Andrea Musuruane 2008-06-25 09:29:59 UTC
(In reply to comment #4)
> And regarding the non-fedora bits, I might remove the check for build root not
> being root directory, but the cleaning of buildroot _is_ a fedora bit.

Cleaning the buildroot this way:
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT
is not a Fedora bit.

Since you should NEVER build an RPM as root, it MUST be done this way:
rm -rf $RPM_BUILD_ROOT

> Regarding the version, I have upgraded (and uploaded) the package to the latest
> stable version (4.5.19)

At each update, you must provide a link to the updated spec file and a link to
the new source RPM file and a changelog. This is a Fedora Packaging guideline.

Moreover, you aren't versioning the spec file in the changelog (something
rpmlint complains about) and you didn't update the changelog.

The licence tag in not correct. GPL is not an acceptable license (but, for
example, GPLv2 is). This is also something rpmlint complains about.

Since you are looking for a sponsor, I suggest you to read this:
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored


Comment 6 Javier Palacios 2008-06-25 20:41:00 UTC
The location for the new spec/srpm are
Spec URL: http://www.freewebs.com/javiplx/Fedora/gforge.spec
SRPM URL: http://www.freewebs.com/javiplx/Fedora/gforge-4.5.19-1.fc9.src.rpm

The check for buildroot value has been removed both within %install and %clean
sections.

Which rpmlint version are you using ? The one I use from
rpmlint-0.82-3.fc9.noarch does not complain about the changelog or license, but
only about the tab-spaces mentioned before. In any case, I've also changed both
items.


Comment 7 Andrea Musuruane 2008-06-25 22:00:03 UTC
Some other notes.

* Buildroot is not correct:
https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

* Changelog format is not correct:
https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs

* License tag is still not correct:
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines

I just read the license in a couple of files and they are GPLv2+. You should
read the license statements in *all* the files. I know it's a PITA but it is for
a good purpose.

* Docs should not include INSTALL instructions.

* I had a quick look at the mandriva RPM. It seems you miss some Requires. For
example, why mailman is not required? And cvs? Or subversion? Again this should
be done using subpackages thus the user can only install what it needs.

* You also seem not to install a lot from the source package. When making a
package you should provide the most out of the upstream package, not the minimum.

* RPM macros are not always used:
https://fedoraproject.org/wiki/Packaging/RPMMacros

For example, you should not use /etc but %{_sysconfdir} instead.

* Source URL is not downloadable:
https://fedoraproject.org/wiki/Packaging/SourceURL



Comment 8 Javier Palacios 2008-06-26 19:07:26 UTC
First of all, I've discovered the rpmlint discrepancies. I was running that on
the specfile itself, but not on the packages. It looks like rpmlint reads the
RPM headers, so I didn't discovered many things (as far as I know, the
non-standard-uid for apache are not important, are they?)

The new versions are on
Spec URL: http://www.freewebs.com/javiplx/Fedora/gforge.spec
SRPM URL: http://www.freewebs.com/javiplx/Fedora/gforge-4.5.19-2.fc9.src.rpm

Buildroot, %_sysconfigdir, and %docs fixed.

For Changelod I preferred the third available option, as I tend to copy the
modifier line and change only the date.

I've changed also the license, but I have a doubt about it. The upstream site
(fixed now on URL) simply states 'GPL'. Shouldn't this be honoured ?

Regarding the Source with full url, the download url
(http://gforge.org/frs/download.php/245/gforge-4.5.19.tar.bz2) includes a
internal release number 245 that changes from one version to another on
unrelated manner (for 4.4 is 102). Is it OK to use this unpredictable value ?

mailman requirement is also fixed. It was actually a mistake.

Regarding subpackages (specifically svn/cvs) I'm still believe the package is
not mature enough. The packaged local.inc fixes sys_use_scm=false, so none of
them are currently a requirement. I obviously plan to make them available, but I
need more work to make that smoothly. Currently I use gforge at workplace, but
without many features (not only SCM but also mailmain) and it's perfectly
functional, even whithout the many subdirs I opted not to copy to buildroot.

My plans, in parallel with made the package compliant with guidelines, is to
enable all this stuff one by one with proper although basic testing, to ensure
that the final package is at least only partly broken. As example, adding the
plugins directory adds some Perl requirements on a full PHP package that I think
must be tracked.

Comment 9 Andrea Musuruane 2008-06-27 09:11:32 UTC
(In reply to comment #8)
> I've changed also the license, but I have a doubt about it. The upstream site
> (fixed now on URL) simply states 'GPL'. Shouldn't this be honoured ?

No, you should follow what is written here ("Q: How do I figure out what version
of the GPL/LGPL my package is under?"):
http://fedoraproject.org/wiki/Licensing/FAQ

> Regarding the Source with full url, the download url
> (http://gforge.org/frs/download.php/245/gforge-4.5.19.tar.bz2) includes a
> internal release number 245 that changes from one version to another on
> unrelated manner (for 4.4 is 102). Is it OK to use this unpredictable value ?

It's OK. It is more important to check that the version you supply matches the
upstream one. You only need to remember to update it at every new release.

> Regarding subpackages (specifically svn/cvs) I'm still believe the package is
> not mature enough. The packaged local.inc fixes sys_use_scm=false, so none of
> them are currently a requirement. I obviously plan to make them available, but I
> need more work to make that smoothly. Currently I use gforge at workplace, but
> without many features (not only SCM but also mailmain) and it's perfectly
> functional, even whithout the many subdirs I opted not to copy to buildroot.

When you request a review is because you think your package is mature enough. I
suggest you to work on adding subpackages and posting an update. Moreover,
gforge is not really useful without an SCM tool.

Meanwhile, since you need to be sponsored, I suggest you to follow what is
described in the HowToGetSponsored wiki page:

http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

And in particular that "The best ways for you to illustrate your understanding
of the packaging guidelines are to submit quality packages and to assist with
package reviews."

Since GForge is a request on the WishList
(https://fedoraproject.org/wiki/PackageMaintainers/WishList), I written a note
that there is this review request. This should give you more visibility.

I'll be on holidays really soon, so I hope someone else can step in and help you.


Comment 10 Javier Palacios 2008-07-03 22:11:05 UTC
The new versions are on
Spec URL: http://www.freewebs.com/javiplx/Fedora/gforge.spec
SRPM URL: http://www.freewebs.com/javiplx/Fedora/gforge-4.5.19-3.fc9.src.rpm

Main changes are inclusion of plugins/subpackages.

Comment 11 Andrea Musuruane 2008-07-19 13:06:48 UTC
Javier, you should really start to comment other review requests as stated here:

http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored#Reviewing_Packages

Otherwise it is very difficult that someone will sponsor you.


Comment 12 Nicolas Chauvet (kwizart) 2008-07-29 15:33:08 UTC
Just a quick look and few advices.

1/ While using cp (ex: cp cronjobs/cvs-cron/cvs.php
%{buildroot}/%{gforge_dir}/cronjobs ) you may need to prevent timestamps changes
by using cp -p (or install -pm 0644 )
2/ 	-e 's/192\.168\.100\.100/127.0.0.1/' \ seems strange for a default install
nobody know which sub-network will be used on the end-users system.
Unfortunately that will probably remains a end-users post install command install
3/ 	# Set password for admin user (pw=admin)
	SITEADMIN_PASSWORD="21232f297a57a5a743894a0e4a801fc3"
This seems scary at first sight...

Does the patches bundled will be merged in the next gforge release ?


Comment 13 Javier Palacios 2008-07-30 12:55:12 UTC
Regarding point one I take note, although I'll not be able to fix in some days
(city shift)

Regarding point 2, as it's not possible to guess the network address that the
gforge serve will have, I prepare everything to work on localhost (as I test
it), leaving to the final user further modifications. In the mid-term, this
values will go into a /etc/sysconfig file, but that probably needs a little bit
patching.

Regarding 3, as with any other web application, there is a need for a initial
administrative account. Harcoding a fixed password is just a little bit better
than leaving blank. But any suggestion is very wellcomed. Asking for the admin
password on %post looks to me a bit ugly, despite the fact that might interfere
with yum based installs.

I pretent to submit the patches upstream, but I've not done yet, partly because
I want to have a look to the 4.6 version, which now is on Rc stage.

Comment 14 Rex Dieter 2008-07-30 13:31:21 UTC
> Asking for the admin password on %post looks to me a bit ugly

more than ugly, broken and not a viable option.  The only sane way here is to
simply have the service disabled by default, and require manual intervention to
enable and set the needed password.

Comment 15 Rex Dieter 2008-07-30 13:31:53 UTC
ok, not the only sane way... but *a* sane way. :)

Comment 16 Javier Palacios 2008-08-05 19:50:47 UTC
I have updated the package. The missing `cp -p` are fixed, and I've also adopted the Users & Groups guidelines.

Spec URL: http://www.freewebs.com/javiplx/Fedora/gforge.spec
SRPM URL: http://www.freewebs.com/javiplx/Fedora/gforge-4.5.19-4.fc9.src.rpm

Regarding the way to hardcoding any password in the package, I'm thinking about writing a helper script that must be run by superuser and performs that task after the package has been installed. The point is the proper way to inform about it. Issuing a message on %post might be unnoticed by many users. Mailing to root account sounds better, but might not be correctly redirected. And adding that into a README/ENABLE file on %doc does not appear as a better solution to me.
Any ideas ?

Comment 17 Andrea Musuruane 2008-08-06 08:10:48 UTC
(In reply to comment #16)
> Regarding the way to hardcoding any password in the package, I'm thinking about
> writing a helper script that must be run by superuser and performs that task
> after the package has been installed. The point is the proper way to inform
> about it. Issuing a message on %post might be unnoticed by many users. Mailing
> to root account sounds better, but might not be correctly redirected. And
> adding that into a README/ENABLE file on %doc does not appear as a better
> solution to me.
> Any ideas ?

The Fedora way is a README.Fedora file in %doc. It is used by many packages and that is what an admin should look for after installing if manual intervention is needed.

https://fedoraproject.org/wiki/Packaging/Guidelines#summary

Comment 18 Javier Palacios 2008-08-13 19:26:47 UTC
> The Fedora way is a README.Fedora file in %doc. It is used by many packages and
> that is what an admin should look for after installing if manual intervention
> is needed.

New versions at

Spec URL: http://www.freewebs.com/javiplx/Fedora/gforge.spec
SRPM URL: http://www.freewebs.com/javiplx/Fedora/gforge-4.5.19-5.fc9.src.rpm

Now there is a README.Fedora package included, with a script to set interactively the administrator password.

It has also been added the svntracker plugin, and the reference to upstream submitted patches (following Packaging/PatchUpstreamStatus)

Comment 19 Nicolas Chauvet (kwizart) 2009-01-28 09:35:07 UTC
Was the project renamed ?
What is the status of the package ?

Comment 20 Nicolas Chauvet (kwizart) 2009-04-14 16:02:53 UTC
closing to NOTABUG since there was no answear.
I think there is some redesign within the gforge team, so maybe it can be re-opened later...

If you think a gforge package can be reviewed for Fedora introduction. please re-open the bug report.


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