Gitea makes update hooks but doesn't use them

[zeripath]

• Gitea version (or commit ref): master

Description

gitea/models/repo.go

Line 979 in 3e166bd

fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' update $1 $2 $3\n", setting.ScriptType, setting.AppPath, setting.CustomConf),

creates update hooks however gitea does not use this hook and the equivalent hook is essentially empty except for reading the settings file.

This would be of little consequence, except update is run for every reference and this means the settings are read for every reference! This means that pushes that involve a large number of references are unnecessarily slowed.

[zeripath]

I'm gonna reopen this because #9496 doesn't actually remove these hooks anymore.

[lunny]

OKay. I think it's clear as @lafriks said #9496 (comment).

pre_receive hook will be invoked before any objects stored on git repository.
update hook will be invoked after that but before change the branch/tag to that new refs.

Currently, we just need to use pre_receive to protect branches and post_receive to update something.

[zeripath]

Almost everything that can be done in an update hook can be done at pre-receive - the only things being different are that pre-receive cannot make references whereas it appears that update can. Despite its name pre-receive is called after the packs have been received and are present on the server in the quarantine directory.

Update hooks have to run once per received reference this makes them a rather expensive hook especially for migrations where many references could be updated at once.

Therefore apart from that slight difference which could easily be done at post-receive I do not believe that there could ever be a reason for having a gitea update hook and therefore I believe we should simply remove it.

I would even go further and state that we should not support update hooks. Their only power above that of pre-receive is to do something that would lead to gitea not knowing about branches in its own repository.

[zeripath]

For reference looking at the Git source code:

You'd think that pre-receive is run before the packs are uploaded... but that's not what the documentation implies nor what this code does, take a look at execute_commands in builtin/receive-pack.c:

https://github.com/git/git/blob/0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6/builtin/receive-pack.c#L1459-L1531

pre-receive is called at

https://github.com/git/git/blob/0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6/builtin/receive-pack.c#L1498

The packs are received at:

https://github.com/git/git/blob/0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6/builtin/receive-pack.c#L2000

which is in the function that calls execute_commands and eventually the pre-receive hooks.

[lunny]

@zeripath see the comments on https://github.com/git/git/blob/0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6/builtin/receive-pack.c#L1506-L1517 . The packs uploaded on temp directory before pre-received hook.

[zeripath]

Yes. The directories are specified as GIT_QUARANTINE_PATH.

All of the objects are on the server already and can be queried with an appropriate look up in the quarantine dir and the alternative dir as appropriate.

We do this already in the pre-receive hook:

gitea/routers/private/hook.go

Lines 65 to 78 in 145c1ea

	if opts.GitAlternativeObjectDirectories != "" {
	env = append(env,
	private.GitAlternativeObjectDirectories+"="+opts.GitAlternativeObjectDirectories)
	}
	if opts.GitObjectDirectory != "" {
	env = append(env,
	private.GitObjectDirectory+"="+opts.GitObjectDirectory)
	}
	if opts.GitQuarantinePath != "" {
	env = append(env,
	private.GitQuarantinePath+"="+opts.GitQuarantinePath)
	}
	output, err := git.NewCommand("rev-list", "--max-count=1", oldCommitID, "^"+newCommitID).RunInDirWithEnv(repo.RepoPath(), env)

We will never need a update hook.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. I am here to help clear issues left open even if solved or waiting for more insight. This issue will be closed if no further activity occurs during the next 2 weeks. If the issue is still valid just add a comment to keep it alive. Thank you for your contributions.

[stale]

This issue has been automatically closed because of inactivity. You can re-open it if needed.