gitea
/
tea
50
100
Fork 46
Code Issues 77 Pull Requests 7 Actions Releases 16 Activity

Labels Milestones

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

Merged
lunny merged 20 commits from noerw/tea:issue-97/pulls-clean into master 3 years ago
Conversation 16 Commits 20 Files Changed 23
noerw commented 3 years ago
Collaborator

fixes #93, #97, #107

fixes #93, #97, #107
lunny reviewed 3 years ago
Dismissed
cmd/pulls.go Outdated
@ -73,0 +174,4 @@
if err != nil {
return err
}
if pr.State == gitea.StateOpen {
lunny commented 3 years ago
Owner

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 3 years ago
Poster
Collaborator

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 3 years ago
Dismissed
modules/git/branch.go Outdated
@ -0,0 +1,107 @@
package git
lunny commented 3 years ago
Owner

copyright head.

copyright head.
lunny added the
kind/feature
 label 3 years ago
lunny added this to the v0.3.0 milestone 3 years ago
lunny commented 3 years ago
Owner

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) 3 years ago
noerw commented 3 years ago
Poster
Collaborator

@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 3 years ago
Dismissed
cmd/issues.go
@ -62,3 +55,1 @@
return err
}
idx, err := argToIndex(index)
6543 commented 3 years ago
Owner

no err check

no err check
noerw commented 3 years ago
Poster
Collaborator

thanks, resolved

thanks, resolved
6543 reviewed 3 years ago
Dismissed
cmd/pulls.go
@ -14,3 +19,3 @@
)
// CmdPulls represents to login a gitea server.
// CmdPulls is the main command to operate on PRs
6543 commented 3 years ago
Owner

good catch!

good catch!
6543 reviewed 3 years ago
Dismissed
cmd/pulls.go
@ -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 3 years ago
Owner

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 3 years ago
Dismissed
cmd/pulls.go
@ -91,0 +202,4 @@
return fmt.Errorf("PR is still open, won't delete branches")
}
// check if the feature branch is ours:
6543 commented 3 years ago
Owner

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

can you move L205-207 too current 213 ...?
6543 approved these changes 3 years ago
Dismissed
6543 left a comment
Owner

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 3 years ago
Owner

Whats the meaning of the FIXME?

Whats the meaning of the FIXME?
lunny commented 3 years ago
Owner

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 3 years ago
Poster
Collaborator

@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 3 years ago
Owner

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 3 years ago
Poster
Collaborator

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 3 years ago
Owner 
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 3 years ago
Dismissed
6543 left a comment
Owner

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

a realy nice to have featur ... but I hit an bug
noerw commented 3 years ago
Poster
Collaborator

@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 3 years ago
Owner

@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 3 years ago
Owner

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 3 years ago
Poster
Collaborator

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 3 years ago
Owner

@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 3 years ago
Poster
Collaborator

@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 3 years ago
Owner

@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 3 years ago
Owner

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 3 years ago
Dismissed
6543 commented 3 years ago
Owner

@noerw can you "update" this pull ?

@noerw can you "update" this pull ?
lunny approved these changes 3 years ago
Dismissed
lunny merged commit 4cda7e0299 into master 3 years ago
continuous-integration/drone/pr Build is passing
Details
The pull request has been merged as 4cda7e0299.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b noerw-issue-97/pulls-clean master
git pull issue-97/pulls-clean

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff noerw-issue-97/pulls-clean
git push origin master
Sign in to join this conversation.
Reviewers
Request review
No reviewers
Labels
Apply 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
Set milestone
Clear milestone
No items
No Milestone
v0.3.0
Assignees
Assign users
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
Write Preview
Loading…
There is no content yet.
Delete Branch 'noerw/tea:issue-97/pulls-clean'

Deleting a branch is permanent. It CANNOT be undone. Continue?