Fix 500 Error with branch and tag sharing the same name

[Viktor-Yakovchuk]

Fixed 500 error while create Pull request when there are more
than one sources (branch, tag) with the same name
The idea is to take branch instead of tag by default if the branch and tag name are equal

Fix #15592

Signed-off-by: Viktor Yakovchuk viktor@yakovchuk.net

[codecov-commenter]

Codecov Report

Merging #16040 (4d64461) into main (cbf3083) will decrease coverage by 0.00%.
The diff coverage is 14.28%.

@@            Coverage Diff             @@
##             main   #16040      +/-   ##
==========================================
- Coverage   44.08%   44.07%   -0.01%     
==========================================
  Files         682      682              
  Lines       82427    82447      +20     
==========================================
+ Hits        36336    36341       +5     
- Misses      40175    40188      +13     
- Partials     5916     5918       +2     

Impacted Files	Coverage Δ
modules/git/error.go	50.00% <0.00%> (-4.24%)	⬇️
modules/git/repo.go	39.66% <0.00%> (-1.38%)	⬇️
services/pull/pull.go	42.95% <30.00%> (-0.89%)	⬇️
modules/charset/charset.go	71.71% <0.00%> (-4.05%)	⬇️
modules/queue/workerpool.go	55.34% <0.00%> (+2.29%)	⬆️
modules/queue/queue_channel.go	96.66% <0.00%> (+5.00%)	⬆️

---

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 cbf3083...4d64461. Read the comment docs.

[zeripath]

Hmm... Do we ever not want to push to refs/heads? I suspect we should be making this determination before push fails in the first place.

I'll have a look.

[Viktor-Yakovchuk]

Hmm... Do we ever not want to push to refs/heads? I suspect we should be making this determination before push fails in the first place.

I'll have a look.

Yes we want to push without "refs/heads" when we create Pull request from tag instead branch. If we always push with refs/heads we will be not able to create pull request from tag.

[zeripath]

I would guess that we should be setting the correct ref in the PR instead.

[Viktor-Yakovchuk]

As I understand it, you will have to rewrite a lot, especially the routing system, to understand from tag or branch we do Pull request. And as I see GitHub use similar routes and take branch as default when the branch and tag names are equal.

[zeripath]

I think the pr.HeadBranch and pr.BaseBranch should have had the refs/heads/... in them and I am fairly certain that at some point I forcibly added this. Somewhere along the line this appears to have been dropped - which is fairly disappointing as this is precisely why I would have added this.

[Viktor-Yakovchuk]

Hello, it seems to me that we have 2 options for solving the #15592:

• this patch, perhaps with additional changes recommended by you
• adding refs/heads wherever such operations occur

Еhe second option makes it impossible to create a PR from the tag, but I can see from code and interface that this functionality is planned, although it does not work yet.

I am ready to implement both options in a few days, which one should I choose?

[Viktor-Yakovchuk]

@zeripath @lunny guys which way we choose to solve this problem?

1. We delete tags tab from source selector when create pull request (in all over cases we keep all as it is) sea also Add compare tag dropdown to releases page #15695 Also add "refs/heads" to the code.
2. We take my pull request and fix pull request from tag (now it doesn't work)

I am ready to prepare new PR after we choose a way to solve the problem.

[zeripath]

OK you have a working fix that fixes the problem for now so I suspect we should merge this.

However, my suggestion is that we should be putting the correct thing in the PR from the start.

So, if you are creating a PR from a branch - then it gets the full ref name: refs/heads/branch.

If you are creating a PR from a tree - it's the full sha

If it's a tag then it's refs/tags/xxx

And so on.

The PR has to be correct and correct forever.

if we use aliases then as you've shown it breaks everything because the name is not a full unambiguous ref.

[zeripath]

This is not the ideal and correct way to fix this but it does at least ameliorate this issue.

We should move to putting the fullref in the PR table so that the situation is always unambiguous.

That would allow us to have PRs opened from remote branches and tags more easily etc.

[Viktor-Yakovchuk]

@techknowlogick please take a look once time permit.