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

Browsing to a repo shows the most recently updated branch, rather than the default #307

Open
mattcen opened this issue Mar 19, 2023 · 17 comments · May be fixed by #308
Open

Browsing to a repo shows the most recently updated branch, rather than the default #307

mattcen opened this issue Mar 19, 2023 · 17 comments · May be fixed by #308

Comments

@mattcen
Copy link

mattcen commented Mar 19, 2023

When I browse to a repo in Klaus whose default branch (as defined in repo.git/HEAD), if I have made a commit to a different branch (e.g. "foo") more recently than the most recent commit on HEAD, it will show me branch foo instead, which is unexpected and, in my opinion, undesirable.

@mattcen
Copy link
Author

mattcen commented Mar 20, 2023

Ah, I see Klaus tries to get the default branch:

for candidate in ["master", "main", "trunk", "default", "gh-pages"]:

But the version I'm running (1.5.2-4) doesn't include 'main' in that list, and at least one of my repos doesn't have a standard default branch name. Ideally "HEAD" should be used, but I haven't investigated if that's easy to reference using dulwich.

@mattcen mattcen linked a pull request Mar 20, 2023 that will close this issue
@jonashaag
Copy link
Owner

HEAD gives the currently checked out branch, not the default branch

@mattcen
Copy link
Author

mattcen commented Mar 20, 2023

For a bare repo, which I assume is the main use case for a web-based repo viewer like Klaus, these are equivalent.

How else would one deal with the scenario where the default branch (e.g. "stable") is neither in the shortlist of repo names, nor the most recently updated branch?

@jonashaag
Copy link
Owner

jonashaag commented Mar 20, 2023

@trentbuck
Copy link

I think if you just browse to https://git.example.com/foo you should see the same branch by default as if you did git clone https://git.example.com/foo. Which is foo.git/HEAD.

https://git-scm.com/docs/git-clone#Documentation/git-clone.txt--bltnamegt says

Instead of pointing the newly created HEAD to the branch pointed to by the cloned repository’s HEAD, point to <name> branch instead.

i.e. if you don't say --branch=X, you get the cloned repository's HEAD.

@jonashaag
Copy link
Owner

Good point, someone needs to think through what this means for

  • the repo web UI
  • the repo list
  • Smart HTTP cloning
  • Bare repos vs non bare repos

As an example for a non bare repo I don’t expect HEAD to be what is cloned and shown by default, but the default branch (whatever way we define that)

@mattcen
Copy link
Author

mattcen commented Apr 1, 2023

Insofar as the repo list, I don't see why this would introduce any changes at all.

Likewise, for Smart HTTP cloning, the behaviour I expect is what would align with cloning over git or ssh protocols: the client should check out the HEAD branch unless explicitly instructed otherwise. If that's not already the case, I consider it a bug.

For the Web UI, what I think should happen is that in most cases when browsing to a repo, browsing to /repoName/ should 302 to /repoName/refName/. Perhaps not for the main repo view, but for viewing any specific part of the tree. This aligns with GitHub's behaviour: if I browse to https://github.com/jonashaag/klaus, I see the view of the server's HEAD. As soon as I click on a file or directory in that view, it adds the refName (in this case master) to the URL: https://github.com/jonashaag/klaus/tree/master/tools, such that if the default branch changes, existing links would still point to the same branch. If this were already the case, there would be no issue with existing links going to different places.

for a non bare repo I don’t expect HEAD to be what is cloned and shown by default

Why don't you expect that? As per git-clone(1),

Clones a repository into a newly created directory, creates remote-tracking branches for each branch in the cloned repository (visible using git branch --remotes), and creates and checks out an initial branch that is forked from the cloned repository’s currently active branch.

The behaviour I'm proposing exactly aligns with this documentation.

@jonashaag
Copy link
Owner

Hmm that’s a good point although I think it can be surprising if you have accidentally checked out eg a feature branch.

@mattcen
Copy link
Author

mattcen commented Apr 1, 2023

The change I'm proposing doesn't change the behaviour of a check-out at all. Check-out doesn't care what branch Klaus displays in the web UI. It will checkout HEAD unless told otherwise, regardless of the change proposed in #308.

@mattcen
Copy link
Author

mattcen commented Apr 1, 2023

Or are you referring to what is checked out on the server-side copy of the repo that Klaus is looking at?

@jonashaag
Copy link
Owner

It will checkout HEAD unless told otherwise

Wow, I didn't know that's already the case! Thanks for pointing that out. I think that will simplify things a lot.

@jonashaag
Copy link
Owner

I think if you just browse to git.example.com/foo you should see the same branch by default as if you did git clone https://git.example.com/foo. Which is foo.git/HEAD.

I like the simplicity of this but I'm not sure it is desirable in all cases. If you have a local repo that you are actively working on and viewing in Klaus, it might be confusing that the "repo start page" will change its contents depending on what's the current HEAD. GitHub doesn't have this problem because they will always have HEAD pointed to the default branch.

@trentbuck
Copy link

Hrm, I didn't realize klaus even worked with non-bare repos!
I have /srv/vcs/foo.git repos, except for one repo. Due to limitations in gitit, /srv/vcs/kb/.git is not bare, and its worktree is /srv/vcs/kb. It is listed in klaus.contrib.wsgi_autoreload, but it's red and says "Invalid git repository". I never bothered to investigate (because gitit has Good Enough internal git history viewing).

@jonashaag
Copy link
Owner

If you'd like to investigate that'd be appreciated :)

@mattcen
Copy link
Author

mattcen commented Apr 7, 2023

it might be confusing that the "repo start page" will change its contents depending on what's the current HEAD.

This is already confusing, because with the current behaviour, if I don't use the "standard" branch names of "main", "master", "trunk", etc., every time somebody makes a new commit to one of the repo's branches, the branch I'm looking at on the start page changes. This is why I filed this bug in the first place.

How about adding a feature flag that can be turned on so one can opt in to the behaviour I'm suggesting, with the default being to do nothing different than is currently happening (at least for the time being; maybe a flag-day in the future can change the default with sufficient notice)?

@jonashaag
Copy link
Owner

This is already confusing, because with the current behaviour, if I don't use the "standard" branch names of "main", "master", "trunk", etc., every time somebody makes a new commit to one of the repo's branches, the branch I'm looking at on the start page changes. This is why I filed this bug in the first place.

You are right, that really makes little sense. Fixed in #312.

@jonashaag
Copy link
Owner

For bare repos we might want to go further and always show HEAD on the repo page. For non-bare repos, I'm not so sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants