Re: [PATCH RFC] git-am: support any number of signatures

2014-06-18 Thread Junio C Hamano
On Tue, Jun 17, 2014 at 8:09 PM, Michael S. Tsirkin m...@redhat.com wrote: OK, after looking into this for a while, I realize this is a special property of the Signed-off-by footer. For now I think it's reasonable to just avoid de-duplicating other footers if any. Agree? Not really. I'd

Re: [PATCH RFC] git-am: support any number of signatures

2014-06-18 Thread Michael S. Tsirkin
On Tue, Jun 17, 2014 at 11:49:11PM -0700, Junio C Hamano wrote: On Tue, Jun 17, 2014 at 8:09 PM, Michael S. Tsirkin m...@redhat.com wrote: OK, after looking into this for a while, I realize this is a special property of the Signed-off-by footer. For now I think it's reasonable to just

Re: Why does gpg.program work for commit but not log?

2014-06-18 Thread Jeff King
On Wed, Jun 18, 2014 at 12:18:35AM -0400, Jason Pyeron wrote: jpyeron@black /projects/microsoft-smartcard-sign/tmp $ git --version git version 1.7.9 That's rather old. In the meantime we have: commit 6005dbb9bf21d10b209f7924e305bd04b9ab56d2 Author: Jacob Sarvis

Re: [PATCH v4] http: fix charset detection of extract_content_type()

2014-06-18 Thread Jeff King
On Wed, Jun 18, 2014 at 07:11:53AM +0900, Yi EungJun wrote: From: Yi EungJun eungjun...@navercorp.com extract_content_type() could not extract a charset parameter if the parameter is not the first one and there is a whitespace and a following semicolon just before the parameter. For

Re: [PATCH v2 3/9] fetch doc: update note on '+' in front of the refspec

2014-06-18 Thread Michael Haggerty
On 06/04/2014 12:16 AM, Junio C Hamano wrote: While it is not *wrong* per-se to say that pulling a rewound/rebased branch will lead to an unnecessary merge conflict, that is not what the leading + sign to allow non-fast-forward update of remote-tracking branch is at all. Helped-by: Marc

Re: progit build instructions and ExtUtils::MakeMaker

2014-06-18 Thread Jeff King
[I added a subject to the thread; that is likely part of why you didn't get any responses sooner] On Sun, Jun 01, 2014 at 02:24:54PM -0700, C. Benson Manica wrote: The documentation for installing git from source here, http://git-scm.com/book/en/Getting-Started-Installing-Git, incorrectly

Re: [puzzled and solved] shortlog not quite understanding all log options

2014-06-18 Thread Jeff King
On Fri, May 30, 2014 at 02:37:02PM -0700, Junio C Hamano wrote: I am slightly puzzled why parse_revision_opt does not just call handle_revision_pseudo_opt. According to f6aca0dc4, it is because pseudo-options need to be acted on in-order, as they affect things like subsequent --not

Re: [PATCH v2 0/3] add strnncmp() function

2014-06-18 Thread Ondřej Bílka
On Tue, Jun 17, 2014 at 01:08:05PM +0200, Torsten Bögershausen wrote: On 2014-06-17 09.34, Jeremiah Mahler wrote: Add a strnncmp() function which behaves like strncmp() except it takes the length of both strings instead of just one. Then simplify tree-walk.c and unpack-trees.c using this

cherry-pick lost

2014-06-18 Thread guangai . che
Hi: I delete a file and push to master branch, after code reviewing in gerrit, then click 'Cherry Pick To' button to cherry-pick to release/1.1 branch, and then code review and merge... but now, the cherry-pick commit seems being lost, the should be deleted file is still on release/1.1

Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs

2014-06-18 Thread Michael Haggerty
On 06/13/2014 11:25 PM, Junio C Hamano wrote: Ronnie Sahlberg sahlb...@google.com writes: It gets even more hairy : If the server has A/a and a/b and you clone it it becomes A/a and A/b locally. Then you push back to the server and you end up with three refs on the server: A/a A/b and a/b.

[RFC] rebase --root: Empty root commit is replaced with sentinel

2014-06-18 Thread Fabian Ruch
`rebase` supports the option `--root` both with and without `--onto`. The case where `--onto` is not specified is handled by creating a sentinel commit and squashing the root commit into it. The sentinel commit refers to the empty tree and does not have a log message associated with it. Its

Re: [PATCH v2 9/9] fetch: allow explicit --refmap to override configuration

2014-06-18 Thread Michael Haggerty
On 06/05/2014 08:36 PM, Junio C Hamano wrote: Marc Branchaud marcn...@xiplink.com writes: I don't have any objection to the option per se. But I do wonder if there's a need to add yet another knob to git just for completeness. Has anyone ever needed this? It is not a good yardstick, as

RE: Why does gpg.program work for commit but not log?

2014-06-18 Thread Jason Pyeron
-Original Message- From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf Of Jeff King Sent: Wednesday, June 18, 2014 3:37 To: Jason Pyeron Cc: git@vger.kernel.org Subject: Re: Why does gpg.program work for commit but not log? On Wed, Jun 18, 2014 at

Re: Why does gpg.program work for commit but not log?

2014-06-18 Thread Jeff King
On Wed, Jun 18, 2014 at 08:38:32AM -0400, Jason Pyeron wrote: That's rather old. In the meantime we have: commit 6005dbb9bf21d10b209f7924e305bd04b9ab56d2 [...] I will (try to) compile master and test. This is the latest version in cygwin. To save you some trouble, I actually found

Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs

2014-06-18 Thread Ronnie Sahlberg
On Wed, Jun 18, 2014 at 4:33 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 06/13/2014 11:25 PM, Junio C Hamano wrote: Ronnie Sahlberg sahlb...@google.com writes: It gets even more hairy : If the server has A/a and a/b and you clone it it becomes A/a and A/b locally. Then you push back

Re: [PATCH v5 10/11] trace: add trace_performance facility to debug performance issues

2014-06-18 Thread Karsten Blees
Am 17.06.2014 19:11, schrieb Junio C Hamano: Karsten Blees karsten.bl...@gmail.com writes: Simple use case (measure one code section): uint64_t start = getnanotime(); /* code section to measure */ trace_performance_since(start, foobar); Medium use case (measure consecutive code

Re: [PATCH v5 09/11] trace: add high resolution timer function to debug performance issues

2014-06-18 Thread Karsten Blees
Am 17.06.2014 18:44, schrieb Junio C Hamano: Karsten Blees karsten.bl...@gmail.com writes: Am 11.06.2014 10:01, schrieb Karsten Blees: the epoch allows using the results (div 10e9) with other time-related APIs. s/10e9/1e9/ That replacement is fine but the (div 1e9) still wants to be

Re: [PATCH v5 00/11] add performance tracing facility

2014-06-18 Thread Karsten Blees
Am 12.06.2014 20:30, schrieb Junio C Hamano: Karsten Blees karsten.bl...@gmail.com writes: Here's v5 of the performance tracing patch series, now including a bunch of cleanups and adding timestamp, file and line to all trace output. I'm particularly interested in feedback for the output

Possible bug in `git reset` in 1.9

2014-06-18 Thread James Coleman
Suppose I have the following branches: * branch-1 with commits A - B - C * branch-2 with commits A - B - C - D Prior to version 1.9, running `git reset --hard D` while branch-1 is checked out will result in changing the current branch HEAD to commit hash D (essentially what update-ref would do).

[PATCH v3 08/14] refs.c: add a flag to allow reflog updates to truncate the log

2014-06-18 Thread Ronnie Sahlberg
Add a flag that allows us to truncate the reflog before we write the update. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 17 +++-- refs.h | 10 +- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/refs.c b/refs.c index d673a0f..c33d19e 100644

[PATCH v3 05/14] refs.c: add a function to append a reflog entry to a fd

2014-06-18 Thread Ronnie Sahlberg
Break out the code to create the string and writing it to the file descriptor from log_ref_write and into a dedicated function log_ref_write_fd. For now this is only used from log_ref_write but later on we will call this function from reflog transactions too which means that we will end up with

[PATCH v3 02/14] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update

2014-06-18 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 17 + refs.h | 2 +- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/refs.c b/refs.c index a9f91ab..0eace70 100644 --- a/refs.c +++ b/refs.c @@ -3503,24 +3503,17 @@ int ref_transaction_delete(struct

[PATCH v3 14/14] refs.c: remove lock_any_ref_for_update

2014-06-18 Thread Ronnie Sahlberg
No one is using this function so we can delete it. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 7 --- refs.h | 10 +- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/refs.c b/refs.c index ff98682..95c3eb8 100644 --- a/refs.c +++ b/refs.c @@

[PATCH v3 12/14] refs.c: rename log_ref_setup to create_reflog

2014-06-18 Thread Ronnie Sahlberg
log_ref_setup is used to do several semi-related things : * sometimes it will create a new reflog including missing parent directories and cleaning up any conflicting stale directories in the path. * fill in a filename buffer for the full path to the reflog. * unconditionally re-adjust the

[PATCH v3 10/14] refs.c: allow multiple reflog updates during a single transaction

2014-06-18 Thread Ronnie Sahlberg
Allow to make multiple reflog updates to the same ref during a transaction. This means we only need to lock the reflog once, during the first update that touches the reflog, and that all further updates can just write the reflog entry since the reflog is already locked. This allows us to write

[PATCH v3 09/14] refs.c: only write reflog update if msg is non-NULL

2014-06-18 Thread Ronnie Sahlberg
When performing a reflog transaction update, only write to the reflog iff msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform an update that only truncates but does not write. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 5 +++-- refs.h | 1 + 2 files

[PATCH v3 03/14] refs.c: rename the transaction functions

2014-06-18 Thread Ronnie Sahlberg
Rename the transaction functions. Remove the leading ref_ from the names and append _sha1 to the names for functions that create/delete/ update sha1 refs. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 11 --- builtin/commit.c | 14 -

[PATCH v3 13/14] refs.c: make unlock_ref/close_ref/commit_ref static

2014-06-18 Thread Ronnie Sahlberg
unlock|close|commit_ref can be made static since there are no more external callers. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 24 refs.h | 9 - 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/refs.c b/refs.c index

[PATCH v3 04/14] refs.c: add a new update_type field to ref_update

2014-06-18 Thread Ronnie Sahlberg
Add a field that describes what type of update this refers to. For now the only type is UPDATE_SHA1 but we will soon add more types. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 25 + 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/refs.c

[PATCH v3 11/14] reflog.c: use a reflog transaction when writing during expire

2014-06-18 Thread Ronnie Sahlberg
Use a transaction for all updates during expire_reflog. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/reflog.c | 84 refs.c | 4 +-- refs.h | 2 +- 3 files changed, 39 insertions(+), 51 deletions(-)

[PATCH v3 00/14] ref-transactions-reflog

2014-06-18 Thread Ronnie Sahlberg
These patches can also be found at: https://github.com/rsahlberg/git/tree/ref-transactions-reflog This series is based on, and applies ontop of, the previous 48 patch long ref-transaction series that is now in origin/pu. This series introduces support for reflog updates to the transaction

[PATCH v3 07/14] refs.c: add a transaction function to append a reflog entry

2014-06-18 Thread Ronnie Sahlberg
Define a new transaction update type, UPDATE_LOG, and a new function transaction_update_reflog. This function will lock the reflog and append an entry to it during transaction commit. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 101

[PATCH v3 01/14] refs.c make ref_transaction_create a wrapper to ref_transaction_update

2014-06-18 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 13 ++--- refs.h | 7 --- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/refs.c b/refs.c index dfbf003..a9f91ab 100644 --- a/refs.c +++ b/refs.c @@ -3487,23 +3487,14 @@ int ref_transaction_create(struct

[PATCH v3 06/14] lockfile.c: make hold_lock_file_for_append preserve meaningful errno

2014-06-18 Thread Ronnie Sahlberg
Update hold_lock_file_for_append and copy_fd to return a meaningful errno on failure. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- copy.c | 20 +--- lockfile.c | 7 ++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/copy.c b/copy.c index

RE: Why does gpg.program work for commit but not log?

2014-06-18 Thread Jason Pyeron
-- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- - - - Jason Pyeron PD Inc. http://www.pdinc.us - - Principal Consultant 10 West 24th Street #100- - +1 (443) 269-1555 x333

[PATCH v3 2/5] refs.c: return error instead of dying when locking fails during transaction

2014-06-18 Thread Ronnie Sahlberg
Change lock_ref_sha1_basic to return an error instead of dying when we fail to lock a file during a transaction. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 11dcb07..a644e7c 100644 ---

[PATCH v3 4/5] refs.c: update rename_ref to use a transaction

2014-06-18 Thread Ronnie Sahlberg
Change refs.c to use a single transaction to copy/rename both the refs and its reflog. Since we are no longer using rename() to move the reflog file we no longer need to disallow rename_ref for refs with a symlink for its reflog so we can remove that test from the testsuite. Change the function

[PATCH v3 3/5] refs.c: use packed refs when deleting refs during a transaction

2014-06-18 Thread Ronnie Sahlberg
Make the deletion of refs during a transaction more atomic. Start by first copying all loose refs we will be deleting to the packed refs file and then commit the packed refs file. Then re-lock the packed refs file to avoid anyone else from modifying the refs we are to delete during this

[PATCH v3 0/5] ref-transactions-rename

2014-06-18 Thread Ronnie Sahlberg
These patches can also be found at: https://github.com/rsahlberg/git/tree/ref-transactions-rename This series is based on, and applies ontop of, the previous ref-transactions-reflog series, also found at my githup repo. This series updates the reflog handling and converts rename_ref to use a

[PATCH v3 1/5] refs.c: allow passing raw git_committer_info as email to _update_reflog

2014-06-18 Thread Ronnie Sahlberg
In many places in the code we do not have access to the individual fields in the committer data. Instead we might only have access to prebaked data such as what is returned by git_committer_info() containing a string that consists of email, timestamp, zone etc. This makes it inconvenient to use

Re: Possible bug in `gitreset` in 1.9

2014-06-18 Thread James Coleman
Followup on this, it looks like the local repository actually didn't contain branch-2. So this doesn't appear to be an issue. -- 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

What's cooking in git.git (Jun 2014, #04; Tue, 17)

2014-06-18 Thread Junio C Hamano
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Many topics that have been cooking in 'next' during the previous cycle, totalling close to 300 individual patches, are in 'master' now. We

Re: [PATCH 0/7] Second part of msysgit/unicode

2014-06-18 Thread Junio C Hamano
Stepan Kasal ka...@ucw.cz writes: Hello Karsten, On Tue, Jun 17, 2014 at 11:06:52AM +0200, Karsten Blees wrote: Am 11.06.2014 11:37, schrieb Stepan Kasal: This is the second part of the time-proven unicode suport branch from msysgit. This batch is a collection of small independent

Re: [PATCH v5 10/11] trace: add trace_performance facility to debug performance issues

2014-06-18 Thread Junio C Hamano
Karsten Blees karsten.bl...@gmail.com writes: Right, it makes no sense for trace_performance(), and for trace_performance_since() only if followed by another 'measured' code section. In that special case, I think it wouldn't hurt if you had to write: uint64_t start = getnanotime(); /*

[PATCH v3 5/5] refs.c: rollback the lockfile before we die() in repack_without_refs

2014-06-18 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 89e5bc0..582591b 100644 --- a/refs.c +++ b/refs.c @@ -2548,8 +2548,10 @@ int repack_without_refs(const char **refnames, int n, struct strbuf

Re: [PATCH RFC] git-am: support any number of signatures

2014-06-18 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: On Tue, Jun 17, 2014 at 8:09 PM, Michael S. Tsirkin m...@redhat.com wrote: OK, after looking into this for a while, I realize this is a special property of the Signed-off-by footer. For now I think it's reasonable to just avoid de-duplicating other

Re: cherry-pick lost

2014-06-18 Thread Junio C Hamano
guangai@travelzen.com writes: I delete a file and push to master branch, after code reviewing in gerrit, then click 'Cherry Pick To' button to cherry-pick to release/1.1 branch, and then code review and merge... Gerrit folks, for which this list may not be the best place to get in

Re: [PATCH RFC] git-am: support any number of signatures

2014-06-18 Thread Michael S. Tsirkin
On Wed, Jun 18, 2014 at 10:51:04AM -0700, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: On Tue, Jun 17, 2014 at 8:09 PM, Michael S. Tsirkin m...@redhat.com wrote: OK, after looking into this for a while, I realize this is a special property of the Signed-off-by footer.

[PATCH v3 0/5] cleanup duplicate name_compare() functions

2014-06-18 Thread Jeremiah Mahler
Version 3 of the patch series to cleanup duplicate name_compare() functions (previously was 'add strnncmp() function' [1]). This version goes in a slightly different direction than the previous version. Before I was trying to add a strnncmp() function so I could remove duplicate copies of the

[PATCH v3 1/5] cache: rename cache_name_compare() to name_compare()

2014-06-18 Thread Jeremiah Mahler
The cache_name_compare() function is not specific to a cache. Make its name more general by renaming it to name_compare(). Simplify cache_name_stage_compare() via name_compare(). Where lengths are involved, change int to size_t. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- cache.h

[PATCH v3 2/5] tree-walk.c: remove name_compare() function

2014-06-18 Thread Jeremiah Mahler
Remove the duplicate name_compare() function and use the one provided by read-cache.c. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- Notes: There is one small difference between the old function and the new one. The old one returned -N and +N whereas the new one returns -1 and

[PATCH v3 5/5] name-hash.c: rename to name_compare()

2014-06-18 Thread Jeremiah Mahler
Rename the call to cache_name_compare() to name_compare(). Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- name-hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/name-hash.c b/name-hash.c index be7c4ae..e2bea88 100644 --- a/name-hash.c +++ b/name-hash.c @@ -179,7

[PATCH v3 4/5] dir.c: rename to name_compare()

2014-06-18 Thread Jeremiah Mahler
Rename the call to cache_name_compare() to name_compare(). Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- Notes: This is a case where cache_name_compare() was used even though it had nothing to do with a cache. The new name makes it clear that no cache is involved. dir.c |

[PATCH v3 3/5] unpack-trees.c: remove name_compare() function

2014-06-18 Thread Jeremiah Mahler
Remove the duplicate name_compare() function and use the one provided by read-cache.c. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- Notes: There is one small difference between the old function and the new one. The old one returned -N and +N whereas the new one returns -1 and

Re: [PATCH v3 2/5] tree-walk.c: remove name_compare() function

2014-06-18 Thread Jonathan Nieder
Jeremiah Mahler wrote: Remove the duplicate name_compare() function and use the one provided by read-cache.c. I'd squash this into patch 1/5. --- Notes: There is one small difference between the old function and the new one. The old one returned -N and +N whereas the new one

Re: [PATCH v3 3/5] unpack-trees.c: remove name_compare() function

2014-06-18 Thread Jonathan Nieder
Jeremiah Mahler wrote: unpack-trees.c | 11 --- 1 file changed, 11 deletions(-) Same thoughts as patch 2/5. :) Thanks, 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

Re: [PATCH v3 4/5] dir.c: rename to name_compare()

2014-06-18 Thread Jonathan Nieder
Jeremiah Mahler wrote: This is a case where cache_name_compare() was used even though it had nothing to do with a cache. The new name makes it clear that no cache is involved. That's a perfect sort of thing to put in the commit message. ;-) Unlike patches 2 and 3, this could

Re: [PATCH v3 5/5] name-hash.c: rename to name_compare()

2014-06-18 Thread Jonathan Nieder
Jeremiah Mahler wrote: name-hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Same thoughts as patch 4/5. -- 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

Re: [PATCH v3 1/5] cache: rename cache_name_compare() to name_compare()

2014-06-18 Thread Jonathan Nieder
Jeremiah Mahler wrote: The cache_name_compare() function is not specific to a cache. Make its name more general by renaming it to name_compare(). Sounds reasonable. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More

Re: [PATCH v3 0/5] cleanup duplicate name_compare() functions

2014-06-18 Thread Jonathan Nieder
Jeremiah Mahler wrote: Jeremiah Mahler (5): cache: rename cache_name_compare() to name_compare() tree-walk.c: remove name_compare() function unpack-trees.c: remove name_compare() function dir.c: rename to name_compare() name-hash.c: rename to name_compare() cache.h| 2

[PATCH 0/16] skip_prefix refactoring and cleanups

2014-06-18 Thread Jeff King
A while ago[1] we discussed refactoring skip_prefix (or adding something like it) to make it more natural to call. This morning I decided to take a look at doing this, and went down a rabbit hole of cleanups. This is part one of the result. The short of it is that skip_prefix can now be used like

[PATCH 02/16] daemon: mark some strings as const

2014-06-18 Thread Jeff King
None of these strings is modified; marking them as const will help later refactoring. Signed-off-by: Jeff King p...@peff.net --- daemon.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/daemon.c b/daemon.c index f9c63e9..18818c3 100644 --- a/daemon.c +++

[PATCH 01/16] parse_diff_color_slot: drop ofs parameter

2014-06-18 Thread Jeff King
This function originally took a whole config variable name (var) and an offset (ofs). It checked var+ofs against each color slot, but reported errors using the whole var. However, since 8b8e862 (ignore unknown color configuration, 2009-12-12), it returns -1 rather than printing its own error, and

[PATCH 03/16] avoid using skip_prefix as a boolean

2014-06-18 Thread Jeff King
There's no point in using: if (skip_prefix(buf, foo)) over if (starts_with(buf, foo)) as the point of skip_prefix is to return a pointer to the data after the prefix. Using starts_with is more readable, and will make refactoring skip_prefix easier. Signed-off-by: Jeff King p...@peff.net

[PATCH 04/16] refactor skip_prefix to return a boolean

2014-06-18 Thread Jeff King
The skip_prefix function returns a pointer to the content past the prefix, or NULL if the prefix was not found. While this is nice and simple, in practice it makes it hard to use for two reasons: 1. When you want to conditionally skip or keep the string as-is, you have to introduce a

[PATCH 05/16] apply: use skip_prefix instead of raw addition

2014-06-18 Thread Jeff King
A submodule diff generally has content like: -Subproject commit [0-9a-f]{40} +Subproject commit [0-9a-f]{40} When we are using git apply --index with a submodule, we first apply the textual diff, and then parse that result to figure out the new sha1. If the diff has bogus input like:

[PATCH 06/16] fast-import: fix read of uninitialized argv memory

2014-06-18 Thread Jeff King
Fast-import shares code between its command-line parser and the option command. To do so, it strips the -- from any command-line options and passes them to the option parser. However, it does not confirm that the option even begins with -- before blindly passing arg + 2. It does confirm that the

[PATCH 07/16] transport-helper: avoid reading past end-of-string

2014-06-18 Thread Jeff King
We detect the import-marks capability by looking for that string, but _without_ a trailing space. Then we skip past it using strlen(import-marks ), with a space. So if a remote helper gives us exactly import-marks, we will read past the end-of-string by one character. This is unlikely to be a

[PATCH 08/16] use skip_prefix to avoid magic numbers

2014-06-18 Thread Jeff King
It's a common idiom to match a prefix and then skip past it with a magic number, like: if (starts_with(foo, bar)) foo += 3; This is easy to get wrong, since you have to count the prefix string yourself, and there's no compiler check if the string changes. We can use skip_prefix to

[PATCH 09/16] use skip_prefix to avoid repeating strings

2014-06-18 Thread Jeff King
It's a common idiom to match a prefix and then skip past it with strlen, like: if (starts_with(foo, bar)) foo += strlen(bar); This avoids magic numbers, but means we have to repeat the string (and there is no compiler check that we didn't make a typo in one of the strings). We can

[PATCH 10/16] fast-import: use skip_prefix for parsing input

2014-06-18 Thread Jeff King
Fast-import does a lot of parsing of commands and dispatching to sub-functions. For example, given option foo, we might recognize option using starts_with, and then hand it off to parse_option() to do the rest. However, we do not let parse_option know that we have parsed the first part already.

[PATCH 11/16] daemon: use skip_prefix to avoid magic numbers

2014-06-18 Thread Jeff King
Like earlier cases, we can use skip_prefix to avoid magic numbers that must match the length of starts_with prefixes. However, the numbers are a little more complicated here, as we keep parsing past the prefix. We can solve it by keeping a running pointer as we parse; its final value is the

[PATCH 12/16] stat_opt: check extra strlen call

2014-06-18 Thread Jeff King
As in earlier commits, the diff option parser uses starts_with to find that an argument starts with --stat-, and then adds strlen(stat-) to find the rest of the option. However, in this case the starts_with and the strlen are separated across functions, making it easy to call the latter without

[PATCH 13/16] fast-import: refactor parsing of spaces

2014-06-18 Thread Jeff King
When we see a file change in a commit, we expect one of: 1. A mark. 2. An inline keyword. 3. An object sha1. The handling of spaces is inconsistent between the three options. Option 1 calls a sub-function which checks for the space, but doesn't parse past it. Option 2 parses the space,

[PATCH] alloc.c: remove alloc_raw_commit_node() function

2014-06-18 Thread Ramsay Jones
In order to encapsulate the setting of the unique commit index, commit 969eba63 (commit: push commit_index update into alloc_commit_node, 10-06-2014) introduced a (logically private) intermediary allocator function. However, this function (alloc_raw_commit_node()) was declared as a public

[PATCH 14/16] fetch-pack: refactor parsing in get_ack

2014-06-18 Thread Jeff King
There are several uses of the magic number line+45 when parsing ACK lines from the server, and it's rather unclear why 45 is the correct number. We can make this more clear by keeping a running pointer as we parse, using skip_prefix to jump past the first ACK , then adding 40 to jump past

[PATCH 15/16] git: avoid magic number with skip_prefix

2014-06-18 Thread Jeff King
After handling options, any leftover arguments should be commands. However, we pass through --help and --version, so that we convert them into git help and git version respectively. This is a straightforward use of skip_prefix to avoid a magic number, but while we are there, it is worth adding a

[PATCH 16/16] use skip_prefix to avoid repeated calculations

2014-06-18 Thread Jeff King
In some cases, we use starts_with to check for a prefix, and then use an already-calculated prefix length to advance a pointer past the prefix. There are no magic numbers or duplicated strings here, but we can still make the code simpler and more obvious by using skip_prefix. Signed-off-by: Jeff

[PATCH 2/2] use xstrdup_fmt in favor of manual size calculations

2014-06-18 Thread Jeff King
In many parts of the code, we do an ugly and error-prone malloc like: const char *fmt = something %s; buf = xmalloc(strlen(foo) + 10 + 1); sprintf(buf, fmt, foo); This makes the code brittle, and if we ever get the allocation wrong, is a potential heap overflow. Let's instead favor

[PATCH 1/2] strbuf: add xstrdup_fmt helper

2014-06-18 Thread Jeff King
You can use a strbuf to build up a string from parts, and then detach it. In the general case, you might use multiple strbuf_add* functions to do the building. However, in many cases, a single strbuf_addf is sufficient, and we end up with: struct strbuf buf = STRBUF_INIT; ...

Re: [PATCH] alloc.c: remove alloc_raw_commit_node() function

2014-06-18 Thread Jeff King
On Wed, Jun 18, 2014 at 08:52:46PM +0100, Ramsay Jones wrote: In order to encapsulate the setting of the unique commit index, commit 969eba63 (commit: push commit_index update into alloc_commit_node, 10-06-2014) introduced a (logically private) intermediary allocator function. However, this

Importing history to show up in a blame

2014-06-18 Thread Jason Pyeron
The only way I have found so far to do this is to merge the branch on to a tmp branch and then back! The only other way I found seems real bad: http://stackoverflow.com/questions/16473363/tell-git-blame-to-use-imported-histo ry And the way below does not work if there are edits already on master

[PATCH 0/7] cleaning up determine_author_info

2014-06-18 Thread Jeff King
This is another function I ran across in today's cleanups. The memory leak in it has bugged me for a while (even though it's really not a big deal in practice). So this is mostly minor cleanups, but I did find a bug in the commit parser. [1/7]: commit: provide a function to find a header in a

[PATCH 1/7] commit: provide a function to find a header in a buffer

2014-06-18 Thread Jeff King
Usually when we parse a commit, we read it line by line and handle each header in a single pass (e.g., in parse_commit and parse_commit_header). Sometimes, however, we only care about extracting a single header. Code in this situation is stuck doing an ad-hoc parse of the commit buffer. Let's

[PATCH 2/7] record_author_info: fix memory leak on malformed commit

2014-06-18 Thread Jeff King
If we hit the end-of-header without finding an author line, we just return from the function. We should jump to the fail_exit path to clean up the buffer that we may have allocated. Signed-off-by: Jeff King p...@peff.net --- commit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff

[PATCH 3/7] record_author_info: use find_commit_header

2014-06-18 Thread Jeff King
This saves us some manual parsing and makes the code more readable. Signed-off-by: Jeff King p...@peff.net --- I suspect there are other sites which could use this helper, too; I didn't do an exhaustive search. commit.c | 23 --- 1 file changed, 8 insertions(+), 15

Re: [PATCH v18 07/48] lockfile.c: make lock_file return a meaningful errno on failurei

2014-06-18 Thread Michael Haggerty
There's a typo in the subject line of this commit. Michael On 06/17/2014 05:53 PM, Ronnie Sahlberg wrote: Making errno when returning from lock_file() meaningful, which should fix * an existing almost-bug in lock_ref_sha1_basic where it assumes errno==ENOENT is meaningful and could

[PATCH 4/7] ident_split: store begin/end pairs on their own struct

2014-06-18 Thread Jeff King
When we parse an ident line, we end up with several fields, each with a begin/end pointer into the buffer, like: const char *name_begin; const char *name_end; There is nothing except the field names to indicate that they are paired. This makes it annoying to write helper functions for

[PATCH 5/7] use strbufs in date functions

2014-06-18 Thread Jeff King
Many of the date functions write into fixed-size buffers. This is a minor pain, as we have to take special precautions, and frequently end up copying the result into a strbuf or heap-allocated buffer anyway (for which we sometimes use strcpy!). Let's instead teach parse_date, datestamp, etc to

Re: [PATCH v3 01/14] refs.c make ref_transaction_create a wrapper to ref_transaction_update

2014-06-18 Thread Junio C Hamano
Ronnie Sahlberg sahlb...@google.com writes: Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 13 ++--- refs.h | 7 --- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/refs.c b/refs.c index dfbf003..a9f91ab 100644 --- a/refs.c +++ b/refs.c @@

[PATCH 6/7] determine_author_info: reuse parsing functions

2014-06-18 Thread Jeff King
Rather than parsing the header manually to find the author field, and then parsing its sub-parts, let's use find_commit_header and split_ident_line. This is shorter and easier to read, and should do a more careful parsing job. For example, the current parser could find the end-of-email

[PATCH 7/7] determine_author_info: stop leaking name/email

2014-06-18 Thread Jeff King
When we get the author name and email either from an existing commit or from the --author option, we create a copy of the strings. We cannot just free() these copies, since the same pointers may also be pointing to getenv() storage which we do not own. Instead, let's treat these the same way as

Re: [PATCH v18 07/48] lockfile.c: make lock_file return a meaningful errno on failurei

2014-06-18 Thread Michael Haggerty
On 06/17/2014 05:53 PM, Ronnie Sahlberg wrote: Making errno when returning from lock_file() meaningful, which should fix * an existing almost-bug in lock_ref_sha1_basic where it assumes errno==ENOENT is meaningful and could waste some work on retries * an existing bug in

Re: [PATCH v3 04/14] refs.c: add a new update_type field to ref_update

2014-06-18 Thread Junio C Hamano
Ronnie Sahlberg sahlb...@google.com writes: Add a field that describes what type of update this refers to. For now the only type is UPDATE_SHA1 but we will soon add more types. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 25 + 1 file changed, 21

Re: [PATCH v18 10/48] refs.c: verify_lock should set errno to something meaningful

2014-06-18 Thread Michael Haggerty
On 06/17/2014 05:53 PM, Ronnie Sahlberg wrote: Making errno when returning from verify_lock() meaningful, which should almost but not completely fix * a bug in git fetch's s_update_ref, which trusts the result of an errno == ENOTDIR check to detect D/F conflicts ENOTDIR makes sense as

Re: [PATCH v18 16/48] refs.c: add an err argument to delete_ref_loose

2014-06-18 Thread Michael Haggerty
On 06/17/2014 05:53 PM, Ronnie Sahlberg wrote: Add an err argument to delete_loose_ref so that we can pass a descriptive error string back to the caller. Pass the err argument from transaction commit to this function so that transaction users will have a nice error string if the transaction

Re: [PATCH v18 11/48] refs.c: make remove_empty_directories alwasy set errno to something sane

2014-06-18 Thread Michael Haggerty
There is a typo in the commit log subject line: s/alwasy/always/ Michael On 06/17/2014 05:53 PM, Ronnie Sahlberg wrote: Making errno when returning from remove_empty_directories() more obviously meaningful, which should provide some peace of mind for people auditing lock_ref_sha1_basic.

Re: [PATCH v18 14/48] refs.c: log_ref_write should try to return meaningful errno

2014-06-18 Thread Michael Haggerty
On 06/17/2014 05:53 PM, Ronnie Sahlberg wrote: Making errno from write_ref_sha1() meaningful, which should fix * a bug in git checkout -b where it prints strerror(errno) despite errno possibly being zero or clobbered * a bug in git fetch's s_update_ref, which trusts the result of an

Re* [PATCH v3 04/14] refs.c: add a new update_type field to ref_update

2014-06-18 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: Ronnie Sahlberg sahlb...@google.com writes: Add a field that describes what type of update this refers to. For now the only type is UPDATE_SHA1 but we will soon add more types. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 25

Re: [msysGit] Re: [PATCH 0/7] Second part of msysgit/unicode

2014-06-18 Thread Johannes Sixt
Am 18.06.2014 19:33, schrieb Junio C Hamano: In the meantime, are Windows folks happy with the four topics queued on 'pu' so far? I would like to start moving them down to 'next' and to 'master' soonish. They consist of these individual patches: $ git shortlog ^master \

  1   2   >