* [PATCH] git-completion: workaround zsh COMPREPLY bug
From: Felipe Contreras @ 2012-01-25 1:37 UTC (permalink / raw)
To: git; +Cc: gitster, Matthieu Moy, Felipe Contreras
zsh adds a backslash (foo\ ) for each item in the COMPREPLY array if IFS
doesn't contain spaces. This issue has been reported[1], but there is no
solution yet.
This wasn't a problem due to another bug[2], which was fixed in zsh
version 4.3.12. After this change, 'git checkout ma<tab>' would resolve
to 'git checkout master\ '.
Aditionally, the introduction of __gitcomp_nl in commit a31e626
(completion: optimize refs completion) in git also made the problem
apparent, as Matthieu Moy reported.
The simplest and most generic solution is to hide all the changes we do
to IFS, so that "foo \nbar " is recognized by zsh as "foo bar". This
works on versions of git before and after the introduction of
__gitcomp_nl (a31e626), and versions of zsh before and after 4.3.12.
Once zsh is fixed, we should conditionally disable this workaround to
have the same benefits as bash users.
[1] http://www.zsh.org/mla/workers/2012/msg00053.html
[2] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=commitdiff;h=2e25dfb8fd38dbef0a306282ffab1d343ce3ad8d
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
contrib/completion/git-completion.bash | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b0062ba..c83c734 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2631,6 +2631,10 @@ _git ()
# workaround zsh's bug that leaves 'words' as a special
# variable in versions < 4.3.12
typeset -h words
+
+ # another workaround for zsh because it would quote spaces in
+ # the COMPREPLY array if IFS doesn't contain spaces
+ typeset -h IFS
fi
local cur words cword prev
@@ -2687,6 +2691,10 @@ _gitk ()
# workaround zsh's bug that leaves 'words' as a special
# variable in versions < 4.3.12
typeset -h words
+
+ # another workaround for zsh because it would quote spaces in
+ # the COMPREPLY array if IFS doesn't contain spaces
+ typeset -h IFS
fi
local cur words cword prev
--
1.7.8.rc1.14.g248db
^ permalink raw reply related
* Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)
From: Felipe Contreras @ 2012-01-25 1:39 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Junio C Hamano, git
In-Reply-To: <vpqlip5qvcm.fsf@bauges.imag.fr>
On Wed, Jan 18, 2012 at 10:16 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> OK, so the issue the patch addresses may not be a regression in the
>> upcoming v1.7.9 we want to fix quickly,
>
> I'm running ZSH 4.3.10 (Debian stable), and for me it is a regression.
> It seems there is another bug elsewhere affecting more recent ZSH (I
> don't have a recent ZSH version installed to test), but fixing the
> regression for old ZSH is still worth it. I'm not even sur the issue
> with recent ZSH is related.
>
> At worse, my patch is not intrusive and can easily be reworked later.
I believe I have found a more generic and simpler fix that works for
both the regression in v1.7.9, and users of zsh >= 4.3.12.
Patch sent.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2 3/3] git-p4: Add test case for complex branch import
From: Vitor Antunes @ 2012-01-25 1:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Pete Wyckoff, Luke Diamand
In-Reply-To: <CAOpHH-Wcf3innjA4LS0TMrLzEwbQzfZmHssxSBYvv4v7UMfi1w@mail.gmail.com>
On Mon, Jan 23, 2012 at 1:53 PM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> On Sat, Jan 21, 2012 at 4:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Vitor Antunes <vitor.hda@gmail.com> writes:
>>
>>> +test_expect_success 'git-p4 add complex branches' '
>>> + test_when_finished cleanup_git &&
>>> + test_create_repo "$git" &&
>>> + (
>>> + cd "$cli" &&
>>> + changelist=$(p4 changes -m1 //depot/... | cut -d" " -f2) &&
>>> + changelist=$(($changelist - 5)) &&
>>> + p4 integrate //depot/branch1/...@$changelist //depot/branch4/... &&
>>> + p4 submit -d "branch4" &&
>>> + changelist=$(($changelist + 2)) &&
>>> + p4 integrate //depot/branch1/...@$changelist //depot/branch5/... &&
>>> + p4 submit -d "branch5"
>>
>> That's a strange quoting convention. Why are "branch4" and "branch5"
>> enclosed in double quotes while "integrate" and "submit" aren't?
>> (rhetorical: do not quote these branch names without a good reason).
>
> There is no reason that I can remember to have those enclosed in double
> quotes. Will double check in my local branches at home tonight. Anyway,
> expect a fix for this in v3.
I now see why I added the quotes. The -d option is used to input the
description of the commit, which can contain spaces and other special
characters. Admittedly they are not required in this case, but from a
consistency point of view I would prefer to keep them. Is this
acceptable?
>>> +# Configure branches through git-config and clone them. git-p4 will only be able
>>> +# to clone the original structure if it is able to detect the origin changelist
>>> +# of each branch.
>>> +test_expect_success 'git-p4 clone complex branches' '
>>> + test_when_finished cleanup_git &&
>>> + test_create_repo "$git" &&
>>> + (
>>> + cd "$git" &&
>>> + git config git-p4.branchList branch1:branch2 &&
>>> + git config --add git-p4.branchList branch1:branch3 &&
>>> + git config --add git-p4.branchList branch1:branch4 &&
>>> + git config --add git-p4.branchList branch1:branch5 &&
>>> + "$GITP4" clone --dest=. --detect-branches //depot@all &&
>>> + git log --all --graph --decorate --stat &&
>>> + git reset --hard p4/depot/branch1 &&
>>> + test_path_is_file file1 &&
>>> + test_path_is_file file2 &&
>>> + test_path_is_file file3 &&
>>> + grep -q update file2 &&
>>
>> Do you really need to use "-q" here? Wouldn't it help if you wrote it
>> without it while debugging tests with "sh ./t9801-*.sh -v"?
>
> Makes sense.
>
> Thank you for the helpful comments.
> Vitor
Apparently I've hit "Reply" instead of "Reply all" before, so I'll keep
the full quote of my previous email for future reference.
Vitor
^ permalink raw reply
* Re: [PATCH] submodule add: fix breakage when re-adding a deep submodule
From: Jehan Bing @ 2012-01-25 1:48 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <4F1F2784.1020904@web.de>
On 2012-01-24 13:49, Jens Lehmann wrote:
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 3adab93..9bb2e13 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -131,6 +131,7 @@ module_clone()
> gitdir=
> gitdir_base=
> name=$(module_name "$path" 2>/dev/null)
> + test -n "$name" || name="$path"
> base_path=$(dirname "$path")
>
> gitdir=$(git rev-parse --git-dir)
That fixed my problem. Thanks Jens.
Jehan
^ permalink raw reply
* Re: Rebase and incrementing version numbers
From: Jeff King @ 2012-01-25 2:09 UTC (permalink / raw)
To: mike; +Cc: Jon Seymour, git
In-Reply-To: <CAH3Anro8T4SJqBvw1E_7u__4kYyB6hMCYPbtHSVxkgSUYSb2+A@mail.gmail.com>
On Fri, Jan 20, 2012 at 08:33:57AM +1100, Jon Seymour wrote:
> I wonder if you can defer your changes to the config files until after
> you have synced with the current SVN head, so that you typically only
> modify the latest configuration file. Then use git to work out what
> numbers you have to update (by working out which files you changed
> that the SVN upstream has not seen yet). Not perfect, because of race
> conditions, and may not work with your integration testing processes,
> but perhaps worth considering.
That was my thought, too (assuming this workflow, which seems slightly
insane, is outside your power to change).
In this list here:
> Something like:
>
> 1. pull latest SVN
> 2. work on file
> 3. test. skip back to 2 until done.
> 4. ready to push to upstream
> 5. pull latest SVN
> 6. calculate configuration changes required
> 7. apply configuration changes
> 8. push work + configuration changes upstream
Steps 5 and 8 are basically "git svn dcommit". I suspect you could use
some combination of "git svn rebase" and "git filter-branch" to rewrite
your commits with the right counters, and then dcommit the result
(hopefully fast enough to avoid races).
-Peff
^ permalink raw reply
* Re: Rebase and incrementing version numbers
From: John Szakmeister @ 2012-01-25 2:18 UTC (permalink / raw)
To: Santi Béjar; +Cc: mike, demerphq, git
In-Reply-To: <CA+gHt1CPBYTLLwSSLdu-BmDfuGDzPwi9RnXAku7KZjHLYhUtjQ@mail.gmail.com>
On Thu, Jan 19, 2012 at 5:31 PM, Santi Béjar <santi@agolina.net> wrote:
[snip]
> Yes, but you can use "git describe" output:
>
> $ git describe
> v1.7.6-180-gdf3f3d8
That doesn't work with git-svn. In Subversion, tags are closer to
branches, which is how git-svn treats them.
-John
^ permalink raw reply
* Re: [PATCH v2 3/3] git-p4: Add test case for complex branch import
From: Junio C Hamano @ 2012-01-25 4:02 UTC (permalink / raw)
To: Vitor Antunes; +Cc: Git Mailing List, Pete Wyckoff, Luke Diamand
In-Reply-To: <CAOpHH-UxD37v7N3U9A0c_MnzSjOcF6eJCx2WdHRKf2CFoYy_tg@mail.gmail.com>
Vitor Antunes <vitor.hda@gmail.com> writes:
>>>> + p4 submit -d "branch5"
>>>
>>> That's a strange quoting convention. Why are "branch4" and "branch5"
>>> enclosed in double quotes while "integrate" and "submit" aren't?
>>> (rhetorical: do not quote these branch names without a good reason).
>>
>> There is no reason that I can remember to have those enclosed in double
>> quotes. Will double check in my local branches at home tonight. Anyway,
>> expect a fix for this in v3.
>
> I now see why I added the quotes. The -d option is used to input the
> description of the commit, which can contain spaces and other special
> characters. Admittedly they are not required in this case, but from a
> consistency point of view I would prefer to keep them.
Hmm, the argument "branch5" made it look like it is the name of the branch
you are giving here. If it is supposed to be human-readable free-form text
description, I would prefer to see it as such, e.g.
p4 submit -d "integrate changes on branch #1 to branch #5"
or something like that.
^ permalink raw reply
* Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)
From: Junio C Hamano @ 2012-01-25 4:06 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Matthieu Moy, git
In-Reply-To: <CAMP44s2x2kJEJsQLZCJiegZY863X=kVO5xof9QBUin35i-BzhA@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
>> At worse, my patch is not intrusive and can easily be reworked later.
>
> I believe I have found a more generic and simpler fix that works for
> both the regression in v1.7.9, and users of zsh >= 4.3.12.
>
> Patch sent.
Matthieu, care to take a look?
It looks fairly straightforward and would have absolutely no impact on
bash users.
Thanks, Felipe.
-- >8 --
From: Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH] git-completion: workaround zsh COMPREPLY bug
zsh adds a backslash (foo\ ) for each item in the COMPREPLY array if IFS
doesn't contain spaces. This issue has been reported[1], but there is no
solution yet.
This wasn't a problem due to another bug[2], which was fixed in zsh
version 4.3.12. After this change, 'git checkout ma<tab>' would resolve
to 'git checkout master\ '.
Aditionally, the introduction of __gitcomp_nl in commit a31e626
(completion: optimize refs completion) in git also made the problem
apparent, as Matthieu Moy reported.
The simplest and most generic solution is to hide all the changes we do
to IFS, so that "foo \nbar " is recognized by zsh as "foo bar". This
works on versions of git before and after the introduction of
__gitcomp_nl (a31e626), and versions of zsh before and after 4.3.12.
Once zsh is fixed, we should conditionally disable this workaround to
have the same benefits as bash users.
[1] http://www.zsh.org/mla/workers/2012/msg00053.html
[2] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=commitdiff;h=2e25dfb8fd38dbef0a306282ffab1d343ce3ad8d
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
contrib/completion/git-completion.bash | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b0062ba..c83c734 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2631,6 +2631,10 @@ _git ()
# workaround zsh's bug that leaves 'words' as a special
# variable in versions < 4.3.12
typeset -h words
+
+ # another workaround for zsh because it would quote spaces in
+ # the COMPREPLY array if IFS doesn't contain spaces
+ typeset -h IFS
fi
local cur words cword prev
@@ -2687,6 +2691,10 @@ _gitk ()
# workaround zsh's bug that leaves 'words' as a special
# variable in versions < 4.3.12
typeset -h words
+
+ # another workaround for zsh because it would quote spaces in
+ # the COMPREPLY array if IFS doesn't contain spaces
+ typeset -h IFS
fi
local cur words cword prev
^ permalink raw reply related
* Re: [PATCH 1/5] t0061: Fix incorrect indentation
From: Frans Klaver @ 2012-01-25 6:27 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <20120124224000.GE8222@burratino>
On Tue, 24 Jan 2012 23:40:00 +0100, Jonathan Nieder <jrnieder@gmail.com>
wrote:
> Frans Klaver wrote:
>
>> +++ b/t/t0061-run-command.sh
>> @@ -8,8 +8,8 @@ test_description='Test run command'
>> . ./test-lib.sh
>>
>> cat >hello-script <<-EOF
>> - #!$SHELL_PATH
>> - cat hello-script
>> +#!$SHELL_PATH
>> +cat hello-script
>> EOF
>
> Looks like a no-op --- the script already started with #! and no
> leading tab for me. Does it behave differently on your machine?
Hurr? I'm fairly sure the script ended up being indented for me. I'll
recheck.
^ permalink raw reply
* Re: [PATCH 2/5] t0061: Add tests
From: Frans Klaver @ 2012-01-25 6:47 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <20120124225636.GF8222@burratino>
On Tue, 24 Jan 2012 23:56:36 +0100, Jonathan Nieder <jrnieder@gmail.com>
wrote:
>> +>empty
>> +
>> +cat >incorrect-interpreter-script <<-EOF
>> +#!someinterpreter
>> +cat hello-script
>> +EOF
>> +>empty
>
> What is the point of repreatedly writing an empty file named "empty"?
There isn't. I copied it along with the hello-script lines and didn't pay
it much heed. I'll remove the excessive '>empty's.
> I think this would be easier to read and maintain if scripts not
> shared between multiple tests were written in the body of the relevant
> tests. For example, that way it is easier to remember to remove a
> helper script if the relevant test assertion changes to no longer need
> it.
Makes sense. I'll reorder.
>
> [...]
>> @@ -26,7 +44,7 @@ test_expect_success 'run_command can run a command' '
>> test_cmp empty err
>> '
>>
>> test_expect_success POSIXPERM 'run_command reports EACCES' '
>> cat hello-script >hello.sh &&
>> chmod -x hello.sh &&
>> test_must_fail test-run-command run-command ./hello.sh 2>err &&
> [...]
>> +test_expect_success POSIXPERM 'run_command reports EACCES, search path
>> perms' '
>> + mkdir -p inaccessible &&
>> + PATH=$(pwd)/inaccessible:$PATH &&
>> + export PATH &&
>> +
>> + cat hello-script >inaccessible/hello.sh &&
>> + chmod 400 inaccessible &&
>> + test_must_fail test-run-command run-command hello.sh 2>err &&
>> + chmod 755 inaccessible &&
>> +
>> + grep "fatal: cannot exec.*hello.sh" err
>> +'
>
> (*) These tests would be easier to understand if squashed with the
> relevant later patch in the series that changes the error message.
You mean "Elaborate execvp error checking"?
> Maybe they could be less repetitive that way, too.
>
> test_expect_success POSIXPERM 'diagnose command in inaccessible part of
> $PATH' '
> mkdir -p subdir &&
> cat hello-script >subdir/hello.sh &&
> chmod +x subdir/hello.sh &&
> chmod -x subdir &&
> (
> PATH=$(pwd)/inaccessible:$PATH &&
> test_must_fail test-run-command run-command hello.sh 2>err
> ) &&
> test_i18ngrep ...
> '
>
> [...]
>> +test_expect_success POSIXPERM 'run_command reports EACCES, interpreter
>> fails' '
>> + cat incorrect-interpreter-script >hello.sh &&
>> + chmod +x hello.sh &&
>> + chmod -x someinterpreter &&
>> + test_must_fail test-run-command run-command ./hello.sh 2>err &&
>> +
>> + grep "fatal: cannot exec.*hello.sh" err
>> +'
>
> Is this the common case? Why would my interpreter be in the designated
> spot but not marked executable? Is there some other motivating
> example? (I'm genuinely curious; it's ok if the answer is "no".)
I wouldn't think so. This particular one is addressing a concern raised by
Johannes Sixt in reaction to a patch from Junio.
http://article.gmane.org/gmane.comp.version-control.git/171848
>
> [...]
>> +
>> +test_expect_failure POSIXPERM 'run_command reports ENOENT,
>> interpreter' '
>> + cat non-existing-interpreter >hello.sh &&
>> + chmod +x hello.sh &&
>> + test_must_fail test-run-command start-command-ENOENT ./hello.sh 2>err
>> &&
>> +
>> + grep "error: cannot exec.*hello.sh" err
>> +'
>
> Maybe:
>
> test_expect_success POSIXPERM 'diagnose missing interpreter' '
> echo "#!/nonexistent/interpreter" >hello.sh &&
> chmod +x hello.sh &&
> test_must_fail test-run-command run-command hello.sh 2>err &&
> test_i18ngrep ...
> '
Will check.
> Hope that helps,
> Jonathan
^ permalink raw reply
* Re: [PATCH 1/5] t0061: Fix incorrect indentation
From: Junio C Hamano @ 2012-01-25 7:00 UTC (permalink / raw)
To: Frans Klaver; +Cc: Jonathan Nieder, git, Johannes Sixt
In-Reply-To: <op.v8mmwftk0aolir@keputer>
"Frans Klaver" <fransklaver@gmail.com> writes:
> On Tue, 24 Jan 2012 23:40:00 +0100, Jonathan Nieder
> <jrnieder@gmail.com> wrote:
>
>> Frans Klaver wrote:
>>
>>> +++ b/t/t0061-run-command.sh
>>> @@ -8,8 +8,8 @@ test_description='Test run command'
>>> . ./test-lib.sh
>>>
>>> cat >hello-script <<-EOF
>>> - #!$SHELL_PATH
>>> - cat hello-script
>>> +#!$SHELL_PATH
>>> +cat hello-script
>>> EOF
>>
>> Looks like a no-op --- the script already started with #! and no
>> leading tab for me. Does it behave differently on your machine?
>
> Hurr? I'm fairly sure the script ended up being indented for me. I'll
> recheck.
It could be that your shell is broken and does not understand the
distinction between the "<<-EOF" vs "<<EOF". What system are you on? If
the problem is real and widespread, we might want to mention it in INSTALL
or Makefile, just like we label Solaris /bin/sh as unusable and advise
people to use /usr/xpg[46]/bin variant.
^ permalink raw reply
* Re: [PATCH 1/5] t0061: Fix incorrect indentation
From: Frans Klaver @ 2012-01-25 7:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, git, Johannes Sixt
In-Reply-To: <7vd3a8466u.fsf@alter.siamese.dyndns.org>
On Wed, 25 Jan 2012 08:00:57 +0100, Junio C Hamano <gitster@pobox.com>
wrote:
> "Frans Klaver" <fransklaver@gmail.com> writes:
>
>> On Tue, 24 Jan 2012 23:40:00 +0100, Jonathan Nieder
>> <jrnieder@gmail.com> wrote:
>>
>>> Frans Klaver wrote:
>>>
>>>> +++ b/t/t0061-run-command.sh
>>>> @@ -8,8 +8,8 @@ test_description='Test run command'
>>>> . ./test-lib.sh
>>>>
>>>> cat >hello-script <<-EOF
>>>> - #!$SHELL_PATH
>>>> - cat hello-script
>>>> +#!$SHELL_PATH
>>>> +cat hello-script
>>>> EOF
>>>
>>> Looks like a no-op --- the script already started with #! and no
>>> leading tab for me. Does it behave differently on your machine?
>>
>> Hurr? I'm fairly sure the script ended up being indented for me. I'll
>> recheck.
>
> It could be that your shell is broken and does not understand the
> distinction between the "<<-EOF" vs "<<EOF". What system are you on?
Gentoo Linux, using bash. Would be hard to believe bash on plain old linux
mucks this up, wouldn't it?
> If
> the problem is real and widespread, we might want to mention it in
> INSTALL
> or Makefile, just like we label Solaris /bin/sh as unusable and advise
> people to use /usr/xpg[46]/bin variant.
I'll check and notify.
^ permalink raw reply
* Re: [PATCH 3/5] run-command: Elaborate execvp error checking
From: Frans Klaver @ 2012-01-25 7:09 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <20120124232239.GG8222@burratino>
On Wed, 25 Jan 2012 00:22:39 +0100, Jonathan Nieder <jrnieder@gmail.com>
wrote:
> Klaver wrote:
>
>> The interpretation of errors from execvp was rather terse. For user
>> convenience communication of the nature of the error can be improved.
>
> Could you give an example?
The case that triggered me to work on this. I had an incorrect entry in my
PATH and some aliasing tests failed. The generated command output was
something like
fatal: script: Access Denied
while I was certain it didn't exist because I was expecting an alias to be
executed. Also, access denied on a file that doesn't exist? That didn't
make sense (it really doesn't if you don't know the execvp workings).
Basically git takes no effort in actually pointing the user to the source
of the problem. During the previous two versions I came to realize that
this goes for all error codes execvp produces.
>
> [...]
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -2,6 +2,7 @@
>> #include "run-command.h"
>> #include "exec_cmd.h"
>> #include "argv-array.h"
>> +#include "dir.h"
>>
>> static inline void close_pair(int fd[2])
>> {
>> @@ -134,6 +135,140 @@ static int wait_or_whine(pid_t pid, const char
>> *argv0, int silent_exec_failure)
>> return code;
>> }
>>
>> +#ifndef WIN32
>
> Not related to this patch, but I wonder if there should be a separate
> run-command-unix.c file so these ifdefs would no longer be necessary.
Might be useful indeed.
> What happens on Windows?
I haven't changed anything on the windows side, so that probably sticks to
the old behavior.
>> +static void die_file_error(const char *file, int err)
>> +{
>> + die("cannot exec '%s': %s", file, strerror(err));
>> +}
>
> I suspect it might be clearer to use die() inline in the two call
> sites so the reader does not have to figure out the calling
> convention.
Fair enough. I originally expected it to be used more than twice in the
code.
>
>> +
>> +static char *get_interpreter(const char *first_line)
>> +{
>> + struct strbuf sb = STRBUF_INIT;
>> + size_t start = strspn(first_line + 2, " \t") + 2;
>> + size_t end = strcspn(first_line + start, " \t\r\n") + start;
>> +
>> + if (start >= end)
>> + return NULL;
>> +
>> + strbuf_add(&sb, first_line + start, end - start);
>> + return strbuf_detach(&sb, NULL);
>> +}
>
> What does this function do? What happens if first_line doesn't start
> with "#!"? What should happen when there is a newline instead of a
> command name? How about commands with quoting characters like " and
> backslash --- are the semantics portable in these cases?
This gets the interpreter from a #! line, assuming that it actually has a
#!. You might call it naive, which would be fair enough. I didn't see much
point in doing the same check twice, but inlining this code in its
callsite felt messy.
> No need to use a strbuf here: xmemdupz would take care of the
> allocation and copy more simply.
Right.
>> +static void inspect_failure(const char *argv0, int silent_exec_failure)
>> +{
>> + int err = errno;
>> + struct strbuf sb = STRBUF_INIT;
>> +
>> + /* errors not related to path */
>> + if (errno == E2BIG || errno == ENOMEM)
>> + die_file_error(argv0, err);
>> +
>> + if (strchr(argv0, '/')) {
>> + if (file_exists(argv0)) {
>> + strbuf_add(&sb, argv0, strlen(argv0));
>> + inspect_file(&sb, err, argv0);
>> + }
>> + } else {
>> + char *path, *next;
>> + path = getenv("PATH");
>
> I wonder if it's possible to rearrange this code to avoid deep
> nesting. What does the function do, anyway? (If the reader has to
> ask, it needs a comment or to be renamed.)
>
> I guess the idea is to diagnose after the fact why execvp failed.
Correct guess.
> Might be simplest like this:
>
> To diagnose execvp failure:
> if filename does not contain a '/':
> if we can't find it on the search path:
> That's the problem, dummy!
> replace filename with full path
> if file does not exist:
> just report strerror(errno)
> if not executable:
> ...
> if interpreter does not exist:
> ...
> if interpreter not executable:
> ...
> otherwise, just report strerror(errno)
>
> with a separate function to find a command on the PATH, complaining
> when it encounters an unsearchable entry.
Well, this approach basically does the same thing, but this may have
gotten a bit tunnel-visioned. I'll have a look at your approach and see
what it gives us.
>
> Thanks for a fun read.
Thanks for an insightful review.
^ permalink raw reply
* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Frans Klaver @ 2012-01-25 7:12 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <20120124232421.GH8222@burratino>
On Wed, 25 Jan 2012 00:24:21 +0100, Jonathan Nieder <jrnieder@gmail.com>
wrote:
> Frans Klaver wrote:
>
>> --- a/t/t0061-run-command.sh
>> +++ b/t/t0061-run-command.sh
>> @@ -76,12 +76,12 @@ test_expect_success POSIXPERM 'run_command reports
>> EACCES, interpreter fails' '
>> grep "bad interpreter" err
>> '
>>
>> -test_expect_failure POSIXPERM 'run_command reports ENOENT,
>> interpreter' '
>> +test_expect_success POSIXPERM 'run_command reports ENOENT,
>> interpreter' '
>> cat non-existing-interpreter >hello.sh &&
>> chmod +x hello.sh &&
>> test_must_fail test-run-command start-command-ENOENT ./hello.sh 2>err
>> &&
>>
>> - grep "error: cannot exec.*hello.sh" err &&
>> + grep "fatal: cannot exec.*hello.sh" err &&
>
> Thanks. I'd suggest using "test_expect_code" rather than the detailed
> wording of the message, since that is what scripts might want to rely
> on.
OK, makes sense.
> What happens on Windows?
I didn't plan anything to happen on windows. Doesn't POSIXPERM rule that
OS out? I guess it could use similar code to this patch series to tackle
all this.
^ permalink raw reply
* Re: [PATCH] git-completion: workaround zsh COMPREPLY bug
From: Matthieu Moy @ 2012-01-25 7:41 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, gitster
In-Reply-To: <1327455422-22340-1-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> The simplest and most generic solution is to hide all the changes we do
> to IFS, so that "foo \nbar " is recognized by zsh as "foo bar". This
> works on versions of git before and after the introduction of
> __gitcomp_nl (a31e626), and versions of zsh before and after 4.3.12.
[...]
> +
> + # another workaround for zsh because it would quote spaces in
> + # the COMPREPLY array if IFS doesn't contain spaces
> + typeset -h IFS
No time to test right now, but is this not going to
1) leave IFS as hidden even outside the completion script, possibly
affecting unrelated scripts that would need to set IFS as local and keep
its special effect?
2) break cases where strings are to be split on \n only (e.g. see
"foo bar\nboz" as three possible completions "foo", "bar", "boz" instead
of "foo bar" and "boz"?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 1/5] t0061: Fix incorrect indentation
From: Frans Klaver @ 2012-01-25 8:08 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <op.v8mmwftk0aolir@keputer>
On Wed, Jan 25, 2012 at 7:27 AM, Frans Klaver <fransklaver@gmail.com> wrote:
> Hurr? I'm fairly sure the script ended up being indented for me. I'll
> recheck.
Hm, that must have been my imagination back then. I'm sure I did this
for a reason. Ah well, we'll drop this patch. I'll put the tabs into
the other test scripts as well, because it does make them that much
more readable.
Thanks for catching.
^ permalink raw reply
* Dear Beloved!
From: Mr Faartir Ibrahim @ 2012-01-25 9:50 UTC (permalink / raw)
Good Day Beloved,
I am a dying man with Cancer,my doctor told me today that i will not survive
it.I have a substantial sum I would like you to help me distribute
to the needy, orphans in society.REPLY TO: ibrahimfaartir@hotmail.com for
details.
MR FAARTIR IBRAHIM
^ permalink raw reply
* Re: Rebase and incrementing version numbers
From: Christian Couder @ 2012-01-25 10:32 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: mike, git
In-Reply-To: <1327000803.5947.59.camel@centaur.lab.cmartin.tk>
On Thu, Jan 19, 2012 at 8:20 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> You could write a merge driver that detects this situation and writes in
> a higher number, but it's all working around the fact that it's a race
> condition.
By "merge" driver you mean a new merge startegy?
Isn't it possible to write a script and use it with git mergetool to
automatically detect and resolve the merge conflicts resulting from
changes in these numbers?
Regards,
Christian.
^ permalink raw reply
* Re: Rebase and incrementing version numbers
From: Carlos Martín Nieto @ 2012-01-25 10:53 UTC (permalink / raw)
To: Christian Couder; +Cc: mike, git
In-Reply-To: <CAP8UFD0gd_-=Cc0vox-6Ts4-iBWcJG8LgmqXteXgp3qc-bX13w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 984 bytes --]
On Wed, 2012-01-25 at 11:32 +0100, Christian Couder wrote:
> On Thu, Jan 19, 2012 at 8:20 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> > You could write a merge driver that detects this situation and writes in
> > a higher number, but it's all working around the fact that it's a race
> > condition.
>
> By "merge" driver you mean a new merge startegy?
No. By "merge driver" I mean a "merge driver".
>
> Isn't it possible to write a script and use it with git mergetool to
> automatically detect and resolve the merge conflicts resulting from
> changes in these numbers?
No. A mergetool is what you call manually to help you resolve a merge
conflict. What you're describing is a merge driver. If you grep for
"driver" in the merge manpage, you'll see how to set it, and it'll tell
you to look in the gitattributes manpage for more information. If you
search the web for "git merge driver" you'll see lots of examples of how
these are done.
cmn
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: Rebase and incrementing version numbers
From: Christian Couder @ 2012-01-25 11:18 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: mike, git
In-Reply-To: <1327488820.3052.15.camel@beez.lab.cmartin.tk>
On Wed, Jan 25, 2012 at 11:53 AM, Carlos Martín Nieto <cmn@elego.de> wrote:
> On Wed, 2012-01-25 at 11:32 +0100, Christian Couder wrote:
>>
>> By "merge" driver you mean a new merge startegy?
>
> No. By "merge driver" I mean a "merge driver".
Oops yeah, sorry, I should have searched.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH] git-completion: workaround zsh COMPREPLY bug
From: Felipe Contreras @ 2012-01-25 12:16 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
In-Reply-To: <vpqwr8g8c03.fsf@bauges.imag.fr>
On Wed, Jan 25, 2012 at 9:41 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> The simplest and most generic solution is to hide all the changes we do
>> to IFS, so that "foo \nbar " is recognized by zsh as "foo bar". This
>> works on versions of git before and after the introduction of
>> __gitcomp_nl (a31e626), and versions of zsh before and after 4.3.12.
> [...]
>> +
>> + # another workaround for zsh because it would quote spaces in
>> + # the COMPREPLY array if IFS doesn't contain spaces
>> + typeset -h IFS
>
> No time to test right now, but is this not going to
>
> 1) leave IFS as hidden even outside the completion script, possibly
> affecting unrelated scripts that would need to set IFS as local and keep
> its special effect?
What special effect?
Unrelated scripts can still set IFS as local.
You can test this:
---
#!/bin/sh
if [[ -n ${ZSH_VERSION-} ]]; then
autoload -U +X bashcompinit && bashcompinit
fi
_foo()
{
typeset -h IFS
local cur="${COMP_WORDS[COMP_CWORD]}"
local IFS=$'\n'
w[0]='first '
w[1]='second and space '
w[2]='third\\ quoted\\ space '
./test1 >> /tmp/t1.txt
COMPREPLY=( $(compgen -W "${w[*]}" -- $cur) )
}
complete -o nospace -F _foo foo
---
test1:
---
#!/bin/sh
test1() {
local IFS=$'\n'
w=( $(echo -e "foo \nbar"))
echo "test1: 0:'${w[0]}' 1:'${w[1]}'"
}
test1
w=( $(echo -e "foo \nbar"))
echo "test2: 0:'${w[0]}' 1:'${w[1]}'"
---
The result in /tmp/t1.txt would be:
test1: 0:'foo ' 1:'bar'
test2: 0:'foo' 1:'bar'
Regardless of whether you use typeset -h or not.
> 2) break cases where strings are to be split on \n only (e.g. see
> "foo bar\nboz" as three possible completions "foo", "bar", "boz" instead
> of "foo bar" and "boz"?
Those cases are already broken, which is what I reported to the zsh
mailing list. You would get "foo\ bar" and "boz", and that's after
4.3.12; before, compgen -W would still break the completions in 3,
because they did 'results+=( $words )', instead of 'results+=(
"$words" )'.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2 3/3] git-p4: Add test case for complex branch import
From: Pete Wyckoff @ 2012-01-25 12:34 UTC (permalink / raw)
To: Vitor Antunes; +Cc: Luke Diamand, Junio C Hamano, git
In-Reply-To: <CAOpHH-V2nZ8meh7x6vCVGUQCKQqJ+sPcnGRo+8SqfNavg7F87w@mail.gmail.com>
vitor.hda@gmail.com wrote on Wed, 25 Jan 2012 01:23 +0000:
> On Mon, Jan 23, 2012 at 10:40 PM, Pete Wyckoff <pw@padd.com> wrote:
> > How about taking what's below and just squashing it in. It's
> > incremental on your changes and would go well with Luke's series
> > that fixes a bunch of scattered quoting issues similarly.
> >
> > The change to "describe %s" is unnecessary, but makes all the
> > invocations look similar. You can leave it out.
>
> I've squashed your patch, but kept the "describe %s" fix in a separate
> commit.
>
> >> BTW, and on an unrelated topic, are any test cases failing on your side?
> >
> > I do run the tests regularly, and your series is good. There's
> > the 'clone --use-client-spec' one that is broken until my
> > 2ea09b5 (git-p4: adjust test to adhere to stricter useClientSpec,
> > 2012-01-11) is merged. It's on pu.
>
> Tests in t9809-git-p4-client-view.sh were failing for me because I'm
> using dash instead of bash. Please check patch below for a fix.
>
> Test 15 of t9800-git-p4-basic.sh is still failing and I've not been able
> to pinpoint the problem. I can send you the logs off-list, if you want.
>
> Thanks,
> Vitor
>
>
>
> diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
> index c9471d5..5b0ad99 100755
> --- a/t/t9809-git-p4-client-view.sh
> +++ b/t/t9809-git-p4-client-view.sh
> @@ -31,7 +31,7 @@ client_view() {
> #
> check_files_exist() {
> ok=0 &&
> - num=${#@} &&
> + num=$# &&
> for arg ; do
> test_path_is_file "$arg" &&
> ok=$(($ok + 1))
>
Yes, thanks. Plain old $# works fine, even if the arguments have
spaces. I'll hang onto this with some other work in the same
area and submit it eventually.
-- Pete
^ permalink raw reply
* Re: Finding all commits which modify a file
From: Neal Groothuis @ 2012-01-25 16:23 UTC (permalink / raw)
To: Santi Béjar; +Cc: Junio C Hamano, Linus Torvalds, git
In-Reply-To: <CA+gHt1BjqJpUke8JKjUmFyg3Zj5FmASd77LR-7P+6RrNLddD1A@mail.gmail.com>
> [ Do not cut the CC]
My apologies.
> On Tue, Jan 24, 2012 at 5:34 PM, Neal Groothuis <ngroot@lo-cal.org> wrote:
>>> On Mon, Jan 23, 2012 at 5:14 PM, Neal Groothuis <ngroot@lo-cal.org>
>>> wrote:
>>>>> On 1/20/2012 3:35 PM, Neal Groothuis wrote:
>>>>>> I'm trying to find /all/ commits that change a file in the
>>>>>> repository...and its proving to be trickier than I thought. :-)
>>>>
>>>> On 1/21/2012 6:16 PM, Neal Kreitzinger wrote:
>>>>> Does git-log --all help?
>>>>
>>>> I don't see how it would. The commits are all reachable from HEAD,
>>>> which
>>>> would seem to be the problem that --all would correct.
>>>>
>>>> What I'm trying to do is find the commits in which a file differs from
>>>> that same file in any of its parents.
>>>
>>> If you add parent rewriting (--parent, --graph or see it in gitk, with
>>> --full-history) you'll get your B2 commit as it adds commits to have a
>>> meaningful history. But I don't think this is what you are asking for.
>>
>> Correct. If I add parent rewriting, I get all merges, even those in
>> which
>> the file is not changed from either parent.
>>
>> Based on what's in the man page for git log about the history
>> simplification algorithm, it seems that B2 should be included in the
>> output when I do a git log --full-history --simplify-history foo.txt, as
>> per the steps I noted in the original post. Is my understanding of the
>> algorithm faulty?
>>
>
> Following your steps in the first post, B2 is excluded in the
> --simplify-merge phase because it is (originally) TREESAME, even if it
> is not in the rewritten history...
Thanks, I see---labeling a commit as TREESAME happens before
simplification, rather than after.
In my example, that results in a simplified history where a commit in
which the contents of the specified paths change gets removed. That seems
perverse; I would think the utility of a simplified history would be to
track down the commits in which the contents of the specified paths change
without having to consider ones in which they do not.
Is there a situation where checking for TREESAMEness before simplification
is desirable and checking after would not be?
- Neal
^ permalink raw reply
* git version not changed after installing new version
From: freefly @ 2012-01-25 16:45 UTC (permalink / raw)
To: git
Hi all,
I am new to Mac OS X lion and I had a previous installation of XCODE 4.2
on my mac mini. It has a git version 1.7.5.4. I installed a new version 1.7.8.4.
and updated the Path variables by running the script comes with the
package as well. but when I type "git --version" in the terminal
I get 1.7.5.4. Can anyone tell me what is going wrong ?
Kind regards
^ permalink raw reply
* Re: git version not changed after installing new version
From: Thomas Rast @ 2012-01-25 17:03 UTC (permalink / raw)
To: freefly; +Cc: git
In-Reply-To: <loom.20120125T173801-500@post.gmane.org>
freefly <free.fly@live.com> writes:
> Hi all,
> I am new to Mac OS X lion and I had a previous installation of XCODE 4.2
> on my mac mini. It has a git version 1.7.5.4. I installed a new version 1.7.8.4.
> and updated the Path variables by running the script comes with the
> package as well. but when I type "git --version" in the terminal
> I get 1.7.5.4. Can anyone tell me what is going wrong ?
Enter 'which git' in a terminal. The 'which' utility looks up which
executable from $PATH will be run if you use 'git' (in this case) as a
command word, i.e., without specifying its path.
This should point to your newly installed git. It probably doesn't, and
you can compare it and your $PATH (try: 'echo "$PATH"' in a terminal) to
see why.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox