Git development
 help / color / mirror / Atom feed
* Re: [PATCH] transport-helper: check when helpers fail
From: Felipe Contreras @ 2012-10-22 14:31 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: git, Junio C Hamano, Jeff King, Sverre Rabbelier, Shawn O. Pearce
In-Reply-To: <50854E20.1040303@viscovery.net>

On Mon, Oct 22, 2012 at 3:46 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 10/22/2012 13:50, schrieb Felipe Contreras:
>> On Mon, Oct 22, 2012 at 8:35 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Another thought: In your use-case, isn't it so that it would be an error
>>> that the process exited for whatever reason? I.e., even if it exited with
>>> code 0 ("success"), it would be an error because it violated the protocol?
>>
>> How is that violating the protocol?
>
> Because the helper stops talking too early. But as I said, I actually
> don't know the protocol.

We could use the 'feature done' of fast-import, but this causes
problems because of the way transport-helper uses it:

-> import refs/heads/master
<- exported stuff
<- done
-> import refs/heads/devel
<- exported stuff
<- done

'done' will terminate the fast-import process, so the second exported
stuff won't be processed; the fast-import process is reused.

For some reason remote-testgit doesn't exercise this multiple import
stuff properly, but my remote-hg certainly does, so I can't just say
'done'.

It would be much better if the transport-helper protocol was something
like this:

-> import-begin
<- feature X
<- feature Y
-> import refs/heads/master
<- exported stuff
-> import refs/heads/devel
<- exported stuff
-> import-end
<- done

This would certainly makes things easier for transport-helpers that
support multiple ref selections (like my remote-hg). Maybe I should
add code that does this if certain feature is specified (so it doesn't
break other helpers)

But at least on my tests, even with 'feature done' the crash is not
detected properly, either by the transport-helper, or fast-import.

And also, the msysgit branch does the same check for fast-export,
which actually uses the 'done' feature always, so it should work fine,
but perhaps because of the strange issue with fast-import I just
mentioned, it's not actually detected. I should add tests for this
too.

> I was just infering what I saw in transport-helper.c: get_helper() dup's
> the output of the helper process and stores it in data->out (after
> fdopen()ing on it). (The original file descriptor is handed over to
> fast-import or fast-export.)
>
> Actually, I didn't find a spot where data->out was used except to fclose()
> it. But I take it that there is a reason that it exists and infer that
> further output from the helper is expected by something after fast-import
> or fast-export have exited.
>
> But I may be completely off...

Yes, further output is expected, or at least in theory.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* [PATCH] fix 'make test' for HP NonStop
From: Joachim Schmitz @ 2012-10-22 14:30 UTC (permalink / raw)
  To: git

This fixes the vast majority of test failures on HP NonStop.

Signed-off-by: Joachim Schmitz <jojo@schmitz-digital.de>
---
A few more still insist on /usr/local/bin being 1st in PATH and having done that
we're down to one single failing test, t0301 #12 "helper (cache --timeout=1) times out"

Makefile | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/Makefile b/Makefile
index f69979e..35380dd 100644
--- a/Makefile
+++ b/Makefile
@@ -1381,6 +1381,15 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
	MKDIR_WO_TRAILING_SLASH = YesPlease
	# RFE 10-120912-4693 submitted to HP NonStop development.
	NO_SETITIMER = UnfortunatelyYes
+
+	# for 'make test'
+	# some test don't work with /bin/diff, some fail with /bin/tar
+	# some need bash, and some need ${prefix}/bin in PATH first
+	SHELL_PATH=${prefix}/bin/bash
+	SANE_TOOL_PATH=${prefix}/bin
+	# as of H06.25/J06.14, we might better use this
+	#SHELL_PATH=/usr/coreutils/bin/bash
+	#SANE_TOOL_PATH=/usr/coreutils/bin:${prefix}/bin
endif
ifneq (,$(findstring MINGW,$(uname_S)))
	pathsep = ;

^ permalink raw reply related

* Re: make test
From: Joachim Schmitz @ 2012-10-22 14:19 UTC (permalink / raw)
  To: git

"Joachim Schmitz" <jojo@schmitz-digital.de> schrieb im Newsbeitrag news:<k5gov5$fe1$1@ger.gmane.org>...
> Hi folks
> 
> I'm trying to understand why certain tests in 'make test' fail. Here's the 
> first one
> 
> $ ../git --version
> git version 1.8.0.rc2.5.g6b89306
> $ GIT_TEST_CMP_USE_COPIED_CONTEXT=true ./t0000-basic.sh # our diff doesn't 
> understand -u
> ok 1 - .git/objects should be empty after git init in an empty repo
> ...
> ok 3 - success is reported like this
> not ok 4 - pretend we have a known breakage # TODO known breakage
> 
>     This is expected, right?
> 
> ok 5 - pretend we have fixed a known breakage (run in sub test-lib)
> ...
> ok 11 - tests clean up after themselves
> 
>     the next is not though? Why might it be failing, where to check?
> 
> not ok - 12 tests clean up even on failures
> #
> #               mkdir failing-cleanup &&
> #               (
> #               cd failing-cleanup &&
> #
> #               cat >failing-cleanup.sh <<-EOF &&
> #               #!/bin/sh
> #
> #               test_description='Failing tests with cleanup commands'
> #
> #               # Point to the t/test-lib.sh, which isn't in ../ as usual
> #               TEST_DIRECTORY="/home/jojo/git/git/t"
> #               . "$TEST_DIRECTORY"/test-lib.sh
> #
> #               test_expect_success 'tests clean up even after a failure' '
> #                       touch clean-after-failure &&
> #                       test_when_finished rm clean-after-failure &&
> #                       (exit 1)
> #               '
> #               test_expect_success 'failure to clean up causes the test to 
> fail' '
> #                       test_when_finished "(exit 2)"
> #               '
> #               test_done
> #
> #               EOF
> #
> #               chmod +x failing-cleanup.sh &&
> #               test_must_fail ./failing-cleanup.sh >out 2>err &&
> #               ! test -s err &&
> #               ! test -f "trash 
> directory.failing-cleanup/clean-after-failure" &&
> #               sed -e 's/Z$//' -e 's/^> //' >expect <<-\EOF &&
> #               > not ok - 1 tests clean up even after a failure
> #               > #     Z
> #               > #     touch clean-after-failure &&
> #               > #     test_when_finished rm clean-after-failure &&
> #               > #     (exit 1)
> #               > #     Z
> #               > not ok - 2 failure to clean up causes the test to fail
> #               > #     Z
> #               > #     test_when_finished "(exit 2)"
> #               > #     Z
> #               > # failed 2 among 2 test(s)
> #               > 1..2
> #               EOF
> #               test_cmp expect out
> #               )
> #
> ok 13 - git update-index without --add should fail adding
> ...
> ok 47 - very long name in the index handled sanely
> # still have 1 known breakage(s)
> # failed 1 among remaining 46 test(s)
> 1..47

As mentioned elsethread this works if using bash rather than the system's sh (which is a ksh)

But there are several other failures. After some investigations and experiments I found the following tests to fail with the system
provided grep (for which I had to set GIT_TEST_CMP_USE_COPIED_CONTEXT), but succeed with GNU grep:
t3308 #14, #15, #17and #19
t3310 #10, #12, #14 and #18
t4047 #38 and #39
t4050 #2 and #3
t4116 #3, #4 and #5
t5509 #2
t7401 #18

The following fail with the system provided tar, but succeed with GNU tar:
t0024 #2
t4116 #4,
t5000 #14, #16, #20, #24, #26 and #51
t5001 #2, #6, #10 and #15

The following tests fail with the system provided sh (which is a ksh really), but succeed with bash:
t0000 #12
t0001 #20
t1450 #17 and #18
(t0204 #3 and #8 succeed in sh but fail in bash. They succeed in bash too when /usr/local/bin is in PATH first though, which would
sort the diff and tar problem too, need to investigate why)
t3006 #2 and #3

t3403 #4, #5, #8 and #9
t3404 #2 - #13, #14 - #18, #20 - #41, #44, #46 - #70
t3409 #2 - #5
t3410 #2 and #3
t3411 #2 and #3
t3412 #8, #10 - #12, #15, #17, #23, #25, -26, #28, #29, #31
t3413 #3, #5 - #10, #14, #15
and many more...

The following needs bash and /usr/local/bin first in PATH ("PATH=/usr/local/bin:$PATH make test")
t0204 #3 and #8 (or just sh, see above)
t3032 #11
t3900 #24, #25
t4201 #8
t5000 #14
t5150 #6

I though "SANE_TOOL_PATH=/usr/local/bin" plus "SHELL_PATH=/usr/local/bin/bash" would to fix the all but it does not and instead
brings up some other failures too:
t5521 #2 and #5
t5526 #2, #5, #8, #10, #12, #13, #16 - #19, #21 - #25
t5702 #3
t5800 #2, #3, #5 - #14
t9001 #66

With additionally having "PATH=/usr/local/bin:$PATH" all but one work, so there must be something wrong with SANE_TOOL_PATH?.
The single failure remaining is
t0301 #12 "helper (cache --timeout=1) times out"
I don't understand this at all, neither the -v options nor running it with bash -x helps me in understanding what the issue is.

Bye, Jojo

^ permalink raw reply

* Re: [PATCH] transport-helper: check when helpers fail
From: Johannes Sixt @ 2012-10-22 13:46 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Jeff King, Sverre Rabbelier, Shawn O. Pearce
In-Reply-To: <CAMP44s2XDkLhKkqvxnGH+U5X=42dXU1550xVQvyQk=WD2p0c6Q@mail.gmail.com>

Am 10/22/2012 13:50, schrieb Felipe Contreras:
> On Mon, Oct 22, 2012 at 8:35 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Another thought: In your use-case, isn't it so that it would be an error
>> that the process exited for whatever reason? I.e., even if it exited with
>> code 0 ("success"), it would be an error because it violated the protocol?
> 
> How is that violating the protocol?

Because the helper stops talking too early. But as I said, I actually
don't know the protocol.

I was just infering what I saw in transport-helper.c: get_helper() dup's
the output of the helper process and stores it in data->out (after
fdopen()ing on it). (The original file descriptor is handed over to
fast-import or fast-export.)

Actually, I didn't find a spot where data->out was used except to fclose()
it. But I take it that there is a reason that it exists and infer that
further output from the helper is expected by something after fast-import
or fast-export have exited.

But I may be completely off...

-- Hannes

^ permalink raw reply

* merge --no-commit not able to report stats more verbosely?
From: Scott R. Godin @ 2012-10-22 13:39 UTC (permalink / raw)
  To: git

As you can see from the below, I can't seem to get it to give me more
verbose results of what's being merged (as in the actual merge below)
with --stat or -v .. is it supposed to do that?

(develop)>$ git merge --no-commit --stat -v widget_twitter
Automatic merge went well; stopped before committing as requested
(develop|MERGING)>$ git merge --abort

(develop)>$ git merge widget_twitter
Merge made by the 'recursive' strategy.
 .../code/community/Dnd/Magentweet/Model/User.php   |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)


(develop)>$ git --version
git version 1.7.7.6



-- 
(please respond to the list as opposed to my email box directly,
unless you are supplying private information you don't want public
on the list)

^ permalink raw reply

* signing commits with openssl/PKCS#11
From: Mat Arge @ 2012-10-22 13:38 UTC (permalink / raw)
  To: git

Hy!

I would like to sign each commit with a X.509 certificate and a private key 
stored on a PKCS#11 token. I assume that that should be possible somehow using 
a hook which calls openssl. Does somebody know a working implementation of 
this?

cheers
Mat

^ permalink raw reply

* Git submodule for a local branch?
From: W. Trevor King @ 2012-10-22 12:37 UTC (permalink / raw)
  To: Git

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

I have a bunch of branches in my repo (a, b, c, …), and I'd like to
check them out into subdirectories of another branch (index).  My
initial inclination was to use something like

  $ git checkout index
  $ git branch
    a
    b
    c
  * index
  $ git submodule add -b a --reference ./ ./ dir-for-a/
  $ git submodule add -b b --reference ./ ./ dir-for-b/
  $ git submodule add -b c --reference ./ ./ dir-for-c/

but cloning a remote repository (vs. checking out a local branch)
seems to be baked into the submodule implementation.  Should I be
thinking about generalizing git-submodule.sh, or am I looking under
the wrong rock?  My ideal syntax would be something like

  $ git submodule add -b c --local dir-for-c/

The motivation is that I have website that contains a bunch of
sub-sites, and the sub-sites share content.  I have per-sub-site
branches (a, b, c) and want a master branch (index) that aggregates
them.  Perhaps this is too much to wedge into a single repository?

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: diff support for the Eiffel language?
From: Johannes Sixt @ 2012-10-22 13:35 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Ulrich Windl, git
In-Reply-To: <CACBZZX4wF8C_9ok+zeTfO70BgZXufvQaJ+8B5EiOAmxvVwr54g@mail.gmail.com>

Am 10/22/2012 15:06, schrieb Ævar Arnfjörð Bjarmason:
> On Mon, Oct 22, 2012 at 1:58 PM, Ulrich Windl 
> <Ulrich.Windl@rz.uni-regensburg.de> wrote:
>> However there's one little thing I noticed with "git diff": The
>> conte4xt lines (staring with "@@") show the current function (in Perl
>> and C), but they show the current "feature clause" in Eiffel (as
>> opposed to the expected current feature). I wonder how hard it is to
>> fix it (Observed in git 1.7.7 of openSUSE 12.1).
> 
> See git.git's e90d065 for an example of adding a new diff pattern.

It's not necessary to wait until there is built-in support for a new language.

For example, for Windows resource files, I have

*.rc    diff=winres

in .gitattributes or .git/info/attributes and

[diff "winres"]
        xfuncname =
"!^(BEGIN|END|FONT|CAPTION|STYLE)\n^[a-zA-Z_][a-zA-Z_0-9]*.*\n^[[:space:]]*([[:alnum:]_]+,
*DIALOG.*)"

in .git/config (the xfuncname is all on a single line). The first part
beginning at ! up to \n tells to ignore the specified matches. The other
parts separated by \n tell the things to put in the hunk header. You can
have "ignore" parts (with exlamation mark) and "take this" parts (without)
in any order that is convenient, as long as the last one is "take this".

-- Hannes

^ permalink raw reply

* Re: [PATCH] grep: remove tautological check
From: Peter Krefting @ 2012-10-22 13:20 UTC (permalink / raw)
  To: David Soria Parra; +Cc: Git Mailing List
In-Reply-To: <1350753964-29346-1-git-send-email-dsp@php.net>

David Soria Parra:

> -		if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
> +		if (GREP_HEADER_FIELD_MAX <= p->field)

A friend taught me this trick, which will check that it isn't negative 
for compilers that have the enumeration be signed (notably MSVC), 
while not complaining for compilers that have it unsigned (GCC, Clang):

   const unsigned int sign = 1u << (sizeof(p->field) * CHAR_BIT - 1);
   if (!(sign & (unsigned int) p->field) || GREP_HEADER_FIELD_MAX <= p->field)

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: diff support for the Eiffel language?
From: Ævar Arnfjörð Bjarmason @ 2012-10-22 13:06 UTC (permalink / raw)
  To: Ulrich Windl; +Cc: git
In-Reply-To: <508550E8020000A10000CF36@gwsmtp1.uni-regensburg.de>

On Mon, Oct 22, 2012 at 1:58 PM, Ulrich Windl
<Ulrich.Windl@rz.uni-regensburg.de> wrote:
> However there's one little thing I noticed with "git diff":
> The conte4xt lines (staring with "@@") show the current function (in Perl and C), but they show the current "feature clause" in Eiffel (as opposed to the expected current feature). I wonder how hard it is to fix it (Observed in git 1.7.7 of openSUSE 12.1).

See git.git's e90d065 for an example of adding a new diff pattern.

You could easily come up with a patch and send it to the list, however
it would probably be good to CC some Eiffel language list in case
there's some syntax oddities you've missed.

^ permalink raw reply

* [PATCH] git-send-email: use compose-encoding for Subject
From: Krzysztof Mazur @ 2012-10-22 12:41 UTC (permalink / raw)
  To: gitster, git; +Cc: Krzysztof Mazur

The commit "git-send-email: introduce compose-encoding" introduced
the compose-encoding option to specify the introduction email encoding
(--compose option), but the email Subject encoding was still hardcoded
to UTF-8.

Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
Patch against km/send-email-compose-encoding
(commit 62e0069056ed11294c29bae25df69b6518f6339e). Cleanly applies to current
next (commit 291341ca77d902dc76e204a3fc498a155f0ab75d)

 git-send-email.perl   |  8 ++++----
 t/t9001-send-email.sh | 14 ++++++++++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 107e814..adcb4e3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -636,15 +636,15 @@ EOT
 	my $need_8bit_cte = file_has_nonascii($compose_filename);
 	my $in_body = 0;
 	my $summary_empty = 1;
+	if (!defined $compose_encoding) {
+		$compose_encoding = "UTF-8";
+	}
 	while(<$c>) {
 		next if m/^GIT:/;
 		if ($in_body) {
 			$summary_empty = 0 unless (/^\n$/);
 		} elsif (/^\n$/) {
 			$in_body = 1;
-			if (!defined $compose_encoding) {
-				$compose_encoding = "UTF-8";
-			}
 			if ($need_8bit_cte) {
 				print $c2 "MIME-Version: 1.0\n",
 					 "Content-Type: text/plain; ",
@@ -658,7 +658,7 @@ EOT
 			my $subject = $initial_subject;
 			$_ = "Subject: " .
 				($subject =~ /[^[:ascii:]]/ ?
-				 quote_rfc2047($subject) :
+				 quote_rfc2047($subject, $compose_encoding) :
 				 $subject) .
 				"\n";
 		} elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 265ae04..89fceda 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -909,6 +909,20 @@ test_expect_success $PREREQ '--compose-encoding overrides sendemail.composeencod
 	grep "^Content-Type: text/plain; charset=iso-8859-2" msgtxt1
 '
 
+test_expect_success $PREREQ '--compose-encoding adds correct MIME for subject' '
+	clean_fake_sendmail &&
+	  GIT_EDITOR="\"$(pwd)/fake-editor\"" \
+	  git send-email \
+	  --compose-encoding iso-8859-2 \
+	  --compose --subject utf8-sübjëct \
+	  --from="Example <nobody@example.com>" \
+	  --to=nobody@example.com \
+	  --smtp-server="$(pwd)/fake.sendmail" \
+	  $patches &&
+	grep "^fake edit" msgtxt1 &&
+	grep "^Subject: =?iso-8859-2?q?utf8-s=C3=BCbj=C3=ABct?=" msgtxt1
+'
+
 test_expect_success $PREREQ 'detects ambiguous reference/file conflict' '
 	echo master > master &&
 	git add master &&
-- 
1.8.0.2.g35080e9

^ permalink raw reply related

* diff support for the Eiffel language?
From: Ulrich Windl @ 2012-10-22 11:58 UTC (permalink / raw)
  To: git

Hi!

After a longer pause, I did some programming in Eiffel again, and while doing so, why not use Git? It works!

However there's one little thing I noticed with "git diff":
The conte4xt lines (staring with "@@") show the current function (in Perl and C), but they show the current "feature clause" in Eiffel (as opposed to the expected current feature). I wonder how hard it is to fix it (Observed in git 1.7.7 of openSUSE 12.1).

For the non-Eiffelists:

Eiffel has a class structure like this:
class FOO

feature {BAR} -- This is a "feature clause", grouping related features (attributes/routines)

   baz (x,y : INTEGER) : STRING is
      -- blabla...
      do
         ...
      end -- baz

end -- class FOO

---
Now if I change something inside "baz", it would be nice if the @@-contect line would show "baz" (or more) instead of "feature {BAR}...".

Regards,
Ulrich
P.S. Apologies if the feature requested had been added already.

^ permalink raw reply

* Re: [PATCH] transport-helper: check when helpers fail
From: Felipe Contreras @ 2012-10-22 11:50 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: git, Junio C Hamano, Jeff King, Sverre Rabbelier, Shawn O. Pearce
In-Reply-To: <5084E931.3010809@viscovery.net>

On Mon, Oct 22, 2012 at 8:35 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 10/21/2012 21:19, schrieb Felipe Contreras:

> I would expect the function to be usable in this way:
>
>         start_command(&proc);
>
>         loop {
>                 if (check_command(&proc))
>                         break;
>         }
>
>         finish_command(&proc);
>
> but it would require a bit more work because it would have to cache the
> exit status in struct child_process.

Yes, I would expect that as well. I just noticed transport-helper also
fails with that, but some reason that's not enough to actually fail
the tests, so something weird is going on.

> BTW, you should check for return value 0 from waitpid() explicitly.

Right.

> Another thought: In your use-case, isn't it so that it would be an error
> that the process exited for whatever reason? I.e., even if it exited with
> code 0 ("success"), it would be an error because it violated the protocol?

How is that violating the protocol?

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Michael Haggerty @ 2012-10-22  8:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git
In-Reply-To: <7v7gqkgvxe.fsf@alter.siamese.dyndns.org>

On 10/21/2012 08:51 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> This patch series has the side effect that all of the directories
>> listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to
>> resolve any symlinks that are present in their paths.  It is
>> admittedly odd that a feature intended to avoid accessing expensive
>> directories would now *intentionally* access directories near the
>> expensive ones.  In the above scenario this shouldn't be a problem,
>> because /home would be the directory listed in
>> GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be
>> expensive.
> 
> Interesting observation.  In the last sentence, "accessing /home"
> does not exactly mean accessing /home, but accessing / to learn
> about "home" in it, no?

This is the extra overhead on my system for using
GIT_CEILING_DIRECTORIES=/home:

stat("/home", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
getcwd("/home/mhagger", 1024)           = 14
chdir("/home")                          = 0
getcwd("/home", 4096)                   = 6
lstat("/home", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
chdir("/home/mhagger")                  = 0

If I use GIT_CEILING_DIRECTORIES=/dev/shm, which is a symlink to
/run/shm on my system, the overhead is comparable:

stat("/dev/shm", {st_mode=S_IFDIR|S_ISVTX|0777, st_size=200, ...}) = 0
getcwd("/home/mhagger", 1024)           = 14
chdir("/dev/shm")                       = 0
getcwd("/run/shm", 4096)                = 9
lstat("/run/shm", {st_mode=S_IFDIR|S_ISVTX|0777, st_size=200, ...}) = 0
chdir("/home/mhagger")                  = 0

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [PATCH] transport-helper: check when helpers fail
From: Johannes Sixt @ 2012-10-22  6:35 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Jeff King, Sverre Rabbelier, Shawn O. Pearce
In-Reply-To: <1350847158-14154-1-git-send-email-felipe.contreras@gmail.com>

Am 10/21/2012 21:19, schrieb Felipe Contreras:
> diff --git a/run-command.c b/run-command.c
> index 1101ef7..2852e9d 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -559,6 +559,23 @@ int run_command(struct child_process *cmd)
>  	return finish_command(cmd);
>  }
>  
> +int check_command(struct child_process *cmd)
> +{
> +	int status;
> +	pid_t pid;
> +
> +	pid = waitpid(cmd->pid, &status, WNOHANG);
> +
> +	if (pid < 0)
> +		return -1;
> +	if (WIFSIGNALED(status))
> +		return WTERMSIG(status);
> +	if (WIFEXITED(status))
> +		return WEXITSTATUS(status);
> +
> +	return 0;
> +}
> +

In this form, the function is not suitable as a public run-command API: If
the child did exit, it does not allow finish_command() to do its thing.
The only thing the caller of this function can do is to die() if it
returns non-zero. It doesn't report treat error cases in the same way as
wait_or_whine().

I would expect the function to be usable in this way:

	start_command(&proc);

	loop {
		if (check_command(&proc))
			break;
	}

	finish_command(&proc);

but it would require a bit more work because it would have to cache the
exit status in struct child_process.

BTW, you should check for return value 0 from waitpid() explicitly.

Another thought: In your use-case, isn't it so that it would be an error
that the process exited for whatever reason? I.e., even if it exited with
code 0 ("success"), it would be an error because it violated the protocol?

-- Hannes

^ permalink raw reply

* Re: Is anyone working on a next-gen Git protocol?
From: Junio C Hamano @ 2012-10-22  4:59 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Jeff King, spearce, Ævar Arnfjörð
In-Reply-To: <7va9vxq5gp.fsf@alter.siamese.dyndns.org>

On Sun, Oct 7, 2012 at 3:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> I and Shawn helped privately somebody from Gerrit circle, where the
> initial ref advertisement is a huge problem (primarily because they
> add tons of refs to one commit that eventually goes to their
> integration branch), to coming up with a problem description and
> proposal document to kick-start a discussion some time ago, but not
> much has happened since.  Unless I hear from them soonish, I'll send
> a cleaned-up version of the draft before I leave for my vacation.

Which I forgot and never happened. Here is a link (not cleaned-up)

http://tinyurl.com/WhoSpeaksFirstInGit

^ permalink raw reply

* Re: Links broken in ref docs.
From: Andrew Ardill @ 2012-10-22  4:45 UTC (permalink / raw)
  To: Mike Norman; +Cc: git, Scott Chacon
In-Reply-To: <CAJr+XPGm4djBh+vacG5Ff=Y6aYmWbcUXOV9x2jekgnsGsk4b-g@mail.gmail.com>

On 21 October 2012 18:31, Mike Norman <mknorman@gmail.com> wrote:
> Many links on scm-git.org/docs simply reload the page.
>
> For example, all of Sharing and Updating section simply reload the
> docs page. And tons others. Must be a broken link or routing problem.
> Repros on FF 14.0.1 and Chrome. Good luck!
>

Including Scott Chacon as he manages this site (to my knowledge).

Looking at the request, I am getting a 302:

Request URL:http://git-scm.com/docs/git-fetch
Request Method:GET
Status Code:302 Moved Temporarily

Maybe those pages are not done yet? That doesn't seem right as this is
simply the reference manual, but perhaps there is something else going
on here.

On another (related) note, the wayback machine has some very
interesting entries for the scm-git.org domain [1] and it seems the
/doc directory is not indexed at all. Is this on purpose?

Regards,

Andrew Ardill

[1] http://wayback.archive.org/web/*/http://git-scm.com/*

^ permalink raw reply

* [PATCH v6 3/3] completion: add new zsh completion
From: Felipe Contreras @ 2012-10-22  1:45 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, SZEDER Gábor, Matthieu Moy,
	Felipe Contreras
In-Reply-To: <1350870342-22653-1-git-send-email-felipe.contreras@gmail.com>

It seems there's always issues with zsh's bash completion emulation.
I've tried to fix as many as I could and most of the fixes are already
in the latest version of zsh, but still, there are issues.

There is no point in going through all that pain; the emulation is easy
to achieve, and this patch works better than zsh's emulation.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

v5:

 * Even more simplification by using __gitcompadd

v4:

 * Simplification updates for the latest bash completion

v3:

 * Simplification
 * Avoid COMPREPLY; call compadd directly
 * Fix _get_comp_words_by_ref

 contrib/completion/git-completion.zsh | 48 +++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 contrib/completion/git-completion.zsh

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
new file mode 100644
index 0000000..ae9c532
--- /dev/null
+++ b/contrib/completion/git-completion.zsh
@@ -0,0 +1,48 @@
+#compdef git gitk
+
+# zsh completion wrapper for git
+#
+# You need git's bash completion script installed somewhere, by default on the
+# same directory as this script.
+#
+# If your script is on ~/.git-completion.sh instead, you can configure it on
+# your ~/.zshrc:
+#
+#  zstyle ':completion:*:*:git:*' script ~/.git-completion.sh
+#
+# The recommended way to install this script is to copy to
+# '~/.zsh/completion/_git', and then add the following to your ~/.zshrc file:
+#
+#  fpath=(~/.zsh/completion $fpath)
+
+complete ()
+{
+	# do nothing
+	return 0
+}
+
+zstyle -s ":completion:*:*:git:*" script script
+test -z "$script" && script="$(dirname ${funcsourcetrace[1]%:*})"/git-completion.bash
+ZSH_VERSION='' . "$script"
+
+__gitcompadd ()
+{
+	compadd -Q -S "$4" -P "${(M)cur#*[=:]}" -p "$2" -- ${=1} && _ret=0
+}
+
+_git ()
+{
+	local _ret=1
+	() {
+		emulate -L ksh
+		local cur cword prev
+		cur=${words[CURRENT-1]}
+		prev=${words[CURRENT-2]}
+		let cword=CURRENT-1
+		__${service}_main
+	}
+	let _ret && _default -S '' && _ret=0
+	return _ret
+}
+
+_git
-- 
1.8.0

^ permalink raw reply related

* [PATCH v6 2/3] completion: add new __gitcompadd helper
From: Felipe Contreras @ 2012-10-22  1:45 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, SZEDER Gábor, Matthieu Moy,
	Felipe Contreras
In-Reply-To: <1350870342-22653-1-git-send-email-felipe.contreras@gmail.com>

The idea is to never touch the COMPREPLY variable directly.

This allows other completion systems override __gitcompadd, and do
something different instead.

Also, this allows the simplification of the completion tests (separate
patch).

There should be no functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7bdd6a8..975ae13 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,6 +225,11 @@ _get_comp_words_by_ref ()
 fi
 fi
 
+__gitcompadd ()
+{
+	COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
+}
+
 # Generates completion reply with compgen, appending a space to possible
 # completion words, if necessary.
 # It accepts 1 to 4 arguments:
@@ -241,9 +246,7 @@ __gitcomp ()
 		;;
 	*)
 		local IFS=$'\n'
-		COMPREPLY=($(compgen -P "${2-}" \
-			-W "$(__gitcomp_1 "${1-}" "${4-}")" \
-			-- "$cur_"))
+		__gitcompadd "$(__gitcomp_1 "${1-}" "${4-}")" "${2-}" "$cur_" ""
 		;;
 	esac
 }
@@ -260,7 +263,7 @@ __gitcomp ()
 __gitcomp_nl ()
 {
 	local IFS=$'\n'
-	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
+	__gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
 }
 
 __git_heads ()
@@ -1603,7 +1606,7 @@ _git_config ()
 		local remote="${prev#remote.}"
 		remote="${remote%.fetch}"
 		if [ -z "$cur" ]; then
-			COMPREPLY=("refs/heads/")
+			__gitcompadd "refs/heads/"
 			return
 		fi
 		__gitcomp_nl "$(__git_refs_remotes "$remote")"
-- 
1.8.0

^ permalink raw reply related

* [PATCH v6 1/3] completion: get rid of empty COMPREPLY assignments
From: Felipe Contreras @ 2012-10-22  1:45 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, SZEDER Gábor, Matthieu Moy,
	Felipe Contreras
In-Reply-To: <1350870342-22653-1-git-send-email-felipe.contreras@gmail.com>

There's no functional reason for those, the only purpose they are
supposed to serve is to say "we don't provide any words here", but even
for that it's not used consitently.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index be800e0..7bdd6a8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -238,7 +238,6 @@ __gitcomp ()
 
 	case "$cur_" in
 	--*=)
-		COMPREPLY=()
 		;;
 	*)
 		local IFS=$'\n'
@@ -486,7 +485,6 @@ __git_complete_remote_or_refspec ()
 			case "$cmd" in
 			push) no_complete_refspec=1 ;;
 			fetch)
-				COMPREPLY=()
 				return
 				;;
 			*) ;;
@@ -502,7 +500,6 @@ __git_complete_remote_or_refspec ()
 		return
 	fi
 	if [ $no_complete_refspec = 1 ]; then
-		COMPREPLY=()
 		return
 	fi
 	[ "$remote" = "." ] && remote=
@@ -776,7 +773,6 @@ _git_am ()
 			"
 		return
 	esac
-	COMPREPLY=()
 }
 
 _git_apply ()
@@ -796,7 +792,6 @@ _git_apply ()
 			"
 		return
 	esac
-	COMPREPLY=()
 }
 
 _git_add ()
@@ -811,7 +806,6 @@ _git_add ()
 			"
 		return
 	esac
-	COMPREPLY=()
 }
 
 _git_archive ()
@@ -856,7 +850,6 @@ _git_bisect ()
 		__gitcomp_nl "$(__git_refs)"
 		;;
 	*)
-		COMPREPLY=()
 		;;
 	esac
 }
@@ -969,7 +962,6 @@ _git_clean ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_clone ()
@@ -993,7 +985,6 @@ _git_clone ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_commit ()
@@ -1027,7 +1018,6 @@ _git_commit ()
 			"
 		return
 	esac
-	COMPREPLY=()
 }
 
 _git_describe ()
@@ -1158,7 +1148,6 @@ _git_fsck ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_gc ()
@@ -1169,7 +1158,6 @@ _git_gc ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_gitk ()
@@ -1246,7 +1234,6 @@ _git_init ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_ls_files ()
@@ -1265,7 +1252,6 @@ _git_ls_files ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_ls_remote ()
@@ -1381,7 +1367,6 @@ _git_mergetool ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_merge_base ()
@@ -1397,7 +1382,6 @@ _git_mv ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_name_rev ()
@@ -1567,7 +1551,6 @@ _git_send_email ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_stage ()
@@ -1680,7 +1663,6 @@ _git_config ()
 		return
 		;;
 	*.*)
-		COMPREPLY=()
 		return
 		;;
 	esac
@@ -2060,7 +2042,6 @@ _git_remote ()
 		__gitcomp "$c"
 		;;
 	*)
-		COMPREPLY=()
 		;;
 	esac
 }
@@ -2104,7 +2085,6 @@ _git_rm ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_shortlog ()
@@ -2173,8 +2153,6 @@ _git_stash ()
 		*)
 			if [ -z "$(__git_find_on_cmdline "$save_opts")" ]; then
 				__gitcomp "$subcommands"
-			else
-				COMPREPLY=()
 			fi
 			;;
 		esac
@@ -2187,14 +2165,12 @@ _git_stash ()
 			__gitcomp "--index --quiet"
 			;;
 		show,--*|drop,--*|branch,--*)
-			COMPREPLY=()
 			;;
 		show,*|apply,*|drop,*|pop,*|branch,*)
 			__gitcomp_nl "$(git --git-dir="$(__gitdir)" stash list \
 					| sed -n -e 's/:.*//p')"
 			;;
 		*)
-			COMPREPLY=()
 			;;
 		esac
 	fi
@@ -2311,7 +2287,6 @@ _git_svn ()
 			__gitcomp "--revision= --parent"
 			;;
 		*)
-			COMPREPLY=()
 			;;
 		esac
 	fi
@@ -2336,13 +2311,10 @@ _git_tag ()
 
 	case "$prev" in
 	-m|-F)
-		COMPREPLY=()
 		;;
 	-*|tag)
 		if [ $f = 1 ]; then
 			__gitcomp_nl "$(__git_tags)"
-		else
-			COMPREPLY=()
 		fi
 		;;
 	*)
-- 
1.8.0

^ permalink raw reply related

* [PATCH v6 0/3] completion: refactor and zsh wrapper
From: Felipe Contreras @ 2012-10-22  1:45 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, SZEDER Gábor, Matthieu Moy,
	Felipe Contreras

Hi,

Here's a bit of reorganition. I'm introducing a new __gitcompadd helper that is
useful to wrapp all changes to COMPREPLY, but first, lets get rid of
unnecessary assignments as SZEDER suggested.

The zsh wrapper is now very very simple.

Since v5:
 
 * Get rid of unnecessary COMPREPLY assignments

Felipe Contreras (3):
  completion: get rid of empty COMPREPLY assignments
  completion: add new __gitcompadd helper
  completion: add new zsh completion

 contrib/completion/git-completion.bash | 41 ++++++-----------------------
 contrib/completion/git-completion.zsh  | 48 ++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 33 deletions(-)
 create mode 100644 contrib/completion/git-completion.zsh

-- 
1.8.0

^ permalink raw reply

* [PATCH 1/2] completion: refactor __gitcomp related tests
From: Felipe Contreras @ 2012-10-22  1:39 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, SZEDER Gábor, Felipe Contreras
In-Reply-To: <1350869941-22485-1-git-send-email-felipe.contreras@gmail.com>

Lots of duplicated code!

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 72 ++++++++++++++++-----------------------------------
 1 file changed, 23 insertions(+), 49 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index cbd0fb6..1c6952a 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -72,87 +72,61 @@ test_completion_long ()
 
 newline=$'\n'
 
-test_expect_success '__gitcomp - trailing space - options' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
-	--reuse-message=Z
-	--reedit-message=Z
-	--reset-author Z
-	EOF
+test_gitcomp ()
+{
+	sed -e 's/Z$//' > expected &&
 	(
 		local -a COMPREPLY &&
-		cur="--re" &&
-		__gitcomp "--dry-run --reuse-message= --reedit-message=
-				--reset-author" &&
+		cur="$1" &&
+		shift &&
+		__gitcomp "$@" &&
 		IFS="$newline" &&
 		echo "${COMPREPLY[*]}" > out
 	) &&
 	test_cmp expected out
+}
+
+test_expect_success '__gitcomp - trailing space - options' '
+	test_gitcomp "--re" "--dry-run --reuse-message= --reedit-message=
+		--reset-author" <<-EOF
+	--reuse-message=Z
+	--reedit-message=Z
+	--reset-author Z
+	EOF
 '
 
 test_expect_success '__gitcomp - trailing space - config keys' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_gitcomp "br" "branch. branch.autosetupmerge
+		branch.autosetuprebase browser." <<-\EOF
 	branch.Z
 	branch.autosetupmerge Z
 	branch.autosetuprebase Z
 	browser.Z
 	EOF
-	(
-		local -a COMPREPLY &&
-		cur="br" &&
-		__gitcomp "branch. branch.autosetupmerge
-				branch.autosetuprebase browser." &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
-	test_cmp expected out
 '
 
 test_expect_success '__gitcomp - option parameter' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_gitcomp "--strategy=re" "octopus ours recursive resolve subtree" \
+		"" "re" <<-\EOF
 	recursive Z
 	resolve Z
 	EOF
-	(
-		local -a COMPREPLY &&
-		cur="--strategy=re" &&
-		__gitcomp "octopus ours recursive resolve subtree
-			" "" "re" &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
-	test_cmp expected out
 '
 
 test_expect_success '__gitcomp - prefix' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_gitcomp "branch.me" "remote merge mergeoptions rebase" \
+		"branch.maint." "me" <<-\EOF
 	branch.maint.merge Z
 	branch.maint.mergeoptions Z
 	EOF
-	(
-		local -a COMPREPLY &&
-		cur="branch.me" &&
-		__gitcomp "remote merge mergeoptions rebase
-			" "branch.maint." "me" &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
-	test_cmp expected out
 '
 
 test_expect_success '__gitcomp - suffix' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_gitcomp "branch.me" "master maint next pu" "branch." \
+		"ma" "." <<-\EOF
 	branch.master.Z
 	branch.maint.Z
 	EOF
-	(
-		local -a COMPREPLY &&
-		cur="branch.me" &&
-		__gitcomp "master maint next pu
-			" "branch." "ma" "." &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
-	test_cmp expected out
 '
 
 test_expect_success 'basic' '
-- 
1.8.0

^ permalink raw reply related

* [PATCH 2/2] completion: simplify __gitcomp test helper
From: Felipe Contreras @ 2012-10-22  1:39 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, SZEDER Gábor, Felipe Contreras
In-Reply-To: <1350869941-22485-1-git-send-email-felipe.contreras@gmail.com>

By using print_comp as suggested by SZEDER Gábor.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 1c6952a..2e7fc06 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -74,15 +74,12 @@ newline=$'\n'
 
 test_gitcomp ()
 {
+	local -a COMPREPLY &&
 	sed -e 's/Z$//' > expected &&
-	(
-		local -a COMPREPLY &&
-		cur="$1" &&
-		shift &&
-		__gitcomp "$@" &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
+	cur="$1" &&
+	shift &&
+	__gitcomp "$@" &&
+	print_comp &&
 	test_cmp expected out
 }
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH 0/2] completion: test simplifications
From: Felipe Contreras @ 2012-10-22  1:38 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, SZEDER Gábor, Felipe Contreras

As the subject says. These came up in a discussion with SZEDER.

Felipe Contreras (2):
  completion: refactor __gitcomp related tests
  completion: simplify __gitcomp test helper

 t/t9902-completion.sh | 71 +++++++++++++++------------------------------------
 1 file changed, 21 insertions(+), 50 deletions(-)

-- 
1.8.0

^ permalink raw reply

* Re: [PATCH v5 1/3] completion: add new __gitcompadd helper
From: Felipe Contreras @ 2012-10-22  0:41 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: git, Junio C Hamano, Matthieu Moy
In-Reply-To: <20121017172808.GD2156@goldbirke>

On Wed, Oct 17, 2012 at 7:28 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:

>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index d743e56..01325de 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -225,6 +225,11 @@ _get_comp_words_by_ref ()
>>  fi
>>  fi
>>
>> +__gitcompadd ()
>> +{
>> +     COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
>> +}
>> +
>>  # Generates completion reply with compgen, appending a space to possible
>>  # completion words, if necessary.
>>  # It accepts 1 to 4 arguments:
>> @@ -238,13 +243,11 @@ __gitcomp ()
>>
>>       case "$cur_" in
>>       --*=)
>> -             COMPREPLY=()
>> +             __gitcompadd
>>               ;;
>>       *)
>>               local IFS=$'\n'
>> -             COMPREPLY=($(compgen -P "${2-}" \
>> -                     -W "$(__gitcomp_1 "${1-}" "${4-}")" \
>> -                     -- "$cur_"))
>> +             __gitcompadd "$(__gitcomp_1 "${1-}" "${4-}")" "${2-}" "$cur_" ""
>>               ;;
>>       esac
>>  }
>> @@ -261,7 +264,7 @@ __gitcomp ()
>>  __gitcomp_nl ()
>>  {
>>       local IFS=$'\n'
>> -     COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
>> +     __gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
>>  }
>
> I feel hesitant about this change.  One of the ways I'm exploring to
> fix the issues with shell metacharacters and expansion in compgen is
> to actually replace compgen.  We already iterate over all possible
> completion words in __gitcomp_1(), so it doesn't make much of a
> difference to do the filtering for the current word while we are at
> it.  However, the way __gitcompadd() encapsulates COMPREPLY=($(compgen
> ...)), and tha basic idea of never touching COMPREPLY directly make
> this basically impossible.

How is it impossible? You can still replace compgen, all you have to
do is modify __gitcompadd and replace that code with whatever custom
code you want. You can change the arguments and everything. The only
limitation is that it should be the only place where COMPREPLY is
modified, and all is good. Well, it doesn't have to be only _one_
place, but the less functions that do this, the better.

>>  __git_heads ()
>> @@ -486,7 +489,7 @@ __git_complete_remote_or_refspec ()
>>                       case "$cmd" in
>>                       push) no_complete_refspec=1 ;;
>>                       fetch)
>> -                             COMPREPLY=()
>> +                             __gitcompadd
>>                               return
>>                               ;;
>>                       *) ;;
>> @@ -502,7 +505,7 @@ __git_complete_remote_or_refspec ()
>>               return
>>       fi
>>       if [ $no_complete_refspec = 1 ]; then
>> -             COMPREPLY=()
>> +             __gitcompadd
>>               return
>>       fi
>>       [ "$remote" = "." ] && remote=
>> @@ -776,7 +779,7 @@ _git_am ()
>>                       "
>>               return
>>       esac
>> -     COMPREPLY=()
>> +     __gitcompadd
>
> These changes effectively run compgen in a subshell to generate an
> empty completion reply.  While it doesn't really matter on Linux,
> it'll add another half a tenth of a second delay in those cases on my
> Windows machine.  At least it should be conditional, i.e. $(compgen
> ...) shouldn't be executed when there are no possible completion
> words.
>
> However, I think those COMPREPLY=() assignments are pointless anyway.
> COMPREPLY is always empty when completion functions are invoked, so
> there is no need to explicitly set it to an empty array when we don't
> provide any words for completion.  Their only use is basically to
> explicitly tell us humans that in those cases we don't offer any words
> for completion.  But we don't do that consistently: there are several
> places without offering words for completion and without COMPREPLY=(),
> e.g. the '__git_has_doubledash && return' pattern.
>
> Perhaps it would be time to get rid of these COMPREPLY=() assignments?

I'm all for it, I never understood what was the purpose of that. I
believe zsh could benefit from this information to decide whether to
run the default completion (e.g. files) or not, but as you said, if
it's not used consistently for bash, there's no point in trying.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox