Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Jeff King
On Wed, May 21, 2014 at 05:07:38PM -0700, Kyle J. McKay wrote: +p = skip_prefix(type-buf, text/plain); +if (!p || (*p *p != ';')) +return 0; + +return 1; +} + I think that a strict reading of RFC 2616 allows text/plain ; charset=utf-8 as well as

Re: [PATCH 9/9] remote-curl: reencode http error messages

2014-05-22 Thread Jeff King
On Wed, May 21, 2014 at 05:07:40PM -0700, Kyle J. McKay wrote: +/* default charset from rfc2616 */ +if (!*charset) +*charset = xstrdup(iso8859-1); Actually the name should be ISO-8859-1. See RFC 2616 section 3.7.1. Since it's case insensitive iso-8859-1 would be fine

Re: [PATCHv2 10/19] git-submodule.sh: convert test -a/-o to and ||

2014-05-22 Thread Matthieu Moy
Elia Pinto gitter.spi...@gmail.com writes: This is version 2 of the patch to git-submodule of the patch series convert test -a/-o to and ||. It contains the fixes identified by Johannes and Matthieu. This version of the patch (not the whole series) is Reviewed-by: Matthieu Moy

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Peter Krefting
Kyle J. McKay: I think that a strict reading of RFC 2616 allows text/plain ; charset=utf-8 as well as text/plain;charset=utf-8 and text/plain; charset=utf-8. It does indeed, and I have seen servers send both variants, so they do need to be catered for. The number of servers that would

Re: [PATCH 9/9] remote-curl: reencode http error messages

2014-05-22 Thread Peter Krefting
Kyle J. McKay: + if (!*charset) + *charset = xstrdup(iso8859-1); Actually the name should be ISO-8859-1. See RFC 2616 section 3.7.1. Since it's case insensitive iso-8859-1 would be fine too. You'd be amazed at what you see in the wild... I'd recommend going with the

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Kyle J. McKay
On May 21, 2014, at 23:05, Jeff King wrote: On Wed, May 21, 2014 at 05:07:38PM -0700, Kyle J. McKay wrote: + p = skip_prefix(type-buf, text/plain); + if (!p || (*p *p != ';')) + return 0; + + return 1; +} + I think that a strict reading of RFC 2616 allows

Re: [PATCHv2 10/19] git-submodule.sh: convert test -a/-o to and ||

2014-05-22 Thread Elia Pinto
2014-05-22 8:49 GMT+02:00 Matthieu Moy matthieu@grenoble-inp.fr: Elia Pinto gitter.spi...@gmail.com writes: This is version 2 of the patch to git-submodule of the patch series convert test -a/-o to and ||. It contains the fixes identified by Johannes and Matthieu. This version of the

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 12:27:38AM -0700, Kyle J. McKay wrote: Yeah I think so too. It's probably enough though just to just strip all and \t characters at the same time the content type is lowercased. While that would cause invalid content types such as text / plain to be recognized it

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 08:12:58AM +0100, Peter Krefting wrote: Kyle J. McKay: I think that a strict reading of RFC 2616 allows text/plain ; charset=utf-8 as well as text/plain;charset=utf-8 and text/plain; charset=utf-8. It does indeed, and I have seen servers send both variants, so

Re: [PATCHv2 10/19] git-submodule.sh: convert test -a/-o to and ||

2014-05-22 Thread Johannes Sixt
Am 5/22/2014 10:38, schrieb Elia Pinto: 2014-05-22 8:49 GMT+02:00 Matthieu Moy matthieu@grenoble-inp.fr: Elia Pinto gitter.spi...@gmail.com writes: @@ -1059,13 +1059,17 @@ cmd_summary() { while read mod_src mod_dst sha1_src sha1_dst status sm_path do

[PATCH v2 0/9] handle alternate charsets for remote http errors

2014-05-22 Thread Jeff King
On Wed, May 21, 2014 at 06:25:24AM -0400, Jeff King wrote: As of commit 426e70d (remote-curl: show server content on http errors, 2013-04-05), we relay any text/plain errors shown by the remote http server to the user. However, we were lazy back then and left this TODO in place: /*

[PATCH v2 1/8] test-lib: preserve GIT_CURL_VERBOSE from the environment

2014-05-22 Thread Jeff King
Turning on this variable can be useful when debugging http tests. It does break a few tests in t5541, but it is not a variable that the user is likely to have enabled accidentally. Signed-off-by: Jeff King p...@peff.net --- t/test-lib.sh | 1 + 1 file changed, 1 insertion(+) diff --git

[PATCH v2 2/8] t/lib-httpd: use write_script to copy CGI scripts

2014-05-22 Thread Jeff King
Using write_script will set our shebang line appropriately with $SHELL_PATH. The script that is there now is quite simple and likely to succeed even with a non-POSIX /bin/sh, but it does not hurt to be defensive. Signed-off-by: Jeff King p...@peff.net --- t/lib-httpd.sh | 6

[PATCH v2 3/8] t5550: test display of remote http error messages

2014-05-22 Thread Jeff King
Since commit 426e70d (remote-curl: show server content on http errors, 2013-04-05), we relay any text/plain error messages from the remote server to the user. However, we never tested it. Signed-off-by: Jeff King p...@peff.net --- t/lib-httpd.sh | 1 + t/lib-httpd/apache.conf|

[PATCH v2 4/8] http: extract type/subtype portion of content-type

2014-05-22 Thread Jeff King
When we get a content-type from curl, we get the whole header line, including any parameters, and without any normalization (like downcasing or whitespace) applied. If we later try to match it with strcmp() or even strcasecmp(), we may get false negatives. This could cause two visible behaviors:

[PATCH v2 5/8] http: optionally extract charset parameter from content-type

2014-05-22 Thread Jeff King
Since the previous commit, we now give a sanitized, shortened version of the content-type header to any callers who ask for it. This patch adds back a way for them to cleanly access specific parameters to the type. We could easily extract all parameters and make them available via a string_list,

[PATCH v2 6/8] strbuf: add strbuf_reencode helper

2014-05-22 Thread Jeff King
This is a convenience wrapper around `reencode_string_len` and `strbuf_attach`. Signed-off-by: Jeff King p...@peff.net --- Documentation/technical/api-strbuf.txt | 5 + strbuf.c | 17 + strbuf.h | 1 + 3 files

[PATCH v2 7/8] remote-curl: reencode http error messages

2014-05-22 Thread Jeff King
We currently recognize an error message with a content-type text/plain; charset=utf-16 as text, but we ignore the charset parameter entirely. Let's encode it to log_output_encoding, which is presumably something the user's terminal can handle. Signed-off-by: Jeff King p...@peff.net ---

[PATCH v2 8/8] http: default text charset to iso-8859-1

2014-05-22 Thread Jeff King
This is specified by RFC 2616 as the default if no charset parameter is given. Signed-off-by: Jeff King p...@peff.net --- I'd prefer to do this simple, standard thing, and see how it works in the real world. We'll hand whatever we get off to iconv, and if it chokes, we'll pass through the data

[PATCH 0/2] tolower cleanups

2014-05-22 Thread Jeff King
These two patches were pulled from the http charset series I posted nearby. The second iteration of that series did not need them, but they may have value as cleanups. [1/2]: daemon/config: factor out duplicate xstrdup_tolower [2/2]: strbuf: add strbuf_tolower function The first one is a

[PATCH 1/2] daemon/config: factor out duplicate xstrdup_tolower

2014-05-22 Thread Jeff King
We have two implementations of the same function; let's drop that to one. We take the name from daemon.c, but the implementation (which is just slightly more efficient) from the config code. Signed-off-by: Jeff King p...@peff.net --- builtin/config.c | 15 +-- daemon.c | 8

[PATCH 2/2] strbuf: add strbuf_tolower function

2014-05-22 Thread Jeff King
This makes config's lowercase() function public. Note that we could continue to offer a pure-string lowercase, but there would be no callers (in most pure-string cases, we actually duplicate and lowercase the duplicate). Signed-off-by: Jeff King p...@peff.net ---

Re: [RFC/PATCH v4 3/3] add command performance tracing to debug scripted commands

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 02:40:48AM +0200, Karsten Blees wrote: E.g. if I'm interested in a particular code section, I throw in 2 lines of code (before and after the code section). This gives very accurate results, without significantly affecting overall performance. I can then push the

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Peter Krefting
Jeff King: I was really hoping to avoid getting into all of the real-world messiness that a real http client needs to deal with (as opposed to just following the standards). Yeah, I agree, you're probably fine without all this detail in over 99% of the cases where this code would ever be

[PATCH] Get rid of the non portable shell export VAR=VALUE costruct

2014-05-22 Thread Elia Pinto
Found by check-non-portable-shell.pl Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- contrib/subtree/t/t7900-subtree.sh |2 +- git-remote-testgit.sh |2 +- git-stash.sh |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git

Re: [PATCH] Get rid of the non portable shell export VAR=VALUE costruct

2014-05-22 Thread Torsten Bögershausen
On 2014-05-22 14.48, Elia Pinto wrote: Found by check-non-portable-shell.pl Thanks for picking this up -export TEST_DIRECTORY=$(pwd)/../../../t +TEST_DIRECTORY=$(pwd)/../../../t export TEST_DIRECTORY Minor remark: Both commands should go on their own line, like this:

Re: [PATCH] Get rid of the non portable shell export VAR=VALUE costruct

2014-05-22 Thread David Kastrup
Torsten Bögershausen tbo...@web.de writes: On 2014-05-22 14.48, Elia Pinto wrote: Found by check-non-portable-shell.pl Thanks for picking this up -export TEST_DIRECTORY=$(pwd)/../../../t +TEST_DIRECTORY=$(pwd)/../../../t export TEST_DIRECTORY Minor remark: Both commands should go on

Re: [PATCH 2/2] strbuf: add strbuf_tolower function

2014-05-22 Thread Jeff King
[re-adding list cc] On Thu, May 22, 2014 at 03:16:45PM +0200, Christian Couder wrote: +void strbuf_tolower(struct strbuf *sb) +{ + char *p; + for (p = sb-buf; *p; p++) + *p = tolower(*p); +} Last time I tried a change like the above, I was told that

Re: [PATCH] Get rid of the non portable shell export VAR=VALUE costruct

2014-05-22 Thread Elia Pinto
5-22 14.48, Elia Pinto wrote: Found by check-non-portable-shell.pl Thanks for picking this up -export TEST_DIRECTORY=$(pwd)/../../../t +TEST_DIRECTORY=$(pwd)/../../../t export TEST_DIRECTORY Minor remark: Both commands should go on their own line, like this:

[PATCH 1/2] completion: add a note that merge options are shared

2014-05-22 Thread John Keeping
This should avoid future confusion after a subsequent patch has added some options to __git_merge_options and some directly in _git_merge(). Signed-off-by: John Keeping j...@keeping.me.uk --- contrib/completion/git-completion.bash | 1 + 1 file changed, 1 insertion(+) diff --git

Re: [PATCH] Get rid of the non portable shell export VAR=VALUE costruct

2014-05-22 Thread Johannes Sixt
Am 5/22/2014 15:19, schrieb David Kastrup: Torsten Bögershausen tbo...@web.de writes: On 2014-05-22 14.48, Elia Pinto wrote: Found by check-non-portable-shell.pl Thanks for picking this up -export TEST_DIRECTORY=$(pwd)/../../../t +TEST_DIRECTORY=$(pwd)/../../../t export TEST_DIRECTORY

Re: [PATCH] Get rid of the non portable shell export VAR=VALUE costruct

2014-05-22 Thread David Kastrup
Johannes Sixt j.s...@viscovery.net writes: Am 5/22/2014 15:19, schrieb David Kastrup: Torsten Bögershausen tbo...@web.de writes: On 2014-05-22 14.48, Elia Pinto wrote: Found by check-non-portable-shell.pl Thanks for picking this up -export TEST_DIRECTORY=$(pwd)/../../../t

Re: [PATCH v8 35/44] refs.c: make delete_ref use a transaction

2014-05-22 Thread Ronnie Sahlberg
On Wed, May 21, 2014 at 4:22 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: --- a/refs.c +++ b/refs.c [...] @@ -2515,24 +2510,18 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err) int delete_ref(const char *refname, const unsigned

Re: [PATCH v8 36/44] refs.c: pass the ref log message to _create/delete/update instead of _commit

2014-05-22 Thread Ronnie Sahlberg
On Wed, May 21, 2014 at 4:47 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: Change the reference transactions so that we pass the reflog message through to the create/delete/update function instead of the commit message. Nice. [...] --- a/builtin/fetch.c +++

Re: [PATCH v8 38/44] refs.c: pack all refs before we start to rename a ref

2014-05-22 Thread Ronnie Sahlberg
On Wed, May 21, 2014 at 4:57 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: This means that most loose refs will no longer be present after the rename Is this to handle the git branch -m foo/bar foo case or for some other purpose? Yes. That is the main reason. [...]

git reset for index restoration?

2014-05-22 Thread David Turner
If I have a git repository with a clean working tree, and I delete the index, then I can use git reset (with no arguments) to recreate it. However, when I do recreate it, it doesn't come back the same. I have not analyzed this in detail, but the effect is that commands like git status take much

Re: [PATCH v8 35/44] refs.c: make delete_ref use a transaction

2014-05-22 Thread Ronnie Sahlberg
On Thu, May 22, 2014 at 8:32 AM, Ronnie Sahlberg sahlb...@google.com wrote: On Wed, May 21, 2014 at 4:22 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: --- a/refs.c +++ b/refs.c [...] @@ -2515,24 +2510,18 @@ static int delete_ref_loose(struct ref_lock *lock, int

Re: [PATCH 8/8] read-cache: inform the daemon that the index has been updated

2014-05-22 Thread David Turner
On Tue, 2014-05-13 at 18:15 +0700, Nguyễn Thái Ngọc Duy wrote: + if (run_command(cp)) + warning(_(failed to start read-cache--daemon: %s), + strerror(errno)); errno is not always (ever?) set, so if read-cache--daemon is missing, you

Re: git reset for index restoration?

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 12:22:43PM -0400, David Turner wrote: If I have a git repository with a clean working tree, and I delete the index, then I can use git reset (with no arguments) to recreate it. However, when I do recreate it, it doesn't come back the same. I have not analyzed this in

Re: git reset for index restoration?

2014-05-22 Thread Elijah Newren
On Thu, May 22, 2014 at 9:22 AM, David Turner dtur...@twopensource.com wrote: If I have a git repository with a clean working tree, and I delete the index, then I can use git reset (with no arguments) to recreate it. However, when I do recreate it, it doesn't come back the same. I have not

Re: [PATCH v8 32/44] refs.c: remove the update_ref_write function

2014-05-22 Thread Ronnie Sahlberg
On Wed, May 21, 2014 at 3:07 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: +++ b/refs.c [...] @@ -3518,14 +3499,16 @@ int ref_transaction_commit(struct ref_transaction *transaction, struct ref_update *update = updates[i]; if

Re: [PATCH v8 34/44] refs.c: make prune_ref use a transaction to delete the ref

2014-05-22 Thread Ronnie Sahlberg
Added a comment that any flags =0x100 are reserved for internal use. On Wed, May 21, 2014 at 4:01 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: Change prune_ref to delete the ref using a ref transaction. To do this we also need to add a new flag REF_ISPRUNING that

Re: [PATCH v8 30/44] refs.c: add transaction.status and track OPEN/CLOSED/ERROR

2014-05-22 Thread Ronnie Sahlberg
On Wed, May 21, 2014 at 3:22 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: Please pull my ref-transactions branch. I'm at bd5736cb (2014-05-21 13:46) now. On Wed, May 21, 2014 at 3:00 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: --- a/refs.c

Re: [PATCH v8 39/44] refs.c: move the check for valid refname to lock_ref_sha1_basic

2014-05-22 Thread Ronnie Sahlberg
On Wed, May 21, 2014 at 6:42 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: --- a/refs.c +++ b/refs.c @@ -2044,6 +2044,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, int missing = 0; int attempts_remaining = 3; + if

Re: [PATCH v8 39/44] refs.c: move the check for valid refname to lock_ref_sha1_basic

2014-05-22 Thread Jonathan Nieder
Ronnie Sahlberg wrote: On Wed, May 21, 2014 at 6:42 PM, Jonathan Nieder jrnie...@gmail.com wrote: $ git rev-parse HEAD .git/refs/heads/foo..bar $ git branch -m foo..bar something-saner fatal: Invalid branch name: 'foo..bar' git branch -m has an explicit codepath

Re: [PATCH v8 40/44] refs.c: call lock_ref_sha1_basic directly from commit

2014-05-22 Thread Jonathan Nieder
Ronnie Sahlberg wrote: Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to

Re: [PATCH v8 39/44] refs.c: move the check for valid refname to lock_ref_sha1_basic

2014-05-22 Thread Ronnie Sahlberg
On Thu, May 22, 2014 at 10:44 AM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: On Wed, May 21, 2014 at 6:42 PM, Jonathan Nieder jrnie...@gmail.com wrote: $ git rev-parse HEAD .git/refs/heads/foo..bar $ git branch -m foo..bar something-saner fatal:

Re: [PATCH v8 38/44] refs.c: pack all refs before we start to rename a ref

2014-05-22 Thread Ronnie Sahlberg
On Thu, May 22, 2014 at 10:51 AM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: On Wed, May 21, 2014 at 4:57 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: This means that most loose refs will no longer be present after the rename Is this to handle

Re: git reset for index restoration?

2014-05-22 Thread David Turner
On Thu, 2014-05-22 at 12:46 -0400, Jeff King wrote: On Thu, May 22, 2014 at 12:22:43PM -0400, David Turner wrote: If I have a git repository with a clean working tree, and I delete the index, then I can use git reset (with no arguments) to recreate it. However, when I do recreate it, it

Re: git reset for index restoration?

2014-05-22 Thread David Turner
On Thu, 2014-05-22 at 09:46 -0700, Elijah Newren wrote: On Thu, May 22, 2014 at 9:22 AM, David Turner dtur...@twopensource.com wrote: If I have a git repository with a clean working tree, and I delete the index, then I can use git reset (with no arguments) to recreate it. However, when I

Re: [PATCH v8 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only

2014-05-22 Thread Jonathan Nieder
Hi, Ronnie Sahlberg wrote: Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete. This flag indicates that the ref does not exist as a loose ref andf only as a packed ref. If this is the case we then change the commit code so that we skip taking out a lock file and we skip

Re: git reset for index restoration?

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 02:08:16PM -0400, David Turner wrote: On Thu, 2014-05-22 at 12:46 -0400, Jeff King wrote: On Thu, May 22, 2014 at 12:22:43PM -0400, David Turner wrote: If I have a git repository with a clean working tree, and I delete the index, then I can use git reset (with

Re: [PATCH 2/9] strbuf: add strbuf_tolower function

2014-05-22 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Wed, May 21, 2014 at 05:07:36PM -0700, Kyle J. McKay wrote: +void strbuf_tolower(struct strbuf *sb) +{ + size_t i; + for (i = 0; i sb-len; i++) + sb-buf[i] = tolower(sb-buf[i]); +} + Wouldn't a direct transfer of the lowercase function

Re: git reset for index restoration?

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 02:17:22PM -0400, David Turner wrote: In fact, git status does not write the index (at least in this context). And what is slow is not (only) checking over the working tree, but reading the packs. There should be no need to read files from the ODB at all, since the

Re: [PATCH 2/9] strbuf: add strbuf_tolower function

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 11:36:37AM -0700, Junio C Hamano wrote: Yes, and that would be fine with me (I actually wrote strbuf_tolower for my own use, and _then_ realized that we already had such a thing that could be replaced). Do we forbid that sb-buf[x] for some x sb-len to be NUL, and

Re: [PATCH v8 0/2] format-patch --signature-file file

2014-05-22 Thread Junio C Hamano
Jeremiah Mahler jmmah...@gmail.com writes: I just notice that my patch is in 'pu'. But it is version 7 instead of the improved version 8. Yeah, I know. In a distributed environment, multiple people work independently and a sequence of event can go like this: - I read v7, comment, and queue

Re: git reset for index restoration?

2014-05-22 Thread David Turner
On Thu, 2014-05-22 at 14:39 -0400, Jeff King wrote: does show some improvement. Perhaps git reset is not writing out the cache-tree extension? Yes, that seems to be exactly what is going on; the two indexes are identical up to the point where the TREE extension appears. Thanks for clearing

Re: git reset for index restoration?

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 03:07:49PM -0400, David Turner wrote: On Thu, 2014-05-22 at 14:39 -0400, Jeff King wrote: does show some improvement. Perhaps git reset is not writing out the cache-tree extension? Yes, that seems to be exactly what is going on; the two indexes are identical up

Re: [PATCH v8 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only

2014-05-22 Thread Ronnie Sahlberg
On Thu, May 22, 2014 at 11:17 AM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Ronnie Sahlberg wrote: Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete. This flag indicates that the ref does not exist as a loose ref andf only as a packed ref. If this is the case we

Re: git reset for index restoration?

2014-05-22 Thread David Turner
On Thu, 2014-05-22 at 14:23 -0400, Jeff King wrote: On Thu, May 22, 2014 at 02:08:16PM -0400, David Turner wrote: On Thu, 2014-05-22 at 12:46 -0400, Jeff King wrote: On Thu, May 22, 2014 at 12:22:43PM -0400, David Turner wrote: If I have a git repository with a clean working tree,

Re: [PATCH v8 42/44] refs.c: pass a skip list to name_conflict_fn

2014-05-22 Thread Jonathan Nieder
Ronnie Sahlberg wrote: --- a/refs.c +++ b/refs.c @@ -798,11 +798,19 @@ struct name_conflict_cb { const char *refname; const char *oldrefname; const char *conflicting_refname; + const char **skip; + int skipnum; Would a struct string_list make sense here? (See

Re: git reset for index restoration?

2014-05-22 Thread Jeff King
[+cc Junio for cache-tree expertise] On Thu, May 22, 2014 at 03:09:59PM -0400, Jeff King wrote: does show some improvement. Perhaps git reset is not writing out the cache-tree extension? [...] Possibly. There is a call to prime_cache_tree in builtin/reset.c, which looks like it should

Re: [PATCH v8 0/2] format-patch --signature-file file

2014-05-22 Thread Jeremiah Mahler
Junio, On Thu, May 22, 2014 at 12:00:39PM -0700, Junio C Hamano wrote: Jeremiah Mahler jmmah...@gmail.com writes: I just notice that my patch is in 'pu'. But it is version 7 instead of the improved version 8. Yeah, I know. In a distributed environment, multiple people work

Re: [PATCH v8 00/44] Use ref transactions for all ref updates

2014-05-22 Thread Jonathan Nieder
Ronnie Sahlberg wrote: This version completes the work to convert all ref updates to use transactions. Finally got through this. It had thorny bits but generally goes in a very good direction. Thanks for a pleasant read. Feel free to send another iteration if you'd like review for the

Re: [PATCH v8 00/44] Use ref transactions for all ref updates

2014-05-22 Thread Jonathan Nieder
Ronnie Sahlberg wrote: This patch series can also be found at https://github.com/rsahlberg/git/tree/ref-transactions Thoughts on 65a1cb7b (2014-05-22 12:08): 01/40 remove ref_transaction_rollback Reviewed-by: Jonathan Nieder jrnie...@gmail.com 02/40 constify the sha arguments for

[PATCH v2] Add an explicit GIT_DIR to the list of excludes

2014-05-22 Thread Pasha Bolokhov
When an explicit '--git-dir' option points to a directory inside the work tree, git treats it as if it were any other directory. In particular, 'git status' lists it as untracked, while 'git add -A' stages the metadata directory entirely Add GIT_DIR to the list of excludes in

Re: [PATCH v8 2/2] format-patch --signature-file file

2014-05-22 Thread Junio C Hamano
Jeremiah Mahler jmmah...@gmail.com writes: Added option that allows a signature file to be used with format-patch so that signatures with newlines and other special characters can be easily included. s/Added option/Add an option/. I do not think with newlines and other special characters is

Re: [PATCH 2/9] strbuf: add strbuf_tolower function

2014-05-22 Thread Junio C Hamano
Jeff King p...@peff.net writes: Yes, and that would be fine with me (I actually wrote strbuf_tolower for my own use, and _then_ realized that we already had such a thing that could be replaced). ... ... I think the bigger question is: is this refactor worth doing, since there is only

Re: git reset for index restoration?

2014-05-22 Thread Junio C Hamano
Jeff King p...@peff.net writes: [+cc Junio for cache-tree expertise] ... We never call reset_index now, because we handle it via diff. We could call prime_cache_tree in this case, but I'm not sure if that is a good idea, because it primes it from scratch (and so it opens up all those trees

Re: git reset for index restoration?

2014-05-22 Thread David Turner
On Thu, 2014-05-22 at 14:34 -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: [+cc Junio for cache-tree expertise] ... We never call reset_index now, because we handle it via diff. We could call prime_cache_tree in this case, but I'm not sure if that is a good idea,

Re: git reset for index restoration?

2014-05-22 Thread Junio C Hamano
David Turner dtur...@twopensource.com writes: On Thu, 2014-05-22 at 14:34 -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: [+cc Junio for cache-tree expertise] ... We never call reset_index now, because we handle it via diff. We could call prime_cache_tree in this case,

Re: [PATCH v8 00/44] Use ref transactions for all ref updates

2014-05-22 Thread Jonathan Nieder
Ronnie Sahlberg wrote: This patch series can also be found at https://github.com/rsahlberg/git/tree/ref-transactions Thoughts on 65a1cb7b (2014-05-22 12:08): 04/40 add a strbuf argument to ref_transaction_commit for error logging Ideally this would come after the functions it calls so the

Re: git reset for index restoration?

2014-05-22 Thread Junio C Hamano
David Turner dtur...@twopensource.com writes: Yes. As I said, that should not usually be a problem for those who do the real work (read: commit), at which time write-tree will fully populate the cache-tree. Git commit does not in fact populate the cache-tree. If that is the case, we must

Re: git reset for index restoration?

2014-05-22 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: David Turner dtur...@twopensource.com writes: Yes. As I said, that should not usually be a problem for those who do the real work (read: commit), at which time write-tree will fully populate the cache-tree. Git commit does not in fact populate the

Re: git reset for index restoration?

2014-05-22 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: But at least my understanding has been that git commit (no partial commit, write the whole index as a commit) which uses the git write-tree machinery knows which subtree has what tree object name and populates the cache-tree fully. Here is what I

Re: [PATCH 2/9] strbuf: add strbuf_tolower function

2014-05-22 Thread Kyle J. McKay
On May 22, 2014, at 11:41, Jeff King wrote: On Thu, May 22, 2014 at 11:36:37AM -0700, Junio C Hamano wrote: Yes, and that would be fine with me (I actually wrote strbuf_tolower for my own use, and _then_ realized that we already had such a thing that could be replaced). Do we forbid

Re: [PATCH v2 4/8] http: extract type/subtype portion of content-type

2014-05-22 Thread Kyle J. McKay
On May 22, 2014, at 02:29, Jeff King wrote: When we get a content-type from curl, we get the whole header line, including any parameters, and without any normalization (like downcasing or whitespace) applied. If we later try to match it with strcmp() or even strcasecmp(), we may get false

Re: [PATCH v8 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only

2014-05-22 Thread Ronnie Sahlberg
I hate rename_ref :-) I have reworked the transaction code to special case the deletion of the old ref for n/n - n and n - n/n renames so that we can carefully avoid n/n.lock files to exist or prevent the directory - file transition for n during these renames. This should allow us to have

Re: git reset for index restoration?

2014-05-22 Thread David Turner
On Thu, 2014-05-22 at 15:29 -0700, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: But at least my understanding has been that git commit (no partial commit, write the whole index as a commit) which uses the git write-tree machinery knows which subtree has what tree object

Re: [PATCH v8 00/44] Use ref transactions for all ref updates

2014-05-22 Thread Jonathan Nieder
Ronnie Sahlberg wrote: This patch series can also be found at https://github.com/rsahlberg/git/tree/ref-transactions Continuing with the review of 65a1cb7b (2014-05-22 12:08): 11/40 change ref_transaction_update() to do error checking and return status The there will be in the future sounds

Re: git reset for index restoration?

2014-05-22 Thread Junio C Hamano
David Turner dtur...@twopensource.com writes: ... I still believe that the cache-tree behavior would be suboptimal, ... I do not think anybody doubts that suboptimal-ness in this thread. As you saw the incremental thing from Peff and my responses to it, there may be more things we could be

Re: [PATCH 2/2] strbuf: add strbuf_tolower function

2014-05-22 Thread Kyle J. McKay
On May 22, 2014, at 06:42, Jeff King wrote: [re-adding list cc] On Thu, May 22, 2014 at 03:16:45PM +0200, Christian Couder wrote: +void strbuf_tolower(struct strbuf *sb) +{ + char *p; + for (p = sb-buf; *p; p++) + *p = tolower(*p); +} Last time I tried a change

Re: git reset for index restoration?

2014-05-22 Thread Duy Nguyen
On Fri, May 23, 2014 at 5:18 AM, Junio C Hamano gits...@pobox.com wrote: ... and the incrementally repair Peff talks about would be to cover more cases where we may know (either because we have already computed it to write out a subtree, or we have just read from a known tree to populate a

Re: git reset for index restoration?

2014-05-22 Thread David Turner
On Fri, 2014-05-23 at 06:33 +0700, Duy Nguyen wrote: On Fri, May 23, 2014 at 5:18 AM, Junio C Hamano gits...@pobox.com wrote: ... and the incrementally repair Peff talks about would be to cover more cases where we may know (either because we have already computed it to write out a subtree,

Re: [PATCH] Get rid of the non portable shell export VAR=VALUE costruct

2014-05-22 Thread Junio C Hamano
Elia Pinto gitter.spi...@gmail.com writes: I have no problems rerolling this simple patch, but i need to know what is the (git) right style in this case. If I were doing this... diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index 66ce4b0..c1d0b23

Re: [PATCH v8 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only

2014-05-22 Thread Jonathan Nieder
Ronnie Sahlberg wrote: I hate rename_ref :-) I have reworked the transaction code to special case the deletion of the old ref for n/n - n and n - n/n renames so that we can carefully avoid n/n.lock files to exist or prevent the directory - file transition for n during these renames.

Re: [PATCH v8 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only

2014-05-22 Thread Jonathan Nieder
Jonathan Nieder wrote: Ronnie Sahlberg wrote: I hate rename_ref :-) I have reworked the transaction code to special case the deletion of the old ref for n/n - n and n - n/n renames so that we can carefully avoid n/n.lock files to exist or prevent the directory - file transition for n

Re: [PATCH 2/2] completion: add missing options for git-merge

2014-05-22 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes: The options added to __git_merge_options are those that git-pull also understands, since that variable is used by both commands. Those added directly in _git_merge() are specific to git-merge and are not supported by git-pull. Interesting.

[no subject]

2014-05-22 Thread Mrs. Jiang Ming
-- Compliment of the day, I am Mrs. Jiang Ming, a staff of Lloyds TSB Group Plc. here in Hong Kong attached with Private Banking Services;I have a secured business proposal for you. Should you be interested please reach me on my private emailaddress (mrsjiangming1...@outlook.com) And after

Re: [PATCH v2 8/8] http: default text charset to iso-8859-1

2014-05-22 Thread brian m. carlson
On Thu, May 22, 2014 at 05:36:12AM -0400, Jeff King wrote: If we do want to do magic like latin1 is really iso-8859-1, that seems like the domain of iconv to me. If iconv doesn't handle it itself, I'd rather have a wrapper there. Putting it at that layer keeps the code cleaner, and it means

Re: [PATCH v2] Add an explicit GIT_DIR to the list of excludes

2014-05-22 Thread Eric Sunshine
On Thu, May 22, 2014 at 4:11 PM, Pasha Bolokhov pasha.bolok...@gmail.com wrote: diff --git a/t/t2205-add-gitdir.sh b/t/t2205-add-gitdir.sh new file mode 100755 index 000..3c6b853 --- /dev/null +++ b/t/t2205-add-gitdir.sh @@ -0,0 +1,61 @@ +#!/bin/sh +# +# Copyright (c) 2014 Pasha

[PATCH v1 1/3] replace: add --graft option

2014-05-22 Thread Christian Couder
The usage string for this option is: git replace [-f] --graft commit [parent...] First we create a new commit that is the same as commit except that its parents are [parents...] Then we create a replace ref that replace commit with the commit we just created. With this new option, it should be

[PATCH v1 0/3] Add --graft option to git replace

2014-05-22 Thread Christian Couder
Here is a small patch series to implement: git replace [-f] --graft commit [parent...] The changes since the RFC/PATCH are the following: - in patch 1/3, parse_commit_buffer() is now used to make sure commit is not corrupt - patch 2/3 add some tests - patch 3/3 add some documentation

[PATCH v1 2/3] replace: add test for --graft

2014-05-22 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..ca45a84 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -351,4 +351,16 @@