Community
Participate
Working Groups
+++ This bug was initially created as a clone of Bug #362076 +++ As discussed in the original bug 362076 there is good reason to restrict deletion of tags and branches to provide "permanent record" of source code at Eclipse. But, as also discussed in that bug, there are some committers or projects who might like to allow committers to create (and later delete) branches such as <userid>/branchname where they might want to temporarily share some work, but then later delete it. Being such a newbie to git, its hard for me to tell how important this is. That is, it does not seem that important to a cvs lover like me :) but I think for those experienced with git they feel like not having this ability would prohibit people from sharing temporary work. Since, if they did, it would "clutter" the repository with a bunch of odd branch names, which a committer should have the ability to remove. IMHO, the "spirit" of openness is to be _completely_ open (warty failed experiments and all) and if there is "clutter" that is due to the UI that is used to display the clutter, that is, the UI should have filters to filter out noise ... a user should not have to remove the data itself. But ... like I said ... I'm a complete git newbie and greatly value the opinion of those that say it is important. But, as someone who's business depends on persistence :) I think that preventing deletes (bug 362361) is the first order of business, and then having the ability to relax that rule would be this secondary order of business ... more of a feature request. I also have no idea what effort it takes to provide custom "hooks" ... but, sure looks easy :) Note for future, if/when this hook is provided, it may interact or change what's done about the receive.denyDeletes = true setting ... I'm not sure if that has to be changed, for repos that adopt this hook ... or ... what. Just wanted to note it was related.
(In reply to comment #0) > Being such a newbie to git, its hard for me to tell how important this is. That > is, it does not seem that important to a cvs lover like me :) but I think for > those experienced with git they feel like not having this ability would > prohibit people from sharing temporary work. Since, if they did, it would > "clutter" the repository with a bunch of odd branch names, which a committer > should have the ability to remove. IMHO, the "spirit" of openness is to be > _completely_ open (warty failed experiments and all) and if there is "clutter" > that is due to the UI that is used to display the clutter, that is, the UI > should have filters to filter out noise ... a user should not have to remove > the data itself. +1. The more I think about it, it seems that deleting stuff - even the wildest and whackiest ideas and implementations - is not a bright idea. You never know what you or someone else might do with the idea a few years down the line, I have seen this happen at least on 2 occasions. Also, the clutter problem is just a UI problem which in my opinion should be handled by EGit. For instance - ability to create filters in Git Repositories view to filter out branches by name - or organization of branches in folders so that branches created by a committer with the pattern "<userid>/branchname" can be shown under a folder "<userid>", or just use "/" to organize branches in folders.
(In reply to comment #1) > +1. The more I think about it, it seems that deleting stuff - even the wildest > and whackiest ideas and implementations - is not a bright idea. You never know > what you or someone else might do with the idea a few years down the line, I > have seen this happen at least on 2 occasions. I have moved my experimental branch to github and deleted it from the foundation repo. If the foundation repos are just the canonical source repos, then, as Chris stated, it is better to experiment elsewhere. Then the foundation repos can be simple and clean and just contain the source code which is actually built (that is, the master branch.) > > Also, the clutter problem is just a UI problem which in my opinion should be > handled by EGit. For instance > - ability to create filters in Git Repositories view to filter out branches by > name > - or organization of branches in folders so that branches created by a > committer with the pattern "<userid>/branchname" can be shown under a folder > "<userid>", or just use "/" to organize branches in folders. Not everyone uses egit. And besides, how can you tell the active (and thus potentially interesting) branches from the dead wood?
(In reply to comment #2) > I have moved my experimental branch to github and deleted it from the > foundation repo. If the foundation repos are just the canonical source repos, > then, as Chris stated, it is better to experiment elsewhere. Then the > foundation repos can be simple and clean and just contain the source code which > is actually built (that is, the master branch.) I know we (platform) will still be using the foundation repos for our experimental branches and sharing topic branches ... I don't find the userid/expBranch restriction to onerous, and I don't see anything wrong with deleting them if that's a common git workflow. Discussion about if there's a policy/what the policy should be should be done in bug 362076 PW
Created attachment 206206 [details] Possible pre-receive hook Here is a possible pre-receive hook that BJ Hargrave wrote. Note that this script has not been fully tested. It does the following: - It disallows deleting branches without a /. - It disallows non fast-forward on branches without a /. (This means that branches with a / can be forced to non fast-forward.) - It disallows deleting tags without a /. - It disallows unannotated tags to be pushed. The last one assumes we want to force annotated tags. The current tagging scripts do not create annotated tags for the builds. It seems annotated tags are recommended for building purposes.
(In reply to comment #4) > Possible pre-receive hook > > Note that this script has not been fully tested. I just noticed a bug in the script, but the example script conveys the basic idea. I will note that, ultimately, it would be better to consolidate this script and the current update hook together so that a single hook does it all.
(In reply to comment #4) > Created attachment 206206 [details] > Possible pre-receive hook > So what would it take to deploy this pre-receive hook to our eclipse.platform.ui.git repo to try it out? PW
(In reply to comment #6) > So what would it take to deploy this pre-receive hook to our > eclipse.platform.ui.git repo to try it out? It's as simple as downloading the attachment and saving it as /gitroot/path/to/repo.git/hooks/pre-receive. I tried it out for Gyrex and it works so far.
(In reply to comment #7) > It's as simple as downloading the attachment and saving it as > /gitroot/path/to/repo.git/hooks/pre-receive. I tried it out for Gyrex and it > works so far. I opened bug 367954 so I could try this out. PW
Created attachment 214233 [details] pre-receive script OK, I've disabled denydeletes and denynonfastforward for eclipse.platform.ui.git and added this pre-receive hook and run some basic tests. I can't non-ff or delete a main branch, like working_sets. remote: *** Non fast-forward of branch working_sets is not permitted. *** remote: *** Deleting the branch working_sets is not permitted. *** I can amend commits and delete my branches, like pwebster/legacyActions + 65ad224...09f7008 HEAD -> pwebster/legacyActions (forced update) - [deleted] pwebster/legacyActions I can't non-ff update or delete another developer topic branch: remote: *** Non fast-forward of branch bdealwis/css-spy-changes is not permitted by pwebster. *** remote: *** Deleting the branch bdealwis/css-spy-changes is not permitted by pwebster. *** PW
(In reply to comment #9) That sounds very good, thanks a lot! If I see that correctly, then you also removed the blocking of unannotated tags that was in the first attachment. So the build scripts should also work fine with this.
(In reply to comment #9) > Created attachment 214233 [details] > pre-receive script > I've updated the deployed script to handle the case where we're pushing a branch for the first time (it errors right now). We can push topic branches, but can't create new "major" branches without disabling the hook (or updating it to read some custom properties we can set on the repo, which I believe is the preferred way) PW
Created attachment 214491 [details] pre-receive script that uses config I'm trying out this script in eclipse.platform.ui. It just has extra checks: allownonffpush=$( git config --bool hooks.allownonffpush ) allowdeletebranch=$( git config --bool hooks.allowdeletebranch ) allowdeletetag=$( git config --bool hooks.allowdeletetag ) allowcreatenottopicbranch=$( git config --bool hooks.allowcreatenottopicbranch ) that should allow the hook to be overridden with a property on the server repo temporarily for admin purposes. PW
I will propagate the hook next Monday to all "my" repositories.
While we're rolling it out, I've stored the version I've deployed in git so we can track and fix it: http://git.eclipse.org/c/e4/org.eclipse.e4.releng.git/tree/org.eclipse.e4.builder/scripts/pre-receive PW
(In reply to comment #13) > I will propagate the hook next Monday to all "my" repositories. Done for most of our repositories except: jdt.core platform.resources platform.swt filed bug 379497 asking the webmaster to do it.
I've also deployed the hook to equinox: rt.equinox.binaries.git rt.equinox.framework.git rt.equinox.p2.git rt.equinox.bundles.git rt.equinox.incubator.git rt.equinox.security.git PW
I've added a section on our pre-receive hook to http://wiki.eclipse.org/Platform-releng/Git_Workflows#Branches_in_our_Platform_Repos PW
I'm marking this as FIXED since the original request from the Eclipse top-level project is indeed fixed by the hook that got added. Bug 362361 covers the discussion for an Eclipse-wide policy.
Apparently this hook now requires webmaster intervention to create new maintenance branches? Or at the very least, requires an committer to use the server's command line to push the hook aside? See bug 383452 This doesn't seem right.
(In reply to comment #19) > Apparently this hook now requires webmaster intervention to create new > maintenance branches? Or at the very least, requires an committer to use the > server's command line to push the hook aside? > > See bug 383452 > > This doesn't seem right. This is intent for how the pre-receive hook is configured now. See http://wiki.eclipse.org/Platform-releng/Git_Workflows#Branches_in_our_Platform_Repos But, if people feel strongly about it (once created, those branches are hard to delete but not impossible) we could open another bug and remove the part that deals with main branch creation. PW
(In reply to comment #19) > > This doesn't seem right. I'll just add that it was deliberate to prevent people from continuing to push random branches up to the repo, to guide them to the correct topic branch format: committerId/branchName Creation of main branches is a relatively rare occurrence (like forking off R3_8_maintenance). PW
(In reply to comment #21) > (In reply to comment #19) > > > > This doesn't seem right. > > I'll just add that it was deliberate to prevent people from continuing to push > random branches up to the repo, to guide them to the correct topic branch > format: committerId/branchName > > Creation of main branches is a relatively rare occurrence (like forking off > R3_8_maintenance). > > PW Perhaps the deluxe solution (if you can do this with hooks) ... that is sort of a compromise of guiding people, but not require webmaster intervention or committers with shell accounts ... is that if --force is not specified, to give a warning "can not and should make branches unless for long term branch such as for maintenance (because once made, they can not be deleted), otherwise use topic branches such as <committer_id>/<topic>, but if you really want to make this branch, use '--force'". Again ... no idea if it can be done in hook scripts, but seems like it would accomplish the goal of guiding without requiring "extra work" from webmaster (or, committer with shell account).
We should not make it more complicated (or less strict) than it is right now. The creation of maintenance / main branches doesn't happen that often, and there's always a committer who has shell access and can do it. It should never be the webmaster who creates a branch on a project.
(In reply to comment #23) > The creation of maintenance / main branches doesn't happen that often, and > there's always a committer who has shell access and can do it. It should never > be the webmaster who creates a branch on a project. Equinox doesn't have a committer with shell access (that's what brought it up). You are correct, the webmaster doesn't created and push branches, but he does need to set a config property on the repo and then unset it when done. PW
We should have a central read-only pre-receive file and replace all copies by soft-links. Then we can just fix this problem by mentioning the resolution path in the error message: *** creation of branch R3_8_maintenance is not permitted. Use <committerId>/<branchname> for topic branches or ask your project lead or PMC *** Ideally, the pre-receive hook would be versioned, e.g. in the eclipse.platform repo.
(In reply to comment #25) > > Ideally, the pre-receive hook would be versioned, e.g. in the eclipse.platform > repo. What's currently deployed is versioned in http://git.eclipse.org/c/e4/org.eclipse.e4.releng.git/tree/org.eclipse.e4.builder/scripts/pre-receive But we can move it to an eclipse.platform.releng* type repo if that's what we want. PW
Created attachment 225214 [details] fix for committerId/sub/topic branches This patch fixes 3 issues: 1) non-fast-forwarding of a committerId/sub/topic branch was not allowed 2) error messages could be more helpful 3) the script didn't identify its origin Dani or Paul, can you please release this to the git repo from comment 26? I've fixed this in the eclipse.jdt.ui.git repo and verified that it works fine. Now, we need a strategy to distribute it (comment #25): > We should have a central read-only pre-receive file and replace all copies > by soft-links. Webmaster: Do you know how widely this pre-receive script is used? If it's still mostly an Eclipse Platform project thing, then I suggest someone in the eclipse.platform group copies this file to /gitroot/platform/eclipse.platform.git/hooks/pre-receive and then replaces the existing pre-receive files in /gitroot/{platform,jdt,pde,e4}/*.git/hooks/ with symbolic links to the master version.
(In reply to comment #27) > Created attachment 225214 [details] > fix for committerId/sub/topic branches > > This patch fixes 3 issues: > 1) non-fast-forwarding of a committerId/sub/topic branch was not allowed > 2) error messages could be more helpful > 3) the script didn't identify its origin > > Dani or Paul, can you please release this to the git repo from comment 26? Fixed with http://git.eclipse.org/c/e4/org.eclipse.e4.releng.git/commit/?id=5bb8408c74aafee5c43bb5b1e5ef8176df92b19e
