bug#25957: gitolite broken: created repositories keep references to /usr/bin for hooks

[Thompson, David]

On Fri, Sep 2, 2022 at 8:50 AM Thompson, David <dthompson2@worcester.edu> wrote:
>
> On Fri, Sep 2, 2022 at 8:44 AM Efraim Flashner <efraim@flashner.co.il> wrote:
> >
> > On Fri, Sep 02, 2022 at 07:11:54AM -0400, Thompson, David wrote:
> > > On Fri, Sep 2, 2022 at 3:00 AM Efraim Flashner <efraim@flashner.co.il> 
> > > wrote:
> > > >
> > > > I took a look at the gitolite service finally and I hadn't realized
> > > > there wasn't a running daemon to containerize. I assumed we could do
> > > > something like:
> > > >
> > > > (start $~(make-forkexec-constructor/container
> > > >             (list ...)
> > > >             #:environment-variables
> > > >             '("PATH=...")
> > > >             #:mappings ...))
> > > >
> > > > Given that's not the case then I'd need to look at gitolite itself to
> > > > see how it calls the other binaries it expects to be available, and if
> > > > wrapping it would be enough or if we would need to just propagate the
> > > > other packages for functionality.
> > >
> > > Gitolite simply expects tools like git to be on $PATH.  It's a pretty
> > > naive system, there's nothing like a configure script that is
> > > determining the absolute file name of these tools and substituting
> > > those names into the built files.
> > >
> > > The executable is already wrapped so that coreutils, findutils, and
> > > git are on $PATH, but notably not openssh:
> > >
> > > (add-after 'install 'wrap-scripts
> > >                     (lambda* (#:key inputs outputs #:allow-other-keys)
> > >                       (let ((out (assoc-ref outputs "out"))
> > >                             (coreutils (assoc-ref inputs "coreutils"))
> > >                             (findutils (assoc-ref inputs "findutils"))
> > >                             (git (assoc-ref inputs "git")))
> > >                         (wrap-program (string-append out "/bin/gitolite")
> > >                           `("PATH" ":" prefix
> > >                             ,(map (lambda (dir)
> > >                                     (string-append dir "/bin"))
> > >                                   (list out coreutils findutils git)))))))
> > >
> > > However, git and openssh are still propagated inputs. I'm going to
> > > move the propagated inputs to regular inputs, potentially add openssh
> > > to the wrapper once I remind myself what gitolite does with those
> > > tools, and test it all out on my server using the gitolite service.
> > > If that all works, we have a good starting point for adding extension
> > > support in the service.
> >
> > I like it. Let us know how it goes.
>
> The problem is that gitolite generates git hooks for the repositories
> that it manages, and those hooks invoke git, so the only way those
> scripts will be able to work (without input propagation) is to find a
> way to inject the proper PATH or find a way to replace references to
> things like 'git diff' with '/gnu/store/.../git diff'.  I'm going to
> keep exploring and report back when I have something to show.

After several rounds of experimentation and breaking my git server a
few times, here's what I've found:

* Changing git and openssh to be regular inputs and wrapping both
gitolite and gitolite-shell with a $PATH that contains git works and
it's very little extra code.

* Trying to replace every invocation of a git command took a lot of
grepping and crafting of regexps to use for substitute* and I never
got to a point where the result wasn't buggy.  In particular,
gitolite-shell never worked properly so I couldn't push to my repos.

So, I think the simple wrapper approach is the way to go.  Patch
attached.  I tested on my git server by making changes to my gitolite
configuration and pushing those changes to the special gitolite-admin
repo.  This causes gitolite to refresh internal configuration using a
git hook, so I know that hooks can find the executables they need.
That plus the 'gitolite setup' invocation made by the service
activation script covers a fair amount of surface area, so I feel
comfortable committing it.  What do you think?

Once this part is done, I'll turn my attention to the optional extensions.

- Dave

0001-gnu-gitolite-Wrap-programs-instead-of-using-propagat.patch
Description: Text Data