# [PATCH] Make 'cvs -n commit ...' not to commit 3 messages from 2012-03-22 to 2012-03-23. Participants: ericc, Junio C Hamano, Eric Chamberland. Thread: https://gitlist.dev/t/30036 ## ericc, 2012-03-22 20:57 Subject: [PATCH] Make 'cvs -n commit ...' not to commit Message-ID: <20120323131100.7262D440B33@melkor.giref.ulaval.ca> URL: https://gitlist.dev/e/20120323131100.7262D440B33%40melkor.giref.ulaval.ca ``` Actually, doing a 'cvs -n commit' will _do_ the commit... With this patch, it now goes through the code, but don't do the commit. A further progress would be to do the pre-commit hook is possible... Eric Chamberland --- git-cvsserver.perl | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/git-cvsserver.perl b/git-cvsserver.perl index b8eddab..67ec4d0 100755 --- a/git-cvsserver.perl +++ b/git-cvsserver.perl @@ -1395,6 +1395,9 @@ sub req_ci push @committedfiles, $committedfile; $log->info("Committing $filename"); + # Don't want to actually _DO_ the update if -n specified + unless ( $state->{globaloptions}{-n} ) + { system("mkdir","-p",$dirpart) unless ( -d $dirpart ); unless ( $rmflag ) @@ -1424,6 +1427,7 @@ sub req_ci $log->info("Updating file '$filename'"); system("git", "update-index", $filename); } + } } unless ( scalar(@committedfiles) > 0 ) @@ -1434,6 +1438,9 @@ sub req_ci return; } + # Don't want to actually _DO_ the update if -n specified + unless ( $state->{globaloptions}{-n} ) + { my $treehash = `git write-tree`; chomp $treehash; @@ -1537,7 +1544,7 @@ sub req_ci print "/$filepart/1.$meta->{revision}//$kopts/\n"; } } - + } cleanupWorkTree(); print "ok\n"; } -- 1.7.3.4 ``` ## Junio C Hamano, 2012-03-23 18:39 Subject: Re: [PATCH] Make 'cvs -n commit ...' not to commit Message-ID: <7vhaxftb54.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhaxftb54.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120323131100.7262D440B33@melkor.giref.ulaval.ca> ``` ericc writes: > Actually, doing a 'cvs -n commit' will _do_ the commit... > With this patch, it now goes through the code, but don't do the commit. OK. > A further progress would be to do the pre-commit hook is possible... It is not clear what you meant here. > Eric Chamberland > --- > git-cvsserver.perl | 9 ++++++++- > 1 files changed, 8 insertions(+), 1 deletions(-) > > diff --git a/git-cvsserver.perl b/git-cvsserver.perl > index b8eddab..67ec4d0 100755 > --- a/git-cvsserver.perl > +++ b/git-cvsserver.perl > @@ -1395,6 +1395,9 @@ sub req_ci > push @committedfiles, $committedfile; > $log->info("Committing $filename"); > > + # Don't want to actually _DO_ the update if -n specified > + unless ( $state->{globaloptions}{-n} ) > + { > system("mkdir","-p",$dirpart) unless ( -d $dirpart ); > > unless ( $rmflag ) > @@ -1424,6 +1427,7 @@ sub req_ci > $log->info("Updating file '$filename'"); > system("git", "update-index", $filename); > } > + } > } I understand that you tried to make the patch smaller by avoiding re-indenting, but this is *yucky*. It looks to me that the above part could be solved with: unless (...) { next; } I think the function being patched is too big. Wouldn't it be better to have a refactoring patch to move the above per-path logic to a helper function that deals with a single path, and then insert the "omit call to that helper when run with -n" code in a separate patch? The same comment applies to the other hunk. Also I notice that the indentation used throughout the file is somewhat broken (e.g. "Emulate by running hooks/update" part is indented to 8 columns, but earlier parts use 4 space indent). The right structure for this change may be: Patch 1: Fix indentation (and do nothing else) to uniformly indent with HT; Patch 2: Refactor this big funciton using a handful of helper functions (and do nothing else); Patch 3: Omit calls to these helper functions under -n option. > @@ -1434,6 +1438,9 @@ sub req_ci > return; > } > > + # Don't want to actually _DO_ the update if -n specified > + unless ( $state->{globaloptions}{-n} ) > + { > my $treehash = `git write-tree`; > chomp $treehash; > > @@ -1537,7 +1544,7 @@ sub req_ci > print "/$filepart/1.$meta->{revision}//$kopts/\n"; > } > } > - > + } > cleanupWorkTree(); > print "ok\n"; > } ``` ## Eric Chamberland, 2012-03-23 19:02 Subject: Re: [PATCH] Make 'cvs -n commit ...' not to commit Message-ID: <4F6CC8AC.4050907@giref.ulaval.ca> URL: https://gitlist.dev/e/4F6CC8AC.4050907%40giref.ulaval.ca In-Reply-To: <7vhaxftb54.fsf@alter.siamese.dyndns.org> ``` On 03/23/2012 02:39 PM, Junio C Hamano wrote: > ericc writes: > >> Actually, doing a 'cvs -n commit' will _do_ the commit... >> With this patch, it now goes through the code, but don't do the commit. > > OK. > >> A further progress would be to do the pre-commit hook is possible... Sorry, I wanted to write: "A further progress would be to do the pre-commit hook *if* possible..." here, we are used to do "cvs -n commit" just to check if the "hooks" on the cvs server will fail or not... > > I understand that you tried to make the patch smaller by avoiding > re-indenting, but this is *yucky*. > > It looks to me that the above part could be solved with: > > unless (...) { > next; > } > > I think the function being patched is too big. Wouldn't it be better to > have a refactoring patch to move the above per-path logic to a helper > function that deals with a single path, and then insert the "omit call to > that helper when run with -n" code in a separate patch? > > The same comment applies to the other hunk. > > Also I notice that the indentation used throughout the file is somewhat > broken (e.g. "Emulate by running hooks/update" part is indented to 8 > columns, but earlier parts use 4 space indent). The right structure for > this change may be: > > Patch 1: Fix indentation (and do nothing else) to uniformly indent with > HT; > > Patch 2: Refactor this big funciton using a handful of helper functions > (and do nothing else); > > Patch 3: Omit calls to these helper functions under -n option. > > Ok you are right... These were my very first lines in Perl... I just wanted to catch the attention of someone who is able to do the changes correctly... and in a more clean way than I... Eric ```