Re: [PATCH 04/19] reset: don't allow git reset -- $pathspec in bare repo
On Wed, Jan 9, 2013 at 11:32 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: --- builtin/reset.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) With the patch that does not have any explicit check for bareness nor new error message to scold user with, it is rather hard to tell what is going on, without any description on what (if anything) is broken at the end user level and what remedy is done about that breakage... Will include the following in a re-roll. reset: don't allow git reset -- $pathspec in bare repo Running e.g. git reset . in a bare repo results in an index file being created from the HEAD commit. The differences compared to the index are then printed as usual, but since there is no worktree, it will appear as if all files are deleted. For example, in a bare clone of git.git: Unstaged changes after reset: D .gitattributes D .gitignore D .mailmap ... This happens because the check for is_bare_repository() happens after we branch off into read_from_tree() to reset with paths. Fix by moving the branching point after the check. -- 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 08/19] reset.c: share call to die_if_unmerged_cache()
On Wed, Jan 9, 2013 at 11:48 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: Use a single condition to guard the call to die_if_unmerged_cache for both --soft and --keep. This avoids the small distraction of the precondition check from the logic following it. Also change an instance of if (e) err = err || f(); to the almost as short, but clearer if (e !err) err = f(); (which is equivalent since we only care whether exit code is 0) It is not just equivalent, but should give us identical result, even if we cared the actual value. If err is initially 0, and f() evaluates to 2, err would be 1 in the first case, but 2 in the second case, right? I think the two might be identical in e.g. JavaScript and Python, but I don't use either much. -- 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 v4] git-clean: Display more accurate delete messages
I think the code before this patch used to say Would not remove and Not removing in certain cases to report the paths that the command decided not to remove, but after this patch these two messages no longer appear in the patch. Is it expected, are we losing information, or...? I do not think we are losing any information. Say, we have a repo like this: test.git |-- untracked_file |-- untracked_bar | |-- bar.txt |-- untracked_foo |-- foo.txt The original version prints out: $ git clean -fn Would remove untracked_file Would not remove untracked_bar/ Would not remove untracked_foo/ We never asked for any directories to be removed so IMHO the Would not remove ... messages are just noise. The new version prints out: $ git clean -fn Would remove untracked_file -- 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/PATCH] avoid SIGPIPE warnings for aliases
On Wed, Jan 09, 2013 at 01:49:41PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: But we still say error: ... died of signal 13, because that comes from inside wait_or_whine. So it is a separate issue whether or not wait_or_whine should be silent on SIGPIPE (we already are on SIGINT and SIGQUIT, as of some recent patches). The upside is that it is noise in this case that we would no longer see. The downside is that we may be losing a clue when debugging server problems, which do not expect to die from SIGPIPE. Should it be an optional run-command flag? Do we know if we are upstream of a pager that reads from us through a pipe (I think we should, especially in a case where we are the one who processed the git -p $alias option)? Is there any other case where we would want to ignore child's death by SIGPIPE? If the answers are yes and no, then perhaps we can ask pager_in_use() to decide this? The answer to the first is unfortunately no. Consider an alias like [alias]foo = !git log (which yes, you could implement as just log, but there are cases where you want to manipulate the environment and we do not allow it). Your process tree for running git foo looks like: git foo (A) git log (B) less (C) The user hits 'q', which kills process C. Process B then dies due to SIGPIPE, and process A sees that the alias command died due to a signal. But process A has no clue that a pager is in effect; only process B, which spawned the pager, can know that. So A cannot see death-by-SIGPIPE and make a decision on whether a pager was in use. If anything, it is process B's responsibility to say Oops, I was killed by SIGPIPE. But that's OK, it's not a big deal to me. Which it could do by installing a SIGPIPE handler that just calls exit(0). -Peff -- 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 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
On Thu, Jan 10, 2013 at 2:26 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: We use the path arguments in two places in reset.c: in interactive_reset() and read_from_tree(). Both of these call get_pathspec(), so we pass the (prefix, arv) pair to both functions. Move the call to get_pathspec() out of these methods, for two reasons: 1) One argument is simpler than two. 2) It lets us use the (arguably clearer) if (pathspec) in place of if (i argc). --- If I understand correctly, this should be rebased on top of nd/parse-pathspec. Please let me know. Yeah, this will conflict with the get_pathspec-to-parse_pathspec conversion Duy has been working on. Or I could hold off nd/parse-pathspec if this series has a better chance of graduation first. Decision? -- Duy -- 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
about vim contrib support
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi. In the contrib/vim/README file there are instructions about how to setup git support with Vim builtin git syntax files. However these instructions seems to be redundant, since the system filetype.vim file already have the autocmd rules. The only issue I found is with: autocmd BufNewFile,BufRead .msg.[0-9]* \ if getline(1) =~ '^From.*# This line is ignored.$' | \ setf gitsendemail | \ endif It should be: autocmd BufNewFile,BufRead [0-9]*.patch IMHO it should contain some other checks, to make sure it is a patch generated by git format-patch, and not, as an example, a plain patch or a Mercurial patch. By the way: I don't understand the purpose of gitsendemail syntax. On my system it does not highlight the diff. I have implemented an alternate gitpatch syntax file, attached. What I would like to get, is to syntax highligth the commit subject message, but I'm not a Vim expert. Regards Manlio -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlDuo0sACgkQscQJ24LbaUTZMQCgm7QRylhxc5v4i4tHBfUXCl8o 36IAn3t72o/+5R/x1TF7r9mu85z6wY25 =b2l0 -END PGP SIGNATURE- Vim syntax file Language: git format-patch message Maintainer: Manlio Perillo Filenames:[0-9]*.patch (first line is From ... # This line is ignored.) Last Change: 2014 Gen 10 if exists(b:current_syntax) finish endif syn case match syn match gitsendemailComment \%^From.*#.* syn match gitsendemailComment ^GIT:.* if has(spell) syn spell toplevel endif syn include @gitcommitMessage syntax/gitcommit.vim syn region gitcommitMessage start=/^Subject: \@=/ end=/^$|^#\@=/ contains=@gitcommitMessage hi def link gitsendemailComment Comment let b:current_syntax = gitpatch
Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
On Wed, Jan 09, 2013 at 04:18:44PM -0800, Jonathan Nieder wrote: Do we know if we are upstream of a pager that reads from us through a pipe (I think we should, especially in a case where we are the one who processed the git -p $alias option)? Is there any other case where we would want to ignore child's death by SIGPIPE? When we die early by SIGPIPE because output was piped to head, I still think the early end of output is not notable enough to complain about. I'm not sure whether there are SIGPIPE instances we really don't want to be silent about, though. I suspect not. ;-) Some of our plumbing writes over pipes (e.g., pack-objects writing back to send-pack, which is multiplexing over the network, or receive-pack writing to index-pack). We _should_ be checking the value of every write(), and your final close(), and making sure that sub-processes reports success. But leaving SIGPIPE on is an extra safety measure; in theory it can catch an unchecked write. When one of those programs goes wrong, the message can be an extra debugging aid. If the process died unexpectedly with no message (since it died by signal), seeing pack-objects died by signal 13 is much better than not seeing anything at all. Usually it is accompanied by other messages (like remote end hung up unexpectedly or similar), but the extra output has helped me track down server-side issues in the past. Compare http://thread.gmane.org/gmane.comp.version-control.git/2062, http://thread.gmane.org/gmane.comp.version-control.git/48469/focus=48665. The argument there seems to be that bash is stupid for complaining about SIGPIPE. And I would agree for the alias case, where we are running commands from the command-line in a very shell-like manner. But wait_or_whine is also used for connecting internal bits together. Maybe the right rule is if we are using the shell to execute, do not mention SIGPIPE? It seems a little iffy at first, but: 1. It tends to coincide with direct use of internal tools versus external tools. 2. We do not reliably get SIGPIPE there, anyway, since most shells will convert it into exit code 141 before we see it. I.e., something like: diff --git a/run-command.c b/run-command.c index 24eaad5..8bd0b08 100644 --- a/run-command.c +++ b/run-command.c @@ -226,7 +226,7 @@ static inline void set_cloexec(int fd) fcntl(fd, F_SETFD, flags | FD_CLOEXEC); } -static int wait_or_whine(pid_t pid, const char *argv0) +static int wait_or_whine(pid_t pid, const char *argv0, int ignore_sigpipe) { int status, code = -1; pid_t waiting; @@ -242,7 +242,8 @@ static int wait_or_whine(pid_t pid, const char *argv0) error(waitpid is confused (%s), argv0); } else if (WIFSIGNALED(status)) { code = WTERMSIG(status); - if (code != SIGINT code != SIGQUIT) + if (code != SIGINT code != SIGQUIT + (!ignore_sigpipe || code != SIGPIPE)) error(%s died of signal %d, argv0, code); /* * This return value is chosen so that code 0xff @@ -433,7 +434,7 @@ fail_pipe: * At this point we know that fork() succeeded, but execvp() * failed. Errors have been reported to our stderr. */ - wait_or_whine(cmd-pid, cmd-argv[0]); + wait_or_whine(cmd-pid, cmd-argv[0], 0); failed_errno = errno; cmd-pid = -1; } @@ -538,7 +539,7 @@ int finish_command(struct child_process *cmd) int finish_command(struct child_process *cmd) { - return wait_or_whine(cmd-pid, cmd-argv[0]); + return wait_or_whine(cmd-pid, cmd-argv[0], cmd-use_shell); } int run_command(struct child_process *cmd) @@ -725,7 +726,7 @@ int finish_async(struct async *async) int finish_async(struct async *async) { #ifdef NO_PTHREADS - return wait_or_whine(async-pid, child process); + return wait_or_whine(async-pid, child process, 0); #else void *ret = (void *)(intptr_t)(-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: about vim contrib support
On Thu, Jan 10, 2013 at 12:17:31PM +0100, Manlio Perillo wrote: In the contrib/vim/README file there are instructions about how to setup git support with Vim builtin git syntax files. However these instructions seems to be redundant, since the system filetype.vim file already have the autocmd rules. What version of vim do you have? As the README says, version 7.2 and on come with the files already, and you do not need to do anything. If you have an older version that does not ship with them, and you are pulling them down directly from the URLs provided, then your vim probably does not already have them in its stock filetype.vim. The only issue I found is with: autocmd BufNewFile,BufRead .msg.[0-9]* \ if getline(1) =~ '^From.*# This line is ignored.$' | \ setf gitsendemail | \ endif It should be: autocmd BufNewFile,BufRead [0-9]*.patch It looks like .msg.[0-9] was originally used for send-email cover letters, and was changed to .gitsendemail.msg.* by commit eed6ca7. I think your [0-9]*.patch would match something else entirely (though it is still broken, of course, as .msg.* does not exist anymore). I'd argue that we should just remove contrib/vim at this point. It has no actual files in it, only pointers to vim.org for pre-7.2 vim users. And that version was released in 2008, so the README is helping almost nobody at this point (if you are on an ancient platform, and are an avid enough vim user to download the syntax files, I suspect you would simply install a newer version of vim). IMHO it should contain some other checks, to make sure it is a patch generated by git format-patch, and not, as an example, a plain patch or a Mercurial patch. By the way: I don't understand the purpose of gitsendemail syntax. On my system it does not highlight the diff. As far as I can tell, it is for cover letters, not for patches. Patches should already be handled by existing RFC822-message highlighting. -Peff -- 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: about vim contrib support
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 10/01/2013 12:39, Jeff King ha scritto: On Thu, Jan 10, 2013 at 12:17:31PM +0100, Manlio Perillo wrote: In the contrib/vim/README file there are instructions about how to setup git support with Vim builtin git syntax files. However these instructions seems to be redundant, since the system filetype.vim file already have the autocmd rules. What version of vim do you have? As the README says, version 7.2 and on come with the files already, and you do not need to do anything. Ah, right. I missed the first lines of the README file, sorry. [...] The only issue I found is with: autocmd BufNewFile,BufRead .msg.[0-9]* \ if getline(1) =~ '^From.*# This line is ignored.$' | \ setf gitsendemail | \ endif It should be: autocmd BufNewFile,BufRead [0-9]*.patch It looks like .msg.[0-9] was originally used for send-email cover letters, Ok, thanks. I was assuming it was used for the generated patched. and was changed to .gitsendemail.msg.* by commit eed6ca7. I think your [0-9]*.patch would match something else entirely (though it is still broken, of course, as .msg.* does not exist anymore). [...] By the way: I don't understand the purpose of gitsendemail syntax. On my system it does not highlight the diff. As far as I can tell, it is for cover letters, not for patches. Patches should already be handled by existing RFC822-message highlighting. .patch files are handled by diff highlight. What I would like to do is to use gitcommit syntax highlight, in order to also enable commit subject message hightlight. Thanks Manlio -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlDuqzYACgkQscQJ24LbaUQ5TgCfQPeX53KOsQDF6WJF1AaSpiRd NpMAn0GcffJwTA/etrnOnXAQctCKAY4W =IDVf -END PGP SIGNATURE- -- 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 v2 0/2] improve-wincred-compatibility
Changes since initial version (see attached diff for details): - split in two patches - removed unused variables - improved the dll error message - changed ?: to if else - added comments Also available here: https://github.com/kblees/git/tree/kb/improve-wincred-compatibility-v2 git pull git://github.com/kblees/git.git kb/improve-wincred-compatibility-v2 Karsten Blees (2): wincred: accept CRLF on stdin to simplify console usage wincred: improve compatibility with windows versions .../credential/wincred/git-credential-wincred.c| 206 - 1 file changed, 75 insertions(+), 131 deletions(-) git diff kb/improve-wincred-compatibility..kb/improve-wincred-compatibility-v2 diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index 3464080..dac19ea 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -66,7 +66,7 @@ typedef BOOL (WINAPI *CredEnumerateWT)(LPCWSTR, DWORD, DWORD *, typedef VOID (WINAPI *CredFreeT)(PVOID); typedef BOOL (WINAPI *CredDeleteWT)(LPCWSTR, DWORD, DWORD); -static HMODULE advapi, credui; +static HMODULE advapi; static CredWriteWT CredWriteW; static CredEnumerateWT CredEnumerateW; static CredFreeT CredFree; @@ -77,7 +77,7 @@ static void load_cred_funcs(void) /* load DLLs */ advapi = LoadLibrary(advapi32.dll); if (!advapi) - die(failed to load DLLs); + die(failed to load advapi32.dll); /* get function pointers */ CredWriteW = (CredWriteWT)GetProcAddress(advapi, CredWriteW); @@ -107,14 +107,34 @@ static void write_item(const char *what, LPCWSTR wbuf, int wlen) free(buf); } +/* + * Match an (optional) expected string and a delimiter in the target string, + * consuming the matched text by updating the target pointer. + */ static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim) { - LPCWSTR start = *ptarget; - LPCWSTR end = *delim ? wcsstr(start, delim) : start + wcslen(start); - int len = end ? end - start : wcslen(start); + LPCWSTR delim_pos, start = *ptarget; + int len; + + /* find start of delimiter (or end-of-string if delim is empty) */ + if (*delim) + delim_pos = wcsstr(start, delim); + else + delim_pos = start + wcslen(start); + + /* +* match text up to delimiter, or end of string (e.g. the '/' after +* host is optional if not followed by a path) +*/ + if (delim_pos) + len = delim_pos - start; + else + len = wcslen(start); + /* update ptarget if we either found a delimiter or need a match */ - if (end || want) - *ptarget = end ? end + wcslen(delim) : start + len; + if (delim_pos || want) + *ptarget = delim_pos ? delim_pos + wcslen(delim) : start + len; + return !want || (!wcsncmp(want, start, len) !want[len]); } @@ -157,9 +177,6 @@ static void get_credential(void) static void store_credential(void) { CREDENTIALW cred; - BYTE *auth_buf; - DWORD auth_buf_size = 0; - CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES]; if (!wusername || !password) return; -- 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 v2 1/2] wincred: accept CRLF on stdin to simplify console usage
The windows credential helper currently only accepts LF on stdin, but bash and cmd.exe both send CRLF. This prevents interactive use in the console. Change the stdin parser to optionally accept CRLF. Signed-off-by: Karsten Blees bl...@dcon.de --- contrib/credential/wincred/git-credential-wincred.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index cbaec5f..94d7140 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -284,10 +284,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) -- 1.8.0.msysgit.0.4.g4e40dea -- 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 v2 2/2] wincred: improve compatibility with windows versions
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. Credentials created with an old wincred version will have mangled or empty passwords after this change. Signed-off-by: Karsten Blees bl...@dcon.de --- .../credential/wincred/git-credential-wincred.c| 199 - 1 file changed, 70 insertions(+), 129 deletions(-) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index 94d7140..dac19ea 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -9,6 +9,8 @@ /* common helpers */ +#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0])) + static void die(const char *err, ...) { char msg[4096]; @@ -30,14 +32,6 @@ static void *xmalloc(size_t size) return ret; } -static char *xstrdup(const char *str) -{ - char *ret = strdup(str); - if (!ret) - die(Out of memory); - return ret; -} - /* MinGW doesn't have wincred.h, so we need to define stuff */ typedef struct _CREDENTIAL_ATTRIBUTEW { @@ -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; +static HMODULE advapi; static CredWriteWT CredWriteW; -static CredUnPackAuthenticationBufferWT CredUnPackAuthenticationBufferW; static CredEnumerateWT CredEnumerateW; -static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW; static CredFreeT CredFree; static CredDeleteWT CredDeleteW; @@ -88,74 +76,84 @@ static void load_cred_funcs(void) { /* load DLLs */ advapi = LoadLibrary(advapi32.dll); - credui = LoadLibrary(credui.dll); - if (!advapi || !credui) - die(failed to load DLLs); + if (!advapi) + die(failed to load advapi32.dll); /* get function pointers */ CredWriteW = (CredWriteWT)GetProcAddress(advapi, CredWriteW); - CredUnPackAuthenticationBufferW = (CredUnPackAuthenticationBufferWT) - GetProcAddress(credui, CredUnPackAuthenticationBufferW); CredEnumerateW = (CredEnumerateWT)GetProcAddress(advapi, CredEnumerateW); - CredPackAuthenticationBufferW = (CredPackAuthenticationBufferWT) - GetProcAddress(credui, CredPackAuthenticationBufferW); CredFree = (CredFreeT)GetProcAddress(advapi, CredFree); CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, CredDeleteW); - if (!CredWriteW || !CredUnPackAuthenticationBufferW || - !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree || - !CredDeleteW) + if (!CredWriteW || !CredEnumerateW || !CredFree || !CredDeleteW) die(failed to load functions); } -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); } -static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword, -const char *want) +/* + * Match an (optional)
[PATCH] git-commit-tree(1): correct description of defaults
The old phrasing indicated that the EMAIL environment variable takes precedence over the user.email configuration setting, but it is the other way around. Signed-off-by: Peter Eisentraut pe...@eisentraut.org --- Documentation/git-commit-tree.txt |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt index 6d5a04c..a221169 100644 --- a/Documentation/git-commit-tree.txt +++ b/Documentation/git-commit-tree.txt @@ -72,13 +72,13 @@ if set: GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE - EMAIL (nb , and \ns are stripped) In case (some of) these environment variables are not set, the information is taken from the configuration items user.name and user.email, or, if not -present, system user name and the hostname used for outgoing mail (taken +present, the environment variable EMAIL, or, if that is not set, +system user name and the hostname used for outgoing mail (taken from `/etc/mailname` and falling back to the fully qualified hostname when that file does not exist). -- 1.7.10.4 -- 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 v4] git-clean: Display more accurate delete messages
Zoltan Klinger zoltan.klin...@gmail.com writes: I think the code before this patch used to say Would not remove and Not removing in certain cases to report the paths that the command decided not to remove, but after this patch these two messages no longer appear in the patch. Is it expected, are we losing information, or...? I do not think we are losing any information. Say, we have a repo like this: test.git |-- untracked_file |-- untracked_bar | |-- bar.txt |-- untracked_foo |-- foo.txt The original version prints out: $ git clean -fn Would remove untracked_file Would not remove untracked_bar/ Would not remove untracked_foo/ We never asked for any directories to be removed so IMHO the Would not remove ... messages are just noise. The new version prints out: $ git clean -fn Would remove untracked_file Oh. I was blinded by the primary reason of your patch being be more careful and defer reporting a removal until we know everything in a directory did get removed and managed to remove the directory, and did not realize this noise removal. Perhaps add Also do not mention that we are not removing directories when the user did not ask us to do so with '-d'. or something to the description? Thanks for a clarification. -- 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
[PATCHv3] commit: make default of cleanup option configurable
The default of the cleanup option in git commit is not configurable. Users who don't want to use the default have to pass this option on every commit since there's no way to configure it. This commit introduces a new config option commit.cleanup which can be used to change the default of the cleanup option in git commit. Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- changes in v3: - remove extra blank line in builtin/commit.c - add newline at end of t/t7500/add-content-and-comment - improve documentation of commit.cleanup configuration variable as Junio suggested (makes a lot of sense) Thanks Documentation/config.txt| 9 + Documentation/git-commit.txt| 4 +- builtin/commit.c| 4 +- t/t7500/add-content-and-comment | 5 +++ t/t7502-commit.sh | 84 + 5 files changed, 97 insertions(+), 9 deletions(-) create mode 100755 t/t7500/add-content-and-comment diff --git a/Documentation/config.txt b/Documentation/config.txt index 53c4ca1..c92a308 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -917,6 +917,15 @@ column.tag:: Specify whether to output tag listing in `git tag` in columns. See `column.ui` for details. +commit.cleanup:: + This setting overrides the default of the `--cleanup` option in + `git commit`. See linkgit:git-commit[1] for details. Changing the + default can be useful when you always want to keep lines that begin + with comment character `#` in your log message, in which case you + would do `git config commit.cleanup whitespace` (note that you will + have to remove the help lines that begin with `#` in the commit log + template yourself, if you do this). + commit.status:: A boolean to enable/disable inclusion of status information in the commit message template when using an editor to prepare the commit diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 7bdb039..41b27da 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -179,7 +179,9 @@ OPTIONS only if the message is to be edited. Otherwise only whitespace removed. The 'verbatim' mode does not change message at all, 'whitespace' removes just leading/trailing whitespace lines - and 'strip' removes both whitespace and commentary. + and 'strip' removes both whitespace and commentary. The default + can be changed by the 'commit.cleanup' configuration variable + (see linkgit:git-config[1]). -e:: --edit:: diff --git a/builtin/commit.c b/builtin/commit.c index d6dd3df..7c2a3d4 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -103,7 +103,7 @@ static enum { CLEANUP_NONE, CLEANUP_ALL } cleanup_mode; -static char *cleanup_arg; +static const char *cleanup_arg; static enum commit_whence whence; static int use_editor = 1, include_status = 1; @@ -1320,6 +1320,8 @@ static int git_commit_config(const char *k, const char *v, void *cb) include_status = git_config_bool(k, v); return 0; } + if (!strcmp(k, commit.cleanup)) + return git_config_string(cleanup_arg, k, v); status = git_gpg_config(k, v, NULL); if (status) diff --git a/t/t7500/add-content-and-comment b/t/t7500/add-content-and-comment new file mode 100755 index 000..c4dccff --- /dev/null +++ b/t/t7500/add-content-and-comment @@ -0,0 +1,5 @@ +#!/bin/sh +echo commit message $1 +echo # comment $1 +exit 0 + diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index 1a5cb69..b1c7648 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -4,6 +4,15 @@ test_description='git commit porcelain-ish' . ./test-lib.sh +commit_msg_is () { + expect=commit_msg_is.expect + actual=commit_msg_is.actual + + printf %s $(git log --pretty=format:%s%b -1) $actual + printf %s $1 $expect + test_i18ncmp $expect $actual +} + # Arguments: [prefix] [commit message] [commit options] check_summary_oneline() { test_tick @@ -168,7 +177,7 @@ test_expect_success 'verbose respects diff config' ' git config --unset color.diff ' -test_expect_success 'cleanup commit messages (verbatim,-t)' ' +test_expect_success 'cleanup commit messages (verbatim option,-t)' ' echo negative { echo;echo # text;echo; } expect @@ -178,7 +187,7 @@ test_expect_success 'cleanup commit messages (verbatim,-t)' ' ' -test_expect_success 'cleanup commit messages (verbatim,-F)' ' +test_expect_success 'cleanup commit messages (verbatim option,-F)' ' echo negative git commit --cleanup=verbatim -F expect -a @@ -187,7 +196,7 @@ test_expect_success 'cleanup commit messages (verbatim,-F)' ' ' -test_expect_success 'cleanup commit messages (verbatim,-m)' ' +test_expect_success 'cleanup commit messages (verbatim option,-m)' ' echo negative
Re: t7400 broken on pu (Mac OS X)
Duy Nguyen pclo...@gmail.com writes: On Wed, Jan 09, 2013 at 07:43:03PM +0100, Torsten Bögershausen wrote: The current pu fails on Mac OS, case insensitive FS. Bisecting points out commit 3f28e4fafc046284657945798d71c57608bee479 [snip] Date: Sun Jan 6 13:21:07 2013 +0700 Convert add_files_to_cache to take struct pathspec I can reproduce it by setting core.ignorecase to true. There is a bug that I overlooked. Can you verify if this throw-away patch fixes it for you? A proper fix will be in the reroll later. I can see why it is wrong to let pathspec.raw be rewritten without making matching change to the containing pathspec, but I find it strange why it matters only on case-insensitive codepath. I agree with the Hack comment that the canonicalization should be done at a higher level upfront. Then ls-files does not need its own strip_trailing_slash_from_submodules(), and check_path_for_gitlink() can (and should---the callers of check_anything would not expect the function to change things) stop rewriting its parameter. Thanks for a quick response. -- 8 -- diff --git a/builtin/add.c b/builtin/add.c index 641037f..61cb8bd 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -155,12 +155,13 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int return seen; } -static void treat_gitlinks(const char **pathspec) +static int treat_gitlinks(const char **pathspec) { int i; + int modified = 0; if (!pathspec || !*pathspec) - return; + return modified; for (i = 0; i active_nr; i++) { struct cache_entry *ce = active_cache[i]; @@ -171,15 +172,17 @@ static void treat_gitlinks(const char **pathspec) if (len2 = len || pathspec[j][len] != '/' || memcmp(ce-name, pathspec[j], len)) continue; - if (len2 == len + 1) + if (len2 == len + 1) { /* strip trailing slash */ pathspec[j] = xstrndup(ce-name, len); - else + modified = 1; + } else die (_(Path '%s' is in submodule '%.*s'), pathspec[j], len, ce-name); } } } + return modified; } static void refresh(int verbose, const struct pathspec *pathspec) @@ -418,7 +421,16 @@ int cmd_add(int argc, const char **argv, const char *prefix) if (read_cache() 0) die(_(index file corrupt)); - treat_gitlinks(pathspec.raw); + if (treat_gitlinks(pathspec.raw)) + /* + * HACK: treat_gitlinks strips the trailing slashes + * out of submodule entries but it only affects + * raw[]. Everything in pathspec.items is not touched. + * Re-init it to propagate the change. Long term, this + * function should be moved to pathspec.c and update + * everything in a consistent way. + */ + init_pathspec(pathspec, pathspec.raw); if (add_new_files) { int baselen; -- 8 -- -- 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 04/19] reset: don't allow git reset -- $pathspec in bare repo
Martin von Zweigbergk martinv...@gmail.com writes: ... Fix by moving the branching point after the check. OK, that is what I missed. We have an existing check for mixed reset, which was originally meant to handle case without any pathspec but can use the same error condition (i.e. type is mixed and repository is bare) and error message (i.e. no mixed reset in a bare repository). reset with pathspec was done before that check kicked in. Thanks for clarification (and sorry for the noise). -- 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-commit-tree(1): correct description of defaults
Peter Eisentraut pe...@eisentraut.org writes: The old phrasing indicated that the EMAIL environment variable takes precedence over the user.email configuration setting, but it is the other way around. Signed-off-by: Peter Eisentraut pe...@eisentraut.org --- It could be argued that the observed behaviour is a bug, by the way. If we followed the normal command line options trump environment variables that in turn trump config variables that in turn trump whatever the default values we compute using cues from the system precedence order, EMAIL ought to come between the more specific GIT_{AUTHOR,COMMITTER}_EMAIL environment variables and the user.email configuration variable. But reading the value of EMAIL can also be seen as part of the using cues from the system (it often is set in equivalents of $HOME/.profile by equivalents of adduser) step, and the original motivation to add user.email indeed was to allow users to override EMAIL (or the name we grab from the system) without having to set the GIT_COMMITTER_EMAIL environment variable. So the current behaviour is correct, and the patch is a good (belated ;-) update to the documentation. Will apply. Thanks. Documentation/git-commit-tree.txt |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt index 6d5a04c..a221169 100644 --- a/Documentation/git-commit-tree.txt +++ b/Documentation/git-commit-tree.txt @@ -72,13 +72,13 @@ if set: GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE - EMAIL (nb , and \ns are stripped) In case (some of) these environment variables are not set, the information is taken from the configuration items user.name and user.email, or, if not -present, system user name and the hostname used for outgoing mail (taken +present, the environment variable EMAIL, or, if that is not set, +system user name and the hostname used for outgoing mail (taken from `/etc/mailname` and falling back to the fully qualified hostname when that file does not exist). -- 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: t7400 broken on pu (Mac OS X)
On 10.01.13 18:58, Junio C Hamano wrote: Duy Nguyen pclo...@gmail.com writes: On Wed, Jan 09, 2013 at 07:43:03PM +0100, Torsten Bögershausen wrote: The current pu fails on Mac OS, case insensitive FS. Bisecting points out commit 3f28e4fafc046284657945798d71c57608bee479 [snip] Date: Sun Jan 6 13:21:07 2013 +0700 Convert add_files_to_cache to take struct pathspec I can reproduce it by setting core.ignorecase to true. There is a bug that I overlooked. Can you verify if this throw-away patch fixes it for you? A proper fix will be in the reroll later. I can see why it is wrong to let pathspec.raw be rewritten without making matching change to the containing pathspec, but I find it strange why it matters only on case-insensitive codepath. I agree with the Hack comment that the canonicalization should be done at a higher level upfront. Then ls-files does not need its own strip_trailing_slash_from_submodules(), and check_path_for_gitlink() can (and should---the callers of check_anything would not expect the function to change things) stop rewriting its parameter. Thanks for a quick response. The patch fixes t7400. Thanks from my side as well /Torsten -- 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] commit: make default of cleanup option configurable
Junio C Hamano gits...@pobox.com writes: I also wonder, as a longer term alternative (which would require a lot of code auditing and some refactoring), if it is useful to have an option and/or configuration that lets you configure the comment in log message editor character from the default # to something else. git -c log.commentchar=% commit may start the log message editor with something like this in it: % Please enter the commit message for your changes. Lines starting % with '%' will be ignored, and an empty message aborts the commit. Naturally, setting log.commentchar to none would disable stripping of any commented lines if such a feature existed, and stop issuing these helpful comments altogether, but still strip excess blank lines and trailing whitespaces. I wouldn't seriously suggest it as I am not sure if the usefulness of such a feature would outweigh the cost of coding it, though; at least not at this point yet. A beginning of a patch to do would be like this, which does not look too bad. There are some low hanging fruits I didn't bother to do in this illustration (see NEEDSWORK comment in builtin/branch.c if some of you are interested in pursuing it). -- 8 -- From: Junio C Hamano gits...@pobox.com Date: Thu, 10 Jan 2013 11:17:21 -0800 Subject: [PATCH] Allow custom comment char Some users do want to write a line that begin with a pound sign, #, in their commit log message. Many tracking system recognise a token of #bugid form, for example. The support we offer these use cases is not very friendly to the end users. They have a choice between - Don't do it. Avoid such a line by rewrapping or indenting; and - Use --cleanup=whitespace but remove all the hint lines we add. Give them a way to set a custom comment char, e.g. $ git -c core.commentchar=% commit so that they do not have to do either of the two workarounds. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/config.txt | 6 ++ builtin/branch.c | 16 builtin/commit.c | 15 --- builtin/fmt-merge-msg.c | 4 +++- builtin/merge.c | 15 +++ builtin/stripspace.c | 2 +- cache.h | 6 ++ config.c | 8 environment.c| 6 ++ wt-status.c | 10 ++ 10 files changed, 67 insertions(+), 21 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index d5809e0..e99b9f2 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -528,6 +528,12 @@ core.editor:: variable when it is set, and the environment variable `GIT_EDITOR` is not set. See linkgit:git-var[1]. +core.commentchar:: + Commands such as `commit` and `tag` that lets you edit + messages consider a line that begins with this character + commented, and removes them after the editor returns + (default '#'). + sequence.editor:: Text editor used by `git rebase -i` for editing the rebase insn file. The value is meant to be interpreted by the shell when it is used. diff --git a/builtin/branch.c b/builtin/branch.c index 873f624..7f8865a 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -706,11 +706,19 @@ static int edit_branch_description(const char *branch_name) read_branch_desc(buf, branch_name); if (!buf.len || buf.buf[buf.len-1] != '\n') strbuf_addch(buf, '\n'); + /* +* NEEDSWORK: introduce a strbuf_commented_addf(), possibly +* sharing code with status_vprintf(), that makes each line +* commented with comment_line_char, and use it here and from +* other places (e.g. write_commented_object() and create_note() +* in builtin/notes.c and create_tag() in builtin/tag.c). +*/ strbuf_addf(buf, - # Please edit the description for the branch\n - # %s\n - # Lines starting with '#' will be stripped.\n, - branch_name); + %c Please edit the description for the branch\n + %c %s\n + %c Lines starting with '%c' will be stripped.\n, + comment_line_char, comment_line_char, + branch_name, comment_line_char, comment_line_char); fp = fopen(git_path(edit_description), w); if ((fwrite(buf.buf, 1, buf.len, fp) buf.len) || fclose(fp)) { strbuf_release(buf); diff --git a/builtin/commit.c b/builtin/commit.c index d6dd3df..a946a13 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -733,15 +733,16 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (cleanup_mode == CLEANUP_ALL) status_printf(s, GIT_COLOR_NORMAL, _(Please enter the commit message for your changes. -Lines starting\nwith
[PATCH] contrib/vim: simplify instructions for old vim support
Rely on the upstream filetype.vim instead of duplicating its rules in git's instructions for syntax highlighting support on pre-7.2 vim versions. The result is a shorter contrib/vim/README. More importantly, it lets us punt on maintenance of the autocmd rules. So now when we fix the upstream gitsendemail rule in light of commit eed6ca7, new git users stuck on old vim reading contrib/vim/README can automagically get the fix without any further changes needed to git. Once the world has moved on to vim 7.2+ completely, we can get rid of these instructions, but for now if they are this simple it's effortless to keep them. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Jeff King wrote: I'd argue that we should just remove contrib/vim at this point. It has no actual files in it, only pointers to vim.org for pre-7.2 vim users. I think that's reasonable. Of course we can still discuss enhancements to the vim support on this list, but ultimately it's easiest to distribute and document such work upstream in the usual way for vim plugins. How about this patch? contrib/vim/README | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/contrib/vim/README b/contrib/vim/README index fca1e17..8f16d06 100644 --- a/contrib/vim/README +++ b/contrib/vim/README @@ -17,16 +17,6 @@ To install: 1. Copy these files to vim's syntax directory $HOME/.vim/syntax 2. To auto-detect the editing of various git-related filetypes: - $ cat $HOME/.vim/filetype.vim 'EOF' - autocmd BufNewFile,BufRead *.git/COMMIT_EDITMSGsetf gitcommit - autocmd BufNewFile,BufRead *.git/config,.gitconfig setf gitconfig - autocmd BufNewFile,BufRead git-rebase-todo setf gitrebase - autocmd BufNewFile,BufRead .msg.[0-9]* - \ if getline(1) =~ '^From.*# This line is ignored.$' | - \ setf gitsendemail | - \ endif - autocmd BufNewFile,BufRead *.git/** - \ if getline(1) =~ '^\x\{40\}\\|^ref: ' | - \ setf git | - \ endif - EOF + + $ curl http://ftp.vim.org/pub/vim/runtime/filetype.vim | + sed -ne '/^ Git$/, /^$/ p' $HOME/.vim/filetype.vim -- 1.8.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: [PATCH] t0008: avoid brace expansion
Am 10.01.2013 01:18, schrieb Junio C Hamano: Adam Spiers g...@adamspiers.org writes: On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe rene.scha...@lsrfire.ath.cx wrote: Brace expansion is not required by POSIX and not supported by dash nor NetBSD's sh. Explicitly list all combinations instead. Good catch, thanks! Yeah; thanks. It would also be nice to avoid touch while we are at it, by the way. Good idea! Replacement patch: --- 8 --- Brace expansion is a shell feature that's not required by POSIX and not supported by dash nor NetBSD's sh. Explicitly list all combinations instead. Also avoid calling touch by creating the test files with a redirection instead, as suggested by Junio. Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx --- t/t0008-ignores.sh | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index 9b0fcd6..d7df719 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -129,8 +129,13 @@ test_expect_success 'setup' ' one ignored-* EOF - touch {,a/}{not-ignored,ignored-{and-untracked,but-in-index}} - git add -f {,a/}ignored-but-in-index + for dir in . a + do + : $dir/not-ignored + : $dir/ignored-and-untracked + : $dir/ignored-but-in-index + done + git add -f ignored-but-in-index a/ignored-but-in-index cat -\EOF a/.gitignore two* *three -- 1.8.0 -- 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] contrib/vim: simplify instructions for old vim support
On Thu, Jan 10, 2013 at 12:54:27PM -0800, Jonathan Nieder wrote: Rely on the upstream filetype.vim instead of duplicating its rules in git's instructions for syntax highlighting support on pre-7.2 vim versions. The result is a shorter contrib/vim/README. More importantly, it lets us punt on maintenance of the autocmd rules. So now when we fix the upstream gitsendemail rule in light of commit eed6ca7, new git users stuck on old vim reading contrib/vim/README can automagically get the fix without any further changes needed to git. Once the world has moved on to vim 7.2+ completely, we can get rid of these instructions, but for now if they are this simple it's effortless to keep them. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Jeff King wrote: I'd argue that we should just remove contrib/vim at this point. It has no actual files in it, only pointers to vim.org for pre-7.2 vim users. I think that's reasonable. Of course we can still discuss enhancements to the vim support on this list, but ultimately it's easiest to distribute and document such work upstream in the usual way for vim plugins. How about this patch? Yeah, I think this makes sense. I'd be fine with removing it entirely, but it doesn't hurt to err on the conservative side and leave it there. It's not like it's generating a huge maintenance burden, and with your patch, there is even less to maintain. Acked-by: Jeff King p...@peff.net -Peff -- 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/PATCH] avoid SIGPIPE warnings for aliases
On Thu, Jan 10, 2013 at 12:22:49PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: Maybe the right rule is if we are using the shell to execute, do not mention SIGPIPE? It seems a little iffy at first, but: 1. It tends to coincide with direct use of internal tools versus external tools. 2. We do not reliably get SIGPIPE there, anyway, since most shells will convert it into exit code 141 before we see it. I.e., something like: Hmph. That may be a good heuristics, but I wonder if we also want to special case WIFEXITED(status) WEXITSTATUS(status) == 141 to pretend as if nothing went wrong, when ignore_sigpipe is in effect? We could, but I don't see much point. There is very little to gain (because nobody is complaining about the exit code, only the message), and we might possibly fail to propagate an error condition (unlikely, but more serious consequences). To be honest, I am having doubts about touching it at all. I had to really work to provoke the error without setting SHELL_PATH=zsh, so I am worried that we are getting worked up over something that just doesn't happen in practice. And I am not sure that setting SHELL_PATH=zsh is a sane thing (I only knew about it because Bart mentioned it he was using zsh). Bart, do you actually set up SHELL_PATH like that? Is /bin/sh on your system zsh? If the latter, I wonder if this is actually a bug in zsh. -Peff -- 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/PATCH] avoid SIGPIPE warnings for aliases
Am 10.01.2013 21:22, schrieb Junio C Hamano: Jeff King p...@peff.net writes: Maybe the right rule is if we are using the shell to execute, do not mention SIGPIPE? It seems a little iffy at first, but: 1. It tends to coincide with direct use of internal tools versus external tools. 2. We do not reliably get SIGPIPE there, anyway, since most shells will convert it into exit code 141 before we see it. I.e., something like: Hmph. That may be a good heuristics, but I wonder if we also want to special case WIFEXITED(status) WEXITSTATUS(status) == 141 to pretend as if nothing went wrong, when ignore_sigpipe is in effect? The purpose of Peff's patch is to remove the error message, but not to pretend success (the return code remains 141). I looked at all instances with use_shell=1 or RUN_USING_SHELL: Most of the time, we do not care where the output of the command goes to, which I regard as the same case as when a shell runs a command: We don't need to report the SIGPIPE death. The interesting cases are when git reads back the output of the command. Here, a SIGPIPE death of the child would indicate a bug in git, I think, and some diagnostic would be worth it. But we can just as well declare that git doesn't have bugs ;) These are the interesting cases: connect.c:640: conn-use_shell = 1; a connection to a local repository convert.c:372: child_process.use_shell = 1; clean/smudge filter credential.c:216: helper.use_shell = 1; credential helper diff.c:4851:child.use_shell = 1; textconv All in all, I think the heuristics makes sense. -- Hannes -- 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/PATCH] avoid SIGPIPE warnings for aliases
Johannes Sixt j...@kdbg.org writes: The interesting cases are when git reads back the output of the command. Here, a SIGPIPE death of the child would indicate a bug in git, I think, and some diagnostic would be worth it. But we can just as well declare that git doesn't have bugs ;) These are the interesting cases: connect.c:640: conn-use_shell = 1; a connection to a local repository convert.c:372: child_process.use_shell = 1; clean/smudge filter credential.c:216: helper.use_shell = 1; credential helper diff.c:4851:child.use_shell = 1; textconv All in all, I think the heuristics makes sense. Fair enough. Thanks for grepping. -- 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 v5] git-clean: Display more accurate delete messages
(1) Only print out the names of the files and directories that got actually deleted. Also do not mention that we are not removing directories when the user did not ask us to do so with '-d'. (2) Show ignore message for skipped untracked git repositories. Consider the following repo layout: test.git/ |-- tracked_dir/ | |-- some_tracked_file | |-- some_untracked_file |-- tracked_file |-- untracked_file |-- untracked_foo/ | |-- bar/ | | |-- bar.txt | |-- emptydir/ | |-- frotz.git/ | |-- frotz.tx |-- untracked_some.git/ |-- some.txt Suppose the user issues 'git clean -fd' from the test.git directory. When -d option is used and untracked directory 'foo' contains a subdirectory 'frotz.git' that is managed by a different git repository therefore it will not be removed. $ git clean -fd Removing tracked_dir/some_untracked_file Removing untracked_file Removing untracked_foo/ Removing untracked_some.git/ The message displayed to the user is slightly misleading. The foo/ directory has not been removed because of foo/frotz.git still exists. On the other hand the subdirectories 'bar' and 'emptydir' have been deleted but they're not mentioned anywhere. Also, untracked_some.git has not been removed either. This behaviour is the result of the way the deletion of untracked directories are reported. In the current implementation they are deleted recursively but only the name of the top most directory is printed out. The calling function does not know about any subdirectories that could not be removed during the recursion. Improve the way the deleted directories are reported back to the user: (1) Create a recursive delete function 'remove_dirs' in builtin/clean.c to run in both dry_run and delete modes with the delete logic as follows: (a) Check if the current directory to be deleted is an untracked git repository. If it is and --force --force option is not set do not touch this directory, print ignore message, set dir_gone flag to false for the caller and return. (b) Otherwise for each item in current directory: (i) If current directory cannot be accessed, print warning, set dir_gone flag to false and return. (ii) If the item is a subdirectory recurse into it, check for the returned value of the dir_gone flag. If the subdirectory is gone, add the name of the deleted directory to a list of successfully removed items 'dels'. Else set the dir_gone flag as the current directory cannot be removed because we have at least one subdirectory hanging around. (iii) If it is a file try to remove it. If success add the file name to the 'dels' list, else print error and set dir_gone flag to false. (c) After we finished deleting all items in the current directory and the dir_gone flag is still true, remove the directory itself. If failed set the dir_gone flag to false. (d) If the current directory cannot be deleted because the dir_gone flag has been set to false, print out all the successfully deleted items for this directory from the 'dels' list. (e) We're done with the current directory, return. (2) Modify the cmd_clean() function to: (a) call the recursive delete function 'remove_dirs()' for each topmost directory it wants to remove (b) check for the returned value of dir_gone flag. If it's true print the name of the directory as being removed. Consider the output of the improved version: $ git clean -fd Removing tracked_dir/some_untracked_file Removing untracked_file Skipping repository untracked_foo/frotz.git Removing untracked_foo/bar Removing untracked_foo/emptydir Skipping repository untracked_some.git/ Now it displays only the file and directory names that got actually deleted and shows the name of the untracked git repositories it ignored. Reported-by: Soren Brinkmann soren.brinkm...@xilinx.com Signed-off-by: Zoltan Klinger zoltan.klin...@gmail.com --- builtin/clean.c | 156 --- 1 file changed, 127 insertions(+), 29 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 69c1cda..943845d 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -10,6 +10,7 @@ #include cache.h #include dir.h #include parse-options.h +#include refs.h #include string-list.h #include quote.h @@ -20,6 +21,12 @@ static const char *const builtin_clean_usage[] = { NULL }; +static const char *msg_remove = N_(Removing %s\n); +static const char *msg_would_remove = N_(Would remove %s\n); +static const char *msg_skip_git_dir = N_(Skipping
Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
Duy Nguyen pclo...@gmail.com writes: On Thu, Jan 10, 2013 at 2:26 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: We use the path arguments in two places in reset.c: in interactive_reset() and read_from_tree(). Both of these call get_pathspec(), so we pass the (prefix, arv) pair to both functions. Move the call to get_pathspec() out of these methods, for two reasons: 1) One argument is simpler than two. 2) It lets us use the (arguably clearer) if (pathspec) in place of if (i argc). --- If I understand correctly, this should be rebased on top of nd/parse-pathspec. Please let me know. Yeah, this will conflict with the get_pathspec-to-parse_pathspec conversion Duy has been working on. Or I could hold off nd/parse-pathspec if this series has a better chance of graduation first. Decision? I am greedy and want to have both ;-) Before deciding that, I'd appreciate a second set of eyes giving Martin's series an independent review, to see if it is going in the right direction. I think I didn't spot anything questionable in it myself, but second opinion always helps. There is no textual conflict between the two topics at the moment, but because the ultimate goal of your series is to remove all uses of the pathspec.raw[] field outside the implementation of pathspec matching, it might help to rename the field to _private_raw (or remove it), and either make get_pathspec() private or disappear, to ensure that the compiler will help us catching semantic conflicts with new users of it at a late stage of your 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
Re: bugreport: stgit cannot export empty patch
Junio C Hamano writes: Stepan Koltsov lt;stepan.koltsov@gt; writes: stgit fails to export empty patches: % stg new empty-patch -m 'asasas' Now at patch empty-patch % stg export empty-patch Checking for changes in the working directory ... done fatal: unrecognized input stg export: git failed with code 128 zsh: exit 2 stg export empty-patch % stg --version Stacked GIT 0.16-3-g67cf git version 1.7.9.1 Python version 2.7.1 (r271:86832, Jul 31 2011, 19:30:53) [GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2335.15.00)] I don't use (or read the sources to) StGIT, but isn't the whole point of stg export to convert your StGIT patches into patch files? For an empty commit, what is an appropriate output? IOW, is it reasonable to have an empty commit in your history if you are planning to stg export it? Here's another example where the functionality is useful: https://github.com/Hobo/agility-gitorial-patches is used to build http://cookbook.hobocentral.net/tutorials/agility Each commit/patch becomes a step in the tutorial. Some tutorial steps don't have any code attached to them. In the past, an export of an empty commit would yield a patch where the last three lines were --- 0 files changed, 0 insertions(+), 0 deletions(-) The latest version of stgit along with git v1.6 eliminates the error. A bisection identifies cc64b318f26c9e176c4f07b1a459a86e7a04c4eb as the source of the problem. Thanks, Bryan -- View this message in context: http://git.661346.n2.nabble.com/bugreport-stgit-cannot-export-empty-patch-tp7559494p7574691.html Sent from the git mailing list archive at Nabble.com. -- 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 02/21] Add parse_pathspec() that converts cmdline args to struct pathspec
On Sat, Jan 5, 2013 at 10:20 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: + + /* No arguments, no prefix - no pathspec */ + if (!entry !prefix) + return; + /* No arguments with prefix - prefix pathspec */ When working with the old get_pathspec(), I remember wondering if a flag switching between no argument - prefix pathspec and no argument - no pathspec would be worthwhile. I think e.g. 'add' and 'clean' would use the former , while 'reset' and 'commit' would use the latter. Since you're now changing all the callers of get_pathspec(), it seems like the perfect time to ask this question. What do you think? -- 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] minor diff between gitweb docs and actual template for $GIT/description
The documentation for gitweb gives one description of the default content for the $GIT/description, the description template has other text. One of these two patches should be applied to bring them into order (applying both would just reverse the problem). Or, both could be changed to the same new text. -tkc (not subscribed to the actual git ML, just git-us...@googlegroups.com) diff --git a/Documentation/gitweb.txt b/Documentation/gitweb.txt index 168e8bf..7c4cb69 100644 --- a/Documentation/gitweb.txt +++ b/Documentation/gitweb.txt @@ -227,7 +227,7 @@ description (or `gitweb.description`):: repository). Plain text file; HTML will be escaped. By default set to + --- -Unnamed repository; edit this file to name it for gitweb. +Unnamed repository; edit this file 'description' to name the repository. --- + from the template during repository creation, usually installed in diff --git a/templates/this--description b/templates/this--description index 498b267..c6f25e8 100644 --- a/templates/this--description +++ b/templates/this--description @@ -1 +1 @@ -Unnamed repository; edit this file 'description' to name the repository. +Unnamed repository; edit this file to name it for gitweb.
Re: [PATCH] minor diff between gitweb docs and actual template for $GIT/description
(+cc: Jakub, who maintains gitweb) Hi Tim, Tim Chase wrote: The documentation for gitweb gives one description of the default content for the $GIT/description, the description template has other text. One of these two patches should be applied to bring them into order (applying both would just reverse the problem). Or, both could be changed to the same new text. May we have your sign-off? (See Documentation/SubmittingPatches for what this means.) --- a/Documentation/gitweb.txt +++ b/Documentation/gitweb.txt @@ -227,7 +227,7 @@ description (or `gitweb.description`):: repository). Plain text file; HTML will be escaped. By default set to + --- -Unnamed repository; edit this file to name it for gitweb. +Unnamed repository; edit this file 'description' to name the repository. --- + from the template during repository creation, usually installed in Looks sane to me. Thanks for noticing. Regards, Jonathan -- 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 02/21] Add parse_pathspec() that converts cmdline args to struct pathspec
On Fri, Jan 11, 2013 at 6:26 AM, Martin von Zweigbergk martinv...@gmail.com wrote: On Sat, Jan 5, 2013 at 10:20 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: + + /* No arguments, no prefix - no pathspec */ + if (!entry !prefix) + return; + /* No arguments with prefix - prefix pathspec */ When working with the old get_pathspec(), I remember wondering if a flag switching between no argument - prefix pathspec and no argument - no pathspec would be worthwhile. I think e.g. 'add' and 'clean' would use the former , while 'reset' and 'commit' would use the latter. Since you're now changing all the callers of get_pathspec(), it seems like the perfect time to ask this question. What do you think? Yes that'll simplify the call sites. Will do. -- Duy -- 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 v2] clone: forbid --bare --separate-git-dir dir
The --separate-git-dir option was introduced to make it simple to put the git directory somewhere outside the worktree, for example when cloning a repository for use as a submodule. It was not intended for use when creating a bare repository. In that case there is no worktree and it is more natural to directly clone the repository and create a .git file as separate steps: git clone --bare /path/to/repo.git bar.git printf 'gitdir: bar.git\n' foo.git Forbid the combination, making the command easier to explain. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Just reword the commit message (or copying it from Jonathan actually). No comments about remove_junk_on_signal because we're less likely to re-enable it again. builtin/clone.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/clone.c b/builtin/clone.c index ec2f75b..b30189f 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -704,6 +704,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (option_origin) die(_(--bare and --origin %s options are incompatible.), option_origin); + if (real_git_dir) + die(_(--bare and --separate-git-dir are incompatible.)); option_no_checkout = 1; } -- 1.8.0.rc2.23.g1fb49df -- 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] clone: forbid --bare --separate-git-dir dir
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
[PATCH 2/2] upload-pack: fix off-by-one depth calculation in shallow clone
get_shallow_commits() is used to determine the cut points at a given depth (i.e. the number of commits in a chain that the user likes to get). However we count current depth up to the commit commit but we do the cutting at its parents (i.e. current depth + 1). This makes upload-pack always return one commit more than requested. This patch fixes it. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- shallow.c | 8 +++- t/t5500-fetch-pack.sh | 25 +++-- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/shallow.c b/shallow.c index a0363de..6be915f 100644 --- a/shallow.c +++ b/shallow.c @@ -72,8 +72,14 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth, } if (parse_commit(commit)) die(invalid commit); - commit-object.flags |= not_shallow_flag; cur_depth++; + if (cur_depth = depth) { + commit_list_insert(commit, result); + commit-object.flags |= shallow_flag; + commit = NULL; + continue; + } + commit-object.flags |= not_shallow_flag; for (p = commit-parents, commit = NULL; p; p = p-next) { if (!p-item-util) { int *pointer = xmalloc(sizeof(int)); diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 6a6e672..58d3bdf 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -130,16 +130,25 @@ test_expect_success 'single given branch clone' ' test_must_fail git --git-dir=branch-a/.git rev-parse origin/B ' +test_expect_success 'clone shallow depth 1' ' + git clone --no-single-branch --depth 1 file://$(pwd)/. shallow0 + test `git --git-dir=shallow0/.git rev-list --count HEAD` = 1 +' + test_expect_success 'clone shallow' ' git clone --no-single-branch --depth 2 file://$(pwd)/. shallow ' +test_expect_success 'clone shallow depth count' ' + test `git --git-dir=shallow/.git rev-list --count HEAD` = 2 +' + test_expect_success 'clone shallow object count' ' ( cd shallow git count-objects -v ) count.shallow - grep ^in-pack: 18 count.shallow + grep ^in-pack: 12 count.shallow ' test_expect_success 'clone shallow object count (part 2)' ' @@ -256,12 +265,16 @@ test_expect_success 'additional simple shallow deepenings' ' ) ' +test_expect_success 'clone shallow depth count' ' + test `git --git-dir=shallow/.git rev-list --count HEAD` = 11 +' + test_expect_success 'clone shallow object count' ' ( cd shallow git count-objects -v ) count.shallow - grep ^count: 52 count.shallow + grep ^count: 55 count.shallow ' test_expect_success 'fetch --depth --no-shallow' ' @@ -289,7 +302,7 @@ test_expect_success 'clone shallow object count' ' cd shallow2 git count-objects -v ) count.shallow2 - grep ^in-pack: 6 count.shallow2 + grep ^in-pack: 3 count.shallow2 ' test_expect_success 'clone shallow with --branch' ' @@ -297,7 +310,7 @@ test_expect_success 'clone shallow with --branch' ' ' test_expect_success 'clone shallow object count' ' - echo in-pack: 6 count3.expected + echo in-pack: 3 count3.expected GIT_DIR=shallow3/.git git count-objects -v | grep ^in-pack count3.actual test_cmp count3.expected count3.actual @@ -326,7 +339,7 @@ EOF GIT_DIR=shallow6/.git git tag -l taglist.actual test_cmp taglist.expected taglist.actual - echo in-pack: 7 count6.expected + echo in-pack: 4 count6.expected GIT_DIR=shallow6/.git git count-objects -v | grep ^in-pack count6.actual test_cmp count6.expected count6.actual @@ -341,7 +354,7 @@ EOF GIT_DIR=shallow7/.git git tag -l taglist.actual test_cmp taglist.expected taglist.actual - echo in-pack: 7 count7.expected + echo in-pack: 4 count7.expected GIT_DIR=shallow7/.git git count-objects -v | grep ^in-pack count7.actual test_cmp count7.expected count7.actual -- 1.8.0.rc2.23.g1fb49df -- 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] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Eric S. Raymond e...@thyrsus.com The combination of git-cvsimport and cvsps had serious problems. Among these were: (1) Analysis of branchy repos was buggy in multiple ways in both programs, leading to incorrect repo translations. (2) Even after a correct branch analysis, extra (redundant) fileops would often be generated on the new-branch side. (3) Inability to report more than one tag pointing to the same revision. (4) Failure in certain cases of clock-skew reported by the t9603 test. (5) Failure to use commitids for changeset ordering in cases were this would have prevented clock skew from causing incorrect grouping. Problems 2-5 and portions of problem 1 have been solved by a major rewrite of cvsps (the 3.x release series); it now emits a git fast-import stream. Also, the buggy attempt at ancestry-branch tracking previously invoked by -A has been replaced with a simpler and better topo analysis. cvsps is now about 20% smaller than formerly. All this changed cvsps's interface enough to require a complete rewrite of git-cvsimport (hence this patch). In the process the code size of the wrapper script dropped by about x3 and it can now support alternate conversion engines; the first new engine is cvs2git, with parsecvs expected to follow shortly. The old Perl git-cvsimport is moved to git-cvsimport-fallback; new git-cvsimport will hand off to the fallback script when it detects that the user has a pre-3.x cvsps. The fallback is only there so that people with simple enough repositories that can be correctly handled by cvsps-2.x but without the bleeding-edge cvsps 3.x installed does not have to be left without any cvsimport that works for them (with the same bugs and all). The fallback support will be removed after cvsps 3.x and the rewritten cvsimport matures and gets widely available. This patch also removes Michael Haggerty's git-cvsimport tests (t960[123]) from the git tree. These are actually conversion-engine tests and have been merged into a larger cvsps test suite, which I intend to spin out into a general CVS-lifting test that can also be applied to utilities such as cvs2git and parsecvs. The t9604 test will move in a future patch, when I likewise have it integrated into the general test suite. The following known bug has not been fixed: If any files were ever cvs imported more than once (e.g., import of more than one vendor release) the HEAD contains the wrong content. However, cvsps now emits a warning in this case. There is also one pathological tagging case that was successful in the former t9602 test that now fails (with a warning). I plan to address these problems. This patch at least gets the cvsps-3.x/git-cvsimport combination to a state that is not too broken to ship - that is, in all failure cases known to me it now emits useful warnings rather than silently botching the import. Signed-off-by: Eric S. Raymond e...@thyrsus.com Signed-off-by: Junio C Hamano gits...@pobox.com --- * This was (re)sent privately to me; the previous attempt by Eric somehow did not reach the list. Let me see if I have better luck, as we really would want to be reviewing these patches. I rewrote one paragraph in the log about the fallback, though. For the record the original read like this: The old Perl git-cvsimport is moved to git-cvsimport-fallback; new git-cvsimport will hand off to the fallback script when it detects that the user has a pre-3.x cvsps. I make no warranties that cvsps-2.x will actually work or do anything more useful than make demons fly out of your nose; the fallback is only there because Junio disliked the idea of a flag day, and should be removed after a decent interval. which I felt somewhat irresponsible to existing users. Makefile | 3 +- git-cvsimport.perl = git-cvsimport-fallback.perl | 8 + git-cvsimport.py | 354 + t/t9601/cvsroot/.gitattributes | 1 - t/t9601/cvsroot/CVSROOT/.gitignore | 2 - t/t9601/cvsroot/module/added-imported.txt,v| 44 --- t/t9601/cvsroot/module/imported-anonymously.txt,v | 42 --- .../module/imported-modified-imported.txt,v| 76 - t/t9601/cvsroot/module/imported-modified.txt,v | 59 t/t9601/cvsroot/module/imported-once.txt,v | 43 --- t/t9601/cvsroot/module/imported-twice.txt,v| 60 t/t9602/README | 62 t/t9602/cvsroot/.gitattributes | 1 - t/t9602/cvsroot/CVSROOT/.gitignore | 2 - t/t9602/cvsroot/module/default,v | 102 -- t/t9602/cvsroot/module/sub1/default,v | 102 -- t/t9602/cvsroot/module/sub1/subsubA/default,v | 101 -- t/t9602/cvsroot/module/sub1/subsubB/default,v | 107 --- .../module/sub2/Attic/branch_B_MIXED_only,v
Suggestion: add option in git-p4 to preserve user in Git repository
Hi, I'm in a situation where I don't have P4 admin rights to use the --preserve-user option of git-p4. However, I would like to keep user information in the associated Git branch. Would it be possible to add an option for this? Thanks, -=- Olivier -- 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/2] fetch, upload-pack: add --no-shallow for infinite depth
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: The user can do --depth=2147483647 (*) for infinite depth now. But it's hard to remember. Any other numbers larger than the longest commit chain in the repository would also do, but some guessing may be involved. Make easy-to-remember --no-shallow an alias for --depth=2147483647. I think no shallow makes sense in a much more important way than infinite depth, and this patch is a good idea for a reason entirely different from the justification your log message makes ;-) We explain that clone --depth=number is a way to end up with a partial repository that contains only the recent history, and while this may give users a smaller repository, in return the result will be a shallow repository with certain limitations (i.e. fetching or cloning from such a repository will be refused). We also explain that the reason to use --depth=number is to grab some history behind what you originally acquired into your shallow repository so that you have deeper history than your original shallow repository. A reader who does not know how this shallowness is implemented cannot be blamed if she thinks the shallowness is a permanent attribute of a repository, and once a repository is tainted by shallowness, it cannot be wiped away by deepening it, because nowhere in the documentation we say the shallowness will go away once you grabbed enough number of older commits with it. Calling the option --no-shallow (or even better, --unshallow, meaning make it a repository that is no longer shallow) makes it crystal clear that the option is about wiping away the shallowness. Of course, the result has to contain an untruncted history, but that is a mere side effect and an implementation detail from the end user's point of view. Make upload-pack recognize this special number as infinite depth. The effect is essentially the same as before, except that upload-pack is more efficient because it does not have to traverse to the bottom any more. The chance of a user actually wanting exactly 2147483647 commits depth, not infinite, on a repository with a history that long, is probably too small to consider. (*) This is the largest positive number a 32-bit signed integer can contain. JGit and older C Git store depth as int so both are OK with this number. Dulwich does not support shallow clone. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/fetch-options.txt | 4 Documentation/git-fetch-pack.txt| 2 ++ Documentation/technical/shallow.txt | 3 +++ builtin/fetch.c | 15 ++- commit.h| 3 +++ t/t5500-fetch-pack.sh | 16 upload-pack.c | 13 ++--- 7 files changed, 52 insertions(+), 4 deletions(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 6e98bdf..012d1b2 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -13,6 +13,10 @@ to the specified number of commits from the tip of each remote branch history. Tags for the deepened commits are not fetched. +--no-shallow:: + Deepen to the roots of the repository's history (i.e. the + result repository is no longer shallow). + Mentioning both is a good thing, but I think no longer shallow is more important aspect of this operation than deepen to the roots. diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 6322e8a..6a6e672 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -264,6 +264,22 @@ test_expect_success 'clone shallow object count' ' grep ^count: 52 count.shallow ' +test_expect_success 'fetch --depth --no-shallow' ' + ( + cd shallow + test_must_fail git fetch --depth=1 --no-shallow + ) +' + +test_expect_success 'infinite deepening (full repo)' ' + ( + cd shallow + git fetch --no-shallow + git fsck --full + ! test -f .git/shallow This looks as if fsck is what removes .git/shallow but I do not think that is what is being tested... diff --git a/upload-pack.c b/upload-pack.c index 6142421..88f0029 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -670,10 +670,17 @@ static void receive_needs(void) if (depth == 0 shallows.nr == 0) return; if (depth 0) { - struct commit_list *result, *backup; + struct commit_list *result = NULL, *backup = NULL; int i; - backup = result = get_shallow_commits(want_obj, depth, - SHALLOW, NOT_SHALLOW); + if (depth == INFINITE_DEPTH) + for (i = 0; i shallows.nr; i++) { + struct object *object = shallows.objects[i].item; + object-flags |= NOT_SHALLOW; + } +
Re: [PATCH] minor diff between gitweb docs and actual template for $GIT/description
On 01/10/13 20:22, Jonathan Nieder wrote: (+cc: Jakub, who maintains gitweb) Hi Tim, Tim Chase wrote: The documentation for gitweb gives one description of the default content for the $GIT/description, the description template has other text. One of these two patches should be applied to bring them into order (applying both would just reverse the problem). Or, both could be changed to the same new text. May we have your sign-off? (See Documentation/SubmittingPatches for what this means.) Hahahahah...a one liner doc-fix copy/pasting code from one place in the codebase to another? If you need it, you've got it: For my one line diff: Signed-off-by: Tim Chase g...@tim.thechases.com Otherwise, consider my contribution CC0 or public domain or whatever suits you best. Or take my patch as a bug report and make the fix yourself. :-) The patches should have been based off the master branch of github.com/git/git, FWIW. -tkc @gumnos -- 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/3] fixup remaining cvsimport tests
These patchs apply on top of of Eric Raymond's cvsimport patch. 7 of 15 tests in t9600 fail, one of which is fixed w/ a cvsps patch I've sent to Eric (fixes revision map.) It no longer uses origin as the default branch which I suspect is a problem for at least some of the remaining tests. Both of the t9604 tests pass. Chris Chris Rorvick (3): t/lib-cvs.sh: allow cvsps version 3.x. t9600: fixup for new cvsimport t9604: fixup for new cvsimport t/lib-cvs.sh| 2 +- t/t9600-cvsimport.sh| 10 -- t/t9604-cvsimport-timestamps.sh | 5 ++--- 3 files changed, 7 insertions(+), 10 deletions(-) -- 1.8.1.1.g220e17a -- 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/3] t9600: fixup for new cvsimport
--- t/t9600-cvsimport.sh | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh index 4c384ff..14f54d5 100755 --- a/t/t9600-cvsimport.sh +++ b/t/t9600-cvsimport.sh @@ -44,7 +44,7 @@ EOF test_expect_success PERL 'import a trivial module' ' - git cvsimport -a -R -z 0 -C module-git module + git cvsimport -R -z 0 -C module-git module test_cmp module-cvs/o_fortuna module-git/o_fortuna ' @@ -90,8 +90,7 @@ test_expect_success PERL 'update git module' ' (cd module-git git config cvsimport.trackRevisions true - git cvsimport -a -z 0 module - git merge origin + git cvsimport -z 0 module ) test_cmp module-cvs/o_fortuna module-git/o_fortuna @@ -119,8 +118,7 @@ test_expect_success PERL 'cvsimport.module config works' ' (cd module-git git config cvsimport.module module git config cvsimport.trackRevisions true - git cvsimport -a -z0 - git merge origin + git cvsimport -z0 ) test_cmp module-cvs/tick module-git/tick @@ -140,7 +138,7 @@ test_expect_success PERL 'import from a CVS working tree' ' $CVS co -d import-from-wt module (cd import-from-wt git config cvsimport.trackRevisions false - git cvsimport -a -z0 + git cvsimport -z0 echo 1 expect git log -1 --pretty=format:%s%n actual test_cmp actual expect -- 1.8.1.1.g220e17a -- 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/3] t9604: fixup for new cvsimport
--- t/t9604-cvsimport-timestamps.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t9604-cvsimport-timestamps.sh b/t/t9604-cvsimport-timestamps.sh index 1fd5142..b1629b6 100755 --- a/t/t9604-cvsimport-timestamps.sh +++ b/t/t9604-cvsimport-timestamps.sh @@ -7,8 +7,7 @@ setup_cvs_test_repository t9604 test_expect_success 'check timestamps are UTC (TZ=CST6CDT)' ' - TZ=CST6CDT git cvsimport -p-x -C module-1 module - git cvsimport -p-x -C module-1 module + TZ=CST6CDT git cvsimport -C module-1 module ( cd module-1 git log --format=%s %ai @@ -42,7 +41,7 @@ test_expect_success 'check timestamps with author-specific timezones' ' user3=User Three us...@domain.org EST5EDT user4=User Four us...@domain.org MST7MDT EOF - git cvsimport -p-x -A cvs-authors -C module-2 module + git cvsimport -A cvs-authors -C module-2 module ( cd module-2 git log --format=%s %ai %an -- 1.8.1.1.g220e17a -- 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: Turning a complete repository to a shallow one
Duy Nguyen pclo...@gmail.com writes: Apparently we could do it: git clone --single-branch git.git cd git git tag -l|xargs git tag -d git fetch --depth=1 origin master git repack -ad I may have been unclear in the earlier message, but this is one of the reasons why I think fetch --depth is misadvertised (it says deepen the history, when the real thing it does is to truncate to the new depth counting from the updated tip), and misdesigned (it forces the user to guess what the new depth should be). It should be advertised correctly, as a way to reset its depth starting from the current history (and of course --depth=1 is truncates). We should introduce a separate fetch --deepen that does what normal people would want (see my earlier message with ASCII art). -- 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 09/19] reset.c: replace switch by if-else
On Wed, Jan 9, 2013 at 11:53 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: --- builtin/reset.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 42d1563..05ccfd4 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -351,18 +351,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix) * saving the previous head in ORIG_HEAD before. */ update_ref_status = update_refs(rev, sha1); - switch (reset_type) { - case HARD: - if (!update_ref_status !quiet) - print_new_head_line(commit); - break; - case SOFT: /* Nothing else to do. */ - break; - case MIXED: /* Report what has not been updated. */ + if (reset_type == HARD !update_ref_status !quiet) + print_new_head_line(commit); + else if (reset_type == MIXED) /* Report what has not been updated. */ update_index_refresh(0, NULL, quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); - break; - } Justification? Clairvoyance -- the HARD case will soon be the only non-empty case. It's also missing KEEP and MERGE (but the empty SOFT block is there). I'll update the message. I will also move the patch a little later in the series, closer to where it will be useful. -- 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
Fwd: git checkout doesn't work?
This is my session on Win7 x64: Microsoft Windows [Version 6.1.7601] (c) Корпорация Майкрософт (Microsoft Corp.), 2009. Все права защищены. C:\Dropbox\Dropbox\Wesnoth\Apocryphscd Apokryphs.Orks C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orksgit status # On branch master # Your branch is behind 'origin/master' by 3 commits, and can be fast-forwarded. # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: scenarios/01_NothernVillage.cfg # no changes added to commit (use git add and/or git commit -a) C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orkscd scenarios C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenariosgit status # On branch master # Your branch is behind 'origin/master' by 3 commits, and can be fast-forwarded. # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: 01_NothernVillage.cfg # no changes added to commit (use git add and/or git commit -a) C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenariosgit checkout -- 01 _NothernVillage.cfg C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenariosgit status # On branch master # Your branch is behind 'origin/master' by 3 commits, and can be fast-forwarded. # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: 01_NothernVillage.cfg # no changes added to commit (use git add and/or git commit -a) C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenarios As I understand after last git checkout in git status I should see that I gave no changes. It looks like an bug, isn't it? -- сохраняйте, пожалуйста, при ответе исходный текст письма С уважением, начальник отдела технической поддержки Московского Еврейского Общинного Центра Ишаяу Ластов телефон: +7-495-645-05-16 моб : +7-901-569-81-86 с 12 до 18, с воскресенья по четверг -- 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 checkout bug on Win7 x64
This is my session on Win7 x64: Microsoft Windows [Version 6.1.7601] (c) Корпорация Майкрософт (Microsoft Corp.), 2009. Все права защищены. C:\Dropbox\Dropbox\Wesnoth\Apocryphscd Apokryphs.Orks C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orksgit status # On branch master # Your branch is behind 'origin/master' by 3 commits, and can be fast-forwarded. # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: scenarios/01_NothernVillage.cfg # no changes added to commit (use git add and/or git commit -a) C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orkscd scenarios C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenariosgit status # On branch master # Your branch is behind 'origin/master' by 3 commits, and can be fast-forwarded. # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: 01_NothernVillage.cfg # no changes added to commit (use git add and/or git commit -a) C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenariosgit checkout -- 01 _NothernVillage.cfg C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenariosgit status # On branch master # Your branch is behind 'origin/master' by 3 commits, and can be fast-forwarded. # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: 01_NothernVillage.cfg # no changes added to commit (use git add and/or git commit -a) C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenarios As I understand after last git checkout in git status I should see that I gave no changes. It looks like an bug, isn't it? -- сохраняйте, пожалуйста, при ответе исходный текст письма С уважением, начальник отдела технической поддержки Московского Еврейского Общинного Центра Ишаяу Ластов телефон: +7-495-645-05-16 моб : +7-901-569-81-86 с 12 до 18, с воскресенья по четверг -- 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