Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 343150

Summary: Should we denyNonFastForward?
Product: Community Reporter: Alexander Kurtakov <akurtakov>
Component: GitAssignee: Eclipse Webmaster <webmaster>
Status: RESOLVED WORKSFORME QA Contact:
Severity: normal    
Priority: P3 CC: bsd, caniszczyk, contact, daniel_megert, david_williams, deepakazad, elias, fiedler.mf, gunnar, hrr, john.arthorne, marcel.bruch, markus.kell.r, michael.norman, Mike_Wilson, milesparker, mknauer, mober.at+eclipse, overholt, pwebster, remy.suen, sptaszkiewicz, stepper, tjwatson, tomasz.zarna, wayne.beaton
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Whiteboard:
Bug Depends on: 387212    
Bug Blocks: 362076    
Attachments:
Description Flags
list of 53 configs without denyNonFastForward none

Description Alexander Kurtakov CLA 2011-04-18 09:02:53 EDT
Should we set receive.denyNonFastForwards in the git server? This will not allow people pushing things that rewrite history but I think rewriting history is not smth that should be allowed at all. 
Comments?
Comment 1 Andrew Overholt CLA 2011-04-18 09:07:51 EDT
A specific example of a "weird" commit is this:

  http://git.eclipse.org/c/linuxtools/org.eclipse.linuxtools.git/commit/?id=f6899c4b07c78de7b68dd339c6007d6f49fb301f
Comment 2 Denis Roy CLA 2011-04-18 10:07:12 EDT
My personal take on this: I'm not a big fan of imposing rules and restrictions on what committers can and cannot do, preferring instead to defer to social convention.  This means that, in my fantasy world, the entire project would agree to not rewrite history unless there's a perfectly valid reason for doing so (removing private information committed accidentally, or bad IP).
Comment 3 Brian de Alwis CLA 2011-06-23 09:11:01 EDT
I think denyNonFastForwards default to true in recent versions of git, and the Git FAQ entry provides a good rationale:

https://git.wiki.kernel.org/index.php/GitFaq#Why_won.27t_.22git_push.22_work_after_I_rebase_a_branch.3F

I'd definitely recommend keeping or setting denyNonFastForwards=true.
Comment 4 David Williams CLA 2011-10-28 16:36:36 EDT
*** Bug 362358 has been marked as a duplicate of this bug. ***
Comment 5 David Williams CLA 2011-10-28 16:52:41 EDT
Created attachment 206156 [details]
list of 53 configs without denyNonFastForward

As indicated in bug 362358, there are 53 projects that are shared, but do not have denyNonFastforwards.
Comment 6 David Williams CLA 2011-10-28 16:54:02 EDT
(In reply to comment #5)
> Created attachment 206156 [details]
> list of 53 configs without denyNonFastForward
> 
> As indicated in bug 362358, there are 53 projects that are shared, but do not
> have denyNonFastforwards.

I should emphasize, this "list of 53" was arrived at via some simple greps. Perhaps they have other ways of preventing fast forwards via hooks, or something, that I do not know anything about.
Comment 7 David Williams CLA 2011-10-28 17:07:58 EDT
The URL in comment #3 isn't working right now (is kernel.org _still_ down?!) but here's another references that discusses the issues and has some pointers to the GitPro book. 

http://stackoverflow.com/questions/6704602/can-git-rebase-completely-remove-remote-history
Comment 8 David Williams CLA 2011-10-28 17:13:15 EDT
(In reply to comment #2)
> My personal take on this: I'm not a big fan of imposing rules and restrictions
> on what committers can and cannot do, preferring instead to defer to social
> convention.  This means that, in my fantasy world, the entire project would
> agree to not rewrite history unless there's a perfectly valid reason for doing
> so (removing private information committed accidentally, or bad IP).

I just wanted to comment I don't think this is really much about "rules and restrictions" as it is about protecting database integrity ... accidents, if nothing else, can happen and IMHO a source-code-repository should be very resistant to accidents. Keep in mind, we are talking here about "the main" repository ... if users want to have their own local one they can, which of course they'd have control over.
Comment 9 Markus Keller CLA 2011-10-29 11:23:44 EDT
A copy of the GitFaq wiki page can be found here:
http://wayback.archive.org/web/20110725033938/https://git.wiki.kernel.org/index.php/GitFaq#Why_won.27t_.22git_push.22_work_after_I_rebase_a_branch.3F

I agree that receive.denyNonFastForwards = true should be set for all canonical repositories hosted at eclipse.org. It can always be relaxed temporarily if a project lead or PMC has a good reason for rewriting history.
Comment 10 Dani Megert CLA 2011-10-31 06:19:40 EDT
+1.
Comment 11 David Williams CLA 2011-12-13 22:57:32 EST
Webmasters, 

  what will it take to get this protection put in place? 

  The only complication might be, that some have mentioned, elsewhere, that branches of the form "developerid/.*" should have "no rules" since this are not meant to be canonical and developers might enjoy more the "git hub" experience for those "personal" branches as a working or sandbox area. 

  I personally think the "database protection" should come first ... and treat the "sandbox experience" as a feature enhancement request, if there's any difference in time, in getting the two done. (such as one might be a property setting, somewhere, the other might have to be part of the "commit script" or something? 

  What steps are required to drive this to resolution? 

Thanks,
Comment 12 Denis Roy CLA 2011-12-14 10:12:02 EST
(In reply to comment #11)
> Webmasters, 
> 
>   what will it take to get this protection put in place? 

Consensus?  I only see four different people who are in on this discussion.  Before doing a change this big to something as important as our SCM, I would appreciate it if the issue was socialized to the wider audience (eclipse.org-committers?)

In the end, if everyone agrees this is the way to go, we'll make it happen.
Comment 13 Wayne Beaton CLA 2011-12-14 10:26:14 EST
(In reply to comment #12)
> (In reply to comment #11)
> > Webmasters, 
> > 
> >   what will it take to get this protection put in place? 
> 
> Consensus?  I only see four different people who are in on this discussion. 
> Before doing a change this big to something as important as our SCM, I would
> appreciate it if the issue was socialized to the wider audience
> (eclipse.org-committers?)
> 
> In the end, if everyone agrees this is the way to go, we'll make it happen.

I'll be sending a note out the members and committers later today; I'll add this discussion on the note.
Comment 14 David Williams CLA 2011-12-14 11:02:46 EST
> 
> Consensus?  I only see four different people who are in on this discussion. 
> Before doing a change this big to something as important as our SCM, I would
> appreciate it if the issue was socialized to the wider audience
> (eclipse.org-committers?)
> 

I think it is great if you want to socialize to solicit other input/feedback, but

a) some of the links/references above do document "consensus in the industry" (of git experts). What's the counter arguments to them?  

b) I'd suggest a deadline be set, say 1/15/2012 by which if there's no objections or counter arguments, the advice of the experts be followed. 

c) some things, such as database integrity should not be up for "popular vote" ... at least, if git is intended as a replacement for cvs at Eclipse, which was/is very safe about "keeping history". 

Git, I'm coming to learn, is well known for "letting you do anything" which means you need to setup/implement the policies you want to follow. Whereas CVS used to "set the policy" for you, so to speak. But, I think our policy should be the same as it was with CVS ... protect the the code history of the canonical repository. 

So ... I just wanted to be sure this was being driven to resolution one way or the other. I don't think it's a bug or enhancement, per se, but more a policy issue. What issue is desired? Protect code history? Let committers change code history if they want? 

Suggestions or counter arguments to clarify my misunderstandings would be welcome.
Comment 15 Dani Megert CLA 2011-12-14 11:27:37 EST
Just FYI: the 'Eclipse' top-level project PMC decided that we want this for all Git repositories in our project. We hope that this will become the default policy after the poll and Denis will actually do the work for us by adding this to all repositories after the poll. If not, we will do it ourselves soon after the X-Mas holidays.
Comment 16 Denis Roy CLA 2011-12-14 11:35:17 EST
(In reply to comment #14)
> I think it is great if you want to socialize to solicit other input/feedback,
> but
> 
> a) some of the links/references above do document "consensus in the industry"
> (of git experts). What's the counter arguments to them?  

A wise man recently sent me a lengthy email regarding the recent change we made to Hudson, and how they felt we hadn't exposed the issue and the proposed change to a wide-enough audience for such an important piece of infrastructure, thus catching many people off-guard.

I'm not trying to argue with experts -- I'm simply trying to avoid a repeat of the above.


(In reply to comment #14)
> c) some things, such as database integrity should not be up for "popular vote"
> ... at least, if git is intended as a replacement for cvs at Eclipse, which
> was/is very safe about "keeping history". 

Not really.  Anyone with shell access can permanently rewrite history in CVS without anyone knowing about it.  You've done so yourself, recently...  in bug 361131 comment 8.



 
> So ... I just wanted to be sure this was being driven to resolution one way or
> the other. I don't think it's a bug or enhancement, per se, but more a policy
> issue. What issue is desired? Protect code history? Let committers change code
> history if they want? 

FWIW, I fully agree with you, and I tend to choose security over convenience.  But that is not always what the committers want or expect; hence the reason I wish to expose this issue to the broader audience before making a change to a global configuration.
Comment 17 Mike Norman CLA 2011-12-14 11:55:58 EST
My $0.02 worth ...

The ability to "re-write" history is not just a powerful capability of Git -
it is also <b>useful</b>. I have seen use cases that range from "Oy! We just
got acquired, lets get rid of all the curse-words in our commit history"
to "a series of REFs were 'borked' by a badly-configured git client and I
now have a commit-tree that represents reality; I need to force a push the repository"

I would not like to see this capability taken away.

Is it possible to write a 'clever' commit-hook that would under 'normal'
circumstances dis-allow such re-writes but if the commit message contains
'special' instructions (a new 'Super-Commit-by' field?) would allow
people to 'fix' their repository.
Comment 18 Paul Webster CLA 2011-12-14 12:02:28 EST
(In reply to comment #17)
> Is it possible to write a 'clever' commit-hook that would under 'normal'
> circumstances dis-allow such re-writes but if the commit message contains
> 'special' instructions (a new 'Super-Commit-by' field?) would allow
> people to 'fix' their repository.

We are looking at a receive hook in bug 362363 that by default would dis-allow deletes and re-writes in the "main" branches but allow whatever you want in the topic branches of the form <committerId>/branchWhatever.

The disastrous case of needing to fix something in master, say, can always be dealt with a bug request to temporarily remove the hooks or set the config properties to false.

PW
Comment 19 David Williams CLA 2011-12-14 12:39:49 EST
(In reply to comment #17)

> 
> I would not like to see this capability taken away.
> 

Yes, I'd like to emphasize, I am not advocating that the policy be that history never be re-written (period), just that the server settings and scripts should disallow it under all normal circumstances (for the canonical repos and branches). If the history of something (in canonical repo or branch) does need to be re-written for the types of exceptions mentioned, data safety seems worth the trade-off to do the extra work of opening a bug and document what's being done, why, and get the webmasters help to "lift the restrictions" temporarily ... all announced ahead of time if possible :) All normal Eclipse openness and transparency. Who knows ... I might want a say about what was a curse word and what wasn't :) or ... make sure all my distributed versions were up to date before the change, so I'd have a permanent record of all those curse words. :) 

Thanks for helping to clarify that this is more about "server settings" than some absolute rule.
Comment 20 Dani Megert CLA 2011-12-15 02:34:09 EST
(In reply to comment #13)
 
> I'll be sending a note out the members and committers later today; I'll add
> this discussion on the note.

Wayne, to which list(s) did you send this? I've not received it.
Comment 21 Wayne Beaton CLA 2011-12-15 09:09:42 EST
(In reply to comment #20)
> (In reply to comment #13)
> 
> > I'll be sending a note out the members and committers later today; I'll add
> > this discussion on the note.
> 
> Wayne, to which list(s) did you send this? I've not received it.

I tweeted about it yesterday. I did compose the note, but haven't sent it yet. I'll send it out to the members-committers list.
Comment 22 Miles Parker CLA 2011-12-15 13:32:00 EST
Having myself stumbled across this git 'feature' and only making my way back
out of it by the skin of my teeth (and the help of Mac time Machine of all
things) I vote:

+1

The fact that it is possible to erase history has been one of my remaining
nagging concerns re: git.
Comment 23 Marcel Bruch CLA 2011-12-15 14:37:05 EST
(In reply to comment #9)
> I agree that receive.denyNonFastForwards = true should be set for all canonical
> repositories hosted at eclipse.org. It can always be relaxed temporarily if a
> project lead or PMC has a good reason for rewriting history.

+1 for "use save defaults but allow project leads to deactivate that (temporarily) w/o asking PMC for permission". I've good reasons to do certain things with Git. But it should prevent me from doing such things accidentally.
Comment 24 David Williams CLA 2011-12-15 15:04:26 EST
In his note to committers, Wayne characterized this as a feature "to not shoot yourself in the foot". If that was all, I'd still be in favor of it, but I think its more than that. The open source community (committers, adopters and products) rely on the history, in addition to the code. If the "history can change" then some in community would lose an important aspect of the integrity and safety in knowing what was there, what they were getting, and that it would remain there, for the many many years they might need it. 

I might be misunderstanding, as I don't know git well, but as I imagine an extreme case, a project could "release" with, say "tagR1" ... adopters pull that to build and include in their product. A few days later the project discovers a last minute bug they want to fix before the actual release, and then "force" the history to say some other change set was "tagR1". So, now, the adopter a) has no easy way to really know the code changed and b) has no easy way to tell what changed between the first and second "tagR1". Am I wrong? Am I misunderstanding this issue? I realize this might be an extreme example, "that no sane developer would do", but it is this kind of "changed history" that most concerns me, and why I think changed history should be documented in bugzillas, etc. As open and transparent as the code. 

I'd love to learn this sort of abuse of "history" was not possible ... but, from the (very) little I know, it sure sounds like it. So, I'd appreciate the education. 

Thanks so much to all who have taken the trouble to discuss this issue.
Comment 25 John Arthorne CLA 2011-12-15 15:54:13 EST
(In reply to comment #24)
> I might be misunderstanding, as I don't know git well, but as I imagine an
> extreme case, a project could "release" with, say "tagR1" ... adopters pull
> that to build and include in their product. A few days later the project
> discovers a last minute bug they want to fix before the actual release, and
> then "force" the history to say some other change set was "tagR1". So, now, the
> adopter a) has no easy way to really know the code changed and b) has no easy
> way to tell what changed between the first and second "tagR1". Am I wrong?

That's effectively right. What happens in practice is that tags are associated with commit hash codes. If a commit is rewritten, its hash changes, and now the tag just points off into space (some call it a zombie tag). Any build based on that tag would be broken.

I'm in favour of adding receive.denyNonFastForwards as a default setting on all git repositories immediately. I saw another case just today of someone accidentally force-pushing an old master branch state and wiping out other people's commits. A social convention to avoid this doesn't help when it is almost always done by accident ("Hmm, git push isn't working, maybe I'll try git push -f").

I believe there are valid cases for non-fastforward pushes. In particular I am working on my own topic branch, and I want to "clean it up" before pushing those changes to master. Nobody is affected by me cleaning up a branch that nobody is building or consuming. So I think we could look at selectively enabling non-FF using a hook. But, that is probably a longer term discussion and the details will vary by project. I suggest we start from a default of denyNonFastForward and projects can then selectively open that up as they see fit.
Comment 26 Chris Aniszczyk CLA 2011-12-15 16:06:46 EST
(In reply to comment #25)
> I'm in favour of adding receive.denyNonFastForwards as a default setting on all
> git repositories immediately. I saw another case just today of someone
> accidentally force-pushing an old master branch state and wiping out other
> people's commits. A social convention to avoid this doesn't help when it is
> almost always done by accident ("Hmm, git push isn't working, maybe I'll try
> git push -f").

+1 on having it as the default

> I believe there are valid cases for non-fastforward pushes. In particular I am
> working on my own topic branch, and I want to "clean it up" before pushing
> those changes to master. Nobody is affected by me cleaning up a branch that
> nobody is building or consuming. So I think we could look at selectively
> enabling non-FF using a hook. But, that is probably a longer term discussion
> and the details will vary by project. I suggest we start from a default of
> denyNonFastForward and projects can then selectively open that up as they see
> fit.

I don't know... with the ease of having a github mirror and your own fork... you can do anything you want to on your own fork before getting your changes back into the blessed repository (at eclipse.org). These changes would be visible and easily shareable without effecting the blessed repos.
Comment 27 Andrew Overholt CLA 2011-12-15 16:13:21 EST
(In reply to comment #26)
> I don't know... with the ease of having a github mirror and your own fork...
> you can do anything you want to on your own fork before getting your changes
> back into the blessed repository (at eclipse.org). These changes would be
> visible and easily shareable without effecting the blessed repos.

This effectively removes eclipse.org as the place people push their work and collaborate on experimental/new features.  I'd like to try to keep this work at eclipse.org as much as possible.  I really like GitHub but Eclipse activity looks pretty low if there are only release branches in the eclipse.org git repos.
Comment 28 Thomas Watson CLA 2011-12-15 16:22:05 EST
(In reply to comment #27)
> (In reply to comment #26)
> > I don't know... with the ease of having a github mirror and your own fork...
> > you can do anything you want to on your own fork before getting your changes
> > back into the blessed repository (at eclipse.org). These changes would be
> > visible and easily shareable without effecting the blessed repos.
> 
> This effectively removes eclipse.org as the place people push their work and
> collaborate on experimental/new features.  I'd like to try to keep this work at
> eclipse.org as much as possible.  I really like GitHub but Eclipse activity
> looks pretty low if there are only release branches in the eclipse.org git
> repos.

I agree with Andrew, committers should have freedom to do what they want with their committer topic branches before they are merged into a main development branch.  And I would like to allow such committer branches to be done at eclipse.org.
Comment 29 Denis Roy CLA 2011-12-15 16:51:07 EST
(In reply to comment #25)
> A social convention to avoid this doesn't help when it is
> almost always done by accident ("Hmm, git push isn't working, maybe I'll try
> git push -f").

When I made that suggestion in comment 2 last April, I was not aware that the default configuration left our repositories vulnerable to destruction so easily.

+1 for receive.denyNonFastForwards from me as well.


(In reply to comment #24)
> If the "history can
> change" then some in community would lose an important aspect of the integrity
> and safety in knowing what was there, what they were getting, and that it would
> remain there, for the many many years they might need it. 

I just want to reiterate the (perhaps) obvious: git, even in its current state, still offers better SCM history integrity than CVS ever has, or ever will (see comment 16).  I simply want to avoid a situation where CVS is artificially placed on a pedestal for having history and integrity that are above reproach.

Let's move forward with David's suggestion:  By 1/15/2012 if there's no
objections or counter arguments, the advice of the experts be followed and receive.denyNonFastForwards will be enabled globally for git.eclipse.org and all its repos.
Comment 30 Denis Roy CLA 2011-12-15 16:51:57 EST
> This effectively removes eclipse.org as the place people push their work and
> collaborate on experimental/new features.  I'd like to try to keep this work at
> eclipse.org as much as possible. 

+1
Comment 31 Henrik Rentz-Reichert CLA 2011-12-16 02:13:55 EST
+1 for receive.denyNonFastForwards
Comment 32 Martin Oberhuber CLA 2011-12-16 03:46:50 EST
+1 for denyNonFastForward, immediately

Lifting the restriction should only be possible by explicit bug request only, at least initially. Clever scripts / trigger don't seem to be worth the effort and risk. Unless at some time webmaster decides that the volume of exception request isn't manageable any more.

I'm usually in favor of soliciting input and advance notice, but this issue seems to call for quick action. Integrity of repositories is top important.
Comment 33 Markus Knauer CLA 2011-12-16 03:48:36 EST
+1 for receive.denyNonFastForwards
Comment 34 Tobias Oberlies CLA 2011-12-16 04:52:04 EST
(In reply to comment #24)
> I might be misunderstanding, as I don't know git well, but as I imagine an
> extreme case, a project could "release" with, say "tagR1" ... adopters pull that
> to build and include in their product. A few days later the project discovers a
> last minute bug they want to fix before the actual release, and then "force" the
> history to say some other change set was "tagR1". So, now, the adopter a) has no
> easy way to really know the code changed and b) has no easy way to tell what
> changed between the first and second "tagR1". Am I wrong?
This is really the critical case: Tags in the central repository must not updated/deleted  
(see "re-tagging" on [1] for a discussion on this topic).
Is this enforced on all eclipse.org git repositories?

Once this is in place, the setting of receive.denyNonFastForwards doesn't really matter, because it can only affect unreleased changes (assuming that all release/milestone/integration builds are tagged). When a commit is tagged, it doesn't matter if it is also part of a branch. So if you were to have master point to the first commit in your repositories, the sources for all release would still not be lost.

[1] http://man.he.net/man1/git-tag
Comment 35 Elias Volanakis CLA 2011-12-16 09:28:46 EST
+1 for receive.denyNonFastForwards
Comment 36 Denis Roy CLA 2011-12-16 09:32:16 EST
FWIW, 179 repos (out of 266 total) are currently configured with receive.denyNonFastForwards in their local config file.
Comment 37 Paul Webster CLA 2011-12-16 09:47:46 EST
(In reply to comment #32)
> Lifting the restriction should only be possible by explicit bug request only,
> at least initially. Clever scripts / trigger don't seem to be worth the effort
> and risk. Unless at some time webmaster decides that the volume of exception
> request isn't manageable any more.
> 

Using and sharing topic branches for larger bug fixes or small features within a team is at the core of the git workflow, and rebasing those topic branches often is part of that regular workflow (and causes a non-ff push).  Even my admittedly git-challenged team has already started taking advantage of this :-)

I'm all for setting the flag to spare us from accidents now, but the support for non-ff pushes on topic branches through the commit/receive hook scripts are actually a very important feature and can't lag behind turning on this flag by much (i.e. more than a couple of weeks).

It won't stop us from using the workflow for now (as we can just create topicBranch, topicBranchA, topicBranchB, etc and litter the repo) until the hook comes online.  They can always be deleted later.

PW
Comment 38 Paul Webster CLA 2011-12-16 09:52:24 EST
Arguably equally as important as receive.denyNonFastForwards is setting receive.denyDeletes=true bug 362361 (also expected to be relaxed by git hooks, bug 362363 )

PW
Comment 39 Eike Stepper CLA 2011-12-18 04:06:56 EST
+1 for anything that makes our repos safer.
Comment 40 David Williams CLA 2012-01-24 11:45:14 EST
(In reply to comment #29)
> (In reply to comment #25)

> Let's move forward with David's suggestion:  By 1/15/2012 if there's no
> objections or counter arguments, the advice of the experts be followed and
> receive.denyNonFastForwards will be enabled globally for git.eclipse.org and
> all its repos.

Well ... happy new year! :) Here it is past 1/15 ... how's progress? I'm in no urgent rush ... just didn't want this to be forgotten or put off too long (It gets more important as more projects use git, and as we git closer to Juno release). 

Since Juno M5 concludes on 2/3, perhaps a good time to make a "large" change would be the following Monday, 2/6. We would still be in the middle of Indigo SR2, but that doesn't conclude until (nearly) the end of February, which seems too long to wait. [But, I am perfectly open to others suggestions of when to make the change, as well as, of course, our webmaster's schedules.] 

I wanted to also say I agree with Paul's comment 38 that receive.denyDeletes=true is just as important. (I read once on a blog somewhere about a "tip" how to get around the "denyNonFastForwards" setting on (some other) server was to use delete and then repush :\ ) 

These (denyNonFastForwards and denyDeletes) ended up being separate bugs in part because I thought there were separate actions required to fix each ... and maybe there still are, for all I know, but perhaps some of the hook scripts such as in bug 362363 might provide a way to "centralize" some of the "rules" that we at Eclipse want to follow?
Comment 41 Dani Megert CLA 2012-02-01 11:45:41 EST
Denis, can you put this in place now? Otherwise we're going to "fix" our repos by hand.
Comment 42 Denis Roy CLA 2012-02-01 15:33:04 EST
I've created /etc/gitconfig which contains the following:

[core]
        filemode = true
        bare = true
        sharedrepository = 1
[receive]
        denyNonFastforwards = 1



Let me know if that does it.
Comment 43 John Arthorne CLA 2012-02-14 13:50:16 EST
(In reply to comment #42)
> I've created /etc/gitconfig which contains the following:
> 
> [core]
>         filemode = true
>         bare = true
>         sharedrepository = 1
> [receive]
>         denyNonFastforwards = 1
> 
> 
> 
> Let me know if that does it.

This doesn't seem to be working. On Orion we have been able to do non-fastforward pushes in the past couple of days. I don't know if this means anything because I might not have permission, but when I try listing git system config settings, it is looking for a different file path than the one you mentioned:

johna@build:/gitroot/orion/org.eclipse.orion.client.git> git config --system --list
fatal: unable to read config file '/usr/local/etc/gitconfig': No such file or directory
Comment 44 Paul Webster CLA 2012-02-14 13:52:18 EST
(In reply to comment #43)

I find the same thing, I can't see the "system" settings in any of my repos.

PW
Comment 45 Denis Roy CLA 2012-02-16 15:12:04 EST
> fatal: unable to read config file '/usr/local/etc/gitconfig': No such file or
> directory

I've fixed that.
Comment 46 John Arthorne CLA 2012-02-21 11:09:48 EST
(In reply to comment #45)
> > fatal: unable to read config file '/usr/local/etc/gitconfig': No such file or
> > directory
> 
> I've fixed that.

Just to confirm, this has done the trick. We have some automated tests in Orion that attempt non-fastforward pushes and they started failing after you made this change. So, it looks like all eclipse.org Git repositories are now set to deny non-fastforward, unless overwritten manually in a local repository configuration. I think this can be marked fixed. Thanks Denis!
Comment 47 Markus Keller CLA 2012-03-12 07:05:07 EDT
This doesn't seem to work.

Although I see the receive.denynonfastforwards property, I could still force-push an amended commit to eclipse.jdt.ui and to eclipse.platform.text.

Concrete scenario: I pushed a commit where I was the author and then amended it locally with the correct author. When I pushed again, I got the non-fastForward error message (good), but pushing refs/heads/master with the force option succeeded (bad).

mkeller@build:/gitroot/jdt/eclipse.jdt.ui.git> git config -l --local
core.repositoryformatversion=0
core.filemode=true
core.bare=true
core.sharedrepository=1
mkeller@build:/gitroot/jdt/eclipse.jdt.ui.git> git config -l --system
core.filemode=true
core.bare=true
core.sharedrepository=1
receive.denynonfastforwards=1

Could it be that the hooks/update script overrides the receive.denynonfastforwards?
Comment 48 Markus Keller CLA 2012-03-12 07:11:43 EDT
> pushing refs/heads/master with the force option succeeded (bad).

I did this via EGit's Push... dialog, with "Force Update" checked.
Comment 49 Paul Webster CLA 2012-03-12 08:00:19 EDT
(In reply to comment #48)
> 
> I did this via EGit's Push... dialog, with "Force Update" checked.

I do a forced push with the command line git on my topic branches regularly as well (I thought it just worked that way :-)

PW
Comment 50 Denis Roy CLA 2012-03-12 09:20:23 EDT
> Concrete scenario: I pushed a commit where I was the author and then amended it
> locally with the correct author. When I pushed again, I got the non-fastForward
> error message (good), but pushing refs/heads/master with the force option
> succeeded (bad).

Could it be that the --force option overrides the receive.denynonfastforwards ?  I looked at tons of docs and I can't say for sure if it does or doesn't.


> Could it be that the hooks/update script overrides the
> receive.denynonfastforwards?

I don't know.  I look at the source and I can't really tell.  I can move away the hook so you can try pushing a non-fast-forward commit again to see what happens.
Comment 51 Markus Keller CLA 2012-03-12 09:42:56 EDT
All information I found in "man git-push" and "man git-config" tell that -force is only a client-side option and that receive.denyNonFastForwards should block non-fast-forwards on the server (search for "deny" in the man pages).

We're currently in the endgame for Juno M6 and I think it's better to wait with experiments on the live repositories until M6 is declared (2012-03-19).
Comment 52 Denis Roy CLA 2012-03-12 09:52:20 EDT
I've copied eclipse.jdt.ui.git as /gitroot/jdt/denis.jdt.ui.git and I've removed the update hook.  Everything else is the same.  Is there any way you can repeat your operation in that test repo?
Comment 53 BJ Hargrave CLA 2012-03-12 10:27:36 EDT
(In reply to comment #50)
> Could it be that the --force option overrides the receive.denynonfastforwards ?
>  I looked at tons of docs and I can't say for sure if it does or doesn't.
> 

On the OSGi server, I can confirm that the server does not care whether the client says --force if denyNonFastForwards is set to true. The server will reject a non fast forward push.


Also, I don't see that the repo is configured for denyNonFastForward:

$ ssh git.eclipse.org git config -f /gitroot/jdt/eclipse.jdt.ui.git/config -l
core.repositoryformatversion=0
core.filemode=true
core.bare=true
core.sharedrepository=1

Here is the Equinox Framework config which has denyNonFastForward set:

$ ssh git.eclipse.org git config -f /gitroot/equinox/rt.equinox.framework.git/config -l
core.repositoryformatversion=0
core.filemode=true
core.bare=true
core.sharedrepository=1
receive.denynonfastforwards=true
Comment 54 John Arthorne CLA 2012-03-12 10:33:22 EDT
(In reply to comment #53)
> Also, I don't see that the repo is configured for denyNonFastForward:

It is set as a system property:


johna@build:/gitroot/jdt/eclipse.jdt.ui.git> git config --system -l
core.filemode=true
core.bare=true
core.sharedrepository=1
receive.denynonfastforwards=1

johna@build:/gitroot/jdt/eclipse.jdt.ui.git> git config -l
core.filemode=true
core.bare=true
core.sharedrepository=1
receive.denynonfastforwards=1
core.repositoryformatversion=0
core.filemode=true
core.bare=true
core.sharedrepository=1
Comment 55 Markus Keller CLA 2012-03-15 17:12:59 EDT
(In reply to comment #52)
> I've copied eclipse.jdt.ui.git as /gitroot/jdt/denis.jdt.ui.git and I've
> removed the update hook.  Everything else is the same.  Is there any way you
> can repeat your operation in that test repo?

- origin/master was commit e0b0a2c4fcec0e789737791caf4143efda6d0d2a
- reset HEAD to parent commit f52357ded2beb1b4fe74a49ac94f928dc8a49f97
- committed a new file /org.eclipse.jdt.astview/deviating.txt
- pushed to upstream
=> error (good): master : master [rejected - non-fast-forward]

- manually pushed master branch with Force Update checked
=> was allowed (bad)
Comment 56 Markus Keller CLA 2012-03-15 17:15:44 EDT
D'oh! We're all blind!

receive.denyNonFastForwards is a boolean option and needs "true", not "1".

Denis, please run this:
root$ git config --system receive.denyNonFastForwards true
Comment 57 Markus Keller CLA 2012-03-15 17:32:04 EDT
Ha, I did this:

mkeller@build:/gitroot/jdt/denis.jdt.ui.git> git config receive.denyNonFastForwards true


Then I amended the last commit again and tried to force-push, and now I get the expected rejection:
------------------------------------------------------------------------
Repository ssh://mkeller@git.eclipse.org/gitroot/jdt/denis.jdt.ui.git

non-fast-forward
error: denying non-fast-forward refs/heads/master (you should pull
first)
------------------------------------------------------------------------
Comment 58 Denis Roy CLA 2012-03-15 19:19:07 EDT
Duh... 

droy@dev1:~> git config -l
core.filemode=true
core.sharedrepository=1                                                           receive.denynonfastforwards=true 


Try now  :)
Comment 59 Markus Keller CLA 2012-03-16 08:14:07 EDT
I did some more experiments and found this:

1. The receive.denyNonFastForwards option does *not* work at the system level. I can still force push non-fast-forward updates.

Maybe that's what the sentence "This configuration variable is set when initializing a shared repository." in the git-config manpage is supposed to mean -- quite confusing if you ask me...

2. When I set receive.denyNonFastForwards=true in the repository, then bad commits are rejected as expected.

3. The "true" vs. "1" was a red herring. Both values work equally fine (but only in the repo, not globally).

=> We have to set receive.denyNonFastForwards=true in each repository.
Comment 60 Denis Roy CLA 2012-03-16 09:44:19 EDT
Perhaps having it in the system config allows the setting to be enabled for new repos... 

$ cd /gitroot/test
$ initrepo test.git
Initialized empty shared Git repository in /gitroot/test/test.git/
$ grep deny /gitroot/test/test.git/config
        denyNonFastforwards = true

Out of the 342 repos in /gitroot 218 already have denyNonFastForwards = true.
Comment 61 Dani Megert CLA 2012-03-21 11:08:45 EDT
(In reply to comment #60)
> Perhaps having it in the system config allows the setting to be enabled for new
> repos... 
> 
> $ cd /gitroot/test
> $ initrepo test.git
> Initialized empty shared Git repository in /gitroot/test/test.git/
> $ grep deny /gitroot/test/test.git/config
>         denyNonFastforwards = true
> 
> Out of the 342 repos in /gitroot 218 already have denyNonFastForwards = true.

I filed bug 374921 which requests to enable it on all Eclipse top-level projects manually.
Comment 62 Denis Roy CLA 2012-03-21 11:12:08 EDT
I'll close this as "works for me"...   The param is set in the system config, which gets inherited when new repos are init'ed, but the setting can be removed by project teams if they wish to do so.  Perhaps they have a good reason to remove the setting temporarily.

Reopen if you somehow feel like we should be forcibly imposing this on every repo.
Comment 63 Paul Webster CLA 2012-08-25 08:15:05 EDT
the system setting doesn't seem to work on git.eclipse.org.  See bug 387212

PW