Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing

2015-10-20 Thread Tobias Klauser
On 2015-10-17 at 23:24:13 +0200, Junio C Hamano  wrote:
> Tobias Klauser  writes:
> 
> > On 2015-10-16 at 19:29:35 +0200, Junio C Hamano  wrote:
> >> Junio C Hamano  writes:
> >> 
> >> >> -   if (mode == INVAL)
> >> >> -   usage(usage_msg);
> >> >
> >> > When given "git stripspace -s blorg", we used to set mode to INVAL
> >> > and then showed the correct usage.  But we no longer have a check
> >> > that corresponds to the old INVAL thing, do we?  Perhaps check argc
> >> > to detect presence of an otherwise ignored non-option argument
> >> > immediately after parse_options() returns?
> >> 
> >> Perhaps like this.
> >
> > Thanks. I'll fold it into v3.
> 
> Before starting v3, please fetch from me and check what is queued on
> 'pu'.  It may turn out that the fix-ups I did while queuing this
> round is sufficient, in which case you can just say that instead ;-)

Now that patches 3 and 4 will be dropped and no changes being necessary
for patches 1 and 2 (except for your changes that I see are already
folded into 'pu'), do you want me to submit a v3 of the series? Or is it
enough if I ask you to drop patches 3 (stripspace: implement
--count-lines option) and 4 (rebase -i: use "stripspace --count-lines"
when counting todo items)?

Thanks
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Git-p4 fails with NameError with python 2.7.2

2015-10-20 Thread Etienne Girard
Hello,

Git-p4 fail when I try to rebase with the error: "NameError: global
name 'ctypes' is not defined". The error occurs when I use python
2.7.2 that is installed by default on my company's computers (it goes
without saying that everything works fine with python 2.7.10).

I'm a beginner in python, but simply importing ctypes at the beginning
of the script does the trick. I was wondering if submitting a patch
for this issue is worth the trouble, when a satisfying solution is not
using a 4 years old version of python.

Best regards
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Commit 5841520b makes it impossible to connect to github from behind my company's firewall.

2015-10-20 Thread Johan Laenen
Commit 5841520b makes it impossible to connect to github from behind my
company's firewall.

I'm running CYGWIN_NT-6.1 and the default git version 2.5.3 complains with a
fatal error when trying to git pull:

$ /bin/git --version
git version 2.5.3
$ /bin/git pull
fatal: unable to access 'https://github.com/gargle/french/': Unknown SSL
protocol error in connection to github.com:443

Taking the sources of git 2.6.1. and compiling with commit 5841520b in
http.c reverted gives me a working git.

My http.c now looks like:

 466 if (curl_http_proxy) {
 467 curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
 468 #if LIBCURL_VERSION_NUM >= 0x070a07
 469 curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
 470 #endif
 471 }

And it works:

$ git --version
git version 2.6.1
$ git pull
Already up-to-date.



Greetings,

Johan

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.

2015-10-20 Thread Matthieu Moy
Hi,

I'm just Cc-ing Enrique, the author of 5841520b.

Johan Laenen  writes:

> Commit 5841520b makes it impossible to connect to github from behind my
> company's firewall.
>
> I'm running CYGWIN_NT-6.1 and the default git version 2.5.3 complains with a
> fatal error when trying to git pull:
>
> $ /bin/git --version
> git version 2.5.3
> $ /bin/git pull
> fatal: unable to access 'https://github.com/gargle/french/': Unknown SSL
> protocol error in connection to github.com:443
>
> Taking the sources of git 2.6.1. and compiling with commit 5841520b in
> http.c reverted gives me a working git.
>
> My http.c now looks like:
>
>  466 if (curl_http_proxy) {
>  467 curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>  468 #if LIBCURL_VERSION_NUM >= 0x070a07
>  469 curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
>  470 #endif
>  471 }
>
> And it works:
>
> $ git --version
> git version 2.6.1
> $ git pull
> Already up-to-date.
>
>
>
> Greetings,
>
> Johan

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4] gitk: Add accelerator to German locale

2015-10-20 Thread Takashi Iwai
Assigned either to the first letter or some unique letter.  At least
there are no conflicts, as far as I see...

Signed-off-by: Takashi Iwai 
---
 gitk-git/po/de.po | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/gitk-git/po/de.po b/gitk-git/po/de.po
index 56c053b98428..d9ba4052e20b 100644
--- a/gitk-git/po/de.po
+++ b/gitk-git/po/de.po
@@ -9,7 +9,7 @@ msgstr ""
 "Project-Id-Version: git-gui\n"
 "Report-Msgid-Bugs-To: \n"
 "POT-Creation-Date: 2015-05-17 14:32+1000\n"
-"PO-Revision-Date: 2010-01-27 20:27+0100\n"
+"PO-Revision-Date: 2015-10-20 14:20+0200\n"
 "Last-Translator: Christian Stimming \n"
 "Language-Team: German\n"
 "Language: \n"
@@ -90,71 +90,71 @@ msgstr "Abbrechen"
 
 #: gitk:2069
 msgid "&Update"
-msgstr "Aktualisieren"
+msgstr "&Aktualisieren"
 
 #: gitk:2070
 msgid "&Reload"
-msgstr "Neu laden"
+msgstr "&Neu laden"
 
 #: gitk:2071
 msgid "Reread re&ferences"
-msgstr "Zweige neu laden"
+msgstr "&Zweige neu laden"
 
 #: gitk:2072
 msgid "&List references"
-msgstr "Zweige/Markierungen auflisten"
+msgstr "Zweige/Markierungen auf&listen"
 
 #: gitk:2074
 msgid "Start git &gui"
-msgstr "»git gui« starten"
+msgstr "»git &gui« starten"
 
 #: gitk:2076
 msgid "&Quit"
-msgstr "Beenden"
+msgstr "&Beenden"
 
 #: gitk:2068
 msgid "&File"
-msgstr "Datei"
+msgstr "&Datei"
 
 #: gitk:2080
 msgid "&Preferences"
-msgstr "Einstellungen"
+msgstr "&Einstellungen"
 
 #: gitk:2079
 msgid "&Edit"
-msgstr "Bearbeiten"
+msgstr "&Bearbeiten"
 
 #: gitk:2084
 msgid "&New view..."
-msgstr "Neue Ansicht ..."
+msgstr "&Neue Ansicht ..."
 
 #: gitk:2085
 msgid "&Edit view..."
-msgstr "Ansicht bearbeiten ..."
+msgstr "Ansicht &bearbeiten ..."
 
 #: gitk:2086
 msgid "&Delete view"
-msgstr "Ansicht entfernen"
+msgstr "Ansicht &entfernen"
 
 #: gitk:2088 gitk:4043
 msgid "&All files"
-msgstr "Alle Dateien"
+msgstr "&Alle Dateien"
 
 #: gitk:2083 gitk:4067
 msgid "&View"
-msgstr "Ansicht"
+msgstr "&Ansicht"
 
 #: gitk:2093 gitk:2103 gitk:3012
 msgid "&About gitk"
-msgstr "Über gitk"
+msgstr "Über &gitk"
 
 #: gitk:2094 gitk:2108
 msgid "&Key bindings"
-msgstr "Tastenkürzel"
+msgstr "&Tastenkürzel"
 
 #: gitk:2092 gitk:2107
 msgid "&Help"
-msgstr "Hilfe"
+msgstr "&Hilfe"
 
 #: gitk:2185 gitk:8652
 msgid "SHA1 ID:"
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] gitk: Update msgid's for menu items with accelerator

2015-10-20 Thread Takashi Iwai
The commit d99b4b0de27a ("gitk: Accelerators for the main menu")
modified the menu item strings with the accelerator, but the
translations didn't follow, thus the menus are shown without
translations.

This patch systematically update the msgid keys just to follow this
change.  The contents aren't changed, so the accelerator won't work in
these locales for now.  Each locale translator needs to add proper
acceleration keys appropriately.

Signed-off-by: Takashi Iwai 
---
 gitk-git/po/bg.po| 34 +-
 gitk-git/po/ca.po| 34 +-
 gitk-git/po/de.po| 34 +-
 gitk-git/po/es.po| 34 +-
 gitk-git/po/fr.po| 34 +-
 gitk-git/po/hu.po| 34 +-
 gitk-git/po/it.po| 36 ++--
 gitk-git/po/ja.po| 34 +-
 gitk-git/po/pt_br.po | 34 +-
 gitk-git/po/ru.po| 34 +-
 gitk-git/po/sv.po| 34 +-
 gitk-git/po/vi.po| 34 +-
 12 files changed, 205 insertions(+), 205 deletions(-)

diff --git a/gitk-git/po/bg.po b/gitk-git/po/bg.po
index 61073ebf6b56..909a56463fe6 100644
--- a/gitk-git/po/bg.po
+++ b/gitk-git/po/bg.po
@@ -89,71 +89,71 @@ msgid "Cancel"
 msgstr "Отказ"
 
 #: gitk:2069
-msgid "Update"
+msgid "&Update"
 msgstr "Обновяване"
 
 #: gitk:2070
-msgid "Reload"
+msgid "&Reload"
 msgstr "Презареждане"
 
 #: gitk:2071
-msgid "Reread references"
+msgid "Reread re&ferences"
 msgstr "Наново прочитане на настройките"
 
 #: gitk:2072
-msgid "List references"
+msgid "&List references"
 msgstr "Изброяване на указателите"
 
 #: gitk:2074
-msgid "Start git gui"
+msgid "Start git &gui"
 msgstr "Стартиране на „git gui“"
 
 #: gitk:2076
-msgid "Quit"
+msgid "&Quit"
 msgstr "Спиране на програмата"
 
 #: gitk:2068
-msgid "File"
+msgid "&File"
 msgstr "Файл"
 
 #: gitk:2080
-msgid "Preferences"
+msgid "&Preferences"
 msgstr "Настройки"
 
 #: gitk:2079
-msgid "Edit"
+msgid "&Edit"
 msgstr "Редактиране"
 
 #: gitk:2084
-msgid "New view..."
+msgid "&New view..."
 msgstr "Нов изглед…"
 
 #: gitk:2085
-msgid "Edit view..."
+msgid "&Edit view..."
 msgstr "Редактиране на изгледа…"
 
 #: gitk:2086
-msgid "Delete view"
+msgid "&Delete view"
 msgstr "Изтриване на изгледа"
 
 #: gitk:2088 gitk:4043
-msgid "All files"
+msgid "&All files"
 msgstr "Всички файлове"
 
 #: gitk:2083 gitk:4067
-msgid "View"
+msgid "&View"
 msgstr "Изглед"
 
 #: gitk:2093 gitk:2103 gitk:3012
-msgid "About gitk"
+msgid "&About gitk"
 msgstr "Относно gitk"
 
 #: gitk:2094 gitk:2108
-msgid "Key bindings"
+msgid "&Key bindings"
 msgstr "Клавишни комбинации"
 
 #: gitk:2092 gitk:2107
-msgid "Help"
+msgid "&Help"
 msgstr "Помощ"
 
 #: gitk:2185 gitk:8652
diff --git a/gitk-git/po/ca.po b/gitk-git/po/ca.po
index 976037a64504..d17691ef8d22 100644
--- a/gitk-git/po/ca.po
+++ b/gitk-git/po/ca.po
@@ -91,71 +91,71 @@ msgid "Cancel"
 msgstr "Cancel·la"
 
 #: gitk:2069
-msgid "Update"
+msgid "&Update"
 msgstr "Actualitza"
 
 #: gitk:2070
-msgid "Reload"
+msgid "&Reload"
 msgstr "Recarrega"
 
 #: gitk:2071
-msgid "Reread references"
+msgid "Reread re&ferences"
 msgstr "Rellegeix les referències"
 
 #: gitk:2072
-msgid "List references"
+msgid "&List references"
 msgstr "Llista les referències"
 
 #: gitk:2074
-msgid "Start git gui"
+msgid "Start git &gui"
 msgstr "Inicia el git gui"
 
 #: gitk:2076
-msgid "Quit"
+msgid "&Quit"
 msgstr "Surt"
 
 #: gitk:2068
-msgid "File"
+msgid "&File"
 msgstr "Fitxer"
 
 #: gitk:2080
-msgid "Preferences"
+msgid "&Preferences"
 msgstr "Preferències"
 
 #: gitk:2079
-msgid "Edit"
+msgid "&Edit"
 msgstr "Edita"
 
 #: gitk:2084
-msgid "New view..."
+msgid "&New view..."
 msgstr "Vista nova..."
 
 #: gitk:2085
-msgid "Edit view..."
+msgid "&Edit view..."
 msgstr "Edita la vista..."
 
 #: gitk:2086
-msgid "Delete view"
+msgid "&Delete view"
 msgstr "Suprimeix vista"
 
 #: gitk:2088 gitk:4043
-msgid "All files"
+msgid "&All files"
 msgstr "Tots els fitxers"
 
 #: gitk:2083 gitk:4067
-msgid "View"
+msgid "&View"
 msgstr "Vista"
 
 #: gitk:2093 gitk:2103 gitk:3012
-msgid "About gitk"
+msgid "&About gitk"
 msgstr "Quant al gitk"
 
 #: gitk:2094 gitk:2108
-msgid "Key bindings"
+msgid "&Key bindings"
 msgstr "Associacions de tecles"
 
 #: gitk:2092 gitk:2107
-msgid "Help"
+msgid "&Help"
 msgstr "Ajuda"
 
 #: gitk:2185 gitk:8652
diff --git a/gitk-git/po/de.po b/gitk-git/po/de.po
index 1a3264b2b002..56c053b98428 100644
--- a/gitk-git/po/de.po
+++ b/gitk-git/po/de.po
@@ -89,71 +89,71 @@ msgid "Cancel"
 msgstr "Abbrechen"
 
 #: gitk:2069
-msgid "Update"
+msgid "&Update"
 msgstr "Aktualisieren"
 
 #: gitk:2070
-msgid "Reload"
+msgid "&Reload"
 msgstr "Neu laden"
 
 #: gitk:2071
-msgid "Reread references"
+msgid "Reread re&ferences"
 msgstr "Zweige neu laden"
 
 #: gitk:2072

[PATCH 0/4] gitk crash fix and locale updates

2015-10-20 Thread Takashi Iwai
Hi,

the recent change in gitk to support the menu accelerator broke the
invocation with --all option in non-English locales.  Also, the whole
menu translations are gone by this, too.  This patchset tries to
address these issues.


Takashi

===

Takashi Iwai (4):
  gitk: Fix crash with --all in non-English locales
  gitk: Update msgid's for menu items with accelerator
  gitk: Add accelerators to Japanese locale
  gitk: Add accelerator to German locale

 gitk-git/gitk|  4 +--
 gitk-git/po/bg.po| 34 -
 gitk-git/po/ca.po| 34 -
 gitk-git/po/de.po| 70 ++--
 gitk-git/po/es.po| 34 -
 gitk-git/po/fr.po| 34 -
 gitk-git/po/hu.po| 34 -
 gitk-git/po/it.po| 36 +--
 gitk-git/po/ja.po| 68 +-
 gitk-git/po/pt_br.po | 34 -
 gitk-git/po/ru.po| 34 -
 gitk-git/po/sv.po| 34 -
 gitk-git/po/vi.po| 34 -
 13 files changed, 242 insertions(+), 242 deletions(-)

-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/4] gitk: Fix crash with --all in non-English locales

2015-10-20 Thread Takashi Iwai
When gitk is invoked with --all option in a non-English locale, it
crashes like:
$ LC_ALL="de_DE.UTF-8" gitk --all
Error in startup script: bad menu entry index "Ansicht bearbeiten ..."
while executing
".bar.view entryconf [mca "Edit view..."] -state normal"
invoked from within
"if {$cmdline_files ne {} || $revtreeargs ne {} || $revtreeargscmd ne {}} {
# create a view for the files/dirs specified on the command line
se..."
(file "/usr/bin/gitk" line 12442)

The reason is the leftover strings that don't match any longer with
the new string containing accelerator mark (&).  This patch corrects
these strings.

Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=951153
Signed-off-by: Takashi Iwai 
---
 gitk-git/gitk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 2028b554f487..fcc606eab735 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -12452,8 +12452,8 @@ if {$cmdline_files ne {} || $revtreeargs ne {} || 
$revtreeargscmd ne {}} {
 set viewchanged(1) 0
 set vdatemode(1) 0
 addviewmenu 1
-.bar.view entryconf [mca "Edit view..."] -state normal
-.bar.view entryconf [mca "Delete view"] -state normal
+.bar.view entryconf [mca "&Edit view..."] -state normal
+.bar.view entryconf [mca "&Delete view"] -state normal
 }
 
 if {[info exists permviews]} {
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] gitk: Add accelerators to Japanese locale

2015-10-20 Thread Takashi Iwai
Just follow the English accelerator keys.

Signed-off-by: Takashi Iwai 
---
 gitk-git/po/ja.po | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/gitk-git/po/ja.po b/gitk-git/po/ja.po
index 9bbbadd3b427..59e42a89fd7e 100644
--- a/gitk-git/po/ja.po
+++ b/gitk-git/po/ja.po
@@ -91,71 +91,71 @@ msgstr "キャンセル"
 
 #: gitk:2069
 msgid "&Update"
-msgstr "更新"
+msgstr "更新(&U)"
 
 #: gitk:2070
 msgid "&Reload"
-msgstr "リロード"
+msgstr "リロード(&R)"
 
 #: gitk:2071
 msgid "Reread re&ferences"
-msgstr "リファレンスを再読み込み"
+msgstr "リファレンスを再読み込み(&F)"
 
 #: gitk:2072
 msgid "&List references"
-msgstr "リファレンスリストを表示"
+msgstr "リファレンスリストを表示(&L)"
 
 #: gitk:2074
 msgid "Start git &gui"
-msgstr "git gui の開始"
+msgstr "git gui の開始(&G)"
 
 #: gitk:2076
 msgid "&Quit"
-msgstr "終了"
+msgstr "終了(&Q)"
 
 #: gitk:2068
 msgid "&File"
-msgstr "ファイル"
+msgstr "ファイル(&F)"
 
 #: gitk:2080
 msgid "&Preferences"
-msgstr "設定"
+msgstr "設定(&P)"
 
 #: gitk:2079
 msgid "&Edit"
-msgstr "編集"
+msgstr "編集(&E)"
 
 #: gitk:2084
 msgid "&New view..."
-msgstr "新規ビュー..."
+msgstr "新規ビュー...(&N)"
 
 #: gitk:2085
 msgid "&Edit view..."
-msgstr "ビュー編集..."
+msgstr "ビュー編集...(&E)"
 
 #: gitk:2086
 msgid "&Delete view"
-msgstr "ビュー削除"
+msgstr "ビュー削除(&D)"
 
 #: gitk:2088 gitk:4043
 msgid "&All files"
-msgstr "全てのファイル"
+msgstr "全てのファイル(&A)"
 
 #: gitk:2083 gitk:4067
 msgid "&View"
-msgstr "ビュー"
+msgstr "ビュー(&V)"
 
 #: gitk:2093 gitk:2103 gitk:3012
 msgid "&About gitk"
-msgstr "gitk について"
+msgstr "gitk について(&A)"
 
 #: gitk:2094 gitk:2108
 msgid "&Key bindings"
-msgstr "キーバインディング"
+msgstr "キーバインディング(&K)"
 
 #: gitk:2092 gitk:2107
 msgid "&Help"
-msgstr "ヘルプ"
+msgstr "ヘルプ(&H)"
 
 #: gitk:2185 gitk:8652
 msgid "SHA1 ID:"
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.

2015-10-20 Thread Enrique Tobis
Hey!

I'm really sorry to hear that.

That change should enable more forms of authentication with your proxy, but it 
does cause libcurl to choose the one it finds most secure, according to the 
docs (http://curl.haxx.se/libcurl/c/CURLOPT_HTTPAUTH.html) What kinds of 
authentication does your proxy use?

Thanks,
Enrique

-Original Message-
From: Matthieu Moy [mailto:matthieu@grenoble-inp.fr] 
Sent: Tuesday, October 20, 2015 07:46
To: Johan Laenen
Cc: git@vger.kernel.org; Enrique Tobis
Subject: Re: Commit 5841520b makes it impossible to connect to github from 
behind my company's firewall.

Hi,

I'm just Cc-ing Enrique, the author of 5841520b.

Johan Laenen  writes:

> Commit 5841520b makes it impossible to connect to github from behind my
> company's firewall.
>
> I'm running CYGWIN_NT-6.1 and the default git version 2.5.3 complains with a
> fatal error when trying to git pull:
>
> $ /bin/git --version
> git version 2.5.3
> $ /bin/git pull
> fatal: unable to access 'https://github.com/gargle/french/': Unknown SSL
> protocol error in connection to github.com:443
>
> Taking the sources of git 2.6.1. and compiling with commit 5841520b in
> http.c reverted gives me a working git.
>
> My http.c now looks like:
>
>  466 if (curl_http_proxy) {
>  467 curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>  468 #if LIBCURL_VERSION_NUM >= 0x070a07
>  469 curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
>  470 #endif
>  471 }
>
> And it works:
>
> $ git --version
> git version 2.6.1
> $ git pull
> Already up-to-date.
>
>
>
> Greetings,
>
> Johan

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git-p4 fails with NameError with python 2.7.2

2015-10-20 Thread Luke Diamand
On 20 October 2015 at 11:34, Etienne Girard  wrote:
> Hello,
>
> Git-p4 fail when I try to rebase with the error: "NameError: global
> name 'ctypes' is not defined". The error occurs when I use python
> 2.7.2 that is installed by default on my company's computers (it goes
> without saying that everything works fine with python 2.7.10).
>
> I'm a beginner in python, but simply importing ctypes at the beginning
> of the script does the trick. I was wondering if submitting a patch
> for this issue is worth the trouble, when a satisfying solution is not
> using a 4 years old version of python.

If you're able to submit a patch that would be great!

Thanks,
Luke
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.

2015-10-20 Thread Johan Laenen
Enrique Tobis  twosigma.com> writes:

> 
> Hey!
> 
> I'm really sorry to hear that.
> 
> That change should enable more forms of authentication with your proxy,
but it does cause libcurl to choose
> the one it finds most secure, according to the docs
> (http://curl.haxx.se/libcurl/c/CURLOPT_HTTPAUTH.html) What kinds of
authentication does your
> proxy use?
> 
> Thanks,
> Enrique
> 

Hi,

Thanks for looking into this.

I'm behind a NTLM proxy :/

Greetings,

Johan


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.

2015-10-20 Thread Johan Laenen
Johan Laenen  gmail.com> writes:

I'm not the only one. Another cygwin user is experiencing the exact same
problem, see http://permalink.gmane.org/gmane.os.cygwin/155039 for more info.

greetings,

Johan

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Attention!

2015-10-20 Thread Mrs. Susan
Congratulations! 
You Won  £1.5 Million Pounds on our Amnesty Int 
online promo. File in the following for your claims

Names: Sex: Country: Tel: 

Best Regards, 
Mrs. Susan Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git-p4 fails with NameError with python 2.7.2

2015-10-20 Thread Junio C Hamano
Luke Diamand  writes:

> On 20 October 2015 at 11:34, Etienne Girard  
> wrote:
>> Hello,
>>
>> Git-p4 fail when I try to rebase with the error: "NameError: global
>> name 'ctypes' is not defined". The error occurs when I use python
>> 2.7.2 that is installed by default on my company's computers (it goes
>> without saying that everything works fine with python 2.7.10).
>>
>> I'm a beginner in python, but simply importing ctypes at the beginning
>> of the script does the trick. I was wondering if submitting a patch
>> for this issue is worth the trouble, when a satisfying solution is not
>> using a 4 years old version of python.
>
> If you're able to submit a patch that would be great!

Lars's 4d25dc44 (git-p4: check free space during streaming,
2015-09-26) introduced two references to ctypes.* and there is no
'import ctypes' anywhere in the script.

I do not follow Python development, but does the above mean that
with recent 2.x you can say ctypes without first saying "import
ctypes"?  It feels somewhat non-pythonesque that identifiers like
this is given to you without you asking with an explicit 'import',
so I am puzzled.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing

2015-10-20 Thread Junio C Hamano
Tobias Klauser  writes:

> On 2015-10-17 at 23:24:13 +0200, Junio C Hamano  wrote:
>> Before starting v3, please fetch from me and check what is queued on
>> 'pu'.  It may turn out that the fix-ups I did while queuing this
>> round is sufficient, in which case you can just say that instead ;-)
>
> Now that patches 3 and 4 will be dropped and no changes being necessary
> for patches 1 and 2 (except for your changes that I see are already
> folded into 'pu'), do you want me to submit a v3 of the series? Or is it
> enough if I ask you to drop patches 3 (stripspace: implement
> --count-lines option) and 4 (rebase -i: use "stripspace --count-lines"
> when counting todo items)?

Yes, the latter is sufficient and preferrable (less work for both of
us, without losing anything to help other people to review and
discuss who may want to chime in).

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.

2015-10-20 Thread Junio C Hamano
Enrique Tobis  writes:

> Hey!
>
> I'm really sorry to hear that.
>
> That change should enable more forms of authentication with your
> proxy, but it does cause libcurl to choose the one it finds most
> secure, according to the docs
> (http://curl.haxx.se/libcurl/c/CURLOPT_HTTPAUTH.html) What kinds of
> authentication does your proxy use?

Good line of thought.  The answer would reveal what non-working
authentication form the proxy claims to support is chosen because
libcurl considers more secure than the one the user wants to use.
I'd imagine that the next step after that would be to make the list
of authentication forms configurable so that the user can say "hey
my proxy claims to support this one but it does not work" to skip
it?

That sounds like a similar approach as what we did for SSL ciphers
in f6f2a9e4 (http: add support for specifying an SSL cipher list,
2015-05-08) where some people had problems with certain cipher the
server/client claimed to support when it was in fact broken.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


How to handle false-positive with git-cherry ?

2015-10-20 Thread Francis Moreau
Hi,

I have several "maintainance" branches which are based on
different version of my software, which contains only fixes,
imported with 'git cherry-pick'.

I use to comparing stable branches to see if one of them is not
missing a fix for instance. For that purpose I use "git cherry"
of "git log --cherry".

But sometimes git reports me some false-positives because one fix
in a particular branch has been slighlty modified when cherry-picking
it (because the context has slighly changed between 2 versions of
my soft).

Basically I'd like to force git-cherry to assume that the
patch-id of a commit is the same as the one it's been cherry
picked from even if the diff is not exactly the same.

One way to do this would be forgit-cherry to use the string
added by "git-cherry-pick -x":

 (cherry picked from commit xxx)

and to handle the indirection in order to calculate the patch-id.

I couldn't find something equivalent of this with git, but maybe
I've missed an option... could anybody tell me if something
similar already exist ?

Thanks.
-- 
Francis
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git-cherry doesn't detect a "copied" commit

2015-10-20 Thread Francis Moreau
Hi,

I'm seeing something odd with git-cherry: it doesn't seem to detect
that a commit has been cherry-picked from master branch.

This happens with the systemd git repository (from github) so it
should be fairly simple to reproduce.

What I did:

$ git --version
git version 2.6.0
$ git checkout -b foo v210
$ git cherry-pick -x 9ea28c55a2488e6cd4a44ac5786f12b71ad5bc9f
$ git branch --contains 9ea28c55a2488e6cd4a44ac5786f12b71ad5bc9f
master
$ git cherry master HEAD
+ fef60bf34d1b372bea1db2515a8d936386dfc523

so git-cherry tells me that the cherry-picked commit has not
equivalent in master, which is no the case.

What am I missing ?

Thanks
-- 
Francis
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git-p4 fails with NameError with python 2.7.2

2015-10-20 Thread Manlio Perillo
On Tue, Oct 20, 2015 at 6:00 PM, Junio C Hamano  wrote:
>
> Luke Diamand  writes:
>
> > On 20 October 2015 at 11:34, Etienne Girard  
> > wrote:
> >> Hello,
> >>
> >> Git-p4 fail when I try to rebase with the error: "NameError: global
> >> name 'ctypes' is not defined". The error occurs when I use python
> >> 2.7.2 that is installed by default on my company's computers (it goes
> >> without saying that everything works fine with python 2.7.10).
> >>
> >> I'm a beginner in python, but simply importing ctypes at the beginning
> >> of the script does the trick. I was wondering if submitting a patch
> >> for this issue is worth the trouble, when a satisfying solution is not
> >> using a 4 years old version of python.
> >
> > If you're able to submit a patch that would be great!
>
> Lars's 4d25dc44 (git-p4: check free space during streaming,
> 2015-09-26) introduced two references to ctypes.* and there is no
> 'import ctypes' anywhere in the script.
>
> I do not follow Python development, but does the above mean that
> with recent 2.x you can say ctypes without first saying "import
> ctypes"?


No.
You need to import the ctypes  module.

However in Python it is possible to "inject" the ctypes module (and
any other name) in the builtin namespace.
The builtin module contains names that are accessible without importing them:
https://docs.python.org/2/library/__builtin__.html

IMHO, some code  is messing with the __builtin__ module.

Running pyflakes on git-p4.py code I get:
git-p4.py:26: 'zlib' imported but unused
git-p4.py:640: local variable 'v' is assigned to but never used
git-p4.py:2114: local variable 'rhs_index' is assigned to but never used

Running pylint I get a **lot** of warning and style issues; and the
following errors:
E:112,21: Undefined variable 'ctypes' (undefined-variable)
E:113, 8: Undefined variable 'ctypes' (undefined-variable)
E:113,51: Undefined variable 'ctypes' (undefined-variable)
E:113,94: Undefined variable 'ctypes' (undefined-variable)
E:1002,51: No value for argument 'contentFile' in method call
(no-value-for-parameter)

pyflakes is not reporting an error for ctypes.
Whatever the cause, the code must be fixed to import the ctypes module.

P.S.:
Sorry for the double message.
The first message contained an HTML part and was rejected by vger.kernel.org.


Regards  Manlio
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pull: add angle brackets to usage string

2015-10-20 Thread Alex Henrie
2015-10-19 23:17 GMT-06:00 Junio C Hamano :
> Alex Henrie  writes:
>
>> 2015-10-16 11:42 GMT-06:00 Junio C Hamano :
>>>
>>> Yes, but that fixes historical "mistake", no?
>>>
>>> With this, you are breaking historical practice by changing only one
>>> instance to deviate from the then-current practice of saying
>>> 'options' without brackets.  It is based on the point of view that
>>> considers anything inside  and a fixed string 'options' are
>>> meant to be replaced by intelligent readers, which is as valid as
>>> the more recent practice to consider only things inside 
>>> are placeholders.
>>
>> OK, I see. You're saying that it's OK to fix typos and grammatical
>> errors in contrib/examples, but it's not okay to modernize the
>> scripts' designs.
>
> Please read it again, look at contrib/examples and realize that that
> is not what I said at all.
>
> This is not about modern vs old-school.  The reason why the part of
> the patch to contrib/ under discussion is wrong is because of
> (in)consistency.
>
> Look at the output from "git grep option contrib/examples/" and
> notice that in the old days, these scripted Porcelains consistently
> said "[options]" without "".
>
> It would have been a different matter if the patch _were_ to update
> all "[options]" to "[]" in contrib/examples/ consistently,
> and such a patch might have even been an improvement, especially if
> the modern style were clearly superiour than the old-school style
> (which is not, by they way [*1*]).
>
> But that is not what the patch did.  It turned only one of them into
> "[]", making the single instance inconsistent from all the
> others around it.  That is why it was wrong.

I understand now, thanks. I really appreciate your commitment to being
consistent.

> [Footnote]
>
> *1* The "modern" style is not necessarily an improvement, by the
> way.  The way we specify that a "thing" in the help text is a
> placeholder and that there may be more instances of the same
> "thing" is to say "[...]", but in your "modernized" form,
> unlike all the other usual "things", possibly multiple options
> are spelled "[]" without having ellipses at the end,
> which is an oddball.  If we are to treat options specially like
> that anyway, intelligent readers can read an "old-school"
> description "[options]" and understand that that token stands
> for possibly multiple options just fine, and all we have gained
> by going to the "modernized" form is to waste two characters for
> .
>
> I am not saying that we should not apply the other half of the
> patch that makes builtin/pull.c say "[]".  These days,
> many other commands nearby (i.e. the "modern" ones) do use that
> form consistently, so it is an improvement.

I pushed to change [options] to [] because even if the angle
brackets don't help new users or translators in this particular case,
the angle brackets encourage Git authors to use angle brackets when
writing commands that are not so easy to understand. If you think that
[...] is better because it is even more consistent, I would be
happy to send a patch to make that change.

Anyway, thanks again for your attention to detail.

-Alex
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.

2015-10-20 Thread Enrique Tobis


From: Junio C Hamano [mailto:jch2...@gmail.com] On Behalf Of Junio C Hamano

> Enrique Tobis  writes:

>> Hey!
>>
>> I'm really sorry to hear that.
>>
>> That change should enable more forms of authentication with your 
>> proxy, but it does cause libcurl to choose the one it finds most 
>> secure, according to the docs
>> (http://curl.haxx.se/libcurl/c/CURLOPT_HTTPAUTH.html) What kinds of 
>> authentication does your proxy use?

> Good line of thought.  The answer would reveal what non-working 
> authentication form the proxy claims to support is chosen because libcurl 
> considers  more secure than the one the user wants to use.
> I'd imagine that the next step after that would be to make the list of 
> authentication forms configurable so that the user can say "hey my proxy 
> claims to support this one but it does not work" to skip it?

> That sounds like a similar approach as what we did for SSL ciphers in 
> f6f2a9e4 (http: add support for specifying an SSL cipher list,
2015-05-08) where some people had problems with certain cipher the 
server/client claimed to support when it was in fact broken.

> Thanks.

@Junio: I agree. From the post in the cygwin mailing list that Johan mentioned, 
the problem seems to be that the proxy supports NEGOTIATE, NTLM and Basic, and 
libcurl is choosing NEGOTIATE. That choice fails for that user.

There is something I don't understand, though. Johan must be configuring his 
proxy either a) through git config files; or b) through environment variables. 
Johan says his proxy uses NTLM authentication. If he is doing a), then my 
change should not have had any impact. We were already setting 
CURLOPT_PROXYAUTH to CURLAUTH_ANY in that case. If it's b), then his proxy 
couldn't have been using NTLM authentication. In the old code path, only _BASIC 
was available as an authentication mechanism. That default is what prompted me 
to make the change in the first place.

@Johan: how are you configuring your proxy? Git configuration or environment 
variables? Also, could you run GIT_CURL_VERBOSE=1 git pull and send the output. 
That should show the failing authentication method.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1] git-p4: Add option to ignore empty commits

2015-10-20 Thread Junio C Hamano
larsxschnei...@gmail.com writes:

> diff --git a/git-p4.py b/git-p4.py
> index 0093fa3..6c50c74 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2288,12 +2288,6 @@ class P4Sync(Command, P4UserMap):
>  filesToDelete = []
>  
>  for f in files:
> -# if using a client spec, only add the files that have
> -# a path in the client
> -if self.clientSpecDirs:
> -if self.clientSpecDirs.map_in_client(f['path']) == "":
> -continue
> -
>  filesForCommit.append(f)
>  if f['action'] in self.delete_actions:
>  filesToDelete.append(f)

Earlier, the paths outside the clientspec were not in filesToDelete
(or filesToRead that is below the context here).  Now they all go to
these arrays, and will hit this loop beyond the context:

# deleted files...
for f in filesToDelete:
self.streamOneP4Deletion(f)

after leaving the above for loop.  I cannot quite see where this
"stream one deletion" is turned into a no-op for paths outside after
this patch gets applied.

Also I have this suspicion that those who do want to use client spec
to get a narrowed view into the history would almost always want
this "ignore empty" behaviour (I'd even say the current behaviour to
leave empty commits by default is a bug).  What are the advantages
of keeping empty commits?  If there aren't many, perhaps git-p4
should by the default skip empties and require p4.keepEmpty
configuration to keep them?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] URL rewrite in .gitmodules

2015-10-20 Thread Junio C Hamano
Lars Schneider  writes:

> If not, what do you think about a patch that adds a "url" section
> similar to the one in git config to a .gitmodules file?
>
> Example:
> --
> [submodule "git"]
>   path = git
> url=git://github.com/larsxschneider/git.git
>
> [url "mycompany.com"]
> insteadOf = outside.com
> --

It is unclear to me if you are adding the last two (or three,
counting the blank before) lines to your company's private fork of
the opensource project, but if that is the case, then that would
defeat your earlier desire:

> ... I also would prefer not to do this as I want to use the
> very same hashes as defined by the "upstream" ...

wouldn't it?

I do not think this topic is specific to use of submodules.  If you
want to encourage your engineers to fetch from nearby mirrors you
maintain, you would want a forest of url.mine.insteadof=theirs for
the external repositories that matter to you specified by
everybody's $HOME/.gitconfig, and one way to do so would be to have
them use the configuration inclusion.  An item in your engineer
orientation material could tell them to add

[include]
path = /usr/local/etc/git/mycompany.urlrewrite

when they set up their "[user] name/email" in there.

And you can update /usr/local/etc/git/mycompany.urlrewrite as
needed.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/5] run-command: Fix missing output from late callbacks

2015-10-20 Thread Stefan Beller
The callbacks in the parallel processing API were given the contract, that
they are not allowed to print anything to stdout/err, but the API will take
care of their output needs instead.

In case a child process is started, the callback can first add its messages
to the buffer and then the child process output is buffered just in the
same buffer, and so the output will be taken care of eventually once the
child process is done.

When no child process is run, we also need to fulfill our promise to
output the buffer eventually. So when no child process is started, we need
to amend the contents of the buffer passed to the child to our buffer for
finished processes. We cannot output directly at that point in time as
another process may be in the middle of its output.

The buffer for finished child processes then also needs to be flushed in
the cleanup phase as it may contain data.

Signed-off-by: Stefan Beller 
---

Another issue I just found. This goes on top of the series
[PATCH 0/5] Fixes for the parallel processing

Thanks,
Stefan

 run-command.c  | 11 ++-
 t/t0061-run-command.sh |  9 +
 test-run-command.c | 13 +
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index 108b930..707e0a6 100644
--- a/run-command.c
+++ b/run-command.c
@@ -979,6 +979,12 @@ static void pp_cleanup(struct parallel_processes *pp)
 
free(pp->children);
free(pp->pfd);
+
+   /*
+* When get_next_task added messages to the buffer in its last
+* iteration, the buffered output is non empty.
+*/
+   fputs(pp->buffered_output.buf, stderr);
strbuf_release(&pp->buffered_output);
 
sigchain_pop_common();
@@ -1016,8 +1022,11 @@ static int pp_start_one(struct parallel_processes *pp)
if (!pp->get_next_task(&pp->children[i].data,
   &pp->children[i].process,
   &pp->children[i].err,
-  pp->data))
+  pp->data)) {
+   strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
+   strbuf_reset(&pp->children[i].err);
return 1;
+   }
 
if (start_command(&pp->children[i].process)) {
int code = pp->start_failure(&pp->children[i].process,
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index f27ada7..12228b4 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -91,4 +91,13 @@ test_expect_success 'run_command is asked to abort 
gracefully' '
test_cmp expect actual
 '
 
+cat >expect <<-EOF
+no further jobs available
+EOF
+
+test_expect_success 'run_command outputs ' '
+   test-run-command run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello 
World" 2>actual &&
+   test_cmp expect actual
+'
+
 test_done
diff --git a/test-run-command.c b/test-run-command.c
index 44710c3..91e94e8 100644
--- a/test-run-command.c
+++ b/test-run-command.c
@@ -34,6 +34,15 @@ static int parallel_next(void** task_cb,
return 1;
 }
 
+static int no_job(void** task_cb,
+ struct child_process *cp,
+ struct strbuf *err,
+ void *cb)
+{
+   strbuf_addf(err, "no further jobs available\n");
+   return 0;
+}
+
 static int task_finished(int result,
 struct child_process *cp,
 struct strbuf *err,
@@ -73,6 +82,10 @@ int main(int argc, char **argv)
exit(run_processes_parallel(jobs, parallel_next,
NULL, task_finished, &proc));
 
+   if (!strcmp(argv[1], "run-command-no-jobs"))
+   exit(run_processes_parallel(jobs, no_job,
+   NULL, task_finished, &proc));
+
fprintf(stderr, "check usage\n");
return 1;
 }
-- 
2.5.0.273.gfd9d19f.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pull: add angle brackets to usage string

2015-10-20 Thread Junio C Hamano
Alex Henrie  writes:

> I pushed to change [options] to [] because even if the angle
> brackets don't help new users or translators in this particular case,
> the angle brackets encourage Git authors to use angle brackets when
> writing commands that are not so easy to understand. If you think that
> [...] is better because it is even more consistent, I would be
> happy to send a patch to make that change.

I do agree that [...] would make things consistent between
options and non-option arguments.  But I also would expect that
reasonably intelligent readers know that options are special, and
they would understand that [options] and [] mean the same
thing as [...] anyway, so I do not feel too strongly either
way myself (meaning: I wouldn't be motivated to prepare patches for
it myself, I wouldn't jump up and down saying they are wrong and
revert them if such patches were applied by an interim maintainer
while I am on vacation, and I would apply such patches myself only
if they do not overly interfere with topics in flight while
merging).

Thanks.







--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] run-command: Fix early shutdown

2015-10-20 Thread Junio C Hamano
Stefan Beller  writes:

> The return value of `pp_collect_finished` indicates if we want to shutdown
> the parallel processing early. Both returns from that function should
> return any accumulated results.
>
> Signed-off-by: Stefan Beller 
> ---

Makes sense.  The code could "break" out of the loop, leaving only
one return site in the function, which would be a way to ensure that
we'd consistently return the accumulated result, but this would also
do.

Thanks.

>  run-command.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index ef3da27..8f47c6e 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -1077,7 +1077,7 @@ static int pp_collect_finished(struct 
> parallel_processes *pp)
>   while (pp->nr_processes > 0) {
>   pid = waitpid(-1, &wait_status, WNOHANG);
>   if (pid == 0)
> - return 0;
> + return result;
>  
>   if (pid < 0)
>   die_errno("wait");
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/5] run-command: Call get_next_task with a clean child process.

2015-10-20 Thread Junio C Hamano
Stefan Beller  writes:

> If the `get_next_task` did not explicitly called child_process_init

I locally did "If get_next_task did not explicitly call child_process_init"

> and only filled in some fields, there may have been some stale data
> in the child process. This is hard to debug and also adds a review
> burden for each new user of that API. To improve the situation, we
> pass only cleanly initialized child structs to the get_next_task.
>
> Signed-off-by: Stefan Beller 
> ---

Again this makes sense.

Another way, which I suspect may be conceptually cleaner, would be
to do this clean-up where pp->children[i].in_use is set to false, as
that is where the particular task is declared to be available for
reuse.  That location should be responsible to ensure that the task
indeed is reusable by calling child_process_init().

By the way, does child_process_init() leak old argv/env arrays, or
have they already been cleared by calling finish_command() when we
come to this codepath?

>  run-command.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/run-command.c b/run-command.c
> index 8f47c6e..b8b5747 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -1010,6 +1010,8 @@ static int pp_start_one(struct parallel_processes *pp)
>   if (i == pp->max_processes)
>   die("BUG: bookkeeping is hard");
>  
> + child_process_init(&pp->children[i].process);
> +
>   if (!pp->get_next_task(&pp->children[i].data,
>  &pp->children[i].process,
>  &pp->children[i].err,
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/5] test-run-command: Increase test coverage

2015-10-20 Thread Junio C Hamano
Stefan Beller  writes:

> Signed-off-by: Stefan Beller 
> ---
>  t/t0061-run-command.sh | 16 +---
>  test-run-command.c | 12 
>  2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
> index 0af77cd..f27ada7 100755
> --- a/t/t0061-run-command.sh
> +++ b/t/t0061-run-command.sh
> @@ -62,8 +62,18 @@ Hello
>  World
>  EOF
>  
> -test_expect_success 'run_command runs in parallel' '
> - test-run-command run-command-parallel-4 sh -c "printf \"%s\n%s\n\" 
> Hello World" 2>actual &&
> +test_expect_success 'run_command runs in parallel with more jobs available 
> than tasks' '
> + test-run-command run-command-parallel 5 sh -c "printf \"%s\n%s\n\" 
> Hello World" 2>actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'run_command runs in parallel with as many jobs as 
> tasks' '
> + test-run-command run-command-parallel 4 sh -c "printf \"%s\n%s\n\" 
> Hello World" 2>actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'run_command runs in parallel with more tasks than jobs 
> available' '
> + test-run-command run-command-parallel 3 sh -c "printf \"%s\n%s\n\" 
> Hello World" 2>actual &&
>   test_cmp expect actual
>  '
>  
> @@ -77,7 +87,7 @@ asking for a quick stop
>  EOF
>  
>  test_expect_success 'run_command is asked to abort gracefully' '
> - test-run-command run-command-abort-3 false 2>actual &&
> + test-run-command run-command-abort 3 false 2>actual &&
>   test_cmp expect actual
>  '
>  
> diff --git a/test-run-command.c b/test-run-command.c
> index 4b59482..44710c3 100644
> --- a/test-run-command.c
> +++ b/test-run-command.c
> @@ -47,6 +47,7 @@ static int task_finished(int result,
>  int main(int argc, char **argv)
>  {
>   struct child_process proc = CHILD_PROCESS_INIT;
> + int jobs;
>  
>   if (argc < 3)
>   return 1;
> @@ -61,12 +62,15 @@ int main(int argc, char **argv)
>   if (!strcmp(argv[1], "run-command"))
>   exit(run_command(&proc));
>  
> - if (!strcmp(argv[1], "run-command-parallel-4"))
> - exit(run_processes_parallel(4, parallel_next,
> + jobs = atoi(argv[2]);
> + proc.argv = (const char **)argv+3;

proc.argv = (const char **)argv + 3;

or

proc.argv = (const char **)&argv[3];

Given the line immediately before refers to argv[2], the latter
might be easier on the eyes to follow.

In what way does this "Increase" test coverage?  By allowing the
caller to specify arbitrarily higher parallelism?

> +
> + if (!strcmp(argv[1], "run-command-parallel"))
> + exit(run_processes_parallel(jobs, parallel_next,
>   NULL, NULL, &proc));
>  
> - if (!strcmp(argv[1], "run-command-abort-3"))
> - exit(run_processes_parallel(3, parallel_next,
> + if (!strcmp(argv[1], "run-command-abort"))
> + exit(run_processes_parallel(jobs, parallel_next,
>   NULL, task_finished, &proc));
>  
>   fprintf(stderr, "check usage\n");
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/5] test-run-command: Increase test coverage

2015-10-20 Thread Stefan Beller
On Tue, Oct 20, 2015 at 11:53 AM, Junio C Hamano  wrote:
>
> proc.argv = (const char **)argv + 3;
>
> or
>
> proc.argv = (const char **)&argv[3];

ok, will fix

>
> Given the line immediately before refers to argv[2], the latter
> might be easier on the eyes to follow.
>
> In what way does this "Increase" test coverage?  By allowing the
> caller to specify arbitrarily higher parallelism?

Currently we have exact 4 jobs to be run with at most 4 parallel
processes. This sounds as if we're testing only one special case,
but in reality we want to have any number of tasks be processed
successfully, so the test I came up with is 3 cases
* more tasks than max jobs (implying reused slots)
* equal number of jobs and max jobs
* less tasks than max jobs (implying unused slots)

>
>> +
>> + if (!strcmp(argv[1], "run-command-parallel"))
>> + exit(run_processes_parallel(jobs, parallel_next,
>>   NULL, NULL, &proc));
>>
>> - if (!strcmp(argv[1], "run-command-abort-3"))
>> - exit(run_processes_parallel(3, parallel_next,
>> + if (!strcmp(argv[1], "run-command-abort"))
>> + exit(run_processes_parallel(jobs, parallel_next,
>>   NULL, task_finished, &proc));
>>
>>   fprintf(stderr, "check usage\n");
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/5] run-command: Call get_next_task with a clean child process.

2015-10-20 Thread Stefan Beller
On Tue, Oct 20, 2015 at 11:49 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> If the `get_next_task` did not explicitly called child_process_init
>
> I locally did "If get_next_task did not explicitly call child_process_init"
>
>> and only filled in some fields, there may have been some stale data
>> in the child process. This is hard to debug and also adds a review
>> burden for each new user of that API. To improve the situation, we
>> pass only cleanly initialized child structs to the get_next_task.
>>
>> Signed-off-by: Stefan Beller 
>> ---
>
> Again this makes sense.
>
> Another way, which I suspect may be conceptually cleaner, would be
> to do this clean-up where pp->children[i].in_use is set to false, as
> that is where the particular task is declared to be available for
> reuse.  That location should be responsible to ensure that the task
> indeed is reusable by calling child_process_init().
>
> By the way, does child_process_init() leak old argv/env arrays, or
> have they already been cleared by calling finish_command() when we
> come to this codepath?

It does, yes. In the reroll I move the initialization to both pp_init (to init
for the first use) and to the place where in_use is set to false.

>
>>  run-command.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/run-command.c b/run-command.c
>> index 8f47c6e..b8b5747 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -1010,6 +1010,8 @@ static int pp_start_one(struct parallel_processes *pp)
>>   if (i == pp->max_processes)
>>   die("BUG: bookkeeping is hard");
>>
>> + child_process_init(&pp->children[i].process);
>> +
>>   if (!pp->get_next_task(&pp->children[i].data,
>>  &pp->children[i].process,
>>  &pp->children[i].err,
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] git-p4: import the ctypes module

2015-10-20 Thread Dennis Kaarsemaker
The ctypes module is used on windows to calculate free disk space, so it
must be imported.

Signed-off-by: Dennis Kaarsemaker 
---
 git-p4.py | 1 +
 1 file changed, 1 insertion(+)

On di, 2015-10-20 at 09:00 -0700, Junio C Hamano wrote:
> Luke Diamand  writes:
> 
> > On 20 October 2015 at 11:34, Etienne Girard <
> > etienne.g.gir...@gmail.com> wrote:
> > > Hello,
> > > 
> > > Git-p4 fail when I try to rebase with the error: "NameError:
> > > global
> > > name 'ctypes' is not defined". The error occurs when I use python
> > > 2.7.2 that is installed by default on my company's computers (it
> > > goes
> > > without saying that everything works fine with python 2.7.10).
> > > 
> > > I'm a beginner in python, but simply importing ctypes at the
> > > beginning
> > > of the script does the trick. I was wondering if submitting a
> > > patch
> > > for this issue is worth the trouble, when a satisfying solution
> > > is not
> > > using a 4 years old version of python.
> > 
> > If you're able to submit a patch that would be great!
> 
> Lars's  (git-p4: check free space during streaming,
> 2015-09-26) introduced two references to ctypes.* and there is no
> 'import ctypes' anywhere in the script.
> 
> I do not follow Python development, but does the above mean that
> with recent 2.x you can say ctypes without first saying "import
> ctypes"?  It feels somewhat non-pythonesque that identifiers like
> this is given to you without you asking with an explicit 'import',
> so I am puzzled.

No, you cannot do that. The reason others may not have noticed this bug is that
in git-p4.py, ctypes is only used on windows.

 111 if platform.system() == 'Windows':
 112 free_bytes = ctypes.c_ulonglong(0)
 113 
ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, 
None, ctypes.pointer(free_bytes))

The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's
on the same system). But this patch should help.

diff --git a/git-p4.py b/git-p4.py
index daa60c6..212ef2b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -24,6 +24,7 @@ import shutil
 import stat
 import zipfile
 import zlib
+import ctypes
 
 try:
 from subprocess import CalledProcessError
-- 
2.6.2-323-g60bd420
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-p4: import the ctypes module

2015-10-20 Thread Junio C Hamano
Dennis Kaarsemaker  writes:

>> I do not follow Python development, but does the above mean that
>> with recent 2.x you can say ctypes without first saying "import
>> ctypes"?  It feels somewhat non-pythonesque that identifiers like
>> this is given to you without you asking with an explicit 'import',
>> so I am puzzled.
>
> No, you cannot do that. The reason others may not have noticed this bug is 
> that
> in git-p4.py, ctypes is only used on windows.
>
>  111 if platform.system() == 'Windows':
>  112 free_bytes = ctypes.c_ulonglong(0)
>  113 
> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), 
> None, None, ctypes.pointer(free_bytes))
>
> The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's
> on the same system).

Exactly.  That is where my "I am puzzled" comes from.

The patch looks obviously the right thing to do.  Luke?  Lars?

>
> diff --git a/git-p4.py b/git-p4.py
> index daa60c6..212ef2b 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -24,6 +24,7 @@ import shutil
>  import stat
>  import zipfile
>  import zlib
> +import ctypes
>  
>  try:
>  from subprocess import CalledProcessError
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/1] Makefile: link libcurl before libssl

2015-10-20 Thread Junio C Hamano
Remi Pommarel  writes:

> On Mon, Oct 05, 2015 at 12:41:34PM -0700, Jonathan Nieder wrote:
> ...
>> To protect against a value that might leak in from the environment, this
>> should say
>> 
>>  IMAP_SEND_LDFLAGS =
>> 
>> [...]
>
> Oups my bad.
> ...

So, what's the status of this patch and other two patches (I
consider them as a three-patch series)?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

2015-10-20 Thread Karthik Nayak
On Mon, Oct 19, 2015 at 1:42 PM, Matthieu Moy
 wrote:
> Junio C Hamano  writes:
>
>> I personally would suggest whichever order you feel more comfortable
>> and less error-prone.
>
> This is a good summary, and I fully agree with it.

Well then, I'm working on the parsing part of it before this series :)

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/12] git submodule update: Handle unmerged submodules in C

2015-10-20 Thread Junio C Hamano
Stefan Beller  writes:

> Signed-off-by: Stefan Beller 
> ---
>  builtin/submodule--helper.c | 15 +++
>  git-submodule.sh|  6 +-
>  2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 47dc9cb..f81f37a 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -284,11 +284,18 @@ static int module_list_or_clone(int argc, const char 
> **argv, const char *prefix)
>   for (i = 0; i < list.nr; i++) {
>   const struct cache_entry *ce = list.entries[i];
>  
> - if (ce_stage(ce))
> - printf("%06o %s U\t", ce->ce_mode, 
> sha1_to_hex(null_sha1));
> - else
> - printf("%06o %s %d\t", ce->ce_mode, 
> sha1_to_hex(ce->sha1), ce_stage(ce));
> + char *env_prefix = getenv("prefix");

This somehow makes me feel dirty.  Do we really export such an
environment variable that is named overly generically to communicate
with our own helpers?

I can see why you need to be able to prefix leading paths (i.e. you
would need to prefix path to the enclosing submodule to a path to
obtain the "global view" from the very top-level superproject while
recursing into nested submodules), but still...

> + if (ce_stage(ce)) {
> + if (env_prefix)
> + fprintf(stderr, "Skipping unmerged submodule 
> %s/%s",
> + env_prefix, ce->name);
> + else
> + fprintf(stderr, "Skipping unmerged submodule 
> %s",
> + ce->name);
> + continue;
> + }
>  
> + printf("%06o %s %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), 
> ce_stage(ce));
>   utf8_fprintf(stdout, "%s\n", ce->name);
>   }
>   return 0;
> diff --git a/git-submodule.sh b/git-submodule.sh
> index d2d80e2..0754ecd 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -661,11 +661,7 @@ cmd_update()
>   while read mode sha1 stage sm_path
>   do
>   die_if_unmatched "$mode"
> - if test "$stage" = U
> - then
> - echo >&2 "Skipping unmerged submodule $prefix$sm_path"
> - continue
> - fi
> +
>   name=$(git submodule--helper name "$sm_path") || exit
>   url=$(git config submodule."$name".url)
>   if ! test -z "$update"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/12] git submodule update: Handle unmerged submodules in C

2015-10-20 Thread Stefan Beller
On Tue, Oct 20, 2015 at 2:11 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> Signed-off-by: Stefan Beller 
>> ---
>>  builtin/submodule--helper.c | 15 +++
>>  git-submodule.sh|  6 +-
>>  2 files changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index 47dc9cb..f81f37a 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -284,11 +284,18 @@ static int module_list_or_clone(int argc, const char 
>> **argv, const char *prefix)
>>   for (i = 0; i < list.nr; i++) {
>>   const struct cache_entry *ce = list.entries[i];
>>
>> - if (ce_stage(ce))
>> - printf("%06o %s U\t", ce->ce_mode, 
>> sha1_to_hex(null_sha1));
>> - else
>> - printf("%06o %s %d\t", ce->ce_mode, 
>> sha1_to_hex(ce->sha1), ce_stage(ce));
>> + char *env_prefix = getenv("prefix");
>

[Just checked the date, it's the old series. I am about to send out a new
series which collapses some patches in here, is on top of the fixes series and
off course fixes this issue ;) ]

> This somehow makes me feel dirty.  Do we really export such an
> environment variable that is named overly generically to communicate
> with our own helpers?

I agree that this is bad. It was the fastest way.
I should have taken the slower road. I think I'll replace this with
another argument.

>
> I can see why you need to be able to prefix leading paths (i.e. you
> would need to prefix path to the enclosing submodule to a path to
> obtain the "global view" from the very top-level superproject while
> recursing into nested submodules), but still...
>
>> + if (ce_stage(ce)) {
>> + if (env_prefix)
>> + fprintf(stderr, "Skipping unmerged submodule 
>> %s/%s",
>> + env_prefix, ce->name);
>> + else
>> + fprintf(stderr, "Skipping unmerged submodule 
>> %s",
>> + ce->name);
>> + continue;
>> + }
>>
>> + printf("%06o %s %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), 
>> ce_stage(ce));
>>   utf8_fprintf(stdout, "%s\n", ce->name);
>>   }
>>   return 0;
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index d2d80e2..0754ecd 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -661,11 +661,7 @@ cmd_update()
>>   while read mode sha1 stage sm_path
>>   do
>>   die_if_unmatched "$mode"
>> - if test "$stage" = U
>> - then
>> - echo >&2 "Skipping unmerged submodule $prefix$sm_path"
>> - continue
>> - fi
>> +
>>   name=$(git submodule--helper name "$sm_path") || exit
>>   url=$(git config submodule."$name".url)
>>   if ! test -z "$update"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 00/34] libify mailinfo and call it directly from am

2015-10-20 Thread Junio C Hamano
Junio C Hamano  writes:

> During the discussion on the recent "git am" regression, I noticed
> that the command reimplemented in C spawns one "mailsplit" and then
> spawns "mailinfo" followed by "apply --index" to commit the changes
> described in each message.  As there are platforms where spawning
> subprocess via run_command() interface is heavy-weight, something
> that is conceptually very simple like "mailinfo" is better called
> directly inside the process---something that is lightweight and
> frequently used is where the overhead of run_command() would be felt
> most.

Although I still haven't seen any offer to help from those who work
on the platforms that may benefit from this series the most, I have
some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB,
running Ubuntu), where the cost of spawning is not as costly as
elsewhere, making this series less pressing.

Between 'master' and the version with this series (on 'jch'),
applying this 34-patch series itself on top of 'master' using "git
am", best of 5 numbers for running:

time git am mbox >/dev/null

are

  (master) (with the series)
real0m0.648sreal0m0.537s
user0m0.358suser0m0.338s
sys 0m0.172ssys 0m0.154s

I haven't re-read the series to catch leaks yet, so in that sense
the series is still not ready to be merged to 'next' (of course,
help with fresh set of eyes is very much appreciated here).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 00/34] libify mailinfo and call it directly from am

2015-10-20 Thread Stefan Beller
On Tue, Oct 20, 2015 at 2:24 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> During the discussion on the recent "git am" regression, I noticed
>> that the command reimplemented in C spawns one "mailsplit" and then
>> spawns "mailinfo" followed by "apply --index" to commit the changes
>> described in each message.  As there are platforms where spawning
>> subprocess via run_command() interface is heavy-weight, something
>> that is conceptually very simple like "mailinfo" is better called
>> directly inside the process---something that is lightweight and
>> frequently used is where the overhead of run_command() would be felt
>> most.
>
> Although I still haven't seen any offer to help from those who work
> on the platforms that may benefit from this series the most, I have
> some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB,
> running Ubuntu), where the cost of spawning is not as costly as
> elsewhere, making this series less pressing.

As far as I understand, this only helps for mailing list workflows, which
in my limited view of the world is only found in established infrastructure
projects, who tend to be maintained by people who run some kind of
ab-nomination of unix.
("Are there people in Windows land who heavily use the email workflow?")

>
> Between 'master' and the version with this series (on 'jch'),
> applying this 34-patch series itself on top of 'master' using "git
> am", best of 5 numbers for running:
>
> time git am mbox >/dev/null
>
> are
>
>   (master) (with the series)
> real0m0.648sreal0m0.537s
> user0m0.358suser0m0.338s
> sys 0m0.172ssys 0m0.154s
>
> I haven't re-read the series to catch leaks yet, so in that sense
> the series is still not ready to be merged to 'next' (of course,
> help with fresh set of eyes is very much appreciated here).

I'll take another look after sending out my series.

> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 00/34] libify mailinfo and call it directly from am

2015-10-20 Thread Junio C Hamano
Stefan Beller  writes:

> As far as I understand, this only helps for mailing list workflows, which
> in my limited view of the world is only found in established infrastructure
> projects, who tend to be maintained by people who run some kind of
> ab-nomination of unix.
> ("Are there people in Windows land who heavily use the email workflow?")

Forgot that "am" is a workhorse behind "rebase"?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 00/34] libify mailinfo and call it directly from am

2015-10-20 Thread Stefan Beller
On Tue, Oct 20, 2015 at 3:06 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> As far as I understand, this only helps for mailing list workflows, which
>> in my limited view of the world is only found in established infrastructure
>> projects, who tend to be maintained by people who run some kind of
>> ab-nomination of unix.
>> ("Are there people in Windows land who heavily use the email workflow?")
>
> Forgot that "am" is a workhorse behind "rebase"?
>

Yes, I was deceived by the name of the series ("it must be mail related and only
mail related" was my thinking).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Oct 2015, #04; Tue, 20)

2015-10-20 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

With somewhat reduced review bandwidth, I'd expect that the upcoming
cycle would be slower than usual.  At tinyurl.com/gitCal, I
tentatively drew a 14-week schedule for this cycle (I plan to be
offline during weeks #7-#9 myself---hopefully we'll have capable
interim maintainers to take care of the list traffic in the meantime
as in past years).

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* cc/quote-comments (2015-10-07) 2 commits
  (merged to 'next' on 2015-10-09 at fc8a359)
 + quote: move comment before sq_quote_buf()
 + quote: fix broken sq_quote_buf() related comment

 A no-op code-health maintenance.


* dt/log-follow-config (2015-10-07) 1 commit
  (merged to 'next' on 2015-10-09 at 64a30d2)
 + log: Update log.follow doc and add to config.txt

 Description of the "log.follow" configuration variable in "git log"
 documentation is now also copied to "git config" documentation.


* es/worktree-add-cleanup (2015-10-07) 1 commit
  (merged to 'next' on 2015-10-09 at 6ffd721)
 + t2026: rename worktree prune test

 A no-op code-health maintenance.


* gr/rebase-i-drop-warn (2015-10-05) 2 commits
  (merged to 'next' on 2015-10-09 at 0626b96)
 + rebase-i: loosen over-eager check_bad_cmd check
 + rebase-i: explicitly accept tab as separator in commands

 "git rebase -i" had a minor regression recently, which stopped
 considering a line that begins with an indented '#' in its insn
 sheet not a comment, which is now fixed.


* jc/doc-gc-prune-now (2015-10-14) 1 commit
  (merged to 'next' on 2015-10-15 at 5c07566)
 + Documentation/gc: warn against --prune=

 "git gc" is safe to run anytime only because it has the built-in
 grace period to protect young objects.  In order to run with no
 grace period, the user must make sure that the repository is
 quiescent.


* jc/fsck-dropped-errors (2015-09-23) 1 commit
  (merged to 'next' on 2015-10-09 at 887fcac)
 + fsck: exit with non-zero when problems are found

 There were some classes of errors that "git fsck" diagnosed to its
 standard error that did not cause it to exit with non-zero status.


* jk/filter-branch-use-of-sed-on-incomplete-line (2015-10-12) 1 commit
  (merged to 'next' on 2015-10-15 at 9e7c032)
 + filter-branch: remove multi-line headers in msg filter

 A recent "filter-branch --msg-filter" broke skipping of the commit
 object header, which is fixed.


* js/clone-dissociate (2015-10-07) 4 commits
  (merged to 'next' on 2015-10-09 at ba30393)
 + clone --dissociate: avoid locking pack files
 + sha1_file.c: add a function to release all packs
 + sha1_file: consolidate code to close a pack's file descriptor
 + t5700: demonstrate a Windows file locking issue with `git clone --dissociate`

 "git clone --dissociate" runs a big "git repack" process at the
 end, and it helps to close file descriptors that are open on the
 packs and their idx files before doing so on filesystems that
 cannot remove a file that is still open.


* js/gc-with-stale-symref (2015-10-08) 2 commits
  (merged to 'next' on 2015-10-09 at 8b89576)
 + pack-objects: do not get distracted by broken symrefs
 + gc: demonstrate failure with stale remote HEAD

 "git gc" used to barf when a symbolic ref has gone dangling
 (e.g. the branch that used to be your upstream's default when you
 cloned from it is now gone, and you did "fetch --prune").


* js/icase-wt-detection (2015-09-28) 1 commit
  (merged to 'next' on 2015-10-09 at 78ff500)
 + setup: fix "inside work tree" detection on case-insensitive filesystems

 On a case insensitive filesystems, setting GIT_WORK_TREE variable
 using a random cases that does not agree with what the filesystem
 thinks confused Git that it wasn't inside the working tree.


* kn/for-each-branch (2015-09-25) 8 commits
  (merged to 'next' on 2015-10-09 at 45723ce)
 + branch: add '--points-at' option
 + branch.c: use 'ref-filter' APIs
 + branch.c: use 'ref-filter' data structures
 + branch: drop non-commit error reporting
 + branch: move 'current' check down to the presentation layer
 + branch: roll show_detached HEAD into regular ref_list
 + branch: bump get_head_description() to the top
 + branch: refactor width computation
 (this branch is used by kn/for-each-branch-remainder.)

 Update "git branch" that list existing branches, using the
 ref-filter API that is shared with "git tag" and "git
 for-each-ref".


* ls/p4-lfs (2015-10-03) 7 commits
  (merged to 'next' on 2015-10-14 at 4b8c365)
 + git-p4: add Git LFS backend for large file system
 + git-p4: add support for large file systems
 + git-p4: check free space during streaming
 + git-p4: add file streaming progress in verbose mode
 + git-p4: return an empty list if a list 

[PATCH 3/8] run-command: Initialize the shutdown flag

2015-10-20 Thread Stefan Beller
It did work out without initializing the flag so far, but to make it
future proof, we want to explicitly initialize the flag.

Signed-off-by: Stefan Beller 
---
 run-command.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/run-command.c b/run-command.c
index a5ef874..d354c01 100644
--- a/run-command.c
+++ b/run-command.c
@@ -962,6 +962,7 @@ static struct parallel_processes *pp_init(int n,
 
pp->nr_processes = 0;
pp->output_owner = 0;
+   pp->shutdown = 0;
pp->children = xcalloc(n, sizeof(*pp->children));
pp->pfd = xcalloc(n, sizeof(*pp->pfd));
strbuf_init(&pp->buffered_output, 0);
-- 
2.5.0.275.gbfc1651.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/8] test-run-command: Test for gracefully aborting

2015-10-20 Thread Stefan Beller
We reuse the get_next_task callback which would stop after invoking the
test program 4 times. However as we have only 3 parallel processes started
(We pass in 3 as max parallel processes and 3 is smaller than the spawn
cap in run-command, so we will start the 3 processes in the first run
deterministically), then the abort logic from the new task_finished
callback kicks in and prevents the parallel processing engine to start
any more processes.

As the task_finished is unconditional, we will see its output 3 times, too.

Signed-off-by: Stefan Beller 
---
 t/t0061-run-command.sh | 14 ++
 test-run-command.c | 14 ++
 2 files changed, 28 insertions(+)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 49aa3db..0af77cd 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -67,4 +67,18 @@ test_expect_success 'run_command runs in parallel' '
test_cmp expect actual
 '
 
+cat >expect <<-EOF
+preloaded output of a child
+asking for a quick stop
+preloaded output of a child
+asking for a quick stop
+preloaded output of a child
+asking for a quick stop
+EOF
+
+test_expect_success 'run_command is asked to abort gracefully' '
+   test-run-command run-command-abort-3 false 2>actual &&
+   test_cmp expect actual
+'
+
 test_done
diff --git a/test-run-command.c b/test-run-command.c
index 699d9e9..4b59482 100644
--- a/test-run-command.c
+++ b/test-run-command.c
@@ -34,6 +34,16 @@ static int parallel_next(void** task_cb,
return 1;
 }
 
+static int task_finished(int result,
+struct child_process *cp,
+struct strbuf *err,
+void *pp_cb,
+void *pp_task_cb)
+{
+   strbuf_addf(err, "asking for a quick stop\n");
+   return 1;
+}
+
 int main(int argc, char **argv)
 {
struct child_process proc = CHILD_PROCESS_INIT;
@@ -55,6 +65,10 @@ int main(int argc, char **argv)
exit(run_processes_parallel(4, parallel_next,
NULL, NULL, &proc));
 
+   if (!strcmp(argv[1], "run-command-abort-3"))
+   exit(run_processes_parallel(3, parallel_next,
+   NULL, task_finished, &proc));
+
fprintf(stderr, "check usage\n");
return 1;
 }
-- 
2.5.0.275.gbfc1651.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/8] run-command: Call get_next_task with a clean child process.

2015-10-20 Thread Stefan Beller
If the `get_next_task` did not explicitly called child_process_init
and only filled in some fields, there may have been some stale data
in the child process. This is hard to debug and also adds a review
burden for each new user of that API. To improve the situation, we
pass only cleanly initialized child structs to the get_next_task.

As an invariant you can now assume any child not in use is
cleaned up and ready for its next reuse.

Signed-off-by: Stefan Beller 
---
 run-command.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index b9363da..a5ef874 100644
--- a/run-command.c
+++ b/run-command.c
@@ -13,6 +13,12 @@ void child_process_init(struct child_process *child)
argv_array_init(&child->env_array);
 }
 
+void child_process_deinit(struct child_process *child)
+{
+   argv_array_clear(&child->args);
+   argv_array_clear(&child->env_array);
+}
+
 struct child_to_clean {
pid_t pid;
struct child_to_clean *next;
@@ -962,6 +968,7 @@ static struct parallel_processes *pp_init(int n,
 
for (i = 0; i < n; i++) {
strbuf_init(&pp->children[i].err, 0);
+   child_process_init(&pp->children[i].process);
pp->pfd[i].events = POLLIN;
pp->pfd[i].fd = -1;
}
@@ -973,8 +980,10 @@ static void pp_cleanup(struct parallel_processes *pp)
 {
int i;
 
-   for (i = 0; i < pp->max_processes; i++)
+   for (i = 0; i < pp->max_processes; i++) {
strbuf_release(&pp->children[i].err);
+   child_process_deinit(&pp->children[i].process);
+   }
 
free(pp->children);
free(pp->pfd);
@@ -1134,6 +1143,8 @@ static int pp_collect_finished(struct parallel_processes 
*pp)
pp->nr_processes--;
pp->children[i].in_use = 0;
pp->pfd[i].fd = -1;
+   child_process_deinit(&pp->children[i].process);
+   child_process_init(&pp->children[i].process);
 
if (i != pp->output_owner) {
strbuf_addbuf(&pp->buffered_output, 
&pp->children[i].err);
-- 
2.5.0.275.gbfc1651.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 8/8] git submodule update: Have a dedicated helper for cloning

2015-10-20 Thread Stefan Beller
This introduces a new helper function in git submodule--helper
which takes care of cloning all submodules, which we want to
parallelize eventually.

Some tests (such as empty URL, update_mode==none) are required in the
helper to make the decision for cloning. These checks have been moved
into the C function as well. (No need to repeat them in the shell
script)

As we can only access the stderr channel from within the parallel
processing engine, so we need to reroute the error message for
specified but initialized submodules to stderr. As it is an error
message, this should have gone to stderr in the first place, so a
bug fix along the way.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 222 
 git-submodule.sh|  45 +++--
 t/t7400-submodule-basic.sh  |   4 +-
 3 files changed, 235 insertions(+), 36 deletions(-)
 
 Review is best done starting at the end and scrolling up, as that's how the
 code flows in submodule--helper.c.

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..6d4815a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -255,6 +255,227 @@ static int module_clone(int argc, const char **argv, 
const char *prefix)
return 0;
 }
 
+static int git_submodule_config(const char *var, const char *value, void *cb)
+{
+   return parse_submodule_config_option(var, value);
+}
+
+struct submodule_update_clone {
+   int count;
+   int quiet;
+   int print_unmatched;
+   char *reference;
+   char *depth;
+   char *update;
+   const char *recursive_prefix;
+   const char *prefix;
+   struct module_list list;
+   struct string_list projectlines;
+   struct pathspec pathspec;
+};
+#define SUBMODULE_UPDATE_CLONE_INIT {0, 0, 0, NULL, NULL, NULL, NULL, NULL, 
MODULE_LIST_INIT, STRING_LIST_INIT_DUP}
+
+static void fill_clone_command(struct child_process *cp, int quiet,
+  const char *prefix, const char *path,
+  const char *name, const char *url,
+  const char *reference, const char *depth)
+{
+   cp->git_cmd = 1;
+   cp->no_stdin = 1;
+   cp->stdout_to_stderr = 1;
+   cp->err = -1;
+   argv_array_push(&cp->args, "submodule--helper");
+   argv_array_push(&cp->args, "clone");
+   if (quiet)
+   argv_array_push(&cp->args, "--quiet");
+
+   if (prefix) {
+   argv_array_push(&cp->args, "--prefix");
+   argv_array_push(&cp->args, prefix);
+   }
+   argv_array_push(&cp->args, "--path");
+   argv_array_push(&cp->args, path);
+
+   argv_array_push(&cp->args, "--name");
+   argv_array_push(&cp->args, name);
+
+   argv_array_push(&cp->args, "--url");
+   argv_array_push(&cp->args, url);
+   if (reference)
+   argv_array_push(&cp->args, reference);
+   if (depth)
+   argv_array_push(&cp->args, depth);
+}
+
+static int get_next_task(void **pp_task_cb,
+struct child_process *cp,
+struct strbuf *err,
+void *pp_cb)
+{
+   struct submodule_update_clone *pp = pp_cb;
+
+   for (; pp->count < pp->list.nr; pp->count++) {
+   const struct submodule *sub = NULL;
+   const char *displaypath = NULL;
+   const struct cache_entry *ce = pp->list.entries[pp->count];
+   struct strbuf sb = STRBUF_INIT;
+   const char *update_module = NULL;
+   const char *url = NULL;
+   int just_cloned = 0;
+
+   if (ce_stage(ce)) {
+   if (pp->recursive_prefix)
+   strbuf_addf(err, "Skipping unmerged submodule 
%s/%s\n",
+   pp->recursive_prefix, ce->name);
+   else
+   strbuf_addf(err, "Skipping unmerged submodule 
%s\n",
+   ce->name);
+   continue;
+   }
+
+   sub = submodule_from_path(null_sha1, ce->name);
+   if (pp->recursive_prefix)
+   displaypath = relative_path(pp->recursive_prefix, 
ce->name, &sb);
+   else
+   displaypath = ce->name;
+
+   if (pp->update)
+   update_module = pp->update;
+   if (!update_module)
+   update_module = sub->update;
+   if (!update_module)
+   update_module = "checkout";
+   if (!strcmp(update_module, "none")) {
+   strbuf_addf(err, "Skipping submodule '%s'\n", 
displaypath);
+   continue;
+   }
+
+   /*
+* Looking up the url in .git/config.
+* We cannot fall back to .gitmodules a

[PATCH 7/8] submodule config: Keep update strategy around

2015-10-20 Thread Stefan Beller
We need the submodule update strategies in a later patch.

Signed-off-by: Stefan Beller 
---

This may conflict with origin/sb/submodule-config-parse, but only on a
syntactical level (this adds an else if {...} just after the refactoredd code).
There is no clash of functionality or semantics.

 submodule-config.c | 11 +++
 submodule-config.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/submodule-config.c b/submodule-config.c
index 393de53..175bcbb 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -194,6 +194,7 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
 
submodule->path = NULL;
submodule->url = NULL;
+   submodule->update = NULL;
submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
submodule->ignore = NULL;
 
@@ -326,6 +327,16 @@ static int parse_config(const char *var, const char 
*value, void *data)
free((void *) submodule->url);
strbuf_addstr(&url, value);
submodule->url = strbuf_detach(&url, NULL);
+   } else if (!strcmp(item.buf, "update")) {
+   if (!value)
+   ret = config_error_nonbool(var);
+   else if (!me->overwrite && submodule->update != NULL)
+   warn_multiple_config(me->commit_sha1, submodule->name,
+"update");
+   else {
+   free((void *)submodule->update);
+   submodule->update = xstrdup(value);
+   }
}
 
 release_return:
diff --git a/submodule-config.h b/submodule-config.h
index 9061e4e..f9e2a29 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -14,6 +14,7 @@ struct submodule {
const char *url;
int fetch_recurse;
const char *ignore;
+   const char *update;
/* the sha1 blob id of the responsible .gitmodules file */
unsigned char gitmodules_sha1[20];
 };
-- 
2.5.0.275.gbfc1651.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/8] run-command: Fix missing output from late callbacks

2015-10-20 Thread Stefan Beller
The callbacks in the parallel processing API were given the contract, that
they are not allowed to print anything to stdout/err, but the API will take
care of their output needs instead.

In case a child process is started, the callback can first add its messages
to the buffer and then the child process output is buffered just in the
same buffer, and so the output will be taken care of eventually once the
child process is done.

When no child process is run, we also need to fulfill our promise to
output the buffer eventually. So when no child process is started, we need
to amend the contents of the buffer passed to the child to our buffer for
finished processes. We cannot output directly at that point in time as
another process may be in the middle of its output.

The buffer for finished child processes then also needs to be flushed in
the cleanup phase as it may contain data.

Signed-off-by: Stefan Beller 
---
 run-command.c  | 11 ++-
 t/t0061-run-command.sh |  9 +
 test-run-command.c | 13 +
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index d354c01..fa73dae 100644
--- a/run-command.c
+++ b/run-command.c
@@ -988,6 +988,12 @@ static void pp_cleanup(struct parallel_processes *pp)
 
free(pp->children);
free(pp->pfd);
+
+   /*
+* When get_next_task added messages to the buffer in its last
+* iteration, the buffered output is non empty.
+*/
+   fputs(pp->buffered_output.buf, stderr);
strbuf_release(&pp->buffered_output);
 
sigchain_pop_common();
@@ -1023,8 +1029,11 @@ static int pp_start_one(struct parallel_processes *pp)
if (!pp->get_next_task(&pp->children[i].data,
   &pp->children[i].process,
   &pp->children[i].err,
-  pp->data))
+  pp->data)) {
+   strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
+   strbuf_reset(&pp->children[i].err);
return 1;
+   }
 
if (start_command(&pp->children[i].process)) {
int code = pp->start_failure(&pp->children[i].process,
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index f27ada7..12228b4 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -91,4 +91,13 @@ test_expect_success 'run_command is asked to abort 
gracefully' '
test_cmp expect actual
 '
 
+cat >expect <<-EOF
+no further jobs available
+EOF
+
+test_expect_success 'run_command outputs ' '
+   test-run-command run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello 
World" 2>actual &&
+   test_cmp expect actual
+'
+
 test_done
diff --git a/test-run-command.c b/test-run-command.c
index c8770c2..13e5d44 100644
--- a/test-run-command.c
+++ b/test-run-command.c
@@ -34,6 +34,15 @@ static int parallel_next(void** task_cb,
return 1;
 }
 
+static int no_job(void** task_cb,
+ struct child_process *cp,
+ struct strbuf *err,
+ void *cb)
+{
+   strbuf_addf(err, "no further jobs available\n");
+   return 0;
+}
+
 static int task_finished(int result,
 struct child_process *cp,
 struct strbuf *err,
@@ -73,6 +82,10 @@ int main(int argc, char **argv)
exit(run_processes_parallel(jobs, parallel_next,
NULL, task_finished, &proc));
 
+   if (!strcmp(argv[1], "run-command-no-jobs"))
+   exit(run_processes_parallel(jobs, no_job,
+   NULL, task_finished, &proc));
+
fprintf(stderr, "check usage\n");
return 1;
 }
-- 
2.5.0.275.gbfc1651.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 5/8] test-run-command: Increase test coverage

2015-10-20 Thread Stefan Beller
Currently we have exact 4 jobs to be run with at most 4 parallel
processes. This sounds as if we're testing only one special case,
but in reality we want to have any number of tasks be processed
successfully, so test:
* more tasks than max jobs (implying slots get reused)
* equal number of jobs and max jobs
* less tasks than max jobs (implying there are unused slots)

Signed-off-by: Stefan Beller 
---
 t/t0061-run-command.sh | 16 +---
 test-run-command.c | 14 +-
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 0af77cd..f27ada7 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -62,8 +62,18 @@ Hello
 World
 EOF
 
-test_expect_success 'run_command runs in parallel' '
-   test-run-command run-command-parallel-4 sh -c "printf \"%s\n%s\n\" 
Hello World" 2>actual &&
+test_expect_success 'run_command runs in parallel with more jobs available 
than tasks' '
+   test-run-command run-command-parallel 5 sh -c "printf \"%s\n%s\n\" 
Hello World" 2>actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'run_command runs in parallel with as many jobs as tasks' '
+   test-run-command run-command-parallel 4 sh -c "printf \"%s\n%s\n\" 
Hello World" 2>actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'run_command runs in parallel with more tasks than jobs 
available' '
+   test-run-command run-command-parallel 3 sh -c "printf \"%s\n%s\n\" 
Hello World" 2>actual &&
test_cmp expect actual
 '
 
@@ -77,7 +87,7 @@ asking for a quick stop
 EOF
 
 test_expect_success 'run_command is asked to abort gracefully' '
-   test-run-command run-command-abort-3 false 2>actual &&
+   test-run-command run-command-abort 3 false 2>actual &&
test_cmp expect actual
 '
 
diff --git a/test-run-command.c b/test-run-command.c
index 4b59482..c8770c2 100644
--- a/test-run-command.c
+++ b/test-run-command.c
@@ -47,10 +47,11 @@ static int task_finished(int result,
 int main(int argc, char **argv)
 {
struct child_process proc = CHILD_PROCESS_INIT;
+   int jobs;
 
if (argc < 3)
return 1;
-   proc.argv = (const char **)argv+2;
+   proc.argv = (const char **)argv + 2;
 
if (!strcmp(argv[1], "start-command-ENOENT")) {
if (start_command(&proc) < 0 && errno == ENOENT)
@@ -61,12 +62,15 @@ int main(int argc, char **argv)
if (!strcmp(argv[1], "run-command"))
exit(run_command(&proc));
 
-   if (!strcmp(argv[1], "run-command-parallel-4"))
-   exit(run_processes_parallel(4, parallel_next,
+   jobs = atoi(argv[2]);
+   proc.argv = (const char **)argv + 3;
+
+   if (!strcmp(argv[1], "run-command-parallel"))
+   exit(run_processes_parallel(jobs, parallel_next,
NULL, NULL, &proc));
 
-   if (!strcmp(argv[1], "run-command-abort-3"))
-   exit(run_processes_parallel(3, parallel_next,
+   if (!strcmp(argv[1], "run-command-abort"))
+   exit(run_processes_parallel(jobs, parallel_next,
NULL, task_finished, &proc));
 
fprintf(stderr, "check usage\n");
-- 
2.5.0.275.gbfc1651.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/8] run-command: Fix early shutdown

2015-10-20 Thread Stefan Beller
The return value of `pp_collect_finished` indicates if we want to shutdown
the parallel processing early. Have just one return path to
return any accumulated results.

Signed-off-by: Stefan Beller 
---
 run-command.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index ef3da27..b9363da 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1077,7 +1077,7 @@ static int pp_collect_finished(struct parallel_processes 
*pp)
while (pp->nr_processes > 0) {
pid = waitpid(-1, &wait_status, WNOHANG);
if (pid == 0)
-   return 0;
+   break;
 
if (pid < 0)
die_errno("wait");
-- 
2.5.0.275.gbfc1651.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/8] Fixes for the parallel processing engine and git submodule update

2015-10-20 Thread Stefan Beller
Patches 1-6 replace the last 6 patches of sb/submodule-parallel-fetch
(Patch 1,2 changed code, 3,4 stayed as is, 5 has more commit message, 
Patch 6 is the same again)

Patches 7,8 are new in the series .
Patch 7 keeps the update strategy in the cached submodue structs around,
Patch 8 rewrites some small part of the git submodule update script in C
by having another larger helper function in builtin/submodule--helper.c
which takes care of the cloning new submodules without having all the
intermediate steps as in previous versions of this series.

The patch 8 is just a rewrite/translation without enabling the parallel
processing though. This will be done in a later patch once we have
bike shedded enough how to name the user facing option for that.
(I guess the CLI option would be --jobs again, but I'd rather hint at
the config option)

This supersedes 
[RFC PATCHv1 00/12] git submodule update in C with parallel cloning

Any feedback welcome!
Thanks,
Stefan

Stefan Beller (8):
  run-command: Fix early shutdown
  run-command: Call get_next_task with a clean child process.
  run-command: Initialize the shutdown flag
  test-run-command: Test for gracefully aborting
  test-run-command: Increase test coverage
  run-command: Fix missing output from late callbacks
  submodule config: Keep update strategy around
  git submodule update: Have a dedicated helper for cloning

 builtin/submodule--helper.c | 222 
 git-submodule.sh|  45 +++--
 run-command.c   |  27 +-
 submodule-config.c  |  11 +++
 submodule-config.h  |   1 +
 t/t0061-run-command.sh  |  37 +++-
 t/t7400-submodule-basic.sh  |   4 +-
 test-run-command.c  |  37 +++-
 8 files changed, 340 insertions(+), 44 deletions(-)

-- 
2.5.0.275.gbfc1651.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-p4: import the ctypes module

2015-10-20 Thread Luke Diamand

On 20/10/15 20:36, Junio C Hamano wrote:

Dennis Kaarsemaker  writes:


I do not follow Python development, but does the above mean that
with recent 2.x you can say ctypes without first saying "import
ctypes"?  It feels somewhat non-pythonesque that identifiers like
this is given to you without you asking with an explicit 'import',
so I am puzzled.


No, you cannot do that. The reason others may not have noticed this bug is that
in git-p4.py, ctypes is only used on windows.

  111 if platform.system() == 'Windows':
  112 free_bytes = ctypes.c_ulonglong(0)
  113 
ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, 
None, ctypes.pointer(free_bytes))

The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's
on the same system).


Exactly.  That is where my "I am puzzled" comes from.

The patch looks obviously the right thing to do.  Luke?  Lars?


It looks sensible to me, and works fine on Linux, thanks. ack.

I can't test on Windows today but I can't see why it wouldn't work.

Luke


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/8] run-command: Call get_next_task with a clean child process.

2015-10-20 Thread Junio C Hamano
Stefan Beller  writes:

> If the `get_next_task` did not explicitly called child_process_init
> and only filled in some fields, there may have been some stale data
> in the child process. This is hard to debug and also adds a review
> burden for each new user of that API. To improve the situation, we
> pass only cleanly initialized child structs to the get_next_task.
>
> As an invariant you can now assume any child not in use is
> cleaned up and ready for its next reuse.
>
> Signed-off-by: Stefan Beller 
> ---
>  run-command.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index b9363da..a5ef874 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -13,6 +13,12 @@ void child_process_init(struct child_process *child)
>   argv_array_init(&child->env_array);
>  }
>  
> +void child_process_deinit(struct child_process *child)
> +{
> + argv_array_clear(&child->args);
> + argv_array_clear(&child->env_array);
> +}
> +

Is this necessary (and is it necessary to make it global)?
I thought that finish_command() already clears them

... goes and looks ...

Ahh, of course, pp_*() functions do use start_command() but do not
use finish_command(), which sort of breaks symmetry, but that cannot
be helped.  Because we want to wait for any of the multiple tasks
running, we cannot call finish_command() that explicitly says "I
want to wait for this one to finish".

And that is why you already have two calls to array-clear inside
collect_finished(), just after calling task_finished().

And of course we already have these array-clear calls in
finish_command().

So I agree that deinit helper should exist, but

 * it should be file-scope static;

 * it should be called by finish_command(); and

 * if you are calling it from collect_finished(), then existing
   calls to array-clear should go.

Other than that, this looks good.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/8] run-command: Call get_next_task with a clean child process.

2015-10-20 Thread Junio C Hamano
Stefan Beller  writes:

> If the `get_next_task` did not explicitly called child_process_init
> and only filled in some fields, there may have been some stale data
> in the child process. This is hard to debug and also adds a review
> burden for each new user of that API. To improve the situation, we
> pass only cleanly initialized child structs to the get_next_task.
>
> As an invariant you can now assume any child not in use is
> cleaned up and ready for its next reuse.
>
> Signed-off-by: Stefan Beller 
> ---
>  run-command.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index b9363da..a5ef874 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -13,6 +13,12 @@ void child_process_init(struct child_process *child)
>   argv_array_init(&child->env_array);
>  }
>  
> +void child_process_deinit(struct child_process *child)
> +{
> + argv_array_clear(&child->args);
> + argv_array_clear(&child->env_array);
> +}
> +

Is this necessary (and is it necessary to make it global)?
I thought that finish_command() already clears them

... goes and looks ...

Ahh, of course, pp_*() functions do use start_command() but do not
use finish_command(), which sort of breaks symmetry, but that cannot
be helped.  Because we want to wait for any of the multiple tasks
running, we cannot call finish_command() that explicitly says "I
want to wait for this one to finish".

And that is why you already have two calls to array-clear inside
collect_finished(), just after calling task_finished().

And of course we already have these array-clear calls in
finish_command().

So I agree that deinit helper should exist, but

 * it should be file-scope static;

 * it should be called by finish_command(); and

 * if you are calling it from collect_finished(), then existing
   calls to array-clear should go.

Other than that, this looks good.

Thansk.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


t5516-fetch-push.sh fails with current master (74301d6)

2015-10-20 Thread Øyvind A . Holm
When building from current master (74301d6, "Sync with maint",
2015-10-20), test #75 in t5516-fetch-push.sh fails:

*** t5516-fetch-push.sh ***
ok 1 - setup
ok 2 - fetch without wildcard
[Snip 70 lines]
ok 73 - fetch exact SHA1
ok 74 - shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=true
not ok 75 - deny fetch unreachable SHA1, allowtipsha1inwant=true
#
# mk_empty testrepo &&
# (
# cd testrepo &&
# git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
# git commit --allow-empty -m foo &&
# git commit --allow-empty -m bar &&
# git commit --allow-empty -m xyz
# ) &&
# SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
# SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
# SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
# (
# cd testrepo &&
# git reset --hard $SHA1_2 &&
# git cat-file commit $SHA1_1 &&
# git cat-file commit $SHA1_3
# ) &&
# mk_empty shallow &&
# (
# cd shallow &&
# test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
# test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
# git --git-dir=../testrepo/.git config
uploadpack.allowreachablesha1inwant true &&
# git fetch ../testrepo/.git $SHA1_1 &&
# git cat-file commit $SHA1_1 &&
# test_must_fail git cat-file commit $SHA1_2 &&
# git fetch ../testrepo/.git $SHA1_2 &&
# git cat-file commit $SHA1_2 &&
# test_must_fail git fetch ../testrepo/.git $SHA1_3
# )
#
ok 76 - shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=false
ok 77 - deny fetch unreachable SHA1, allowtipsha1inwant=false
ok 78 - fetch follows tags by default
ok 79 - pushing a specific ref applies remote.$name.push as refmap
ok 80 - with no remote.$name.push, it is not used as refmap
ok 81 - with no remote.$name.push, upstream mapping is used
ok 82 - push does not follow tags by default
ok 83 - push --follow-tag only pushes relevant tags
ok 84 - push --no-thin must produce non-thin pack
ok 85 - pushing a tag pushes the tagged object
ok 86 - push into bare respects core.logallrefupdates
ok 87 - fetch into bare respects core.logallrefupdates
ok 88 - receive.denyCurrentBranch = updateInstead
ok 89 - updateInstead with push-to-checkout hook
# failed 1 among 89 test(s)
1..89
make[2]: *** [t5516-fetch-push.sh] Error 1
make[2]: Leaving directory `/home/sunny/src/other/git/build-git/t'
make[1]: *** [test] Error 2
make[1]: Leaving directory `/home/sunny/src/other/git/build-git/t'
make: *** [test] Error 2

(Removed indents to reduce email wrapping)

OS: Debian GNU/Linux 7.9 (wheezy)
gcc (Debian 4.7.2-5) 4.7.2

Regards,
Øyvind
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t5516-fetch-push.sh fails with current master (74301d6)

2015-10-20 Thread Øyvind A . Holm
On 21 October 2015 at 03:40, Øyvind A. Holm  wrote:
> When building from current master (74301d6, "Sync with maint",
> 2015-10-20), test #75 in t5516-fetch-push.sh fails

If it's of any value, the contents from
t/trash\ directory.t5516-fetch-push/ can be downloaded from
.

Regards,
Øyvind
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?

2015-10-20 Thread Noam Postavsky
On Sun, Oct 18, 2015 at 1:58 PM, Junio C Hamano  wrote:
> I cannot speak for the person who was primarily responsible for
> designing this behaviour, but I happen to agree with the current
> behaviour in the situation where it was designed to be used.  Upon
> the first use in your session, the "daemon" is auto-spawned, you can
> keep talking with that same instance during your session, and you do
> not have to do anything special to shut it down when you log out.
> Isn't that what happens here?

After looking at this some more, I've discovered this is NOT what
actually happens here. If I "git push" from a shell and then log out
and log in again, another "git push" does NOT ask me for a password.
In other words, the daemon is NOT shut down automatically when I log
out. Given that, does it make sense to change the daemon to ignore
SIGHUP, or is there some way to change it so that it does exit on
logout?

-

I don't understand why this happens, but the attached self-contained
pair of programs demonstrate the behaviour:

If I do

   make call-note-sighup note-sighup
   urxvt -e ./call-note-sighup ; cat note-sighup

sighup.log DOES contain "got sighup", but if I instead do

   make call-note-sighup note-sighup
   ./call-note-sighup ; exit

afterwards sighup.log does NOT contain "got sighup" (and I must do
"pkill note-sighup" to get rid of the lingering note-sighup process).
#include 
#include 

int main(void) {

remove("sighup.log");

int pid = fork();
if (pid < 0) {
perror("fork");
return 1;
} else if (pid == 0) {
execl("./note-sighup", "./note-sighup", NULL);
perror("execl(./note-sighup)");
return 1;
} else {
fprintf(stderr, "waiting for sigup.log to appear\n");
while (!access("sighup.log", F_OK)) {
}
fprintf(stderr, "okay, note-sighup.log has arrived, exiting in 5 seconds...\n");
sleep(5);
}

return 0;
}
#include 
#include 
#include 
#include 

static volatile int got_sighup = 0;

void sighup_handler(int sig) {
got_sighup = 1;
}

int main(void) {

struct sigaction act;
memset(&act, 0, sizeof act);
act.sa_handler = sighup_handler;
sigaction(SIGHUP, &act, NULL);

FILE* out = fopen("sighup.log", "w");
setbuf(out, NULL);
fprintf(out, "starting note-sighup\n");

for (;;) {
sleep(1000);
if (got_sighup) {
got_sighup = 0;
fprintf(out, "got sighup\n");
}
}

return 0;
}


[PATCH 0/2] blame: allow blame --reverse --first-parent when it makes sense

2015-10-20 Thread Max Kirillov
This enables --reverse --first-parent back.

Max Kirillov (2):
  Add test to describe expectation of blame --reverse with branched
history
  blame: allow blame --reverse --first-parent when it makes sense

 builtin/blame.c  | 11 +--
 t/t8009-blame-reverse.sh | 39 +++
 2 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100755 t/t8009-blame-reverse.sh

-- 
2.3.4.2801.g3d0809b

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] Add test to describe expectation of blame --reverse with branched history

2015-10-20 Thread Max Kirillov
If history contains merges from feature branches, `blame --reverse`
reports not the commit when the line was actually edited, but head of
the last merged branch which was created before the edit.

As a workaround, `blame --reverse --first-parent` could be used to find
the merge of branch containing the edit, but it was disabled in
95a4fb0eac, because incorrectly specified range could produce in
unexpected and meaningless result.

Add tests which describe ideal functionality with and without
`--first-parent`.

Signed-off-by: Max Kirillov 
---
 t/t8009-blame-reverse.sh | 34 ++
 1 file changed, 34 insertions(+)
 create mode 100755 t/t8009-blame-reverse.sh

diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh
new file mode 100755
index 000..9f40613
--- /dev/null
+++ b/t/t8009-blame-reverse.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='git blame reverse'
+. ./test-lib.sh
+
+test_expect_success setup '
+   test_commit A0 file.t line0 &&
+   test_commit A1 &&
+   git reset --hard A0 &&
+   test_commit B1 &&
+   test_commit B2 file.t line0changed &&
+   git reset --hard A1 &&
+   test_merge A2 B2 &&
+   git reset --hard A1 &&
+   test_commit C1 &&
+   git reset --hard A2 &&
+   test_merge A3 C1
+   '
+
+test_expect_failure 'blame --reverse finds B1, not C1' '
+   git blame --porcelain --reverse A0..A3 -- file.t >actual_full &&
+   head -1 actual &&
+   git rev-parse B1 >expect &&
+   test_cmp expect actual
+   '
+
+test_expect_failure 'blame --reverse --first-parent finds A1' '
+   git blame --porcelain --reverse --first-parent A0..A3 -- file.t 
>actual_full &&
+   head -1 actual &&
+   git rev-parse A1 >expect &&
+   test_cmp expect actual
+   '
+
+test_done
-- 
2.3.4.2801.g3d0809b

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] blame: allow blame --reverse --first-parent when it makes sense

2015-10-20 Thread Max Kirillov
Do not die immediately when the two flags are specified. Instead
check that the specified range is along first-parent chain.  Explioit
how prepare_revision_walk() handles first_parent_only flag: the commits
outside of first-parent chain are either unknown (and do not have any
children recorded) or appear as non-first parent of a commit along the
first-parent chain.

Since the check seems fragile, add test which makes sure that blame dies
in both cases.

Signed-off-by: Max Kirillov 
---
 builtin/blame.c  | 11 +--
 t/t8009-blame-reverse.sh |  7 ++-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 295ce92..27de544 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2692,8 +2692,6 @@ parse_done:
}
else if (contents_from)
die("--contents and --children do not blend well.");
-   else if (revs.first_parent_only)
-   die("combining --first-parent and --reverse is not supported");
else {
final_commit_name = prepare_initial(&sb);
sb.commits.compare = compare_commits_by_reverse_commit_date;
@@ -2721,6 +2719,15 @@ parse_done:
if (prepare_revision_walk(&revs))
die(_("revision walk setup failed"));
 
+   if (reverse && revs.first_parent_only) {
+   struct commit_list *final_children = 
lookup_decoration(&revs.children,
+  
&sb.final->object);
+   if (!final_children ||
+   hashcmp(final_children->item->parents->item->object.sha1,
+   sb.final->object.sha1))
+   die("--reverse --first-parent together require range along 
first-parent chain");
+   }
+
if (is_null_sha1(sb.final->object.sha1)) {
o = sb.final->util;
sb.final_buf = xmemdupz(o->file.ptr, o->file.size);
diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh
index 9f40613..042863b 100755
--- a/t/t8009-blame-reverse.sh
+++ b/t/t8009-blame-reverse.sh
@@ -24,11 +24,16 @@ test_expect_failure 'blame --reverse finds B1, not C1' '
test_cmp expect actual
'
 
-test_expect_failure 'blame --reverse --first-parent finds A1' '
+test_expect_success 'blame --reverse --first-parent finds A1' '
git blame --porcelain --reverse --first-parent A0..A3 -- file.t 
>actual_full &&
head -1 actual &&
git rev-parse A1 >expect &&
test_cmp expect actual
'
 
+test_expect_success 'blame --reverse --first-parse dies if no first parent 
chain' '
+   test_must_fail git blame --porcelain --reverse --first-parent B1..A3 -- 
file.t &&
+   test_must_fail git blame --porcelain --reverse --first-parent B2..A3 -- 
file.t
+   '
+
 test_done
-- 
2.3.4.2801.g3d0809b

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] blame: allow blame --reverse --first-parent when it makes sense

2015-10-20 Thread Eric Sunshine
On Wed, Oct 21, 2015 at 12:08 AM, Max Kirillov  wrote:
> Do not die immediately when the two flags are specified. Instead
> check that the specified range is along first-parent chain.  Explioit

s/Explioit/Exploit/

> how prepare_revision_walk() handles first_parent_only flag: the commits
> outside of first-parent chain are either unknown (and do not have any
> children recorded) or appear as non-first parent of a commit along the
> first-parent chain.
>
> Since the check seems fragile, add test which makes sure that blame dies
> in both cases.
>
> Signed-off-by: Max Kirillov 
> ---
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 295ce92..27de544 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2692,8 +2692,6 @@ parse_done:
> }
> else if (contents_from)
> die("--contents and --children do not blend well.");
> -   else if (revs.first_parent_only)
> -   die("combining --first-parent and --reverse is not 
> supported");
> else {
> final_commit_name = prepare_initial(&sb);
> sb.commits.compare = compare_commits_by_reverse_commit_date;
> @@ -2721,6 +2719,15 @@ parse_done:
> if (prepare_revision_walk(&revs))
> die(_("revision walk setup failed"));
>
> +   if (reverse && revs.first_parent_only) {
> +   struct commit_list *final_children = 
> lookup_decoration(&revs.children,
> +  
> &sb.final->object);
> +   if (!final_children ||
> +   hashcmp(final_children->item->parents->item->object.sha1,
> +   sb.final->object.sha1))
> +   die("--reverse --first-parent together require range 
> along first-parent chain");
> +   }
> +
> if (is_null_sha1(sb.final->object.sha1)) {
> o = sb.final->util;
> sb.final_buf = xmemdupz(o->file.ptr, o->file.size);
> diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh
> index 9f40613..042863b 100755
> --- a/t/t8009-blame-reverse.sh
> +++ b/t/t8009-blame-reverse.sh
> @@ -24,11 +24,16 @@ test_expect_failure 'blame --reverse finds B1, not C1' '
> test_cmp expect actual
> '
>
> -test_expect_failure 'blame --reverse --first-parent finds A1' '
> +test_expect_success 'blame --reverse --first-parent finds A1' '
> git blame --porcelain --reverse --first-parent A0..A3 -- file.t 
> >actual_full &&
> head -1 actual &&
> git rev-parse A1 >expect &&
> test_cmp expect actual
> '
>
> +test_expect_success 'blame --reverse --first-parse dies if no first parent 
> chain' '
> +   test_must_fail git blame --porcelain --reverse --first-parent B1..A3 
> -- file.t &&
> +   test_must_fail git blame --porcelain --reverse --first-parent B2..A3 
> -- file.t
> +   '
> +
>  test_done
> --
> 2.3.4.2801.g3d0809b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git ls-files should not show worktree files as untracked

2015-10-20 Thread 乙酸鋰
Hi,
Using git 2.6.2
I think worktree should behave consistently like submodule

Run following commands
```
echo 1 > 1.txt
git init
git add 1.txt
git commit -m "initial commit"
echo 2 > 2.txt # untracked file
mkdir def
cd def
git clone --separate-git-dir ../.git/ghi .. . # simulate an untracked submodule
cd ..
git worktree add abc

git ls-files --exclude-standard --full-name --others
```

Actual: list all the files of abc
2.txt
abc/1.txt
def/

Expected: just the directory of abc
2.txt
abc/
def/


Hint: path.c: git_path_submodule() should read "commondir" ?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t5516-fetch-push.sh fails with current master (74301d6)

2015-10-20 Thread Øyvind A . Holm
On 21 October 2015 at 04:01, Øyvind A. Holm  wrote:
> On 21 October 2015 at 03:40, Øyvind A. Holm  wrote:
> > When building from current master (74301d6, "Sync with maint",
> > 2015-10-20), test #75 in t5516-fetch-push.sh fails

Hm, seems as I'm unable to reproduce the failure. That's strange, I have
an automated build script I've been using for years, so the build
environment is unchanged from the previous builds. I'll investigate this
a bit and come back if there's anything newsworthy. In the meantime,
sorry about the noise.

Regards,
Øyvind
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.

2015-10-20 Thread Johan Laenen
Enrique Tobis  twosigma.com> writes:

>   Johan: how are you configuring your proxy? Git configuration or
environment variables? Also, could you


I'm just using the https_proxy environment variable, not the git configuration:

$ export https_proxy=http://:@myproxy:8080



Sorry for the multiple attemps at answering your questions, but the mailing
list keeps on complaining about spam :/

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] fix flaky untracked-cache test

2015-10-20 Thread Torsten Bögershausen
On 19.10.15 21:48, David Turner wrote:

> + echo test >base && #we need to ensure that the root dir is touched
> + rm base
>  '
Thanks for working on this, (I can run the test as soon as I have access to a 
Mac with SSD)
Minor remark, the echo test can be removed (and may be the comment ?)
> + >base &&
> + rm base
>  '


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.

2015-10-20 Thread Johan Laenen
Enrique Tobis  twosigma.com> writes:

> run GIT_CURL_VERBOSE=1 git pull and send the output. That should show the 
> failing authentication method.



Please see the file in attachment,

greetings,

Johan
$ GIT_CURL_VERBOSE=1 /bin/git pull
* STATE: INIT => CONNECT handle 0x20081bd0; line 1090 (connection #-5000)
* Couldn't find host github.com in the .netrc file; using defaults
* Added connection 0. The cache now contains 1 members
*   Trying 10.192.45.149...
* STATE: CONNECT => WAITCONNECT handle 0x20081bd0; line 1143 (connection #0)
* Connected to myproxy (10.192.45.149) port 8080 (#0)
* STATE: WAITCONNECT => WAITPROXYCONNECT handle 0x20081bd0; line 1240 
(connection #0)
* Establish HTTP proxy tunnel to github.com:443
> CONNECT github.com:443 HTTP/1.1
Host: github.com:443
User-Agent: git/2.5.3
Proxy-Connection: Keep-Alive

* Read response immediately from proxy CONNECT
< HTTP/1.1 407 Proxy Authentication Required
< Proxy-Authenticate: NEGOTIATE
< Proxy-Authenticate: NTLM
< Proxy-Authenticate: BASIC realm="POST_AD"
< Cache-Control: no-cache
< Pragma: no-cache
< Content-Type: text/html; charset=utf-8
< Proxy-Connection: close
< Connection: close
< Content-Length: 1025
<
* Ignore 1025 bytes of response-body
* Connect me again please
* ALPN, offering http/1.1
* Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH
* successfully set certificate verify locations:
*   CAfile: /etc/pki/tls/certs/ca-bundle.crt
  CApath: none
* Unknown SSL protocol error in connection to github.com:443
* Curl_done
* Closing connection 0
* The cache now contains 0 members
* STATE: WAITPROXYCONNECT => CONNECT handle 0x20081bd0; line 1223 (connection 
#-5000)
* Couldn't find host github.com in the .netrc file; using defaults
* Added connection 1. The cache now contains 1 members
* Hostname myproxy was found in DNS cache
*   Trying 10.192.45.149...
* STATE: CONNECT => WAITCONNECT handle 0x20081bd0; line 1143 (connection #1)
* Connected to myproxy (10.192.45.149) port 8080 (#1)
* STATE: WAITCONNECT => WAITPROXYCONNECT handle 0x20081bd0; line 1240 
(connection #1)
* Establish HTTP proxy tunnel to github.com:443
> CONNECT github.com:443 HTTP/1.1
Host: github.com:443
User-Agent: git/2.5.3
Proxy-Connection: Keep-Alive

* Read response immediately from proxy CONNECT
< HTTP/1.1 407 Proxy Authentication Required
< Proxy-Authenticate: NEGOTIATE
* gss_init_sec_context() failed: : SPNEGO cannot find mechanisms to negotiate
< Proxy-Authenticate: NTLM
< Proxy-Authenticate: BASIC realm="POST_AD"
< Cache-Control: no-cache
< Pragma: no-cache
< Content-Type: text/html; charset=utf-8
< Proxy-Connection: close
< Connection: close
< Content-Length: 1025
<
* Received HTTP code 407 from proxy after CONNECT
* Expire cleared
* Curl_done
* Closing connection 1
* The cache now contains 0 members
fatal: unable to access 'https://github.com/gargle/french/': Unknown SSL 
protocol error in connection to github.com:443


Re: [PATCH v1] git-p4: Add option to ignore empty commits

2015-10-20 Thread Luke Diamand

On 19/10/15 19:43, larsxschnei...@gmail.com wrote:

From: Lars Schneider 

A changelist that contains only excluded files (e.g. via client spec or
branch prefix) will be imported as empty commit. Add option
"git-p4.ignoreEmptyCommits" to ignore these commits.

Signed-off-by: Lars Schneider 
---
  Documentation/git-p4.txt   |   5 ++
  git-p4.py  |  41 -
  t/t9826-git-p4-ignore-empty-commits.sh | 103 +
  3 files changed, 133 insertions(+), 16 deletions(-)
  create mode 100755 t/t9826-git-p4-ignore-empty-commits.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..f096a6a 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,11 @@ git-p4.useClientSpec::
option '--use-client-spec'.  See the "CLIENT SPEC" section above.
This variable is a boolean, not the name of a p4 client.

+git-p4.ignoreEmptyCommits::
+   A changelist that contains only excluded files will be imported
+   as empty commit. To ignore these commits set this boolean option
+   to 'true'.


s/as empty/as an empty/

Possibly putting 'true' in quotes could be confusing.


+
  Submit variables
  
  git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 0093fa3..6c50c74 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2288,12 +2288,6 @@ class P4Sync(Command, P4UserMap):
  filesToDelete = []

  for f in files:
-# if using a client spec, only add the files that have
-# a path in the client
-if self.clientSpecDirs:
-if self.clientSpecDirs.map_in_client(f['path']) == "":
-continue
-
  filesForCommit.append(f)
  if f['action'] in self.delete_actions:
  filesToDelete.append(f)
@@ -2368,18 +2362,33 @@ class P4Sync(Command, P4UserMap):
  if self.verbose:
  print "commit into %s" % branch

-# start with reading files; if that fails, we should not
-# create a commit.
-new_files = []
-for f in files:
-if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], 
p)]:
-new_files.append (f)
-else:
-sys.stderr.write("Ignoring file outside of prefix: %s\n" % 
f['path'])
-
  if self.clientSpecDirs:
  self.clientSpecDirs.update_client_spec_path_cache(files)

+def inClientSpec(path):


This seems to be adding a new function in the middle of an existing 
function. Is that right?



+if not self.clientSpecDirs:
+return True
+inClientSpec = self.clientSpecDirs.map_in_client(path)
+if not inClientSpec and self.verbose:
+print '\n  Ignoring file outside of client spec' % path
+return inClientSpec


Any particular reason for putting a \n at the start of the message?

Also, could you use python3 style print stmnts, print("whatever") ?




+
+def hasBranchPrefix(path):
+if not self.branchPrefixes:
+return True
+hasPrefix = [p for p in self.branchPrefixes
+if p4PathStartsWith(path, p)]
+if hasPrefix and self.verbose:
+print '\n  Ignoring file outside of prefix: %s' % path
+return hasPrefix
+
+files = [f for f in files
+if inClientSpec(f['path']) and hasBranchPrefix(f['path'])]
+
+if not files and gitConfigBool('git-p4.ignoreEmptyCommits'):
+print '\n  Ignoring change %s as it would produce an empty commit.'
+return


As with Junio's comment elsewhere, I worry about deletion here.



+
  self.gitStream.write("commit %s\n" % branch)
  #gitStream.write("mark :%s\n" % details["change"])
  self.committedChanges.add(int(details["change"]))
@@ -2403,7 +2412,7 @@ class P4Sync(Command, P4UserMap):
  print "parent %s" % parent
  self.gitStream.write("from %s\n" % parent)

-self.streamP4Files(new_files)
+self.streamP4Files(files)
  self.gitStream.write("\n")

  change = int(details["change"])
diff --git a/t/t9826-git-p4-ignore-empty-commits.sh 
b/t/t9826-git-p4-ignore-empty-commits.sh
new file mode 100755
index 000..5ddccde
--- /dev/null
+++ b/t/t9826-git-p4-ignore-empty-commits.sh
@@ -0,0 +1,103 @@
+#!/bin/sh
+
+test_description='Clone repositories and ignore empty commits'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+test_expect_success 'Create a repo' '
+   client_view "//depot/... //client/..." &&
+   (
+   cd "$cli" &&
+
+   mkdir -p subdir &&
+
+   >subdir/file1.txt &&
+   p4 add subdir/file1.txt &&
+   p4 submit -d "Add file 1" &&
+
+   >file2.txt &&
+   p4 add file2.txt