Bumper: Allow user to specify working directory for git push #19159

GeoBK · 2020-09-09T16:14:21Z

This change would

add the RunAndCommitIfChanged function which would run the passed in commands and commit the changes if any.
allow the user to specify the working directory of a GitPush

These functions have been added as part of the bugzilla backporter tool. The GetAllClones function would return all the clones of a particular bug(including its parent, grandparent etc.), unlike the GetClones function which would return only the clones which were cloned from that particular bug. The GetRoot function would return the original bug which produced the clone even if that bug is a clone of a clone.

k8s-ci-robot · 2020-09-09T16:14:29Z

Hi @GeoBK. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

stevekuznetsov · 2020-09-09T21:32:12Z

prow/flagutil/git.go

@@ -28,8 +29,8 @@ import (
 // GitOptions holds options for interacting with git.
 type GitOptions struct {
 	host          string
-	user          string
-	email         string
+	User          string


So the use from bumper is not the git library. Although the options seem the same, they are different. We should have a separate struct for them that's not tied to this, unless we expect a single binary to use both?

I dont understand what you mean by "So the use from bumper is not the git library".
I am using both git and bumper here(https://github.com/openshift/ci-tools/pull/1211/files#diff-4440cf6f38e6026b7a8028aa83c9f664R33)
Am I doing that wrong?

moved this from git options into new class GitAuthorOptions in bumper.go

MushuEE · 2020-09-09T23:10:05Z

experiment/autobumper/bumper/bumper.go

+
+	gitCmd := "git"
+	gitArgs := []string{}
+	if workingDir != "" {


If this might be used across even more functions in the future is it worth making something like (below)?

type GitCommand struct { baseCommand string workingDir string args []string{} } func (gc GitCommand) Call(stdout, stderr io.Writer) error { if gc.workingDir != "" { gc.args = append(gc.args, fmt.Sprintf("--git-dir=%s/.git", gc.workingDir), fmt.Sprintf("--work-tree=%s", gc.workingDir)) } c := exec.Command(gc.baseCommand, gc.args...) c.Stdout = stdout c.Stderr = stderr return c.Run() }

petr-muller · 2020-09-10T09:50:00Z

experiment/autobumper/bumper/bumper.go

-	if err := Call(stdout, stderr, "git", "push", "-f", remote, fmt.Sprintf("HEAD:%s", remoteBranch)); err != nil {
+	gitArgs := []string{}
+	if workingDir != "" {
+		gitArgs = append(gitArgs, fmt.Sprintf("--git-dir=%s/.git", workingDir), fmt.Sprintf("--work-tree=%s", workingDir))


I think we can simplify the --git-dir=%s/.git --work-tree=%s to -C %s.

petr-muller · 2020-09-10T09:51:40Z

experiment/autobumper/bumper/bumper.go

+	if workingDir != "" {
+		gitArgs = append(gitArgs, fmt.Sprintf("--git-dir=%s/.git", workingDir), fmt.Sprintf("--work-tree=%s", workingDir))
+	}
+	gitPushArgs := append(gitArgs, "push", "-f", remote, fmt.Sprintf("HEAD:%s", remoteBranch))


IMO no need to introduce a new variable, just use gitArgs (and because we're in GitPush function, I'd even call it just args.

petr-muller · 2020-09-10T09:53:22Z

experiment/autobumper/bumper/bumper.go

+// RunAndCommitIfNeeded makes a commit in the workingDir if there are
+// any changes resulting from the command execution. Returns true if a commit is made
+func RunAndCommitIfNeeded(stdout, stderr io.Writer, author, cmd string, args []string, workingDir string) (bool, error) {
+	fullCommand := fmt.Sprintf("%s %s", filepath.Base(cmd), strings.Join(args, " "))


Same comment like I had in the ci-tools PR - fullCommand is used in logging. While reading logs, I want to see what the tool really does (calls cmd) without prettyfing it with filepath.Base.

petr-muller · 2020-09-10T09:58:01Z

/ok-to-test

GeoBK · 2020-09-11T17:32:13Z

/label tide/merge-method-squash

k8s-ci-robot · 2020-09-14T14:34:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: GeoBK, stevekuznetsov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [stevekuznetsov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

GeoBK and others added 3 commits June 11, 2020 19:54

Merge branch 'master' of github.com:GeoBK/test-infra

a9e34e8

Merge remote-tracking branch 'upstream/master'

4e70140

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 9, 2020

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 9, 2020

k8s-ci-robot requested review from clarketm and petr-muller September 9, 2020 16:14

k8s-ci-robot added area/prow Issues or PRs related to prow sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Sep 9, 2020

GeoBK force-pushed the chore/github/add-options-for-github-commit branch from 694c9fe to fc230f6 Compare September 9, 2020 21:11

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 9, 2020

stevekuznetsov reviewed Sep 9, 2020

View reviewed changes

MushuEE reviewed Sep 9, 2020

View reviewed changes

petr-muller reviewed Sep 10, 2020

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 10, 2020

Add login and name options for git commits

7227458

GeoBK force-pushed the chore/github/add-options-for-github-commit branch from fc230f6 to 7227458 Compare September 10, 2020 19:25

Move git author options to bumper.go

00472c8

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 11, 2020

Use constant instead of hardcoding "git"

083b47f

GeoBK changed the title ~~Chore/GitHub/add options for GitHub commit~~ Allow user to specify working directory for git push Sep 11, 2020

GeoBK changed the title ~~Allow user to specify working directory for git push~~ Bumper: Allow user to specify working directory for git push Sep 11, 2020

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Sep 11, 2020

stevekuznetsov approved these changes Sep 14, 2020

View reviewed changes

k8s-ci-robot assigned stevekuznetsov Sep 14, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 14, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 14, 2020

k8s-ci-robot merged commit 6e1d254 into kubernetes:master Sep 14, 2020

k8s-ci-robot added this to the v1.20 milestone Sep 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bumper: Allow user to specify working directory for git push #19159

Bumper: Allow user to specify working directory for git push #19159

GeoBK commented Sep 9, 2020 •

edited

Loading

k8s-ci-robot commented Sep 9, 2020

stevekuznetsov Sep 9, 2020

GeoBK Sep 9, 2020

GeoBK Sep 11, 2020

MushuEE Sep 9, 2020 •

edited

Loading

petr-muller Sep 10, 2020

petr-muller Sep 10, 2020

petr-muller Sep 10, 2020

petr-muller commented Sep 10, 2020

GeoBK commented Sep 11, 2020

k8s-ci-robot commented Sep 14, 2020

Bumper: Allow user to specify working directory for git push #19159

Bumper: Allow user to specify working directory for git push #19159

Conversation

GeoBK commented Sep 9, 2020 • edited Loading

k8s-ci-robot commented Sep 9, 2020

stevekuznetsov Sep 9, 2020

Choose a reason for hiding this comment

GeoBK Sep 9, 2020

Choose a reason for hiding this comment

GeoBK Sep 11, 2020

Choose a reason for hiding this comment

MushuEE Sep 9, 2020 • edited Loading

Choose a reason for hiding this comment

petr-muller Sep 10, 2020

Choose a reason for hiding this comment

petr-muller Sep 10, 2020

Choose a reason for hiding this comment

petr-muller Sep 10, 2020

Choose a reason for hiding this comment

petr-muller commented Sep 10, 2020

GeoBK commented Sep 11, 2020

k8s-ci-robot commented Sep 14, 2020

GeoBK commented Sep 9, 2020 •

edited

Loading

MushuEE Sep 9, 2020 •

edited

Loading