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

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

[ng0]

Our gitolite package currently creates all
(including gitolite-admin.git) git repositories with references to
"/usr/bin/perl" as shebang, which makes it completely useless on
serverside.
Given that the server side in the case of a gitolite from Guix runs an
environment where you will not run perl from /usr/bin/, you will have to
change all hooks manually currently.

When you install perl into the profile of the user which hosts the
repositories and change the shebangs, gitolite can be used.

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

[ng0]

What makes this worse, with every update (push) of gitolite-admin
repository the shebang of "hooks/update" is reset.
Other repositories seem to keep changes in the hooks shebangs so
far.

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

[ng0]

On 17-03-03 22:27:43, ng0 wrote:
> What makes this worse, with every update (push) of gitolite-admin
> repository the shebang of "hooks/update" is reset.
> Other repositories seem to keep changes in the hooks shebangs so
> far.
> 
> 
> 

When I build gitolite from guix, this looks trivial to fix.

[user@abyayala /gnu/store/jw252kw9blfh1lrrib3yk4fkbj5mvdpm-gitolite-3.6.5/share/gitolite]$ egrep -nr "/usr/"
commands/svnserve:9:$svnserve ||= "/usr/bin/svnserve -r /var/svn/ -t --tunnel-user=%u";
lib/Gitolite/Test/Tsh.pm:42:# path when cwd is [...] at /usr/share/perl5/File/Temp.pm line 902".
lib/Gitolite/Hooks/PostUpdate.pm:62:#!/usr/bin/perl
lib/Gitolite/Hooks/Update.pm:158:#!/usr/bin/perl
lib/Gitolite/Cache.pm:127:    open( REDIS, "|-", "/usr/sbin/redis-server", "-" ) or die "start redis server fail
ed: $!"; 


The parts I want to fix as my immediately affect every user, are in
the directory "lib/Gitolite/Hooks/", I have no idea about redis, but
I think there should be a reference to /gnu/store/ reddis and not
"/usr/sbin/redis-server". Different problem, related bug.. This can be
solved in a commit after this bug.

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

[Danny Milosavljevic]

Hi ng0,

> I think there should be a reference to /gnu/store/ reddis and not
> "/usr/sbin/redis-server". Different problem, related bug.. This can be
> solved in a commit after this bug.

Yeah.

I would question why a normal application needs to start a redis *server* in the first place. Sounds strange to me. But I agree that if it wants to do that it should use a store reference.

<https://redis.io/topics/admin> says "Redis is designed to be a very long running process in your server" so that definitely reads to me that a normal program shouldn't just start redis-server when it feels like it (and I hope it stops it again later? After reading the source code it doesn't appear that way...).

<http://gitolite.com/gitolite/cache.html> says "WARNING: this has not been tested in a while. YMMV". Uhhhh. Not confidence-inspiring.

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

[ng0]

On 17-03-04 16:43:09, Danny Milosavljevic wrote:
> Hi ng0,
> 
> > I think there should be a reference to /gnu/store/ reddis and not
> > "/usr/sbin/redis-server". Different problem, related bug.. This can be
> > solved in a commit after this bug.
> 
> Yeah.
> 
> I would question why a normal application needs to start a redis *server* in the first place. Sounds strange to me. But I agree that if it wants to do that it should use a store reference.
> 
> <https://redis.io/topics/admin> says "Redis is designed to be a very long running process in your server" so that definitely reads to me that a normal program shouldn't just start redis-server when it feels like it (and I hope it stops it again later? After reading the source code it doesn't appear that way...).
> 
> <http://gitolite.com/gitolite/cache.html> says "WARNING: this has not been tested in a while. YMMV". Uhhhh. Not confidence-inspiring.

It was the first time I read about reddis in gitolite context, and in
all the time I used gitolite I never really needed it when building or
running.
I disregard this as not very important and not really important at all
to fix. It should be fixed in the long run, but my main concern was
usability of gitolite, which has been addressed in one of the two
patches I've sent.

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

[zimoun]

Hi,

This old bug [1] is still relevant.

1: <http://issues.guix.gnu.org/issue/25957>

On Sat, 04 Mar 2017 at 13:32, ng0 <contact.ng0@cryptolab.net> wrote:
> On 17-03-03 22:27:43, ng0 wrote:

This previous…

> [user@abyayala /gnu/store/jw252kw9blfh1lrrib3yk4fkbj5mvdpm-gitolite-3.6.5/share/gitolite]$ egrep -nr "/usr/"
> commands/svnserve:9:$svnserve ||= "/usr/bin/svnserve -r /var/svn/ -t --tunnel-user=%u";
> lib/Gitolite/Test/Tsh.pm:42:# path when cwd is [...] at /usr/share/perl5/File/Temp.pm line 902".
> lib/Gitolite/Hooks/PostUpdate.pm:62:#!/usr/bin/perl
> lib/Gitolite/Hooks/Update.pm:158:#!/usr/bin/perl
> lib/Gitolite/Cache.pm:127:    open( REDIS, "|-", "/usr/sbin/redis-server", "-" ) or die "start redis server fail
> ed: $!";

…is now…

--8<---------------cut here---------------start------------->8---
share/gitolite/lib/Gitolite/Test/Tsh.pm:42:# path when cwd is [...] at /usr/share/perl5/File/Temp.pm line 902".
share/gitolite/lib/Gitolite/Cache.pm:127:    open( REDIS, "|-", "/usr/sbin/redis-server", "-" ) or die "start redis server failed: $!";
share/gitolite/commands/svnserve:9:$svnserve ||= "/usr/bin/svnserve -r /var/svn/ -t --tunnel-user=%u";
--8<---------------cut here---------------end--------------->8---

…but…

> The parts I want to fix as my immediately affect every user, are in
> the directory "lib/Gitolite/Hooks/", I have no idea about redis, but
> I think there should be a reference to /gnu/store/ reddis and not
> "/usr/sbin/redis-server". Different problem, related bug.. This can be
> solved in a commit after this bug.

…the package redis is not a dependency of gitolite.  Therefore, the
question is: is our Gitolite package working with Redis?  Even using the
/usr/bin one?  Idem for SVN.

Otherwise, I am favor to remove the 2 “problematic” references.   WDYT?


Cheers,
simon

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

[Efraim Flashner]

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

On Wed, Jan 05, 2022 at 01:07:07AM +0100, zimoun wrote:
> Hi,
> 
> This old bug [1] is still relevant.
> 
> 1: <http://issues.guix.gnu.org/issue/25957>
> 
> On Sat, 04 Mar 2017 at 13:32, ng0 <contact.ng0@cryptolab.net> wrote:
> > On 17-03-03 22:27:43, ng0 wrote:
> 
> This previous…
> 
> > [user@abyayala /gnu/store/jw252kw9blfh1lrrib3yk4fkbj5mvdpm-gitolite-3.6.5/share/gitolite]$ egrep -nr "/usr/"
> > commands/svnserve:9:$svnserve ||= "/usr/bin/svnserve -r /var/svn/ -t --tunnel-user=%u";
> > lib/Gitolite/Test/Tsh.pm:42:# path when cwd is [...] at /usr/share/perl5/File/Temp.pm line 902".
> > lib/Gitolite/Hooks/PostUpdate.pm:62:#!/usr/bin/perl
> > lib/Gitolite/Hooks/Update.pm:158:#!/usr/bin/perl
> > lib/Gitolite/Cache.pm:127:    open( REDIS, "|-", "/usr/sbin/redis-server", "-" ) or die "start redis server fail
> > ed: $!";
> 
> …is now…
> 
> --8<---------------cut here---------------start------------->8---
> share/gitolite/lib/Gitolite/Test/Tsh.pm:42:# path when cwd is [...] at /usr/share/perl5/File/Temp.pm line 902".
> share/gitolite/lib/Gitolite/Cache.pm:127:    open( REDIS, "|-", "/usr/sbin/redis-server", "-" ) or die "start redis server failed: $!";
> share/gitolite/commands/svnserve:9:$svnserve ||= "/usr/bin/svnserve -r /var/svn/ -t --tunnel-user=%u";
> --8<---------------cut here---------------end--------------->8---
> 
> …but…
> 
> > The parts I want to fix as my immediately affect every user, are in
> > the directory "lib/Gitolite/Hooks/", I have no idea about redis, but
> > I think there should be a reference to /gnu/store/ reddis and not
> > "/usr/sbin/redis-server". Different problem, related bug.. This can be
> > solved in a commit after this bug.
> 
> …the package redis is not a dependency of gitolite.  Therefore, the
> question is: is our Gitolite package working with Redis?  Even using the
> /usr/bin one?  Idem for SVN.
> 
> Otherwise, I am favor to remove the 2 “problematic” references.   WDYT?

Or change it to search the $PATH for the binary, so it would just be
'redis-server' or 'svnserve'

-- 
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 --]

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

[zimoun]

Hi Efraim,

On Wed, 12 Jan 2022 at 20:07, Efraim Flashner <efraim@flashner.co.il> wrote:

>> …the package redis is not a dependency of gitolite.  Therefore, the
>> question is: is our Gitolite package working with Redis?  Even using the
>> /usr/bin one?  Idem for SVN.
>>
>> Otherwise, I am favor to remove the 2 “problematic” references.   WDYT?
>
> Or change it to search the $PATH for the binary, so it would just be
> 'redis-server' or 'svnserve'

Is our Gitolite package working with Redis?  If not, why try to fix. ;-)


Cheers,
simon

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

[zimoun]

Hi,

On Thu, 03 Feb 2022 at 03:49, zimoun <zimon.toutoune@gmail.com> wrote:
> On Wed, 12 Jan 2022 at 20:07, Efraim Flashner <efraim@flashner.co.il> wrote:
>
>>> …the package redis is not a dependency of gitolite.  Therefore, the
>>> question is: is our Gitolite package working with Redis?  Even using the
>>> /usr/bin one?  Idem for SVN.
>>>
>>> Otherwise, I am favor to remove the 2 “problematic” references.   WDYT?
>>
>> Or change it to search the $PATH for the binary, so it would just be
>> 'redis-server' or 'svnserve'
>
> Is our Gitolite package working with Redis?  If not, why try to fix. ;-)

What is the status of this old bug [1]?  Is it actionable?  If yes, what
is the action?  If no, let close it. :-)  WDYT?


1: <http://issues.guix.gnu.org/issue/25957>


Cheers,
simon

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

[Maxime Devos]

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

zimoun schreef op wo 23-03-2022 om 11:45 [+0100]:
> > On Wed, 12 Jan 2022 at 20:07, Efraim Flashner
> > <efraim@flashner.co.il> wrote:
> > 
> > > > …the package redis is not a dependency of gitolite.  Therefore,
> > > > the
> > > > question is: is our Gitolite package working with Redis?  Even
> > > > using the
> > > > /usr/bin one?  Idem for SVN.
> > > > 
> > > > Otherwise, I am favor to remove the 2 “problematic”
> > > > references.   WDYT?
> > > 
> > > Or change it to search the $PATH for the binary, so it would just
> > > be
> > > 'redis-server' or 'svnserve'
> > 
> > Is our Gitolite package working with Redis?  If not, why try to
> > fix. ;-)
> 
> What is the status of this old bug [1]?  Is it actionable?  If yes,
> what
> is the action?  If no, let close it. :-)  WDYT?

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?).

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

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

[Efraim Flashner]

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

On Wed, Mar 23, 2022 at 01:44:29PM +0100, Maxime Devos wrote:
> zimoun schreef op wo 23-03-2022 om 11:45 [+0100]:
> > > On Wed, 12 Jan 2022 at 20:07, Efraim Flashner
> > > <efraim@flashner.co.il> wrote:
> > > 
> > > > > …the package redis is not a dependency of gitolite.  Therefore,
> > > > > the
> > > > > question is: is our Gitolite package working with Redis?  Even
> > > > > using the
> > > > > /usr/bin one?  Idem for SVN.
> > > > > 
> > > > > Otherwise, I am favor to remove the 2 “problematic”
> > > > > references.   WDYT?
> > > > 
> > > > Or change it to search the $PATH for the binary, so it would just
> > > > be
> > > > 'redis-server' or 'svnserve'
> > > 
> > > Is our Gitolite package working with Redis?  If not, why try to
> > > fix. ;-)
> > 
> > What is the status of this old bug [1]?  Is it actionable?  If yes,
> > what
> > is the action?  If no, let close it. :-)  WDYT?
> 
> 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.
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.

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.

-- 
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 --]

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

[Thompson, David]

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.

> 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.

> 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.

- Dave

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

[Efraim Flashner]

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

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

> > 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.

> > 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.

> - Dave

Interestingly, I almost have a working ghc-8.6 for aarch64 after all
these years.

-- 
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 --]

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

[Thompson, David]

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.  :)

- Dave

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

[zimoun]

Hi,

On jeu., 01 sept. 2022 at 10:41, "Thompson, David" <dthompson2@worcester.edu> wrote:

> Thanks, pushed as commit c053dfa52dc778eb3d965f58a85c435ae7fab0dd.

Cool!  Thank you.


Cheers,
simon

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

[Efraim Flashner]

[-- 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 --]

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

[Thompson, David]

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.

- Dave

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

[Efraim Flashner]

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

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.

-- 
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 --]

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

[Thompson, David]

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.

- Dave

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

[Thompson, David]

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

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

[-- Attachment #2: 0001-gnu-gitolite-Wrap-programs-instead-of-using-propagat.patch --]
[-- Type: text/x-patch, Size: 2261 bytes --]

From 413f2d28aa8bea2274b74c2b574fb9f8bf9c16ba Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Fri, 2 Sep 2022 14:33:01 -0400
Subject: [PATCH] gnu: gitolite: Wrap programs instead of using propagated
 inputs.

* gnu/packages/version-control.scm (gitolite)[arguments]: Add git to wrapped
$PATH and additionally wrap gitolite-shell.
[inputs]: Add git and openssh.
[propagated-inputs]: Remove it.
---
 gnu/packages/version-control.scm | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/gnu/packages/version-control.scm b/gnu/packages/version-control.scm
index 15a9278fe8..1c775932c0 100644
--- a/gnu/packages/version-control.scm
+++ b/gnu/packages/version-control.scm
@@ -1573,17 +1573,15 @@ (define-public gitolite
                             (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))))))))))
+                        (for-each (lambda (file-name)
+                                    (wrap-program (string-append out file-name)
+                                      `("PATH" ":" prefix
+                                        ,(map (lambda (dir)
+                                                (string-append dir "/bin"))
+                                              (list out coreutils findutils git)))))
+                                  '("/bin/gitolite" "/bin/gitolite-shell"))))))))
     (inputs
-     (list bash-minimal perl coreutils findutils inetutils))
-    ;; git and openssh are propagated because trying to patch the source via
-    ;; regexp matching is too brittle and prone to false positives.
-    (propagated-inputs
-     (list git openssh))
+     (list bash-minimal git perl coreutils findutils inetutils openssh))
     (home-page "https://gitolite.com")
     (synopsis "Git access control layer")
     (description
-- 
2.37.2

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

[Efraim Flashner]

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

On Fri, Sep 02, 2022 at 03:58:09PM -0400, Thompson, David wrote:
> 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.

Overall it looks good to me. I was going to ask about inetutils and
openssh since they're not wrapping the binaries but I see their paths
are substituted in the 'patch-source phase.

LGTM!

> From 413f2d28aa8bea2274b74c2b574fb9f8bf9c16ba Mon Sep 17 00:00:00 2001
> From: David Thompson <dthompson2@worcester.edu>
> Date: Fri, 2 Sep 2022 14:33:01 -0400
> Subject: [PATCH] gnu: gitolite: Wrap programs instead of using propagated
>  inputs.
> 
> * gnu/packages/version-control.scm (gitolite)[arguments]: Add git to wrapped
> $PATH and additionally wrap gitolite-shell.
> [inputs]: Add git and openssh.
> [propagated-inputs]: Remove it.
> ---
>  gnu/packages/version-control.scm | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/gnu/packages/version-control.scm b/gnu/packages/version-control.scm
> index 15a9278fe8..1c775932c0 100644
> --- a/gnu/packages/version-control.scm
> +++ b/gnu/packages/version-control.scm
> @@ -1573,17 +1573,15 @@ (define-public gitolite
>                              (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))))))))))
> +                        (for-each (lambda (file-name)
> +                                    (wrap-program (string-append out file-name)
> +                                      `("PATH" ":" prefix
> +                                        ,(map (lambda (dir)
> +                                                (string-append dir "/bin"))
> +                                              (list out coreutils findutils git)))))
> +                                  '("/bin/gitolite" "/bin/gitolite-shell"))))))))
>      (inputs
> -     (list bash-minimal perl coreutils findutils inetutils))
> -    ;; git and openssh are propagated because trying to patch the source via
> -    ;; regexp matching is too brittle and prone to false positives.
> -    (propagated-inputs
> -     (list git openssh))
> +     (list bash-minimal git perl coreutils findutils inetutils openssh))
>      (home-page "https://gitolite.com")
>      (synopsis "Git access control layer")
>      (description
> -- 
> 2.37.2
> 


-- 
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 --]

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

[Thompson, David]

Hi Efraim,

On Sun, Sep 4, 2022 at 3:26 AM Efraim Flashner <efraim@flashner.co.il> wrote:
>
> Overall it looks good to me. I was going to ask about inetutils and
> openssh since they're not wrapping the binaries but I see their paths
> are substituted in the 'patch-source phase.
>
> LGTM!

Thanks! I made one minor tweak to sort the inputs list alphabetically
and pushed as commit 1aa46a7e29c5bd892219fe20fefb883d2103e29e.

I also pushed a follow-up commit
e4ccfcb22ad96e71ca4dfad95af5aa6229ed9869 that swaps out 'git' for
'git-minimal', saving about 75MiB in the package closure.

I think, technically speaking, this bug has been resolved.  There are
no longer /usr/bin, /usr/sbin, etc. references in our gitolite
package, so extensions should work as long as the user adds the
relevant packages to their user or system profile.  I will keep this
bug open for the moment, though, since I haven't gotten to the final
patch I said I would submit which will make those optional
dependencies easy to add via the gitolite service.  Stay tuned!

- Dave

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

[zimoun]

Hi,

On dim., 04 sept. 2022 at 09:26, "Thompson, David" <dthompson2@worcester.edu> wrote:

> Thanks! I made one minor tweak to sort the inputs list alphabetically
> and pushed as commit 1aa46a7e29c5bd892219fe20fefb883d2103e29e.

Cool!

> I also pushed a follow-up commit
> e4ccfcb22ad96e71ca4dfad95af5aa6229ed9869 that swaps out 'git' for
> 'git-minimal', saving about 75MiB in the package closure.

Neat!

> I think, technically speaking, this bug has been resolved.  There are
> no longer /usr/bin, /usr/sbin, etc. references in our gitolite
> package, so extensions should work as long as the user adds the
> relevant packages to their user or system profile.  I will keep this
> bug open for the moment, though, since I haven't gotten to the final
> patch I said I would submit which will make those optional
> dependencies easy to add via the gitolite service.  Stay tuned!

Ok, thanks for almost closing this old bugs. :-)


Cheers,
simon

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

[Thompson, David]

On Mon, Sep 5, 2022 at 5:33 AM zimoun <zimon.toutoune@gmail.com> wrote:
>
> > I also pushed a follow-up commit
> > e4ccfcb22ad96e71ca4dfad95af5aa6229ed9869 that swaps out 'git' for
> > 'git-minimal', saving about 75MiB in the package closure.
>
> Neat!

Unfortunately, it was so neat that it broke the system test for the
gitolite service so I had to revert it.

- Dave

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

[Thompson, David]

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

Hello again Simon and Efraim,

On Mon, Sep 5, 2022 at 5:33 AM zimoun <zimon.toutoune@gmail.com> wrote:
>
> Hi,
>
> On dim., 04 sept. 2022 at 09:26, "Thompson, David" <dthompson2@worcester.edu> wrote:
>
> > Thanks! I made one minor tweak to sort the inputs list alphabetically
> > and pushed as commit 1aa46a7e29c5bd892219fe20fefb883d2103e29e.
>
> Cool!
>
> > I also pushed a follow-up commit
> > e4ccfcb22ad96e71ca4dfad95af5aa6229ed9869 that swaps out 'git' for
> > 'git-minimal', saving about 75MiB in the package closure.
>
> Neat!
>
> > I think, technically speaking, this bug has been resolved.  There are
> > no longer /usr/bin, /usr/sbin, etc. references in our gitolite
> > package, so extensions should work as long as the user adds the
> > relevant packages to their user or system profile.  I will keep this
> > bug open for the moment, though, since I haven't gotten to the final
> > patch I said I would submit which will make those optional
> > dependencies easy to add via the gitolite service.  Stay tuned!
>
> Ok, thanks for almost closing this old bugs. :-)

Some news: I have updated the gitolite package to use G-expressions.
The package builds and the gitolite system test passes so I pushed
that change to master a little while ago.  That patch has made the
(hopefully) final step in this saga easier. The attached patch
introduces a 'make-gitolite' procedure that can be used to add
arbitrary packages to the wrappers for the gitolite and gitolite-shell
programs.  The return value of this procedure can be used in the
gitolite service configuration to enable the desired optional features
like Redis or git-annex.  The base package inputs are unchanged and
the gitolite system test still passes.

What do you think?

- Dave

[-- Attachment #2: 0001-gnu-version-control-Add-make-gitolite-procedure.patch --]
[-- Type: text/x-patch, Size: 3552 bytes --]

From 3f3e2d002cb8c740081d58e83b6e89236d11f15f Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Thu, 6 Oct 2022 08:45:48 -0400
Subject: [PATCH] gnu: version-control: Add make-gitolite procedure.

* gnu/packages/version-control.scm (make-gitolite): New procedure.
(gitolite): Use make-gitolite.
* doc/guix.texi (Gitolite service): Document how to use make-gitolite.
---
 doc/guix.texi                    | 15 ++++++++++++++-
 gnu/packages/version-control.scm | 15 +++++++++++----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 523711bdf6..533b12d738 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -35285,7 +35285,20 @@ Data type representing the configuration for @code{gitolite-service-type}.
 
 @table @asis
 @item @code{package} (default: @var{gitolite})
-Gitolite package to use.
+Gitolite package to use.  There are optional Gitolite dependencies that
+are not included in the default package, such as Redis and git-annex.
+These features can be made available by using the @code{make-gitolite}
+procedure in the @code{(gnu packages version-control}) module to produce
+a variant of Gitolite with the desired additional dependencies.
+
+The following code returns a package in which the Redis and git-annex
+programs can be invoked by Gitolite's scripts:
+
+@example
+(use-modules (gnu packages databases)
+             (gnu packages haskell-apps))
+(make-gitolite (list redis git-annex))
+@end example
 
 @item @code{user} (default: @var{git})
 User to use for Gitolite.  This will be user that you use when accessing
diff --git a/gnu/packages/version-control.scm b/gnu/packages/version-control.scm
index bd0ad70ce8..68358cc1d1 100644
--- a/gnu/packages/version-control.scm
+++ b/gnu/packages/version-control.scm
@@ -137,7 +137,8 @@ (define-module (gnu packages version-control)
   #:use-module (gnu packages tls)
   #:use-module (gnu packages)
   #:use-module (ice-9 match)
-  #:use-module (srfi srfi-1))
+  #:use-module (srfi srfi-1)
+  #:export (make-gitolite))
 
 (define-public breezy
   (package
@@ -1482,7 +1483,9 @@ (define-public git-test-sequence
 also walk each side of a merge and test those changes individually.")
       (license (license:x11-style "file://LICENSE")))))
 
-(define-public gitolite
+(define* (make-gitolite #:optional (extra-inputs '()))
+  "Make a gitolite package object with EXTRA-INPUTS added to the binary
+wrappers, to be used for optional gitolite extensions."
   (package
     (name "gitolite")
     (version "3.6.12")
@@ -1574,10 +1577,12 @@ (define-public gitolite
                                          (list #$output
                                                #$coreutils
                                                #$findutils
-                                               #$git)))))
+                                               #$git
+                                               #$@extra-inputs)))))
                              '("/bin/gitolite" "/bin/gitolite-shell")))))))
     (inputs
-     (list bash-minimal coreutils findutils git inetutils openssh perl))
+     (append (list bash-minimal coreutils findutils git inetutils openssh perl)
+             extra-inputs))
     (home-page "https://gitolite.com")
     (synopsis "Git access control layer")
     (description
@@ -1585,6 +1590,8 @@ (define-public gitolite
 control to Git repositories.")
     (license license:gpl2)))
 
+(define-public gitolite (make-gitolite))
+
 (define-public gitile
   (package
     (name "gitile")
-- 
2.37.2

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

[Thompson, David]

I made a small mistake in the last patch and forgot to include '(gnu
packages version-control)' in the 'use-modules' form in the example
within the manual.  I have fixed that in my local copy.

- Dave

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

[zimoun]

Hi Dave,

On Thu, 06 Oct 2022 at 09:26, "Thompson, David" <dthompson2@worcester.edu> wrote:

> Some news: I have updated the gitolite package to use G-expressions.
> The package builds and the gitolite system test passes so I pushed
> that change to master a little while ago.  That patch has made the
> (hopefully) final step in this saga easier. The attached patch
> introduces a 'make-gitolite' procedure that can be used to add
> arbitrary packages to the wrappers for the gitolite and gitolite-shell
> programs.  The return value of this procedure can be used in the
> gitolite service configuration to enable the desired optional features
> like Redis or git-annex.  The base package inputs are unchanged and
> the gitolite system test still passes.

Thank you for working on this!  Neat.

Well, your proposal LGTM although I do not see the difference between
’make-gitolite’ and a simple ’inherent’ for building a package variant.
Yeah, this make-gitolite is probably more handy.

I do not have a strong opinion on the matter. :-)

Cheers,
simon

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

[Thompson, David]

On Sat, Oct 8, 2022 at 11:16 AM zimoun <zimon.toutoune@gmail.com> wrote:
>
> Hi Dave,
>
> On Thu, 06 Oct 2022 at 09:26, "Thompson, David" <dthompson2@worcester.edu> wrote:
>
> > Some news: I have updated the gitolite package to use G-expressions.
> > The package builds and the gitolite system test passes so I pushed
> > that change to master a little while ago.  That patch has made the
> > (hopefully) final step in this saga easier. The attached patch
> > introduces a 'make-gitolite' procedure that can be used to add
> > arbitrary packages to the wrappers for the gitolite and gitolite-shell
> > programs.  The return value of this procedure can be used in the
> > gitolite service configuration to enable the desired optional features
> > like Redis or git-annex.  The base package inputs are unchanged and
> > the gitolite system test still passes.
>
> Thank you for working on this!  Neat.
>
> Well, your proposal LGTM although I do not see the difference between
> ’make-gitolite’ and a simple ’inherent’ for building a package variant.
> Yeah, this make-gitolite is probably more handy.

The reason for the constructor is so that extra packages can be easily
added to the gexp that calls wrap-program.  It would be much harder to
modify the package in this way without a helper procedure.

- Dave

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

[Thompson, David]

On Sat, Oct 8, 2022 at 9:40 PM Thompson, David <dthompson2@worcester.edu> wrote:
>
> On Sat, Oct 8, 2022 at 11:16 AM zimoun <zimon.toutoune@gmail.com> wrote:
> >
> > Hi Dave,
> >
> > On Thu, 06 Oct 2022 at 09:26, "Thompson, David" <dthompson2@worcester.edu> wrote:
> >
> > > Some news: I have updated the gitolite package to use G-expressions.
> > > The package builds and the gitolite system test passes so I pushed
> > > that change to master a little while ago.  That patch has made the
> > > (hopefully) final step in this saga easier. The attached patch
> > > introduces a 'make-gitolite' procedure that can be used to add
> > > arbitrary packages to the wrappers for the gitolite and gitolite-shell
> > > programs.  The return value of this procedure can be used in the
> > > gitolite service configuration to enable the desired optional features
> > > like Redis or git-annex.  The base package inputs are unchanged and
> > > the gitolite system test still passes.
> >
> > Thank you for working on this!  Neat.
> >
> > Well, your proposal LGTM although I do not see the difference between
> > ’make-gitolite’ and a simple ’inherent’ for building a package variant.
> > Yeah, this make-gitolite is probably more handy.
>
> The reason for the constructor is so that extra packages can be easily
> added to the gexp that calls wrap-program.  It would be much harder to
> modify the package in this way without a helper procedure.

Pushed as commit 966118da711506b04c11fbfcac9483d59ed2d912.  This bug
can finally be closed!

- Dave

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

[zimoun]

Hi Dave,

On Mon, 24 Oct 2022 at 17:21, "Thompson, David" <dthompson2@worcester.edu> wrote:

> Pushed as commit 966118da711506b04c11fbfcac9483d59ed2d912.  This bug
> can finally be closed!

\o/ Awesome!  Thank you!

Cheers,
simon