Bug 548324

Summary: Review Request: gitolite - Highly flexible server for git directory version tracker
Product: [Fedora] Fedora Reporter: Lubomir Rintel <lkundrak>
Component: Package ReviewAssignee: Gerd Pokorra <gp>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, gp, notting, opensource
Target Milestone: ---Flags: gp: fedora-review+
j: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-01-20 23:54:25 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Lubomir Rintel 2009-12-17 06:09:46 UTC
SPEC: http://v3.sk/~lkundrak/SPECS/gitolite.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/gitolite-0.95-1.20091216git.fc12.src.rpm
scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1876569

Description:

Gitolite allows a server to host many git repositories and provide access
to many developers, without having to give them real userids on the server.
The essential magic in doing this is ssh's pubkey access and the authorized
keys file, and the inspiration was an older program called gitosis.

Gitolite can restrict who can read from (clone/fetch) or write to (push) a
repository. It can also restrict who can push to what branch or tag, which
is very important in a corporate environment. Gitolite can be installed
without requiring root permissions, and with no additional software than git
itself and perl. It also has several other neat features described below and
elsewhere in the doc/ directory.

Comment 1 Lubomir Rintel 2009-12-17 06:10:31 UTC
RPMlint only complains about gitolite:gitolite not being standard uid:git; which is ok since we create the user.

Comment 2 Lubomir Rintel 2009-12-17 06:15:29 UTC
Whoops, here's the correct scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1876573

Comment 3 Gerd Pokorra 2009-12-28 11:55:18 UTC
This is a suggestion. A user "gitolite" will be added by installing the package but will not deleted when the package will be erased. Other packages like "mysql-server" also do not deleted the user they add so this is only a suggestion:

Some lines like this may be added in spec-file:

Requires(postun): shadow-utils
...

%postun
userdel gitolite



What about a start-script /etc/init.d/gitolite, so that gitolite will be in the list of the commnad output from "chkconfig --list". It would be nice if the server could be started with a simple command by using a default local configuration.


I am sorry for having no time to have a closer look to this package at the moment.

Comment 4 Gerd Pokorra 2009-12-29 17:00:25 UTC
Hi Lubomir,

after a short second look I saw that gitolite works without a daemon, so my question for a init-script is nonsense.

Would you short discribe me what I have to do to get a testing repository after I have installed the package.

Comment 5 Till Maas 2010-01-06 14:18:37 UTC
Here are some annotationes:

- Please use the package name as a prefix for the patch/source to avoid conflicts with other packages in the SOURCES directory
- the patch needs a comment wrt to the upstream status, i.e. was it sent to upstream or is it upstreamable?

Comment 6 Lubomir Rintel 2010-01-07 16:09:28 UTC
(In reply to comment #5)
> Here are some annotationes:
> 
> - Please use the package name as a prefix for the patch/source to avoid
> conflicts with other packages in the SOURCES directory

Done.

> - the patch needs a comment wrt to the upstream status, i.e. was it sent to
> upstream or is it upstreamable?  

Done.

(In reply to comment #4)
> Hi Lubomir,
> 
> after a short second look I saw that gitolite works without a daemon, so my
> question for a init-script is nonsense.
> 
> Would you short discribe me what I have to do to get a testing repository after
> I have installed the package.  

Note that the installation and configuration is a bit weird. There are two ways to install gitolite, traditional and "easy" (remote). Both were adjusted to work with this package. You'll do an easy install like this:

Create ~gitolite/.ssh/authorized-keys and add your key there
$ gl-easy-install gitolite localhost $LOGNAME

The other way to do that would be to su to gitolite user, run "gl-install", follow the instructions and then manually populate the gitolite-admin.git repository (and recompile the configuration, as you'd be instructed).

New package:

SPEC: http://v3.sk/~lkundrak/SPECS/gitolite.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/gitolite-0.95-2.20091216git.fc12.src.rpm

Comment 7 Gerd Pokorra 2010-01-08 04:06:14 UTC
The name of the patch is changed, but not the name of the source-file.

I suggest to extend the two lines in the spec-file from

# $ make ed2bf5.tar
Source0:        ed2bf5.tar

to

# $ make ed2bf5.tar
# $ mv ed2bf5.tar gitolite-%{version}.tar
# $ gzip -9 gitolite-%{version}.tar
Source0:        %{name}-%{version}.tar.gz

Comment 8 Gerd Pokorra 2010-01-08 04:41:27 UTC
I tried gitolite from the git. The command to clone the "testing" repository works
with:

$ git clone gitolite:testing.git

but using the username and hostname

$ git clone <user>@<host.domain>:testing.git again2

gives out the following error:

Initialized empty Git repository in /home/gz016/gitolite/testing/again2/.git/
fatal: 'testing.git': unable to chdir or not a git archive
fatal: The remote end hung up unexpectedly
[gz016@hilton testing]$

Comment 9 Lubomir Rintel 2010-01-08 08:38:20 UTC
Hi Gerd,

(In reply to comment #7)
> The name of the patch is changed, but not the name of the source-file.
> 
> I suggest to extend the two lines in the spec-file from
> 
> # $ make ed2bf5.tar
> Source0:        ed2bf5.tar
> 
> to
> 
> # $ make ed2bf5.tar
> # $ mv ed2bf5.tar gitolite-%{version}.tar
> # $ gzip -9 gitolite-%{version}.tar
> Source0:        %{name}-%{version}.tar.gz  

Umm, that was an upstream decision (the file name), we don't usually rename upstream tarballs and I'm reluctant to do so.

(In reply to comment #8)
> I tried gitolite from the git. The command to clone the "testing" repository
> works
> with:
> 
> $ git clone gitolite:testing.git
> 
> but using the username and hostname
> 
> $ git clone <user>@<host.domain>:testing.git again2
> 
> gives out the following error:
> 
> Initialized empty Git repository in /home/gz016/gitolite/testing/again2/.git/
> fatal: 'testing.git': unable to chdir or not a git archive
> fatal: The remote end hung up unexpectedly
> [gz016@hilton testing]$  

The <user> above is always "gitolite". Gitolite doesn't use UNIX users/permissions to implement access control, it implements its own and determines the user name from the key used.

Therefore, after initial checkout of gitolite-admin, add your key there, push back and you should be able to access the repositories.

Comment 10 Mamoru TASAKA 2010-01-08 09:20:50 UTC
(In reply to comment #9)
> (In reply to comment #7)
> > I suggest to extend the two lines in the spec-file from
> > 
> > # $ make ed2bf5.tar
> > Source0:        ed2bf5.tar
> > 
> > to
> > 
> > # $ make ed2bf5.tar
> > # $ mv ed2bf5.tar gitolite-%{version}.tar
> > # $ gzip -9 gitolite-%{version}.tar
> > Source0:        %{name}-%{version}.tar.gz  
> 
> Umm, that was an upstream decision (the file name), we don't usually rename
> upstream tarballs and I'm reluctant to do so.

Fedora already has guidelines for creating source tarball
based on the upstream VCS:
https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control

i.e. tarball name must begin with %{name}- in this case, and the tarball
     should contain the information about version, revision, etc in
     its name.

Comment 11 Lubomir Rintel 2010-01-08 10:02:51 UTC
(In reply to comment #10)
> Fedora already has guidelines for creating source tarball
> based on the upstream VCS:
> https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control

Thank you, I did not know. I will fix this up upon next package spin (when the full review is done).

Comment 12 Gerd Pokorra 2010-01-08 17:13:30 UTC
Hi Lubomir,

I got the gitolite package configured and cloned a repository from
/var/lib/gitolite/repositories. Your instructions in comment #6 was
very helpful. I think you should add this in a README file to the package.
So other users can start quickly and find the basics to work with the
gitolite package.
My suggestion is to call the file README.Fedora or README.RPM or 
CONFIG.Fedora or only README and to add it to your patch
(diff -u /dev/null README.Fedora >> gitolite-0.95-conf.patch)
or try to get it in the upstream.

If this is done und the source file name is changed for me the RPM for
the nice software looks okay.

If then not anyone have other comments in the next five days I would like to
set the status from the review to done.

Comment 13 Lubomir Rintel 2010-01-08 18:48:19 UTC
(In reply to comment #12)
> I got the gitolite package configured and cloned a repository from
> /var/lib/gitolite/repositories. Your instructions in comment #6 was
> very helpful. I think you should add this in a README file to the package.
> So other users can start quickly and find the basics to work with the
> gitolite package.
> My suggestion is to call the file README.Fedora or README.RPM or 
> CONFIG.Fedora or only README and to add it to your patch
> (diff -u /dev/null README.Fedora >> gitolite-0.95-conf.patch)
> or try to get it in the upstream.

While upstream had really weird deployment process of the thing, I intended not to break the existing documentation when adapting the package to RPM packaging.

Is there anything that was unclear, wrong or missing from upstream documentation? In that case we should probably fix it up instead.

On the other hand if your idea is just to add a few lines of quick start info, I won't mind adding it.

> If this is done und the source file name is changed for me the RPM for
> the nice software looks okay.
> 
> If then not anyone have other comments in the next five days I would like to
> set the status from the review to done.  

This is your review, therefore approval relies upon your findings. If anyone else finds a problem, he's free to open bugs, or commit into the package.

Comment 14 Gerd Pokorra 2010-01-10 18:05:13 UTC
(In reply to comment #13)

Hi Lubomir,
> On the other hand if your idea is just to add a few lines of quick start info,
> I won't mind adding it.

Yes, this is my idea. You are the owner of the package so this is your decision.
I only want to suggest it, because I think someone who read the text in

/usr/share/doc/gitolite-0.95/0-INSTALL.html

will not undestand that he had to do something like that

Create ~gitolite/.ssh/authorized-keys and add your key there
$ gl-easy-install gitolite localhost $LOGNAME

Comment 15 Gerd Pokorra 2010-01-10 18:22:52 UTC
P. S.

I think the two warnings of rpmlint 

gitolite.noarch: W: non-standard-uid /var/lib/gitolite gitolite
gitolite.noarch: W: non-standard-gid /var/lib/gitolite gitolite

can be ignored.

Comment 16 Lubomir Rintel 2010-01-10 22:53:31 UTC
New Package CVS Request
=======================
Package Name: gitolite
Short Description: Highly flexible server for git directory version tracker
Owners: lkundrak
Branches: EL-5 F-11 F-12

Comment 17 Jason Tibbitts 2010-01-12 06:19:08 UTC
CVS done (by process-cvs-requests.py)

Comment 18 Lubomir Rintel 2010-01-20 23:54:25 UTC
Imported and built.
Thanks for the review!

Comment 19 Till Maas 2010-01-26 13:55:02 UTC
(In reply to comment #18)
> Imported and built.
> Thanks for the review!    

Not true! :-/ It failed for EL-5. Did you ask the perl-Text-Markdown, whether he will maintain it for EPEL, too? This package is missing to build it. The review request of perl-Text-Markdown is bug 243716.

Comment 20 Lubomir Rintel 2010-01-26 14:07:54 UTC
(In reply to comment #19)
> (In reply to comment #18)
> > Imported and built.
> > Thanks for the review!    
> 
> Not true! :-/

Not not true :F

Unless I'm wrong, it's sufficient to build in one of possible branches and I don't really care about EL-5 usually. This time I cared a bit, though I forgot somehow...

> It failed for EL-5. Did you ask the perl-Text-Markdown, whether
> he will maintain it for EPEL, too?

Yes I did.

> This package is missing to build it. The
> review request of perl-Text-Markdown is bug 243716.    

I originally bugged wrong ticket #532190 of Text::MultiMarkdown and then forgot to correct my mistake. Thanks a lot for reminder, I'll request the branch now.

By the way, I am thankful when contributors who have some interest in packages comaintain the packages. Would you mind adding yourself to gitolite in pkgdb?

Comment 21 Till Maas 2010-01-26 14:45:39 UTC
(In reply to comment #20)
> (In reply to comment #19)
> > (In reply to comment #18)
> > > Imported and built.
> > > Thanks for the review!    
> > 
> > Not true! :-/
> 
> Not not true :F
> 
> Unless I'm wrong, it's sufficient to build in one of possible branches and I
> don't really care about EL-5 usually. This time I cared a bit, though I forgot
> somehow...

Not sure, in which way it is sufficient, but it's miscommunication, because I wanted to test the gitolite EL-5 package and all information from the bug report said that I could do it by now. I did not want to offend you, but it was pretty disappointing to read that the package was built, but then find out when I wanted to use it, that it did not actually build.

> By the way, I am thankful when contributors who have some interest in packages
> comaintain the packages. Would you mind adding yourself to gitolite in pkgdb?    

I did not yet use gitolite, I planned to look into it today, but since it does not build, I deferred it. If I took a look, I'll apply as co-maintainer. But I am not a very good perl coder.