* 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: [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
* 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 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 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 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 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 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: 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] 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 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: 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: 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: [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: [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] 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
* [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 v2 3/3] git-p4: Add test case for complex branch import
From: Vitor Antunes @ 2012-01-25 1:23 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: Luke Diamand, Junio C Hamano, git
In-Reply-To: <20120123224012.GA10626@padd.com>
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))
^ permalink raw reply related
* Re: Autocompletion - commands no longer work as stand alone
From: SZEDER Gábor @ 2012-01-24 23:26 UTC (permalink / raw)
To: Nathan Bullock; +Cc: Junio C Hamano, git
In-Reply-To: <7vwr8mdvo8.fsf@alter.siamese.dyndns.org>
Hi,
> Nathan Bullock <nathanbullock@gmail.com> writes:
>
> > I have for a number of years had the following in my .bashrc
> >
> > alias br="git branch"
> > complete -F _git_branch br
> >
> > As well as similar commands for co and log.
> >
> > Recently though this broke, now when I type something like "br
> > mas<command completion>" it will occasionally complain with messages
> > like:
> > bash: [: 1: unary operator expected
> >
> > From digging through the source it looks like this was broken back in
> > April. (The commit is show at the bottom of this email.)
> >
> > So my questions are:
> > 1. Is it reasonable for things like _git_branch to work as a
> > standalone autocompletion function instead of having to go through
> > _git? I certainly like it to work as a standalone function. I also use
> > it to add autocompletion to other bash scripts that I use frequently.
> >
> > 2. If I add code that verifies that the variable cword exists at the
> > start of these functions and only if not call something like
> > _get_comp_words_by_ref -n =: cur words cword prev. Would that be
> > reasonable?
That would be too fragile, it will break if $cword is set in the
environment from which you invoke _git_<cmd>() completion functions
directly (i.e. not though _git()).
> > I think this should address the performance concerns that
> > caused these to be removed in the first place, but it may make the
> > code uglier.
Actually it was not a performance problem, but a cleanup in a patch
series to fix a zsh-related bug. Without this cleanup the bugfix
would have been much more intrusive.
http://thread.gmane.org/gmane.comp.version-control.git/172142/focus=172369
> > I have already added wrapper functions in my bashrc so that this is no
> > longer a problem for me, but there may be other people who start
> > hitting this as well once they start using newer versions of git.
This issue was reported earlier, so it seems there are people who
would like to use it. But getting $cur, $cword, etc. variables right
in _git_<cmd>() completion functions is just part of the problem,
there are other issues, as mentioned in the previous thread:
http://thread.gmane.org/gmane.comp.version-control.git/185184/focus=185232
Unfortunately, I couldn't come up with a solution yet that doesn't
introduce too much code churn and doesn't cause yet another
inconsistency between bash and zsh. I also haven't looked whether
there are other issues similar to that with _git_fetch() mentioned on
the above link.
Best,
Gábor
^ permalink raw reply
* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Jonathan Nieder @ 2012-01-24 23:24 UTC (permalink / raw)
To: Frans Klaver; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <1327444346-6243-6-git-send-email-fransklaver@gmail.com>
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.
What happens on Windows?
^ permalink raw reply
* Re: [PATCH 3/5] run-command: Elaborate execvp error checking
From: Jonathan Nieder @ 2012-01-24 23:22 UTC (permalink / raw)
To: Frans Klaver; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <1327444346-6243-4-git-send-email-fransklaver@gmail.com>
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?
[...]
> --- 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.
What happens on Windows?
> +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.
> +
> +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?
No need to use a strbuf here: xmemdupz would take care of the
allocation and copy more simply.
> +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.
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.
Thanks for a fun read.
Jonathan
^ permalink raw reply
* Re: [PATCH 2/5] t0061: Add tests
From: Jonathan Nieder @ 2012-01-24 22:56 UTC (permalink / raw)
To: Frans Klaver; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <1327444346-6243-3-git-send-email-fransklaver@gmail.com>
Hi,
Frans Klaver wrote:
> --- a/t/t0061-run-command.sh
> +++ b/t/t0061-run-command.sh
> @@ -13,6 +13,24 @@ cat hello-script
> EOF
> >empty
>
> +cat >someinterpreter <<-EOF
> +#!$SHELL_PATH
> +cat hello-script
> +EOF
> +>empty
> +
> +cat >incorrect-interpreter-script <<-EOF
> +#!someinterpreter
> +cat hello-script
> +EOF
> +>empty
Thanks for writing tests. Some hints on mechanics below, and one on
strategy (see (*) below).
What is the point of repreatedly writing an empty file named "empty"?
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.
[...]
> @@ -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.
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".)
[...]
> +
> +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 ...
'
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH 1/5] t0061: Fix incorrect indentation
From: Jonathan Nieder @ 2012-01-24 22:40 UTC (permalink / raw)
To: Frans Klaver; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <1327444346-6243-2-git-send-email-fransklaver@gmail.com>
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?
^ permalink raw reply
* Re: [PATCH 1/5] t0061: Fix incorrect indentation
From: Junio C Hamano @ 2012-01-24 22:39 UTC (permalink / raw)
To: Frans Klaver; +Cc: git, Jonathan Nieder, Johannes Sixt
In-Reply-To: <1327444346-6243-2-git-send-email-fransklaver@gmail.com>
Frans Klaver <fransklaver@gmail.com> writes:
> The hello.sh script started with <tab>#!, which is not considered a
> correct hash-bang line.
Isn't that exactly the reason why we start the here text with "<<-EOF",
not the usual "<<EOF"?
> required for this specific test.
>
> Signed-off-by: Frans Klaver <fransklaver@gmail.com>
> ---
> t/t0061-run-command.sh | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
> index 8d4938f..95e89bc 100755
> --- a/t/t0061-run-command.sh
> +++ 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
> >empty
^ permalink raw reply
* Re: [BUG] Fail to add a module in a subdirectory if module is already cloned
From: Junio C Hamano @ 2012-01-24 22:38 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git
In-Reply-To: <4F1F2D38.9050909@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> Just for the record: I checked that and git-submodule does not set the
> SUBDIRECTORY_OK environment variable so every time it is not run in the
> top level directory it aborts with:
> "You need to run this command from the toplevel of the working tree."
Ah, Ok.
^ 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