The Wayback Machine - https://web.archive.org/web/20210807164320/https://github.com/go-gitea/gitea/pull/14295
Skip to content

/ gitea

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

Jump to bottom

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 •
edited by lunny

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 mentioned this pull request Jan 13, 2021
Open
1 task
@a1012112796 a1012112796 mentioned this pull request Jan 27, 2021
Closed
12 tasks
a1012112796 added 3 commits Feb 7, 2021
@a1012112796
feature: add agit flow support
36c77da 
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]>
@a1012112796
Merge branch 'master' into git_simple_pr
b8784f9 
* master:
  [skip ci] Updated licenses and gitignores
@a1012112796
fix lint
a757ed5
@a1012112796 a1012112796 force-pushed the a1012112796:git_simple_pr branch from 736f1b8 to a757ed5 Feb 7, 2021
@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 •
edited

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?
@lunny
lunny reviewed Feb 8, 2021
View changes
models/pull.go Outdated Show resolved Hide resolved
@lunny
lunny reviewed Feb 8, 2021
View changes
models/pull.go Outdated Show resolved Hide resolved
a1012112796 added 4 commits Feb 8, 2021
@a1012112796
simplify code add fix some nits
f96e9e7
@a1012112796
Merge branch 'master' into git_simple_pr
501f850 
* master:
  Fixed irritating error message related to go version (#14611)
  Use OldRef instead of CommitSHA for DeleteBranch comments (#14604)
  Add information on how to build statically (#14594)
  [skip ci] Updated translations via Crowdin
  Exclude the current dump file from the dump (#14606)
  Remove spurious DataAsync Error logging (#14599)
  [API] Add  delete release by tag & fix unreleased inconsistency (#14563)
  Fix rate limit bug when downloading assets on migrating from github (#14564)
  [API] Add affected files of commits to commit struct (#14579)
@a1012112796
update merge help message
7294bfa
@a1012112796
Merge branch 'master' into git_simple_pr
c1291fa 
* master:
  Add support for ref parameter to get raw file API (#14602)
@jiangxin
jiangxin suggested changes Feb 9, 2021
View changes
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) {

This comment has been minimized.

Sign in to view
@jiangxin

jiangxin Feb 9, 2021

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") {

This comment has been minimized.

Sign in to view
@jiangxin

jiangxin Feb 9, 2021

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 added 3 commits Feb 9, 2021
@a1012112796
Apply suggestions from code review. Thanks @jiangxin
0d9442e
@a1012112796
add forced-update message
86acbe1
@a1012112796
fix lint
95ca26a
@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.
@a1012112796
fix version negotiation
22fd300
@lunny
lunny reviewed Jul 20, 2021
View changes
models/migrations/v189.go Outdated Show resolved Hide resolved
@lunny
lunny reviewed Jul 20, 2021
View changes
models/pull.go Outdated Show resolved Hide resolved
a1012112796 and others added 6 commits Jul 21, 2021
@a1012112796
Merge branch 'main' into git_simple_pr
d322b82
@6543
Merge branch 'main' into git_simple_pr
4d4aa45
@a1012112796
remane
19edf9a
@a1012112796
fix template
fe505de
@6543
Merge branch 'main' into git_simple_pr
5a840ac
@a1012112796
Merge branch 'main' into git_simple_pr
a86ee55
@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 •
edited

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.
@a1012112796
handle empty repo
25c4b5c
@lunny
Copy link
Member

@lunny lunny commented Jul 25, 2021

Maybe you should rewrite the gitea hooks on migrations if git version matched.
@a1012112796
ui: fix branch link under the title
b42ebdd
@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.
@lunny
lunny reviewed Jul 28, 2021
View changes
models/migrations/v190.go Outdated Show resolved Hide resolved
@lunny
lunny approved these changes Jul 28, 2021
View changes
Copy link
Member

@lunny lunny left a comment

Please change more PullRequestStyle to PullRequestFlow, otherwise LGTM
lunny and others added 2 commits Jul 28, 2021
@lunny
Merge branch 'main' into git_simple_pr
3b6594a
@a1012112796
fix nits
f351805
@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
2 checks passed
approvals/lgtm this commit looks good
continuous-integration/drone/pr Build is passing
Details
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf commented Jul 28, 2021

🚀
@a1012112796 a1012112796 deleted the a1012112796:git_simple_pr branch Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lafriks lafriks

@zeripath zeripath

@jiangxin jiangxin

@lunny lunny

@6543 6543

Assignees
No one assigned
Labels
kind/feature lgtm/done
Milestone
1.16.0
Linked issues

Successfully merging this pull request may close these issues.

None yet