Allow administrative modification of the default branch/tag referenced by HEAD #5

Merged
merged 3 commits into from Jan 30, 2012

Conversation

Projects
None yet
2 participants
@plm

plm commented Jan 27, 2012

This allows control over the default branch after clone, which is equivalent to running:
git symbolic-ref HEAD refs/heads/mybranch

This is for use in environments where the "master" branch does not exist or should not be the default upon clone, but administrators do not want to access the GitBlit via shell and run Git commands against the bare repository.

I haven't written any unit tests for the changes to JGitUtils, but it "worked for me" in my ad hoc testing.

I didn't add a tab stop for the new form field on the "edit repository" HTML template; the indexes will need to be adjusted as part of the merge.

plm added some commits Jan 27, 2012

Allow administrative modification of the default branch/tag reference…
…d by HEAD.

This allows control over the default branch after clone, which is equivalent to running:
  git symbolic-ref HEAD refs/heads/mybranch
Correct update of HEAD symbolic reference when target is a tag.
Revised update results which are considered successful and improved error
messaging.
@gitblit

This comment has been minimized.

Show comment Hide comment
@gitblit

gitblit Jan 27, 2012

Owner

edited for < and > entities

This looks like a useful feature. I have some comments and questions for you.

Since gitblit refers to heads as branches I would change verbage from "head" to "branch". I believe this to be consistent with GitHub, etc.

RepositoryModel. Because the RepositoryModel is used for federation and JSON-RPC calls I do not want List<RefModel>, it is too heavy. We could do a List<String>, though.

I see in your second commit that you are using the Ref object for dealing with tags. We can still get the tag's objectid from the name, but I am wondering why I would ever want to set HEAD to a tag? Especially since you are not making a distinction between an annotated tag and a tag? My first reaction is to not include tags in the list of available branches at all. Can you point me to an example of where/why this functionality is wanted?

EditRepositoryPage. I would not restrict the access to the default branch combobox. Repository Owners should be allowed to change this and only an owner or an admin can access this page anyway.

EditRepositoryPage/JGitUtils.setDefaultHead. You rely on RefModel.reference which is a transient variable. It is transient because it is not serializable and Wicket demands that any model object be serializable. Wicket can and will serialize pages to disk and deserialize them as needed so we need to be careful here because the reference may be null. This is another reason why List<String>, while primitive, would be better.

Gitblit.updateRepositoryModel. Maybe setDefaultHead should only be called if it has changed. I'm uncomfortable relinking HEAD on every model update.

Owner

gitblit commented Jan 27, 2012

edited for < and > entities

This looks like a useful feature. I have some comments and questions for you.

Since gitblit refers to heads as branches I would change verbage from "head" to "branch". I believe this to be consistent with GitHub, etc.

RepositoryModel. Because the RepositoryModel is used for federation and JSON-RPC calls I do not want List<RefModel>, it is too heavy. We could do a List<String>, though.

I see in your second commit that you are using the Ref object for dealing with tags. We can still get the tag's objectid from the name, but I am wondering why I would ever want to set HEAD to a tag? Especially since you are not making a distinction between an annotated tag and a tag? My first reaction is to not include tags in the list of available branches at all. Can you point me to an example of where/why this functionality is wanted?

EditRepositoryPage. I would not restrict the access to the default branch combobox. Repository Owners should be allowed to change this and only an owner or an admin can access this page anyway.

EditRepositoryPage/JGitUtils.setDefaultHead. You rely on RefModel.reference which is a transient variable. It is transient because it is not serializable and Wicket demands that any model object be serializable. Wicket can and will serialize pages to disk and deserialize them as needed so we need to be careful here because the reference may be null. This is another reason why List<String>, while primitive, would be better.

Gitblit.updateRepositoryModel. Maybe setDefaultHead should only be called if it has changed. I'm uncomfortable relinking HEAD on every model update.

@plm

This comment has been minimized.

Show comment Hide comment
@plm

plm Jan 28, 2012

RepositoryModel use of RefModel (and use in setDefaultHead)
In my initial implementation, I actually did use List<String> to track the full ref name of the symbolic HEAD ref. I'll make the change to use a String to hold the ref name.

Gitblit.updateRepositoryModel
I'll revise the code to only update the reference if it changes.

EditRepositoryPage permissions
Agreed, I'll make the change.

"head" vs. "branch" and use of a tag as symbolic HEAD ref
The impetus for this patch is to address a specific use case where a repository may have no branches, as the repository is considered "retired" in the sense that there is no active development. In this case, the repository would contain tags and their history. Removal of the branches is a policy-driven choice (by my employer) which would lead to an error when cloning the repository because the previous symbolic HEAD reference will point to a branch which no longer exists. This approach would be in preference to Gitblit's "freeze repo" feature, since hotfix branches would still be allowed, but only off of an existing tag. In this use case, I would want the ability to specify either a branch or tag, and this is the reasoning for using the term "head" rather than "branch". As this may not be the normal use case, I could add a configuration option which would allow inclusion of tags in this "availableHeads" list. This option would be disabled by default. Then the UI verbiage could be changed to the term "branch" and advanced users could turn on the "allow tags as HEAD" feature if necessary.

plm commented Jan 28, 2012

RepositoryModel use of RefModel (and use in setDefaultHead)
In my initial implementation, I actually did use List<String> to track the full ref name of the symbolic HEAD ref. I'll make the change to use a String to hold the ref name.

Gitblit.updateRepositoryModel
I'll revise the code to only update the reference if it changes.

EditRepositoryPage permissions
Agreed, I'll make the change.

"head" vs. "branch" and use of a tag as symbolic HEAD ref
The impetus for this patch is to address a specific use case where a repository may have no branches, as the repository is considered "retired" in the sense that there is no active development. In this case, the repository would contain tags and their history. Removal of the branches is a policy-driven choice (by my employer) which would lead to an error when cloning the repository because the previous symbolic HEAD reference will point to a branch which no longer exists. This approach would be in preference to Gitblit's "freeze repo" feature, since hotfix branches would still be allowed, but only off of an existing tag. In this use case, I would want the ability to specify either a branch or tag, and this is the reasoning for using the term "head" rather than "branch". As this may not be the normal use case, I could add a configuration option which would allow inclusion of tags in this "availableHeads" list. This option would be disabled by default. Then the UI verbiage could be changed to the term "branch" and advanced users could turn on the "allow tags as HEAD" feature if necessary.

@gitblit

This comment has been minimized.

Show comment Hide comment
@gitblit

gitblit Jan 28, 2012

Owner

Ok lets do this: Switch from RefModel to full name strings. Keep the tags, now that I understand the use it makes sense. I will probably change the dropdown renderer to differentiate between branches and tags and I may add a pop-up prompt to confirm relinking to a tag.

In the updateModel method make sure to disregard null or empty defaultHead values (there is a StringUtils method for this). Models can also be updated via RPC or federation both of which may be from older versions and thus have null or empty default heads.

Lets also leave your "head" labeling as it does make more sense but lets follow the Git convention of uppercase here so all methods and labels, etc should say HEAD.

I have an immediate use for this relinking capability so your timing is excellent!

Owner

gitblit commented Jan 28, 2012

Ok lets do this: Switch from RefModel to full name strings. Keep the tags, now that I understand the use it makes sense. I will probably change the dropdown renderer to differentiate between branches and tags and I may add a pop-up prompt to confirm relinking to a tag.

In the updateModel method make sure to disregard null or empty defaultHead values (there is a StringUtils method for this). Models can also be updated via RPC or federation both of which may be from older versions and thus have null or empty default heads.

Lets also leave your "head" labeling as it does make more sense but lets follow the Git convention of uppercase here so all methods and labels, etc should say HEAD.

I have an immediate use for this relinking capability so your timing is excellent!

RepositoryModel will use String rather than RefModel to track the cur…
…rent

symbolic head and available heads.

Added convenience methods to JGitUtils to support retrieving available
heads as List<String>.
When resolving the symbolic head target as a String, if the head is
detached, attempt to match the commit SHA1 against the known tags, using
the most recent tag if more than one matches.
Revised error messaging to better reflect actual outcome.
Adjusted tab indexes on edit repository page to include default head combo
box.
Updated message key for default head combo box to use uppercase "HEAD".
@plm

This comment has been minimized.

Show comment Hide comment
@plm

plm Jan 30, 2012

I've revised the code per your recommended changes.

plm commented Jan 30, 2012

I've revised the code per your recommended changes.

gitblit added a commit that referenced this pull request Jan 30, 2012

Merge pull request #5 from plm/admin_default_head
Allow administrative modification of the default branch/tag referenced by HEAD

@gitblit gitblit merged commit 2dfa21c into gitblit:master Jan 30, 2012

@gitblit

This comment has been minimized.

Show comment Hide comment
@gitblit

gitblit Jan 30, 2012

Owner

Thanks Phil, this looks great.

Owner

gitblit commented Jan 30, 2012

Thanks Phil, this looks great.

gitblit added a commit that referenced this pull request Feb 21, 2014

@gitblit gitblit referenced this pull request Aug 12, 2015

Closed

Unfederated repos #348

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment