-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Take back control of hooks #1006
Changes from all commits
ec82565
24be2d7
6a98cc0
a3ae540
a62eb0f
9a9f14d
2c33815
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
// Copyright 2017 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package cmd | ||
|
||
import ( | ||
"fmt" | ||
"os" | ||
|
||
"code.gitea.io/gitea/models" | ||
|
||
"github.com/urfave/cli" | ||
) | ||
|
||
var ( | ||
// CmdHook represents the available hooks sub-command. | ||
CmdHook = cli.Command{ | ||
Name: "hook", | ||
Usage: "Delegate commands to corresponding Git hooks", | ||
Description: "This should only be called by Git", | ||
Flags: []cli.Flag{ | ||
cli.StringFlag{ | ||
Name: "config, c", | ||
Value: "custom/conf/app.ini", | ||
Usage: "Custom configuration file path", | ||
}, | ||
}, | ||
Subcommands: []cli.Command{ | ||
subcmdHookPreReceive, | ||
subcmdHookUpadte, | ||
subcmdHookPostReceive, | ||
}, | ||
} | ||
|
||
subcmdHookPreReceive = cli.Command{ | ||
Name: "pre-receive", | ||
Usage: "Delegate pre-receive Git hook", | ||
Description: "This command should only be called by Git", | ||
Action: runHookPreReceive, | ||
} | ||
subcmdHookUpadte = cli.Command{ | ||
Name: "update", | ||
Usage: "Delegate update Git hook", | ||
Description: "This command should only be called by Git", | ||
Action: runHookUpdate, | ||
} | ||
subcmdHookPostReceive = cli.Command{ | ||
Name: "post-receive", | ||
Usage: "Delegate post-receive Git hook", | ||
Description: "This command should only be called by Git", | ||
Action: runHookPostReceive, | ||
} | ||
) | ||
|
||
func runHookPreReceive(c *cli.Context) error { | ||
if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 { | ||
return nil | ||
} | ||
if err := setup("hooks/pre-receive.log"); err != nil { | ||
fail("Hook pre-receive init failed", fmt.Sprintf("setup: %v", err)) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func runHookUpdate(c *cli.Context) error { | ||
if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand why the check is needed, with the nearby context. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the SSH_ORIGINAL_COMMAND is not set, that's because it is called by shell on manually. |
||
return nil | ||
} | ||
|
||
if err := setup("hooks/update.log"); err != nil { | ||
fail("Hook update init failed", fmt.Sprintf("setup: %v", err)) | ||
} | ||
|
||
args := c.Args() | ||
if len(args) != 3 { | ||
fail("Arguments received are not equal to three", "Arguments received are not equal to three") | ||
} else if len(args[0]) == 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose the two conditions are not mutually exclusive? Not sure if I understand it correctly, but the use of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Here I just move codes from update.go to hook.go. I think it's no problem. Suppose the command |
||
fail("First argument 'refName' is empty", "First argument 'refName' is empty") | ||
} | ||
|
||
uuid := os.Getenv(envUpdateTaskUUID) | ||
if err := models.AddUpdateTask(&models.UpdateTask{ | ||
UUID: uuid, | ||
RefName: args[0], | ||
OldCommitID: args[1], | ||
NewCommitID: args[2], | ||
}); err != nil { | ||
fail("Internal error", "Fail to add update task '%s': %v", uuid, err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func runHookPostReceive(c *cli.Context) error { | ||
if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 { | ||
return nil | ||
} | ||
|
||
if err := setup("hooks/post-receive.log"); err != nil { | ||
fail("Hook post-receive init failed", fmt.Sprintf("setup: %v", err)) | ||
} | ||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
// Copyright 2017 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package migrations | ||
|
||
import ( | ||
"fmt" | ||
"io/ioutil" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
|
||
"code.gitea.io/gitea/modules/setting" | ||
|
||
"github.com/Unknwon/com" | ||
"github.com/go-xorm/xorm" | ||
) | ||
|
||
func generateAndMigrateGitHooks(x *xorm.Engine) (err error) { | ||
type Repository struct { | ||
ID int64 | ||
OwnerID int64 | ||
Name string | ||
} | ||
type User struct { | ||
ID int64 | ||
Name string | ||
} | ||
|
||
var ( | ||
hookNames = []string{"pre-receive", "update", "post-receive"} | ||
hookTpls = []string{ | ||
fmt.Sprintf("#!/usr/bin/env %s\nORI_DIR=`pwd`\nSHELL_FOLDER=$(cd \"$(dirname \"$0\")\";pwd)\ncd \"$ORI_DIR\"\nfor i in `ls \"$SHELL_FOLDER/pre-receive.d\"`; do\n sh \"$SHELL_FOLDER/pre-receive.d/$i\"\ndone", setting.ScriptType), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's late but still want to comment it. You don't need the quotes for the ls and you don't need to call ls. Just something like for I in folder/*; do done. Beside that I would use source instead of sh to execute the scripts, than you can also define env variables in scripts that can be used in another script later on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you are right since I'm not familiar with shell script. We also have |
||
fmt.Sprintf("#!/usr/bin/env %s\nORI_DIR=`pwd`\nSHELL_FOLDER=$(cd \"$(dirname \"$0\")\";pwd)\ncd \"$ORI_DIR\"\nfor i in `ls \"$SHELL_FOLDER/update.d\"`; do\n sh \"$SHELL_FOLDER/update.d/$i\" $1 $2 $3\ndone", setting.ScriptType), | ||
fmt.Sprintf("#!/usr/bin/env %s\nORI_DIR=`pwd`\nSHELL_FOLDER=$(cd \"$(dirname \"$0\")\";pwd)\ncd \"$ORI_DIR\"\nfor i in `ls \"$SHELL_FOLDER/post-receive.d\"`; do\n sh \"$SHELL_FOLDER/post-receive.d/$i\"\ndone", setting.ScriptType), | ||
} | ||
giteaHookTpls = []string{ | ||
fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' pre-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf), | ||
fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' update $1 $2 $3\n", setting.ScriptType, setting.AppPath, setting.CustomConf), | ||
fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' post-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf), | ||
} | ||
) | ||
|
||
return x.Where("id > 0").Iterate(new(Repository), | ||
func(idx int, bean interface{}) error { | ||
repo := bean.(*Repository) | ||
user := new(User) | ||
has, err := x.Where("id = ?", repo.OwnerID).Get(user) | ||
if err != nil { | ||
return fmt.Errorf("query owner of repository [repo_id: %d, owner_id: %d]: %v", repo.ID, repo.OwnerID, err) | ||
} else if !has { | ||
return nil | ||
} | ||
|
||
repoPath := filepath.Join(setting.RepoRootPath, strings.ToLower(user.Name), strings.ToLower(repo.Name)) + ".git" | ||
hookDir := filepath.Join(repoPath, "hooks") | ||
|
||
for i, hookName := range hookNames { | ||
oldHookPath := filepath.Join(hookDir, hookName) | ||
newHookPath := filepath.Join(hookDir, hookName+".d", "gitea") | ||
|
||
if err = os.MkdirAll(filepath.Join(hookDir, hookName+".d"), os.ModePerm); err != nil { | ||
return fmt.Errorf("create hooks dir '%s': %v", filepath.Join(hookDir, hookName+".d"), err) | ||
} | ||
|
||
// WARNING: Old server-side hooks will be moved to sub directory with the same name | ||
if hookName != "update" && com.IsExist(oldHookPath) { | ||
newPlace := filepath.Join(hookDir, hookName+".d", hookName) | ||
if err = os.Rename(oldHookPath, newPlace); err != nil { | ||
return fmt.Errorf("Remove old hook file '%s' to '%s': %v", oldHookPath, newPlace, err) | ||
} | ||
} | ||
|
||
if err = ioutil.WriteFile(oldHookPath, []byte(hookTpls[i]), 0777); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. It seems 0755 is enough. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If 0700 is enough then 0700 is better. I cannot think of a scenario where 'others' need the permissions to R/W it. Edit: Okay, it would be much convenient when debugging 😆 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right. But I ls -l the repository's root dir, all the permission is 755. Don't know why. |
||
return fmt.Errorf("write old hook file '%s': %v", oldHookPath, err) | ||
} | ||
|
||
if err = ioutil.WriteFile(newHookPath, []byte(giteaHookTpls[i]), 0777); err != nil { | ||
return fmt.Errorf("write new hook file '%s': %v", oldHookPath, err) | ||
} | ||
} | ||
return nil | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name 'setup' is a bit of vague as to what it actually does. I guess it's about setting up a logger?
But I don't understand why it has anything to do with the database engine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setup is to initialize the command, this function is for serval commands which the different is the log place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it's resulted from factoring out the common 'preparing' part of all the commands. Maybe the
Before
callback is a suitable place for it.https://sourcegraph.com/github.com/lunny/gitea@f132edf80f2de3b9335e99eec7d23abdfcc6670f/-/blob/vendor/github.com/urfave/cli/command.go#L32:2-32:8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before should be the same actions, but this time we have a parameter - log name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Before callback has the
context
parameter which has the needed (I guess) information like the command name to construct the path of the log file, if I wasn't misunderstanding somethingActually, I am not sure if putting it in Before is better or not. But there is the possibility. 😉