Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give support for hooks based on platform #1069

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rafael-santiago
Copy link

@rafael-santiago rafael-santiago commented Aug 19, 2021

The idea behind this commit can be useful for teams that share git-hooks into a custom directory and dealing with projects that must be developed, built, maintained on several different platforms.

This commit allows the execution of git hooks based on the current operating system. A "native hook" is defined in the form: hooks/hook-name_platform

Where platform must be equivalent to the content returned in sysname field in utsname struct when calling uname() [but all normalized in lowercase].

On Windows, independent of version, flavor, SP, whatever it is simply "windows".

When a native hook is not found the standard hook (.git/hook/hook-name), if found is executed, of course. In other words, the hook without a platform postfix (_yyz) is the standard hook. When native hook is not set as executable but standard is set, the
standard will be executed.

The main motivation of this extension is to reduce dependency of scripting languages, logical trinkets etc just to execute minor tasks during scm events that could be done natively but differently from a platform to another. Less dependencies, cleaner
repos: a small step for a better world for any software developer.

cc: "brian m. carlson" sandals@crustytoothpaste.net
cc: Jeff King peff@peff.net

The idea behind this commit can be useful for teams
that share git-hooks into a custom directory and
dealing with projects that must be developed,
built, maintained on several different platforms.

This commit allows the execution of git hooks
based on the current operating system.
A "native hook" is defined in the form:
    hooks/hook-name_platform
Where platform must be equivalent to the
content returned in sysname field in utsname
struct when calling uname() [but all normalized
in lowercase].

On Windows, independent of version, flavor, SP,
whatever it is simply "windows".

When a native hook is not found the standard
hook (.git/hook/hook-name), if found is executed,
of course. In other words, the hook without a
platform postfix (_yyz) is the standard hook.
When native hook is not set as executable but
standard is set, the standard will be executed.

The main motivation of this extension is to
reduce dependency of scripting languages,
logical trinkets etc just to execute minor
tasks during scm events that could be done
natively but differently from a platform
to another. Less dependencies, cleaner
repos: a small step for a better world
for any software developer.

Signed-off-by: Rafael Santiago <voidbrainvoid@tutanota.com>
@dscho
Copy link
Member

dscho commented Aug 19, 2021

Since this is a continuation of #1062, I'd like to point out that force-pushing the PR branch will update the PR, there is not actually a need for opening a new PR 😃

@rafael-santiago
Copy link
Author

rafael-santiago commented Aug 19, 2021

Since this is a continuation of #1062, I'd like to point out that force-pushing the PR branch will update the PR, there is not actually a need for opening a new PR 😃

The problem was that all git incantation to put mine commits at the top of fixed merged stuff from upstream was becoming tricky and I was not secure about not screw up other people work. So I chose the "lumberjack's solution" of squashing mine changes from my fork, reserve a patch from it, remove my fork and forking again from original git's repo and patch it. When I deleted my prior fork the related PR here was closed I was unable to open it again, so another "lumberjack's solution": open a new one... 😃

@rafael-santiago
Copy link
Author

/submit

@gitgitgadget-git
Copy link

Submitted as pull.1069.git.git.1629576007891.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git pr-git-1069/rafael-santiago/master-v1

To fetch this version to local tag pr-git-1069/rafael-santiago/master-v1:

git fetch --no-tags https://github.com/gitgitgadget/git tag pr-git-1069/rafael-santiago/master-v1

@gitgitgadget-git
Copy link

On the Git mailing list, "brian m. carlson" wrote (reply to this):


--mSbqCZqt8HM1OZDv
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On 2021-08-21 at 20:00:07, Rafael Santiago via GitGitGadget wrote:
> From: rafael-santiago <voidbrainvoid@tutanota.com>
>=20
> The idea behind this commit can be useful for teams
> that share git-hooks into a custom directory and
> dealing with projects that must be developed,
> built, maintained on several different platforms.
>=20
> This commit allows the execution of git hooks
> based on the current operating system.
> A "native hook" is defined in the form:
>     hooks/hook-name_platform
> Where platform must be equivalent to the
> content returned in sysname field in utsname
> struct when calling uname() [but all normalized
> in lowercase].
>
> On Windows, independent of version, flavor, SP,
> whatever it is simply "windows".

I'm not sure that this is going to work out very well.  The MINGW
environment used by Git for Windows and Cygwin are quite different.  I
would fully expect to write shell scripts and Unix tooling in Cygwin,
whereas users using Git for Windows might not want that at all.  That
also doesn't take into effect using Git for Windows in WSL, which
introduces some interesting logistical challenges.

In addition, I have a few concerns about the grouping of Linux
altogether.  While in many cases it is possible to write tooling that
works natively across Linux distros, most binaries will not.  Therefore,
binary hooks that might run fine on Debian would fail on a Fedora or Red
Hat system, especially if those binaries link to any of a number of
different shared libraries (e.g., OpenSSL).

There's also work to move hooks into the config and out of the hooks
directory, and I don't think this will mesh well with it.

> The main motivation of this extension is to
> reduce dependency of scripting languages,
> logical trinkets etc just to execute minor
> tasks during scm events that could be done
> natively but differently from a platform
> to another. Less dependencies, cleaner
> repos: a small step for a better world
> for any software developer.

Is there a reason that the proper hooks couldn't be copied or symlinked
into place with a script?  I think that would resolve this concern with
a lot less work.
--=20
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

--mSbqCZqt8HM1OZDv
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.3.1 (GNU/Linux)

iHUEABYKAB0WIQQILOaKnbxl+4PRw5F8DEliiIeigQUCYSF1GAAKCRB8DEliiIei
gQReAP4/kvvrpzTtAezpmHmjNVC4JM6I/AFXEcpLBg5y8bExtwEA1g526Pw3YLLg
Yg1AP2JUiSxagfbnUynmNaiSUMVTcQo=
=LArw
-----END PGP SIGNATURE-----

--mSbqCZqt8HM1OZDv--

@gitgitgadget-git
Copy link

User "brian m. carlson" <sandals@crustytoothpaste.net> has been added to the cc: list.

@gitgitgadget-git
Copy link

On the Git mailing list, Rafael Santiago wrote (reply to this):

In my opinion "binary hooks" (hooks that execute specific binaries not present in the system as a default tool) should be versioned and built as a support tool into the repository or in the worst case downloaded from somewhere, even because versioning binaries into git repos is considered a bad practice that could make bloated repos.

The point is that in many cases a dependency with a script language is created only to make the hook actions portable from a platform to other, but what this script in its essence does is a thing that could be done with basic tools delivered with the current operating system.

There is no problem on using cygwin on windows, you should use standard hook and do all the effort to make it unique for cygwin environments and true unix boxes (in other words: you would continue doing what you are doing, because it attends yours requirements). Notice that everything that have been working will stay working as before. Anyway, if cygwin becomes a point of incompatibility at some point, you could use the "_windows" version by coding your "cygwin script" there.
Maybe this would add more possibilities in git-hook tooling by making them less plastered, I think.

Rafael Santiago
-- 
21 de ago. de 2021 18:50 por sandals@crustytoothpaste.net:

> On 2021-08-21 at 20:00:07, Rafael Santiago via GitGitGadget wrote:
>
>> From: rafael-santiago <voidbrainvoid@tutanota.com>
>>
>> The idea behind this commit can be useful for teams
>> that share git-hooks into a custom directory and
>> dealing with projects that must be developed,
>> built, maintained on several different platforms.
>>
>> This commit allows the execution of git hooks
>> based on the current operating system.
>> A "native hook" is defined in the form:
>>  hooks/hook-name_platform
>> Where platform must be equivalent to the
>> content returned in sysname field in utsname
>> struct when calling uname() [but all normalized
>> in lowercase].
>>
>> On Windows, independent of version, flavor, SP,
>> whatever it is simply "windows".
>>
>
> I'm not sure that this is going to work out very well.  The MINGW
> environment used by Git for Windows and Cygwin are quite different.  I
> would fully expect to write shell scripts and Unix tooling in Cygwin,
> whereas users using Git for Windows might not want that at all.  That
> also doesn't take into effect using Git for Windows in WSL, which
> introduces some interesting logistical challenges.
>
> In addition, I have a few concerns about the grouping of Linux
> altogether.  While in many cases it is possible to write tooling that
> works natively across Linux distros, most binaries will not.  Therefore,
> binary hooks that might run fine on Debian would fail on a Fedora or Red
> Hat system, especially if those binaries link to any of a number of
> different shared libraries (e.g., OpenSSL).
>
> There's also work to move hooks into the config and out of the hooks
> directory, and I don't think this will mesh well with it.
>
>> The main motivation of this extension is to
>> reduce dependency of scripting languages,
>> logical trinkets etc just to execute minor
>> tasks during scm events that could be done
>> natively but differently from a platform
>> to another. Less dependencies, cleaner
>> repos: a small step for a better world
>> for any software developer.
>>
>
> Is there a reason that the proper hooks couldn't be copied or symlinked
> into place with a script?  I think that would resolve this concern with
> a lot less work.
> -- 
> brian m. carlson (he/him or they/them)
> Toronto, Ontario, CA
>

@gitgitgadget-git
Copy link

On the Git mailing list, "brian m. carlson" wrote (reply to this):


--vo3ImlIhhqRttfNQ
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On 2021-08-21 at 23:11:27, Rafael Santiago wrote:
> In my opinion "binary hooks" (hooks that execute specific binaries not
> present in the system as a default tool) should be versioned and built
> as a support tool into the repository or in the worst case downloaded
> from somewhere, even because versioning binaries into git repos is
> considered a bad practice that could make bloated repos.

Yes, I agree binary hooks should not be checked into the repository.

> The point is that in many cases a dependency with a script language is
> created only to make the hook actions portable from a platform to
> other, but what this script in its essence does is a thing that could
> be done with basic tools delivered with the current operating system.

Then, in general, it can be done in a shell script containing an if-then
statement per platform using the native tools, so I'm not seeing the
particular reason that this series is necessary if the hooks being
executed aren't binaries.  All systems on which Git runs must contain a
POSIX-compatible shell.

Can you explain the rationale for your proposal in more detail so that
we can understand why this change is necessary?  Typically this is done
in the commit message, but I don't think I understand why you want to do
this.

> There is no problem on using cygwin on windows, you should use
> standard hook and do all the effort to make it unique for cygwin
> environments and true unix boxes (in other words: you would continue
> doing what you are doing, because it attends yours requirements).
> Notice that everything that have been working will stay working as
> before. Anyway, if cygwin becomes a point of incompatibility at some
> point, you could use the "_windows" version by coding your "cygwin
> script" there.

Right, my point is that your commit message proposes using "windows" for
Cygwin.  The patch doesn't, but your commit message says that every
version of Windows is considered "windows".
--=20
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

--vo3ImlIhhqRttfNQ
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.3.1 (GNU/Linux)

iHUEABYKAB0WIQQILOaKnbxl+4PRw5F8DEliiIeigQUCYSLKrQAKCRB8DEliiIei
gTcaAQD+hOCkNiNsSPMRbscxwgy1xaVrRrnHepfwL5tOukqtnwEA2IAadlxWUN3K
Fs0dTkl7AoGqoeLEkfAIKR1NHIfVVwg=
=qECC
-----END PGP SIGNATURE-----

--vo3ImlIhhqRttfNQ--

@gitgitgadget-git
Copy link

On the Git mailing list, Rafael Santiago wrote (reply to this):

Well, you are taking into consideration that every git user that needs to
automate some stuff during a scm event will do it from a shell
(I meant true shell or a mocked up one such as cygwin, msys, etc)
and it is true, by design.

However, not all git users (out unix) uses git from "bash-like"
environments. I know people that prefers using it from a well-cooked
IDE by clicking and dragging things or even from command prompt when
on Windows. Those people are not able to handle some scm event
automation as unix users because git hook in its essence presupposes
the existence/necessity of a powerful shell (okay, it is possible to
put a shebang and call a batch file on windows, maybe, but it is a
little clumsy, in my opinion). On Windows, users can do a bunch of stuff
just by using the ready to go powershell, but open an if/else on a bash
script to run a cygwin instance by calling powershell from there is not a
good and clean solution for this type of user. Presupposing shell for git,
limitates the idea behind the scm event handling with hooks, because
currently it is strongly dependent from shell to work on every git
supported platform.

The idea of having hooks being executed by platform would be the first
step to give support to execute commands on scm events without
obligating users out of a unix have a shell interpreter to access
native stuff. Currently, this commit does not implement it but would be
possible to do and in a  less noisy way for all unix-like stuff. I am not sure
but currently a _windows hook out from cygwin would result on a spawn
error, would not?

Git hooks are useful features but would be more useful if it breaked up
the shell jail. It could make git much more integrated with the current
platform.  Being possible to make it powerful as it is on a unix even on
a total different platform as Windows, let's say.

For sure, this commit are not a "panacea" but intends to start making
git-hooks more independent from 3rd party software to work on as expected,
on every platform that a git-repo is expected to be handled.

I hope I was clearer from this time.

Rafael Santiago
--
22 de ago. de 2021 19:07 por sandals@crustytoothpaste.net:

> On 2021-08-21 at 23:11:27, Rafael Santiago wrote:
>
>> In my opinion "binary hooks" (hooks that execute specific binaries not
>> present in the system as a default tool) should be versioned and built
>> as a support tool into the repository or in the worst case downloaded
>> from somewhere, even because versioning binaries into git repos is
>> considered a bad practice that could make bloated repos.
>>
>
> Yes, I agree binary hooks should not be checked into the repository.
>
>> The point is that in many cases a dependency with a script language is
>> created only to make the hook actions portable from a platform to
>> other, but what this script in its essence does is a thing that could
>> be done with basic tools delivered with the current operating system.
>>
>
> Then, in general, it can be done in a shell script containing an if-then
> statement per platform using the native tools, so I'm not seeing the
> particular reason that this series is necessary if the hooks being
> executed aren't binaries.  All systems on which Git runs must contain a
> POSIX-compatible shell.
>
> Can you explain the rationale for your proposal in more detail so that
> we can understand why this change is necessary?  Typically this is done
> in the commit message, but I don't think I understand why you want to do
> this.
>
>> There is no problem on using cygwin on windows, you should use
>> standard hook and do all the effort to make it unique for cygwin
>> environments and true unix boxes (in other words: you would continue
>> doing what you are doing, because it attends yours requirements).
>> Notice that everything that have been working will stay working as
>> before. Anyway, if cygwin becomes a point of incompatibility at some
>> point, you could use the "_windows" version by coding your "cygwin
>> script" there.
>>
>
> Right, my point is that your commit message proposes using "windows" for
> Cygwin.  The patch doesn't, but your commit message says that every
> version of Windows is considered "windows".
> -- 
> brian m. carlson (he/him or they/them)
> Toronto, Ontario, CA
>

@gitgitgadget-git
Copy link

On the Git mailing list, Jeff King wrote (reply to this):

On Sun, Aug 22, 2021 at 10:07:41PM +0000, brian m. carlson wrote:

> > The point is that in many cases a dependency with a script language is
> > created only to make the hook actions portable from a platform to
> > other, but what this script in its essence does is a thing that could
> > be done with basic tools delivered with the current operating system.
> 
> Then, in general, it can be done in a shell script containing an if-then
> statement per platform using the native tools, so I'm not seeing the
> particular reason that this series is necessary if the hooks being
> executed aren't binaries.  All systems on which Git runs must contain a
> POSIX-compatible shell.

This is my gut feeling, too (whether users know it or not, even on
Windows most programs specified by config are being run by the shell).

However, I do think there is room for Git to make this case a bit
easier: conditional config includes. Once we are able to specify hooks
via config (which is being worked on elsewhere), then we ought to be
able to implement an includeIf like:

  [includeIf "uname_s:linux"]
  path = linux-hooks.config
  [includeIf "uname_s:windows"]
  path = windows-hooks.config

The advantage being that this could apply to _all_ config, and not just
hooks.

It may still require fighting over which values should match on cygwin. ;)

-Peff

@gitgitgadget-git
Copy link

User Jeff King <peff@peff.net> has been added to the cc: list.

@gitgitgadget-git
Copy link

On the Git mailing list, Junio C Hamano wrote (reply to this):

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> Then, in general, it can be done in a shell script containing an if-then
> statement per platform using the native tools, so I'm not seeing the
> particular reason that this series is necessary if the hooks being
> executed aren't binaries.  All systems on which Git runs must contain a
> POSIX-compatible shell.

When we start defining our hooks in the configuration, this may fit
with the conditional inclusion of configuration files.  Current
conditions only can depend on where the repository is, but it is
easy to imagine that a conditional inclusion based on the value of
the configuration variable, so

	[includeIf "var:dev.host=mac"]
		path = ...
	[includeIf "var:dev.host=win"]
		path = ...

might be a way to say "if the dev.host configuration (presumably set
in somewhere like /etc/gitconfig or ~/.gitconfig) is set to this
value, take the configuration from the specified path.

It is up to the project to define the variable they use to switch
on; some project may ship with a set of hooks that can be used on
both windows and cygwin at the same time, in which case they do not
need the distinction between the two, and some other project may
care about the distinction.  Git does not have to impose or enforce
any policy about the granularity of what "the platform" is, with
such a scheme.

@gitgitgadget-git
Copy link

On the Git mailing list, Junio C Hamano wrote (reply to this):

Jeff King <peff@peff.net> writes:

> On Sun, Aug 22, 2021 at 10:07:41PM +0000, brian m. carlson wrote:
>
>> > The point is that in many cases a dependency with a script language is
>> > created only to make the hook actions portable from a platform to
>> > other, but what this script in its essence does is a thing that could
>> > be done with basic tools delivered with the current operating system.
>> 
>> Then, in general, it can be done in a shell script containing an if-then
>> statement per platform using the native tools, so I'm not seeing the
>> particular reason that this series is necessary if the hooks being
>> executed aren't binaries.  All systems on which Git runs must contain a
>> POSIX-compatible shell.
>
> This is my gut feeling, too (whether users know it or not, even on
> Windows most programs specified by config are being run by the shell).
>
> However, I do think there is room for Git to make this case a bit
> easier: conditional config includes. Once we are able to specify hooks
> via config (which is being worked on elsewhere), then we ought to be
> able to implement an includeIf like:
>
>   [includeIf "uname_s:linux"]
>   path = linux-hooks.config
>   [includeIf "uname_s:windows"]
>   path = windows-hooks.config
>
> The advantage being that this could apply to _all_ config, and not just
> hooks.

Heh, it seems great minds think alike.

@gitgitgadget-git
Copy link

On the Git mailing list, Jeff King wrote (reply to this):

On Mon, Aug 23, 2021 at 10:59:28AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Sun, Aug 22, 2021 at 10:07:41PM +0000, brian m. carlson wrote:
> >
> >> > The point is that in many cases a dependency with a script language is
> >> > created only to make the hook actions portable from a platform to
> >> > other, but what this script in its essence does is a thing that could
> >> > be done with basic tools delivered with the current operating system.
> >> 
> >> Then, in general, it can be done in a shell script containing an if-then
> >> statement per platform using the native tools, so I'm not seeing the
> >> particular reason that this series is necessary if the hooks being
> >> executed aren't binaries.  All systems on which Git runs must contain a
> >> POSIX-compatible shell.
> >
> > This is my gut feeling, too (whether users know it or not, even on
> > Windows most programs specified by config are being run by the shell).
> >
> > However, I do think there is room for Git to make this case a bit
> > easier: conditional config includes. Once we are able to specify hooks
> > via config (which is being worked on elsewhere), then we ought to be
> > able to implement an includeIf like:
> >
> >   [includeIf "uname_s:linux"]
> >   path = linux-hooks.config
> >   [includeIf "uname_s:windows"]
> >   path = windows-hooks.config
> >
> > The advantage being that this could apply to _all_ config, and not just
> > hooks.
> 
> Heh, it seems great minds think alike.

One important distinction in what you wrote is that you're expecting the
user to set dev.host once. That nicely sidesteps any question of "how
does Git label each platform?", but it does mean the user has to do that
setup manually (which maybe is amortized across many repos, but in
practice for many people I suspect is no better than them setting up the
correct "include" in the first place).

I hoped that by calling it "uname_s", it would be clear it was the same
as "uname -s", and then we could blame any naming confusion on the OS. :)

But even if it is not used for this particular application, I think the
[includeIf "var:..."] you proposed might be a reasonable thing to
support.

-Peff

@sancarn
Copy link

sancarn commented Sep 23, 2021

This would be pretty big for me, look forward to cross-platform hooks being easier in the near future 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants