Re: How to get notified of new releases?

2014-01-27 Thread Matthieu Moy
- Original Message -
> Are there any dedicated mailing lists for git releases, or RSS feeds?

Not sure if there's a Windows-dedicated list, but there's this:

  http://gitrss.q42.co.uk/

It filters the mailing-list posts starting with eg. [ANNOUNCE] and turns it 
into an RSS feed.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: How to get notified of new releases?

2014-01-27 Thread Thomas Hochstein
Robert Dailey schrieb:

> Are there any dedicated mailing lists for git releases, or RSS feeds?
> I am on Windows so I'd specifically be interested in notifications
> when new releases or preview binaries are released on the Windows
> platform. I'm constantly checking the website currently.

You may want to subscribe to , which is
low-traffic.

--
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


What's with git blame --reverse ?

2014-01-27 Thread David Kastrup

The git blame manual page talks about using git blame --reverse to
figure out when a particular change disappeared, but I cannot make it
produce anything useful regardless of what range I give it.  Using
--root delivers a different state of uselessness.

Can anyone give a recipe for using git blame --reverse on the Git code
base for figuring out anything of relevance?

Since I am in the process of rewriting git-blame, of course I want to
verify that everything works, but while I achieve the same results, they
seem fabulously useless before and after my rewrite.

-- 
David Kastrup

--
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: What's with git blame --reverse ?

2014-01-27 Thread Duy Nguyen
On Mon, Jan 27, 2014 at 7:45 PM, David Kastrup  wrote:
>
> The git blame manual page talks about using git blame --reverse to
> figure out when a particular change disappeared, but I cannot make it
> produce anything useful regardless of what range I give it.  Using
> --root delivers a different state of uselessness.
>
> Can anyone give a recipe for using git blame --reverse on the Git code
> base for figuring out anything of relevance?

I rarely use blame, but the commit that introduces --reverse seems to
have an example. See 85af792 (git-blame --reverse - 2008-04-02)
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: How to substructure rewrites?

2014-01-27 Thread Junio C Hamano
David Kastrup  writes:

> As it can easily be guessed, the "add xxx function" commits are
> basically adding not-yet-used code (and so will not disrupt
> compilation), but everything starting with "Reorganize blame data
> structures" up until the final commit will not work or compile since the
> code does not match the data structures.
>
> So there is little point in substructing all that, right?  Even
> something seemingly isolated like
>
> commit f64b41c472442ae9971321fe8f62c3885ba4d8b7
> Author: David Kastrup 
> Date:   Sun Jan 19 02:16:21 2014 +0100
>
> blame.c: Let output determine MORE_THAN_ONE_PATH more efficiently
>
> is not really useful as a separate commit since while it does implement
> a particular task, this is done starting with non-working code relying
> on no-longer existent data structures.

Small pieces that are incrementally added with their own
documentation would certainly be a lot easier to read than one big
ball of wax.  I am wondering if it would make it easier for
everybody to tentatively do "git-blame vs git-blame2" dance here,
just like we did "git-blame vs git-annotate" dance some years ago.
That is, to add a completely new command and have them in parallel
while cooking in 'next' (or we could even keep them in a few
releases if we are not absolutely certain about the correctness of
the result of the new code), aiming to eventually retire the current
implementation and replace it with the new one.  We have already
have test infrastructure to allow us to run variants of blames, too,
to help that kind of transition.

> In general, the rule is likely "any commit should not create a
> non-working state" right?

Yes.
--
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] send-email: If the ca path is not specified, use the defaults

2014-01-27 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> Junio C Hamano wrote:
>> This change could introduce a regression for people on a platform
>> whose certificate directory is /etc/ssl/certs but its IO::Socket:SSL
>> somehow fails to use it as SSL_ca_path without being told.
>
> I can confirm that my git-send-email doesn't regress to the
> pre-35035bbf state; my certificate directory is /etc/ssl/certs. I'm
> somewhat surprised that IO::Socket::SSL picks the right file/
> directory on every platform without being told explicitly. This change
> definitely looks like the right fix.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] doc: remote author/documentation sections from more pages

2014-01-27 Thread Junio C Hamano
Michael Haggerty  writes:

> On 01/27/2014 01:15 AM, Eric Sunshine wrote:
>> On Sun, Jan 26, 2014 at 6:43 PM, Michael Haggerty  
>> wrote:
>>> Subject: [PATCH] doc: remote author/documentation sections from more pages
>> 
>> s/remote/remove/
>
> Gaah!  Git is a virus that invades your muscle memory and prevents you
> from typing words that are similar to git subcommands.
>
> Thanks for noticing.
>
> Michael

Thanks; s/remote/remove/ will be done on this end; no need to
resend.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/4] t3030-merge-recursive: Test known breakage with empty work tree

2014-01-27 Thread Brad King
Sometimes when working with a large repository it can be useful to try
out a merge and only check out conflicting files to disk (for example as
a speed optimization on a server).  Until v1.7.7-rc1~28^2~20
(merge-recursive: When we detect we can skip an update, actually skip
it, 2011-08-11), it was possible to do so with the following idiom:

# Prepare a temporary index and empty work tree.
GIT_INDEX_FILE="$PWD/tmp-$$-index" &&
export GIT_INDEX_FILE &&
GIT_WORK_TREE="$PWD/tmp-$$-work" &&
export GIT_WORK_TREE &&
mkdir "$GIT_WORK_TREE" &&

# Convince the index that our side is on disk.
git read-tree -i -m $ours &&
git update-index --ignore-missing --refresh &&

# Merge their side into our side.
bases=$(git merge-base --all $ours $theirs) &&
git merge-recursive $bases -- $ours $theirs &&
tree=$(git write-tree)

Nowadays, that still works and the exit status is the same, but
merge-recursive produces a diagnostic if "our" side renamed a file:

error: addinfo_cache failed for path 'dst'

Add a test to document this regression.

Signed-off-by: Brad King 
---
 t/t3030-merge-recursive.sh | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 2f96100..3db3bf6 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -257,6 +257,7 @@ test_expect_success 'setup 8' '
git add e &&
test_tick &&
git commit -m "rename a->e" &&
+   c7=$(git rev-parse --verify HEAD) &&
git checkout rename-ln &&
git mv a e &&
test_ln_s_add e a &&
@@ -517,6 +518,52 @@ test_expect_success 'reset and bind merge' '
 
 '
 
+test_expect_failure 'merge-recursive w/ empty work tree - ours has rename' '
+   (
+   GIT_WORK_TREE="$PWD/ours-has-rename-work" &&
+   export GIT_WORK_TREE &&
+   GIT_INDEX_FILE="$PWD/ours-has-rename-index" &&
+   export GIT_INDEX_FILE &&
+   mkdir "$GIT_WORK_TREE" &&
+   git read-tree -i -m $c7 &&
+   git update-index --ignore-missing --refresh &&
+   git merge-recursive $c0 -- $c7 $c3 &&
+   git ls-files -s >actual-files
+   ) 2>actual-err &&
+   >expected-err &&
+   cat >expected-files <<-EOF &&
+   100644 $o3 0b/c
+   100644 $o0 0c
+   100644 $o0 0d/e
+   100644 $o0 0e
+   EOF
+   test_cmp expected-files actual-files &&
+   test_cmp expected-err actual-err
+'
+
+test_expect_success 'merge-recursive w/ empty work tree - theirs has rename' '
+   (
+   GIT_WORK_TREE="$PWD/theirs-has-rename-work" &&
+   export GIT_WORK_TREE &&
+   GIT_INDEX_FILE="$PWD/theirs-has-rename-index" &&
+   export GIT_INDEX_FILE &&
+   mkdir "$GIT_WORK_TREE" &&
+   git read-tree -i -m $c3 &&
+   git update-index --ignore-missing --refresh &&
+   git merge-recursive $c0 -- $c3 $c7 &&
+   git ls-files -s >actual-files
+   ) 2>actual-err &&
+   >expected-err &&
+   cat >expected-files <<-EOF &&
+   100644 $o3 0b/c
+   100644 $o0 0c
+   100644 $o0 0d/e
+   100644 $o0 0e
+   EOF
+   test_cmp expected-files actual-files &&
+   test_cmp expected-err actual-err
+'
+
 test_expect_success 'merge removes empty directories' '
 
git reset --hard master &&
-- 
1.8.5.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/3] merge-recursive: Avoid diagnostic on empty work tree

2014-01-27 Thread Brad King
Hi Folks,

Here is the third revision of this series.  The previous
revisions can be found at $gmane/241009 and $gmane/241030.

Updates since the previous revision of the series:

* Handling of lstat ENOENT has been moved down into refresh_cache_ent
  and activated by a new CE_MATCH_IGNORE_MISSING option.

* Rather than adding a new argument to make_cache_entry, the existing
  'refresh' boolean argument has been generalized to a set of options.
  This required the addition of a new CE_MATCH_REFRESH option to
  enable refresh with no other options.

Thanks,
-Brad

Brad King (4):
  t3030-merge-recursive: Test known breakage with empty work tree
  read-cache.c: Refactor --ignore-missing implementation
  read-cache.c: Extend make_cache_entry refresh flag with options
  merge-recursive.c: Tolerate missing files while refreshing index

 cache.h|  6 +-
 merge-recursive.c  |  4 +++-
 read-cache.c   | 27 ++
 t/t3030-merge-recursive.sh | 47 ++
 4 files changed, 70 insertions(+), 14 deletions(-)

-- 
1.8.5.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/4] read-cache.c: Refactor --ignore-missing implementation

2014-01-27 Thread Brad King
Move lstat ENOENT handling from refresh_index to refresh_cache_ent and
activate it with a new CE_MATCH_IGNORE_MISSING option.  This will allow
other call paths into refresh_cache_ent to use the feature.

Signed-off-by: Brad King 
---
 cache.h  | 2 ++
 read-cache.c | 8 +---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index c9efe88..c96ada7 100644
--- a/cache.h
+++ b/cache.h
@@ -498,6 +498,8 @@ extern void *read_blob_data_from_index(struct index_state 
*, const char *, unsig
 #define CE_MATCH_RACY_IS_DIRTY 02
 /* do stat comparison even if CE_SKIP_WORKTREE is true */
 #define CE_MATCH_IGNORE_SKIP_WORKTREE  04
+/* ignore non-existent files during stat update  */
+#define CE_MATCH_IGNORE_MISSING0x08
 extern int ie_match_stat(const struct index_state *, const struct cache_entry 
*, struct stat *, unsigned int);
 extern int ie_modified(const struct index_state *, const struct cache_entry *, 
struct stat *, unsigned int);
 
diff --git a/read-cache.c b/read-cache.c
index 33dd676..d61846c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1031,6 +1031,7 @@ static struct cache_entry *refresh_cache_ent(struct 
index_state *istate,
int changed, size;
int ignore_valid = options & CE_MATCH_IGNORE_VALID;
int ignore_skip_worktree = options & CE_MATCH_IGNORE_SKIP_WORKTREE;
+   int ignore_missing = options & CE_MATCH_IGNORE_MISSING;
 
if (ce_uptodate(ce))
return ce;
@@ -1050,6 +1051,8 @@ static struct cache_entry *refresh_cache_ent(struct 
index_state *istate,
}
 
if (lstat(ce->name, &st) < 0) {
+   if (ignore_missing && errno == ENOENT)
+   return ce;
if (err)
*err = errno;
return NULL;
@@ -1127,7 +1130,8 @@ int refresh_index(struct index_state *istate, unsigned 
int flags,
int ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;
int first = 1;
int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
-   unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
+   unsigned int options = ((really ? CE_MATCH_IGNORE_VALID : 0) |
+   (not_new ? CE_MATCH_IGNORE_MISSING : 0));
const char *modified_fmt;
const char *deleted_fmt;
const char *typechange_fmt;
@@ -1176,8 +1180,6 @@ int refresh_index(struct index_state *istate, unsigned 
int flags,
if (!new) {
const char *fmt;
 
-   if (not_new && cache_errno == ENOENT)
-   continue;
if (really && cache_errno == EINVAL) {
/* If we are doing --really-refresh that
 * means the index is not valid anymore.
-- 
1.8.5.2

--
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: How to substructure rewrites?

2014-01-27 Thread David Kastrup
Junio C Hamano  writes:

> David Kastrup  writes:
>
>> As it can easily be guessed, the "add xxx function" commits are
>> basically adding not-yet-used code (and so will not disrupt
>> compilation), but everything starting with "Reorganize blame data
>> structures" up until the final commit will not work or compile since the
>> code does not match the data structures.
>>
>> So there is little point in substructing all that, right?  Even
>> something seemingly isolated like
>>
>> commit f64b41c472442ae9971321fe8f62c3885ba4d8b7
>> Author: David Kastrup 
>> Date:   Sun Jan 19 02:16:21 2014 +0100
>>
>> blame.c: Let output determine MORE_THAN_ONE_PATH more efficiently
>>
>> is not really useful as a separate commit since while it does implement
>> a particular task, this is done starting with non-working code relying
>> on no-longer existent data structures.
>
> Small pieces that are incrementally added with their own
> documentation would certainly be a lot easier to read than one big
> ball of wax.

Sure.  The problem is that my rewrite is characterized by doing as
little as possible in order to achieve identical results (with the
conceivable exception of picking a different, equally scored variant in
those parts of the algorithm choosing a maximum).  That also means that
the basic logic and layout of the program stays the same while the data
flow and parts of the data structures are replaced.

> I am wondering if it would make it easier for everybody to tentatively
> do "git-blame vs git-blame2" dance here, just like we did "git-blame
> vs git-annotate" dance some years ago.  That is, to add a completely
> new command and have them in parallel while cooking in 'next' (or we
> could even keep them in a few releases if we are not absolutely
> certain about the correctness of the result of the new code), aiming
> to eventually retire the current implementation and replace it with
> the new one.  We have already have test infrastructure to allow us to
> run variants of blames, too, to help that kind of transition.

Well, the point is that the implementation is supposed to
a) deliver identical results
b) reuse as much code as possible
so there is no real point in working with a separate source file.

For the "if we are not absolutely certain about the correctness of the
result of the new code" angle, this should be covered with the usual
stable/unstable/proposed division most projects have in some way or
another for quality assurance.  I have absolutely no clue how Git
organizes that, but it would usually mean that the new code is not
placed in a different _file_ (or a differently named command) but rather
in a different _branch_ as compared with the current implementation.

>> In general, the rule is likely "any commit should not create a
>> non-working state" right?
>
> Yes.

My current aim is to complete the code to the point where it is
a) fully operative and delivering equivalent results to the current
implementation
b) in every aspect at least as efficient as the current implementation
and in a state that is not basically less comprehensible than what I
started with

Since the change of the data structures and data flow requires changing
all affected program parts to get to a working state, and since I don't
have ambitions to do more than that which is required to get there,
I don't see how the bulk of the work can sensibly avoid coming as one
"omnibus" patch.  Most changes, however, will be understandable quite
well locally.

For example, currently the code has a number of loops traversing one
global linked list, ignoring all entries not relevant to a particular
target, and doing something with the rest.  Those loops generally are
replaced with a simpler loop just running through a single _completely_
relevant linked list.  Even while those replacements are scattered
throughout the patch, they make sense without having to look at the rest
of the patch.

-- 
David Kastrup

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] setup: Don't dereference in-tree symlinks for absolute paths

2014-01-27 Thread Junio C Hamano
Martin Erik Werner  writes:

> In order to manipulate symliks in the
> work tree using absolute paths, symlinks should only be dereferenced
> outside the work tree.

I agree 100% with this reasoning (modulo s/symliks/symlinks/).

As to the implementation, it looks a bit overly complicated,
though.  I haven't tried writing the same myself, but 

 * I suspect that strbuf would help simplifying the allocation and
   deallocation;

 * Also I suspect that use of string-list to split and then join is
   making the code unnecessarily complex. In Python/Perl, that would
   be a normal approach, but in C, it would be a lot simpler if you
   prepare a writable temporary in wtpart[], walk from left to right
   finding '/' and replacing it temporarily with NUL to terminate in
   order to check with real_path(), restore the NUL back to '/' to
   check deeper, and rinse and repeat.

   Having said that, I am not absolutely sure if the repeated
   calls to real_path() are doing the right thing, though ;-)

> diff --git a/setup.c b/setup.c
> index 5432a31..0789a96 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -22,11 +22,51 @@ char *prefix_path_gently(const char *prefix, int len,
>   const char *orig = path;
>   char *sanitized;
>   if (is_absolute_path(orig)) {
> - const char *temp = real_path(path);
> - sanitized = xmalloc(len + strlen(temp) + 1);
> - strcpy(sanitized, temp);
> + int i, match;
> + size_t wtpartlen;
> + char *npath, *wtpart;
> + struct string_list list = STRING_LIST_INIT_DUP;
> + const char *work_tree = get_git_work_tree();
> + if (!work_tree)
> + return NULL;
> + npath = xmalloc(strlen(path) + 1);
>   if (remaining_prefix)
>   *remaining_prefix = 0;
> + if (normalize_path_copy_len(npath, path, remaining_prefix)) {
> + free(npath);
> + return NULL;
> + }
> +
> + string_list_split(&list, npath, '/', -1);
> + wtpart = xmalloc(strlen(npath) + 1);
> + i = 0;
> + match = 0;
> + strcpy(wtpart, list.items[i++].string);
> + strcat(wtpart, "/");
> + if (strcmp(real_path(wtpart), work_tree) == 0) {
> + match = 1;
> + } else {
> + while (i < list.nr) {
> + strcat(wtpart, list.items[i++].string);
> + if (strcmp(real_path(wtpart), work_tree) == 0) {
> + match = 1;
> + break;
> + }
> + strcat(wtpart, "/");
> + }
> + }
> + string_list_clear(&list, 0);
> + if (!match) {
> + free(npath);
> + free(wtpart);
> + return NULL;
> + }
> +
> + wtpartlen = strlen(wtpart);
> + sanitized = xmalloc(strlen(npath) - wtpartlen);
> + strcpy(sanitized, npath + wtpartlen + 1);
> + free(npath);
> + free(wtpart);
>   } else {
>   sanitized = xmalloc(len + strlen(path) + 1);
>   if (len)
> @@ -34,26 +74,10 @@ char *prefix_path_gently(const char *prefix, int len,
>   strcpy(sanitized + len, path);
>   if (remaining_prefix)
>   *remaining_prefix = len;
> - }
> - if (normalize_path_copy_len(sanitized, sanitized, remaining_prefix))
> - goto error_out;
> - if (is_absolute_path(orig)) {
> - size_t root_len, len, total;
> - const char *work_tree = get_git_work_tree();
> - if (!work_tree)
> - goto error_out;
> - len = strlen(work_tree);
> - root_len = offset_1st_component(work_tree);
> - total = strlen(sanitized) + 1;
> - if (strncmp(sanitized, work_tree, len) ||
> - (len > root_len && sanitized[len] != '\0' && sanitized[len] 
> != '/')) {
> - error_out:
> + if (normalize_path_copy_len(sanitized, sanitized, 
> remaining_prefix)) {
>   free(sanitized);
>   return NULL;
>   }
> - if (sanitized[len] == '/')
> - len++;
> - memmove(sanitized, sanitized + len, total - len);
>   }
>   return sanitized;
>  }
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 3a0677a..03a12ac 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -190,7 +190,7 @@ test_expect_success SYMLINKS 'real path works on 
> symlinks' '
>   test "$sym" = "$(test-path-utils real_path "$dir2/syml")"
>  '
>  
> -test_expect_failure SYMLINKS 'prefix_path works with work tree 

[PATCH v3 4/4] merge-recursive.c: Tolerate missing files while refreshing index

2014-01-27 Thread Brad King
Teach add_cacheinfo to tell make_cache_entry to skip refreshing stat
information when a file is missing from the work tree.  We do not want
the index to be stat-dirty after the merge but also do not want to fail
when a file happens to be missing.

This fixes the 'merge-recursive w/ empty work tree - ours has rename'
case in t3030-merge-recursive.

Suggested-by: Elijah Newren 
Signed-off-by: Brad King 
---
 merge-recursive.c  | 3 ++-
 t/t3030-merge-recursive.sh | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index c3753c8..b8ea172 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -202,7 +202,8 @@ static int add_cacheinfo(unsigned int mode, const unsigned 
char *sha1,
 {
struct cache_entry *ce;
ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage,
- (refresh ? CE_MATCH_REFRESH : 0 ));
+ (refresh ? (CE_MATCH_REFRESH |
+ CE_MATCH_IGNORE_MISSING) : 0 ));
if (!ce)
return error(_("addinfo_cache failed for path '%s'"), path);
return add_cache_entry(ce, options);
diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 3db3bf6..82e1854 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -518,7 +518,7 @@ test_expect_success 'reset and bind merge' '
 
 '
 
-test_expect_failure 'merge-recursive w/ empty work tree - ours has rename' '
+test_expect_success 'merge-recursive w/ empty work tree - ours has rename' '
(
GIT_WORK_TREE="$PWD/ours-has-rename-work" &&
export GIT_WORK_TREE &&
-- 
1.8.5.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 3/4] read-cache.c: Extend make_cache_entry refresh flag with options

2014-01-27 Thread Brad King
Convert the make_cache_entry boolean 'refresh' argument to a more
general 'refresh_options' argument.  Pass the value through to the
underlying refresh_cache_ent call.  Add option CE_MATCH_REFRESH to
enable stat refresh.  Update call sites to use the new signature.

Signed-off-by: Brad King 
---
 cache.h   |  4 +++-
 merge-recursive.c |  3 ++-
 read-cache.c  | 21 +++--
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/cache.h b/cache.h
index c96ada7..e8820e1 100644
--- a/cache.h
+++ b/cache.h
@@ -487,7 +487,7 @@ extern int remove_file_from_index(struct index_state *, 
const char *path);
 #define ADD_CACHE_IMPLICIT_DOT 32  /* internal to "git add -u/-A" */
 extern int add_to_index(struct index_state *, const char *path, struct stat *, 
int flags);
 extern int add_file_to_index(struct index_state *, const char *path, int 
flags);
-extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned 
char *sha1, const char *path, int stage, int refresh);
+extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned 
char *sha1, const char *path, int stage, unsigned int refresh_options);
 extern int ce_same_name(const struct cache_entry *a, const struct cache_entry 
*b);
 extern int index_name_is_other(const struct index_state *, const char *, int);
 extern void *read_blob_data_from_index(struct index_state *, const char *, 
unsigned long *);
@@ -500,6 +500,8 @@ extern void *read_blob_data_from_index(struct index_state 
*, const char *, unsig
 #define CE_MATCH_IGNORE_SKIP_WORKTREE  04
 /* ignore non-existent files during stat update  */
 #define CE_MATCH_IGNORE_MISSING0x08
+/* enable stat refresh */
+#define CE_MATCH_REFRESH   0x10
 extern int ie_match_stat(const struct index_state *, const struct cache_entry 
*, struct stat *, unsigned int);
 extern int ie_modified(const struct index_state *, const struct cache_entry *, 
struct stat *, unsigned int);
 
diff --git a/merge-recursive.c b/merge-recursive.c
index a18bd15..c3753c8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -201,7 +201,8 @@ static int add_cacheinfo(unsigned int mode, const unsigned 
char *sha1,
const char *path, int stage, int refresh, int options)
 {
struct cache_entry *ce;
-   ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage, 
refresh);
+   ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage,
+ (refresh ? CE_MATCH_REFRESH : 0 ));
if (!ce)
return error(_("addinfo_cache failed for path '%s'"), path);
return add_cache_entry(ce, options);
diff --git a/read-cache.c b/read-cache.c
index d61846c..db3902e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -15,7 +15,8 @@
 #include "strbuf.h"
 #include "varint.h"
 
-static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int 
really);
+static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
+  unsigned int options);
 
 /* Mask for the name length in ce_flags in the on-disk index */
 
@@ -696,7 +697,7 @@ int add_file_to_index(struct index_state *istate, const 
char *path, int flags)
 
 struct cache_entry *make_cache_entry(unsigned int mode,
const unsigned char *sha1, const char *path, int stage,
-   int refresh)
+   unsigned int refresh_options)
 {
int size, len;
struct cache_entry *ce;
@@ -716,10 +717,7 @@ struct cache_entry *make_cache_entry(unsigned int mode,
ce->ce_namelen = len;
ce->ce_mode = create_ce_mode(mode);
 
-   if (refresh)
-   return refresh_cache_entry(ce, 0);
-
-   return ce;
+   return refresh_cache_entry(ce, refresh_options);
 }
 
 int ce_same_name(const struct cache_entry *a, const struct cache_entry *b)
@@ -1029,11 +1027,12 @@ static struct cache_entry *refresh_cache_ent(struct 
index_state *istate,
struct stat st;
struct cache_entry *updated;
int changed, size;
+   int refresh = options & CE_MATCH_REFRESH;
int ignore_valid = options & CE_MATCH_IGNORE_VALID;
int ignore_skip_worktree = options & CE_MATCH_IGNORE_SKIP_WORKTREE;
int ignore_missing = options & CE_MATCH_IGNORE_MISSING;
 
-   if (ce_uptodate(ce))
+   if (!refresh || ce_uptodate(ce))
return ce;
 
/*
@@ -1130,7 +1129,8 @@ int refresh_index(struct index_state *istate, unsigned 
int flags,
int ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;
int first = 1;
int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
-   unsigned int options = ((really ? CE_MATCH_IGNORE_VALID : 0) |
+   unsigned int options = (CE_MATCH_REFRESH |
+   (really ? CE_MATCH_IGNORE_VALID : 0) |
(not_new ? CE_MATCH_IGNORE_MISSING : 0));
const char *modified_fmt;
const 

Re: How to get notified of new releases?

2014-01-27 Thread Jonathan Nieder
Matthieu Moy wrote:
> Robert Dailey wrote:

>> Are there any dedicated mailing lists for git releases, or RSS feeds?
>
> Not sure if there's a Windows-dedicated list, but there's this:
>
>   http://gitrss.q42.co.uk/
>
> It filters the mailing-list posts starting with eg. [ANNOUNCE] and turns it 
> into an RSS feed.

There's also https://github.com/msysgit/msysgit/commits/master.atom,
though that might be more activity than you're looking for (it would
be the feed to follow if you want to build your own snapshots of git
on Windows and try every change).

Perhaps https://github.com/msysgit/msysgit/tags.atom would do the
trick.

Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] builtin/blame.c: large-scale rewrite

2014-01-27 Thread Junio C Hamano
David Kastrup  writes:

> The previous implementation uses a sorted linear list of struct
> blame_entry in a struct scoreboard for organizing all partial or
> completed work.  Every task that is done requires going through the
> whole list where most entries are not relevant to the task at hand.
>
> This commit reorganizes the data structures in order to have each
> remaining subtask work with its own sorted linear list it can work off
> front to back.  Subtasks are organized into "struct origin" chains
> hanging off particular commits.  Commits are organized into a priority
> queue, processing them in commit date order in order to keep most of
> the work affecting a particular blob collated even in the presence of
> an extensive merge history.  In that manner, linear searches can be
> basically avoided anywhere.  They still are done for identifying one
> of multiple analyzed files in a given commit, but the degenerate case
> of a single large file being assembled from a multitude of smaller
> files in the past is not likely to occur often enough to warrant
> special treatment.
> ---

Sign-off?

Actually, I'd like to take my previous suggestion to add this as
blame2 (to replace blame in the future) back.  That was based on my
fear/hope to see an implementation based on a completely different
approach, but the basic premise of having one blame_entry record per
the lines of the final image in the scoreboard, using diff between
parents to the child to find common lines for passing the blame up,
etc. have not changed at all and the change is all about organizing
the pieces of data in a *much* *better* way to avoid needlessly
finding what we already have computed.  It does look big (and if you
removed those "#if 0/#endif", the final patch would be a lot bigger
because we will see a lot of deleted lines), but I do not think the
code conceptually is a different implementation to warrant "create a
totally new blame2 and let it sit next to the original".

Looks nice overall.

> diff --git a/builtin/blame.c b/builtin/blame.c
> index ead6148..994a9e8 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -18,7 +18,9 @@
>  #include "cache-tree.h"
>  #include "string-list.h"
>  #include "mailmap.h"
> +#include "mergesort.h"
>  #include "parse-options.h"
> +#include "prio-queue.h"
>  #include "utf8.h"
>  #include "userdiff.h"
>  #include "line-range.h"
> @@ -83,11 +85,43 @@ static unsigned blame_copy_score;
>   */
>  struct origin {
>   int refcnt;
> + /* Record preceding blame record for this blob */
>   struct origin *previous;
> + /* origins are put in a list linked via `next' hanging off the
> +  * corresponding commit's util field in order to make finding
> +  * them fast.  The presence in this chain does not count
> +  * towards the origin's reference count.  It is tempting to
> +  * let it count as long as the commit is pending examination,
> +  * but even under circumstances where the commit will be
> +  * present multiple times in the priority queue of unexamined
> +  * commits, processing the first instance will not leave any
> +  * work requiring the origin data for the second instance.  An
> +  * interspersed commit changing that would have to be
> +  * preexisting with a different ancestry and with the same
> +  * commit date in order to wedge itself between two instances
> +  * of the same commit in the priority queue _and_ produce
> +  * blame entries relevant for it.  While we don't want to let
> +  * us get tripped up by this case, it certainly does not seem
> +  * worth optimizing for.
> +  */

Style; please make /* and */ sit on their own line without anything
else in multi-line comments.

> + struct origin *next;
>   struct commit *commit;
> + /* `suspects' contains blame entries that may be attributed to
> +  * this origin's commit or to parent commits.  When a commit
> +  * is being processed, all suspects will be moved, either by
> +  * assigning them to an origin in a different commit, or by
> +  * shipping them to the scoreboard's ent list because they
> +  * cannot be attributed to a different commit.
> +  */
> + struct blame_entry *suspects;
>   mmfile_t file;
>   unsigned char blob_sha1[20];
>   unsigned mode;
> + /* shipped gets set when shipping any suspects to the final
> +  * blame list instead of other commits
> +  */
> + char shipped;

Unused?

> + char guilty;
>   char path[FLEX_ARRAY];
>  };
>  
> @@ -176,10 +210,23 @@ static inline struct origin *origin_incref(struct 
> origin *o)
>  static void origin_decref(struct origin *o)
>  {
>   if (o && --o->refcnt <= 0) {
> + struct origin *p, *l = NULL;
>   if (o->previous)
>   origin_decref(o->previous);
>   free(o->file.ptr);
> - free(o);
> + /* Should be present exactly once in commit chain */
> +

Re: [PATCH] tree_entry_interesting: match against all pathspecs

2014-01-27 Thread Junio C Hamano
Duy Nguyen  writes:

> Ack. Perhaps this on top to verify it
>
> -- 8< --
> diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
> index af5134b..d9f37c3 100755
> --- a/t/t4010-diff-pathspec.sh
> +++ b/t/t4010-diff-pathspec.sh
> @@ -110,4 +110,17 @@ test_expect_success 'diff-tree -r with wildcard' '
>   test_cmp expected result
>  '
>  
> +test_expect_success 'diff multiple wildcard pathspecs' '
> + mkdir path2 &&
> + echo rezrov >path2/file1 &&
> + git update-index --add path2/file1 &&
> + tree3=`git write-tree` &&
> + git diff --name-only $tree $tree3 -- "path2*1" "path1*1" >actual &&
> + cat  +path1/file1
> +path2/file1
> +EOF
> + test_cmp expect actual
> +'
> +
>  test_done
> -- 8< --

Thanks, both.  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 v3 2/4] read-cache.c: Refactor --ignore-missing implementation

2014-01-27 Thread Junio C Hamano
Brad King  writes:

> Move lstat ENOENT handling from refresh_index to refresh_cache_ent and
> activate it with a new CE_MATCH_IGNORE_MISSING option.  This will allow
> other call paths into refresh_cache_ent to use the feature.
>
> Signed-off-by: Brad King 
> ---

Good!

I forgot that we had "update-index --ignore-missing --refresh", and
that is conceptually the thing you want to use in your "perform
merge-recursive in an empty tree while pretending that the working
tree is fully populated and up-to-date" scenario.

>  cache.h  | 2 ++
>  read-cache.c | 8 +---
>  2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index c9efe88..c96ada7 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -498,6 +498,8 @@ extern void *read_blob_data_from_index(struct index_state 
> *, const char *, unsig
>  #define CE_MATCH_RACY_IS_DIRTY   02
>  /* do stat comparison even if CE_SKIP_WORKTREE is true */
>  #define CE_MATCH_IGNORE_SKIP_WORKTREE04
> +/* ignore non-existent files during stat update  */
> +#define CE_MATCH_IGNORE_MISSING  0x08
>  extern int ie_match_stat(const struct index_state *, const struct 
> cache_entry *, struct stat *, unsigned int);
>  extern int ie_modified(const struct index_state *, const struct cache_entry 
> *, struct stat *, unsigned int);
>  
> diff --git a/read-cache.c b/read-cache.c
> index 33dd676..d61846c 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1031,6 +1031,7 @@ static struct cache_entry *refresh_cache_ent(struct 
> index_state *istate,
>   int changed, size;
>   int ignore_valid = options & CE_MATCH_IGNORE_VALID;
>   int ignore_skip_worktree = options & CE_MATCH_IGNORE_SKIP_WORKTREE;
> + int ignore_missing = options & CE_MATCH_IGNORE_MISSING;
>  
>   if (ce_uptodate(ce))
>   return ce;
> @@ -1050,6 +1051,8 @@ static struct cache_entry *refresh_cache_ent(struct 
> index_state *istate,
>   }
>  
>   if (lstat(ce->name, &st) < 0) {
> + if (ignore_missing && errno == ENOENT)
> + return ce;
>   if (err)
>   *err = errno;
>   return NULL;
> @@ -1127,7 +1130,8 @@ int refresh_index(struct index_state *istate, unsigned 
> int flags,
>   int ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;
>   int first = 1;
>   int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
> - unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
> + unsigned int options = ((really ? CE_MATCH_IGNORE_VALID : 0) |
> + (not_new ? CE_MATCH_IGNORE_MISSING : 0));
>   const char *modified_fmt;
>   const char *deleted_fmt;
>   const char *typechange_fmt;
> @@ -1176,8 +1180,6 @@ int refresh_index(struct index_state *istate, unsigned 
> int flags,
>   if (!new) {
>   const char *fmt;
>  
> - if (not_new && cache_errno == ENOENT)
> - continue;
>   if (really && cache_errno == EINVAL) {
>   /* If we are doing --really-refresh that
>* means the index is not valid anymore.
--
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: [ANNOUNCE] Git v1.9-rc0

2014-01-27 Thread Kacper Kornet
On Tue, Jan 21, 2014 at 02:14:22PM -0800, Junio C Hamano wrote:
> It has been reported that turning git.rc into git.res does not like
> the new 2-dewey-decimal release numbering scheme; packagers of
> various distro might find similar issues in their build procedures,
> in which case they have about 3 weeks to update them until the final
> release.

The change in release numbering also breaks down gitolite v2 setups. One
of the gitolite commands, gl-compile-conf, expects the output of git --version 
to match /git version (\d+)\.(\d+)\.(\d+)/. 

I have no idea how big problem it is, as I don't know how many people
hasn't migrate to gitolite v3 yet. 

-- 
  Kacper
--
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: [ANNOUNCE] Git v1.9-rc0

2014-01-27 Thread Jonathan Nieder
Hi,

Kacper Kornet wrote:

> The change in release numbering also breaks down gitolite v2 setups. One
> of the gitolite commands, gl-compile-conf, expects the output of git 
> --version 
> to match /git version (\d+)\.(\d+)\.(\d+)/. 
>
> I have no idea how big problem it is, as I don't know how many people
> hasn't migrate to gitolite v3 yet. 

http://qa.debian.org/popcon.php?package=gitolite says there are some.
I guess soon we'll see if there are complaints.

http://gitolite.com/gitolite/migr.html says gitolite v2 is still
maintained.  Hopefully the patch to gitolite v2 to fix this would not
be too invasive --- e.g., how about this patch (untested)?

Thanks,
Jonathan

diff --git i/src/gl-compile-conf w/src/gl-compile-conf
index f497ae5..8508313 100755
--- i/src/gl-compile-conf
+++ w/src/gl-compile-conf
@@ -394,8 +394,9 @@ die "
 the server.  If it is not, please edit ~/.gitolite.rc on the server and
 set the \$GIT_PATH variable to the correct value\n
 " unless $git_version;
-my ($gv_maj, $gv_min, $gv_patchrel) = ($git_version =~ m/git version 
(\d+)\.(\d+)\.(\d+)/);
+my ($gv_maj, $gv_min, $gv_patchrel) = ($git_version =~ m/git version 
(\d+)\.(\d+)\.([^.-]*)/);
 die "$ABRT I can't understand $git_version\n" unless ($gv_maj >= 1);
+$gv_patchrel = 0 unless ($gv_patchrel =~ m/^\d+$/);
 $git_version = $gv_maj*1 + $gv_min*100 + $gv_patchrel;  # now it's 
"normalised"
 
 die "\n\t\t* AAARGH! *\n" .
--
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 3/3] builtin/blame.c: large-scale rewrite

2014-01-27 Thread David Kastrup
Junio C Hamano  writes:

> David Kastrup  writes:
>
>> The previous implementation uses a sorted linear list of struct
>> blame_entry in a struct scoreboard for organizing all partial or
>> completed work.  Every task that is done requires going through the
>> whole list where most entries are not relevant to the task at hand.
>>
>> This commit reorganizes the data structures in order to have each
>> remaining subtask work with its own sorted linear list it can work off
>> front to back.  Subtasks are organized into "struct origin" chains
>> hanging off particular commits.  Commits are organized into a priority
>> queue, processing them in commit date order in order to keep most of
>> the work affecting a particular blob collated even in the presence of
>> an extensive merge history.  In that manner, linear searches can be
>> basically avoided anywhere.  They still are done for identifying one
>> of multiple analyzed files in a given commit, but the degenerate case
>> of a single large file being assembled from a multitude of smaller
>> files in the past is not likely to occur often enough to warrant
>> special treatment.
>> ---
>
> Sign-off?

Not while this is not fit for merging because of #if 0 etc and missing
functionality.  This is just for review.

> Actually, I'd like to take my previous suggestion to add this as
> blame2 (to replace blame in the future) back.  That was based on my
> fear/hope to see an implementation based on a completely different
> approach, but the basic premise of having one blame_entry record per
> the lines of the final image in the scoreboard, using diff between
> parents to the child to find common lines for passing the blame up,
> etc. have not changed at all and the change is all about organizing
> the pieces of data in a *much* *better* way to avoid needlessly
> finding what we already have computed.

Yes.  Basically, the call graph outside of blame.c itself should be
pretty much the same.

> Style; please make /* and */ sit on their own line without anything
> else in multi-line comments.

Will do.

>> +struct origin *next;
>>  struct commit *commit;
>> +/* `suspects' contains blame entries that may be attributed to
>> + * this origin's commit or to parent commits.  When a commit
>> + * is being processed, all suspects will be moved, either by
>> + * assigning them to an origin in a different commit, or by
>> + * shipping them to the scoreboard's ent list because they
>> + * cannot be attributed to a different commit.
>> + */
>> +struct blame_entry *suspects;
>>  mmfile_t file;
>>  unsigned char blob_sha1[20];
>>  unsigned mode;
>> +/* shipped gets set when shipping any suspects to the final
>> + * blame list instead of other commits
>> + */
>> +char shipped;
>
> Unused?

More like "added, forgotten, added under yet another name":

>> +char guilty;

I'll have to decide which one to keep.

>> +/* Should be present exactly once in commit chain */
>> +for (p = o->commit->util; p; l = p, p = p->next) {
>> +if (p == o)
>> +{
>
> Style; please have { sit with the control structure that opens the
> block, unless it is the opening brace of the function body or
> struct/union definition.

Ok.

>> +static struct blame_entry *
>> +blame_merge(struct blame_entry *list1,
>> +struct blame_entry *list2)
>
> Style; we do not do GNU-ish "function name begins a line".  Even
> though I personally find it easier to use for things like 'grep',
> that is not the style we use around here.

Ok.  I'm also certain to have a few "space between function name and
arguments" left and will grep for those before submitting the next
version.

Next version will at least include -M option, possibly leaving -C for
later.

-- 
David Kastrup

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] repack.c: Use move_temp_to_file() instead of rename()

2014-01-27 Thread Torsten Bögershausen
In a1bbc6c0 a shell command "mv -f" was replaced with the rename() function.

Use move_temp_to_file() from sha1_file.c instead of rename().
This is in line with the handling of other Git internal tmp files,
and calls adjust_shared_perm()

Signed-off-by: Torsten Bögershausen 
---
Thanks for all comments.
I haven't been able to reproduce the problem here.
Tips and information how to reproduce it are wellcome.

I think this patch makes sense to support core.sharedRepository(),
but I haven't made a test case for the pack/idx files.

Jochen, doess this patch fix your problem, or do we need
another patch on top of this?

 builtin/repack.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index ba66c6e..4b6d663 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -271,7 +271,7 @@ int cmd_repack(int argc, const char **argv, const char 
*prefix)
if (unlink(fname_old))
failed = 1;
 
-   if (!failed && rename(fname, fname_old)) {
+   if (!failed && move_temp_to_file(fname, fname_old)) {
free(fname);
failed = 1;
break;
@@ -288,7 +288,7 @@ int cmd_repack(int argc, const char **argv, const char 
*prefix)
char *fname, *fname_old;
fname = mkpathdup("%s/%s", packdir, item->string);
fname_old = mkpath("%s/old-%s", packdir, item->string);
-   if (rename(fname_old, fname))
+   if (move_temp_to_file(fname_old, fname))
string_list_append(&rollback_failure, fname);
free(fname);
}
@@ -324,7 +324,7 @@ int cmd_repack(int argc, const char **argv, const char 
*prefix)
statbuffer.st_mode &= ~(S_IWUSR | S_IWGRP | 
S_IWOTH);
chmod(fname_old, statbuffer.st_mode);
}
-   if (rename(fname_old, fname))
+   if (move_temp_to_file(fname_old, fname))
die_errno(_("renaming '%s' failed"), fname_old);
free(fname);
free(fname_old);
-- 
1.8.5.2

--
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 v3 17/17] Documentation: add documentation for 'git interpret-trailers'

2014-01-27 Thread Christian Couder
From: Eric Sunshine 
>
> On Sun, Jan 26, 2014 at 12:00 PM, Christian Couder
>  wrote:
>> Signed-off-by: Christian Couder 
>> ---
>> diff --git a/Documentation/git-interpret-trailers.txt 
>> b/Documentation/git-interpret-trailers.txt
>> new file mode 100644
>> index 000..f74843e
>> --- /dev/null
>> +++ b/Documentation/git-interpret-trailers.txt
>> @@ -0,0 +1,137 @@
>> +git-interpret-trailers(1)
>> +=
>> +
>> +NAME
>> +
>> +git-interpret-trailers - help add stuctured information into commit messages
>> +
>> +SYNOPSIS
>> +
>> +[verse]
>> +'git interpret-trailers' [--trim-empty] [--infile=file] [...]
> 
> Would it be more consistent with existing documentation to format this as so?
> 
>   [--infile=] [[=]]...

No, it would be very inconsistent:

$ grep '\.\.\.\]' *.txt | wc -l
103
$ grep '\]\.\.\.' *.txt | wc -l
0

>> +DESCRIPTION
>> +---
>> +Help add RFC 822 like headers, called 'trailers', at the end of the
> 
> s/822 like/822-like/

Ok.

> Was the suggestion, early in the discussion, to use "footer" rather
> than "trailer" dismissed?

During the early discussions people initially talked about "footer"
while Junio and others talked about "trailer".

See:

http://thread.gmane.org/gmane.comp.version-control.git/236429/
http://thread.gmane.org/gmane.comp.version-control.git/236770/

I have no preference for one or the other, but by default I use what
Junio uses.

>> +otherwise free-form part of a commit message.
>> +
>> +Unless `--infile=file` is used, this command is a filter. It reads the
>> +standard input for a commit message and apply the `token` arguments,
> 
> s/apply/applies/

Ok.

>> +if any, to this message. The resulting message is emited on the
>> +standard output.
>> +
>> +Some configuration variables control the way the `token` arguments are
>> +applied to the message and the way any existing trailer in the message
>> +is changed. They also make it possible to automatically add some
>> +trailers.
>> +
>> +By default, a 'token=value' or 'token:value' argument will be added
>> +only if no trailer with the same (token, value) pair is already in the
>> +message. The 'token' and 'value' parts will be trimmed to remove
>> +starting and trailing white spaces, and the resulting trimmed 'token'
> 
> Other git documentation uniformly spells it as "whitespace" rather
> than "white spaces".

You are right I will change that.

>> +and 'value' will appear in the message like this:
>> +
>> +
>> +token: value
>> +
>> +
>> +By default, if there are already trailers with the same 'token' the
>> +trailer will appear just after the last trailer with the same
> 
> It might be a bit clearer to say "the _new_ trailer will appear...".

Ok.

>> +'token'. Otherwise it will appear at the end of the message.
>> +
>> +Note that 'trailers' do not follow and are not intended to follow many
>> +rules that are in RFC 822. For example they do not follow the line
>> +breaking rules, the encoding rules and probably many other rules.
>> +
>> +Trailers have become a de facto standard way to add helpful structured
>> +information into commit messages. For example the well known
>> +"Signed-off-by: " trailer is used by many projects like the Linux
>> +kernel and Git.
> 
> This "justification" paragraph might make more sense near or at the
> very the top of the Description section.

Yeah, or maybe I can remove it entirely.

>> +OPTIONS
>> +---
>> +--trim-empty::
>> +   If the 'value' part of any trailer contains onlywhite spaces,
> 
> s/onlywhite spaces/only whitespace/

Ok.

>> +   the whole trailer will be removed from the resulting message.
>> +
>> +infile=file::
>> +   Read the commit message from `file` instead of the standard
>> +   input.
>> +
>> +CONFIGURATION VARIABLES
>> +---
>> +
>> +trailer..key::
>> +   This 'key' will be used instead of 'token' in the
>> +   trailer. After some alphanumeric characters, it can contain
>> +   some non alphanumeric characters like ':', '=' or '#' that will
>> +   be used instead of ':' to separate the token from the value in
>> +   the trailer, though the default ':' is more standard.
>> +
>> +trailer..where::
>> +   This can be either `after`, which is the default, or
>> +   `before`. If it is `before`, then a trailer with the specified
>> +   token, will appear before, instead of after, other trailers
>> +   with the same token, or otherwise at the beginning, instead of
>> +   at the end, of all the trailers.
>> +
>> +trailer..ifexist::
>> +   This option makes it possible to chose what action will be
> 
> s/chose/choose/

Ok.

>> +   performed when there is already at least one trailer with the
>> +   same token in the message.
>> ++
>> +The valid values for this option are: `addIfDifferent` (this is the
>> +default), `addIfDifferentNeighbor`, `add`, `ove

Re: [PATCH 3/3] builtin/blame.c: large-scale rewrite

2014-01-27 Thread Junio C Hamano
David Kastrup  writes:

> Junio C Hamano  writes:
>
>> David Kastrup  writes:
>>
>>> The previous implementation uses a sorted linear list of struct
>>> blame_entry in a struct scoreboard for organizing all partial or
>>> completed work.  Every task that is done requires going through the
>>> whole list where most entries are not relevant to the task at hand.
>>>
>>> This commit reorganizes the data structures in order to have each
>>> remaining subtask work with its own sorted linear list it can work off
>>> front to back.  Subtasks are organized into "struct origin" chains
>>> hanging off particular commits.  Commits are organized into a priority
>>> queue, processing them in commit date order in order to keep most of
>>> the work affecting a particular blob collated even in the presence of
>>> an extensive merge history.  In that manner, linear searches can be
>>> basically avoided anywhere.  They still are done for identifying one
>>> of multiple analyzed files in a given commit, but the degenerate case
>>> of a single large file being assembled from a multitude of smaller
>>> files in the past is not likely to occur often enough to warrant
>>> special treatment.
>>> ---
>>
>> Sign-off?
>
> Not while this is not fit for merging because of #if 0 etc and missing
> functionality.  This is just for review.

That is not what Signing off a patch means, though ;-)

>> Actually, I'd like to take my previous suggestion to add this as
>> blame2 (to replace blame in the future) back.  That was based on my
>> fear/hope to see an implementation based on a completely different
>> approach, but the basic premise of having one blame_entry record per
>> the lines of the final image in the scoreboard, using diff between
>> parents to the child to find common lines for passing the blame up,
>> etc. have not changed at all and the change is all about organizing
>> the pieces of data in a *much* *better* way to avoid needlessly
>> finding what we already have computed.
>
> Yes.  Basically, the call graph outside of blame.c itself should be
> pretty much the same.
> ...
> Ok.  I'm also certain to have a few "space between function name and
> arguments" left and will grep for those before submitting the next
> version.
>
> Next version will at least include -M option, possibly leaving -C for
> later.

OK.  As we do not want to break the build in the middle of the
series, but we still want to keep the individual steps reviewable
incrementally, I would think the best structure would be have the
series that consists of multiple steps "the basic infrastructure is
there, but no rename following, and neither -M or -C works", "now
renames are followed", "now -M works", etc., with tests that are not
yet working (in the beginning, most of "git blame" test may no
longer work until the series is finished) marked with

s/test_expect_success/test_expect_failure/

and turn expect_failure into expect_success as the series advances.
That way, we may get a better picture of what each step achieves.  I
dunno.
--
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 v3 00/17] Add interpret-trailers builtin

2014-01-27 Thread Junio C Hamano
Is this from the same Christian?

The series seems to have unusually high rate of style violations
compared to the usual submission, like these:

ERROR: open brace '{' following function declarations go on the next line
#78: FILE: trailer.c:44:
+static size_t alnum_len(const char *buf, size_t len) {

ERROR: trailing statements should be on next line
#79: FILE: trailer.c:45:
+   while (--len >= 0 && !isalnum(buf[len]));

ERROR: space required before the open parenthesis '('
#70: FILE: trailer.c:90:
+   switch(arg_tok->conf->if_exist) {

WARNING: braces {} are not necessary for any arm of this statement
#66: FILE: trailer.c:270:
+   if (!strcasecmp("doNothing", value)) {
[...]
+   } else if (!strcasecmp("add", value)) {
[...]
+   } else
[...]

ERROR: that open brace { should be on the previous line
#96: FILE: trailer.c:300:
+   for (previous = NULL, item = first_conf_item;
+item;
+previous = item, item = item->next)
+   {

Just trying to see if there is an impersonation ;-)

--
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 v3 17/17] Documentation: add documentation for 'git interpret-trailers'

2014-01-27 Thread Junio C Hamano
Christian Couder  writes:

>>> +'git interpret-trailers' [--trim-empty] [--infile=file] 
>>> [...]
>> 
>> Would it be more consistent with existing documentation to format this as so?
>> 
>>   [--infile=] [[=]]...
>
> No, it would be very inconsistent:
>
> $ grep '\.\.\.\]' *.txt | wc -l
> 103
> $ grep '\]\.\.\.' *.txt | wc -l
> 0

I have a feeling that you are missing the point Eric is making.  The
value given to the --infile option can be anything, i.e. 'file'
there is a placeholder, hence "--infile=" not "--infile=file"
as you wrote.

Also I think "[[=]]..." is the correct way to spell
that there can be 0 or more "[=]".  ""
in the original does not make any sense, as <> is meant to say "this
thing is a placeholder", and we do not try to say, with the string
inside <>, what shape that placeholder takes.  In fact '=' part is
_not_ a placeholder but is required syntactically when you want to
supply a value to the token, so the original doubly is incorrect.

I find it a bad taste to allow unbound set of  on the LHS of
'=' on the command line, but that is a separate issue in the design,
not in the documentation of the design.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] builtin/blame.c: large-scale rewrite

2014-01-27 Thread David Kastrup
Junio C Hamano  writes:

> David Kastrup  writes:
>
>> Junio C Hamano  writes:
>>
>>> David Kastrup  writes:
>>>
 The previous implementation uses a sorted linear list of struct
 blame_entry in a struct scoreboard for organizing all partial or
 completed work.  Every task that is done requires going through the
 whole list where most entries are not relevant to the task at hand.

 This commit reorganizes the data structures in order to have each
 remaining subtask work with its own sorted linear list it can work off
 front to back.  Subtasks are organized into "struct origin" chains
 hanging off particular commits.  Commits are organized into a priority
 queue, processing them in commit date order in order to keep most of
 the work affecting a particular blob collated even in the presence of
 an extensive merge history.  In that manner, linear searches can be
 basically avoided anywhere.  They still are done for identifying one
 of multiple analyzed files in a given commit, but the degenerate case
 of a single large file being assembled from a multitude of smaller
 files in the past is not likely to occur often enough to warrant
 special treatment.
 ---
>>>
>>> Sign-off?
>>
>> Not while this is not fit for merging because of #if 0 etc and missing
>> functionality.  This is just for review.
>
> That is not what Signing off a patch means, though ;-)

>From Documentation/SubmittingPatches:

The sign-off is a simple line at the end of the explanation for
the patch, which certifies that you wrote it or otherwise have
the right to pass it on as a open-source patch.  The rules are
pretty simple: if you can certify the below:

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

Well, and the patch is not in a state where I want to contribute it to
this project.  Basically I first want to bring it into a state where
I am not embarrassed by having my name attached to it.

Yes, that's probably not quite the idea of signing off...

> OK.  As we do not want to break the build in the middle of the
> series, but we still want to keep the individual steps reviewable
> incrementally, I would think the best structure would be have the
> series that consists of multiple steps "the basic infrastructure is
> there, but no rename following, and neither -M or -C works", "now
> renames are followed", "now -M works", etc., with tests that are not
> yet working (in the beginning, most of "git blame" test may no
> longer work until the series is finished) marked with
>
> s/test_expect_success/test_expect_failure/
>
> and turn expect_failure into expect_success as the series advances.
> That way, we may get a better picture of what each step achieves.  I
> dunno.

Seems a bit pointless as the various functionalities are implemented in
separate code passages.  Basically the only "common" code is

static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
{
struct rev_info *revs = sb->revs;
int i, pass, num_sg;
struct commit *commit = origin->commit;
[...]

#if 0
/*
 * Optionally find moves in parents' files.
 */
if (opt & PICKAXE_BLAME_MOVE)
for (i = 0, sg = first_scapegoat(revs, commit);
 i < num_sg && sg;
 sg = sg->next, i++) {
struct origin *porigin = sg_origin[i];
if (!porigin)
continue;
if (find_move_in_parent(sb, origin, porigin))
goto finish;
}

/*
 * Optionally find copies from parents' files.
 */
if (opt & PICKAXE_BLAME_COPY)
for (i = 0, sg = first_scapegoat(revs, commit);
 i < num_sg && sg;
 sg = sg->next, i++) {
struct origin *porigin = sg_origin[i];
if (find_copy_in_parent(sb, origin, sg->item,
porigin, opt))
goto finish;
}
#endif

 finish:
for (i = 0; i < num_sg; i++) {
if (sg_origin[i]) {
drop_origin_blob(sg_origin[i]);
origin_decref(sg_origin[i]);
}
}
drop_origin_blob(origin);
if (sg_buf != sg_origin)
free(sg_origin);
}


So what will happen here is that the #if 0 will get removed again, and
there will be somewhat different code for

/*
 * Optionally find moves in parents' files.
 */

and then somewhat different code for

/*
 * Optionally find copies from parents' files.
 */

and some section elsewhere with the functions being called here.  It's
not like there will be significant int

[ANNOUNCE] Git v1.9-rc1

2014-01-27 Thread Junio C Hamano
The first release candidate for this cycle, Git v1.9-rc1, is now
available for testing at the usual places.

The release tarballs are found at:

http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

acc2343b4a0a0ed1920036fde1b1bf2109feb969  git-1.9.rc1.tar.gz
37bc0f5ef8f777a980304d58df003515364a54d0  git-htmldocs-1.9.rc1.tar.gz
c378ff4aca0737c9773181f4e97d36a8d8413e9a  git-manpages-1.9.rc1.tar.gz

The following public repositories all have a copy of the v1.9-rc1
tag and the master branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git v1.9 Release Notes (draft)
==

Backward compatibility notes


"git submodule foreach $cmd $args" used to treat "$cmd $args" the same
way "ssh" did, concatenating them into a single string and letting the
shell unquote. Careless users who forget to sufficiently quote $args
gets their argument split at $IFS whitespaces by the shell, and got
unexpected results due to this. Starting from this release, the
command line is passed directly to the shell, if it has an argument.

Read-only support for experimental loose-object format, in which users
could optionally choose to write in their loose objects for a short
while between v1.4.3 to v1.5.3 era, has been dropped.

The meanings of "--tags" option to "git fetch" has changed; the
command fetches tags _in addition to_ what are fetched by the same
command line without the option.

The way "git push $there $what" interprets $what part given on the
command line, when it does not have a colon that explicitly tells us
what ref at the $there repository is to be updated, has been enhanced.

A handful of ancient commands that have long been deprecated are
finally gone (repo-config, tar-tree, lost-found, and peek-remote).


Backward compatibility notes (for Git 2.0)
--

When "git push [$there]" does not say what to push, we have used the
traditional "matching" semantics so far (all your branches were sent
to the remote as long as there already are branches of the same name
over there).  In Git 2.0, the default will change to the "simple"
semantics, which pushes:

 - only the current branch to the branch with the same name, and only
   when the current branch is set to integrate with that remote
   branch, if you are pushing to the same remote as you fetch from; or

 - only the current branch to the branch with the same name, if you
   are pushing to a remote that is not where you usually fetch from.

Use the user preference configuration variable "push.default" to
change this.  If you are an old-timer who is used to the "matching"
semantics, you can set the variable to "matching" to keep the
traditional behaviour.  If you want to live in the future early, you
can set it to "simple" today without waiting for Git 2.0.

When "git add -u" (and "git add -A") is run inside a subdirectory and
does not specify which paths to add on the command line, it
will operate on the entire tree in Git 2.0 for consistency
with "git commit -a" and other commands.  There will be no
mechanism to make plain "git add -u" behave like "git add -u .".
Current users of "git add -u" (without a pathspec) should start
training their fingers to explicitly say "git add -u ."
before Git 2.0 comes.  A warning is issued when these commands are
run without a pathspec and when you have local changes outside the
current directory, because the behaviour in Git 2.0 will be different
from today's version in such a situation.

In Git 2.0, "git add " will behave as "git add -A ", so
that "git add dir/" will notice paths you removed from the directory
and record the removal.  Versions before Git 2.0, including this
release, will keep ignoring removals, but the users who rely on this
behaviour are encouraged to start using "git add --ignore-removal "
now before 2.0 is released.

The default prefix for "git svn" will change in Git 2.0.  For a long
time, "git svn" created its remote-tracking branches directly under
refs/remotes, but it will place them under refs/remotes/origin/ unless
it is told otherwise with its --prefix option.


Updates since v1.8.5


Foreign interfaces, subsystems and ports.

 * The HTTP transport, when talking GSS-Negotiate, uses "100
   Continue" response to avoid having to rewind and resend a large
   payload, which may not be always doable.

 * Various bugfixes to remote-bzr and remote-hg (in contrib/).

 * The build procedure is aware of MirBSD now.

 * Various "git p4", "git svn" and "gitk" updates.


UI, Workflows & Features

 * Fetching from a shallowly-cloned repository used to be forbidden,
   primarily because the codepaths involved were not

What's cooking in git.git (Jan 2014, #05; Mon, 27)

2014-01-27 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'.

The tip of 'master' is at 1.9-rc1; as far as new features are
concerned, this is pretty much it until the final.

On the maintenance track, we may have another 1.8.5.x maintenance
release before 1.9 final is tagged in 2-3 weeks.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* ab/subtree-doc (2014-01-13) 1 commit
  (merged to 'next' on 2014-01-22 at e352c75)
 + subtree: fix argument validation in add/pull/push


* ef/mingw-write (2014-01-17) 2 commits
  (merged to 'next' on 2014-01-22 at b9ddab2)
 + mingw: remove mingw_write
 + prefer xwrite instead of write


* jc/maint-pull-docfix (2014-01-14) 2 commits
  (merged to 'next' on 2014-01-22 at 2e62ef4)
 + Documentation: "git pull" does not have the "-m" option
 + Merge branch 'jc/maint-pull-docfix-for-409b8d82' into jc/maint-pull-docfix
 (this branch uses jc/maint-pull-docfix-for-409b8d82.)


* jc/revision-range-unpeel (2014-01-15) 2 commits
  (merged to 'next' on 2014-01-22 at ab2a159)
 + revision: propagate flag bits from tags to pointees
 + revision: mark contents of an uninteresting tree uninteresting

 "git log --left-right A...B" lost the "leftness" of commits
 reachable from A when A is a tag as a side effect of a recent
 bugfix.  This is a regression in 1.8.4.x series.


* jk/allow-fetch-onelevel-refname (2014-01-15) 1 commit
  (merged to 'next' on 2014-01-22 at 15089b1)
 + fetch-pack: do not filter out one-level refs

 "git clone" would fail to clone from a repository that has a ref
 directly under "refs/", e.g. "refs/stash", because different
 validation paths do different things on such a refname.  Loosen the
 client side's validation to allow such a ref.


* jk/complete-merge-base (2014-01-13) 2 commits
  (merged to 'next' on 2014-01-22 at 91c428d)
 + completion: handle --[no-]fork-point options to git-rebase
 + completion: complete merge-base options


* jk/diff-filespec-cleanup (2014-01-17) 5 commits
  (merged to 'next' on 2014-01-22 at fb26e43)
 + diff_filespec: use only 2 bits for is_binary flag
 + diff_filespec: reorder is_binary field
 + diff_filespec: drop xfrm_flags field
 + diff_filespec: drop funcname_pattern_ident field
 + diff_filespec: reorder dirty_submodule macro definitions


* jk/interpret-branch-name-fix (2014-01-15) 5 commits
  (merged to 'next' on 2014-01-22 at f113c68)
 + interpret_branch_name: find all possible @-marks
 + interpret_branch_name: avoid @{upstream} past colon
 + interpret_branch_name: always respect "namelen" parameter
 + interpret_branch_name: rename "cp" variable to "at"
 + interpret_branch_name: factor out upstream handling
 (this branch is used by jk/branch-at-publish-rebased.)

 Fix a handful of bugs around interpreting $branch@{upstream}
 notation and its lookalike, when $branch part has interesting
 characters, e.g. "@", and ":".


* jk/mark-edges-uninteresting (2014-01-21) 2 commits
  (merged to 'next' on 2014-01-22 at d8807c4)
 + list-objects: only look at cmdline trees with edge_hint
 + t/perf: time rev-list with UNINTERESTING commits

 Fix to regression in v1.8.4.x and later.


* jk/test-fixes (2014-01-23) 2 commits
  (merged to 'next' on 2014-01-23 at 6515089)
 + t7700: do not use "touch" unnecessarily
 + t7501: fix "empty commit" test with NO_PERL


* jn/ignore-doc (2014-01-16) 1 commit
  (merged to 'next' on 2014-01-22 at a07ac63)
 + gitignore doc: add global gitignore to synopsis

 Explicitly list $HOME/.config/git/ignore as one of the places you
 can use to keep ignore patterns that depend on your personal choice
 of tools, e.g. *~ for Emacs users.


* mh/attr-macro-doc (2014-01-14) 1 commit
  (merged to 'next' on 2014-01-22 at e3ed767)
 + gitattributes: document more clearly where macros are allowed


* mh/retire-ref-fetch-rules (2014-01-14) 1 commit
  (merged to 'next' on 2014-01-22 at 753b1f6)
 + refname_match(): always use the rules in ref_rev_parse_rules

 Code simplification.


* mh/safe-create-leading-directories (2014-01-21) 17 commits
  (merged to 'next' on 2014-01-22 at ad38e73)
 + rename_tmp_log(): on SCLD_VANISHED, retry
 + rename_tmp_log(): limit the number of remote_empty_directories() attempts
 + rename_tmp_log(): handle a possible mkdir/rmdir race
 + rename_ref(): extract function rename_tmp_log()
 + remove_dir_recurse(): handle disappearing files and directories
 + remove_dir_recurse(): tighten condition for removing unreadable dir
 + lock_ref_sha1_basic(): if locking fails with ENOENT, retry
 + lock_ref_sha1_basic(): on SCLD_VANISHED, retry
 + safe_create_leading_directories(): add new error value SCLD_VANISHED
 + cmd_init_db(): when creating directories, handle errors conservatively
 + safe_create_leading_directories(): introduce enu

Re: [PATCH v2 0/9] About the trailing slashes

2014-01-27 Thread Junio C Hamano
Thanks; will try to rebase on top of more recent codebase and then
review.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/3] diff: turn skip_stat_unmatch on selectively

2014-01-27 Thread Nguyễn Thái Ngọc Duy
skip_stat_unmatch flag is added in fb13227 (git-diff: squelch "empty"
diffs - 2007-08-03) to ignore empty diffs caused by stat-only
dirtiness. In some diff case, stat is not involved at all. While
the code is written in a way that no expensive I/O is done, we still
need to move all file pairs from the old queue to the new queue in
diffcore_skip_stat_unmatch().

Only enable it when worktree is involved: "diff" and "diff ".
This should help track down how skip_stat_unmatch is actually used
when bugs occur.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 This replaces 'diff: turn off skip_stat_unmatch on "diff --cached"'
 The previous patch obviously leaves skip_stat_unmatch on in "diff
  " and maybe other cases.

 builtin/diff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 0f247d2..88542d9 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -150,6 +150,7 @@ static int builtin_diff_index(struct rev_info *revs,
perror("read_cache_preload");
return -1;
}
+   revs->diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
} else if (read_cache() < 0) {
perror("read_cache");
return -1;
@@ -252,6 +253,7 @@ static int builtin_diff_files(struct rev_info *revs, int 
argc, const char **argv
perror("read_cache_preload");
return -1;
}
+   revs->diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
return run_diff_files(revs, options);
 }
 
@@ -343,7 +345,6 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
diff_no_index(&rev, argc, argv, prefix);
 
/* Otherwise, we are doing the usual "git" diff */
-   rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
 
/* Scale to real terminal size and respect statGraphWidth config */
rev.diffopt.stat_width = -1;
-- 
1.8.5.2.240.g8478abd

--
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: [ANNOUNCE] Git v1.9-rc0

2014-01-27 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Jan 23, 2014 at 10:15:33AM -0800, Junio C Hamano wrote:
>
>> Jeff King  writes:
>> 
>> > Junio, since you prepare such tarballs[1] anyway for kernel.org, it
>> > might be worth uploading them to the "Releases" page of git/git.  I
>> > imagine there is a programmatic way to do so via GitHub's API, but I
>> > don't know offhand. I can look into it if you are interested.
>> 
>> I already have a script that takes the three tarballs and uploads
>> them to two places, so adding GitHub as the third destination should
>> be a natural and welcome way to automate it.
>
> I came up with the script below, which you can use like:
>
>   ./script v1.8.2.3 git-1.8.2.3.tar.gz
>
> It expects the tag to already be pushed up to GitHub.  I'll leave
> sticking it on the "todo" branch and integrating it into RelUpload to
> you. This can also be used to backfill the old releases (though I looked
> on k.org and it seems to have only partial coverage).
>
> It sets the "prerelease" flag for -rc releases, but I did not otherwise
> fill in any fields, including the summary and description. GitHub seems
> to display reasonably if they are not set.

Thanks.

> -- >8 --
> #!/bin/sh
> #
> # usage: $0  
>
> repo=git/git
>
> # replace this with however you store your oauth token
> # if you don't have one, make one here:
> # https://github.com/settings/tokens/new
> token() {
>   pass -n github.web.oauth

Hmph, what is this "pass" thing?

> }
>
> post() {
>   curl -H "Authorization: token $(token)" "$@"
> }
>
> # usage: create 
> create() {
>   case "$1" in
>   *-rc*)
> prerelease=true
> ;;
>   *)
> prerelease=false
> ;;
>   esac
>
>   post -d '
>   {
> "tag_name": "'"$1"'",
> "prerelease": '"$prerelease"'
>   }' "https://api.github.com/repos/$repo/releases";
> }
>
> # use: upload  
> upload() {
>   url="https://uploads.github.com/repos/$repo/releases/$1/assets"; &&
>   url="$url?name=$(basename $2)" &&
>   post -H "Content-Type: $(file -b --mime-type "$2")" \
>--data-binary "@$2" \
>"$url"
> }
>
> # This is a hack. If you don't mind a dependency on
> # perl's JSON (or another parser), we can do a lot better.
> extract_id() {
>   perl -lne '/"id":\s*(\d+)/ or next; print $1; exit 0'
> }
>
> create "$1" >release.json &&
> id=$(extract_id  upload "$id" "$2" >/dev/null &&
> rm -f release.json
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/9] git-sh-setup.sh: add variable to use the stuck-long mode

2014-01-27 Thread Junio C Hamano
"brian m. carlson"  writes:

> From: Nicolas Vigier 
>
> If the variable $OPTIONS_STUCKLONG is not empty, then rev-parse
> option parsing is done in --stuck-long mode.
>
> Signed-off-by: Nicolas Vigier 
> Signed-off-by: brian m. carlson 
> ---
>  contrib/examples/git-checkout.sh | 1 +
>  contrib/examples/git-clean.sh| 1 +
>  contrib/examples/git-clone.sh| 1 +
>  contrib/examples/git-merge.sh| 1 +
>  contrib/examples/git-repack.sh   | 1 +

Hmmm, especially given that nobody is exercising the ``updated''
code even in our testsuite, is it a good idea to touch these?

The STUCKLONG never existed back when they were scripted.  It just
adds noise when trying to view them for hints, no?

> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index fffa3c7..5f28b32 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -72,6 +72,8 @@ if test -n "$OPTIONS_SPEC"; then
>   parseopt_extra=
>   [ -n "$OPTIONS_KEEPDASHDASH" ] &&
>   parseopt_extra="--keep-dashdash"
> + [ -n "$OPTIONS_STUCKLONG" ] &&
> + parseopt_extra="$parseopt_extra --stuck-long"
>  
>   eval "$(
>   echo "$OPTIONS_SPEC" |
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 4/9] am: add the --gpg-sign option

2014-01-27 Thread Junio C Hamano
"brian m. carlson"  writes:

> @@ -900,7 +906,8 @@ did you forget to use 'git add'?"
>   GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"
>   export GIT_COMMITTER_DATE
>   fi &&
> - git commit-tree $tree ${parent:+-p} $parent 
> <"$dotest/final-commit"
> + git commit-tree ${gpg_sign_opt:+"$gpg_sign_opt"} $tree 
> ${parent:+-p} $parent \
> + <"$dotest/final-commit"

Yuck.  This is unsightly.

It used to be that "commit-tree" was an oddball that took a
mandatory argument $tree always as its first thing, and required
zero or more "-p $parent" after and it did not take any other
options (no -m, no -F, no -S, no nothing).

The original is written in such a way that honors that odd
tradition, because it was how it was written originally, and it did
not need to pass any other option to require the new style
invocation (see "git commit-tree --help" and look at the SYNOPSIS
section).

Now you are passing dash-options to the command, I think it is time
to update the call to the modern style even in the default, non-signed
case.

git commit-tree ${parent:+-p} $parent ${gpg_sign_opt:+"$gpg_sign_opt"} 
$tree




>   ) &&
>   git update-ref -m "$GIT_REFLOG_ACTION: $FIRSTLINE" HEAD $commit $parent 
> ||
>   stop_here $this
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 9/9] pull: add the --gpg-sign option.

2014-01-27 Thread Junio C Hamano
"brian m. carlson"  writes:

> git merge already allows us to sign commits, and git rebase has recently
> learned how to do so as well.  Teach git pull to parse the -S/--gpg-sign
> option and pass this along to merge or rebase, as appropriate.
>
> Signed-off-by: brian m. carlson 
> ---
>  git-pull.sh | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 0a5aa2c..4164dac 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -138,6 +138,15 @@ do
>   --no-verify-signatures)
>   verify_signatures=--no-verify-signatures
>   ;;
> + --gpg-sign|-S)
> + gpg_sign_args=-S
> + ;;
> + --gpg-sign=*)
> + gpg_sign_args="-S${1#--gpg-sign=}"
> + ;;

Here, $1 is taken from the end-user without any extra quoting...

> + -S*)
> + gpg_sign_args="-S${1#-S}"
> + ;;
>   --d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
>   dry_run=--dry-run
>   ;;
> @@ -305,11 +314,13 @@ merge_name=$(git fmt-merge-msg $log_arg 
> <"$GIT_DIR/FETCH_HEAD") || exit
>  case "$rebase" in
>  true)
>   eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args 
> $verbosity"
> + eval="$eval $gpg_sign_args"

... but here it is used as if it is properly quoted so that later
"eval $eval" will take it as a single argument.

git pull --gpg-sign='foo bar'

will probably ask the command to use 'foo' as the signer key id,
with 'bar' as an extra, unknown token on the command line of the
underlying 'git merge', I suspect.  A "git rev-parse --sq-quote"
in the earlier hunk may be all it takes to fix it.

Thanks.

>   eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}"
>   ;;
>  *)
>   eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash 
> $no_ff $ff_only"
> - eval="$eval  $log_arg $strategy_args $merge_args $verbosity $progress"
> + eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
> + eval="$eval $gpg_sign_args"
>   eval="$eval \"\$merge_name\" HEAD $merge_head"
>   ;;
>  esac
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 8/9] rebase: add the --gpg-sign option

2014-01-27 Thread Junio C Hamano
"brian m. carlson"  writes:

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 43c19e0..73d32dd 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -181,7 +181,7 @@ exit_with_patch () {
>   git rev-parse --verify HEAD > "$amend"
>   warn "You can amend the commit now, with"
>   warn
> - warn "  git commit --amend"
> + warn "  git commit --amend $gpg_sign_opt"

I think this is meant to be cut-&-pasted, so you may have to quote
the RHS of --gpg-sign= (or the key part of -S).

The same comment as the one for 'git pull' patch applies around
'eval's in the remainder of the patch.

> @@ -248,7 +248,8 @@ pick_one () {
>  
>   test -d "$rewritten" &&
>   pick_one_preserving_merges "$@" && return
> - output eval git cherry-pick "$strategy_args" $empty_args $ff "$@"
> + output eval git cherry-pick ${gpg_sign_opt:+"$gpg_sign_opt"} \
> + "$strategy_args" $empty_args $ff "$@"
>  }
> [rest snipped]

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 3/3] diff: turn skip_stat_unmatch on selectively

2014-01-27 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> skip_stat_unmatch flag is added in fb13227 (git-diff: squelch "empty"
> diffs - 2007-08-03) to ignore empty diffs caused by stat-only
> dirtiness. In some diff case, stat is not involved at all. While
> the code is written in a way that no expensive I/O is done, we still
> need to move all file pairs from the old queue to the new queue in
> diffcore_skip_stat_unmatch().
>
> Only enable it when worktree is involved: "diff" and "diff ".
> This should help track down how skip_stat_unmatch is actually used
> when bugs occur.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  This replaces 'diff: turn off skip_stat_unmatch on "diff --cached"'
>  The previous patch obviously leaves skip_stat_unmatch on in "diff
>   " and maybe other cases.

Oops, I lost track.  Sorry.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] rev-parse: Check argc before using argv[i+1]

2014-01-27 Thread David Sharp
Without this patch, git-rev-parse --prefix, --default, or
--resolve-git-dir, without a value argument, would result in a segfault.
Instead, die() with a message.

Signed-off-by: David Sharp 
---
 builtin/rev-parse.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index aaeb611..3bf65c5 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -547,15 +547,17 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, "--default")) {
-   def = argv[i+1];
-   i++;
+   if (++i >= argc)
+   die("--default requires an argument");
+   def = argv[i];
continue;
}
if (!strcmp(arg, "--prefix")) {
-   prefix = argv[i+1];
+   if (++i >= argc)
+   die("--prefix requires an argument");
+   prefix = argv[i];
startup_info->prefix = prefix;
output_prefix = 1;
-   i++;
continue;
}
if (!strcmp(arg, "--revs-only")) {
@@ -738,9 +740,11 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, "--resolve-git-dir")) {
-   const char *gitdir = resolve_gitdir(argv[i+1]);
+   if (++i >= argc)
+   die("--resolve-git-dir requires an 
argument");
+   const char *gitdir = resolve_gitdir(argv[i]);
if (!gitdir)
-   die("not a gitdir '%s'", argv[i+1]);
+   die("not a gitdir '%s'", argv[i]);
puts(gitdir);
continue;
}
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 8/9] rebase: add the --gpg-sign option

2014-01-27 Thread brian m. carlson
On Mon, Jan 27, 2014 at 03:36:44PM -0800, Junio C Hamano wrote:
> "brian m. carlson"  writes:
> 
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index 43c19e0..73d32dd 100644
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -181,7 +181,7 @@ exit_with_patch () {
> > git rev-parse --verify HEAD > "$amend"
> > warn "You can amend the commit now, with"
> > warn
> > -   warn "  git commit --amend"
> > +   warn "  git commit --amend $gpg_sign_opt"
> 
> I think this is meant to be cut-&-pasted, so you may have to quote
> the RHS of --gpg-sign= (or the key part of -S).
> 
> The same comment as the one for 'git pull' patch applies around
> 'eval's in the remainder of the patch.
> 
> > @@ -248,7 +248,8 @@ pick_one () {
> >  
> > test -d "$rewritten" &&
> > pick_one_preserving_merges "$@" && return
> > -   output eval git cherry-pick "$strategy_args" $empty_args $ff "$@"
> > +   output eval git cherry-pick ${gpg_sign_opt:+"$gpg_sign_opt"} \
> > +   "$strategy_args" $empty_args $ff "$@"
> >  }
> > [rest snipped]
> 
> Thanks.

Thanks for the review.  I'll get working on a reroll.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [ANNOUNCE] Git v1.9-rc0

2014-01-27 Thread Jeff King
On Mon, Jan 27, 2014 at 02:56:28PM -0800, Junio C Hamano wrote:

> > # replace this with however you store your oauth token
> > # if you don't have one, make one here:
> > # https://github.com/settings/tokens/new
> > token() {
> >   pass -n github.web.oauth
> 
> Hmph, what is this "pass" thing?

It's a poor man's Keychain:

  https://github.com/peff/pass

Judging from your use of netrc in Meta/RelUpload, you probably just
want:

  token() {
cat ~/.github-token
  }

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jan 2014, #05; Mon, 27)

2014-01-27 Thread Jeff King
On Mon, Jan 27, 2014 at 02:09:52PM -0800, Junio C Hamano wrote:

> [Stalled]
> 
> * jk/color-for-more-pagers (2014-01-17) 4 commits
>  - pager: disable colors for some known-bad configurations
>  - DONOTMERGE: needs matching change to git-sh-setup
>  - setup_pager: set MORE=R
>  - setup_pager: refactor LESS/LV environment setting
> 
>  'more' implementation of BSD wants to be told with MORE=R
>  environment before it shows colored output, while 'more' on some
>  other platforms will die when seeing MORE=R environment.
> 
>  It appears that we are coming to the consensus that trying to be
>  too intimately knowledgeable about quirks of various pager
>  implementations on different platforms is a losing proposition, so
>  perhaps I should replace this with the alternative and simpler
>  approach I posted, unless there are strong objections.

It saddens me that we can't better help people who have a custom
LESS/MORE/LV set out of the box, but I think I agree that the intimacy
with the pagers was just getting too ugly. So I agree we want to drop
what you have there.

I was planning to try to clean up the "like this..." patch to the
Makefile I posted earlier, but haven't gotten around to it. It seemed
like you were positive on that approach. Let me see if I can do that.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Added get sendmail from .mailrc

2014-01-27 Thread Jeff King
On Sun, Jan 26, 2014 at 09:17:09AM +, Eric Wong wrote:

> > > > +use File::HomeDir;
> > > 
> > > We should probably avoid a new dependency and also remain consistent
> > > with the rest of git handles home directories.
> > > 
> > > Unfortunately, expand_user_path()/git_config_pathname() isn't currently
> > > exposed to scripters right now...
> > 
> > Ok, if new dependency is not allowed I see next ways:
> 
> Not saying it's not allowed.  I meant we should probably expose
> expand_user_path()/git_config_pathname() C functions to script helpers
> (so git-config or git-rev-parse can provide them to sh or perl scripts).

I do not think we need anything so complex. Most of the logic in
expand_user_path is about handling "~" and "~user". But here we _just_
want to know the current user's home directory, and for that
expand_user_path always just looks in $HOME.

So I think $ENV{HOME} would be fine to match what git does. My
understanding is that File::HomeDir does some magic that may work better
on non-Unix platforms. I do not know if we even care for this feature,
since .mailrc is presumably a Unix thing. But if we do, I think our
usual strategy with such things is to optionally use the dependency if
available, and fall back to something sane. Like:

  sub homedir {
if (eval { require File::HomeDir; 1 }) {
return File::HomeDir->my_home;
}
return $ENV{HOME};
  }

Whichever code path is followed, you should probably also check the
result for "undef", which the original patch did not do.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Added get sendmail from .mailrc

2014-01-27 Thread Jeff King
On Sat, Jan 25, 2014 at 01:46:50PM +0400, Brilliantov Kirill Vladimirovich 
wrote:

> + if (!defined $smtp_server) {
> + my $mailrc = File::HomeDir->my_home . "/.mailrc";

The new module dependency has been discussed elsewhere in the thread.

> + if (-e $mailrc) {
> + open FILE, $mailrc or die "Failed open $mailrc: $!";

Please use the three-argument of 'open', and use a regular scalar
instead of a glob. Both are safer, and we assume a modern enough perl to
support both. I.e.:

  open(my $file, '<', $mailrc)
  or die "failed to open $mailrc: $!";

> + while () {
> + chomp;
> + if (/set sendmail=.*/) {
> + my @data = split '"';
> + $smtp_server = $data[1];
> + last;
> + }

Your split is a rather unusual way to do the parsing, and it took me a
minute to figure it out. It might be more obvious as:

  if (/set sendmail="(.*)"/) {
  $smtp_server = $1;
  last;
  }

I do not know anything about the mailrc format, nor does it seem to be
well documented. Are the double-quotes required? If not, then the above
regex can easily make them optional. I also wonder if any whitespace is
allowed. E.g., this might be more forgiving:

  /set sendmail\s*=\s*"?(.*?)"?/

but I am just guessing at what the format allows.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] expand_user_path: do not look at NULL path

2014-01-27 Thread Jeff King
We explicitly check for and handle the case that the
incoming "path" variable is NULL, but before doing so we
call strchrnul on it, leading to a potential segfault.

We can fix this simply by moving the strchrnul call down; as
a bonus, we can tighten the scope on the associated
variable.

Signed-off-by: Jeff King 
---
Most of the callers already check for NULL, so the only way to trigger
this is via an empty "include.path". That is addressed separately in the
next patch, so technically this is not needed if we apply that one. But
in that case, the "path == NULL" check here is useless, so I think this
makes things safer overall.

 path.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/path.c b/path.c
index 24594c4..f9c5062 100644
--- a/path.c
+++ b/path.c
@@ -265,12 +265,12 @@ static struct passwd *getpw_str(const char *username, 
size_t len)
 char *expand_user_path(const char *path)
 {
struct strbuf user_path = STRBUF_INIT;
-   const char *first_slash = strchrnul(path, '/');
const char *to_copy = path;
 
if (path == NULL)
goto return_null;
if (path[0] == '~') {
+   const char *first_slash = strchrnul(path, '/');
const char *username = path + 1;
size_t username_len = first_slash - username;
if (username_len == 0) {
-- 
1.8.5.2.500.g8060133

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] handle_path_include: don't look at NULL value

2014-01-27 Thread Jeff King
When we see config like:

  [include]
  path

the expand_user_path helper notices that the config value is
empty, but we then dereference NULL while printing the error
message (glibc will helpfully print "(null)" for us here,
but we cannot rely on that).

  $ git -c include.path rev-parse
  error: Could not expand include path '(null)'
  fatal: unable to parse command-line config

Instead of tweaking our message, let's actually use
config_error_nonbool to match other config variables that
expect a value:

  $ git -c include.path rev-parse
  error: Missing value for 'include.path'
  fatal: unable to parse command-line config

Signed-off-by: Jeff King 
---
 config.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index d969a5a..314d8ee 100644
--- a/config.c
+++ b/config.c
@@ -84,8 +84,12 @@ static int handle_path_include(const char *path, struct 
config_include_data *inc
 {
int ret = 0;
struct strbuf buf = STRBUF_INIT;
-   char *expanded = expand_user_path(path);
+   char *expanded;
 
+   if (!path)
+   return config_error_nonbool("include.path");
+
+   expanded = expand_user_path(path);
if (!expanded)
return error("Could not expand include path '%s'", path);
path = expanded;
-- 
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] repack: add `repack.honorpackkeep` config var

2014-01-27 Thread Jeff King
On Thu, Jan 23, 2014 at 06:44:43PM -0800, Siddharth Agarwal wrote:

> On 01/23/2014 06:28 PM, Jeff King wrote:
> >I think your understanding is accurate here. So we want repack to
> >respect keep files for deletion, but we _not_ necessarily want
> >pack-objects to avoid packing an object just because it's in a pack
> >marked by .keep (see my other email).
> 
> Yes, that makes sense and sounds pretty safe.
> 
> So the right solution for us probably is to apply the patch Vicent
> posted, set repack.honorpackkeep to false, and also have a cron job
> that cleans up stale .keep files so that subsequent repacks clean it
> up.

Yes, that matches what we do at GitHub.

Here's Vicent's patch, with documentation and an expanded commit
message. I think it should be suitable for upstream git.

-- >8 --
From: Vicent Marti 
Subject: repack: add `repack.honorpackkeep` config var

The git-repack command always passes `--honor-pack-keep`
to pack-objects. This has traditionally been a good thing,
as we do not want to duplicate those objects in a new pack,
and we are not going to delete the old pack.

However, when bitmaps are in use, it is important for a full
repack to include all reachable objects, even if they may be
duplicated in a .keep pack. Otherwise, we cannot generate
the bitmaps, as the on-disk format requires the set of
objects in the pack to be fully closed.

Even if the repository does not generally have .keep files,
a simultaneous push could cause a race condition in which a
.keep file exists at the moment of a repack. The repack may
try to include those objects in one of two situations:

  1. The pushed .keep pack contains objects that were
 already in the repository (e.g., blobs due to a revert of
 an old commit).

  2. Receive-pack updates the refs, making the objects
 reachable, but before it removes the .keep file, the
 repack runs.

In either case, we may prefer to duplicate some objects in
the new, full pack, and let the next repack (after the .keep
file is cleaned up) take care of removing them.

This patch introduces an option to disable the
`--honor-pack-keep` option.  It is not triggered by default,
even when pack.writeBitmaps is turned on, because its use
depends on your overall packing strategy and use of .keep
files.

Note that this option just disables the pack-objects
behavior. We still leave packs with a .keep in place, as we
do not necessarily know that we have duplicated all of their
objects.

Signed-off-by: Jeff King 
---
Intended for the jk/pack-bitmap topic.

 Documentation/config.txt | 8 
 builtin/repack.c | 8 +++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 947e6f8..5036a10 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2128,6 +2128,14 @@ repack.usedeltabaseoffset::
"false" and repack. Access from old Git versions over the
native protocol are unaffected by this option.
 
+repack.honorPackKeep::
+   If set to false, include objects in `.keep` files when repacking
+   via `git repack`. Note that we still do not delete `.keep` packs
+   after `pack-objects` finishes. This means that we may duplicate
+   objects, but this makes the option safe to use when there are
+   concurrent pushes or fetches. This option is generally only
+   useful if you have set `pack.writeBitmaps`. Defaults to true.
+
 rerere.autoupdate::
When set to true, `git-rerere` updates the index with the
resulting contents after it cleanly resolves conflicts using
diff --git a/builtin/repack.c b/builtin/repack.c
index a9c4593..585c41d 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -9,6 +9,7 @@
 #include "argv-array.h"
 
 static int delta_base_offset = 1;
+static int honor_pack_keep = 1;
 static char *packdir, *packtmp;
 
 static const char *const git_repack_usage[] = {
@@ -22,6 +23,10 @@ static int repack_config(const char *var, const char *value, 
void *cb)
delta_base_offset = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "repack.honorpackkeep")) {
+   honor_pack_keep = git_config_bool(var, value);
+   return 0;
+   }
return git_default_config(var, value, cb);
 }
 
@@ -190,10 +195,11 @@ int cmd_repack(int argc, const char **argv, const char 
*prefix)
 
argv_array_push(&cmd_args, "pack-objects");
argv_array_push(&cmd_args, "--keep-true-parents");
-   argv_array_push(&cmd_args, "--honor-pack-keep");
argv_array_push(&cmd_args, "--non-empty");
argv_array_push(&cmd_args, "--all");
argv_array_push(&cmd_args, "--reflog");
+   if (honor_pack_keep)
+   argv_array_push(&cmd_args, "--honor-pack-keep");
if (window)
argv_array_pushf(&cmd_args, "--window=%u", window);
if (window_memory)
-- 
1.8.5.2.500.g8060133

--
To unsubscribe from this list: send

Re: [PATCH] Added get sendmail from .mailrc

2014-01-27 Thread Kyle J. McKay

On Jan 27, 2014, at 17:15, Jeff King wrote:
On Sat, Jan 25, 2014 at 01:46:50PM +0400, Brilliantov Kirill  
Vladimirovich wrote:



+   if (!defined $smtp_server) {
+   my $mailrc = File::HomeDir->my_home . "/.mailrc";


Actually, based on the output of "man mail", this should probably be  
something more like


  my $mailrc = $ENV{'MAILRC'} || "$ENV{'HOME'}/.mailrc";

which takes into account any MAILRC setting and also avoids the use of  
File::HomeDir.



+   while () {
+   chomp;
+   if (/set sendmail=.*/) {
+   my @data = split '"';
+   $smtp_server = $data[1];
+   last;
+   }


Your split is a rather unusual way to do the parsing, and it took me a
minute to figure it out. It might be more obvious as:

 if (/set sendmail="(.*)"/) {
 $smtp_server = $1;
 last;
 }

I do not know anything about the mailrc format, nor does it seem to be
well documented. Are the double-quotes required? If not, then the  
above
regex can easily make them optional. I also wonder if any whitespace  
is

allowed.


From "man mail":

   set   (se) With no arguments, prints all variable values.   
Otherwise,

 sets option.  Arguments are of the form option=value (no space
 before or after `=') or option.  Quotation marks may be placed
 around any part of the assignment statement to quote blanks or
 tabs, i.e. ``set indentprefix="->"''

My version of "man mail" does not list all the variables that can be  
set but it refers to "The Mail Reference Manual" document which  
presumably does.  I did find this [1] that documents many of the  
available variables including the sendmail one.  I then tried this:


cat < /tmp/shim
#!/bin/sh
exec cat
EOF
chmod a+x /tmp/shim
cat < /tmp/testrc
  se   send"mail"=/tm"p/"shim
EOF
echo 'testing' | MAILRC=/tmp/testrc mail -s test nobody

And to my surprise the contents of the new message were cat'd out to  
the terminal rather than being sent.  So clearly there's some room for  
improvement with the "set", white space and quote checking.


[1] http://www.cs.fsu.edu/sysinfo/mail/mailrc.html

--Kyle
--
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