The Wayback Machine - https://web.archive.org/web/20220211074802/https://github.com/go-gitea/gitea/pull/14295
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 added 3 commits Feb 7, 2021
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 <[email protected]>
* 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 marked this pull request as ready for review Feb 7, 2021
@codecov-io
Copy link

@codecov-io 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.

@lunny lunny added this to the 1.15.0 milestone Feb 8, 2021
@lunny
Copy link
Member

@lunny 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
a1012112796 added 4 commits Feb 8, 2021
* 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

@jiangxin jiangxin Feb 9, 2021

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

@jiangxin jiangxin Feb 9, 2021

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 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

@a1012112796 a1012112796 commented Feb 9, 2021

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

@a1012112796
Copy link
Member Author

@a1012112796 a1012112796 commented Feb 9, 2021

@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 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 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

@a1012112796 a1012112796 commented Jul 25, 2021

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 lunny commented Jul 25, 2021

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

@a1012112796
Copy link
Member Author

@a1012112796 a1012112796 commented Jul 25, 2021

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 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 a1012112796 commented Jul 26, 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?

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

models/migrations/v190.go Outdated Show resolved Hide resolved
lunny
lunny approved these changes Jul 28, 2021
Copy link
Member

@lunny lunny left a comment

Please change more PullRequestStyle to PullRequestFlow, otherwise LGTM

@lunny
Copy link
Member

@lunny lunny commented Jul 28, 2021

make L-G-T-M work.

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

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf commented Jul 28, 2021

🚀

@a1012112796 a1012112796 deleted the git_simple_pr branch Jul 29, 2021
AbdulrhmnGhanem added a commit to kitspace/gitea that referenced this issue 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 <[email protected]>

* 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 <[email protected]>

* fix permission check bug

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

Signed-off-by: a1012112796 <[email protected]>

* 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 <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
zeripath added a commit to zeripath/gitea that referenced this issue 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 <[email protected]>
zeripath added a commit to zeripath/gitea that referenced this issue 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 <[email protected]>
6543 pushed a commit that referenced this issue 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 <[email protected]>
@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.
Linked issues

Successfully merging this pull request may close these issues.

None yet