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.

Sign up for GitHub

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

Jump to bottom

Migrate temp_repo.go to use git.NewCommand #8918

Merged
merged 9 commits into from
Nov 11, 2019
Merged

Migrate temp_repo.go to use git.NewCommand #8918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f9719d5
partial complete
zeripath Nov 10, 2019
7865170
finally move completely to git.NewCommand
zeripath Nov 10, 2019
cd31533
golangci-lint fixes
zeripath Nov 10, 2019
dbe0013
Merge branch 'master' into git-language-fixes
lunny Nov 11, 2019
af13b0c
Merge branch 'master' into git-language-fixes
lunny Nov 11, 2019
3513bf9
Merge branch 'master' into git-language-fixes
lunny Nov 11, 2019
1d8b60f
Ensure pipe closed
zeripath Nov 11, 2019
b033c0d
Update temp_repo.go
zeripath Nov 11, 2019
7c9ac7e
Merge branch 'master' into git-language-fixes
lunny Nov 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions modules/git/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ func (c *Command) RunInDirTimeoutEnvPipeline(env []string, timeout time.Duration
// RunInDirTimeoutEnvFullPipeline executes the command in given directory with given timeout,
// it pipes stdout and stderr to given io.Writer and passes in an io.Reader as stdin.
func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Duration, dir string, stdout, stderr io.Writer, stdin io.Reader) error {
return c.RunInDirTimeoutEnvFullPipelineFunc(env, timeout, dir, stdout, stderr, stdin, nil)
}

// RunInDirTimeoutEnvFullPipelineFunc executes the command in given directory with given timeout,
// it pipes stdout and stderr to given io.Writer and passes in an io.Reader as stdin. Between cmd.Start and cmd.Wait the passed in function is run.
func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.Duration, dir string, stdout, stderr io.Writer, stdin io.Reader, fn func(context.Context, context.CancelFunc)) error {

if timeout == -1 {
timeout = DefaultCommandExecutionTimeout
}
Expand Down Expand Up @@ -98,6 +105,10 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura
pid := process.GetManager().Add(fmt.Sprintf("%s %s %s [repo_path: %s]", GitExecutable, c.name, strings.Join(c.args, " "), dir), cmd)
defer process.GetManager().Remove(pid)

if fn != nil {
fn(ctx, cancel)
}

if err := cmd.Wait(); err != nil {
return err
}
Expand Down
245 changes: 82 additions & 163 deletions modules/repofiles/temp_repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@ import (
"fmt"
"io"
"os"
"os/exec"
"regexp"
"strings"
"time"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/process"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/services/gitdiff"

Expand Down Expand Up @@ -51,9 +49,8 @@ func (t *TemporaryUploadRepository) Close() {

// Clone the base repository to our path and set branch as the HEAD
func (t *TemporaryUploadRepository) Clone(branch string) error {
if _, stderr, err := process.GetManager().ExecTimeout(5*time.Minute,
fmt.Sprintf("Clone (git clone -s --bare): %s", t.basePath),
git.GitExecutable, "clone", "-s", "--bare", "-b", branch, t.repo.RepoPath(), t.basePath); err != nil {
if _, err := git.NewCommand("clone", "-s", "--bare", "-b", branch, t.repo.RepoPath(), t.basePath).Run(); err != nil {
stderr := err.Error()
if matched, _ := regexp.MatchString(".*Remote branch .* not found in upstream origin.*", stderr); matched {
return git.ErrBranchNotExist{
Name: branch,
Expand All @@ -79,11 +76,8 @@ func (t *TemporaryUploadRepository) Clone(branch string) error {

// SetDefaultIndex sets the git index to our HEAD
func (t *TemporaryUploadRepository) SetDefaultIndex() error {
if _, stderr, err := process.GetManager().ExecDir(5*time.Minute,
t.basePath,
fmt.Sprintf("SetDefaultIndex (git read-tree HEAD): %s", t.basePath),
git.GitExecutable, "read-tree", "HEAD"); err != nil {
return fmt.Errorf("SetDefaultIndex: %v %s", err, stderr)
if _, err := git.NewCommand("read-tree", "HEAD").RunInDir(t.basePath); err != nil {
return fmt.Errorf("SetDefaultIndex: %v", err)
}
return nil
}
Expand All @@ -93,33 +87,16 @@ func (t *TemporaryUploadRepository) LsFiles(filenames ...string) ([]string, erro
stdOut := new(bytes.Buffer)
stdErr := new(bytes.Buffer)

timeout := 5 * time.Minute
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

cmdArgs := []string{"ls-files", "-z", "--"}
for _, arg := range filenames {
if arg != "" {
cmdArgs = append(cmdArgs, arg)
}
}

cmd := exec.CommandContext(ctx, git.GitExecutable, cmdArgs...)
desc := fmt.Sprintf("lsFiles: (git ls-files) %v", cmdArgs)
cmd.Dir = t.basePath
cmd.Stdout = stdOut
cmd.Stderr = stdErr

if err := cmd.Start(); err != nil {
return nil, fmt.Errorf("exec(%s) failed: %v(%v)", desc, err, ctx.Err())
}

pid := process.GetManager().Add(desc, cmd)
err := cmd.Wait()
process.GetManager().Remove(pid)

if err != nil {
err = fmt.Errorf("exec(%d:%s) failed: %v(%v) stdout: %v stderr: %v", pid, desc, err, ctx.Err(), stdOut, stdErr)
if err := git.NewCommand(cmdArgs...).RunInDirPipeline(t.basePath, stdOut, stdErr); err != nil {
log.Error("Unable to run git ls-files for temporary repo: %s (%s) Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), t.basePath, err, stdOut.String(), stdErr.String())
err = fmt.Errorf("Unable to run git ls-files for temporary repo of: %s Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), err, stdOut.String(), stdErr.String())
return nil, err
}

Expand All @@ -128,7 +105,7 @@ func (t *TemporaryUploadRepository) LsFiles(filenames ...string) ([]string, erro
filelist = append(filelist, string(line))
}

return filelist, err
return filelist, nil
}

// RemoveFilesFromIndex removes the given files from the index
Expand All @@ -144,90 +121,50 @@ func (t *TemporaryUploadRepository) RemoveFilesFromIndex(filenames ...string) er
}
}

timeout := 5 * time.Minute
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

cmdArgs := []string{"update-index", "--remove", "-z", "--index-info"}
cmd := exec.CommandContext(ctx, git.GitExecutable, cmdArgs...)
desc := fmt.Sprintf("removeFilesFromIndex: (git update-index) %v", filenames)
cmd.Dir = t.basePath
cmd.Stdout = stdOut
cmd.Stderr = stdErr
cmd.Stdin = bytes.NewReader(stdIn.Bytes())

if err := cmd.Start(); err != nil {
return fmt.Errorf("exec(%s) failed: %v(%v)", desc, err, ctx.Err())
}

pid := process.GetManager().Add(desc, cmd)
err := cmd.Wait()
process.GetManager().Remove(pid)

if err != nil {
err = fmt.Errorf("exec(%d:%s) failed: %v(%v) stdout: %v stderr: %v", pid, desc, err, ctx.Err(), stdOut, stdErr)
if err := git.NewCommand("update-index", "--remove", "-z", "--index-info").RunInDirFullPipeline(t.basePath, stdOut, stdErr, stdIn); err != nil {
log.Error("Unable to update-index for temporary repo: %s (%s) Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), t.basePath, err, stdOut.String(), stdErr.String())
return fmt.Errorf("Unable to update-index for temporary repo: %s Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), err, stdOut.String(), stdErr.String())
}

return err
return nil
}

// HashObject writes the provided content to the object db and returns its hash
func (t *TemporaryUploadRepository) HashObject(content io.Reader) (string, error) {
timeout := 5 * time.Minute
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

hashCmd := exec.CommandContext(ctx, git.GitExecutable, "hash-object", "-w", "--stdin")
hashCmd.Dir = t.basePath
hashCmd.Stdin = content
stdOutBuffer := new(bytes.Buffer)
stdErrBuffer := new(bytes.Buffer)
hashCmd.Stdout = stdOutBuffer
hashCmd.Stderr = stdErrBuffer
desc := fmt.Sprintf("hashObject: (git hash-object)")
if err := hashCmd.Start(); err != nil {
return "", fmt.Errorf("git hash-object: %s", err)
}

pid := process.GetManager().Add(desc, hashCmd)
err := hashCmd.Wait()
process.GetManager().Remove(pid)
stdOut := new(bytes.Buffer)
stdErr := new(bytes.Buffer)

if err != nil {
err = fmt.Errorf("exec(%d:%s) failed: %v(%v) stdout: %v stderr: %v", pid, desc, err, ctx.Err(), stdOutBuffer, stdErrBuffer)
return "", err
if err := git.NewCommand("hash-object", "-w", "--stdin").RunInDirFullPipeline(t.basePath, stdOut, stdErr, content); err != nil {
log.Error("Unable to hash-object to temporary repo: %s (%s) Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), t.basePath, err, stdOut.String(), stdErr.String())
return "", fmt.Errorf("Unable to hash-object to temporary repo: %s Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), err, stdOut.String(), stdErr.String())
}

return strings.TrimSpace(stdOutBuffer.String()), nil
return strings.TrimSpace(stdOut.String()), nil
}

// AddObjectToIndex adds the provided object hash to the index with the provided mode and path
func (t *TemporaryUploadRepository) AddObjectToIndex(mode, objectHash, objectPath string) error {
if _, stderr, err := process.GetManager().ExecDir(5*time.Minute,
t.basePath,
fmt.Sprintf("addObjectToIndex (git update-index): %s", t.basePath),
git.GitExecutable, "update-index", "--add", "--replace", "--cacheinfo", mode, objectHash, objectPath); err != nil {
if _, err := git.NewCommand("update-index", "--add", "--replace", "--cacheinfo", mode, objectHash, objectPath).RunInDir(t.basePath); err != nil {
stderr := err.Error()
if matched, _ := regexp.MatchString(".*Invalid path '.*", stderr); matched {
return models.ErrFilePathInvalid{
Message: objectPath,
Path: objectPath,
}
}
return fmt.Errorf("git update-index: %s", stderr)
log.Error("Unable to add object to index: %s %s %s in temporary repo %s(%s) Error: %v", mode, objectHash, objectPath, t.repo.FullName(), t.basePath, err)
return fmt.Errorf("Unable to add object to index at %s in temporary repo %s Error: %v", objectPath, t.repo.FullName(), err)
}
return nil
}

// WriteTree writes the current index as a tree to the object db and returns its hash
func (t *TemporaryUploadRepository) WriteTree() (string, error) {
treeHash, stderr, err := process.GetManager().ExecDir(5*time.Minute,
t.basePath,
fmt.Sprintf("WriteTree (git write-tree): %s", t.basePath),
git.GitExecutable, "write-tree")
stdout, err := git.NewCommand("write-tree").RunInDir(t.basePath)
if err != nil {
return "", fmt.Errorf("git write-tree: %s", stderr)
log.Error("Unable to write tree in temporary repo: %s(%s): Error: %v", t.repo.FullName(), t.basePath, err)
return "", fmt.Errorf("Unable to write-tree in temporary repo for: %s Error: %v", t.repo.FullName(), err)
}
return strings.TrimSpace(treeHash), nil
return strings.TrimSpace(stdout), nil
}

// GetLastCommit gets the last commit ID SHA of the repo
Expand All @@ -240,14 +177,12 @@ func (t *TemporaryUploadRepository) GetLastCommitByRef(ref string) (string, erro
if ref == "" {
ref = "HEAD"
}
treeHash, stderr, err := process.GetManager().ExecDir(5*time.Minute,
t.basePath,
fmt.Sprintf("GetLastCommit (git rev-parse %s): %s", ref, t.basePath),
git.GitExecutable, "rev-parse", ref)
stdout, err := git.NewCommand("rev-parse", ref).RunInDir(t.basePath)
if err != nil {
return "", fmt.Errorf("git rev-parse %s: %s", ref, stderr)
log.Error("Unable to get last ref for %s in temporary repo: %s(%s): Error: %v", ref, t.repo.FullName(), t.basePath, err)
return "", fmt.Errorf("Unable to rev-parse %s in temporary repo for: %s Error: %v", ref, t.repo.FullName(), err)
}
return strings.TrimSpace(treeHash), nil
return strings.TrimSpace(stdout), nil
}

// CommitTree creates a commit from a given tree for the user with provided message
Expand Down Expand Up @@ -287,64 +222,64 @@ func (t *TemporaryUploadRepository) CommitTree(author, committer *models.User, t
}
}

commitHash, stderr, err := process.GetManager().ExecDirEnvStdIn(5*time.Minute,
t.basePath,
fmt.Sprintf("commitTree (git commit-tree): %s", t.basePath),
env,
messageBytes,
git.GitExecutable, args...)
if err != nil {
return "", fmt.Errorf("git commit-tree: %s", stderr)
stdout := new(bytes.Buffer)
stderr := new(bytes.Buffer)
if err := git.NewCommand(args...).RunInDirTimeoutEnvFullPipeline(env, -1, t.basePath, stdout, stderr, messageBytes); err != nil {
log.Error("Unable to commit-tree in temporary repo: %s (%s) Error: %v\nStdout: %s\nStderr: %s",
t.repo.FullName(), t.basePath, err, stdout, stderr)
return "", fmt.Errorf("Unable to commit-tree in temporary repo: %s Error: %v\nStdout: %s\nStderr: %s",
t.repo.FullName(), err, stdout, stderr)
}
return strings.TrimSpace(commitHash), nil
return strings.TrimSpace(stdout.String()), nil
}

// Push the provided commitHash to the repository branch by the provided user
func (t *TemporaryUploadRepository) Push(doer *models.User, commitHash string, branch string) error {
// Because calls hooks we need to pass in the environment
env := models.PushingEnvironment(doer, t.repo)

if _, stderr, err := process.GetManager().ExecDirEnv(5*time.Minute,
t.basePath,
fmt.Sprintf("actuallyPush (git push): %s", t.basePath),
env,
git.GitExecutable, "push", t.repo.RepoPath(), strings.TrimSpace(commitHash)+":refs/heads/"+strings.TrimSpace(branch)); err != nil {
return fmt.Errorf("git push: %s", stderr)
if _, err := git.NewCommand("push", t.repo.RepoPath(), strings.TrimSpace(commitHash)+":refs/heads/"+strings.TrimSpace(branch)).RunInDirWithEnv(t.basePath, env); err != nil {
log.Error("Unable to push back to repo from temporary repo: %s (%s) Error: %v",
t.repo.FullName(), t.basePath, err)
return fmt.Errorf("Unable to push back to repo from temporary repo: %s (%s) Error: %v",
t.repo.FullName(), t.basePath, err)
}
return nil
}

// DiffIndex returns a Diff of the current index to the head
func (t *TemporaryUploadRepository) DiffIndex() (diff *gitdiff.Diff, err error) {
timeout := 5 * time.Minute
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

stdErr := new(bytes.Buffer)

cmd := exec.CommandContext(ctx, git.GitExecutable, "diff-index", "--cached", "-p", "HEAD")
cmd.Dir = t.basePath
cmd.Stderr = stdErr

stdout, err := cmd.StdoutPipe()
func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) {
stdoutReader, stdoutWriter, err := os.Pipe()
if err != nil {
return nil, fmt.Errorf("StdoutPipe: %v stderr %s", err, stdErr.String())
}

if err = cmd.Start(); err != nil {
return nil, fmt.Errorf("Start: %v stderr %s", err, stdErr.String())
}

pid := process.GetManager().Add(fmt.Sprintf("diffIndex [repo_path: %s]", t.repo.RepoPath()), cmd)
defer process.GetManager().Remove(pid)

diff, err = gitdiff.ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdout)
if err != nil {
return nil, fmt.Errorf("ParsePatch: %v", err)
}

if err = cmd.Wait(); err != nil {
return nil, fmt.Errorf("Wait: %v", err)
log.Error("Unable to open stdout pipe: %v", err)
return nil, fmt.Errorf("Unable to open stdout pipe: %v", err)
}
defer func() {
_ = stdoutReader.Close()
_ = stdoutWriter.Close()
}()
stderr := new(bytes.Buffer)
var diff *gitdiff.Diff
var finalErr error

if err := git.NewCommand("diff-index", "--cached", "-p", "HEAD").
RunInDirTimeoutEnvFullPipelineFunc(nil, 30*time.Second, t.basePath, stdoutWriter, stderr, nil, func(ctx context.Context, cancel context.CancelFunc) {
_ = stdoutWriter.Close()
diff, finalErr = gitdiff.ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader)
if finalErr != nil {
log.Error("ParsePatch: %v", finalErr)
cancel()
}
_ = stdoutReader.Close()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hummm... Now this and L267 are no longer required. 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L267 is definitely still required. The process will hang without it.

L273 maybe isn't.

}); err != nil {
if finalErr != nil {
log.Error("Unable to ParsePatch in temporary repo %s (%s). Error: %v", t.repo.FullName(), t.basePath, finalErr)
return nil, finalErr
}
log.Error("Unable to run diff-index pipeline in temporary repo %s (%s). Error: %v\nStderr: %s",
t.repo.FullName(), t.basePath, err, stderr)
return nil, fmt.Errorf("Unable to run diff-index pipeline in temporary repo %s. Error: %v\nStderr: %s",
t.repo.FullName(), err, stderr)
}

return diff, nil
Expand All @@ -358,12 +293,8 @@ func (t *TemporaryUploadRepository) CheckAttribute(attribute string, args ...str
return nil, err
}

stdOut := new(bytes.Buffer)
stdErr := new(bytes.Buffer)

timeout := 5 * time.Minute
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
stdout := new(bytes.Buffer)
stderr := new(bytes.Buffer)

cmdArgs := []string{"check-attr", "-z", attribute}

Expand All @@ -379,26 +310,14 @@ func (t *TemporaryUploadRepository) CheckAttribute(attribute string, args ...str
}
}

cmd := exec.CommandContext(ctx, git.GitExecutable, cmdArgs...)
desc := fmt.Sprintf("checkAttr: (git check-attr) %s %v", attribute, cmdArgs)
cmd.Dir = t.basePath
cmd.Stdout = stdOut
cmd.Stderr = stdErr

if err := cmd.Start(); err != nil {
return nil, fmt.Errorf("exec(%s) failed: %v(%v)", desc, err, ctx.Err())
}

pid := process.GetManager().Add(desc, cmd)
err = cmd.Wait()
process.GetManager().Remove(pid)

if err != nil {
err = fmt.Errorf("exec(%d:%s) failed: %v(%v) stdout: %v stderr: %v", pid, desc, err, ctx.Err(), stdOut, stdErr)
return nil, err
if err := git.NewCommand(cmdArgs...).RunInDirPipeline(t.basePath, stdout, stderr); err != nil {
log.Error("Unable to check-attr in temporary repo: %s (%s) Error: %v\nStdout: %s\nStderr: %s",
t.repo.FullName(), t.basePath, err, stdout, stderr)
return nil, fmt.Errorf("Unable to check-attr in temporary repo: %s Error: %v\nStdout: %s\nStderr: %s",
t.repo.FullName(), err, stdout, stderr)
}

fields := bytes.Split(stdOut.Bytes(), []byte{'\000'})
fields := bytes.Split(stdout.Bytes(), []byte{'\000'})

if len(fields)%3 != 1 {
return nil, fmt.Errorf("Wrong number of fields in return from check-attr")
Expand Down