* Re: [PATCH] wincred: improve compatibility with windows versions
From: Erik Faye-Lund @ 2013-01-04 21:57 UTC (permalink / raw)
To: blees; +Cc: git, msysgit, Jeff King
In-Reply-To: <50E73B80.4070105@gmail.com>
On Fri, Jan 4, 2013 at 9:28 PM, Karsten Blees <karsten.blees@gmail.com> wrote:
> On WinXP, the windows credential helper doesn't work at all (due to missing
> Cred[Un]PackAuthenticationBuffer APIs). On Win7, the credential format used
> by wincred is incompatible with native Windows tools (such as the control
> panel applet or 'cmdkey.exe /generic'). These Windows tools only set the
> TargetName, UserName and CredentialBlob members of the CREDENTIAL
> structure (where CredentialBlob is the UTF-16-encoded password).
>
> Remove the unnecessary packing / unpacking of the password, along with the
> related API definitions, for compatibility with Windows XP.
>
> Don't use CREDENTIAL_ATTRIBUTEs to identify credentials for compatibility
> with Windows credential manager tools. Parse the protocol, username, host
> and path fields from the credential's target name instead.
>
> While we're at it, optionally accept CRLF (instead of LF only) to simplify
> debugging in bash / cmd.
>
> Signed-off-by: Karsten Blees <blees@dcon.de>
> ---
>
> Hi,
>
> I tried the windows credential helper yesterday and found that it doesn't work on XP at all, and doesn't work properly in combination with Win7 credential manager tools. So here's a patch that reduces it to the functionality provided / expected by Windows.
>
> @Erik, Jeff: Please let me know if I'm missing something and the use of Cred[Un]PackAuthenticationBuffer or CREDENTIAL_ATTRIBUTEs actually served a useful purpose.
>
The only reason why I used Cred[Un]PackAuthenticationBuffer, were that
I wasn't aware that it was possible any other way. I didn't even know
there was a Windows Credential Manager in Windows XP.
The credential attributes were because they were convenient, and I'm
not sure I understand what you mean about the Win7 credential manager
tools. I did test my code with it - in fact, it was a very useful tool
for debugging the helper.
Are you referring to the credentials not *looking* like normal
HTTP-credentials? If so, I simply didn't see that as a goal. Why would
it be? Compatibility with IE? We already lose that with our "git:"
prefix in the target, no? Perhaps we can lose the "git:"-prefix, and
gain IE-compatibility when the protocol matches?
But, if we do any of these changes, does this mean I will lose my
existing credentials? It's probably not a big deal, but it's worth a
mention, isn't it?
> @@ -67,20 +61,14 @@ typedef struct _CREDENTIALW {
> #define CRED_MAX_ATTRIBUTES 64
>
> typedef BOOL (WINAPI *CredWriteWT)(PCREDENTIALW, DWORD);
> -typedef BOOL (WINAPI *CredUnPackAuthenticationBufferWT)(DWORD, PVOID, DWORD,
> - LPWSTR, DWORD *, LPWSTR, DWORD *, LPWSTR, DWORD *);
> typedef BOOL (WINAPI *CredEnumerateWT)(LPCWSTR, DWORD, DWORD *,
> PCREDENTIALW **);
> -typedef BOOL (WINAPI *CredPackAuthenticationBufferWT)(DWORD, LPWSTR, LPWSTR,
> - PBYTE, DWORD *);
> typedef VOID (WINAPI *CredFreeT)(PVOID);
> typedef BOOL (WINAPI *CredDeleteWT)(LPCWSTR, DWORD, DWORD);
>
> static HMODULE advapi, credui;
Perhaps get rid of credui also?
> static CredWriteWT CredWriteW;
> -static CredUnPackAuthenticationBufferWT CredUnPackAuthenticationBufferW;
> static CredEnumerateWT CredEnumerateW;
> -static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;
> static CredFreeT CredFree;
> static CredDeleteWT CredDeleteW;
>
> @@ -88,74 +76,64 @@ static void load_cred_funcs(void)
> {
> /* load DLLs */
> advapi = LoadLibrary("advapi32.dll");
> - credui = LoadLibrary("credui.dll");
> - if (!advapi || !credui)
> + if (!advapi)
> die("failed to load DLLs");
It's not multiple DLLs any more, so perhaps "failed to load
advapi32.dll" instead?
> -static char target_buf[1024];
> -static char *protocol, *host, *path, *username;
> -static WCHAR *wusername, *password, *target;
> +static WCHAR *wusername, *password, *protocol, *host, *path, target[1024];
>
> -static void write_item(const char *what, WCHAR *wbuf)
> +static void write_item(const char *what, LPCWSTR wbuf, int wlen)
> {
> char *buf;
> - int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL,
> + int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, wlen, NULL, 0, NULL,
> FALSE);
> buf = xmalloc(len);
>
> - if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE))
> + if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, wlen, buf, len, NULL, FALSE))
> die("WideCharToMultiByte failed!");
>
> printf("%s=", what);
> - fwrite(buf, 1, len - 1, stdout);
> + fwrite(buf, 1, len, stdout);
> putchar('\n');
> free(buf);
> }
>
When the password-blob is simply UTF-16 encoded, are the
zero-termination not included? That's the reason for this change,
right?
> -static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
> - const char *want)
> +static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
> {
> - int i;
> - if (!want)
> - return 1;
> -
> - for (i = 0; i < cred->AttributeCount; ++i)
> - if (!wcscmp(cred->Attributes[i].Keyword, keyword))
> - return !strcmp((const char *)cred->Attributes[i].Value,
> - want);
> -
> - return 0; /* not found */
> + LPCWSTR start = *ptarget;
> + LPCWSTR end = *delim ? wcsstr(start, delim) : start + wcslen(start);
> + int len = end ? end - start : wcslen(start);
> + /* update ptarget if we either found a delimiter or need a match */
> + if (end || want)
> + *ptarget = end ? end + wcslen(delim) : start + len;
> + return !want || (!wcsncmp(want, start, len) && !want[len]);
> }
>
I'm a bit tired now, but I'm having a hard time reading this code.
Right now it seems it's a bit too ternary-expression heavy for my
taste, though.
> static int match_cred(const CREDENTIALW *cred)
> {
> - return (!wusername || !wcscmp(wusername, cred->UserName)) &&
> - match_attr(cred, L"git_protocol", protocol) &&
> - match_attr(cred, L"git_host", host) &&
> - match_attr(cred, L"git_path", path);
> + LPCWSTR target = cred->TargetName;
> + if (wusername && wcscmp(wusername, cred->UserName))
> + return 0;
> +
> + return match_part(&target, L"git", L":") &&
> + match_part(&target, protocol, L"://") &&
> + match_part(&target, wusername, L"@") &&
> + match_part(&target, host, L"/") &&
> + match_part(&target, path, L"");
> }
>
Ugh, it feels a bit wrong to store and verify the username twice. Do
we really have to?
The target needs to be unique, even if two different usernames are
stored for the same site under the same conditions. So probably. It
just doesn't feel quite right.
> @@ -165,44 +143,15 @@ static void get_credential(void)
> /* search for the first credential that matches username */
> for (i = 0; i < num_creds; ++i)
> if (match_cred(creds[i])) {
> - cred = creds[i];
> + write_item("username", creds[i]->UserName,
> + wcslen(creds[i]->UserName));
> + write_item("password",
> + (LPCWSTR)creds[i]->CredentialBlob,
> + creds[i]->CredentialBlobSize / sizeof(WCHAR));
> break;
> }
> - if (!cred)
> - return;
> -
> - CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
> - cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL,
> - NULL, &pass_buf_size);
> -
> - user_buf = xmalloc(user_buf_size * sizeof(WCHAR));
> - pass_buf = xmalloc(pass_buf_size * sizeof(WCHAR));
> -
> - if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
> - cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL,
> - pass_buf, &pass_buf_size))
> - die("CredUnPackAuthenticationBuffer failed");
>
> CredFree(creds);
> -
> - /* zero-terminate (sizes include zero-termination) */
> - user_buf[user_buf_size - 1] = L'\0';
> - pass_buf[pass_buf_size - 1] = L'\0';
> -
> - write_item("username", user_buf);
> - write_item("password", pass_buf);
> -
> - free(user_buf);
> - free(pass_buf);
> -}
Nice!
> -
> -static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword,
> - const char *value)
> -{
> - attr->Keyword = (LPWSTR)keyword;
> - attr->Flags = 0;
> - attr->ValueSize = strlen(value) + 1; /* store zero-termination */
> - attr->Value = (LPBYTE)value;
> }
>
> static void store_credential(void)
> @@ -215,40 +164,18 @@ static void store_credential(void)
> if (!wusername || !password)
> return;
>
> - /* query buffer size */
> - CredPackAuthenticationBufferW(0, wusername, password,
> - NULL, &auth_buf_size);
> -
> - auth_buf = xmalloc(auth_buf_size);
> -
> - if (!CredPackAuthenticationBufferW(0, wusername, password,
> - auth_buf, &auth_buf_size))
> - die("CredPackAuthenticationBuffer failed");
> -
> cred.Flags = 0;
> cred.Type = CRED_TYPE_GENERIC;
> cred.TargetName = target;
> cred.Comment = L"saved by git-credential-wincred";
> - cred.CredentialBlobSize = auth_buf_size;
> - cred.CredentialBlob = auth_buf;
> + cred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);
> + cred.CredentialBlob = (LPVOID)password;
> cred.Persist = CRED_PERSIST_LOCAL_MACHINE;
> - cred.AttributeCount = 1;
> - cred.Attributes = attrs;
> + cred.AttributeCount = 0;
> + cred.Attributes = NULL;
> cred.TargetAlias = NULL;
> cred.UserName = wusername;
>
> - write_attr(attrs, L"git_protocol", protocol);
> -
> - if (host) {
> - write_attr(attrs + cred.AttributeCount, L"git_host", host);
> - cred.AttributeCount++;
> - }
> -
> - if (path) {
> - write_attr(attrs + cred.AttributeCount, L"git_path", path);
> - cred.AttributeCount++;
> - }
> -
> if (!CredWriteW(&cred, 0))
> die("CredWrite failed");
> }
Nice.
> @@ -284,10 +211,13 @@ static void read_credential(void)
>
> while (fgets(buf, sizeof(buf), stdin)) {
> char *v;
> + int len = strlen(buf);
> + /* strip trailing CR / LF */
> + while (len && strchr("\r\n", buf[len - 1]))
> + buf[--len] = 0;
>
> - if (!strcmp(buf, "\n"))
> + if (!*buf)
> break;
> - buf[strlen(buf)-1] = '\0';
>
> v = strchr(buf, '=');
> if (!v)
I think this part deserves a separate patch, no?
> @@ -295,13 +225,12 @@ static void read_credential(void)
> *v++ = '\0';
>
> if (!strcmp(buf, "protocol"))
> - protocol = xstrdup(v);
> + protocol = utf8_to_utf16_dup(v);
> else if (!strcmp(buf, "host"))
> - host = xstrdup(v);
> + host = utf8_to_utf16_dup(v);
> else if (!strcmp(buf, "path"))
> - path = xstrdup(v);
> + path = utf8_to_utf16_dup(v);
> else if (!strcmp(buf, "username")) {
> - username = xstrdup(v);
> wusername = utf8_to_utf16_dup(v);
> } else if (!strcmp(buf, "password"))
> password = utf8_to_utf16_dup(v);
So, you need the strings as UTF-16 instead so you can match against them...
> @@ -330,22 +259,20 @@ int main(int argc, char *argv[])
> return 0;
>
> /* prepare 'target', the unique key for the credential */
> - strncat(target_buf, "git:", sizeof(target_buf));
> - strncat(target_buf, protocol, sizeof(target_buf));
> - strncat(target_buf, "://", sizeof(target_buf));
> - if (username) {
> - strncat(target_buf, username, sizeof(target_buf));
> - strncat(target_buf, "@", sizeof(target_buf));
> + wcscpy(target, L"git:");
> + wcsncat(target, protocol, ARRAY_SIZE(target));
> + wcsncat(target, L"://", ARRAY_SIZE(target));
> + if (wusername) {
> + wcsncat(target, wusername, ARRAY_SIZE(target));
> + wcsncat(target, L"@", ARRAY_SIZE(target));
> }
> if (host)
> - strncat(target_buf, host, sizeof(target_buf));
> + wcsncat(target, host, ARRAY_SIZE(target));
> if (path) {
> - strncat(target_buf, "/", sizeof(target_buf));
> - strncat(target_buf, path, sizeof(target_buf));
> + wcsncat(target, L"/", ARRAY_SIZE(target));
> + wcsncat(target, path, ARRAY_SIZE(target));
> }
>
> - target = utf8_to_utf16_dup(target_buf);
> -
...Which means that you are composing a UTF-16 target directly rather
than ASCII. Looks sane.
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* All is proceeding as I have foreseen
From: Eric S. Raymond @ 2013-01-04 22:11 UTC (permalink / raw)
To: git
>From the #irker channel on freenode:
[14:52] TkTech esr: Oh, and I was talking to scanlime earlier since the "Ilkotech" kids messed up again. She'll be putting up a landing page on her server listing alternatives, I gave her irker, kgb, and mine.
[14:52] esr "The Ilkotech kids messed up gain"? What'd they do? And what'd they do the last time?
[14:53] TkTech They haven't paid their hosting bills so their host is down, not that they've done anything.
[14:53] TkTech I talked to them and they had about 10 minutes of interest in keeping cia.vc alive, then moved on.
CIA is dead beyond recall. I wrote the code in contrib/ciabot/ and I
think it should be removed. Sometime soon I'll ship another patch
deleting it.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
The most foolish mistake we could possibly make would be to permit
the conquered Eastern peoples to have arms. History teaches that all
conquerors who have allowed their subject races to carry arms have
prepared their own downfall by doing so.
-- Adolph Hitler, April 11 1942.
^ permalink raw reply
* Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
From: Junio C Hamano @ 2013-01-04 22:20 UTC (permalink / raw)
To: Jeff King; +Cc: git, Bart Trojanowski
In-Reply-To: <20130104124756.GA402@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I have two reservations with this patch:
>
> 1. We are ignoring SIGPIPE all the time. For an alias that is calling
> "log", that is fine. But if pack-objects dies on the server side,
> seeing that it died from SIGPIPE is useful data, and we are
> squelching that. Maybe callers of run-command should have to pass
> an "ignore SIGPIPE" flag?
What should this do:
GIT_PAGER='head -n 1' git -p -c alias.o='!cat longfile' o
Should it behave just like
cat longfile | head -n 1
or should it behave differently?
I am having a feeling that whatever external command given as the
value of alias.$cmd should choose what error status it wants to be
reported.
> 2. The die_errno in handle_alias is definitely wrong. Even if we want
> to print a message for signal death, showing errno is bogus unless
> the return value was -1. But is it the right thing to just pass the
> negative value straight to exit()? It works, but it is depending on
> the fact that (unsigned char)(ret & 0xff) behaves in a certain way
> (i.e., that we are on a twos-complement platform, and -13 becomes
> 141).
Isn't that what POSIX.1 guarantees us, though?
The value of status may be 0, EXIT_SUCCESS, EXIT_FAILURE, or any
other value, though only the least significant 8 bits (that is,
status & 0377) shall be available to a waiting parent process.
^ permalink raw reply
* Re: git.wiki.kernel.org spam ...
From: rupert THURNER @ 2013-01-04 23:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.1212311806080.32206@s15462909.onlinehome-server.info>
2012/12/31 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi Rupert,
>
> On Sat, 29 Dec 2012, rupert THURNER wrote:
>
>> ich hab gesehen, du bist ober-meister des kernle.org git wikis. da
>> gibt es ganz schön viel neue user und spam derzeit, zb:
>> https://git.wiki.kernel.org/index.php?title=User_talk:Bridgetevans0521&redirect=no
>>
>> möchtest du das erzeugen von user accounts erschweren, wie zb hier:
>> http://kiwix.org/index.php/Special:UserLogin?type=signup&returnto=Main_Page/en
>>
>> falls du noch leute als admin haben willst, ich melde mich freiwillig,
>> ThurnerRupert ist mein account.
>
> Since my trustworthiness was questioned, I have stopped completely with
> the maintenance of the Wiki.
>
> The mailing list (Cc:ed) may have additional comments.
there are 3 admins:
* https://git.wiki.kernel.org/index.php/Special:Contributions/KorgWikiSysop,
last contribution january 2010
* https://git.wiki.kernel.org/index.php/Special:Contributions/Warthog9,
last contribution august 2010
* https://git.wiki.kernel.org/index.php/Special:Contributions/Dscho,
last contribution august 2010
and you were (by far) the most active. this leaves me a little
confused. who would be then be responsible? who would be responsible
for upgrading / installing anything at the wiki?
rupert.
^ permalink raw reply
* Re: [PATCH v4] git-completion.bash: add support for path completion
From: Junio C Hamano @ 2013-01-04 23:20 UTC (permalink / raw)
To: Marc Khouzam
Cc: 'Manlio Perillo', 'git@vger.kernel.org',
'szeder@ira.uka.de', 'felipe.contreras@gmail.com'
In-Reply-To: <E59706EF8DB1D147B15BECA3322E4BDC0672D1@eusaamb103.ericsson.se>
Marc Khouzam <marc.khouzam@ericsson.com> writes:
> I've been playing with it but I'm not getting the expected
> behavior when I cd to a sub-directory.
Thanks for testing. Manlio?
^ permalink raw reply
* Re: git.wiki.kernel.org spam ...
From: Johannes Schindelin @ 2013-01-04 23:27 UTC (permalink / raw)
To: rupert THURNER; +Cc: git
In-Reply-To: <CAJs9aZ_eL1jR=GqxUEy3vEWbMz6kEYOHb7pZkZWFh6yXXSx-Jg@mail.gmail.com>
Hi Rupert,
On Sat, 5 Jan 2013, rupert THURNER wrote:
> 2012/12/31 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> there are 3 admins:
> * https://git.wiki.kernel.org/index.php/Special:Contributions/KorgWikiSysop,
> last contribution january 2010
> * https://git.wiki.kernel.org/index.php/Special:Contributions/Warthog9,
> last contribution august 2010
> * https://git.wiki.kernel.org/index.php/Special:Contributions/Dscho,
> last contribution august 2010
>
> and you were (by far) the most active.
I was. John Hawley trusted me when I asked for admin privileges to keep
the spam at bay, but a very vocal voice on the mailing list tried to
discredit my work, and in the wake of the ensuing mailing list thread I
got the impression that that feeling was universal, so I abided and
stopped.
> this leaves me a little confused. who would be then be responsible? who
> would be responsible for upgrading / installing anything at the wiki?
That would be John Hawley.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] gitk: Display the date of a tag in a human friendly way.
From: Anand Kumria @ 2013-01-04 15:47 UTC (permalink / raw)
To: git; +Cc: Anand Kumria
By selecting a tag within gitk you can display information about it.
This information is output by using the command
'git cat-file tag <tagid>'
This outputs the *raw* information from the tag, amongst which is the
time - in seconds since the epoch. As useful as that value is, I find it
a lot easier to read and process time which it is something like:
"Mon Dec 31 14:26:11 2012 -0800"
This change will modify the display of tags in gitk like so:
@@ -1,7 +1,7 @@
object 5d417842efeafb6e109db7574196901c4e95d273
type commit
tag v1.8.1
-tagger Junio C Hamano <gitster@pobox.com> 1356992771 -0800
+tagger Junio C Hamano <gitster@pobox.com> Mon Dec 31 14:26:11 2012 -0800
Git 1.8.1
-----BEGIN PGP SIGNATURE-----
Signed-off-by: Anand Kumria <wildfire@progsoc.org>
---
gitk-git/gitk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..aae1c58 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -10675,7 +10675,7 @@ proc showtag {tag isnew} {
set linknum 0
if {![info exists cached_tagcontent($tag)]} {
catch {
- set cached_tagcontent($tag) [exec git cat-file tag $tag]
+ set cached_tagcontent($tag) [exec git cat-file -p $tag]
}
}
if {[info exists cached_tagcontent($tag)]} {
--
1.7.9.5
^ permalink raw reply related
* Re: git.wiki.kernel.org spam ...
From: Theodore Ts'o @ 2013-01-04 23:47 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: rupert THURNER, git
In-Reply-To: <alpine.DEB.1.00.1301050022000.32206@s15462909.onlinehome-server.info>
On Sat, Jan 05, 2013 at 12:27:12AM +0100, Johannes Schindelin wrote:
>
> I was. John Hawley trusted me when I asked for admin privileges to keep
> the spam at bay, but a very vocal voice on the mailing list tried to
> discredit my work, and in the wake of the ensuing mailing list thread I
> got the impression that that feeling was universal, so I abided and
> stopped.
>
> > this leaves me a little confused. who would be then be responsible? who
> > would be responsible for upgrading / installing anything at the wiki?
>
> That would be John Hawley.
John is one of the Linux Foundation staff members that are responsible
for the system administration of wiki.kernel.org (and kernel.org, and
bugzilla.kernel.org, etc.) They are *not* responsible for the
contents of the *.wiki.kernel.org; someone from the project has to be
the wiki maintainer.
(Note: the *.wiki.kernel.org infrastructure was originally set up at
my request, and the first such hosted wiki was ext4.wiki.kernel.org;
the second was rt.wiki.kernel.org, for which I was also the primary
wiki administrator initially. I'm confident the policy on this hasn't
changed since those early days because LF sysadmins (e.g., John and
Konstantin) do *not* have time to police the various wikis for
spam....)
- Ted
^ permalink raw reply
* Re: [PATCH] gitk: Display the date of a tag in a human friendly way.
From: Junio C Hamano @ 2013-01-04 23:50 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Anand Kumria
In-Reply-To: <1357314431-32710-1-git-send-email-wildfire@progsoc.org>
Anand Kumria <wildfire@progsoc.org> writes:
> By selecting a tag within gitk you can display information about it.
> This information is output by using the command
>
> 'git cat-file tag <tagid>'
>
> This outputs the *raw* information from the tag, amongst which is the
> time - in seconds since the epoch. As useful as that value is, I find it
> a lot easier to read and process time which it is something like:
>
> "Mon Dec 31 14:26:11 2012 -0800"
>
> This change will modify the display of tags in gitk like so:
>
> @@ -1,7 +1,7 @@
> object 5d417842efeafb6e109db7574196901c4e95d273
> type commit
> tag v1.8.1
> -tagger Junio C Hamano <gitster@pobox.com> 1356992771 -0800
> +tagger Junio C Hamano <gitster@pobox.com> Mon Dec 31 14:26:11 2012 -0800
>
> Git 1.8.1
> -----BEGIN PGP SIGNATURE-----
>
> Signed-off-by: Anand Kumria <wildfire@progsoc.org>
> ---
Sounds like a sensible thing to do but I didn't check how else
(other than purely for displaying) this string is used.
Paul, the patch is not made against your tree, so if you choose to
take it you would need to strip the leading directory at the top.
Thanks.
PS. I haven't received a pull request from you for a while; are
there accumulated changes I should be pulling in before -rc0 of the
next release we are working on?
> gitk-git/gitk | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index d93bd99..aae1c58 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -10675,7 +10675,7 @@ proc showtag {tag isnew} {
> set linknum 0
> if {![info exists cached_tagcontent($tag)]} {
> catch {
> - set cached_tagcontent($tag) [exec git cat-file tag $tag]
> + set cached_tagcontent($tag) [exec git cat-file -p $tag]
> }
> }
> if {[info exists cached_tagcontent($tag)]} {
^ permalink raw reply
* [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: David Michael @ 2013-01-05 0:35 UTC (permalink / raw)
To: git@vger.kernel.org, Junio C Hamano
It is possible for this pointer of the GIT_DIR environment variable to
survive unduplicated until further getenv calls are made. The standards
allow for subsequent calls of getenv to overwrite the string located at
its returned pointer, and this can result in broken git operations on
certain platforms.
Signed-off-by: David Michael <fedora.dm0@gmail.com>
---
I have encountered an issue with consecutive calls to getenv
overwriting earlier values. Most notably, it prevents a plain "git
clone" from working.
Long story short: This value of GIT_DIR gets passed around setup.c
until it reaches check_repository_format_gently. This function calls
git_config_early, which eventually runs getenv("HOME"). When it
returns back to check_repository_format_gently, the gitdir variable
contains my home directory path. The end result is that I wind up
with ~/objects/ etc. and a failed repository clone. (Simply adding a
bare getenv("GIT_DIR") afterwards to reset the pointer also corrects
the problem.)
Since other platforms are apparently working, yet this getenv behavior
is supported by the standards, I am left wondering if this could be a
symptom of something else being broken on my platform (z/OS). Can
anyone more familiar with this part of git identify any condition that
obviously should not be occurring?
Thanks.
setup.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index f108c4b..64fb160 100644
--- a/setup.c
+++ b/setup.c
@@ -675,8 +675,12 @@ static const char
*setup_git_directory_gently_1(int *nongit_ok)
* validation.
*/
gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
- if (gitdirenv)
- return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+ if (gitdirenv) {
+ gitdirenv = xstrdup(gitdirenv);
+ ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+ free(gitdirenv);
+ return ret;
+ }
if (env_ceiling_dirs) {
string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
--
1.7.11.7
^ permalink raw reply related
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Junio C Hamano @ 2013-01-05 1:17 UTC (permalink / raw)
To: David Michael; +Cc: git@vger.kernel.org
In-Reply-To: <CAEvUa7niTJVfp8_kuWs50kvhfZ59F-yAuAmeOXEduHXOq-tRFA@mail.gmail.com>
David Michael <fedora.dm0@gmail.com> writes:
> I have encountered an issue with consecutive calls to getenv
> overwriting earlier values. Most notably, it prevents a plain "git
> clone" from working.
>
> Long story short: This value of GIT_DIR gets passed around setup.c
> until it reaches check_repository_format_gently. This function calls
> git_config_early, which eventually runs getenv("HOME"). When it
> returns back to check_repository_format_gently, the gitdir variable
> contains my home directory path. The end result is that I wind up
> with ~/objects/ etc. and a failed repository clone. (Simply adding a
> bare getenv("GIT_DIR") afterwards to reset the pointer also corrects
> the problem.)
>
> Since other platforms are apparently working, yet this getenv behavior
> is supported by the standards, I am left wondering if this could be a
> symptom of something else being broken on my platform (z/OS).
The execve(2) function
int execve(const char *filename, char *const argv[],
char *const envp[]);
takes a NULL terminated array of NUL terminated strings of form
"VAR=VAL" in envp[], and this is kept in:
extern char **environ;
of the new image that runs.
The most naive and straight-forward way to implement getenv(3) is to
iterate over this environ[] array to look for an element that begins
with "GIT_DIR=", and return the pointer pointing at the location one
byte past that equal sign. So even if the standard allowed the
returned value to be volatile across calls to getenv(3), it will
take *more* work for implementations if they want to break our use
pattern. They have to deliberately return a string that they will
overwrite in subsequent calls to getenv(3).
Also the natural way to implement putenv(3) and setenv(3) is to
replace the pointer in the environ[] array (not overwrite the
existing string in environ[] that holds the "VAR=VAL" string that
represents the current value, which might be shorter than the new
value of the enviornment variable), hence even calling these
functions is unlikely to invalidate the result you previously
received from getenv(3).
I am not at all surprised that nobody from other platforms has seen
this breakage.
In fact,
http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html
says that only setenv(), unsetenv() and putenv() may invalidate
previous return values. Note that getenv() is not listed as a
function that is allowed to break return values from a previous call
to getenv().
So in short, your platform's getenv(3) emulation may be broken. We
have other calls to getenv() where we rely on the result of it being
stable, and you might discover these places also break.
Having said that, we do have codepaths to update a handful of
environment variables ourselves (GIT_DIR is among them), so I think
your patch is a good safety measure in general.
> setup.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index f108c4b..64fb160 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -675,8 +675,12 @@ static const char
> *setup_git_directory_gently_1(int *nongit_ok)
> * validation.
> */
> gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
> - if (gitdirenv)
> - return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
> + if (gitdirenv) {
> + gitdirenv = xstrdup(gitdirenv);
> + ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
> + free(gitdirenv);
> + return ret;
> + }
>
> if (env_ceiling_dirs) {
> string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
> --
> 1.7.11.7
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: David Michael @ 2013-01-05 2:15 UTC (permalink / raw)
To: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <7vsj6gqvhc.fsf@alter.siamese.dyndns.org>
Hi,
On Fri, Jan 4, 2013 at 8:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
> In fact,
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html
>
> says that only setenv(), unsetenv() and putenv() may invalidate
> previous return values. Note that getenv() is not listed as a
> function that is allowed to break return values from a previous call
> to getenv().
Before I sent the e-mail, I checked that very page to be sure I wasn't
entirely insane. Specifically, the second paragraph begins with:
> The string pointed to may be overwritten by a subsequent call to getenv(), [...]
I read that line as confirmation that this is indeed acceptably
standard behavior. Even the getenv man page on my Fedora workstation
says:
> The string pointed to by the return value of getenv() may be statically allocated, and can be modified by a subsequent call to getenv(), putenv(3), setenv(3), or unsetenv(3).
Am I misinterpreting these statements?
Thanks.
David
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Duy Nguyen @ 2013-01-05 2:45 UTC (permalink / raw)
To: David Michael; +Cc: git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CAEvUa7niTJVfp8_kuWs50kvhfZ59F-yAuAmeOXEduHXOq-tRFA@mail.gmail.com>
On Sat, Jan 5, 2013 at 7:35 AM, David Michael <fedora.dm0@gmail.com> wrote:
> - if (gitdirenv)
> - return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
> + if (gitdirenv) {
> + gitdirenv = xstrdup(gitdirenv);
> + ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
> + free(gitdirenv);
> + return ret;
> + }
Maybe we could all this into a wrapper? If getenv() here has a
problem, many other places may have the same problem too. This
simplifies the change. But one has to check that getenv() must not be
used in threaded code.
char *git_getenv(const char *env)
{
static int bufno;
static char *buf[4];
bufno = (bufno + 1) % 4;
free(buf[bufno]);
buf[bufno] = xstrdup(getenv(env));
return buf[bufno];
}
#define getenv(x) git_getenv(x)
--
Duy
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Junio C Hamano @ 2013-01-05 4:32 UTC (permalink / raw)
To: David Michael; +Cc: git@vger.kernel.org
In-Reply-To: <7vsj6gqvhc.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> ... So even if the standard allowed the
> returned value to be volatile across calls to getenv(3),...
> ...
> In fact,
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html
>
> says that only ...
Apparently I wasn't even reading what I was quoting carefully
enough. The above does include getenv() as one of the functions
that are allowed to invalidate earlier return values.
Sorry about that. I'll go back to bed (I am a bit under the weather
and OOO today). The conclusion in my original message is still
valid.
> Having said that, we do have codepaths to update a handful of
> environment variables ourselves (GIT_DIR is among them), so I think
> your patch is a good safety measure in general.
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Junio C Hamano @ 2013-01-05 4:38 UTC (permalink / raw)
To: Duy Nguyen; +Cc: David Michael, git@vger.kernel.org
In-Reply-To: <CACsJy8BeuV8esGTWsQiT_G9pZE28s5KJxH6+dzdhioLgmSiNVg@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> Maybe we could all this into a wrapper? If getenv() here has a
> problem, many other places may have the same problem too. This
> simplifies the change. But one has to check that getenv() must not be
> used in threaded code.
That needs to be done regardless, if we care; POSIX explicitly says
getenv() need not be thread-safe.
I personally do not think a wrapper with limited slots is a healthy
direction to go. Most places we use getenv() do not let the return
value live across their scope, and those that do should explicitly
copy the value away. It's between validating that there is _no_ *env()
calls in the codepath between a getenv() call and the use of its
return value, and validating that there is at most 4 such calls there.
The former is much easier to verify and maintain, I think.
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Duy Nguyen @ 2013-01-05 6:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Michael, git@vger.kernel.org
In-Reply-To: <7vk3rsqm6u.fsf@alter.siamese.dyndns.org>
On Sat, Jan 5, 2013 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I personally do not think a wrapper with limited slots is a healthy
> direction to go. Most places we use getenv() do not let the return
> value live across their scope, and those that do should explicitly
> copy the value away. It's between validating that there is _no_ *env()
> calls in the codepath between a getenv() call and the use of its
> return value, and validating that there is at most 4 such calls there.
> The former is much easier to verify and maintain, I think.
I did not look carefully and was scared of 143 getenv calls. But with
about 4 calls, yes it's best to do without the wrapper.
--
Duy
^ permalink raw reply
* Re: [PATCH v4] git-completion.bash: add support for path completion
From: Junio C Hamano @ 2013-01-05 6:27 UTC (permalink / raw)
To: Marc Khouzam, Manlio Perillo; +Cc: git, szeder, felipe.contreras
In-Reply-To: <7vobh4sffw.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Marc Khouzam <marc.khouzam@ericsson.com> writes:
>
>> I've been playing with it but I'm not getting the expected
>> behavior when I cd to a sub-directory.
>
> Thanks for testing. Manlio?
Can you try the attached patch?
As I am not familiar with the completion machinery, take this with a
large grain of salt. Here is my explanation of what is going on in
this "how about this" fixup:
* Giving --git-dir from the command line (or GIT_DIR environment)
without specifying GIT_WORK_TREE is to signal Git that you are at
the top of the working tree. "git ls-files" will then show the
full tree even outside the real $cwd because you are lying to
Git.
* "git diff-index" could be told to show only the $cwd and its
subdirectory with the "--relative" option, but it alone is not
sufficient if you throw --git-dir at it; again, you end up lying
that you are at the top.
* As far as I can tell, there is no reason you would want to pass
"--git-dir" to these invocations of ls-files and diff-index. If
the previous call to "__gitdir" could figure out where it is, the
command should be able to figure it out the same way.
There seem to be millions of other existing "git --git-dir=$there"
in this script. As I already said, I am not familiar with the
completion machinery, and I do not know what they are for in the
first place. Perhaps people put them there for a reason, but I do
not know what that reason is.
I think the ones for "for-each-ref", "config" and "stash" should be
harmless. They are commands that do not care about the working
tree.
There is one given to "ls-tree" used in __git_complete_revlist_file;
I do not know if this was intended, what it is for, and if that is
working as intended, though.
I've been CC'ing two people who touched this script rather heavily,
are expected to know the completion machinery, and should be able to
help polishing this topic and moving it forward. Perhaps one of
them can shed light on this.
-- >8 --
Subject: completion: do not pass harmful "--git-dir=$there"
The recently added __git_index_files and __git_diff_index_files
helper functions invoke "ls-files" and "diff_index" while explicitly
passing "--git-dir=$there", to tell them that the invocation is done
at the top of the working tree, which may not be the case when the
user is in a subdirectory. Remove the harmful use of this option,
Tell "diff-index" to show only the paths in the $cwd and show them
relative to the $cwd by passing "--relative". The "ls-files" does
not need this, as that is already its default mode of operation.
---
contrib/completion/git-completion.bash | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c8c6464..f4bd548 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -267,9 +267,9 @@ __git_index_files ()
if [ -d "$dir" ]; then
# NOTE: $1 is not quoted in order to support multiple options
- git --git-dir="$dir" ls-files --exclude-standard $1 ${2+"$2"} 2>/dev/null |
- __git_index_file_list_filter ${2+"$2"} |
- uniq
+ git ls-files --exclude-standard $1 ${2+"$2"} 2>/dev/null |
+ __git_index_file_list_filter ${2+"$2"} |
+ uniq
fi
}
@@ -284,9 +284,9 @@ __git_diff_index_files ()
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
- git --git-dir="$dir" diff-index --name-only "$1" 2>/dev/null |
- __git_index_file_list_filter ${2+"$2"} |
- uniq
+ git diff-index --name-only --relative "$1" 2>/dev/null |
+ __git_index_file_list_filter ${2+"$2"} |
+ uniq
fi
}
^ permalink raw reply related
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Junio C Hamano @ 2013-01-05 6:47 UTC (permalink / raw)
To: Duy Nguyen; +Cc: David Michael, git@vger.kernel.org
In-Reply-To: <CACsJy8CZe=qyzmG_1vdLYp07OvkDAU4wYc8MN3et7WBVmMhJOQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Sat, Jan 5, 2013 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> I personally do not think a wrapper with limited slots is a healthy
>> direction to go. Most places we use getenv() do not let the return
>> value live across their scope, and those that do should explicitly
>> copy the value away. It's between validating that there is _no_ *env()
>> calls in the codepath between a getenv() call and the use of its
>> return value, and validating that there is at most 4 such calls there.
>> The former is much easier to verify and maintain, I think.
>
> I did not look carefully and was scared of 143 getenv calls. But with
> about 4 calls, yes it's best to do without the wrapper.
Just to make sure you did not misunderstand, the 4 (four) in my
message is not about "4 calls among 143 are unsafe".
It was referring to the number of rotating slots your patch defined,
which means
val = getenv("FOO");
... random other code ...
use(val)
is safe only if random other code makes less than 4 getenv() calls.
I didn't verify all of the call sites. It needs to be done with or
without your wrapper patch. Without your wrapper, the validation
needs to make sure "random other code" above does not make any getenv()
call. With your wrapper, the validation needs to make sure "random
other code" above does not make more than 4 such calls. My point
was that the effort needed for both are about the same, so your
wrapper does not buy us much.
^ permalink raw reply
* [PATCH 00/10] push: switch default from "matching" to "simple"
From: Junio C Hamano @ 2013-01-05 6:52 UTC (permalink / raw)
To: git
We promised to change the behaviour of lazy "git push [there]" that
does not say what to push on the command line from "matching" to
"simple" in Git 2.0. Even though the top-level RelNotes symbolic
link points at 1.8.2, we can change our mind to tag 2.0 at the end
of this cycle.
This carries out the threat ;-)
Mmany tests have assumed that the long established default will
never change and relied on the convenience of the "matching"
behaviour. They need to be updated to either specify what they want
from the command line explicitly, or configure the default behaviour
they would expect. And then finally we can flip the default.
Junio C Hamano (10):
t5404: do not assume the "matching" push is the default
t5505: do not assume the "matching" push is the default
t5516: do not assume the "matching" push is the default
t5517: do not assume the "matching" push is the default
t5519: do not assume the "matching" push is the default
t5531: do not assume the "matching" push is the default
t7406: do not assume the "matching" push is the default
t9400: do not assume the "matching" push is the default
t9401: do not assume the "matching" push is the default
push: switch default from "matching" to "simple"
Documentation/config.txt | 6 +++---
builtin/push.c | 31 +++++++------------------------
t/t5404-tracking-branches.sh | 2 +-
t/t5505-remote.sh | 2 +-
t/t5516-fetch-push.sh | 10 +++++-----
t/t5517-push-mirror.sh | 2 +-
t/t5519-push-alternates.sh | 12 ++++++------
t/t5531-deep-submodule-push.sh | 1 +
t/t7406-submodule-update.sh | 4 ++--
t/t9400-git-cvsserver-server.sh | 1 +
t/t9401-git-cvsserver-crlf.sh | 1 +
11 files changed, 29 insertions(+), 43 deletions(-)
--
1.8.1.299.gc73b41f
^ permalink raw reply
* [PATCH 01/10] t5404: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:52 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5404-tracking-branches.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh
index c240035..2b8c0ba 100755
--- a/t/t5404-tracking-branches.sh
+++ b/t/t5404-tracking-branches.sh
@@ -36,7 +36,7 @@ test_expect_success 'prepare pushable branches' '
'
test_expect_success 'mixed-success push returns error' '
- test_must_fail git push
+ test_must_fail git push origin :
'
test_expect_success 'check tracking branches updated correctly after push' '
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 02/10] t5505: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5505-remote.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index ccc55eb..6579a86 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -345,7 +345,7 @@ test_expect_success 'fetch mirrors do not act as mirrors during push' '
) &&
(cd mirror-fetch/child &&
git branch -m renamed renamed2 &&
- git push parent
+ git push parent :
) &&
(cd mirror-fetch/parent &&
git rev-parse --verify renamed &&
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 04/10] t5517: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5517-push-mirror.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
index e2ad260..12a5dfb 100755
--- a/t/t5517-push-mirror.sh
+++ b/t/t5517-push-mirror.sh
@@ -256,7 +256,7 @@ test_expect_success 'remote.foo.mirror=no has no effect' '
git branch keep master &&
git push --mirror up &&
git branch -D keep &&
- git push up
+ git push up :
) &&
(
cd mirror &&
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 03/10] t5516: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5516-fetch-push.sh | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..1a8753d 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -247,7 +247,7 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
test_expect_success 'push with matching heads' '
mk_test heads/master &&
- git push testrepo &&
+ git push testrepo : &&
check_push_result $the_commit heads/master
'
@@ -276,7 +276,7 @@ test_expect_success 'push --force with matching heads' '
mk_test heads/master &&
git push testrepo : &&
git commit --amend -massaged &&
- git push --force testrepo &&
+ git push --force testrepo : &&
! check_push_result $the_commit heads/master &&
git reset --hard $the_commit
@@ -507,7 +507,7 @@ test_expect_success 'push with config remote.*.pushurl' '
git checkout master &&
git config remote.there.url test2repo &&
git config remote.there.pushurl testrepo &&
- git push there &&
+ git push there : &&
check_push_result $the_commit heads/master
'
@@ -521,7 +521,7 @@ test_expect_success 'push with dry-run' '
cd testrepo &&
old_commit=$(git show-ref -s --verify refs/heads/master)
) &&
- git push --dry-run testrepo &&
+ git push --dry-run testrepo : &&
check_push_result $old_commit heads/master
'
@@ -981,7 +981,7 @@ test_expect_success 'push --porcelain --dry-run rejected' '
test_expect_success 'push --prune' '
mk_test heads/master heads/second heads/foo heads/bar &&
- git push --prune testrepo &&
+ git push --prune testrepo : &&
check_push_result $the_commit heads/master &&
check_push_result $the_first_commit heads/second &&
! check_push_result $the_first_commit heads/foo heads/bar
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 05/10] t5519: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5519-push-alternates.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh
index c00c9b0..11fcd37 100755
--- a/t/t5519-push-alternates.sh
+++ b/t/t5519-push-alternates.sh
@@ -40,7 +40,7 @@ test_expect_success 'alice works and pushes' '
cd alice-work &&
echo more >file &&
git commit -a -m second &&
- git push ../alice-pub
+ git push ../alice-pub :
)
'
@@ -57,7 +57,7 @@ test_expect_success 'bob fetches from alice, works and pushes' '
git pull ../alice-pub master &&
echo more bob >file &&
git commit -a -m third &&
- git push ../bob-pub
+ git push ../bob-pub :
) &&
# Check that the second commit by Alice is not sent
@@ -86,7 +86,7 @@ test_expect_success 'alice works and pushes again' '
cd alice-work &&
echo more alice >file &&
git commit -a -m fourth &&
- git push ../alice-pub
+ git push ../alice-pub :
)
'
@@ -99,7 +99,7 @@ test_expect_success 'bob works and pushes' '
cd bob-work &&
echo yet more bob >file &&
git commit -a -m fifth &&
- git push ../bob-pub
+ git push ../bob-pub :
)
'
@@ -115,7 +115,7 @@ test_expect_success 'alice works and pushes yet again' '
git commit -a -m sixth.2 &&
echo more and more alice >>file &&
git commit -a -m sixth.3 &&
- git push ../alice-pub
+ git push ../alice-pub :
)
'
@@ -136,7 +136,7 @@ test_expect_success 'bob works and pushes again' '
git hash-object -t commit -w commit &&
echo even more bob >file &&
git commit -a -m seventh &&
- git push ../bob-pub
+ git push ../bob-pub :
)
'
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 06/10] t5531: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5531-deep-submodule-push.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 1947c28..8c16e04 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -16,6 +16,7 @@ test_expect_success setup '
(
cd gar/bage &&
git init &&
+ git config push.default matching &&
>junk &&
git add junk &&
git commit -m "Initial junk"
--
1.8.1.299.gc73b41f
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox