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.

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

Add agit flow support in gitea #14295

Merged
merged 63 commits into from
Jul 28, 2021
Merged

Add agit flow support in gitea #14295

merged 63 commits into from
Jul 28, 2021

Conversation

a1012112796
Copy link
Member

@a1012112796 a1012112796 commented Jan 10, 2021

ref: https://git-repo.info/en/2020/03/agit-flow-and-git-repo/

examle:
test

NOTICE: All existed repositories will not have the new hooks, you have to rewrite the hooks via admin panel.

@a1012112796 a1012112796 added type/proposal The new feature has not been accepted yet but needs to be discussed first. pr/wip This PR is not ready for review labels Jan 10, 2021
@a1012112796 a1012112796 mentioned this pull request Jan 13, 2021
1 task
@a1012112796 a1012112796 mentioned this pull request Jan 27, 2021
12 tasks
ref: https://git-repo.info/en/2020/03/agit-flow-and-git-repo/

example:

```Bash
git checkout -b test
echo "test" >> README.md
git commit -m "test"
git push origin HEAD:refs/for/master -o topic=test
```

Signed-off-by: a1012112796 <1012112796@qq.com>
* master:
  [skip ci] Updated licenses and gitignores
@a1012112796 a1012112796 changed the title WIP: try add agit flow support in gitea add agit flow support in gitea Feb 7, 2021
@a1012112796 a1012112796 added type/feature Completely new functionality. Can only be merged if feature freeze is not active. and removed type/proposal The new feature has not been accepted yet but needs to be discussed first. pr/wip This PR is not ready for review labels Feb 7, 2021
@a1012112796 a1012112796 marked this pull request as ready for review February 7, 2021 08:39
@codecov-io
Copy link

codecov-io commented Feb 7, 2021

Codecov Report

Merging #14295 (5140fd7) into master (487f2ee) will decrease coverage by 0.18%.
The diff coverage is 13.10%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14295      +/-   ##
==========================================
- Coverage   42.21%   42.03%   -0.19%     
==========================================
  Files         767      768       +1     
  Lines       81624    82118     +494     
==========================================
+ Hits        34458    34515      +57     
- Misses      41531    41944     +413     
- Partials     5635     5659      +24     
Impacted Files Coverage Δ
cmd/serv.go 2.53% <0.00%> (-0.10%) ⬇️
models/migrations/migrations.go 2.59% <ø> (ø)
models/migrations/v172.go 0.00% <0.00%> (ø)
modules/git/repo_branch.go 67.50% <0.00%> (-6.48%) ⬇️
modules/private/hook.go 0.00% <0.00%> (ø)
modules/repository/hooks.go 18.80% <0.00%> (-0.84%) ⬇️
routers/repo/http.go 40.86% <0.00%> (-0.21%) ⬇️
routers/repo/issue.go 38.37% <0.00%> (-0.03%) ⬇️
routers/routes/web.go 89.98% <0.00%> (-1.26%) ⬇️
services/pull/commit_status.go 4.10% <0.00%> (-0.37%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 487f2ee...5140fd7. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 7, 2021
@lunny lunny added this to the 1.15.0 milestone Feb 8, 2021
@lunny
Copy link
Member

lunny commented Feb 8, 2021

@jiangxin Could we ask you to help to review this PR?

models/pull.go Outdated Show resolved Hide resolved
models/pull.go Outdated Show resolved Hide resolved
* master:
  Fixed irritating error message related to go version (go-gitea#14611)
  Use OldRef instead of CommitSHA for DeleteBranch comments (go-gitea#14604)
  Add information on how to build statically (go-gitea#14594)
  [skip ci] Updated translations via Crowdin
  Exclude the current dump file from the dump (go-gitea#14606)
  Remove spurious DataAsync Error logging (go-gitea#14599)
  [API] Add  delete release by tag & fix unreleased inconsistency (go-gitea#14563)
  Fix rate limit bug when downloading assets on migrating from github (go-gitea#14564)
  [API] Add affected files of commits to commit struct (go-gitea#14579)
* master:
  Add support for ref parameter to get raw file API (go-gitea#14602)
cmd/hook.go Outdated Show resolved Hide resolved
cmd/hook.go Outdated Show resolved Hide resolved
cmd/hook.go Outdated Show resolved Hide resolved
cmd/hook.go Outdated Show resolved Hide resolved
cmd/hook.go Outdated Show resolved Hide resolved
cmd/hook.go Outdated Show resolved Hide resolved
cmd/hook.go Outdated

const VersionHead string = "version=1"

if !strings.HasPrefix(rs.Data, VersionHead) {
Copy link

Choose a reason for hiding this comment

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

You should split rs.Data by the NUL character ('\0'). Part one is version, and part two is capabilities.

cmd/hook.go Outdated

hasPushOptions := false
response := []byte(VersionHead)
if strings.Contains(rs.Data, "push-options") {
Copy link

Choose a reason for hiding this comment

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

You should split capabilities using space character, and try to match each capability.
If there is a new capability called "push-options-v2", using string.Contains is wrong.

cmd/hook.go Outdated Show resolved Hide resolved
cmd/hook.go Show resolved Hide resolved
@jiangxin
Copy link

jiangxin commented Feb 9, 2021

It's not good to have a merge commit for others to review, so please rebase your topic.

@a1012112796
Copy link
Member Author

@jiangxin Thanks for your carefully check, I will change them soon.

@a1012112796
Copy link
Member Author

@jiangxin Another question, I know your designed style to create pull request is git push origin HEAD:refs/for/<target-branch>/<session>, But the <target-branch> also maybe contain /.
I wonder.... (now I have to change it to git push origin HEAD:refs/for/<target-branch> -o topic=<session>).
Can you tell me reason? Thanks.

@lunny
Copy link
Member

lunny commented Jul 25, 2021

When push to an empty repository, an error returned remote: error: cannot find hook 'proc-receive'.

@lunny
Copy link
Member

lunny commented Jul 25, 2021

Another two problems:

  • The branch link under the title should be disabled because there is no branch created.
  • Documentations should be updated.

@a1012112796
Copy link
Member Author

When push to an empty repository, an error returned remote: error: cannot find hook 'proc-receive'.

If a repo created before this pull request, should run Resynchronize pre-receive, update and post-receive hooks of all repositories.. or if create new repo, It will response other error message example view:
Peek 2021-07-25 23-06

I will add more check to response a more better error message.

@lunny
Copy link
Member

lunny commented Jul 25, 2021

Maybe you should rewrite the gitea hooks on migrations if git version matched.

@a1012112796
Copy link
Member Author

Maybe you should rewrite the gitea hooks on migrations if git version matched.

I think just add a note on blog is enough

@6543 6543 changed the title add agit flow support in gitea Add agit flow support in gitea Jul 25, 2021
@6543
Copy link
Member

6543 commented Jul 25, 2021

@a1012112796 well since we check for the git version and assume if it is high enough we do recive proc-receive.
thats why we skip some checks on post - if there are no proc hooks in place, can this be used to pass code into a repo without permissions to do so?

@a1012112796
Copy link
Member Author

@a1012112796 well since we check for the git version and assume if it is high enough we do recive proc-receive.
thats why we skip some checks on post - if there are no proc hooks in place, can this be used to pass code into a repo without permissions to do so?

Willn't, Because will check permission in pre-receive hook.

models/migrations/v190.go Outdated Show resolved Hide resolved
Copy link
Member

@lunny lunny left a comment

Choose a reason for hiding this comment

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

Please change more PullRequestStyle to PullRequestFlow, otherwise LGTM

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 28, 2021
@lunny
Copy link
Member

lunny commented Jul 28, 2021

make L-G-T-M work.

@lunny lunny merged commit 3705168 into go-gitea:main Jul 28, 2021
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

🚀

@a1012112796 a1012112796 deleted the git_simple_pr branch July 29, 2021 03:36
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 10, 2021
* feature: add agit flow support

ref: https://git-repo.info/en/2020/03/agit-flow-and-git-repo/

example:

```Bash
git checkout -b test
echo "test" >> README.md
git commit -m "test"
git push origin HEAD:refs/for/master -o topic=test
```

Signed-off-by: a1012112796 <1012112796@qq.com>

* fix lint

* simplify code add fix some nits

* update merge help message

* Apply suggestions from code review. Thanks @jiangxin

* add forced-update message

* fix lint

* splite writePktLine

* add refs/for/<target-branch>/<topic-branch> support also

* Add test code add fix api

* fix lint

* fix test

* skip test if git version < 2.29

* try test with git 2.30.1

* fix permission check bug

* fix some nit

* logic implify and test code update

* fix bug

* apply suggestions from code review

* prepare for merge

Signed-off-by: Andrew Thornton <art27@cantab.net>

* fix permission check bug

- test code update
- apply suggestions from code review @zeripath

Signed-off-by: a1012112796 <1012112796@qq.com>

* fix bug when target branch isn't exist

* prevent some special push and fix some nits

* fix lint

* try splite

* Apply suggestions from code review

- fix permission check
- handle user rename

* fix version negotiation

* remane

* fix template

* handle empty repo

* ui: fix  branch link under the title

* fix nits

Co-authored-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
zeripath added a commit to zeripath/gitea that referenced this pull request Aug 16, 2021
…ndard refs

There was an inadvertent breaking change in go-gitea#15629 meaning that notes refs and other
git extension refs will be automatically rejected.

Further following go-gitea#14295 and go-gitea#15629 the pre-recieve hook code is untenably long and
too complex.

This PR refactors the hook code and removes the incorrect forced rejection of
non-standard refs.

Fix go-gitea#16688

Signed-off-by: Andrew Thornton <art27@cantab.net>
6543 pushed a commit that referenced this pull request Sep 16, 2021
…ndard refs (#16705)

* Clean-up HookPreReceive and restore functionality for pushing non-standard refs

There was an inadvertent breaking change in #15629 meaning that notes refs and other
git extension refs will be automatically rejected.

Further following #14295 and #15629 the pre-recieve hook code is untenably long and
too complex.

This PR refactors the hook code and removes the incorrect forced rejection of
non-standard refs.

Fix #16688

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.