Re: [PATCH v2 17/25] commit_lock_file(): make committing an unlocked lockfile a NOP

2014-04-14 Thread Michael Haggerty
On 04/07/2014 09:31 PM, Jeff King wrote: On Mon, Apr 07, 2014 at 01:33:59AM +0200, Michael Haggerty wrote: It was previously a bug to call commit_lock_file() with a lock_file object that was not active (an illegal access would happen within the function). It was presumably never done, but

Re: [PATCH 1/3] rebase: avoid non-function use of return on FreeBSD

2014-04-14 Thread Matthieu Moy
Kyle J. McKay mack...@gmail.com writes: So I suggest that in the interest of fixing rebase on FreeBSD in an expeditious fashion, patches 1/3 and 2/3 get picked up (see note below) now and that the follow-on patch below, after being enhanced to pass all tests, be submitted separately at

Re: Our official home page and logo for the Git project

2014-04-14 Thread Stefan Beller
So this is really bikeshedding at its finest. I'd personally do agree on the logo proposed in the first mail by Junio. However who is the core community, who am I to judge? So maybe the decision process on this issue may need a more centrally steered opinion, so why not call for votes and weight

Re: [PATCH v3 19/27] refs: add a concept of a reference transaction

2014-04-14 Thread Michael Haggerty
On 04/07/2014 09:13 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: +void ref_transaction_create(struct ref_transaction *transaction, +const char *refname, +unsigned char *new_sha1, +int flags)

Re: Our official home page and logo for the Git project

2014-04-14 Thread Erik Faye-Lund
On Fri, Apr 11, 2014 at 9:25 PM, Junio C Hamano gits...@pobox.com wrote: The motion is about this: Outside people, like the party who approached us about putting our logo on their trinket, seem to associate that logo we see on git-scm.com today with our project, but we never

[PATCH v3 00/25] Lockfile correctness and refactoring

2014-04-14 Thread Michael Haggerty
Round v3. Thanks to Johannes Sixt and Peff for feedback on v2. This version addresses all issues raised for v1 [1] and v2 [2]. Changes since v2: * Instead of keeping track of whether a lock_file object is active via a new bit in a flags bitmask, store it in a separate volatile sig_atomic_t

[PATCH v3 01/25] unable_to_lock_die(): rename function from unable_to_lock_index_die()

2014-04-14 Thread Michael Haggerty
This function is used for other things besides the index, so rename it accordingly. Suggested-by: Jeff King p...@peff.net Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/update-index.c | 2 +- cache.h| 2 +- lockfile.c | 6 +++--- refs.c

[PATCH v3 17/25] lockfile: avoid transitory invalid states

2014-04-14 Thread Michael Haggerty
Because remove_lock_file() can be called any time by the signal handler, it is important that any lock_file objects that are in the lock_file_list are always in a valid state. And since lock_file objects are often reused (but are never removed from lock_file_list), that means we have to be

[PATCH v3 15/25] commit_lock_file(): inline temporary variable

2014-04-14 Thread Michael Haggerty
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lockfile.c b/lockfile.c index 477bf4b..664b0c3 100644 --- a/lockfile.c +++ b/lockfile.c @@ -288,12 +288,14 @@ int close_lock_file(struct lock_file *lk)

[PATCH v3 04/25] rollback_lock_file(): set fd to -1

2014-04-14 Thread Michael Haggerty
When rolling back the lockfile, call close_lock_file() so that the lock_file's fd field gets set back to -1. This keeps the lock_file object in a valid state, which is important because these objects are allowed to be reused. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c |

[PATCH v3 12/25] write_packed_entry_fn(): convert cb_data into a (const int *)

2014-04-14 Thread Michael Haggerty
This makes it obvious that we have no plans to change the integer pointed to, which is actually the fd field from a struct lock_file. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index

[PATCH v3 06/25] hold_lock_file_for_append(): release lock on errors

2014-04-14 Thread Michael Haggerty
If there is an error copying the old contents to the lockfile, roll back the lockfile before exiting so that the lockfile is not held until process cleanup. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git

[PATCH v3 16/25] commit_lock_file(): die() if called for unlocked lockfile object

2014-04-14 Thread Michael Haggerty
It was previously a bug to call commit_lock_file() with a lock_file object that was not active (an illegal access would happen within the function). It was presumably never done, but this would be an easy programming error to overlook. So before continuing, do a consistency check that the

[PATCH v3 10/25] delete_ref_loose(): don't muck around in the lock_file's filename

2014-04-14 Thread Michael Haggerty
It's bad manners. Especially since, if unlink_or_warn() failed, the memory wasn't restored to its original contents. So make our own copy to work with. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff

[PATCH v3 25/25] trim_last_path_elm(): replace last_path_elm()

2014-04-14 Thread Michael Haggerty
Rewrite last_path_elm() to take a strbuf parameter and to trim off the last path name element in place rather than returning a pointer to the beginning of the last path name element. This simplifies the function a bit and makes it integrate better with its caller, which is also strbuf-based.

[PATCH v3 09/25] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN

2014-04-14 Thread Michael Haggerty
There are a few places that use these values, so define constants for them. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- If I were confident that it would always be inlined, I would have used #define LOCK_SUFFIX_LEN strlen(LOCK_SUFFIX) Or maybe even have omitted this constant

[PATCH v3 19/25] try_merge_strategy(): remove redundant lock_file allocation

2014-04-14 Thread Michael Haggerty
By the time the if block is entered, the lock_file instance from the main function block is no longer in use, so re-use that one instead of allocating a second one. Note that the lock variable in the if block shadowed the lock variable at function scope, so the only change needed is to remove the

[PATCH v3 22/25] Change lock_file::filename into a strbuf

2014-04-14 Thread Michael Haggerty
For now, we still make sure to allocate at least PATH_MAX characters for the strbuf because resolve_symlink() doesn't know how to expand the space for its return value. (That will be fixed in a moment.) Another alternative would be to just use a strbuf as scratch space in lock_file() but then

[PATCH v3 21/25] commit_lock_file(): use a strbuf to manage temporary space

2014-04-14 Thread Michael Haggerty
Avoid relying on the filename length restrictions that are currently checked by lock_file(). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lockfile.c b/lockfile.c index fce53f1..0aa2998 100644

[PATCH v3 20/25] try_merge_strategy(): use a statically-allocated lock_file object

2014-04-14 Thread Michael Haggerty
Even the one lockfile object needn't be allocated each time the function is called. Instead, define one statically-allocated lock_file object and reuse it for every call. Suggested-by: Jeff King p...@peff.net Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/merge.c | 14

[PATCH v3 23/25] resolve_symlink(): use a strbuf for internal scratch space

2014-04-14 Thread Michael Haggerty
Aside from shortening and simplifying the code, this removes another place where the path name length is arbitrarily limited. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 33 - 1 file changed, 12 insertions(+), 21 deletions(-) diff --git

[PATCH v3 13/25] lock_file(): exit early if lockfile cannot be opened

2014-04-14 Thread Michael Haggerty
This is a bit easier to read than the old version, which nested part of the non-error code in an if block. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lockfile.c b/lockfile.c index

[PATCH v3 24/25] resolve_symlink(): take a strbuf parameter

2014-04-14 Thread Michael Haggerty
Change resolve_symlink() to take a strbuf rather than a string as parameter. This simplifies the code and removes an arbitrary pathname length restriction. It also means that lock_file's filename field no longer needs to be initialized to a large size. Signed-off-by: Michael Haggerty

[PATCH v3 14/25] remove_lock_file(): call rollback_lock_file()

2014-04-14 Thread Michael Haggerty
It does just what we need. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lockfile.c b/lockfile.c index ba791d5..477bf4b 100644 --- a/lockfile.c +++ b/lockfile.c @@ -63,12 +63,8 @@ static void

[PATCH v3 18/25] struct lock_file: declare some fields volatile

2014-04-14 Thread Michael Haggerty
The function remove_lock_file_on_signal() is used as a signal handler. It is not realistic to make the signal handler conform strictly to the C standard, which is very restrictive about what a signal handler is allowed to do. But let's increase the likelihood that it will work: The

[PATCH v3 07/25] lock_file(): always add lock_file object to lock_file_list

2014-04-14 Thread Michael Haggerty
It used to be that if locking failed, lock_file() usually did not register the lock_file object in lock_file_list but sometimes it did. This confusion was compounded if lock_file() was called via hold_lock_file_for_append(), which has its own failure modes. The ambiguity didn't have any ill

[PATCH v3 08/25] lockfile.c: document the various states of lock_file objects

2014-04-14 Thread Michael Haggerty
Document the valid states of lock_file objects, how they get into each state, and how the state is encoded in the object's fields. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 51 +++ 1 file changed, 51 insertions(+) diff

[PATCH v3 11/25] prepare_index(): declare return value to be (const char *)

2014-04-14 Thread Michael Haggerty
Declare the return value to be const to make it clear that we aren't giving callers permission to write over the string that it points at. (The return value is the filename field of a struct lock_file, which can be used by a signal handler at any time and therefore shouldn't be tampered with.)

[PATCH v3 05/25] lockfile: unlock file if lockfile permissions cannot be adjusted

2014-04-14 Thread Michael Haggerty
If the call to adjust_shared_perm() fails, lock_file returns -1, which to the caller looks like any other failure to lock the file. So in this case, roll back the lockfile before returning so that the lock file is deleted immediately and the lockfile object is left in a predictable state (namely,

[PATCH v3 02/25] api-lockfile: expand the documentation

2014-04-14 Thread Michael Haggerty
Document a couple more functions and the flags argument as used by hold_lock_file_for_update() and hold_lock_file_for_append(). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/technical/api-lockfile.txt | 36 +--- 1 file changed, 33

[PATCH v3 03/25] rollback_lock_file(): do not clear filename redundantly

2014-04-14 Thread Michael Haggerty
It is only necessary to clear the lock_file's filename field if it was not already clear. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lockfile.c b/lockfile.c index d4bfb3f..7701267 100644 --- a/lockfile.c

Re: Get all tips quickly

2014-04-14 Thread Kirill Likhodedov
Hi Michael, Ævar, Thank you very much for your answers. Each of 'git show-ref’ and ‘git for-each-ref’ is 2 times faster than ‘git log --branches --tags --remotes’ on “warmed up FS caches, but take the same time on “cold” FS. It seems that all these approaches internally walk down from all

GitMinutes about Git for Windows

2014-04-14 Thread Johannes Schindelin
Dear friends of Git for Windows, it was very delightful to be on the show, hosted by Thomas Ferris Nicolaisen: http://episodes.gitminutes.com/2014/04/gitminutes-28-johannes-schindelin-on.html Enjoy, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a

Re: Silly time stamps

2014-04-14 Thread Andreas Krey
On Wed, 09 Apr 2014 20:50:57 +, Mahmoud Asshole wrote: ... This was raised previously[1], but none of the responses are convincing. Please go directly to the special hell of debugging timezone-related stuff in customer data, for which this extra bit of information is heaven-sent. Do not

Re: [msysGit] GitMinutes about Git for Windows

2014-04-14 Thread Erik Faye-Lund
On Mon, Apr 14, 2014 at 5:14 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Dear friends of Git for Windows, it was very delightful to be on the show, hosted by Thomas Ferris Nicolaisen: http://episodes.gitminutes.com/2014/04/gitminutes-28-johannes-schindelin-on.html Really

[PATCH v4 2/3] refs.c: split delete_ref_loose() into a separate flag-for-deletion and commit phase

2014-04-14 Thread Ronnie Sahlberg
Change delete_ref_loose()) to just flag that a ref is to be deleted but do not actually unlink the files. Change commit_ref_lock() so that it will unlink refs that are flagged for deletion. Change all callers of delete_ref_loose() to explicitely call commit_ref_lock() to commit the deletion. The

Re: [PATCH v3 2/2] commit: add --ignore-submodules[=when] parameter

2014-04-14 Thread Junio C Hamano
Ronald Weiss weiss.ron...@gmail.com writes: On 8. 4. 2014 20:43, Jens Lehmann wrote: Useful when values for commit are 'all' (default) or 'none'. The others ('dirty' and 'untracked') have same effect as 'none', as commit is only interested in whether the submodule's HEAD differs from what is

[PATCH v4 3/3] refs.c: change ref_transaction_commit to run the commit loops once all work is finished

2014-04-14 Thread Ronnie Sahlberg
During a transaction commit we will both update and delete refs. Since both update and delete now use the same pattern lock = lock_ref_sha1_basic() (or varient of) write_ref_sha1(lock)/delete_ref_loose(lock) unlock_ref(lock) | commit_ref_lock(lock) we can now simplify

[PATCH v4 0/3] Make update refs more atomic

2014-04-14 Thread Ronnie Sahlberg
refs.c:ref_transaction_commit() intermingles doing updates and checks with actually applying changes to the refs in loops that abort on error. This is done one ref at a time and means that if an error is detected that will fail the operation partway through the list of refs to update we will end

Re: [PATCH v7 2/2] Verify index file before we opportunistically update it

2014-04-14 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes: On Sat, Apr 12, 2014 at 11:19 AM, Junio C Hamano gits...@pobox.com wrote: In the spectrum between useful and insane, there is a point where we should just tell the insane: don't do it then. ... A process' dying is a way of telling people this is insane

[PATCH v4 1/3] refs.c: split writing and commiting a ref into two separate functions

2014-04-14 Thread Ronnie Sahlberg
Change the function write_ref_sha1() to just write the ref but not commit the ref or the lockfile. Add a new function commit_ref_lock() that will commit the change done by a previous write_ref_sha1(). Update all callers of write_ref_sha1() to call commit_ref_lock(). The new pattern for updating a

Re: [PATCH] git-rebase: Print name of rev when using shorthand

2014-04-14 Thread Junio C Hamano
Brian Gesiak modoca...@gmail.com writes: The output from a successful invocation of the shorthand command git rebase - is something like Fast-forwarded HEAD to @{-1}, which includes a relative reference to a revision. Other commands that use the shorthand -, such as git checkout -, typically

Re: [PATCH v3 2/2] commit: add --ignore-submodules[=when] parameter

2014-04-14 Thread Ronald Weiss
On 14. 4. 2014 20:30, Junio C Hamano wrote: Ronald Weiss weiss.ron...@gmail.com writes: On 8. 4. 2014 20:43, Jens Lehmann wrote: Useful when values for commit are 'all' (default) or 'none'. The others ('dirty' and 'untracked') have same effect as 'none', as commit is only interested in

Re: [PATCH 5/5] completion: fix completion of certain aliases

2014-04-14 Thread Junio C Hamano
Gábor Szeder sze...@ira.uka.de writes: words[] is just fine, we never modify it after it is filled by _get_comp_words_by_ref() at the very beginning. Hmph. I would have understood if the latter were we never look at it (to decide what to do). we never modify it does not sound like an enough

Re: [PATCH 1/2] Makefile: use curl-config to determine curl flags

2014-04-14 Thread Junio C Hamano
Dave Borowitz dborow...@google.com writes: curl-config should always be installed alongside a curl distribution, and its purpose is to provide flags for building against libcurl, so use it instead of guessing flags and dependent libraries. Allow overriding CURL_CONFIG to a custom path to

Re: [PATCH v4 0/3] Make update refs more atomic

2014-04-14 Thread Junio C Hamano
Thanks; will queue. -- 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] prompt: fix missing file errors in zsh

2014-04-14 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes: zsh seems to have a bug while redirecting the stderr of the 'read' command: % read foo 2 /dev/null foo zsh: no such file or directory: foo Which causes errors to be displayed when certain files are missing. Let's add a convenience

Re: [PATCH v9 0/6] transport-helper: fixes

2014-04-14 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes: These patches add support for remote helpers --force, --dry-run, and reporting forced update. Changes since v8: --- a/transport-helper.c +++ b/transport-helper.c @@ -734,7 +734,7 @@ static int push_update_ref_status(struct strbuf *buf,

Re: [PATCH 4/5] transport-helper: trivial cleanup

2014-04-14 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes: It's simpler to store the file names directly, and form the fast-export arguments only when needed, and re-use the same strbuf with a format. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- transport-helper.c | 23

Re: [PATCH v3 2/2] commit: add --ignore-submodules[=when] parameter

2014-04-14 Thread Junio C Hamano
Ronald Weiss weiss.ron...@gmail.com writes: On 14. 4. 2014 20:30, Junio C Hamano wrote: Ronald Weiss weiss.ron...@gmail.com writes: On 8. 4. 2014 20:43, Jens Lehmann wrote: Useful when values for commit are 'all' (default) or 'none'. The others ('dirty' and 'untracked') have same effect as

Re: [PATCH v3 19/27] refs: add a concept of a reference transaction

2014-04-14 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes: I forgot to confirm that the callers *do* verify that they don't pass incorrect values to ref_transaction_create() and ref_transaction_delete(). But if they wouldn't, then die(BUG:) would not be appropriate either, would it? It would have to be

Re: [PATCH 5/5] transport-helper: fix sync issue on crashes

2014-04-14 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes: When a remote helper crashes while pushing we should revert back to the state before the push, however, it's possible that `git fast-export` already finished its job, and therefore has exported the marks already. This creates a

Re: On interpret-trailers standalone tool

2014-04-14 Thread Junio C Hamano
Christian Couder chrisc...@tuxfamily.org writes: However, I am wondering if the current everything on the command line is instruction to the command is too limiting to allow the use of the tool both as a filter and as a tool that can work on one or more files named on the command line. If we

Re: Clear an invalid password out of the credential-cache?

2014-04-14 Thread Jeff King
On Sat, Apr 12, 2014 at 09:12:57AM -0400, Jason Pyeron wrote: Is it me or is the only way to clear a single invalid password out of the credential-cache is by git credential-cache exit? Try the reject action: $ : remember a credential $ url() { echo url=https://example.com; } $ (url;

Re: [PATCH 1/3] rebase: avoid non-function use of return on FreeBSD

2014-04-14 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: Kyle J. McKay mack...@gmail.com writes: So I suggest that in the interest of fixing rebase on FreeBSD in an expeditious fashion, patches 1/3 and 2/3 get picked up (see note below) now and that the follow-on patch below, after being

Re: [PATCH 1/3] rebase: avoid non-function use of return on FreeBSD

2014-04-14 Thread Junio C Hamano
Kyle J. McKay mack...@gmail.com writes: For convenience, here are the diffs using -w: |--- a/git-rebase--am.sh |+++ b/git-rebase--am.sh |@@ -4,6 +4,7 @@ # Copyright (c) 2010 Junio C Hamano. # +git_rebase__am() { case $action in continue) git am --resolved

Re: [PATCH v3 19/27] refs: add a concept of a reference transaction

2014-04-14 Thread Michael Haggerty
On 04/14/2014 11:25 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: I forgot to confirm that the callers *do* verify that they don't pass incorrect values to ref_transaction_create() and ref_transaction_delete(). But if they wouldn't, then die(BUG:) would not be

Re: [PATCH v2 6/9] branch: display publish branch

2014-04-14 Thread Jeff King
On Sat, Apr 12, 2014 at 10:05:15AM -0500, Felipe Contreras wrote: As you can see; some branches are published, others are not. The ones that are not published don't have a @{publish}, and `git branch -v` doesn't show them. Why is that hard to understand? Do you ever push the unpublished