no git hook executed during &#34;gitea squash and merge&#34; #2877

7fb09f035a/models/pull.go (L570)

@apricote commented on GitHub (Feb 8, 2019):

From the official documentation on Git Hooks:

[Server-Side Hooks] run before and after pushes to the server.

Emphasis mine.

This explains why it works when you manually do a git merge and git push.

When the Gitea Server performs a merge, it checks out your repository into a temporary location, runs git merge and then git pushes the new commit back into the main repository:

So it looks like there is an actual bug here.

I am not sure if we want to support this though, as the error from post-receive would be displayed as a 500 error page and the error message probably only includes the exit code and not your stderr output.

References:

The error that is thrown if the hook fails: https://sourcegraph.com/github.com/golang/go@go1.11.4/-/blob/src/os/exec/exec.go#L483:1

@apricote commented on GitHub (Feb 8, 2019): From the official documentation on [Git Hooks](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks): > [Server-Side Hooks] run before and after **pushes** to the server. Emphasis mine. This explains why it works when you manually do a `git merge` and `git push`. When the Gitea Server performs a merge, it checks out your repository into a temporary location, runs `git merge` and then `git push`es the new commit back into the main repository: https://github.com/go-gitea/gitea/blob/7fb09f035ac8b10191c154ed4395a8e02d3c5e8d/models/pull.go#L570 So it looks like there is an actual bug here. I am not sure if we want to support this though, as the error from `post-receive` would be displayed as a `500` error page and the error message probably only includes the `exit code` and not your stderr output. --- References: - The error that is thrown if the hook fails: https://sourcegraph.com/github.com/golang/go@go1.11.4/-/blob/src/os/exec/exec.go#L483:1

GiteaMirror commented

@uwebartels commented on GitHub (Feb 8, 2019):

Thanks for your response.
There is no 500 error page and no log entry which shows a hint of the post-receive hook.
What I'm wondering is, why the push in pull.go:570 is not triggering the post-receive hook. Cause when I merge the branch via cmd I also merge locally and then push the commited result. At least it would be nice to add some debug output.

Best...
Uwe

@uwebartels commented on GitHub (Feb 8, 2019): Thanks for your response. There is no 500 error page and no log entry which shows a hint of the post-receive hook. What I'm wondering is, why the push in pull.go:570 is not triggering the post-receive hook. Cause when I merge the branch via cmd I also merge locally and then push the commited result. At least it would be nice to add some debug output. Best... Uwe

GiteaMirror commented

@apricote commented on GitHub (Feb 8, 2019):

Yes, the git push in line 570 should trigger the post-receive hook. Currently I can not confirm that this is or is not happening.
If it would trigger the post-receive hook, you would still not get any useful output, per the reasons described in my first comment.

Can you maybe add some HTTP call to your post-receive script to a server that you control (local nginx on the machine?) so you can verify that the post-receive is not executed.

@apricote commented on GitHub (Feb 8, 2019): 1) Yes, the `git push` in line 570 should trigger the `post-receive` hook. Currently I can not confirm that this is or is not happening. 2) If it would trigger the `post-receive` hook, you would still not get any useful output, per the reasons described in my first comment. --- Can you maybe add some HTTP call to your `post-receive` script to a server that you control (local nginx on the machine?) so you can verify that the `post-receive` is not executed.

GiteaMirror commented

@zeripath commented on GitHub (Feb 8, 2019):

The prereceive and post-receive hooks won't run because the environment variables aren't being set properly.

Well aren't being set at all.

@zeripath commented on GitHub (Feb 8, 2019): The prereceive and post-receive hooks won't run because the environment variables aren't being set properly. Well aren't being set at all.

GiteaMirror commented

@zeripath commented on GitHub (Feb 8, 2019):

Even in my make upload not clone branch (#5702) they won't run because they don't run in the editor currently.

Personally I think they probably should run.

See my pr for refactoring the editor to use LFS. That goes almost all the way but doesn't set the final key variable to make them run SSH_ORIGINAL_COMMAND. (Which can be set to anything it just has to be set.)

@zeripath commented on GitHub (Feb 8, 2019): Even in my make upload not clone branch (#5702) they won't run because they don't run in the editor currently. Personally I think they probably should run. See my pr for refactoring the editor to use LFS. That goes almost all the way but doesn't set the final key variable to make them run SSH_ORIGINAL_COMMAND. (Which can be set to anything it just has to be set.)

GiteaMirror commented

@lunny commented on GitHub (Feb 9, 2019):

@zeripath did you mean your PR will fix this bug?

@lunny commented on GitHub (Feb 9, 2019): @zeripath did you mean your PR will fix this bug?

GiteaMirror commented

@zeripath commented on GitHub (Feb 9, 2019):

Nope, it won't. I meant that as it currently stands it won't because that's current behaviour but I set almost all of the necessary environment variables to make them run.

I do think we should probably run the hooks but it looks like they were deliberately set up so that they wouldn't run for internal pushes etc.

So, there are two ways of fixing this:

Add the appropriate environment variables to internal pushes thus granting them hooks by default. We then have to account for the possibility that the git push might fail and be prepared to send that error back to the user.
Replicate the functionality of the hooks in our internal code.

I suspect that they original idea was that we would end up replicating the functionality of the hooks in the internal code, but of course that's code duplication so it gets missed/forgotten.

Therefore we should set it so that the hooks are run.

@zeripath commented on GitHub (Feb 9, 2019): Nope, it won't. I meant that as it currently stands it won't because that's current behaviour but I set almost all of the necessary environment variables to make them run. I do think we should probably run the hooks but it looks like they were deliberately set up so that they wouldn't run for internal pushes etc. So, there are two ways of fixing this: * Add the appropriate environment variables to internal pushes thus granting them hooks by default. We then have to account for the possibility that the git push might fail and be prepared to send that error back to the user. * Replicate the functionality of the hooks in our internal code. I suspect that they original idea was that we would end up replicating the functionality of the hooks in the internal code, but of course that's code duplication so it gets missed/forgotten. Therefore we should set it so that the hooks are run.

GiteaMirror commented

@zeripath commented on GitHub (Feb 9, 2019):

For clarity; in order for the hooks to be run you need to set the following environment variables:

SSH_ORIGINAL_COMMAND
GITEA_REPO_ID (models.ProtectedBranchRepoID)
GITEA_REPO_NAME (models.EnvRepoName)
GITEA_REPO_USER_NAME (models.EnvRepoUsername)
GITEA_REPO_IS_WIKI (models.EnvRepoIsWiki)
GITEA_PUSHER_NAME (models.EnvPusherName)
GITEA_PUSHER_ID (models.EnvPusherID)

That means that if you call a git command to commit or push you should probably set:

isWiki := "false"
if strings.HasSuffix(repo.Name, ".wiki") {
	isWiki = "true"
}
sig := doer.NewGitSig()

env := append(os.Environ(),
		"SSH_ORIGINAL_COMMAND=internal",
		"GIT_AUTHOR_NAME="+sig.Name,
		"GIT_AUTHOR_EMAIL="+sig.Email,
		"GIT_COMMITTER_NAME="+sig.Name,
		"GIT_COMMITTER_EMAIL="+sig.Email,
		models.EnvRepoName+"="+t.repo.Name,
		models.EnvRepoUsername+"="+repo.OwnerName,
		models.EnvRepoIsWiki+"="+isWiki,
		models.EnvPusherName+"="+doer.Name,
		models.EnvPusherID+"="+fmt.Sprintf("%d", doer.ID),
		models.ProtectedBranchRepoID+"="+fmt.Sprintf("%d", t.repo.ID),
)

@zeripath commented on GitHub (Feb 9, 2019): For clarity; in order for the hooks to be run you need to set the following environment variables: ``` SSH_ORIGINAL_COMMAND GITEA_REPO_ID (models.ProtectedBranchRepoID) GITEA_REPO_NAME (models.EnvRepoName) GITEA_REPO_USER_NAME (models.EnvRepoUsername) GITEA_REPO_IS_WIKI (models.EnvRepoIsWiki) GITEA_PUSHER_NAME (models.EnvPusherName) GITEA_PUSHER_ID (models.EnvPusherID) ``` That means that if you call a git command to commit or push you should probably set: ``` isWiki := "false" if strings.HasSuffix(repo.Name, ".wiki") { isWiki = "true" } sig := doer.NewGitSig() env := append(os.Environ(), "SSH_ORIGINAL_COMMAND=internal", "GIT_AUTHOR_NAME="+sig.Name, "GIT_AUTHOR_EMAIL="+sig.Email, "GIT_COMMITTER_NAME="+sig.Name, "GIT_COMMITTER_EMAIL="+sig.Email, models.EnvRepoName+"="+t.repo.Name, models.EnvRepoUsername+"="+repo.OwnerName, models.EnvRepoIsWiki+"="+isWiki, models.EnvPusherName+"="+doer.Name, models.EnvPusherID+"="+fmt.Sprintf("%d", doer.ID), models.ProtectedBranchRepoID+"="+fmt.Sprintf("%d", t.repo.ID), ) ```

GiteaMirror commented

@zeripath commented on GitHub (Feb 9, 2019):

The issue is that our integration tests fail if you set SSH_ORIGINAL_COMMAND, presumably because the environment isn't completely set up. I think if we migrate the affected tests to TestGit style it might work though.

@zeripath commented on GitHub (Feb 9, 2019): The issue is that our integration tests fail if you set `SSH_ORIGINAL_COMMAND`, presumably because the environment isn't completely set up. I think if we migrate the affected tests to TestGit style it might work though.

GiteaMirror commented

@uwebartels commented on GitHub (Feb 28, 2019):

Hiho,

any update? Do you think somebody could fix this?

I'm heavily dependent on git hooks and the code review process.
webhooks unfortunately are not an option as they are asynchronous.

Muchas...
Uwe

@uwebartels commented on GitHub (Feb 28, 2019): Hiho, any update? Do you think somebody could fix this? I'm heavily dependent on git hooks and the code review process. webhooks unfortunately are not an option as they are asynchronous. Muchas... Uwe

GiteaMirror commented

@theAkito commented on GitHub (Mar 22, 2019):

Does post-receive git hook work with other commands or does it not work at all?

@theAkito commented on GitHub (Mar 22, 2019): Does post-receive git hook work with other commands or does it not work at all?

GiteaMirror commented

@uwebartels commented on GitHub (Mar 22, 2019):

yes. the current workaround for us to work directly with git commands like

git merge --squash <branch>
git commit -m '<message>'
git push

I just cannot enforce merging via gitea gui, thus not using the branch protection.

Best Regards
Uwe

@uwebartels commented on GitHub (Mar 22, 2019): yes. the current workaround for us to work directly with git commands like ``` git merge --squash <branch> git commit -m '<message>' git push ``` I just cannot enforce merging via gitea gui, thus not using the branch protection. Best Regards Uwe

GiteaMirror commented

@theAkito commented on GitHub (Mar 22, 2019):

Yes, but do Git hooks over Gitea GUI not work at all or does only merging over Gitea GUI not work?

@theAkito commented on GitHub (Mar 22, 2019): Yes, but do Git hooks over Gitea GUI not work at all or does _only merging_ over Gitea GUI not work?

GiteaMirror commented

@uwebartels commented on GitHub (Mar 22, 2019):

git hooks over gitea gui are not triggered at all. merging works well.

@uwebartels commented on GitHub (Mar 22, 2019): git hooks over gitea gui are not triggered at all. merging works well.

GiteaMirror commented