{"thread":{"id":"30144","subject":"[PATCH] prohibit \"svn dcommit\" on remote-tracking-branches","startedAt":"2012-04-04T08:42:34Z","lastAt":"2012-04-15T21:23:58Z","messageCount":5,"participants":["Christian Engwer","Eric Wong","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"188478","messageId":"20120404084234.GA17768@mathe-macht-spass.de","threadId":"30144","inReplyTo":null,"subject":"[PATCH] prohibit \"svn dcommit\" on remote-tracking-branches","fromName":"Christian Engwer","fromEmail":"christian.engwer@wwu.de","sentAt":"2012-04-04T08:42:34Z","receivedAt":"2012-04-04T08:42:34Z","isPatch":true,"sender":{"key":"christian.engwer@wwu.de","avatar":null},"body":"A branch should either be a local copy of an svn branch, or a remote\ntracking branch. After a \"git svn dcommit\" a remote tracking branch\ncould not be synced with the git remote due to the rebase that occured\nduring the dcommit. Thus we check for a remote entry in the git config\nfor the current branch and prohibit the \"dcommit\" if such an entry\nexists.\n---\n git-svn.perl |   17 +++++++++++++++++\n 1 files changed, 17 insertions(+), 0 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4334b95..f9c8440 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -759,6 +759,23 @@ sub cmd_dcommit {\n \t\t'Cannot dcommit with a dirty index.  Commit your changes first, '\n \t\t. \"or stash them with `git stash'.\\n\";\n \t$head ||= 'HEAD';\n+    my ($branches,$ctx) = eval { command_output_pipe('branch') };\n+    my $branch = undef;\n+    while (<$branches>) {\n+        chomp;\n+        if (s/^\\* +//)\n+        {\n+            $branch = $_;\n+        }\n+    }\n+\tcommand_close_pipe($branches, $ctx);\n+\n+    my $remote = eval { command_oneline('config', '--get',\n+                                      \"branch.${branch}.remote\") };\n+\tif ($remote) {\n+\t\tdie \"You specified a non-svn remote for branch ${branch}.\\n\".\n+            \"To change this setting, modify \\\"branch.${branch}.remote\\\" using git config.\\n\";\n+\t}\n \n \tmy $old_head;\n \tif ($head ne 'HEAD') {\n-- \n1.7.9.1\n"},{"id":"188910","messageId":"20120410211732.GB27555@dcvr.yhbt.net","threadId":"30144","inReplyTo":"20120404084234.GA17768@mathe-macht-spass.de","subject":"Re: [PATCH] prohibit \"svn dcommit\" on remote-tracking-branches","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-04-10T21:17:32Z","receivedAt":"2012-04-10T21:17:32Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Christian Engwer <christian.engwer@wwu.de> wrote:\n> A branch should either be a local copy of an svn branch, or a remote\n> tracking branch. After a \"git svn dcommit\" a remote tracking branch\n> could not be synced with the git remote due to the rebase that occured\n> during the dcommit. Thus we check for a remote entry in the git config\n> for the current branch and prohibit the \"dcommit\" if such an entry\n> exists.\n\nShould there be an option to force/override this?\n\ngit-svn predates remote tracking branches, and I've never gotten in the\nhabit using remote tracking branches.  I'll wait for others to chime\nin...\n\n> +        if (s/^\\* +//)\n> +        {\n\nstyle, opening brace should be on the same line as the if/while/for:\n\n\tif (...) {\n\nAlso, indentation should be with hard tabs.  (Basically follow existing\nstyle conventions when you see them).  Thanks.\n"},{"id":"189339","messageId":"20120415134051.GA16786@mathe-macht-spass.de","threadId":"30144","inReplyTo":"20120410211732.GB27555@dcvr.yhbt.net","subject":"Re: [PATCH] prohibit \"svn dcommit\" on remote-tracking-branches","fromName":"Christian Engwer","fromEmail":"christian.engwer@wwu.de","sentAt":"2012-04-15T13:40:51Z","receivedAt":"2012-04-15T13:40:51Z","isPatch":true,"sender":{"key":"christian.engwer@wwu.de","avatar":null},"body":"Hi,\n\nwe had a litte discussion on this topic in the debian bug-tracker:\n\nhttp://bugs.debian.org/cgi-bin/bugreport.cgi?bug=667488\n\n> > A branch should either be a local copy of an svn branch, or a remote\n> > tracking branch. After a \"git svn dcommit\" a remote tracking branch\n> > could not be synced with the git remote due to the rebase that occured\n> > during the dcommit. Thus we check for a remote entry in the git config\n> > for the current branch and prohibit the \"dcommit\" if such an entry\n> > exists.\n> \n> Should there be an option to force/override this?\n\nThe suggestion was implement this as a hook, instead of being the\ndefault behavior. An other option is to use the proposed syntax and\nallow an explicit overwrite by setting the commiturl, or to add a\ncompletely new option, in order to avoid confusion.\n\nOpinions?\n\n> git-svn predates remote tracking branches, and I've never gotten in the\n> habit using remote tracking branches.  I'll wait for others to chime\n> in...\n> \n> > +        if (s/^\\* +//)\n> > +        {\n> \n> style, opening brace should be on the same line as the if/while/for:\n> \n> \tif (...) {\n> \n> Also, indentation should be with hard tabs.  (Basically follow existing\n> style conventions when you see them).  Thanks.\n\nI'll update the patch.\n\nCheers\nChristian\n"},{"id":"189343","messageId":"20120415185535.GA9150@mathe-macht-spass.de","threadId":"30144","inReplyTo":"20120410211732.GB27555@dcvr.yhbt.net","subject":"Re: [PATCH] prohibit \"svn dcommit\" on remote-tracking-branches","fromName":"Christian Engwer","fromEmail":"christian.engwer@wwu.de","sentAt":"2012-04-15T18:55:35Z","receivedAt":"2012-04-15T18:55:35Z","isPatch":true,"sender":{"key":"christian.engwer@wwu.de","avatar":null},"body":"Hi!\n\nOn Tue, Apr 10, 2012 at 09:17:32PM +0000, Eric Wong wrote:\n> Christian Engwer <christian.engwer@wwu.de> wrote:\n> > A branch should either be a local copy of an svn branch, or a remote\n> > tracking branch. After a \"git svn dcommit\" a remote tracking branch\n> > could not be synced with the git remote due to the rebase that occured\n> > during the dcommit. Thus we check for a remote entry in the git config\n> > for the current branch and prohibit the \"dcommit\" if such an entry\n> > exists.\n> \n> Should there be an option to force/override this?\n\nAs stated there is the alternative idea to use pre-dcommit-hook.\n\nI prepared an updated version which behaves as follows:\n\na) prohibit commit if a remote-tracking branch is used\nb) allow commit if --commit-url is used\nc) allow commit if the allow-dcommit flag is true in .git/config\n\nPlease give comments...\n\n<snip/>\n\nCheers\nChristian\n\n\n\n>From 1e1e151c358b9a8c472e70eaf7aa3f6855554f6c Mon Sep 17 00:00:00 2001\nFrom: Christian Engwer <christian.engwer@wwu.de>\nDate: Sun, 15 Apr 2012 19:27:55 +0200\nSubject: [PATCH] prohibit \"svn dcommit\" on remote-tracking-branches\n\nA branch should either be a local copy of an svn branch, or a remote\ntracking branch. After a \"git svn dcommit\" a remote tracking branch\ncould not be synced with the git remote due to the rebase that occured\nduring the dcommit. Thus we check for a remote entry in the git config\nfor the current branch and prohibit the \"dcommit\" if such an entry\nexists.\n\nThis behaviour can be overwritten by either providing an explicit commit url\non the command line via \"--commit-url\" or by setting branch.${branch}.allow-dcommit\nto true.\n---\n git-svn.perl |   24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4334b95..4a334b9 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -759,6 +759,30 @@ sub cmd_dcommit {\n \t\t'Cannot dcommit with a dirty index.  Commit your changes first, '\n \t\t. \"or stash them with `git stash'.\\n\";\n \t$head ||= 'HEAD';\n+\tmy ($branches,$ctx) = eval { command_output_pipe('branch') };\n+\tmy $branch = undef;\n+\twhile (<$branches>) {\n+\t\tchomp;\n+\t\tif (s/^\\* +//) {\n+\t\t\t$branch = $_;\n+\t\t}\n+\t}\n+\tcommand_close_pipe($branches, $ctx);\n+\n+\tmy $allowdcommit = eval { command_oneline('config', '--get',\n+\t\t\t\t\t\t\t\t\t\t      \"branch.${branch}.allow-dcommit\") };\n+\n+\tif ((! defined $_commit_url) and $allowdcommit ne 'true') {\n+\t\tmy $remote = eval { command_oneline('config', '--get',\n+\t\t\t\t\t\t\t\t\t\t  \"branch.${branch}.remote\") };\n+\t\tif ($remote) {\n+\t\t\tdie \"You specified a non-svn remote for branch ${branch}.\\n\".\n+\t\t\t\t\"To commit to an svn repository, you can either remove the\\n\".\n+\t\t\t\t\"  \\\"branch.${branch}.remote\\\" entry, or explicitly set a commit url\\n\".\n+\t\t\t\t\"  on the command-line via \\\"--commit-url\\\" or override the bahavior\\n\".\n+\t\t\t\t\"  by setting \\\"branch.${branch}.allow-dcommit = true\\\" via git config\";\n+\t\t}\n+\t}\n \n \tmy $old_head;\n \tif ($head ne 'HEAD') {\n-- \n1.7.9.5\n\n"},{"id":"189361","messageId":"20120415212358.GA5813@burratino","threadId":"30144","inReplyTo":"20120415185535.GA9150@mathe-macht-spass.de","subject":"Re: [PATCH] prohibit \"svn dcommit\" on remote-tracking-branches","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-15T21:23:58Z","receivedAt":"2012-04-15T21:23:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(dropping cc to debian bug)\nHi Christian,\n\nChristian Engwer wrote:\n\n> a) prohibit commit if a remote-tracking branch is used\n> b) allow commit if --commit-url is used\n> c) allow commit if the allow-dcommit flag is true in .git/config\n\nAs I mentioned before, I really do want to be able to dcommit from my\ncurrent branch, even if it is not called \"master\" and is set up,\nintentionally or not, to pull from a remote git repository.  I would\nbe a bit bewildered if git required me to say \"yes, please do perform\nthe action I already requested of you\" in order to do that, and I do\nnot think I am the only one.\n\nOn the other hand, I believe it would be useful to be able to prevent\n_all_ rebasing on some branches to avoid fat-finger accidents.  It\nwould be especially useful for new users who do not know how to\nrecover.  This includes the rebasing performed using \"git rebase\" or\n\"git reset\" by \"git svn rebase\".\n\nTherefore I would be much happier to see git svn learn a pre-dcommit\nhook (see githooks(5)) that can handle multiple situations than the\ncheck that follows the particular policy you have specified above.\n\nThat said.  I am reluctant to make the following comment because\nI really would love to have that pre-dcommit hook and I do not want to\ntake away a use case for it. ;-)\n\nUntil some interested person introduces a pre-dcommit hook, would it\nmake sense in your deployment to add a wrapper around \"git svn\ndcommit\" (let's call it git-commit-to-svn) on the $PATH that performs\nthe check itself, so the user experience would be as follows?\n\n    $ git commit-to-svn\n    fatal: this branch is set up to push to git, not to svn\n    hint: switch to a new branch before pushing\n    hint: or use \"git commit-to-svn --force\" to override this check\n\nThat already works.  It does not require any change to git.  What do\nyou think?\n\nJust my two cents,\nJonathan\n"}]}