Re: [msysGit] [PATCH] Do not call built-in aliases from scripts
On Thu, Jun 27, 2013 at 8:52 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: --- a/git-merge-octopus.sh +++ b/git-merge-octopus.sh @@ -97,7 +97,7 @@ do if test $? -ne 0 then echo Simple merge did not work, trying automatic merge. - git-merge-index -o git-merge-one-file -a || + git merge-index -o git-merge-one-file -a || This is a problem. 'git-merge-one-file' cannot be split here AFAICT. Of course, we could teach merge-index to read *two* parameters instead of one when it encounters git as the merge-program. But that would be as hacky as the whole dashed-form business to begin with. I agree to all of your comments except this one: I did not split 'git-merge-one-file' here ... diff --git a/git-merge-resolve.sh b/git-merge-resolve.sh index c9da747..343fe7b 100755 --- a/git-merge-resolve.sh +++ b/git-merge-resolve.sh @@ -45,7 +45,7 @@ then exit 0 else echo Simple merge failed, trying Automatic merge. - if git-merge-index -o git-merge-one-file -a + if git merge-index -o git-merge-one-file -a As above, with -octopus. Sorry, I can't follow you here. -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH] Do not call built-in aliases from scripts
Hi Sebastian, On Fri, 28 Jun 2013, Sebastian Schuberth wrote: On Thu, Jun 27, 2013 at 8:52 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: --- a/git-merge-octopus.sh +++ b/git-merge-octopus.sh @@ -97,7 +97,7 @@ do if test $? -ne 0 then echo Simple merge did not work, trying automatic merge. - git-merge-index -o git-merge-one-file -a || + git merge-index -o git-merge-one-file -a || This is a problem. 'git-merge-one-file' cannot be split here AFAICT. Of course, we could teach merge-index to read *two* parameters instead of one when it encounters git as the merge-program. But that would be as hacky as the whole dashed-form business to begin with. I agree to all of your comments except this one: I did not split 'git-merge-one-file' here ... I know. That is what I pointed out. git-merge-one-file was *not* un-dashed. And I explained the reason, too. But if we really want to solve the problem you described earlier, we also must not rely on git-merge-one-file to be present in libexec. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH] Do not call built-in aliases from scripts
Hi Sebastian,
On Thu, 27 Jun 2013, Sebastian Schuberth wrote:
Call built-in commands via the main executable (non-dashed form) without
relying on the aliases (dashed form) to be present. On some platforms,
e.g. those that do not properly support file system links, it is
inconvenient to ship the built-in aliases, so do not depend on their
presence.
A laudable goal!
diff --git a/git-am.sh b/git-am.sh
index 9f44509..ad67194 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -16,8 +16,8 @@ s,signoff add a Signed-off-by line to the commit
message
u,utf8 recode into utf8 (default)
k,keep pass -k flag to git-mailinfo
keep-non-patch pass -b flag to git-mailinfo
-keep-cr pass --keep-cr flag to git-mailsplit for mbox format
-no-keep-cr do not pass --keep-cr flag to git-mailsplit
independent of am.keepcr
That looks to me as if an overly-long line in the diff (not your fault)
was wrapped in the mailer...
+keep-cr pass --keep-cr flag to git mailsplit for mbox format
At first, I wondered what changed in this line. But then I realized that
you separated git-mailsplit into git mailsplit. This is purely
nitpicking and I am almost ashamed to do so, but I think it might be
*slightly* easier to read if the git mailsplit was quoted.
Having said that, I think it is an important change.
It is a different change, philosophically, though, from changes like this
one:
@@ -174,7 +174,7 @@ It does not apply to blobs recorded in its index.)
then
GIT_MERGE_VERBOSITY=0 export GIT_MERGE_VERBOSITY
fi
-git-merge-recursive $orig_tree -- HEAD $his_tree || {
+git merge-recursive $orig_tree -- HEAD $his_tree || {
This change is code while the former change is documentation. It might be
prudent to split this commit into two, one for calls in scripts, one for
documentation. That way, we could carry the documentation change (which I
whole-heartedly agree with) in Git for Windows should upstream stop before
the fence.
diff --git a/git-archimport.perl b/git-archimport.perl
This file's diff would be less long if the code was a bit more DRY. But
your changes are sound.
diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index d13f02d..6718bad 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -18,7 +18,7 @@ $opt_h usage();
die Need at least one commit identifier! unless @ARGV;
-# Get git-config settings
+# Get git config settings
This is not as clear-cut as the changes above. I would tend to call it a
documentation change, though.
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index a0d796e..53c136f 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -358,10 +358,10 @@ sub req_Root
my @gitvars = `git config -l`;
if ($?) {
- print E problems executing git-config on the server -- this
is not a git repository or the PATH is not set correctly.\n;
+print E problems executing git config on the server -- this
is not a git repository or the PATH is not set correctly.\n;
print E \n;
print error 1 - problem executing git-config\n;
- return 0;
+return 0;
Please don't. I know, it is a whitespace error. But it makes reviewing
more tedious...
@@ -3936,14 +3936,14 @@ sub update
if ( defined ( $lastpicked ) )
{
-my $filepipe = open(FILELIST, '-|', 'git', 'diff-tree',
'-z', '-r', $lastpicked, $commit-{hash}) or die(Cannot call
git-diff-tree : $!);
+my $filepipe = open(FILELIST, '-|', 'git', 'diff-tree',
'-z', '-r', $lastpicked, $commit-{hash}) or die(Cannot call git
diff-tree : $!);
Likewise, this would be a documentation change. It is funny that the
open() did not require a change: apparently, your intended code fixes were
already started at some point, but not finished.
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index 8643f74..ec1d65b 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -97,7 +97,7 @@ do
if test $? -ne 0
then
echo Simple merge did not work, trying automatic merge.
- git-merge-index -o git-merge-one-file -a ||
+ git merge-index -o git-merge-one-file -a ||
This is a problem. 'git-merge-one-file' cannot be split here AFAICT.
Of course, we could teach merge-index to read *two* parameters instead of
one when it encounters git as the merge-program. But that would be as
hacky as the whole dashed-form business to begin with.
diff --git a/git-merge-resolve.sh b/git-merge-resolve.sh
index c9da747..343fe7b 100755
--- a/git-merge-resolve.sh
+++ b/git-merge-resolve.sh
@@ -45,7 +45,7 @@ then
exit 0
else
echo Simple merge failed, trying Automatic merge.
- if git-merge-index -o git-merge-one-file -a
+ if git merge-index -o git-merge-one-file -a
As above, with -octopus.
Apart from the split and the merge-one-file problem, absolutely no
objections
Re: [msysGit] [PATCH] Do not call built-in aliases from scripts
Am 27.06.2013 14:47, schrieb Sebastian Schuberth:
diff --git a/git-archimport.perl b/git-archimport.perl
index 9cb123a..ed2c741 100755
--- a/git-archimport.perl
+++ b/git-archimport.perl
...
@@ -477,8 +477,8 @@ sub process_patchset_fast {
unlink @$del;
while (@$del) {
my @slice = splice(@$del, 0, 100);
-system('git-update-index','--remove','--',@slice) == 0 or
-die Error in git-update-index --remove: $!
$?\n;
+system('git update-index','--remove','--',@slice) == 0 or
+die Error in git update-index --remove: $!
$?\n;
Shouldn't this become 'git','update-index'?
}
}
@@ -496,25 +496,25 @@ sub process_patchset_fast {
}
# print moving $from $to;
rename($from, $to) or die Error renaming '$from' '$to': $!\n;
-system('git-update-index','--remove','--',$from) == 0 or
-die Error in git-update-index --remove: $!
$?\n;
-system('git-update-index','--add','--',$to) == 0 or
-die Error in git-update-index --add: $! $?\n;
+system('git update-index','--remove','--',$from) == 0 or
+die Error in git update-index --remove: $!
$?\n;
+system('git update-index','--add','--',$to) == 0 or
+die Error in git update-index --add: $! $?\n;
Twice here, too.
}
}
if (my $add = $ps-{new_files}) {
while (@$add) {
my @slice = splice(@$add, 0, 100);
-system('git-update-index','--add','--',@slice) == 0 or
-die Error in git-update-index --add: $! $?\n;
+system('git update-index','--add','--',@slice) == 0 or
+die Error in git update-index --add: $! $?\n;
Again.
}
}
if (my $mod = $ps-{modified_files}) {
while (@$mod) {
my @slice = splice(@$mod, 0, 100);
-system('git-update-index','--',@slice) == 0 or
+system('git update-index','--',@slice) == 0 or
Ditto.
-- Hannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
