gitea/tea
gitea/tea
57
136
Fork 67
Code Issues 102 Pull Requests 8 Actions Releases 16 Activity

add tea pulls [checkout | clean] commands (#93 #97 #107) #105

Merged
lunny merged 20 commits from noerw/tea:issue-97/pulls-clean into master 2020-04-19 03:09:08 +00:00
Conversation 16 Commits 20 Files Changed 23 +2050 -92
noerw commented 2020-03-07 13:31:47 +00:00
Member
Copy Link

fixes #93, #97, #107

fixes #93, #97, #107
lunny reviewed 2020-03-08 01:32:07 +00:00
Dismissed
cmd/pulls.go Outdated
@ -73,0 +174,4 @@
if err != nil {
return err
}
if pr.State == gitea.StateOpen {
lunny commented 2020-03-08 01:32:07 +00:00
Owner
Copy Link

User may still want to clean it, but we can give a confirmation.

User may still want to clean it, but we can give a confirmation.
noerw commented 2020-03-08 12:16:14 +00:00
Author
Member
Copy Link

But we can't safely remove the remote branch in this case, right?
Effectively tea wouldn't do anything different than git branch -D <featurebranch>, so I don't see the point.
We could guide the user to call tea pulls close 1 first if he really wants to, thats more expressive than a --force flag

But we can't safely remove the remote branch in this case, right? Effectively tea wouldn't do anything different than `git branch -D <featurebranch>`, so I don't see the point. We could guide the user to call `tea pulls close 1` first if he really wants to, thats more expressive than a `--force` flag
lunny reviewed 2020-03-08 01:32:36 +00:00
Dismissed
modules/git/branch.go Outdated
@ -0,0 +1,107 @@
package git
lunny commented 2020-03-08 01:32:36 +00:00
Owner
Copy Link

copyright head.

copyright head.
lunny added the
kind
feature
 label 2020-03-08 12:30:40 +00:00
lunny added this to the v0.3.0 milestone 2020-03-08 12:30:45 +00:00
lunny commented 2020-03-10 07:57:47 +00:00
Owner
Copy Link

I encountered an panic when I input tea pr clean 7 on xorm/reverse#7.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x14becf0]

goroutine 1 [running]:
code.gitea.io/tea/modules/git.TeaRepo.TeaFindBranch.func1(0xc0000204c0, 0xc0000204c0, 0x0)
	/gitea/tea/modules/git/branch.go:96 +0x190
gopkg.in/src-d/go-git.v4/plumbing/storer.(*referenceFilteredIter).ForEach(0xc0001aeec0, 0xc00019e810, 0x0, 0x0)
	/gitea/tea/vendor/gopkg.in/src-d/go-git.v4/plumbing/storer/reference.go:82 +0xb8
code.gitea.io/tea/modules/git.TeaRepo.TeaFindBranch(0xc00019e4b0, 0xc0001aa450, 0x28, 0xc0001aa780, 0x22, 0x7, 0xc000190140, 0x0)
	/gitea/tea/modules/git/branch.go:88 +0x1b7
code.gitea.io/tea/cmd.runPullsClean(0xc0001808c0, 0x5, 0x8)
	/gitea/tea/cmd/pulls.go:188 +0x24f
github.com/urfave/cli/v2.(*Command).Run(0x1b9f080, 0xc000180600, 0x0, 0x0)
	/gitea/tea/vendor/github.com/urfave/cli/v2/command.go:161 +0x4b9
github.com/urfave/cli/v2.(*App).RunAsSubcommand(0xc000001980, 0xc0001802c0, 0x0, 0x0)
	/gitea/tea/vendor/github.com/urfave/cli/v2/app.go:433 +0xa16
github.com/urfave/cli/v2.(*Command).startApp(0x1b9ee40, 0xc0001802c0, 0xc000140ae0, 0x2)
	/gitea/tea/vendor/github.com/urfave/cli/v2/command.go:275 +0x67f
github.com/urfave/cli/v2.(*Command).Run(0x1b9ee40, 0xc0001802c0, 0x0, 0x0)
	/tea/vendor/github.com/urfave/cli/v2/command.go:91 +0xa25
github.com/urfave/cli/v2.(*App).RunContext(0xc000001800, 0x17a25c0, 0xc000028108, 0xc000020080, 0x4, 0x4, 0x0, 0x0)
	/tea/vendor/github.com/urfave/cli/v2/app.go:302 +0x790
github.com/urfave/cli/v2.(*App).Run(...)
	/tea/vendor/github.com/urfave/cli/v2/app.go:211
main.main()
	/tea/main.go:47 +0x1e8
I encountered an panic when I input `tea pr clean 7` on https://gitea.com/xorm/reverse/pulls/7. ```log panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x14becf0] goroutine 1 [running]: code.gitea.io/tea/modules/git.TeaRepo.TeaFindBranch.func1(0xc0000204c0, 0xc0000204c0, 0x0) /gitea/tea/modules/git/branch.go:96 +0x190 gopkg.in/src-d/go-git.v4/plumbing/storer.(*referenceFilteredIter).ForEach(0xc0001aeec0, 0xc00019e810, 0x0, 0x0) /gitea/tea/vendor/gopkg.in/src-d/go-git.v4/plumbing/storer/reference.go:82 +0xb8 code.gitea.io/tea/modules/git.TeaRepo.TeaFindBranch(0xc00019e4b0, 0xc0001aa450, 0x28, 0xc0001aa780, 0x22, 0x7, 0xc000190140, 0x0) /gitea/tea/modules/git/branch.go:88 +0x1b7 code.gitea.io/tea/cmd.runPullsClean(0xc0001808c0, 0x5, 0x8) /gitea/tea/cmd/pulls.go:188 +0x24f github.com/urfave/cli/v2.(*Command).Run(0x1b9f080, 0xc000180600, 0x0, 0x0) /gitea/tea/vendor/github.com/urfave/cli/v2/command.go:161 +0x4b9 github.com/urfave/cli/v2.(*App).RunAsSubcommand(0xc000001980, 0xc0001802c0, 0x0, 0x0) /gitea/tea/vendor/github.com/urfave/cli/v2/app.go:433 +0xa16 github.com/urfave/cli/v2.(*Command).startApp(0x1b9ee40, 0xc0001802c0, 0xc000140ae0, 0x2) /gitea/tea/vendor/github.com/urfave/cli/v2/command.go:275 +0x67f github.com/urfave/cli/v2.(*Command).Run(0x1b9ee40, 0xc0001802c0, 0x0, 0x0) /tea/vendor/github.com/urfave/cli/v2/command.go:91 +0xa25 github.com/urfave/cli/v2.(*App).RunContext(0xc000001800, 0x17a25c0, 0xc000028108, 0xc000020080, 0x4, 0x4, 0x0, 0x0) /tea/vendor/github.com/urfave/cli/v2/app.go:302 +0x790 github.com/urfave/cli/v2.(*App).Run(...) /tea/vendor/github.com/urfave/cli/v2/app.go:211 main.main() /tea/main.go:47 +0x1e8 ```
noerw changed title from add `tea pulls clean` command (#97) to add `tea pulls [checkout | clean]` commands (#93 #97 #107) 2020-03-11 20:04:40 +00:00
noerw commented 2020-03-11 20:09:40 +00:00
Author
Member
Copy Link

@lunny There were flaws in the way I looked up branches, should be fixed now among other problems.

@lunny There were flaws in the way I looked up branches, should be fixed now among other problems.
6543 requested changes 2020-03-29 23:19:11 +00:00
Dismissed
cmd/issues.go Outdated
@ -62,3 +55,1 @@
return err
}
idx, err := argToIndex(index)
6543 commented 2020-03-29 23:11:57 +00:00
Owner
Copy Link

no err check

no err check
noerw commented 2020-03-30 13:25:47 +00:00
Author
Member
Copy Link

thanks, resolved

thanks, resolved
6543 reviewed 2020-03-31 22:55:53 +00:00
Dismissed
cmd/pulls.go Outdated
@ -14,3 +19,3 @@
)
// CmdPulls represents to login a gitea server.
// CmdPulls is the main command to operate on PRs
6543 commented 2020-03-31 22:55:53 +00:00
Owner
Copy Link

good catch!

good catch!
6543 reviewed 2020-03-31 23:03:57 +00:00
Dismissed
cmd/pulls.go Outdated
@ -91,0 +134,4 @@
}
// verify related remote is in local repo, otherwise add it
newRemoteName := fmt.Sprintf("pulls/%v", pr.Head.Repository.Owner.UserName)
6543 commented 2020-03-31 23:03:57 +00:00
Owner
Copy Link

future impruvement: define prefix via config ... (for examle i sue user/)

but this can be added after this pull :)

future impruvement: define prefix via config ... (for examle i sue `user/`) but this can be added after this pull :)
6543 reviewed 2020-03-31 23:07:43 +00:00
Dismissed
cmd/pulls.go Outdated
@ -91,0 +202,4 @@
return fmt.Errorf("PR is still open, won't delete branches")
}
// check if the feature branch is ours:
6543 commented 2020-03-31 23:07:43 +00:00
Owner
Copy Link

can you move L205-207 too current 213 ...?

can you move L205-207 too current 213 ...?
6543 approved these changes 2020-03-31 23:28:59 +00:00
Dismissed
6543 left a comment
Owner
Copy Link

a few questions ... but looks good

a few questions ... but looks good
modules/git/branch.go Outdated
@ -0,0 +20,4 @@
localBranchRefName := git_plumbing.NewBranchReferenceName(localBranchName)
err := r.CreateBranch(&git_config.Branch{
Name: localBranchName,
Merge: git_plumbing.NewBranchReferenceName(remoteBranchName), // FIXME: should be remoteBranchName
6543 commented 2020-03-31 23:18:01 +00:00
Owner
Copy Link

Whats the meaning of the FIXME?

Whats the meaning of the FIXME?
lunny commented 2020-04-01 08:43:41 +00:00
Owner
Copy Link

Panic is gone, but it reported no authentication but in fact when I input tea issues, there are content listed.

Checking out 'master' to delete local branch 'lunny/reverse_reader'
Deleting local branch lunny/reverse_reader and remote branch lunny/reverse_reader
2020/04/01 16:41:44 Failed to run app with [tea pr clean 7]: authentication required
Panic is gone, but it reported no authentication but in fact when I input `tea issues`, there are content listed. ``` Checking out 'master' to delete local branch 'lunny/reverse_reader' Deleting local branch lunny/reverse_reader and remote branch lunny/reverse_reader 2020/04/01 16:41:44 Failed to run app with [tea pr clean 7]: authentication required ```
noerw commented 2020-04-01 11:59:51 +00:00
Author
Member
Copy Link

@lunny Interesting. I guess you use a https remote? For SSH it worked fine.
I'm not sure how one would safely provide interactive authentication for https.

When developing, I noted that in pushing/pulling you can provide an AuthMethod, for which I found no further documentation though, so I'd need help here

@lunny Interesting. I guess you use a `https` remote? For SSH it worked fine. I'm not sure how one would safely provide interactive authentication for https. When developing, I noted that in pushing/pulling you can provide an [`AuthMethod`](https://godoc.org/gopkg.in/src-d/go-git.v4/plumbing/transport#AuthMethod), for which I found no further documentation though, so I'd need help here
lunny commented 2020-04-01 13:16:55 +00:00
Owner
Copy Link

We should care that the head branch maybe in the forked repsitory and a non-origin remote.

We should care that the head branch maybe in the forked repsitory and a non-origin remote.
noerw commented 2020-04-01 13:50:49 +00:00
Author
Member
Copy Link

You mean that we should not clean on remotes that are not origin, as the user might have no write permission? Whitelisting only origin is too strict (i use origin as upstream and noerw for my fork).
I guess there is is no other safe way than trying if the user may push there, and notifying that the remote branch deletion failed

You mean that we should not clean on remotes that are not `origin`, as the user might have no write permission? Whitelisting only `origin` is too strict (i use origin as upstream and `noerw` for my fork). I guess there is is no other safe way than trying if the user may push there, and notifying that the remote branch deletion failed
6543 commented 2020-04-01 22:26:00 +00:00
Owner
Copy Link
Fetching PR 333 (head https://gitea.com/gitnex/GitNex.git:304-popup-menus-to-bottomsheets) from remote 'main'
2020/04/02 00:24:56 Failed to run app with [tea pr checkout 333]: error creating SSH agent: "SSH agent requested but SSH_AUTH_SOCK not-specified"
```sh Fetching PR 333 (head https://gitea.com/gitnex/GitNex.git:304-popup-menus-to-bottomsheets) from remote 'main' 2020/04/02 00:24:56 Failed to run app with [tea pr checkout 333]: error creating SSH agent: "SSH agent requested but SSH_AUTH_SOCK not-specified" ```
6543 requested changes 2020-04-01 22:26:39 +00:00
Dismissed
6543 left a comment
Owner
Copy Link

a realy nice to have featur ... but I hit an bug

a realy nice to have featur ... but I hit an bug
noerw commented 2020-04-04 22:21:56 +00:00
Author
Member
Copy Link

@6543 could you post the result of the following command? I suspect your SSH agent is not set up correctly, which wouldn't really be a bug in tea.

echo "pid $SSH_AGENT_PID sock $SSH_AUTH_SOCK"
@6543 could you post the result of the following command? I suspect your SSH agent is not set up correctly, which wouldn't really be a bug in `tea`. ``` echo "pid $SSH_AGENT_PID sock $SSH_AUTH_SOCK" ```
6543 commented 2020-04-04 22:35:02 +00:00
Owner
Copy Link

@6543 could you post the result of the following command? I suspect your SSH agent is not set up correctly, which wouldn't really be a bug in tea.

echo "pid $SSH_AGENT_PID sock $SSH_AUTH_SOCK"

This vars are not set at all

EDIT: so it need a ssh agent running in the background :/

> @6543 could you post the result of the following command? I suspect your SSH agent is not set up correctly, which wouldn't really be a bug in `tea`. > > ``` > echo "pid $SSH_AGENT_PID sock $SSH_AUTH_SOCK" > ``` This vars are not set at all EDIT: so it need a ssh agent running in the background :/
6543 commented 2020-04-04 22:41:44 +00:00
Owner
Copy Link

if I start ssh-agent and export this vars I get this:

2020/04/05 00:40:55 Failed to run app with [tea pr checkout 139]: ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain
if I start ssh-agent and export this vars I get this: ``` 2020/04/05 00:40:55 Failed to run app with [tea pr checkout 139]: ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain ```
noerw commented 2020-04-04 22:59:34 +00:00
Author
Member
Copy Link

Seems like it selects the wrong key.

I could specify a key to use in repo.Push(). Then no ssh-agent is needed, but the default key ~/.ssh/id_rsa will not always be the right one.
Instead we could add a config option for a login to specify an ssh key, defaulting to id_rsa.

Seems like it selects the wrong key. I could specify a key to use in `repo.Push()`. Then no ssh-agent is needed, but the default key `~/.ssh/id_rsa` will not always be the right one. Instead we could add a config option for a login to specify an ssh key, defaulting to `id_rsa`.
👍 1
6543 commented 2020-04-05 02:46:01 +00:00
Owner
Copy Link

@noerw why not fallback -> use ssh-agent and if not afailable fall back to configurable ssh key witch by default use ~/.ssh/id_rsa or is this to mouch?
if so I'm for reading the key direct

@noerw why not fallback -> use ssh-agent and if not afailable fall back to configurable ssh key witch by default use `~/.ssh/id_rsa` or is this to mouch? if so I'm for reading the key direct
noerw commented 2020-04-05 20:37:36 +00:00
Author
Member
Copy Link

@6543 I added authentication for https & ssh (trying ssh-agent, and then falling back to reading a key manually), so it should work for your setup now as well.

(Logins now store username & path to ssh key, both config values are optional)

Edit:
It seems like gitea does not support Token Auth for git pushes over https? If it does (I couldn't get it to work), one could use the tea token instead of prompting users for credentials

@6543 I added authentication for https & ssh (trying ssh-agent, and then falling back to reading a key manually), so it should work for your setup now as well. (Logins now store username & path to ssh key, both config values are optional) Edit: It seems like gitea does not support Token Auth for git pushes over https? If it does (I couldn't get it to work), one could use the tea token instead of prompting users for credentials
🎉 1
6543 commented 2020-04-09 12:06:26 +00:00
Owner
Copy Link

@noerw It asks for authentification on checkout for a https conection every time

@noerw It asks for authentification on checkout for a https conection every time
6543 commented 2020-04-18 00:24:50 +00:00
Owner
Copy Link

had this issue only on a single repo - if it happen's again I will debug it cant find any issues - thanks @noerw this subcomand is a huge help (testing this branch since 1w 👍 )

had this issue only on a single repo - if it happen's again I will debug it cant find any issues - thanks @noerw this subcomand is a huge help (testing this branch since 1w :+1: )
❤️ 1
6543 approved these changes 2020-04-18 00:25:05 +00:00
Dismissed
6543 commented 2020-04-18 00:26:27 +00:00
Owner
Copy Link

@noerw can you "update" this pull ?

@noerw can you "update" this pull ?
lunny approved these changes 2020-04-19 03:08:45 +00:00
Dismissed
lunny merged commit 4cda7e0299 into master 2020-04-19 03:09:08 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
kind/breaking

   
kind/bug

   
kind/build

   
kind/dependency

   
kind/deployment

   
kind/docs
  
kind
enhancement

  
kind
feature

  
kind/proposal
  
kind
question

  
kind
refactor

  
kind/security

   
kind/testing

   
kind/translation

   
priority/critical

   
priority/high

   
priority/low

   
priority/medium

   
reviewed/duplicate

   
reviewed/invalid

   
reviewed/wontfix

   
skip-changelog

   
status/blocked

   
status/has-backport

   
status/has-pull

   
status/needs-backport

   
status/needs-feedback

   
status/needs-reviews

   
status/wip

   
upstream/gitea

   
upstream/sdk
No Label
kind/breaking
kind/bug
kind/build
kind/dependency
kind/deployment
kind/docs
kind
enhancement
kind
feature
kind/proposal
kind
question
kind
refactor
kind/security
kind/testing
kind/translation
priority/critical
priority/high
priority/low
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/has-backport
status/has-pull
status/needs-backport
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
upstream/sdk
Milestone
Clear milestone
No items
No Milestone
v0.3.0
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/tea#105
No description provided.
Delete Branch "noerw/tea:issue-97/pulls-clean"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?