#4571 Unable to use "Blame" feature when there is no MASTER branch
Closed: Fixed 4 years ago by pingou. Opened 4 years ago by arrfab.

One user complained that it's impossible on git.centos.org (migrated to pagure 5.7.4) to use the "blame" feature.
We had a quick look and we can reproduce the issue easily on our dev instance.

By default, all /rpms/* projects don't have currently a MASTER branch, as it was supposed to come from Fedora (and so from src.fedoraproject.org for the git.centos.org counter part).

On dev :
https://git.dev.centos.org/rpms/httpd/blame/SPECS/httpd.spec?identifier=c8 => 404

To confirm the theory of master branch :
- forked a repo
- git merge a branch back into master to create it
- have a look : works :

https://git.dev.centos.org/fork/arrfab/rpms/httpd/blame/SPECS/httpd.spec?identifier=c8

It wasn't working in that forked project until we had a master branch


Metadata Update from @jlanda:
- Issue tagged with: bug

4 years ago

The 404 comes from the repo_obj.head_is_unborn check on https://pagure.io/pagure/blob/master/f/pagure/ui/repo.py#_742

Looking on my dev repos data...

[jlanda@local blametest.git]$ git symbolic-ref HEAD
refs/heads/master

dada! The bare repo inside pagure references HEAD to master, but there is not master branch, so the repo_obj.head_is_unborn is correct.

@pingou how should we handle this?

More info...

Correctly setting the default branch on settings#defaultbranch-tab button fixes the HEAD symbolic ref. So... should we fix the ref on pushing of the first branch? The first branch pushed to the repo should be properly set as default one, HEAD reference included

@jlanda : good catch. OTOH, it's really not usual to have git repositories without any master branch. As said, when the whole "converged git" project started, it was clear that master would come from fedora (aka rawhide) but as fedora hasn't migrated yet to repospanner, on the centos side we only have the c7/c8 branches for now.
Next question : is that then possible to (at least as a temporary workaround) set the default branch for /rpms/* projects through pagure api ? as I don't think we want to go through each tab/project to do this :)

Via api, or fedora-admin?

It might worth an admin cli option for mass set default branch for repos.

Something similar to https://pagure.io/pagure/issue/4490 , but setting the dedault branch

@jlanda via api or pagure-admin, both would work.
For the time being, I confirm that with the following small change, it works on git.dev.centos.org :

-    if repo_obj.is_empty or repo_obj.head_is_unborn:
+    if repo_obj.is_empty:

https://git.dev.centos.org/rpms/httpd/blame/SPECS/httpd.spec?identifier=c8

So, no default/master branch and blame feature works fine

We know that fix is incomplete, because when the branch is not passed, it returns 500 instead of 404.
I have this completely untested patch if anyone wants to try it out (and it makes sense)
https://paste.fedoraproject.org/paste/rHaM9B~HPFu0KfJyfBMXZQ

Since the unborn head is a problem just when we don't have a branch argument, what about this?

diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py
index e3cb45c3..6ec4e937 100644
--- a/pagure/ui/repo.py
+++ b/pagure/ui/repo.py
@@ -739,7 +739,7 @@ def view_blame_file(repo, filename, username=None, namespace=None):

     branchname = flask.request.args.get("identifier", "master")

-    if repo_obj.is_empty or repo_obj.head_is_unborn:
+    if repo_obj.is_empty or not 'identifier' in flask.request.args and repo_obj.head_is_unborn:
         flask.abort(404, description="Empty repo cannot have a file")

     if branchname in repo_obj.listall_branches():

The previous patch fails with invalid branchnames as identifier argument.
This one meet's better codes logic:

diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py
index e3cb45c3..411363a0 100644
--- a/pagure/ui/repo.py
+++ b/pagure/ui/repo.py
@@ -739,9 +739,12 @@ def view_blame_file(repo, filename, username=None, namespace=None):

     branchname = flask.request.args.get("identifier", "master")

-    if repo_obj.is_empty or repo_obj.head_is_unborn:
+    if repo_obj.is_empty:
         flask.abort(404, description="Empty repo cannot have a file")

+    if not branchname in repo_obj.listall_branches() and repo_obj.head_is_unborn:
+        flask.abort(404, description="Specified branch does not exist")
+
     if branchname in repo_obj.listall_branches():
         branch = repo_obj.lookup_branch(branchname)
         commit = branch.peel(pygit2.Commit)

@jlanda via api or pagure-admin, both would work.
For the time being, I confirm that with the following small change, it works on git.dev.centos.org :
- if repo_obj.is_empty or repo_obj.head_is_unborn:
+ if repo_obj.is_empty:

https://git.dev.centos.org/rpms/httpd/blame/SPECS/httpd.spec?identifier=c8
So, no default/master branch and blame feature works fine

#4582 for the fedora-admin set-default action

Metadata Update from @pingou:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

4 years ago

Login to comment on this ticket.

Metadata
Related Pull Requests
  • #4584 Merged 4 years ago