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

[Efraim Flashner <efraim@flashner.co.il>]

[-- Attachment #1: Type: text/plain, Size: 5993 bytes --]

On Thu, Sep 01, 2022 at 10:41:21AM -0400, Thompson, David wrote:
> Hi Efraim,
> 
> On Thu, Sep 1, 2022 at 10:20 AM Efraim Flashner <efraim@flashner.co.il> wrote:
> >
> > On Thu, Sep 01, 2022 at 09:59:55AM -0400, Thompson, David wrote:
> > > Hi all,
> > >
> > > Reviving this old thread.
> > >
> > > On Mon, Mar 28, 2022 at 2:51 AM Efraim Flashner <efraim@flashner.co.il> wrote:
> > > > >
> > > > > Seems like all we have to do is 'substitute*' a '/usr/bin/svnserve'
> > > > > into a '/gnu/store/...' (untested), so seems actionable to me.
> > > > > Alternatively, as Efraim wrote, let it search the $PATH (that might be
> > > > > useful if adding svnserve would increase the closure too much and it is
> > > > > an optional dependency in practice?).
> > > >
> > > > I spent some time looking at gitolite and the service. As I understand
> > > > it, with the exception of svnserve, it searches $PATH for a number of
> > > > different binaries, including git-annex. I believe that this would only
> > > > work if git-annex (and potentially other packages) are installed
> > > > globally.
> > > >
> > > > In addition, git (not git-minimal) and openssh are propagated inputs AND
> > > > wrapped. I haven't tested to see if wrapping only is enough.
> > > >
> > > > I think the best choice is to:
> > > > A: Replace /usr/bin/svnserve with svnserve so it will just search $PATH,
> > > > like it does with the other helpers.
> > >
> > > I see that you have done this.  Thanks!  We could also replace the
> > > reference to /usr/sbin/redis-server in src/lib/Gitolite/Cache.pm.
> > > That's the only other /usr reference I can find (that isn't in a
> > > comment) in the output.  I have the patch ready if that sounds good to
> > > you.
> >
> > Sounds good to me
> 
> Thanks, pushed as commit c053dfa52dc778eb3d965f58a85c435ae7fab0dd.
> 
> > > > B: Adjust the service so that it automatically creates a variant (or
> > > > just a wrapped version) of the package which is wrapped with a list of
> > > > additional packages so that they can be in gitolite's path. If I were
> > > > deploying this to an arm device I wouldn't want it wrapped with
> > > > git-annex since it doesn't build, but would definitely want it for an
> > > > x86_64 machine.
> > >
> > > The service configuration record could accept a list of addons like
> > > '(git-annex cache svnserve), with a default of no addons '(), and
> > > create a package that extends the gitolite package with the
> > > appropriate propagated inputs.  Does that sound like what you had in
> > > mind?  A more robust solution could modify the build to hardcode the
> > > store paths needed for the add-ons but given that we already propagate
> > > git and openssh I don't think it's necessary right now.
> >
> > Assuming this is deployed into some sort of container then propagated
> > inputs wouldn't help much, we'd need either the PATH for the container
> > to be extended to include those extra packages or to have gitolite
> > itself wrapped similar to icedove/wayland. Just extending the PATH in
> > the #:environment-variables would be enough I'd think.
> 
> Hmm, I hadn't thought about the container use case.  Your approach
> sounds like the way to go.  For what it's worth, I think the gitolite
> service as-is would suffer the same issue in a containerized
> environment because it relies upon the git and openssh propagated
> inputs to do anything at all.  With the gitolite service in my system,
> /run/current-system/profile/bin has both git and ssh in it due to the
> propagation.  So it sounds like there's 2 steps needed: 1) Use a
> wrapper like icedove/wayland for the base gitolite package so that git
> and openssh no longer need propagation, and then 2) extend the
> gitolite service to wrap up additional packages needed for the desired
> extensions.  Sound good?
> 
> > > > I suppose we should try to find someone who is using the gitolite
> > > > service and see if they can be our test subject for wrapping the package
> > > > with optional addons.
> > >
> > > I use the gitolite service and can be the test subject.  I don't
> > > currently use any add-ons, but the redis one sounds easy enough to try
> > > and hey maybe it's a good excuse to finally learn how to use
> > > git-annex.
> > >
> > > As a longer term thing, it would be cool to revisit propagating git
> > > and openssh in this package.  I punted on it back in 2015 for the
> > > reason stated in the source comments but maybe there's a reasonable
> > > and reliable way to directly embed the store paths now.
> >
> > It's actually been forever since I looked at gitolite so I don't know
> > remember what those inputs were needed for, but it'd be great to improve
> > the service.
> 
> Are you referring to git and openssh or redis, svnserve, git-annex,
> etc.?  I'm no expert and I really don't like Perl, but I know gitolite
> well enough to explain some of this stuff.
> 
> > Interestingly, I almost have a working ghc-8.6 for aarch64 after all
> > these years.
> 
> Some things move at a glacial pace, but eventually they get done.
> Best of luck wrapping that up.  :)

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.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]