Repo with only one file

2013-08-06 Thread shawn wilson
I started writing this script in a repo I have called misc-scripts
where I just keep one off projects and the like (notes, throw away
scripts, etc). Well, my boss asked me to create a repo for one of
these scripts and I'd like to keep the commit history.

Ok, so:
% find -type f ! -iname "webban.pl" | while read f; do git
filter-branch -f --index-filter "git rm --cached --ignore-unmatch $f"
HEAD ; done

Which basically did it. But, I've got this one commit that seems to be
orphaned - it doesn't change any files. That is, it shows up in a git
log but not a git whatchanged. When I try to rebase -p --onto, I get
tons of conflicts, git adding blah in every other line of the file and
after I totally mess up the repo, that commit is still there. If I do
a git checkout and try to amend the message, I get:

# Not currently on any branch.
#
# Initial commit
#
# No changes
You asked to amend the most recent commit, but doing so would make
it empty. You can repeat your command with --allow-empty, or you can
remove the commit entirely with "git reset HEAD^".

iWhen I do --allow-empty, I can no longer see any other commits.

So, how do I remove this commit or what is the proper way to get this
one file into a repo with nothing else?

Also, I sign my commits and would like to keep each commit signed if
at all possible.
--
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] replace: forbid replacing an object with one of a different type

2013-08-06 Thread Christian Couder
Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

The doc will be updated in a later patch.

Signed-off-by: Christian Couder 
---
If this patch is considered useful, I will update the doc and
maybe add tests.

 builtin/replace.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..0246ab3 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char 
*replace_ref,
  int force)
 {
unsigned char object[20], prev[20], repl[20];
+   enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
 
@@ -100,6 +101,14 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
if (check_refname_format(ref, 0))
die("'%s' is not a valid ref name.", ref);
 
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(repl, NULL);
+   if (obj_type != repl_type)
+   die("Object ref '%s' is of type '%s'\n"
+   "while replace ref '%s' is of type '%s'.",
+   object_ref, typename(obj_type),
+   replace_ref, typename(repl_type));
+
if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
-- 
1.8.4.rc1.22.g132b1a0

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


Re: [PATCH] git exproll: steps to tackle gc aggression

2013-08-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> Imagine we have a cheap way to enumerate the young objects without
> the usual history traversal.

Before we discuss the advantages, can you outline how we can possibly
get this data without actually walking downwards from the roots
(refs)? One way to do it is to pull data out of a log of ref updates
(aka. reflog), but we both know how unreliable that can be.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git exproll: steps to tackle gc aggression

2013-08-06 Thread Ramkumar Ramachandra
Martin Fick wrote:
> So, it has me wondering if there isn't a more accurate way
> to estimate the new packfile without wasting a ton of time?

I'm not sure there is. Adding the sizes of individual packs can be off
by a lot, because your deltification will be more effective if you
have more data to slide windows over and compress. For the purposes of
illustration, take a simple example:

packfile-1 has a 30M Makefile and several tiny deltas. Total = 40M.
packfile-2 has a 31M Makefile.um and several tiny deltas. Total = 40M.

Now, what is the size of packfile-3 which contains the contents of
both packfile-1 and packfile-2? 80M is a bad estimate, because you can
store deltas against just one Makefile.

So, unless you do an in-depth analysis of the objects in the packfiles
(which can be terribly expensive), I don't see how you can arrive at a
better estimate.

> If not, one approach which might be worth experimenting with
> is to just assume that new packfiles have size 0!  Then just
> consolidate them with any other packfile which is ready for
> consolidation, or if none are ready, with the smallest
> packfile.  I would not be surprised to see this work on
> average better than the current summation,

That is assuming that all fetches (or pushes) are small, which is
probably a good rule; you might like to have a "smallness threshold",
although I haven't thought hard about the problem.
--
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: change remote to track new branch

2013-08-06 Thread David Aguilar
On Tue, Aug 6, 2013 at 5:30 PM, Daniel Convissor
 wrote:
> Hi Folks:
>
> On Sat, Aug 03, 2013 at 12:52:15PM -0400, Daniel Convissor wrote:
>>
>> Yeah.  I had contemplated using the following commands:
>>
>> git config remote.wp.fetch \
>> "+refs/heads/3.6-branch:refs/remotes/wp/3.6-branch"
>> git config branch.wp.merge "refs/heads/3.6-branch"
>>
>> So is "git remote set-branches" and "git branch --set-upstream-to" just
>> another syntax for making those same changes to git config?  Or do the
>> new commands do some additional work on the repository (to better keep
>> track of things, or whatever)?
>
> Sorry to be a pest, but I'm curious what the answer is, please.

There's really nothing more to it.

There is one other setting -- "branch.wp.remote" should be set to "wp".

If you edit your .git/config by hand you'll see what's really going
on.  It's quite simple under the hood.
-- 
David
--
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: change remote to track new branch

2013-08-06 Thread Daniel Convissor
Hi Folks:

On Sat, Aug 03, 2013 at 12:52:15PM -0400, Daniel Convissor wrote:
> 
> Yeah.  I had contemplated using the following commands:
> 
> git config remote.wp.fetch \
> "+refs/heads/3.6-branch:refs/remotes/wp/3.6-branch"
> git config branch.wp.merge "refs/heads/3.6-branch"
> 
> So is "git remote set-branches" and "git branch --set-upstream-to" just
> another syntax for making those same changes to git config?  Or do the
> new commands do some additional work on the repository (to better keep
> track of things, or whatever)?

Sorry to be a pest, but I'm curious what the answer is, please.

Thank you,

--Dan

-- 
 T H E   A N A L Y S I S   A N D   S O L U T I O N S   C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/
4015 7th Ave #4, Brooklyn NY 11232  v: 718-854-0335
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git exproll: steps to tackle gc aggression

2013-08-06 Thread Martin Fick
On Monday, August 05, 2013 08:38:47 pm Ramkumar Ramachandra 
wrote:
> This is the rough explanation I wrote down after reading
> it:
> 
> So, the problem is that my .git/objects/pack is polluted
> with little packs everytime I fetch (or push, if you're
> the server), and this is problematic from the
> perspective of a overtly (naively) aggressive gc that
> hammers out all fragmentation.  So, on the first run,
> the little packfiles I have are all "consolidated" into
> big packfiles; you also write .keep files to say that
> "don't gc these big packs we just generated".  In
> subsequent runs, the little packfiles from the fetch are
> absorbed into a pack that is immune to gc.  You're also
> using a size heuristic, to consolidate similarly sized
> packfiles.  You also have a --ratio to tweak the ratio
> of sizes.
> 
> From: Martin Fick
> See: https://gerrit-review.googlesource.com/#/c/35215/
> Thread:
> http://thread.gmane.org/gmane.comp.version-control.git/2
> 31555 (Martin's emails are missing from the archive)
> ---

After analyzing today's data, I recognize that in some 
circumstances the size estimation after consolidation can be 
off by huge amounts.  The script naively just adds the 
current sizes together.  This gives a very rough estimate, 
of the new packfile size, but sometimes it can be off by 
over 2 orders of magnitude. :(  While many new packfiles are 
tiny (several K only), it seems like the larger new 
packfiles have a terrible tendency to throw the estimate way 
off (I suspect they simply have many duplicate objects).  
But despite this poor estimate, the script still offers 
drastic improvements over plain git gc.

So, it has me wondering if there isn't a more accurate way 
to estimate the new packfile without wasting a ton of time?

If not, one approach which might be worth experimenting with 
is to just assume that new packfiles have size 0!  Then just 
consolidate them with any other packfile which is ready for 
consolidation, or if none are ready, with the smallest 
packfile.  I would not be surprised to see this work on 
average better than the current summation,

-Martin


-- 
The Qualcomm Innovation Center, Inc. is a member of Code 
Aurora Forum, hosted by The Linux Foundation
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git exproll: steps to tackle gc aggression

2013-08-06 Thread Martin Fick
On Tuesday, August 06, 2013 06:24:50 am Duy Nguyen wrote:
> On Tue, Aug 6, 2013 at 9:38 AM, Ramkumar Ramachandra 
 wrote:
> > +   Garbage collect using a pseudo
> > logarithmic packfile maintenance +  
> > approach.  This approach attempts to minimize packfile
> > churn +   by keeping several generations
> > of varying sized packfiles around +   and
> > only consolidating packfiles (or loose objects) which
> > are +   either new packfiles, or packfiles
> > close to the same size as +   another
> > packfile.
> 
> I wonder if a simpler approach may be nearly efficient as
> this one: keep the largest pack out, repack the rest at
> fetch/push time so there are at most 2 packs at a time.
> Or we we could do the repack at 'gc --auto' time, but
> with lower pack threshold (about 10 or so). When the
> second pack is as big as, say half the size of the
> first, merge them into one at "gc --auto" time. This can
> be easily implemented in git-repack.sh.

It would definitely be better than the current gc approach.  

However, I suspect it is still at least one to two orders of 
magnitude off from where it should be.  To give you a real 
world example, on our server today when gitexproll ran on 
our kernel/msm repo, it consolidated 317 pack files into one 
almost 8M packfile (it compresses/dedupes shockingly well, 
one of those new packs was 33M).  Our largest packfile in 
that repo is 1.5G!  

So let's now imagine that the second closest packfile is 
only 100M, it would keep getting consolidated with 8M worth 
of data every day (assuming the same conditions and no extra 
compression).  That would take (750M-100M)/8M ~ 81 days to 
finally build up large enough to no longer consolidate the 
new packs with the second largest pack file daily.  During 
those 80+ days, it will be on average writing 325M too much 
per day (when it should be writing just 8M).

So I can see the appeal of a simple solution, unfortunately 
I think one layer would still "suck" though.  And if you are 
going to add even just one extra layer, I suspect that you 
might as well go the full distance since you probably 
already need to implement the logic to do so?

-Martin

-- 
The Qualcomm Innovation Center, Inc. is a member of Code 
Aurora Forum, hosted by The Linux Foundation
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/4] Update built-in parseopt macros

2013-08-06 Thread Junio C Hamano
As a follow-up to Stefan's series, we may want to think about the
following as well:

 - OPT__VERBOSE() is defined in terms of OPT_BOOLEAN().  I think we
   use it to represent increasing levels of verbosity, so we cannot
   turn this into OPT_BOOL().  Its implementation has to become
   OPT_COUNTUP().

 - OPT__QUIET() is defined in terms of OPT_BOOLEAN().  I offhand do
   not know if we have increasing levels of quietness.  The users
   need auditing before we can decide to turn this into either
   OPT_COUNTUP() or OPT_BOOL().

 - OPT__DRY_RUN() should probably become OPT_BOOL().

 - OPT_FORCE(); do we have levels of forcefulness?  If so
   OPT_COUNTUP(), otherwise OPT_BOOL().

And here is my attempt.  The last one is iffy as I didn't bother
auditing the existing callers.

Junio C Hamano (4):
  OPT__QUIET(): switch from count-up to true bool
  OPT__VERBOSE(): clarify its expected use by using OPT_COUNTUP
  OPT__DRY_RUN(): use OPT_BOOL, not OPT_BOOLEAN
  OPT__FORCE(): clarify its expected use by using OPT_COUNTUP

 parse-options.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
1.8.4-rc1-210-gf6d87e2

--
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/4] OPT__QUIET(): switch from count-up to true bool

2013-08-06 Thread Junio C Hamano
The parseopt parsing for OPT__QUIET() is implemented in terms of
OPT_BOOLEAN aka OPT_COUNTUP, so a user _could_ theoretically have
used it to make "git cmd -q -q" and "git cmd -q" behave differently.

However, no existing user does so (a summary of the audit at the
end).  Use OPT_BOOL to make sure our choices are either 0 or 1.

builtin/branch.c:

quiet is passed to create_branch() in branch.c and
delete_branches().  The former passes it to setup_tracking()
which is used as a bool to decide use of BRANCH_CONFIG_VERBOSE.
The latter uses it as a bool to give a single printf() for
reporting the names of deleted branches.

builtin/check-ignore.c:

all users of quiet use it as a bool.

builtin/checkout-index.c:

quiet is assigned to state.quite and only the latter is used
throughout the program.  It is a single-bit bitfield.

builtin/checkout.c:

quiet is stored in checkout_opts.quiet which is of type int.  It
is used in many places:

- reset_tree() uses it as a bool;

- merge_working_tree() uses it twice, as a bool at both
  places;

- update_refs_for_switch() uses it three times, all as a bool.
  It also passes it to create_branch() which we already verified
  above.

- switch_branches() and switch_unborn_to_new_branch() use it
  once each, as a bool at both places.

Signed-off-by: Junio C Hamano 
---
 parse-options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index c378b75..f2b01ee 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -231,7 +231,7 @@ extern int parse_opt_string_list(const struct option *, 
const char *, int);
 extern int parse_opt_noop_cb(const struct option *, const char *, int);
 
 #define OPT__VERBOSE(var, h)  OPT_BOOLEAN('v', "verbose", (var), (h))
-#define OPT__QUIET(var, h)OPT_BOOLEAN('q', "quiet",   (var), (h))
+#define OPT__QUIET(var, h)OPT_BOOL('q', "quiet",   (var), (h))
 #define OPT__VERBOSITY(var) \
{ OPTION_CALLBACK, 'v', "verbose", (var), NULL, N_("be more verbose"), \
  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }, \
-- 
1.8.4-rc1-210-gf6d87e2

--
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/4] OPT__VERBOSE(): clarify its expected use by using OPT_COUNTUP

2013-08-06 Thread Junio C Hamano
The parseopt parsing for OPT__VERBOSE() is implemented in terms of
OPT_BOOLEAN() and users of it do take advantage of the "counting up"
behaviour to implement increasing levels of verbosity by
differentiating "git cmd -v" and "git cmd -v -v".

Clarify this by explicitly using OPT_COUNTUP() instead.

Signed-off-by: Junio C Hamano 
---
 parse-options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index f2b01ee..582dd4b 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -230,7 +230,7 @@ extern int parse_opt_tertiary(const struct option *, const 
char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
 extern int parse_opt_noop_cb(const struct option *, const char *, int);
 
-#define OPT__VERBOSE(var, h)  OPT_BOOLEAN('v', "verbose", (var), (h))
+#define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', "verbose", (var), (h))
 #define OPT__QUIET(var, h)OPT_BOOL('q', "quiet",   (var), (h))
 #define OPT__VERBOSITY(var) \
{ OPTION_CALLBACK, 'v', "verbose", (var), NULL, N_("be more verbose"), \
-- 
1.8.4-rc1-210-gf6d87e2

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


[PATCH 3/4] OPT__DRY_RUN(): use OPT_BOOL, not OPT_BOOLEAN

2013-08-06 Thread Junio C Hamano
There cannot be "git cmd -n -n" that behaves even less drier than
"git cmd -n", and no existing users of the macro implements such
a semantics (a summary of the audit at the end).

Instead of OPT_BOOLEAN, use OPT_BOOL to clarify.

builtin/add.c:

uses "show_only" as a bool.

builtin/clean.c:

uses "dry_run" as a bool, and passes it to the parameter
with the same name of remove_dirs(), which is also used as a
bool.

builtin/mv.c:

uses "show_only" as a bool.

builtin/notes.c:

uses "show_only" as a bool.

builtin/prune.c:

uses "show_only" as a bool (including its use as a bool to
decide use of PRUNE_PACKED_DRY_RUN as a parameter to
prune_packed_objects()).

builtin/read-tree.c:

opts.dry_run is used as a bool, in this file and also in
unpack_trees() that is called from here.

builtin/remote.c:

dry_run is passed to prune_remote() and used as a bool.

builtin/rm.c:

show_only is used as a bool.

Signed-off-by: Junio C Hamano 
---
 parse-options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index 582dd4b..78f52c2 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -237,7 +237,7 @@ extern int parse_opt_noop_cb(const struct option *, const 
char *, int);
  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }, \
{ OPTION_CALLBACK, 'q', "quiet", (var), NULL, N_("be more quiet"), \
  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }
-#define OPT__DRY_RUN(var, h)  OPT_BOOLEAN('n', "dry-run", (var), (h))
+#define OPT__DRY_RUN(var, h)  OPT_BOOL('n', "dry-run", (var), (h))
 #define OPT__FORCE(var, h)OPT_BOOLEAN('f', "force",   (var), (h))
 #define OPT__ABBREV(var)  \
{ OPTION_CALLBACK, 0, "abbrev", (var), N_("n"), \
-- 
1.8.4-rc1-210-gf6d87e2

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


[RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP

2013-08-06 Thread Junio C Hamano
The parseopt parsing for OPT__FORCE() is implemented in terms of
OPT_BOOLEAN() and users of it can take advantage of the "counting
up" behaviour to implement increasing levels of forcefulness by
differentiating "git cmd -f" and "git cmd -f -f".

Clarify this by explicitly using OPT_COUNTUP() instead.

Signed-off-by: Junio C Hamano 
---

 * This _should_ be done with a similar audit of existing callers,
   but I ran out of concentration.

 parse-options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index 78f52c2..1eeb0d9 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -238,7 +238,7 @@ extern int parse_opt_noop_cb(const struct option *, const 
char *, int);
{ OPTION_CALLBACK, 'q', "quiet", (var), NULL, N_("be more quiet"), \
  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }
 #define OPT__DRY_RUN(var, h)  OPT_BOOL('n', "dry-run", (var), (h))
-#define OPT__FORCE(var, h)OPT_BOOLEAN('f', "force",   (var), (h))
+#define OPT__FORCE(var, h)OPT_COUNTUP('f', "force",   (var), (h))
 #define OPT__ABBREV(var)  \
{ OPTION_CALLBACK, 0, "abbrev", (var), N_("n"), \
  N_("use  digits to display SHA-1s"),   \
-- 
1.8.4-rc1-210-gf6d87e2

--
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: [[TIG][PATCH v2] 2/3] Display correct diff the context in split log view

2013-08-06 Thread Kumar Appaiah
On Tue, Aug 06, 2013 at 12:58:20AM -0400, Kumar Appaiah wrote:
>  tig-1.1
>  ---
> diff --git a/tig.c b/tig.c
> index 845153f..256b589 100644
> --- a/tig.c
> +++ b/tig.c
> @@ -4475,8 +4475,15 @@ log_request(struct view *view, enum request request, 
> struct line *line)
>   state->recalculate_commit_context = TRUE;
>   return request;
>  
> + case REQ_ENTER:
> + state->recalculate_commit_context = TRUE;
> + if (VIEW(REQ_VIEW_DIFF)->ref != ref_commit)
> + open_view(view, REQ_VIEW_DIFF, OPEN_SPLIT);
> + update_view_title(view);
^^^

I missed removing update_view_title. I've done it locally, though.

Thanks.

Kumar

> + return request;
> +
>   default:
> - return pager_request(view, request, line);
> + return request;
>   }
>  }
>  
> -- 
> 1.8.3.2

-- 
Kumar Appaiah
--
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 05/16] blame: accept multiple -L ranges

2013-08-06 Thread Junio C Hamano
Eric Sunshine  writes:

> Each constructed blame_entry must own a reference to the suspect.
> o->refcnt should equal the number of blame_entries. At construction, a
> 'struct origin' has refcnt 1. In the original code, which supported
> only a single initial range (blame_entry), we had:
>
>   o = get-initial-suspect();  # refcnt already 1
>   ent->suspect = o;  # refcnt still 1

Ah, of course.

I forgot that I initialized a new origin with refcnt 1 exactly for
this.  As you use it once for each range, you would need to
compensate for it.

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 05/16] blame: accept multiple -L ranges

2013-08-06 Thread Eric Sunshine
On Tue, Aug 6, 2013 at 5:33 PM, Junio C Hamano  wrote:
>> + for (range_i = ranges.nr; range_i > 0; --range_i) {
>> + const struct range *r = &ranges.ranges[range_i - 1];
>> + long bottom = r->start;
>> + long top = r->end;
>> + struct blame_entry *next = ent;
>> + ent = xcalloc(1, sizeof(*ent));
>> + ent->lno = bottom;
>> + ent->num_lines = top - bottom;
>> + ent->suspect = o;
>> + ent->s_lno = bottom;
>> + ent->next = next;
>> + if (next)
>> + next->prev = ent;
>> + origin_incref(o);
>> + }
>> + origin_decref(o);
>
> Hmph, I do not see where the need for this decref is coming from.
> Did we incref once too many somewhere?

Each constructed blame_entry must own a reference to the suspect.
o->refcnt should equal the number of blame_entries. At construction, a
'struct origin' has refcnt 1. In the original code, which supported
only a single initial range (blame_entry), we had:

  o = get-initial-suspect();  # refcnt already 1
  ent->suspect = o;  # refcnt still 1
  sb.ent = ent;
  assign_blame(&sb);

So, o->refcnt equals the number of blame_entries (1) when
assign_blame() is called.

The new for-loop calls origin_incref() on each iteration since each
blame_entry needs to own a reference to the suspect. Assume that we
have two disjoint -L ranges:

  o = get-initial-suspect();  # refcnt already 1
  foreach range:
ent = new blame_entry;
ent->suspect = o;
origin_incref(o);  # refcnt++
  end
  # for 2 ranges, refcnt incremented twice, so value is 3
  origin_decref(o);  # refcnt = 2
  sb.ent = ent;
  assign_blame(&sb);

Thus, as with the original code, o->refcnt equals the number of
blame_entries (2) when assign_blame() is called.

The same holds for the boundary case when the file is empty and there
is no range. o->refcnt starts at 1, the loop is never entered so no
blame_entries are created, and o->refcnt gets decremented to 0, which
again matches the number of blame_entries (0).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] fetch: opportunistically update tracking refs

2013-08-06 Thread Junio C Hamano
Jeff King  writes:

> Two reasons:

OK, both boils down to "Junio did not consider 'master:foobar'
case".

Thanks; it makes sense now.
--
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 08/16] line-range: teach -L/RE/ to search relative to anchor point

2013-08-06 Thread Junio C Hamano
Eric Sunshine  writes:

> Range specification -L/RE/ for blame/log unconditionally begins
> searching at line one. Mailing list discussion [1] suggests that, in the
> presence of multiple -L options, -L/RE/ should search relative to the
> endpoint of the previous -L range, if any.
>
> Teach the parsing machinery underlying blame's and log's -L options to
> accept a start point for -L/RE/ searches. Follow-up patches will upgrade
> blame and log to take advantage of this ability.
>
> [1]: 
> http://thread.gmane.org/gmane.comp.version-control.git/229755/focus=229966
>
> Signed-off-by: Eric Sunshine 
> ---
>  builtin/blame.c |  2 +-
>  line-log.c  |  2 +-
>  line-range.c| 30 ++
>  line-range.h|  5 -
>  4 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 2f4d9e2..7b084d8 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2479,7 +2479,7 @@ parse_done:
>   for (range_i = 0; range_i < range_list.nr; ++range_i) {
>   long bottom, top;
>   if (parse_range_arg(range_list.items[range_i].string,
> - nth_line_cb, &sb, lno,
> + nth_line_cb, &sb, lno, 1,
>   &bottom, &top, sb.path))
>   usage(blame_usage);
>   if (lno < top || ((lno || bottom) && lno < bottom))
> diff --git a/line-log.c b/line-log.c
> index bdadf35..38f827b 100644
> --- a/line-log.c
> +++ b/line-log.c
> @@ -591,7 +591,7 @@ parse_lines(struct commit *commit, const char *prefix, 
> struct string_list *args)
>   cb_data.line_ends = ends;
>  
>   if (parse_range_arg(range_part, nth_line, &cb_data,
> - lines, &begin, &end,
> + lines, 1, &begin, &end,

This one feeds "1" to anchor, which in turn is given to parse_loc as
"-1" and then (after bypassing the  part support) its sign
flipped once again to become "begin=1" in parse_loc().  Then we run
regexp search starting from (1-based) 1st line, retaining the
"always scan from the beginning" behaviour in this step in the
series.

OK, looks sensible.
--
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 4/4] fetch: opportunistically update tracking refs

2013-08-06 Thread Jeff King
On Tue, Aug 06, 2013 at 09:28:28AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > @@ -170,6 +172,20 @@ static struct ref *get_ref_map(struct transport 
> > *transport,
> > rm->fetch_head_status = FETCH_HEAD_MERGE;
> > if (tags == TAGS_SET)
> > get_fetch_map(remote_refs, tag_refspec, &tail, 0);
> > +
> > +   /*
> > +* For any refs that we happen to be fetching via command-line
> > +* arguments, take the opportunity to update their configured
> > +* counterparts. However, we do not want to mention these
> > +* entries in FETCH_HEAD at all, as they would simply be
> > +* duplicates of existing entries.
> > +*/
> > +   old_tail = tail;
> > +   for (i = 0; i < transport->remote->fetch_refspec_nr; i++)
> > +   get_fetch_map(ref_map, &transport->remote->fetch[i],
> > + &tail, 0);
> > +   for (rm = *old_tail; rm; rm = rm->next)
> > +   rm->fetch_head_status = FETCH_HEAD_IGNORE;
> 
> Was there a reason why this change was done by appending new ref at
> the tail of the ref_map list?  I would have expected that a naïve
> and obvious implementation would be to iterate existing refs over
> ref_map to find refs with an empty RHS whose LHS is configured to
> usually store the fetched result to somewhere and to update their
> RHS in place.
> 
> Being curious.

Two reasons:

  1. The implementation you suggest above behaves differently than the
 current code. It does not look for refspecs with an empty RHS. It
 looks for any LHS that matches our configured entries. So if you do
 "git fetch origin master:foobar", we will update both
 "refs/heads/foobar" as well as "refs/remotes/origin/master".

 That means it is purely an opportunistic "hey, during another
 operation we happened to find out something new about
 origin/master, so let's update our tracking branch". Whereas what
 you stated above is more like "we are fetching into FETCH_HEAD, so
 let's also update the tracking branch".

  2. The list of refs after get_ref_map is actually more of an
 instruction/command list for the rest of the code to follow. It
 gets fed to store_updated_ref, but also impacts the status table we
 show. You could implement it such that a single ref entry had
 multiple storage destinations, but that would require changes to
 all of the consumers of the instruction list. Since we already need
 to handle extra refspecs (e.g., you can say "master:foobar
 master:refs/remotes/origin/master" on the command line already), we
 can treat these the same way. We append to the instruction list,
 and the rest of the code treats it as if you specified it on the
 command line.

-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 v2 09/16] blame: teach -L/RE/ to search from end of previous -L range

2013-08-06 Thread Junio C Hamano
Eric Sunshine  writes:

> Signed-off-by: Eric Sunshine 
> ---

With the previous step, what this one does is fairly obvious and
straight-forward.  Looking good ;-)

>  builtin/blame.c |  5 -
>  t/annotate-tests.sh | 20 
>  2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 7b084d8..1bf8056 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2280,6 +2280,7 @@ int cmd_blame(int argc, const char **argv, const char 
> *prefix)
>   int cmd_is_annotate = !strcmp(argv[0], "annotate");
>   struct range_set ranges;
>   unsigned int range_i;
> + long anchor;
>  
>   git_config(git_blame_config, NULL);
>   init_revisions(&revs, NULL);
> @@ -2475,11 +2476,12 @@ parse_done:
>   if (lno && !range_list.nr)
>   string_list_append(&range_list, xstrdup("1"));
>  
> + anchor = 1;
>   range_set_init(&ranges, range_list.nr);
>   for (range_i = 0; range_i < range_list.nr; ++range_i) {
>   long bottom, top;
>   if (parse_range_arg(range_list.items[range_i].string,
> - nth_line_cb, &sb, lno, 1,
> + nth_line_cb, &sb, lno, anchor,
>   &bottom, &top, sb.path))
>   usage(blame_usage);
>   if (lno < top || ((lno || bottom) && lno < bottom))
> @@ -2490,6 +2492,7 @@ parse_done:
>   top = lno;
>   bottom--;
>   range_set_append_unsafe(&ranges, bottom, top);
> + anchor = top + 1;
>   }
>   sort_and_merge_range_set(&ranges);
>  
> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
> index 77083d9..b963d36 100644
> --- a/t/annotate-tests.sh
> +++ b/t/annotate-tests.sh
> @@ -303,6 +303,26 @@ test_expect_success 'blame -L multiple (superset/subset: 
> unordered)' '
>   check_count -L3,5 -L2,8 A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1
>  '
>  
> +test_expect_success 'blame -L /RE/ (relative)' '
> + check_count -L3,3 -L/fox/ B1 1 B2 1 C 1 D 1 "A U Thor" 1
> +'
> +
> +test_expect_success 'blame -L /RE/ (relative: no preceding range)' '
> + check_count -L/dog/ A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1
> +'
> +
> +test_expect_success 'blame -L /RE/ (relative: adjacent)' '
> + check_count -L1,1 -L/dog/,+1 A 1 E 1
> +'
> +
> +test_expect_success 'blame -L /RE/ (relative: not found)' '
> + test_must_fail $PROG -L4,4 -L/dog/ file
> +'
> +
> +test_expect_success 'blame -L /RE/ (relative: end-of-file)' '
> + test_must_fail $PROG -L, -L/$/ file
> +'
> +
>  test_expect_success 'setup -L :regex' '
>   tr Q "\\t" >hello.c <<-\EOF &&
>   int main(int argc, const char *argv[])
--
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 05/16] blame: accept multiple -L ranges

2013-08-06 Thread Junio C Hamano
> + for (range_i = ranges.nr; range_i > 0; --range_i) {
> + const struct range *r = &ranges.ranges[range_i - 1];
> + long bottom = r->start;
> + long top = r->end;
> + struct blame_entry *next = ent;
> + ent = xcalloc(1, sizeof(*ent));
> + ent->lno = bottom;
> + ent->num_lines = top - bottom;
> + ent->suspect = o;
> + ent->s_lno = bottom;
> + ent->next = next;
> + if (next)
> + next->prev = ent;
> + origin_incref(o);
> + }
> + origin_decref(o);

Hmph, I do not see where the need for this decref is coming from.
Did we incref once too many somewhere?

> + range_set_release(&ranges);
> + string_list_clear(&range_list, 0);
>  
>   sb.ent = ent;
>   sb.path = path;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] rm: delete .gitmodules entry of submodules removed from the work tree

2013-08-06 Thread Junio C Hamano
Thanks, will replace the top two commits and queue.  Looks like we
are getting ready for 'next'?
--
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 revised] git_mkstemps: add test suite test

2013-08-06 Thread Junio C Hamano
wor...@alum.mit.edu (Dale R. Worley) writes:

>> git commit --allow-empty -m message <&-
>
> Though as of [fb56570] "Sync with maint to grab trivial doc fixes",
> that test doesn't fail for me if I revert to
>
>   fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
>   if (fd > 0)
>   return fd;
>
> I haven't been watching the code changes carefully; has there been a
> fix that is expected to cause that?
>
> Dale

That is because a11c3964 (git: ensure 0/1/2 are open in main(),
2013-07-16) happened in the meantime, I think.

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


[PATCH] branch, commit, name-rev: ease up boolean conditions

2013-08-06 Thread Stefan Beller
Now that the variables are set by OPT_BOOL, which makes sure
to have the values being 0 or 1 after parsing, we do not need
the double negation to map any other value to 1 for integer
variables.

Signed-off-by: Stefan Beller 
---
 builtin/branch.c   | 3 ++-
 builtin/commit.c   | 2 +-
 builtin/name-rev.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 4daed0b..0dca694 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
if (with_commit || merge_filter != NO_FILTER)
list = 1;
 
-   if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + 
!!unset_upstream > 1)
+   if (force_create + list + unset_upstream +
+   !!delete + !!rename + !!new_upstream > 1)
usage_with_options(builtin_branch_usage, options);
 
if (abbrev == -1)
diff --git a/builtin/commit.c b/builtin/commit.c
index c20426b..b0f86c8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const 
char *argv[],
if (patch_interactive)
interactive = 1;
 
-   if (!!also + !!only + !!all + !!interactive > 1)
+   if (also + only + all + interactive > 1)
die(_("Only one of --include/--only/--all/--interactive/--patch 
can be used."));
if (argc == 0 && (also || (only && !amend)))
die(_("No paths with --include/--only does not make sense."));
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a908a34..20fcf8c 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char 
*prefix)
 
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
-   if (!!all + !!transform_stdin + !!argc > 1) {
+   if (all + transform_stdin + !!argc > 1) {
error("Specify either a list, or --all, not both!");
usage_with_options(name_rev_usage, opts);
}
-- 
1.8.4.rc0.16.g7fca822.dirty

--
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] branch, commit, name-rev: ease up boolean conditions

2013-08-06 Thread Stefan Beller
On 08/06/2013 08:46 PM, Eric Sunshine wrote:
> On Tue, Aug 6, 2013 at 9:07 AM, Stefan Beller
>  wrote:
>> Now that the variables are readin by OPT_BOOL, which makes sure
> 
> Do you mean s/readin/read in/ ?
> 
> Or should it be s/readin/set/ ?
> 
>> to have the values being 0 or 1 after reading, we do not need
>> the double negation to map any other value to 1 for integer
>> variables.
>>
>> Signed-off-by: Stefan Beller 

I think s/readin/set/ is best.
Also s/after reading/after parsing/



signature.asc
Description: OpenPGP digital signature


Re: git rebase -i error message interprets \t in commit message

2013-08-06 Thread Junio C Hamano
David Kastrup  writes:

> Matthieu Moy  writes:
>
>>>From 7962ac8d8f2cbc556f669fd97487f9d70edc4ea1 Mon Sep 17 00:00:00 2001
>> From: Matthieu Moy 
>> Date: Tue, 6 Aug 2013 19:13:03 +0200
>> Subject: [PATCH] die_with_status: use "printf '%s\n'", not "echo"
>>
>> At least GNU echo interprets backslashes in its arguments.
>
> I think that's incorrect in several respects.  For one thing, echo is
> never called for most Bourne shells since echo is a builtin (might have
> been different for UNIX version 7 or so).  For another, GNU echo would
> behave like Bash:
>
> And GNU Bash does not interpret escapes unless you do echo -e.  Escape
> sequence interpretation, however, happens for Dash:
>
> dak@lola:/usr/local/tmp/lilypond$ dash -c 'echo "x\tx"'
> x x
> dak@lola:/usr/local/tmp/lilypond$ bash -c 'echo "x\tx"'
> x\tx
> dak@lola:/usr/local/tmp/lilypond$ /bin/echo "x\tx"
> x\tx
>
> So replace "GNU echo" in your commit message with "Dash's echo builtin"
> and you get closer.

I'll queue the attached.

POSIX makes it an implementation defined behaviour when the first
argument is "-n", or any argument contains a backslas (X/Open System
Interfaces wants to treat "-n" as literal and always interpret the
backslash sequence), so it is definitely safer to avoid running
'echo' on any random string.

Thanks.

Author: Matthieu Moy 
Date:   Tue Aug 6 20:26:44 2013 +0200

die_with_status: use "printf '%s\n'", not "echo"

Some implementations of 'echo' (e.g. dash's built-in) interpret
backslash sequences in their arguments.

This triggered at least one bug: the error message of "rebase -i" was
turning \t in commit messages into actual tabulations. There may be
others.

Using "printf '%s\n'" instead avoids this bad behavior, and is the form
used by the "say" function.

Noticed-by: David Kastrup 
Signed-off-by: Matthieu Moy 
Signed-off-by: Junio C Hamano 

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a964ad..e15be51 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -53,7 +53,7 @@ die () {
 die_with_status () {
status=$1
shift
-   echo >&2 "$*"
+   printf >&2 '%s\n' "$*"
exit "$status"
 }
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 49ccb38..074deb1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -976,4 +976,17 @@ test_expect_success 'rebase -i with --strategy and -X' '
test $(cat file1) = Z
 '
 
+test_expect_success 'rebase -i error on commits with \ in message' '
+   current_head=$(git rev-parse HEAD)
+   test_when_finished "git rebase --abort; git reset --hard $current_head; 
rm -f error" &&
+   test_commit TO-REMOVE will-conflict old-content &&
+   test_commit "\temp" will-conflict new-content dummy &&
+   (
+   EDITOR=true &&
+   export EDITOR &&
+   test_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error
+   ) &&
+   grep -v "   " error
+'
+
 test_done
--
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: git rebase -i error message interprets \t in commit message

2013-08-06 Thread David Kastrup
Matthieu Moy  writes:

>>From 7962ac8d8f2cbc556f669fd97487f9d70edc4ea1 Mon Sep 17 00:00:00 2001
> From: Matthieu Moy 
> Date: Tue, 6 Aug 2013 19:13:03 +0200
> Subject: [PATCH] die_with_status: use "printf '%s\n'", not "echo"
>
> At least GNU echo interprets backslashes in its arguments.

I think that's incorrect in several respects.  For one thing, echo is
never called for most Bourne shells since echo is a builtin (might have
been different for UNIX version 7 or so).  For another, GNU echo would
behave like Bash:

And GNU Bash does not interpret escapes unless you do echo -e.  Escape
sequence interpretation, however, happens for Dash:

dak@lola:/usr/local/tmp/lilypond$ dash -c 'echo "x\tx"'
x   x
dak@lola:/usr/local/tmp/lilypond$ bash -c 'echo "x\tx"'
x\tx
dak@lola:/usr/local/tmp/lilypond$ /bin/echo "x\tx"
x\tx

So replace "GNU echo" in your commit message with "Dash's echo builtin"
and you get closer.

-- 
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] SubmittingPatches: clarify some details of the patch format

2013-08-06 Thread Junio C Hamano
wor...@alum.mit.edu (Dale R. Worley) writes:

> ---
> This is a first draft of a patch that clarifies a number of points
> about how patches should be formatted that have tripped me up.  I have
> re-filled a few of the paragraphs, which makes it hard to see from the
> diff what I've changed.  This listing shows the changed words between
> { ... }:
>
>  { This first line should be a separate paragraph, that is, it should be
> followed by an empty line, which is then followed by the body of the
> commit message } .

Correct; I tend to prefer phrasing that avoid "that is", though.

the first line should be followed by an empty line to form a
separate paragraph on its own and give a summary of the change.

>  { At the end of the commit message should be a Signed-off-by line giving
> your name.  This can be added to the commit message automatically by
> giving "git commit" the "--signoff" option.  This line has legal
> implications, see "Sign your work" below } .

OK.

> People on the Git mailing list need to be able to read and comment on
> the changes you are submitting.  It is important for a developer to be
> able to "quote" your changes, using standard e-mail tools, so that
> they may comment on specific portions of your code.  For this reason,
> all patches should be submitted "inline" { rather than as message
> attachments } .  If your log message (including your name on the
> Signed-off-by line) is not writable in ASCII, make sure that you send
> off { the } message in the correct encoding.

OK.

> "git format-patch" command follows the best current practice to
> format the { patch for transmission as an e-mail message.  The files
> that it outputs are sample e-mail messages in "mh" format.  The

Not "mh", but "mbox".

> initial lines are sample From, To, Date, and Subject headers, which
> you will likely have to remove or revise, depending on how your MUA
> operates } .

Correct.  They are designed to be "moved" to your MUA header fields
(so they will disappear from the body but you do not have to type
them to your MUA).
>
> At the beginning of the patch should come your commit message ( { the
> first line in the Subject header, the remainder in the body of the
> message } ), ending with the Signed-off-by: lines, and a line that
> consists of three dashes, followed by the diffstat information and the
> patch itself.  If you are forwarding a patch from somebody else,
> optionally, at the beginning of the e-mail message just before the
> commit message starts, you can put a "From: " line to name that
> person.

... followed by an empty line?

> You often want to add additional explanation about the patch,
> other than the commit message itself.  Place such "cover letter"
> material between the three-dash { line } and the diffstat. Git-notes
> can also be inserted using the `--notes` option.

OK.

>> From: Junio C Hamano 
>> 
>> I am not sure if SubmittingPatches is a good place, though.
>> The document is a guidance for people who contribute to _this_
>> project.
>> 
>> But the specialness of the first paragraph applies to any project
>> that uses Git, so people other than those who contribute to this
>> project should be aware of it.
>
> All of that is true.  But what can we do to ensure that someone who
> submits a patch does so with the right format?  The special treatment
> of the first line is not a requirement.  See, e.g., the git-commit
> manual page:
>
>Though not required, it’s a good idea to begin the commit message with
>a single short (less than 50 character) line summarizing the change,
>followed by a blank line and then a more thorough description. Tools
>that turn commits into email, for example, use the first line on the
>Subject: line and the rest of the commit in the body.

This is one of the thing I explained in the "Originally we literally
used" background story in the previous message.  The paragraph's
"use the first line" is no longer correct, afaik.

We take the first paragraph and make it a logical single line if/as
necessary, using RFC2822 header "folding" to put it on the "Subject:"
line.

This project requires you to have a short single-line paragraph as
the first paragraph of the log message, and stating it in SubP
document is good.

> -People on the Git mailing list need to be able to read and
> -comment on the changes you are submitting.  It is important for
> -a developer to be able to "quote" your changes, using standard
> -e-mail tools, so that they may comment on specific portions of
> -your code.  For this reason, all patches should be submitted
> -"inline".  If your log message (including your name on the
> -Signed-off-by line) is not writable in ASCII, make sure that
> -you send off a message in the correct encoding.
> +People on the Git mailing list need to be able to read and comme

Re: [PATCH v4 5/5] rm: delete .gitmodules entry of submodules removed from the work tree

2013-08-06 Thread Jens Lehmann
Currently using "git rm" on a submodule removes the submodule's work tree
from that of the superproject and the gitlink from the index. But the
submodule's section in .gitmodules is left untouched, which is a leftover
of the now removed submodule and might irritate users (as opposed to the
setting in .git/config, this must stay as a reminder that the user showed
interest in this submodule so it will be repopulated later when an older
commit is checked out).

Let "git rm" help the user by not only removing the submodule from the
work tree but by also removing the "submodule." section
from the .gitmodules file and stage both. This doesn't happen when the
"--cached" option is used, as it would modify the work tree. This also
silently does nothing when no .gitmodules file is found and only issues a
warning when it doesn't have a section for this submodule. This is because
the user might just use plain gitlinks without the .gitmodules file or has
already removed the section by hand before issuing the "git rm" command
(in which case the warning reminds him that rm would have done that for
him). Only when .gitmodules is found and contains merge conflicts the rm
command will fail and tell the user to resolve the conflict before trying
again.

Also extend the man page to inform the user about this new feature. While
at it promote the submodule sub-section to a chapter as it made not much
sense under "REMOVING FILES THAT HAVE DISAPPEARED FROM THE FILESYSTEM".

In t7610 three uses of "git rm submod" had to be replaced with "git rm
--cached submod" because that test expects .gitmodules and the work tree
to stay untouched. Also in t7400 the tests for the remaining settings in
the .gitmodules file had to be changed to assert that these settings are
missing.

Signed-off-by: Jens Lehmann 
---

This version fixes the missing strbuf_release() noticed by Frederik.


 Documentation/git-rm.txt   |  8 ++--
 builtin/rm.c   | 19 +++--
 submodule.c| 33 
 submodule.h|  1 +
 t/t3600-rm.sh  | 98 +++---
 t/t7400-submodule-basic.sh | 14 ++-
 t/t7610-mergetool.sh   |  6 +--
 7 files changed, 154 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 1d876c2..9d731b4 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -134,14 +134,16 @@ use the following command:
 git diff --name-only --diff-filter=D -z | xargs -0 git rm --cached
 

-Submodules
-~~
+SUBMODULES
+--
 Only submodules using a gitfile (which means they were cloned
 with a Git version 1.7.8 or newer) will be removed from the work
 tree, as their repository lives inside the .git directory of the
 superproject. If a submodule (or one of those nested inside it)
 still uses a .git directory, `git rm` will fail - no matter if forced
-or not - to protect the submodule's history.
+or not - to protect the submodule's history. If it exists the
+submodule. section in the linkgit:gitmodules[5] file will also
+be removed and that file will be staged (unless --cached or -n are used).

 A submodule is considered up-to-date when the HEAD is the same as
 recorded in the index, no tracked files are modified and no untracked
diff --git a/builtin/rm.c b/builtin/rm.c
index df85f98..a9d45dd 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -283,6 +283,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
struct pathspec pathspec;
char *seen;

+   gitmodules_config();
git_config(git_default_config, NULL);

argc = parse_options(argc, argv, prefix, builtin_rm_options,
@@ -324,7 +325,10 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
continue;
ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
list.entry[list.nr].name = ce->name;
-   list.entry[list.nr++].is_submodule = S_ISGITLINK(ce->ce_mode);
+   list.entry[list.nr].is_submodule = S_ISGITLINK(ce->ce_mode);
+   if (list.entry[list.nr++].is_submodule &&
+   !is_staging_gitmodules_ok())
+   die (_("Please, stage your changes to .gitmodules or 
stash them to proceed"));
}

if (pathspec.nr) {
@@ -396,13 +400,15 @@ int cmd_rm(int argc, const char **argv, const char 
*prefix)
 * in the middle)
 */
if (!index_only) {
-   int removed = 0;
+   int removed = 0, gitmodules_modified = 0;
for (i = 0; i < list.nr; i++) {
const char *path = list.entry[i].name;
if (list.entry[i].is_submodule) {
if (is_empty_dir(path)) {
if (!rmdir(path)) {
removed = 1;
+   if 
(!remove_path_fr

Re: [PATCH v4 4/5] Teach mv to update the path entry in .gitmodules for moved submodules

2013-08-06 Thread Jens Lehmann
Currently using "git mv" on a submodule moves the submodule's work tree in
that of the superproject. But the submodule's path setting in .gitmodules
is left untouched, which is now inconsistent with the work tree and makes
git commands that rely on the proper path -> name mapping (like status and
diff) behave strangely.

Let "git mv" help here by not only moving the submodule's work tree but
also updating the "submodule..path" setting from the
.gitmodules file and stage both. This doesn't happen when no .gitmodules
file is found and only issues a warning when it doesn't have a section for
this submodule. This is because the user might just use plain gitlinks
without the .gitmodules file or has already updated the path setting by
hand before issuing the "git mv" command (in which case the warning
reminds him that mv would have done that for him). Only when .gitmodules
is found and contains merge conflicts the mv command will fail and tell
the user to resolve the conflict before trying again.

Also extend the man page to inform the user about this new feature.

Signed-off-by: Jens Lehmann 
---

This version fixes the missing strbuf_release() noticed by Frederik.


 Documentation/git-mv.txt |  2 ++
 builtin/mv.c | 10 ++-
 submodule.c  | 34 ++
 submodule.h  |  1 +
 t/t7001-mv.sh| 75 
 5 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt
index 1f6fce0..b1f7988 100644
--- a/Documentation/git-mv.txt
+++ b/Documentation/git-mv.txt
@@ -49,6 +49,8 @@ SUBMODULES
 Moving a submodule using a gitfile (which means they were cloned
 with a Git version 1.7.8 or newer) will update the gitfile and
 core.worktree setting to make the submodule work in the new location.
+It also will attempt to update the submodule..path setting in
+the linkgit:gitmodules[5] file and stage that file (unless -n is used).

 GIT
 ---
diff --git a/builtin/mv.c b/builtin/mv.c
index 68b7060..7dd6bb4 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -58,7 +58,7 @@ static struct lock_file lock_file;

 int cmd_mv(int argc, const char **argv, const char *prefix)
 {
-   int i, newfd;
+   int i, newfd, gitmodules_modified = 0;
int verbose = 0, show_only = 0, force = 0, ignore_errors = 0;
struct option builtin_mv_options[] = {
OPT__VERBOSE(&verbose, N_("be verbose")),
@@ -72,6 +72,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
struct stat st;
struct string_list src_for_dst = STRING_LIST_INIT_NODUP;

+   gitmodules_config();
git_config(git_default_config, NULL);

argc = parse_options(argc, argv, prefix, builtin_mv_options,
@@ -125,6 +126,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
struct strbuf submodule_dotgit = STRBUF_INIT;
if (!S_ISGITLINK(active_cache[first]->ce_mode))
die (_("Huh? Directory %s is in index 
and no submodule?"), src);
+   if (!is_staging_gitmodules_ok())
+   die (_("Please, stage your changes to 
.gitmodules or stash them to proceed"));
strbuf_addf(&submodule_dotgit, "%s/.git", src);
submodule_gitfile[i] = 
read_gitfile(submodule_dotgit.buf);
if (submodule_gitfile[i])
@@ -229,6 +232,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
die_errno (_("renaming '%s' failed"), src);
if (submodule_gitfile[i])
connect_work_tree_and_git_dir(dst, 
submodule_gitfile[i]);
+   if (!update_path_in_gitmodules(src, dst))
+   gitmodules_modified = 1;
}

if (mode == WORKING_DIRECTORY)
@@ -240,6 +245,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
rename_cache_entry_at(pos, dst);
}

+   if (gitmodules_modified)
+   stage_updated_gitmodules();
+
if (active_cache_changed) {
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(&lock_file))
diff --git a/submodule.c b/submodule.c
index 5874d08..1c2714f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -47,6 +47,40 @@ int is_staging_gitmodules_ok(void)
return !gitmodules_is_modified;
 }

+/*
+ * Try to update the "path" entry in the "submodule." section of the
+ * .gitmodules file. Return 0 only if a .gitmodules file was found, a section
+ * with the correct path= setting was found and we could update it.
+ */
+int update_path_in_gitmodules(const char *oldpath, const char *newpath)
+{
+   struct strbuf entry = STRBUF_INIT;
+   struct string_list_item

Re: [PATCH revised] git_mkstemps: add test suite test

2013-08-06 Thread Dale R. Worley
> git commit --allow-empty -m message <&-

Though as of [fb56570] "Sync with maint to grab trivial doc fixes",
that test doesn't fail for me if I revert to

fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
if (fd > 0)
return fd;

I haven't been watching the code changes carefully; has there been a
fix that is expected to cause that?

Dale
--
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 revised] git_mkstemps: add test suite test

2013-08-06 Thread Dale R. Worley
> From: Junio C Hamano 
> 
> Thanks. I thought I've already queued 
> 
> Message-ID: <7vfvuokpr0@alter.siamese.dyndns.org>
> aka 
> http://article.gmane.org/gmane.comp.version-control.git/231680
> 
> which tests
> 
> git commit --allow-empty -m message <&-

My mistake...  I've been so intent on revising my repository and
rewriting the patch that I overlooked that you'd done the revision
already.

Dale
--
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] branch, commit, name-rev: ease up boolean conditions

2013-08-06 Thread Eric Sunshine
On Tue, Aug 6, 2013 at 9:07 AM, Stefan Beller
 wrote:
> Now that the variables are readin by OPT_BOOL, which makes sure

Do you mean s/readin/read in/ ?

Or should it be s/readin/set/ ?

> to have the values being 0 or 1 after reading, we do not need
> the double negation to map any other value to 1 for integer
> variables.
>
> Signed-off-by: Stefan Beller 
--
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] SubmittingPatches: clarify some details of the patch format

2013-08-06 Thread Dale R. Worley
---
This is a first draft of a patch that clarifies a number of points
about how patches should be formatted that have tripped me up.  I have
re-filled a few of the paragraphs, which makes it hard to see from the
diff what I've changed.  This listing shows the changed words between
{ ... }:

 { This first line should be a separate paragraph, that is, it should be
followed by an empty line, which is then followed by the body of the
commit message } .

 { At the end of the commit message should be a Signed-off-by line giving
your name.  This can be added to the commit message automatically by
giving "git commit" the "--signoff" option.  This line has legal
implications, see "Sign your work" below } .

People on the Git mailing list need to be able to read and comment on
the changes you are submitting.  It is important for a developer to be
able to "quote" your changes, using standard e-mail tools, so that
they may comment on specific portions of your code.  For this reason,
all patches should be submitted "inline" { rather than as message
attachments } .  If your log message (including your name on the
Signed-off-by line) is not writable in ASCII, make sure that you send
off { the } message in the correct encoding.

"git format-patch" command follows the best current practice to
format the { patch for transmission as an e-mail message.  The files
that it outputs are sample e-mail messages in "mh" format.  The
initial lines are sample From, To, Date, and Subject headers, which
you will likely have to remove or revise, depending on how your MUA
operates } .

At the beginning of the patch should come your commit message ( { the
first line in the Subject header, the remainder in the body of the
message } ), ending with the Signed-off-by: lines, and a line that
consists of three dashes, followed by the diffstat information and the
patch itself.  If you are forwarding a patch from somebody else,
optionally, at the beginning of the e-mail message just before the
commit message starts, you can put a "From: " line to name that
person.

You often want to add additional explanation about the patch,
other than the commit message itself.  Place such "cover letter"
material between the three-dash { line } and the diffstat. Git-notes
can also be inserted using the `--notes` option.

> From: Junio C Hamano 
> 
> I am not sure if SubmittingPatches is a good place, though.
> The document is a guidance for people who contribute to _this_
> project.
> 
> But the specialness of the first paragraph applies to any project
> that uses Git, so people other than those who contribute to this
> project should be aware of it.

All of that is true.  But what can we do to ensure that someone who
submits a patch does so with the right format?  The special treatment
of the first line is not a requirement.  See, e.g., the git-commit
manual page:

   Though not required, it’s a good idea to begin the commit message with
   a single short (less than 50 character) line summarizing the change,
   followed by a blank line and then a more thorough description. Tools
   that turn commits into email, for example, use the first line on the
   Subject: line and the rest of the commit in the body.

> Originally we literally used "first line", but that made many things
> like shortlog output and patch Subject: useless when people write a
> block of text starting from the first line without a title.  Also
> after resurrecting such a text from e-mail, "am" couldn't tell if
> the "first line" on the "Subject:" is meant to be the first line of
> the same first paragraph (which is not what we encourage), or it is
> properly a single line title, and need a blank line before the first
> line of the body.  So quite a while ago, we changed the rule to take
> "the first paragraph" and use that in these places where we want to
> give a title of a patch.

And as you note, if organizing the first line this way was a
requirement, git-am would be able to unambiguously reconstruct the
commit message from an e-mail.  The only way to minimize errors in
this probject is to clearly specify what is required within this
project.

Dale


 Documentation/SubmittingPatches |   47 +-
 1 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index d0a4733..e974dd7 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -85,6 +85,10 @@ identifier for the general area of the code being modified, 
e.g.
 If in doubt which identifier to use, run "git log --no-merges" on the
 files you are modifying to see the current conventions.
 
+This first line should be a separate paragraph, that is, it should be
+followed by an empty line, which is then followed by the body of the
+commit message.
+
 The body 

[PATCH] l10n: de.po: translate 5 messages

2013-08-06 Thread Ralf Thielow
Translate 5 new messages came from git.pot update in b8ecf23
(l10n: git.pot: v1.8.4 round 2 (5 new, 3 removed)).

Signed-off-by: Ralf Thielow 
---
 po/de.po | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/po/de.po b/po/de.po
index 7599f9c..11dde11 100644
--- a/po/de.po
+++ b/po/de.po
@@ -1210,11 +1210,10 @@ msgstr "Kann nicht als allerersten Commit einen Revert 
ausführen."
 #: sequencer.c:1130
 msgid "Can't cherry-pick into empty head"
 msgstr "Kann nicht als allerersten Commit einen Cherry-Pick ausführen."
 
 #: sha1_name.c:440
-#, fuzzy
 msgid ""
 "Git normally never creates a ref that ends with 40 hex characters\n"
 "because it will be ignored when you just specify 40-hex. These refs\n"
 "may be created by mistake. For example,\n"
 "\n"
@@ -1232,11 +1231,11 @@ msgstr ""
 "  git checkout -b $br $(git rev-parse ...)\n"
 "\n"
 "wobei \"$br\" leer ist und eine 40-Hex-Referenz erzeugt\n"
 "wurde. Bitte prüfen Sie diese Referenzen und löschen\n"
 "Sie sie gegebenenfalls. Unterdrücken Sie diese Meldung\n"
-"indem Sie \"git config advice.object_name_warning false\"\n"
+"indem Sie \"git config advice.objectNameWarning false\"\n"
 "ausführen."
 
 #: sha1_name.c:1097
 msgid "HEAD does not point to a branch"
 msgstr "HEAD zeigt auf keinen Branch"
@@ -4000,11 +3999,10 @@ msgstr ""
 "Sie fragten den jüngsten Commit nachzubessern, aber das würde diesen leer\n"
 "machen. Sie können Ihr Kommando mit --allow-empty wiederholen, oder diesen\n"
 "Commit mit \"git reset HEAD^\" vollständig entfernen.\n"
 
 #: builtin/commit.c:62
-#, fuzzy
 msgid ""
 "The previous cherry-pick is now empty, possibly due to conflict resolution.\n"
 "If you wish to commit it anyway, use:\n"
 "\n"
 "git commit --allow-empty\n"
@@ -4014,25 +4012,30 @@ msgstr ""
 "Konfliktauflösung.\n"
 "Wenn Sie dies trotzdem committen wollen, benutzen Sie:\n"
 "\n"
 "git commit --allow-empty\n"
 "\n"
-"Andernfalls benutzen Sie bitte 'git reset'\n"
 
 #: builtin/commit.c:69
 msgid "Otherwise, please use 'git reset'\n"
-msgstr ""
+msgstr "Andernfalls benutzen Sie bitte 'git reset'\n"
 
 #: builtin/commit.c:72
 msgid ""
 "If you wish to skip this commit, use:\n"
 "\n"
 "git reset\n"
 "\n"
 "Then \"git cherry-pick --continue\" will resume cherry-picking\n"
 "the remaining commits.\n"
 msgstr ""
+"Wenn Sie diesen Commit auslassen möchten, benutzen Sie:\n"
+"\n"
+"git reset\n"
+"\n"
+"Benutzen Sie anschließend \"git cherry-pick --continue\", um die\n"
+"Cherry-Pick-Operation mit den verbleibenden Commits fortzusetzen.\n"
 
 #: builtin/commit.c:279
 msgid "failed to unpack HEAD tree object"
 msgstr "Fehler beim Entpacken des \"Tree\"-Objektes von HEAD."
 
@@ -10040,11 +10043,10 @@ msgstr "\"autostash\" angewendet."
 #, sh-format
 msgid "Cannot store $stash_sha1"
 msgstr "Kann $stash_sha1 nicht speichern."
 
 #: git-rebase.sh:160
-#, fuzzy
 msgid ""
 "Applying autostash resulted in conflicts.\n"
 "Your changes are safe in the stash.\n"
 "You can run \"git stash pop\" or \"git stash drop\" at any time.\n"
 msgstr ""
-- 
1.8.2.1873.gfc589a4

--
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] die_with_status: use "printf '%s\n'", not "echo"

2013-08-06 Thread Matthieu Moy
At least GNU echo interprets backslashes in its arguments.

This triggered at least one bug: the error message of "rebase -i" was
turning \t in commit messages into actual tabulations. There may be
others.

Using "printf '%s\n'" instead avoids this bad behavior, and is the form
used by the "say" function.

Noticed-by: David Kastrup 
Signed-off-by: Matthieu Moy 
---
 git-sh-setup.sh   |  2 +-
 t/t3404-rebase-interactive.sh | 13 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a964ad..e15be51 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -53,7 +53,7 @@ die () {
 die_with_status () {
status=$1
shift
-   echo >&2 "$*"
+   printf >&2 '%s\n' "$*"
exit "$status"
 }
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 49ccb38..074deb1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -976,4 +976,17 @@ test_expect_success 'rebase -i with --strategy and -X' '
test $(cat file1) = Z
 '
 
+test_expect_success 'rebase -i error on commits with \ in message' '
+   current_head=$(git rev-parse HEAD)
+   test_when_finished "git rebase --abort; git reset --hard $current_head; 
rm -f error" &&
+   test_commit TO-REMOVE will-conflict old-content &&
+   test_commit "\temp" will-conflict new-content dummy &&
+   (
+   EDITOR=true &&
+   export EDITOR &&
+   test_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error
+   ) &&
+   grep -v "   " error
+'
+
 test_done
-- 
1.8.3.3.797.gb72c616

--
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 revised] git_mkstemps: add test suite test

2013-08-06 Thread Junio C Hamano
wor...@alum.mit.edu (Dale R. Worley) writes:

> Commit a2cb86 ("git_mkstemps: correctly test return value of open()",
> 12 Jul 2013) fixes a bug regarding testing the return of an open()
> call for success/failure.  Add a testsuite test for that fix.  The
> test exercises a situation where that open() is known to return 0.
>
> Signed-off-by: Dale Worley 
> ---
> This version of the patch cleans up a number of errors in my previous
> version (which were ultimately due to my faulty updating of my master
> branch).  The commit that added the open() test is now correctly
> described.  Since the test was not present in the test suite at all,
> the patch is described as adding the test rather than improving it.
>
> a2cb86 is on branch tr/fd-gotcha-fixes, but that has been merged into
> master now.

Thanks. I thought I've already queued 

Message-ID: <7vfvuokpr0@alter.siamese.dyndns.org>
aka 
http://article.gmane.org/gmane.comp.version-control.git/231680

which tests

git commit --allow-empty -m message <&-

> +test_expect_success 'git_mkstemps_mode does not fail if fd 0 is not open' '
> + git init &&

This does not do anything useful; you are in the test playpen aka
"trash" which is an already initialized git repository.

> + echo Test. >test-file &&
> + git add test-file &&

You do not have to have extra contents...

> + git commit -m Message. <&-

...you can do with just "--allow-empty" instead.


--
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 revised] git_mkstemps: add test suite test

2013-08-06 Thread Dale R. Worley
Commit a2cb86 ("git_mkstemps: correctly test return value of open()",
12 Jul 2013) fixes a bug regarding testing the return of an open()
call for success/failure.  Add a testsuite test for that fix.  The
test exercises a situation where that open() is known to return 0.

Signed-off-by: Dale Worley 
---
This version of the patch cleans up a number of errors in my previous
version (which were ultimately due to my faulty updating of my master
branch).  The commit that added the open() test is now correctly
described.  Since the test was not present in the test suite at all,
the patch is described as adding the test rather than improving it.

a2cb86 is on branch tr/fd-gotcha-fixes, but that has been merged into
master now.

(Thanks for your patience with this.)

Dale

 t/t0070-fundamental.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 986b2a8..d427f3a 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -25,6 +25,13 @@ test_expect_success POSIXPERM,SANITY 'mktemp to unwritable 
directory prints file
grep "cannotwrite/test" err
 '
 
+test_expect_success 'git_mkstemps_mode does not fail if fd 0 is not open' '
+   git init &&
+   echo Test. >test-file &&
+   git add test-file &&
+   git commit -m Message. <&-
+'
+
 test_expect_success 'check for a bug in the regex routines' '
# if this test fails, re-build git with NO_REGEX=1
test-regex
-- 
1.8.4.rc1.24.gd407a5c

--
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: git rebase -i error message interprets \t in commit message

2013-08-06 Thread Junio C Hamano
Matthieu Moy  writes:

> David Kastrup  writes:
>
>> Could not apply 16de9d2... Make tempo range  empo 20~30 be input as  empo 
>> 20-30 instead
>
> Indeed. The source of the problem is that our "die" shell function
> interprets \t (because it uses "echo").
>
> A simple fix would be this:
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 7a964ad..97258d5 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -53,7 +53,7 @@ die () {
>  die_with_status () {
> status=$1
> shift
> -   echo >&2 "$*"
> +   printf >&2 "%s\n" "$*"
> exit "$status"
>  }
>  
> It does not sound crazy as the shell function "say" right below uses the
> same printf "%s\n" "$*", but I'm wondering whether this could have other
> bad implications (e.g. if there are escape sequences in the commit
> message, aren't we going to screw up the terminal?).

I gave a quick look at "git grep -e 'die ' -- \*.sh" output, and I
do not think this change will break things (i.e. no caller expects
the non-portable behaviour of the shell such as "\c" at the end to
omit the trailing newline).

Thanks, care to roll it into a patch with a test or two?


--
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: git rebase -i error message interprets \t in commit message

2013-08-06 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> So, it's the shell script.  Now, read about shell escaping [1] and
> submit a patch.

This is not about shell escaping at all.  I think the message is fed
to echo as-is, or to printf as its first parameter.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git exproll: steps to tackle gc aggression

2013-08-06 Thread Junio C Hamano
Duy Nguyen  writes:

> On Tue, Aug 6, 2013 at 9:38 AM, Ramkumar Ramachandra  
> wrote:
>> +   Garbage collect using a pseudo logarithmic packfile 
>> maintenance
>> +   approach.  This approach attempts to minimize packfile churn
>> +   by keeping several generations of varying sized packfiles 
>> around
>> +   and only consolidating packfiles (or loose objects) which are
>> +   either new packfiles, or packfiles close to the same size as
>> +   another packfile.
>
> I wonder if a simpler approach may be nearly efficient as this one:
> keep the largest pack out, repack the rest at fetch/push time so there
> are at most 2 packs at a time. Or we we could do the repack at 'gc
> --auto' time, but with lower pack threshold (about 10 or so). When the
> second pack is as big as, say half the size of the first, merge them
> into one at "gc --auto" time. This can be easily implemented in
> git-repack.sh.

Another random thought.

Imagine we have a cheap way to enumerate the young objects without
the usual history traversal.  For example, list of all loose objects
and what appears in the .idx files that are young.

We can reconstruct "names" for trees and blobs from such a list of
object names; if a commit in the list refers to a tree, that tree is
the top level, and a blob or a tree that appears in such a top-level
tree can be given a "name" for its place in the tree (recursively).
I suspect we would end up giving names to large majority of trees
and blobs in such a list by doing so.

If these suspicions turn out to be true, then we could:

 - run that enumeration algorithm to come up with a set of object
   names;

 - emit the tag objects in that set in the tagger timestamp order;

 - emit the commit objects in that set in the commit timestamp
   order, while noting the tree objects contained in the set, giving
   them name "";

 - "traverse" the trees and blobs in that set, giving the found ones
   names (do so without stepping outside the set);

 - emit the trees and blobs with their names.  Some objects may not
   have given any name, but that is OK as long as they are in the
   minority.

And feeding it to pack-objects to produce a single pack, and then
prune away the source of these young objects in the end.

The above could turn out to be much cheaper than the traditional
history traversal.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] submodule: fix confusing variable name

2013-08-06 Thread Jens Lehmann
Am 04.08.2013 23:29, schrieb Fredrik Gustafsson:
> On Sun, Aug 04, 2013 at 07:34:48PM +0200, Jens Lehmann wrote:
>> But we'll have to use sm_path here (like everywhere else in the
>> submodule script), because we'll run into problems under Windows
>> otherwise (see 64394e3ae9 for details). Apart from that the patch
>> is fine.
> 
> We're still using path= in the foreach-script. Or rather, we're setting
> it. From what I can see and from the commit message 64394e3ae9 it could
> possible be a problem.

The commit message of 64394e3ae9 explicitly talks about that one:

  We note that the foreach subcommand provides $path to user scripts
  (ie it is part of the API), so we can't simply rename it to $sm_path.

> Not sure how to solve it though... Just a simple correction would break
> all script depending on that.

That's exactly why that one is still there. We could deprecate it in
favor of $sm_path and remove it some time later, but I'm not convinced
it is an urgent issue. But me thinks the documentation of foreach should
be updated, as it still talks about $path.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Makefile: Fix APPLE_COMMON_CRYPTO with BLK_SHA1

2013-08-06 Thread Junio C Hamano
David Aguilar  writes:

> I tested the tip of da/darwin (pu) w/ and w/out BLK_SHA1.
>
> Tested-by: David Aguilar 

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] Add missing test file for UTF-16.

2013-08-06 Thread Junio C Hamano
Duy Nguyen  writes:

> The intention was "UTF-16 is not supported yet but we want to". But I
> don't think we (at least I) will put any effort on that front to allow
> NUL in commit message, so the patch, as in "we do not support UTF-16",
> is fine.

Agreed.  Here is what I queued.

-- >8 --
Subject: [PATCH] t3900: test rejecting log message with NULs correctly

It is not like that our longer term desire is to someday start
accepting log messages with NULs in them, so it is wrong to mark a test
that demonstrates "git commit" that correctly fails given such an
input as "expect-failure".  "git commit" should fail today, and it
should fail the same way in the future given a message with NUL in it.

Signed-off-by: Junio C Hamano 
---
 t/t3900-i18n-commit.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
index d48a7c0..a9e5662 100755
--- a/t/t3900-i18n-commit.sh
+++ b/t/t3900-i18n-commit.sh
@@ -34,9 +34,9 @@ test_expect_success 'no encoding header for base case' '
test z = "z$E"
 '
 
-test_expect_failure 'UTF-16 refused because of NULs' '
+test_expect_success 'UTF-16 refused because of NULs' '
echo UTF-16 >F &&
-   git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
+   test_must_fail git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
 '
 
 
--
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: [PATCHv3 0/9] Removing deprecated parsing macros

2013-08-06 Thread Junio C Hamano
Stefan Beller  writes:

> On 08/06/2013 08:39 AM, Junio C Hamano wrote:
>> Thanks.  Queued this at the tip of 'pu'.  There seem to be some
>> fallouts found in the test suite, though.
>> 
>
> Thanks. I am sorry for forgetting 'make test' before sending patches.
> And the test suite is correct.
>
> e35ea450 (branch, commit, name-rev: ease up boolean conditions)
> is faulty. In builtin/branch.c the variables 'delete' and 'rename' 
> are not used as a true boolean but I assumed so. 
> These are parsed in via OPT_BIT and depending if you pass -d or -D 
> for deletion you have value 1 or 2 in delete.

Likewise for 'rename' that becomes 2 when -M is given.

> Hence this change is incorrect:
> - if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + 
> !!unset_upstream > 1)
> + if (delete + rename + force_create + list + unset_upstream +
> + !!new_upstream > 1)

As a follow-up to this series, we may want to think about the
following as well:

 - OPT__VERBOSE() is defined in terms of OPT_BOOLEAN().  I think we
   use it to represent increasing levels of verbosity, so we cannot
   turn this into OPT_BOOL().  Its implementation has to become
   OPT_COUNTUP().

 - OPT__QUIET() is defined in terms of OPT_BOOLEAN().  I offhand do
   not know if we have increasing levels of quietness.  The users
   need auditing before we can decide to turn this into either
   OPT_COUNTUP() or OPT_BOOL().

 - OPT__DRY_RUN() should probably become OPT_BOOL().

 - OPT_FORCE(); do we have levels of forcefulness?  If so
   OPT_COUNTUP(), otherwise OPT_BOOL().
--
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: git rebase -i error message interprets \t in commit message

2013-08-06 Thread Matthieu Moy
David Kastrup  writes:

>> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
>> index 7a964ad..97258d5 100644
>> --- a/git-sh-setup.sh
>> +++ b/git-sh-setup.sh
>> @@ -53,7 +53,7 @@ die () {
>>  die_with_status () {
>> status=$1
>> shift
>> -   echo >&2 "$*"
>> +   printf >&2 "%s\n" "$*"
>> exit "$status"
>>  }
>>  
>> It does not sound crazy as the shell function "say" right below uses the
>> same printf "%s\n" "$*",
>
> Sounds reasonable, though I don't know off-hand (not having the source
> here) whether using "say" inside of die_with_status 

The definition of say is:

say () {
if test -z "$GIT_QUIET"
then
printf '%s\n' "$*"
fi
}

I don't think we want to disable die's output even when the caller
requested to be quiet. Currently, my patch is:

>From 7962ac8d8f2cbc556f669fd97487f9d70edc4ea1 Mon Sep 17 00:00:00 2001
From: Matthieu Moy 
Date: Tue, 6 Aug 2013 19:13:03 +0200
Subject: [PATCH] die_with_status: use "printf '%s\n'", not "echo"

At least GNU echo interprets backslashes in its arguments.

This triggered at least one bug: the error message of "rebase -i" was
turning \t in commit messages into actual tabulations. There may be
others.

Using "printf '%s\n'" instead avoids this bad behavior, and is the form
used by the "say" function.

Noticed-by: David Kastrup 
Signed-off-by: Matthieu Moy 
---
 git-sh-setup.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a964ad..e15be51 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -53,7 +53,7 @@ die () {
 die_with_status () {
status=$1
shift
-   echo >&2 "$*"
+   printf >&2 '%s\n' "$*"
exit "$status"
 }
 
-- 
1.8.3.3.797.gb72c616

I'll resend properly for inclusion if no one objects.

-- 
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: git rebase -i error message interprets \t in commit message

2013-08-06 Thread David Kastrup
Matthieu Moy  writes:

> David Kastrup  writes:
>
>> Could not apply 16de9d2... Make tempo range empo 20~30 be input as
>> empo 20-30 instead
>
> Indeed. The source of the problem is that our "die" shell function
> interprets \t (because it uses "echo").
>
> A simple fix would be this:
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 7a964ad..97258d5 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -53,7 +53,7 @@ die () {
>  die_with_status () {
> status=$1
> shift
> -   echo >&2 "$*"
> +   printf >&2 "%s\n" "$*"
> exit "$status"
>  }
>  
> It does not sound crazy as the shell function "say" right below uses the
> same printf "%s\n" "$*",

Sounds reasonable, though I don't know off-hand (not having the source
here) whether using "say" inside of die_with_status (and thus not having
two different places to maintain) might not be feasible instead.  It's
not like die_with_status is going to be called often enough to become a
performance hog.

> but I'm wondering whether this could have other bad implications
> (e.g. if there are escape sequences in the commit message, aren't we
> going to screw up the terminal?).

One person's escape sequences are another's multibyte characters.  Less
cheesy, it would seem that the above change is a strict improvement and
requires little thought to implement.

Not interpreting escape sequences is orthogonal from output
sanitization.

-- 
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 cooking in git.git (Aug 2013, #01; Thu, 1)

2013-08-06 Thread Junio C Hamano
Thomas Rast  writes:

>> So while I do understand why sometimes you wish to see full diff 
>> "git log --full-diff -- " to match output from "git show"
>> without pathspec, I am not sure doing it unconditionally and by
>> default like your patch does is the best way to go.
>
> I'm utterly unconvinced.

I was waiting you to say "I'm positively convinced that showing
local diff with the true parent is right!"; the reason why I planted
that seed in "What's cooking" to discuss this was because I needed
to play devil's advocate to make sure that all of us know that this
change is a good one, and that we know we agree that this change is
a good one, as the change is large insemantics, and the bookkeeping
burden is not negligible.  I suspect that we may want to restrict
the copying of the true parents only to limited cases (e.g. when
"--full-diff" is given), but I think that can be done on top, as it
is only the performance thing, not correctness.


--
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 4/4] fetch: opportunistically update tracking refs

2013-08-06 Thread Junio C Hamano
Jeff King  writes:

> @@ -170,6 +172,20 @@ static struct ref *get_ref_map(struct transport 
> *transport,
>   rm->fetch_head_status = FETCH_HEAD_MERGE;
>   if (tags == TAGS_SET)
>   get_fetch_map(remote_refs, tag_refspec, &tail, 0);
> +
> + /*
> +  * For any refs that we happen to be fetching via command-line
> +  * arguments, take the opportunity to update their configured
> +  * counterparts. However, we do not want to mention these
> +  * entries in FETCH_HEAD at all, as they would simply be
> +  * duplicates of existing entries.
> +  */
> + old_tail = tail;
> + for (i = 0; i < transport->remote->fetch_refspec_nr; i++)
> + get_fetch_map(ref_map, &transport->remote->fetch[i],
> +   &tail, 0);
> + for (rm = *old_tail; rm; rm = rm->next)
> + rm->fetch_head_status = FETCH_HEAD_IGNORE;

Was there a reason why this change was done by appending new ref at
the tail of the ref_map list?  I would have expected that a naïve
and obvious implementation would be to iterate existing refs over
ref_map to find refs with an empty RHS whose LHS is configured to
usually store the fetched result to somewhere and to update their
RHS in place.

Being curious.
--
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: git rebase -i error message interprets \t in commit message

2013-08-06 Thread Matthieu Moy
David Kastrup  writes:

> Could not apply 16de9d2... Make tempo range   empo 20~30 be input as  empo 
> 20-30 instead

Indeed. The source of the problem is that our "die" shell function
interprets \t (because it uses "echo").

A simple fix would be this:

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a964ad..97258d5 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -53,7 +53,7 @@ die () {
 die_with_status () {
status=$1
shift
-   echo >&2 "$*"
+   printf >&2 "%s\n" "$*"
exit "$status"
 }
 
It does not sound crazy as the shell function "say" right below uses the
same printf "%s\n" "$*", but I'm wondering whether this could have other
bad implications (e.g. if there are escape sequences in the commit
message, aren't we going to screw up the terminal?).

-- 
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: git rebase -i error message interprets \t in commit message

2013-08-06 Thread Ramkumar Ramachandra
David Kastrup wrote:
> As you can see, the first message starting with "error: could not apply"
> outputs a reasonable rendition of the commit summary line.  However, the
> final "Could not apply" message (starting with a capital C) converts
> instances of \t to a tab.

To get you started:

  $ git grep "could not apply"
  sequencer.c=475=static int do_pick_commit(struct commit *commit,
  sequencer.c:628:  : _("could not apply %s... %s"),

  $ git grep "Could not apply"
  git-rebase--interactive.sh=431=die_failed_squash() {
  git-rebase--interactive.sh:436: warn "Could not apply $1... $2"
  git-rebase--interactive.sh=460=do_pick () {
  git-rebase--interactive.sh:476: die_with_patch $1 "Could not apply $1... $2"
  git-rebase--interactive.sh:479: die_with_patch $1 "Could not apply $1... $2"

So, it's the shell script.  Now, read about shell escaping [1] and
submit a patch.

Thanks.

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


git rebase -i error message interprets \t in commit message

2013-08-06 Thread David Kastrup

Hi,

I just got the following error message:

dak@lola:/usr/local/tmp/lilypond$ git rebase -i
Waiting for Emacs...
error: could not apply 16de9d2... Make tempo range \tempo 20~30 be input as 
\tempo 20-30 instead

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".
Could not apply 16de9d2... Make tempo range empo 20~30 be input as  empo 
20-30 instead
dak@lola:/usr/local/tmp/lilypond$ git --version
git version 1.8.1.2

As you can see, the first message starting with "error: could not apply"
outputs a reasonable rendition of the commit summary line.  However, the
final "Could not apply" message (starting with a capital C) converts
instances of \t to a tab.

This is on Ubuntu 13.04 with

lrwxrwxrwx 1 root root 4 Aug 15  2012 /bin/sh -> dash

-- 
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 11/16] line-range-format.txt: document -L/RE/ relative search

2013-08-06 Thread Eric Sunshine
Option -L/RE/ of blame/log now searches relative to the previous -L
range, if any. Document this.

Signed-off-by: Eric Sunshine 
---
 Documentation/line-range-format.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/line-range-format.txt 
b/Documentation/line-range-format.txt
index a1b2f4e..42d74f7 100644
--- a/Documentation/line-range-format.txt
+++ b/Documentation/line-range-format.txt
@@ -9,7 +9,9 @@ absolute line number (lines count from 1).
 - /regex/
 +
 This form will use the first line matching the given
-POSIX regex.  If  is a regex, it will search
+POSIX regex. If  is a regex, it will search from the end of
+the previous `-L` range, if any, otherwise from the start of file.
+If  is a regex, it will search
 starting at the line given by .
 +
 
-- 
1.8.4.rc1.409.gbd48715

--
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 05/16] blame: accept multiple -L ranges

2013-08-06 Thread Eric Sunshine
git-blame accepts only a single -L option or none. Clients requiring
blame information for multiple disjoint ranges are therefore forced
either to invoke git-blame multiple times, once for each range, or only
once with no -L option to cover the entire file, both of which can be
costly.  Teach git-blame to accept multiple -L ranges.  Overlapping and
out-of-order ranges are accepted.

In this patch, the X in -LX,Y is absolute (for instance, /RE/ patterns
search from line 1), and Y is relative to X. Follow-up patches provide
more flexibility over how X is anchored.

Signed-off-by: Eric Sunshine 
---

This patch is a bit less scary when whitespace changes are ignored.

 builtin/blame.c | 79 ++---
 1 file changed, 47 insertions(+), 32 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 9db01b0..2f4d9e2 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -22,6 +22,7 @@
 #include "utf8.h"
 #include "userdiff.h"
 #include "line-range.h"
+#include "line-log.h"
 
 static char blame_usage[] = N_("git blame [options] [rev-opts] [rev] [--] 
file");
 
@@ -2233,29 +2234,18 @@ static int blame_move_callback(const struct option 
*option, const char *arg, int
return 0;
 }
 
-static int blame_bottomtop_callback(const struct option *option, const char 
*arg, int unset)
-{
-   const char **bottomtop = option->value;
-   if (!arg)
-   return -1;
-   if (*bottomtop)
-   die("More than one '-L n,m' option given");
-   *bottomtop = arg;
-   return 0;
-}
-
 int cmd_blame(int argc, const char **argv, const char *prefix)
 {
struct rev_info revs;
const char *path;
struct scoreboard sb;
struct origin *o;
-   struct blame_entry *ent;
-   long dashdash_pos, bottom, top, lno;
+   struct blame_entry *ent = NULL;
+   long dashdash_pos, lno;
const char *final_commit_name = NULL;
enum object_type type;
 
-   static const char *bottomtop = NULL;
+   static struct string_list range_list;
static int output_option = 0, opt = 0;
static int show_stats = 0;
static const char *revs_file = NULL;
@@ -2281,13 +2271,15 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
OPT_STRING(0, "contents", &contents_from, N_("file"), N_("Use 
's contents as the final image")),
{ OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line 
copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line 
movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
-   OPT_CALLBACK('L', NULL, &bottomtop, N_("n,m"), N_("Process only 
line range n,m, counting from 1"), blame_bottomtop_callback),
+   OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process 
only line range n,m, counting from 1")),
OPT__ABBREV(&abbrev),
OPT_END()
};
 
struct parse_opt_ctx_t ctx;
int cmd_is_annotate = !strcmp(argv[0], "annotate");
+   struct range_set ranges;
+   unsigned int range_i;
 
git_config(git_blame_config, NULL);
init_revisions(&revs, NULL);
@@ -2480,23 +2472,46 @@ parse_done:
num_read_blob++;
lno = prepare_lines(&sb);
 
-   bottom = top = 0;
-   if (bottomtop && parse_range_arg(bottomtop, nth_line_cb, &sb, lno,
-&bottom, &top, sb.path))
-   usage(blame_usage);
-   if (lno < top || ((lno || bottom) && lno < bottom))
-   die("file %s has only %lu lines", path, lno);
-   if (bottom < 1)
-   bottom = 1;
-   if (top < 1)
-   top = lno;
-   bottom--;
-
-   ent = xcalloc(1, sizeof(*ent));
-   ent->lno = bottom;
-   ent->num_lines = top - bottom;
-   ent->suspect = o;
-   ent->s_lno = bottom;
+   if (lno && !range_list.nr)
+   string_list_append(&range_list, xstrdup("1"));
+
+   range_set_init(&ranges, range_list.nr);
+   for (range_i = 0; range_i < range_list.nr; ++range_i) {
+   long bottom, top;
+   if (parse_range_arg(range_list.items[range_i].string,
+   nth_line_cb, &sb, lno,
+   &bottom, &top, sb.path))
+   usage(blame_usage);
+   if (lno < top || ((lno || bottom) && lno < bottom))
+   die("file %s has only %lu lines", path, lno);
+   if (bottom < 1)
+   bottom = 1;
+   if (top < 1)
+   top = lno;
+   bottom--;
+   range_set_append_unsafe(&ranges, bottom, top);
+   }
+   sort_and_merge_range_set(&ranges);
+
+   for (range_i = ranges.nr; range_i > 0; --range_i) {
+   const struct range *r = &r

[PATCH v2 07/16] blame: document multiple -L support

2013-08-06 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 Documentation/blame-options.txt |  8 +---
 Documentation/git-blame.txt | 10 +++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 489032c..0cebc4f 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -11,9 +11,11 @@
 
 -L ,::
 -L :::
-   Annotate only the given line range.   and  are optional.
-   ``-L '' or ``-L ,'' spans from  to end of file.
-   ``-L ,'' spans from start of file to .
+   Annotate only the given line range. May be specified multiple times.
+   Overlapping ranges are allowed.
++
+ and  are optional. ``-L '' or ``-L ,'' spans from
+ to end of file. ``-L ,'' spans from start of file to .
 +
 include::line-range-format.txt[]
 
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 6cea7f1..f2c85cc 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
-   [-L n,m | -L :fn] [-S ] [-M] [-C] [-C] [-C] 
[--since=]
+   [-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
[--abbrev=] [ | --contents  | --reverse ] [--] 

 
 DESCRIPTION
@@ -18,7 +18,8 @@ DESCRIPTION
 Annotates each line in the given file with information from the revision which
 last modified the line. Optionally, start annotating from the given revision.
 
-The command can also limit the range of lines annotated.
+When specified one or more times, `-L` restricts annotation to the requested
+lines.
 
 The origin of lines is automatically followed across whole-file
 renames (currently there is no option to turn the rename-following
@@ -130,7 +131,10 @@ SPECIFYING RANGES
 
 Unlike 'git blame' and 'git annotate' in older versions of git, the extent
 of the annotation can be limited to both line ranges and revision
-ranges.  When you are interested in finding the origin for
+ranges. The `-L` option, which limits annotation to a range of lines, may be
+specified multiple times.
+
+When you are interested in finding the origin for
 lines 40-60 for file `foo`, you can use the `-L` option like so
 (they mean the same thing -- both ask for 21 lines starting at
 line 40):
-- 
1.8.4.rc1.409.gbd48715

--
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 10/16] log: teach -L/RE/ to search from end of previous -L range

2013-08-06 Thread Eric Sunshine
This is complicated slightly by having to remember the previous -L range
for each file specified via -L:file.

The existing implementation coalesces ranges for each file as each -L is
parsed which makes it impossible to refer back to the previous -L range
for any particular file. Re-implement to instead store each file's set
of -L ranges verbatim, and then coalesce the ranges in a post-processing
step.

Signed-off-by: Eric Sunshine 
---
 line-log.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/line-log.c b/line-log.c
index 38f827b..d40c79d 100644
--- a/line-log.c
+++ b/line-log.c
@@ -291,7 +291,6 @@ static void line_log_data_insert(struct line_log_data 
**list,
 
if (p) {
range_set_append_unsafe(&p->ranges, begin, end);
-   sort_and_merge_range_set(&p->ranges);
free(path);
return;
}
@@ -299,7 +298,6 @@ static void line_log_data_insert(struct line_log_data 
**list,
p = xcalloc(1, sizeof(struct line_log_data));
p->path = path;
range_set_append(&p->ranges, begin, end);
-   sort_and_merge_range_set(&p->ranges);
if (ip) {
p->next = ip->next;
ip->next = p;
@@ -566,12 +564,14 @@ parse_lines(struct commit *commit, const char *prefix, 
struct string_list *args)
struct nth_line_cb cb_data;
struct string_list_item *item;
struct line_log_data *ranges = NULL;
+   struct line_log_data *p;
 
for_each_string_list_item(item, args) {
const char *name_part, *range_part;
char *full_name;
struct diff_filespec *spec;
long begin = 0, end = 0;
+   long anchor;
 
name_part = skip_range_arg(item->string);
if (!name_part || *name_part != ':' || !name_part[1])
@@ -590,8 +590,14 @@ parse_lines(struct commit *commit, const char *prefix, 
struct string_list *args)
cb_data.lines = lines;
cb_data.line_ends = ends;
 
+   p = search_line_log_data(ranges, full_name, NULL);
+   if (p && p->ranges.nr)
+   anchor = p->ranges.ranges[p->ranges.nr - 1].end + 1;
+   else
+   anchor = 1;
+
if (parse_range_arg(range_part, nth_line, &cb_data,
-   lines, 1, &begin, &end,
+   lines, anchor, &begin, &end,
full_name))
die("malformed -L argument '%s'", range_part);
if (lines < end || ((lines || begin) && lines < begin))
@@ -608,6 +614,9 @@ parse_lines(struct commit *commit, const char *prefix, 
struct string_list *args)
ends = NULL;
}
 
+   for (p = ranges; p; p = p->next)
+   sort_and_merge_range_set(&p->ranges);
+
return ranges;
 }
 
-- 
1.8.4.rc1.409.gbd48715

--
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 14/16] line-range: teach -L^:RE to search from start of file

2013-08-06 Thread Eric Sunshine
The -L:RE option of blame/log searches from the end of the previous -L
range, if any. Add new notation -L^:RE to override this behavior and
search from start of file.

Signed-off-by: Eric Sunshine 
---
 Documentation/line-range-format.txt |  1 +
 line-range.c|  9 +++--
 t/annotate-tests.sh | 17 +
 t/t4211-line-log.sh |  1 +
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/line-range-format.txt 
b/Documentation/line-range-format.txt
index 469d80b..d7f2603 100644
--- a/Documentation/line-range-format.txt
+++ b/Documentation/line-range-format.txt
@@ -26,3 +26,4 @@ If ``:'' is given in place of  and , it 
denotes the range
 from the first funcname line that matches , up to the next
 funcname line. ``:'' searches from the end of the previous `-L` range,
 if any, otherwise from the start of file.
+``^:'' searches from the start of file.
diff --git a/line-range.c b/line-range.c
index 4bae4bf..ede0c6c 100644
--- a/line-range.c
+++ b/line-range.c
@@ -173,6 +173,11 @@ static const char *parse_range_funcname(const char *arg, 
nth_line_fn_t nth_line_
int reg_error;
regex_t regexp;
 
+   if (*arg == '^') {
+   anchor = 1;
+   arg++;
+   }
+
assert(*arg == ':');
term = arg+1;
while (*term && *term != ':') {
@@ -245,7 +250,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t 
nth_line_cb,
if (anchor > lines)
anchor = lines + 1;
 
-   if (*arg == ':') {
+   if (*arg == ':' || (*arg == '^' && *(arg + 1) == ':')) {
arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines, 
anchor, begin, end, path);
if (!arg || *arg)
return -1;
@@ -270,7 +275,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t 
nth_line_cb,
 
 const char *skip_range_arg(const char *arg)
 {
-   if (*arg == ':')
+   if (*arg == ':' || (*arg == '^' && *(arg + 1) == ':'))
return parse_range_funcname(arg, NULL, NULL, 0, 0, NULL, NULL, 
NULL);
 
arg = parse_loc(arg, NULL, NULL, 0, -1, NULL);
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 4f7d6ba..dabe89d 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -398,6 +398,23 @@ test_expect_success 'blame -L :RE (relative: end-of-file)' 
'
test_must_fail $PROG -L, -L:main hello.c
 '
 
+test_expect_success 'blame -L ^:RE (absolute)' '
+   check_count -f hello.c -L3,3 -L^:ma.. F 4 G 1
+'
+
+test_expect_success 'blame -L ^:RE (absolute: no preceding range)' '
+   check_count -f hello.c -L^:ma.. F 4 G 1
+'
+
+test_expect_success 'blame -L ^:RE (absolute: not found)' '
+   test_must_fail $PROG -L4,4 -L^:tambourine hello.c
+'
+
+test_expect_success 'blame -L ^:RE (absolute: end-of-file)' '
+   n=$(printf "%d" $(wc -l http://vger.kernel.org/majordomo-info.html


[PATCH v2 06/16] t8001/t8002: blame: add tests of multiple -L options

2013-08-06 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/annotate-tests.sh | 32 
 1 file changed, 32 insertions(+)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index ce5b8ed..77083d9 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -271,6 +271,38 @@ test_expect_success 'blame -L ,Y (Y > nlines)' '
test_must_fail $PROG -L,12345 file
 '
 
+test_expect_success 'blame -L multiple (disjoint)' '
+   check_count -L2,3 -L6,7 A 1 B1 1 B2 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L multiple (disjoint: unordered)' '
+   check_count -L6,7 -L2,3 A 1 B1 1 B2 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L multiple (adjacent)' '
+   check_count -L2,3 -L4,5 A 1 B 1 B2 1 D 1
+'
+
+test_expect_success 'blame -L multiple (adjacent: unordered)' '
+   check_count -L4,5 -L2,3 A 1 B 1 B2 1 D 1
+'
+
+test_expect_success 'blame -L multiple (overlapping)' '
+   check_count -L2,4 -L3,5 A 1 B 1 B2 1 D 1
+'
+
+test_expect_success 'blame -L multiple (overlapping: unordered)' '
+   check_count -L3,5 -L2,4 A 1 B 1 B2 1 D 1
+'
+
+test_expect_success 'blame -L multiple (superset/subset)' '
+   check_count -L2,8 -L3,5 A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L multiple (superset/subset: unordered)' '
+   check_count -L3,5 -L2,8 A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1
+'
+
 test_expect_success 'setup -L :regex' '
tr Q "\\t" >hello.c <<-\EOF &&
int main(int argc, const char *argv[])
-- 
1.8.4.rc1.409.gbd48715

--
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 03/16] range-set: publish API for re-use by git-blame -L

2013-08-06 Thread Eric Sunshine
git-blame is slated to accept multiple -L ranges.  git-log already
accepts multiple -L's but its implementation of range-set, which
organizes and normalizes -L ranges, is private.  Publish the small
subset of range-set API which is needed for git-blame multiple -L
support.

Signed-off-by: Eric Sunshine 
---

Since range-set is generic -- not at all specific to -L line ranges -- a
future patch series may want to publish the entire API, move the
implementation to its own file, and add documentation and full unit-test
coverage, however, such a topic is orthogonal to the present series
which adds multiple -L support to git-blame.

 line-log.c | 10 +-
 line-log.h | 12 
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/line-log.c b/line-log.c
index 1c3ac8d..bdadf35 100644
--- a/line-log.c
+++ b/line-log.c
@@ -23,7 +23,7 @@ static void range_set_grow(struct range_set *rs, size_t extra)
 /* Either initialization would be fine */
 #define RANGE_SET_INIT {0}
 
-static void range_set_init(struct range_set *rs, size_t prealloc)
+void range_set_init(struct range_set *rs, size_t prealloc)
 {
rs->alloc = rs->nr = 0;
rs->ranges = NULL;
@@ -31,7 +31,7 @@ static void range_set_init(struct range_set *rs, size_t 
prealloc)
range_set_grow(rs, prealloc);
 }
 
-static void range_set_release(struct range_set *rs)
+void range_set_release(struct range_set *rs)
 {
free(rs->ranges);
rs->alloc = rs->nr = 0;
@@ -56,7 +56,7 @@ static void range_set_move(struct range_set *dst, struct 
range_set *src)
 }
 
 /* tack on a _new_ range _at the end_ */
-static void range_set_append_unsafe(struct range_set *rs, long a, long b)
+void range_set_append_unsafe(struct range_set *rs, long a, long b)
 {
assert(a <= b);
range_set_grow(rs, 1);
@@ -65,7 +65,7 @@ static void range_set_append_unsafe(struct range_set *rs, 
long a, long b)
rs->nr++;
 }
 
-static void range_set_append(struct range_set *rs, long a, long b)
+void range_set_append(struct range_set *rs, long a, long b)
 {
assert(rs->nr == 0 || rs->ranges[rs->nr-1].end <= a);
range_set_append_unsafe(rs, a, b);
@@ -107,7 +107,7 @@ static void range_set_check_invariants(struct range_set *rs)
  * In-place pass of sorting and merging the ranges in the range set,
  * to establish the invariants when we get the ranges from the user
  */
-static void sort_and_merge_range_set(struct range_set *rs)
+void sort_and_merge_range_set(struct range_set *rs)
 {
int i;
int o = 0; /* output cursor */
diff --git a/line-log.h b/line-log.h
index 8bea45f..a9212d8 100644
--- a/line-log.h
+++ b/line-log.h
@@ -25,6 +25,18 @@ struct diff_ranges {
struct range_set target;
 };
 
+extern void range_set_init(struct range_set *, size_t prealloc);
+extern void range_set_release(struct range_set *);
+/* Range includes start; excludes end */
+extern void range_set_append_unsafe(struct range_set *, long start, long end);
+/* New range must begin at or after end of last added range */
+extern void range_set_append(struct range_set *, long start, long end);
+/*
+ * In-place pass of sorting and merging the ranges in the range set,
+ * to sort and make the ranges disjoint.
+ */
+extern void sort_and_merge_range_set(struct range_set *);
+
 /* Linked list of interesting files and their associated ranges.  The
  * list must be kept sorted by path.
  *
-- 
1.8.4.rc1.409.gbd48715

--
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 02/16] line-range-format.txt: clarify -L:regex usage form

2013-08-06 Thread Eric Sunshine
blame/log documentation describes -L option as:

  -L,
  -L:

   and  can take one of these forms:

* number
* /regex/
* +offset or -offset
* :regex

which is incorrect and confusing since :regex is not one of the valid
forms of  or ; in fact, it must be -L's lone argument.

Clarify by discussing : at the same indentation level as "
and ...":

  -L,
  -L:

   and  can take one of these forms:

* number
* /regex/
* +offset or -offset

  If : is given in place of  and  ...

Signed-off-by: Eric Sunshine 
---
 Documentation/blame-options.txt | 2 --
 Documentation/git-log.txt   | 2 --
 Documentation/line-range-format.txt | 7 +++
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 4e55b15..489032c 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -15,8 +15,6 @@
``-L '' or ``-L ,'' spans from  to end of file.
``-L ,'' spans from start of file to .
 +
- and  can take one of these forms:
-
 include::line-range-format.txt[]
 
 -l::
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index fe04e06..34097ef 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -72,8 +72,6 @@ produced by --stat etc.
give zero or one positive revision arguments.
You can specify this option more than once.
 +
- and  can take one of these forms:
-
 include::line-range-format.txt[]
 
 ::
diff --git a/Documentation/line-range-format.txt 
b/Documentation/line-range-format.txt
index 3e7ce72..a1b2f4e 100644
--- a/Documentation/line-range-format.txt
+++ b/Documentation/line-range-format.txt
@@ -1,3 +1,5 @@
+ and  can take one of these forms:
+
 - number
 +
 If  or  is a number, it specifies an
@@ -15,11 +17,8 @@ starting at the line given by .
 +
 This is only valid for  and will specify a number
 of lines before or after the line given by .
-+
 
-- :regex
 +
-If the option's argument is of the form :regex, it denotes the range
+If ``:'' is given in place of  and , it denotes the range
 from the first funcname line that matches , up to the next
 funcname line.
-+
-- 
1.8.4.rc1.409.gbd48715

--
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 09/16] blame: teach -L/RE/ to search from end of previous -L range

2013-08-06 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 builtin/blame.c |  5 -
 t/annotate-tests.sh | 20 
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 7b084d8..1bf8056 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2280,6 +2280,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
int cmd_is_annotate = !strcmp(argv[0], "annotate");
struct range_set ranges;
unsigned int range_i;
+   long anchor;
 
git_config(git_blame_config, NULL);
init_revisions(&revs, NULL);
@@ -2475,11 +2476,12 @@ parse_done:
if (lno && !range_list.nr)
string_list_append(&range_list, xstrdup("1"));
 
+   anchor = 1;
range_set_init(&ranges, range_list.nr);
for (range_i = 0; range_i < range_list.nr; ++range_i) {
long bottom, top;
if (parse_range_arg(range_list.items[range_i].string,
-   nth_line_cb, &sb, lno, 1,
+   nth_line_cb, &sb, lno, anchor,
&bottom, &top, sb.path))
usage(blame_usage);
if (lno < top || ((lno || bottom) && lno < bottom))
@@ -2490,6 +2492,7 @@ parse_done:
top = lno;
bottom--;
range_set_append_unsafe(&ranges, bottom, top);
+   anchor = top + 1;
}
sort_and_merge_range_set(&ranges);
 
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 77083d9..b963d36 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -303,6 +303,26 @@ test_expect_success 'blame -L multiple (superset/subset: 
unordered)' '
check_count -L3,5 -L2,8 A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1
 '
 
+test_expect_success 'blame -L /RE/ (relative)' '
+   check_count -L3,3 -L/fox/ B1 1 B2 1 C 1 D 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L /RE/ (relative: no preceding range)' '
+   check_count -L/dog/ A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L /RE/ (relative: adjacent)' '
+   check_count -L1,1 -L/dog/,+1 A 1 E 1
+'
+
+test_expect_success 'blame -L /RE/ (relative: not found)' '
+   test_must_fail $PROG -L4,4 -L/dog/ file
+'
+
+test_expect_success 'blame -L /RE/ (relative: end-of-file)' '
+   test_must_fail $PROG -L, -L/$/ file
+'
+
 test_expect_success 'setup -L :regex' '
tr Q "\\t" >hello.c <<-\EOF &&
int main(int argc, const char *argv[])
-- 
1.8.4.rc1.409.gbd48715

--
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 16/16] line-range: reject -L line numbers less than 1

2013-08-06 Thread Eric Sunshine
Since inception, git-blame -L has been documented as accepting 1-based
line numbers. When handed a line number less than 1, -L's behavior is
undocumented and undefined; it's also nonsensical and should be
diagnosed as an error. Do so.

Signed-off-by: Eric Sunshine 
---
 line-range.c|  5 -
 t/annotate-tests.sh | 18 +-
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/line-range.c b/line-range.c
index ede0c6c..de4e32f 100644
--- a/line-range.c
+++ b/line-range.c
@@ -54,8 +54,11 @@ static const char *parse_loc(const char *spec, nth_line_fn_t 
nth_line,
}
num = strtol(spec, &term, 10);
if (term != spec) {
-   if (ret)
+   if (ret) {
+   if (num <= 0)
+   die("-L invalid line number: %ld", num);
*ret = num;
+   }
return term;
}
 
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 376b042..99caa42 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -185,15 +185,15 @@ test_expect_success 'blame -L Y,X (undocumented)' '
check_count -L6,3 B 1 B1 1 B2 1 D 1
 '
 
-test_expect_failure 'blame -L -X' '
+test_expect_success 'blame -L -X' '
test_must_fail $PROG -L-1 file
 '
 
-test_expect_failure 'blame -L 0' '
+test_expect_success 'blame -L 0' '
test_must_fail $PROG -L0 file
 '
 
-test_expect_failure 'blame -L ,0' '
+test_expect_success 'blame -L ,0' '
test_must_fail $PROG -L,0 file
 '
 
@@ -447,8 +447,8 @@ test_expect_success 'blame empty' '
check_count -h HEAD^^ -f incremental
 '
 
-test_expect_success 'blame -L 0 empty (undocumented)' '
-   check_count -h HEAD^^ -f incremental -L0
+test_expect_success 'blame -L 0 empty' '
+   test_must_fail $PROG -L0 incremental HEAD^^
 '
 
 test_expect_success 'blame -L 1 empty' '
@@ -463,8 +463,8 @@ test_expect_success 'blame half' '
check_count -h HEAD^ -f incremental I 1
 '
 
-test_expect_success 'blame -L 0 half (undocumented)' '
-   check_count -h HEAD^ -f incremental -L0 I 1
+test_expect_success 'blame -L 0 half' '
+   test_must_fail $PROG -L0 incremental HEAD^
 '
 
 test_expect_success 'blame -L 1 half' '
@@ -483,8 +483,8 @@ test_expect_success 'blame full' '
check_count -f incremental I 1
 '
 
-test_expect_success 'blame -L 0 full (undocumented)' '
-   check_count -f incremental -L0 I 1
+test_expect_success 'blame -L 0 full' '
+   test_must_fail $PROG -L0 incremental
 '
 
 test_expect_success 'blame -L 1 full' '
-- 
1.8.4.rc1.409.gbd48715

--
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 15/16] t8001/t8002: blame: add tests of -L line numbers less than 1

2013-08-06 Thread Eric Sunshine
git-blame -L is documented as accepting 1-based line numbers. When
handed a line number less than 1, -L's behavior is undocumented and
undefined; it's also nonsensical and should be rejected but is
nevertheless accepted. Demonstrate this shortcoming.

Signed-off-by: Eric Sunshine 
---
 t/annotate-tests.sh | 12 
 1 file changed, 12 insertions(+)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index dabe89d..376b042 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -185,6 +185,18 @@ test_expect_success 'blame -L Y,X (undocumented)' '
check_count -L6,3 B 1 B1 1 B2 1 D 1
 '
 
+test_expect_failure 'blame -L -X' '
+   test_must_fail $PROG -L-1 file
+'
+
+test_expect_failure 'blame -L 0' '
+   test_must_fail $PROG -L0 file
+'
+
+test_expect_failure 'blame -L ,0' '
+   test_must_fail $PROG -L,0 file
+'
+
 test_expect_success 'blame -L ,+0' '
test_must_fail $PROG -L,+0 file
 '
-- 
1.8.4.rc1.409.gbd48715

--
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 13/16] line-range: teach -L:RE to search from end of previous -L range

2013-08-06 Thread Eric Sunshine
For consistency with -L/RE/, teach -L:RE to search relative to the end
of the previous -L range, if any.

The new behavior invalidates one test in t4211 which assumes that -L:RE
begins searching at start of file. This test will be resurrected in a
follow-up patch which teaches -L:RE how to override the default relative
search behavior.

Signed-off-by: Eric Sunshine 
---
 Documentation/line-range-format.txt |  3 ++-
 line-range.c| 12 +++-
 t/annotate-tests.sh | 16 
 t/t4211-line-log.sh |  1 -
 4 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/Documentation/line-range-format.txt 
b/Documentation/line-range-format.txt
index cf84417..469d80b 100644
--- a/Documentation/line-range-format.txt
+++ b/Documentation/line-range-format.txt
@@ -24,4 +24,5 @@ of lines before or after the line given by .
 +
 If ``:'' is given in place of  and , it denotes the range
 from the first funcname line that matches , up to the next
-funcname line.
+funcname line. ``:'' searches from the end of the previous `-L` range,
+if any, otherwise from the start of file.
diff --git a/line-range.c b/line-range.c
index 7048489..4bae4bf 100644
--- a/line-range.c
+++ b/line-range.c
@@ -161,7 +161,7 @@ static const char 
*find_funcname_matching_regexp(xdemitconf_t *xecfg, const char
 }
 
 static const char *parse_range_funcname(const char *arg, nth_line_fn_t 
nth_line_cb,
-   void *cb_data, long lines, long *begin, 
long *end,
+   void *cb_data, long lines, long anchor, 
long *begin, long *end,
const char *path)
 {
char *pattern;
@@ -187,7 +187,8 @@ static const char *parse_range_funcname(const char *arg, 
nth_line_fn_t nth_line_
 
pattern = xstrndup(arg+1, term-(arg+1));
 
-   start = nth_line_cb(cb_data, 0);
+   anchor--; /* input is in human terms */
+   start = nth_line_cb(cb_data, anchor);
 
drv = userdiff_find_by_path(path);
if (drv && drv->funcname.pattern) {
@@ -205,7 +206,8 @@ static const char *parse_range_funcname(const char *arg, 
nth_line_fn_t nth_line_
 
p = find_funcname_matching_regexp(xecfg, (char*) start, ®exp);
if (!p)
-   die("-L parameter '%s': no match", pattern);
+   die("-L parameter '%s' starting at line %ld: no match",
+   pattern, anchor + 1);
*begin = 0;
while (p > nth_line_cb(cb_data, *begin))
(*begin)++;
@@ -244,7 +246,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t 
nth_line_cb,
anchor = lines + 1;
 
if (*arg == ':') {
-   arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines, 
begin, end, path);
+   arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines, 
anchor, begin, end, path);
if (!arg || *arg)
return -1;
return 0;
@@ -269,7 +271,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t 
nth_line_cb,
 const char *skip_range_arg(const char *arg)
 {
if (*arg == ':')
-   return parse_range_funcname(arg, NULL, NULL, 0, NULL, NULL, 
NULL);
+   return parse_range_funcname(arg, NULL, NULL, 0, 0, NULL, NULL, 
NULL);
 
arg = parse_loc(arg, NULL, NULL, 0, -1, NULL);
 
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 5a7d7c7..4f7d6ba 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -382,6 +382,22 @@ test_expect_success 'blame -L :nomatch' '
test_must_fail $PROG -L:nomatch hello.c
 '
 
+test_expect_success 'blame -L :RE (relative)' '
+   check_count -f hello.c -L3,3 -L:ma.. F 1 H 4
+'
+
+test_expect_success 'blame -L :RE (relative: no preceding range)' '
+   check_count -f hello.c -L:ma.. F 4 G 1
+'
+
+test_expect_success 'blame -L :RE (relative: not found)' '
+   test_must_fail $PROG -L3,3 -L:tambourine hello.c
+'
+
+test_expect_success 'blame -L :RE (relative: end-of-file)' '
+   test_must_fail $PROG -L, -L:main hello.c
+'
+
 test_expect_success 'setup incremental' '
(
GIT_AUTHOR_NAME=I &&
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index b01b3dd..8ba2d51 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -48,7 +48,6 @@ canned_test "-M -L '/long f/,/^}/:b.c' move-support" 
move-support-f
 canned_test "-M -L ':f:b.c' parallel-change" parallel-change-f-to-main
 
 canned_test "-L 4,12:a.c -L :main:a.c simple" multiple
-canned_test "-L 4,18:a.c -L :main:a.c simple" multiple-overlapping
 canned_test "-L :main:a.c -L 4,18:a.c simple" multiple-overlapping
 canned_test "-L 4:a.c -L 8,12:a.c simple" multiple-superset
 canned_test "-L 8,12:a.c -L 4:a.c simple" multiple-superset
-- 
1.8.4.rc1.409.gbd48715

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

[PATCH v2 04/16] blame: inline one-line function into its lone caller

2013-08-06 Thread Eric Sunshine
As of 25ed3412 (Refactor parse_loc; 2013-03-28),
blame.c:prepare_blame_range() became effectively a one-line function
which merely passes its arguments along to another function. This
indirection does not bring clarity to the code. Simplify by inlining
prepare_blame_range() into its lone caller.

Signed-off-by: Eric Sunshine 
---
 builtin/blame.c | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index e70b089..9db01b0 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1937,18 +1937,6 @@ static const char *add_prefix(const char *prefix, const 
char *path)
return prefix_path(prefix, prefix ? strlen(prefix) : 0, path);
 }
 
-/*
- * Parsing of -L option
- */
-static void prepare_blame_range(struct scoreboard *sb,
-   const char *bottomtop,
-   long lno,
-   long *bottom, long *top)
-{
-   if (parse_range_arg(bottomtop, nth_line_cb, sb, lno, bottom, top, 
sb->path))
-   usage(blame_usage);
-}
-
 static int git_blame_config(const char *var, const char *value, void *cb)
 {
if (!strcmp(var, "blame.showroot")) {
@@ -2493,8 +2481,9 @@ parse_done:
lno = prepare_lines(&sb);
 
bottom = top = 0;
-   if (bottomtop)
-   prepare_blame_range(&sb, bottomtop, lno, &bottom, &top);
+   if (bottomtop && parse_range_arg(bottomtop, nth_line_cb, &sb, lno,
+&bottom, &top, sb.path))
+   usage(blame_usage);
if (lno < top || ((lno || bottom) && lno < bottom))
die("file %s has only %lu lines", path, lno);
if (bottom < 1)
-- 
1.8.4.rc1.409.gbd48715

--
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 08/16] line-range: teach -L/RE/ to search relative to anchor point

2013-08-06 Thread Eric Sunshine
Range specification -L/RE/ for blame/log unconditionally begins
searching at line one. Mailing list discussion [1] suggests that, in the
presence of multiple -L options, -L/RE/ should search relative to the
endpoint of the previous -L range, if any.

Teach the parsing machinery underlying blame's and log's -L options to
accept a start point for -L/RE/ searches. Follow-up patches will upgrade
blame and log to take advantage of this ability.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/229755/focus=229966

Signed-off-by: Eric Sunshine 
---
 builtin/blame.c |  2 +-
 line-log.c  |  2 +-
 line-range.c| 30 ++
 line-range.h|  5 -
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 2f4d9e2..7b084d8 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2479,7 +2479,7 @@ parse_done:
for (range_i = 0; range_i < range_list.nr; ++range_i) {
long bottom, top;
if (parse_range_arg(range_list.items[range_i].string,
-   nth_line_cb, &sb, lno,
+   nth_line_cb, &sb, lno, 1,
&bottom, &top, sb.path))
usage(blame_usage);
if (lno < top || ((lno || bottom) && lno < bottom))
diff --git a/line-log.c b/line-log.c
index bdadf35..38f827b 100644
--- a/line-log.c
+++ b/line-log.c
@@ -591,7 +591,7 @@ parse_lines(struct commit *commit, const char *prefix, 
struct string_list *args)
cb_data.line_ends = ends;
 
if (parse_range_arg(range_part, nth_line, &cb_data,
-   lines, &begin, &end,
+   lines, 1, &begin, &end,
full_name))
die("malformed -L argument '%s'", range_part);
if (lines < end || ((lines || begin) && lines < begin))
diff --git a/line-range.c b/line-range.c
index 69e8d6b..bbf3c0f 100644
--- a/line-range.c
+++ b/line-range.c
@@ -6,6 +6,18 @@
 
 /*
  * Parse one item in the -L option
+ *
+ * 'begin' is applicable only to relative range anchors. Absolute anchors
+ * ignore this value.
+ *
+ * When parsing "-L A,B", parse_loc() is called once for A and once for B.
+ *
+ * When parsing A, 'begin' must be a negative number, the absolute value of
+ * which is the line at which relative start-of-range anchors should be
+ * based. Beginning of file is represented by -1.
+ *
+ * When parsing B, 'begin' must be the positive line number immediately
+ * following the line computed for 'A'.
  */
 static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
 void *data, long lines, long begin, long *ret)
@@ -46,6 +58,10 @@ static const char *parse_loc(const char *spec, nth_line_fn_t 
nth_line,
*ret = num;
return term;
}
+
+   if (begin < 0)
+   begin = -begin;
+
if (spec[0] != '/')
return spec;
 
@@ -85,7 +101,8 @@ static const char *parse_loc(const char *spec, nth_line_fn_t 
nth_line,
else {
char errbuf[1024];
regerror(reg_error, ®exp, errbuf, 1024);
-   die("-L parameter '%s': %s", spec + 1, errbuf);
+   die("-L parameter '%s' starting at line %ld: %s",
+   spec + 1, begin + 1, errbuf);
}
 }
 
@@ -210,11 +227,16 @@ static const char *parse_range_funcname(const char *arg, 
nth_line_fn_t nth_line_
 }
 
 int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb,
-   void *cb_data, long lines, long *begin, long *end,
-   const char *path)
+   void *cb_data, long lines, long anchor,
+   long *begin, long *end, const char *path)
 {
*begin = *end = 0;
 
+   if (anchor < 1)
+   anchor = 1;
+   if (anchor > lines)
+   anchor = lines + 1;
+
if (*arg == ':') {
arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines, 
begin, end, path);
if (!arg || *arg)
@@ -222,7 +244,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t 
nth_line_cb,
return 0;
}
 
-   arg = parse_loc(arg, nth_line_cb, cb_data, lines, 1, begin);
+   arg = parse_loc(arg, nth_line_cb, cb_data, lines, -anchor, begin);
 
if (*arg == ',')
arg = parse_loc(arg + 1, nth_line_cb, cb_data, lines, *begin + 
1, end);
diff --git a/line-range.h b/line-range.h
index ae3d012..83ba3c2 100644
--- a/line-range.h
+++ b/line-range.h
@@ -9,6 +9,9 @@
  * line 'lno' inside the 'cb_data'.  The caller is expected to already
  * have a suitable map at hand to make this a constant-time lookup.
  *
+ * 'anchor' is the 1-based line at which relative range specifications
+ * should be anchored. Absolute ranges are unaffected by this value.
+ *
 

[PATCH v2 12/16] line-range: teach -L^/RE/ to search from start of file

2013-08-06 Thread Eric Sunshine
The -L/RE/ option of blame/log searches from the end of the previous -L
range, if any. Add new notation -L^/RE/ to override this behavior and
search from start of file.

The new ^/RE/ syntax is valid only as the  argument of
-L,. The  argument, as usual, is relative to .

Signed-off-by: Eric Sunshine 
---
 Documentation/line-range-format.txt |  1 +
 line-range.c| 10 --
 t/annotate-tests.sh | 21 +
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/line-range-format.txt 
b/Documentation/line-range-format.txt
index 42d74f7..cf84417 100644
--- a/Documentation/line-range-format.txt
+++ b/Documentation/line-range-format.txt
@@ -11,6 +11,7 @@ absolute line number (lines count from 1).
 This form will use the first line matching the given
 POSIX regex. If  is a regex, it will search from the end of
 the previous `-L` range, if any, otherwise from the start of file.
+If  is ``^/regex/'', it will search from the start of file.
 If  is a regex, it will search
 starting at the line given by .
 +
diff --git a/line-range.c b/line-range.c
index bbf3c0f..7048489 100644
--- a/line-range.c
+++ b/line-range.c
@@ -59,8 +59,14 @@ static const char *parse_loc(const char *spec, nth_line_fn_t 
nth_line,
return term;
}
 
-   if (begin < 0)
-   begin = -begin;
+   if (begin < 0) {
+   if (spec[0] != '^')
+   begin = -begin;
+   else {
+   begin = 1;
+   spec++;
+   }
+   }
 
if (spec[0] != '/')
return spec;
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index b963d36..5a7d7c7 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -323,6 +323,23 @@ test_expect_success 'blame -L /RE/ (relative: 
end-of-file)' '
test_must_fail $PROG -L, -L/$/ file
 '
 
+test_expect_success 'blame -L ^/RE/ (absolute)' '
+   check_count -L3,3 -L^/dog/,+2 A 1 B2 1
+'
+
+test_expect_success 'blame -L ^/RE/ (absolute: no preceding range)' '
+   check_count -L^/dog/,+2 A 1 B2 1
+'
+
+test_expect_success 'blame -L ^/RE/ (absolute: not found)' '
+   test_must_fail $PROG -L4,4 -L^/tambourine/ file
+'
+
+test_expect_success 'blame -L ^/RE/ (absolute: end-of-file)' '
+   n=$(expr $(wc -l hello.c <<-\EOF &&
int main(int argc, const char *argv[])
@@ -464,3 +481,7 @@ test_expect_success 'blame -L X,+N (non-numeric N)' '
 test_expect_success 'blame -L X,-N (non-numeric N)' '
test_must_fail $PROG -L1,-N file
 '
+
+test_expect_success 'blame -L ,^/RE/' '
+   test_must_fail $PROG -L1,^/99/ file
+'
-- 
1.8.4.rc1.409.gbd48715

--
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 01/16] git-log.txt: place each -L option variation on its own line

2013-08-06 Thread Eric Sunshine
Standard practice in Git documentation is for each variation of an
option (such as: -p / --porcelain) to be placed on its own line in the
OPTIONS table. The -L option does not follow suit. It cuddles "-L
,:" and "-L ::", separated by a comma.
This is inconsistent and potentially confusing since the comma
separating them is typeset the same as the comma in ",". Fix
this by placing each variation on its own line.

Signed-off-by: Eric Sunshine 
---
 Documentation/git-log.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index ac2694d..fe04e06 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -62,7 +62,8 @@ produced by --stat etc.
Note that only message is considered, if also a diff is shown
its size is not included.
 
--L ,:, -L 
+-L ,:::
+-L 
 
Trace the evolution of the line range given by ","
(or the funcname regex ) within the .  You may
-- 
1.8.4.rc1.409.gbd48715

--
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 00/16] blame: accept multiple -L ranges

2013-08-06 Thread Eric Sunshine
This is a re-roll of [1] which teaches git-blame to accept multiple -L
ranges. It is built atop [6] (es/blame-L-more in 'pu').

The series is longer than expected since it includes a few more cleanup
patches beyond those already posted separately [2], [3], [4], [5], [6];
and because it implements relative /RE/ searches requested by Junio [7],
and brings git-log's multiple -L behavior in line with git-blame's.
Despite the length of the series, the patches are mostly small and
simple.


Changes since v1:

* Make /RE/ searches relative to end of previous -L, if any [7]. Ditto
  for :RE searches.

* Introduce ^/RE/ and ^:RE to search from start of file.

* Add (lots of) tests.

* Update documentation.

* Collect ranges via 'struct range_set' rather than (ab)using
  blame.c:coalesce() and add_blame_range().


Quick overview of patches:

1-2: More cleanups akin to [2], [3], [4], [5], [6].

3-7: Implement git-blame multiple -L support; add tests; update
  documentation.

8-14: Make /RE/ and :RE searches relative; introduce ^/RE/ and ^:RE to
  search from start of file; add tests; update documentation.

15-16: Reject bogus -L inputs for line numbers less than 1. This comes
  late in the series because this case becomes much easier to detect
  following patch 8.


[1]: http://thread.gmane.org/gmane.comp.version-control.git/229755
[2]: http://thread.gmane.org/gmane.comp.version-control.git/229917
[3]: http://thread.gmane.org/gmane.comp.version-control.git/230532
[4]: 
http://git.661346.n2.nabble.com/PATCH-0-6-fix-blame-L-regression-add-tests-tp7592174.html
[5]: http://thread.gmane.org/gmane.comp.version-control.git/231035
[6]: http://thread.gmane.org/gmane.comp.version-control.git/231412
[7]: http://article.gmane.org/gmane.comp.version-control.git/229966


Eric Sunshine (16):
  git-log.txt: place each -L option variation on its own line
  line-range-format.txt: clarify -L:regex usage form
  range-set: publish API for re-use by git-blame -L
  blame: inline one-line function into its lone caller
  blame: accept multiple -L ranges
  t8001/t8002: blame: add tests of multiple -L options
  blame: document multiple -L support
  line-range: teach -L/RE/ to search relative to anchor point
  blame: teach -L/RE/ to search from end of previous -L range
  log: teach -L/RE/ to search from end of previous -L range
  line-range-format.txt: document -L/RE/ relative search
  line-range: teach -L^/RE/ to search from start of file
  line-range: teach -L:RE to search from end of previous -L range
  line-range: teach -L^:RE to search from start of file
  t8001/t8002: blame: add tests of -L line numbers less than 1
  line-range: reject -L line numbers less than 1

 Documentation/blame-options.txt |  10 +--
 Documentation/git-blame.txt |  10 ++-
 Documentation/git-log.txt   |   5 +-
 Documentation/line-range-format.txt |  16 +++--
 builtin/blame.c |  93 ++
 line-log.c  |  25 ---
 line-log.h  |  12 
 line-range.c|  62 +
 line-range.h|   5 +-
 t/annotate-tests.sh | 130 ++--
 t/t4211-line-log.sh |   2 +-
 11 files changed, 282 insertions(+), 88 deletions(-)

-- 
1.8.4.rc1.409.gbd48715

--
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] branch, commit, name-rev: ease up boolean conditions

2013-08-06 Thread Stefan Beller
Now that the variables are readin by OPT_BOOL, which makes sure
to have the values being 0 or 1 after reading, we do not need
the double negation to map any other value to 1 for integer
variables.

Signed-off-by: Stefan Beller 
---
 builtin/branch.c   | 3 ++-
 builtin/commit.c   | 2 +-
 builtin/name-rev.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 4daed0b..0dca694 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
if (with_commit || merge_filter != NO_FILTER)
list = 1;
 
-   if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + 
!!unset_upstream > 1)
+   if (force_create + list + unset_upstream +
+   !!delete + !!rename + !!new_upstream > 1)
usage_with_options(builtin_branch_usage, options);
 
if (abbrev == -1)
diff --git a/builtin/commit.c b/builtin/commit.c
index c20426b..b0f86c8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const 
char *argv[],
if (patch_interactive)
interactive = 1;
 
-   if (!!also + !!only + !!all + !!interactive > 1)
+   if (also + only + all + interactive > 1)
die(_("Only one of --include/--only/--all/--interactive/--patch 
can be used."));
if (argc == 0 && (also || (only && !amend)))
die(_("No paths with --include/--only does not make sense."));
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a908a34..20fcf8c 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char 
*prefix)
 
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
-   if (!!all + !!transform_stdin + !!argc > 1) {
+   if (all + transform_stdin + !!argc > 1) {
error("Specify either a list, or --all, not both!");
usage_with_options(name_rev_usage, opts);
}
-- 
1.8.4.rc0.16.g7fca822.dirty

--
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: [PATCHv3 0/9] Removing deprecated parsing macros

2013-08-06 Thread Stefan Beller
On 08/06/2013 08:39 AM, Junio C Hamano wrote:
> Thanks.  Queued this at the tip of 'pu'.  There seem to be some
> fallouts found in the test suite, though.
> 

Thanks. I am sorry for forgetting 'make test' before sending patches.
And the test suite is correct.

e35ea450 (branch, commit, name-rev: ease up boolean conditions)
is faulty. In builtin/branch.c the variables 'delete' and 'rename' 
are not used as a true boolean but I assumed so. 
These are parsed in via OPT_BIT and depending if you pass -d or -D 
for deletion you have value 1 or 2 in delete.

Hence this change is incorrect:
-   if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + 
!!unset_upstream > 1)
+   if (delete + rename + force_create + list + unset_upstream +
+   !!new_upstream > 1)
usage_with_options(builtin_branch_usage, options);

The current patch e35ea450 ([PATCHv3 5/9] branch, commit, name-rev: ease up 
boolean conditions)
can be dropped/reverted and I'll resend it. (The order doesn't matter, 
you can just drop that commit and apply the resend version on top of 
origin/sb/parseopt-boolean-removal

Stefan





signature.asc
Description: OpenPGP digital signature


Re: [PATCH] git exproll: steps to tackle gc aggression

2013-08-06 Thread Duy Nguyen
On Tue, Aug 6, 2013 at 9:38 AM, Ramkumar Ramachandra  wrote:
> +   Garbage collect using a pseudo logarithmic packfile 
> maintenance
> +   approach.  This approach attempts to minimize packfile churn
> +   by keeping several generations of varying sized packfiles 
> around
> +   and only consolidating packfiles (or loose objects) which are
> +   either new packfiles, or packfiles close to the same size as
> +   another packfile.

I wonder if a simpler approach may be nearly efficient as this one:
keep the largest pack out, repack the rest at fetch/push time so there
are at most 2 packs at a time. Or we we could do the repack at 'gc
--auto' time, but with lower pack threshold (about 10 or so). When the
second pack is as big as, say half the size of the first, merge them
into one at "gc --auto" time. This can be easily implemented in
git-repack.sh.
-- 
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: [PATCH] Add missing test file for UTF-16.

2013-08-06 Thread Duy Nguyen
On Mon, Aug 5, 2013 at 11:44 PM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> On Sun, Aug 4, 2013 at 12:26 AM, brian m. carlson
>>  wrote:
>>> The test file that the UTF-16 rejection test looks for is missing, but this 
>>> went
>>> unnoticed because the test is expected to fail anyway; as a consequence, the
>>> test fails because the file containing the commit message is missing, and 
>>> not
>>> because the test file contains a NUL byte.  Fix this by including a sample 
>>> text
>>> file containing a commit message encoded in UTF-16.
>>
>> Tested-by: Duy Nguyen 
>>
>> and sorry, my bad. I think we need your sign-off in this patch.
>
> I think 37576c14 (commit_tree(): refuse commit messages that contain
> NULs, 2011-12-15) that marked this test with "test_expect_failure" is
> broken with or without this fix.  It should be more like so:
>
> diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
> index 37ddabb..5e72d72 100755
> --- a/t/t3900-i18n-commit.sh
> +++ b/t/t3900-i18n-commit.sh
> @@ -34,9 +34,9 @@ test_expect_success 'no encoding header for base case' '
> test z = "z$E"
>  '
>
> -test_expect_failure 'UTF-16 refused because of NULs' '
> +test_expect_success 'UTF-16 refused because of NULs' '
> echo UTF-16 >F &&
> -   git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
> +   test_must_fail git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
>  '

The intention was "UTF-16 is not supported yet but we want to". But I
don't think we (at least I) will put any effort on that front to allow
NUL in commit message, so the patch, as in "we do not support UTF-16",
is fine.
-- 
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: [PATCH 2/3] OS X: Fix redeclaration of die warning

2013-08-06 Thread David Aguilar
On Mon, Aug 5, 2013 at 11:00 AM, Junio C Hamano  wrote:
> Brian Gernhardt  writes:
>
>> compat/apple-common-crypto.h uses die() in one of its macros, but was
>> included in git-compat-util.h before the definition of die.
>>
>> Fix by simply moving the relevant block after the die/error/warning
>> declarations.
>
> Puzzled.  What needs fixing???
>
> Ahh, that one is not just making #define macros, but defining static
> inline functions.
>
> I wonder if they need to be static inlines to be duplicated at each
> call sites in the first place.  Wouldn't it be better to create a
> compat/something.c file to be linked with?

IMO it's not worth it right now because there's only a single call
site (imap-send).  The moment another call site is introduced then
compat/apple-common-crypto.c would be the natural home for it.
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Makefile: Fix APPLE_COMMON_CRYPTO with BLK_SHA1

2013-08-06 Thread David Aguilar
On Mon, Aug 5, 2013 at 10:52 AM, Junio C Hamano  wrote:
> Brian Gernhardt  writes:
>
>> It used to be that APPLE_COMMON_CRYPTO did nothing when BLK_SHA1 was
>> set.  But APPLE_COMMON_CRYPTO is now used for more than just SHA1 (see
>> 3ef2bca) so make sure that the appropriate libraries are always set.
>>
>> Signed-off-by: Brian Gernhardt 
>> ---
>>  Makefile | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 82f2e22..7051956 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1182,6 +1182,9 @@ ifdef NEEDS_SSL_WITH_CRYPTO
>>  else
>>   LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto
>>  endif
>> +ifdef APPLE_COMMON_CRYPTO
>> + LIB_4_CRYPTO += -framework Security -framework CoreFoundation
>> +endif
>>  endif
>>  ifdef NEEDS_LIBICONV
>>   ifdef ICONVDIR
>> @@ -1413,7 +1416,6 @@ ifdef PPC_SHA1
>>   LIB_H += ppc/sha1.h
>>  else
>>  ifdef APPLE_COMMON_CRYPTO
>> - LIB_4_CRYPTO += -framework Security -framework CoreFoundation
>>   COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>>   SHA1_HEADER = 
>>  else
>
> Hmph.
>
> So the people previously tested this must have built imap-send
> without blk-sha1, which not just linked with these libs but also
> included the  header file and defined
> the -DCOMMON_DIGEST_FOR_OPENSSL preprocessor macro.  Building with
> blk-sha1 would not have worked for them.
> Now we always link with these libraries, even when building with
> blk-sha1.  Do the COMPAT_CFLAGS and SHA1_HEADER pieces only needed
> when using the SHA1 digest implementation from CommonCrypto and
> nothing imap-send uses?

LIB_4_CRYPTO is used by imap-send only, and these libraries are needed
for the base64 git_CC_EVP_(Encode|Decode), so unconditionally adding
these libraries there is correct.

COMPAT_CFLAGS and SHA1_HEADER enable the common crypto SHA1 only.
BLK_SHA1 provides its own SHA1 so they're not needed there.

I tested the tip of da/darwin (pu) w/ and w/out BLK_SHA1.

Tested-by: David Aguilar 
-- 
David
--
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 (Aug 2013, #01; Thu, 1)

2013-08-06 Thread Thomas Rast
Junio C Hamano  writes:

> Thomas Rast  writes:
>
>> Junio C Hamano  writes:
>>
>>> * tr/log-full-diff-keep-true-parents (2013-08-01) 1 commit
>>>  - log: use true parents for diff even when rewriting
>>>
>>>  Output from "git log --full-diff -- " looked strange,
>>>  because comparison was done with the previous ancestor that touched
>>>  the specified , causing the patches for paths outside the
>>>  pathspec to show more than the single commit has changed.
>>>
>>>  I am not sure if that is necessarily a problem, though.  Output
>>>  from "git log --full-diff -2 -- " without this change
>>>  will be applicable to some codebase, but after this change that
>>>  will no longer be true (you will get only tiny parts of the change
>>>  that were made by the two commits in question, while missing all
>>>  the other changes).
>>
>> Hmm.  Uwe's original complaint was that --stat -- as in "what other
>> things are touched when we modify foo" -- is nonsensical.
[...]
> I am not so worried about actually applying that kind of patch, but
> more about reviewing what happened in each of the single logical
> step shown.  If you made two changes in two far-apart commits to
> files you are interested in and want to look at the changes made to
> other files to make each of them a complete solution, depending on
> how the change was split across files, you would get useful to
> useless result with your patch.  In one extreme, if your commits are
> too fine-grained and you committed one path at a time, then
> "full-diff" will not show anything useful, as the commits that hit
> the pathspec do not have changes to any other paths.  In another
> extreme, you may have preparatory changes to other paths in commits
> that immediately precede the commit that hits the pathspec and then
> finally conclude the topic by introducing a caller of the prepared
> codepath in other files to the file you are interested in, and your
> patch will show only the endgame change without showing the
> preparatory steps, which may or may not be useful, depending on what
> you are looking for.
>
> So while I do understand why sometimes you wish to see full diff 
> "git log --full-diff -- " to match output from "git show"
> without pathspec, I am not sure doing it unconditionally and by
> default like your patch does is the best way to go.

I'm utterly unconvinced.

First, note that the behavior only shows when you use an option that
enables parent rewriting, such as --graph or --parents.

So if we went with a default-off option, there would be something like
--diff-original-parents.  The user would have to discover this option,
and use it whenever he wants to use --full-diff in combination with
--graph.  To obtain the same diffs as without --graph.

How do you explain that to a user?  "git-log has about 150 options.
Some of them interact badly, but don't worry, there are options in there
somewhere that make the weirdness go away."

  [No, I didn't actually count, but it seems a good estimate from
  looking at

git grep -c -E '^-.*::' Documentation/rev-list-options.txt 
Documentation/diff-options.txt

  ]


Second, gitk's option "Limit diffs to listed paths", corresponding to
the inverse of --full-diff, shows the commits as with git-show.  (Not
internally, but still, the format is very similar.)  In particular it
only shows the changes from the commit itself.


Third, the only actual user data point I have is Uwe, who clearly
expected his diffs to remain the same across --graph.  There's a lot of
selection bias in this, because a happy user would not have complained,
but we can look at his example again (I added a right-hand side to the
range to make it reproducible).

With the patched version:

  $ git log --parents --stat v3.8..v3.9 --full-diff -- 
drivers/net/ethernet/freescale/fec.c
  commit 8d7ed0f051caf192994c02fbefb859eac03d1646 
14109a59caf93e6eae726a34bf2f0897508ec8c1
  [...]
  net: fec: fix regression in link change accounting
  [...]
   drivers/net/ethernet/freescale/fec.c | 1 +
   1 file changed, 1 insertion(+)

With current master:

  commit 8d7ed0f051caf192994c02fbefb859eac03d1646 
14109a59caf93e6eae726a34bf2f0897508ec8c1
  [...]
  net: fec: fix regression in link change accounting
  [...]
   Documentation/sound/alsa/ALSA-Configuration.txt|5 +-
   MAINTAINERS|   22 +-
   Makefile   |2 +-
   arch/alpha/Makefile|2 +-
  [...]
   509 files changed, 9507 insertions(+), 7014 deletions(-)

509 files.  I don't know about you, but I think that's completely
useless for reviewing anything.


The only compromise I can see flying is using something like
--diff-parents=original and --diff-parents=rewritten, defaulting to
'original'.  Behind the scenes they would toggle the appropriate
machinery: 'rewritten' would enable parent simplification even when it
is not otherwise enabled, and 'original' would enable saving p

Re: About close() in commit_lock_file()

2013-08-06 Thread Duy Nguyen
On Tue, Aug 6, 2013 at 1:41 PM, Johannes Sixt  wrote:
> Am 8/5/2013 16:23, schrieb Duy Nguyen:
>> close() is added in commit_lock_file(), before rename(), by 4723ee9
>> (Close files opened by lock_file() before unlinking. - 2007-11-13),
>> which is needed by Windows. But doesn't that create a gap between
>> close() and rename() on other platforms where another process can
>> replace .lock file with something else before rename() is executed?
>
> First, files manipulated by commit_lock_file() are to be opened only using
> lock_file() by definition. Opening such a file in with open() or fopen()
> or renaming it via rename() without using the lockfile.[ch] API is
> possible regardless of whether commit_lock_file() has closed the file or
> not. Such manipulation is already undefined behavior (from Git's point of
> view), and there is nothing we can do about "misbehaving" processes.
>
> Second, lock_file() uses O_CREAT|O_EXCL, which fails when the file exists,
> regardless of whether it is open or not.

The second process could unlink(.lock file) first, then open it, even
using lock_file(), but I think that falls into your "first" and is an
invalid case from git point of view, so..

> Conclusion: There is no problem.

Agreed. False alarm.
-- 
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: [PATCH] commit: reject non-characters

2013-08-06 Thread Peter Krefting

Junio C Hamano:


Indeed.


Thanks. Testcases are good, but not if they don't actually catch the 
bug one has just introduced :-)



-- >8 --
Subject: [PATCH] commit: typofix for xxFFF[EF] check

We wanted to catch all codepoints that ends with FFFE and ,
not with 0FFFE and 0.

Noticed and corrected by Peter Krefting.

Signed-off-by: Junio C Hamano 


Signed-off-by: Peter Krefting 

--
\\// Peter - http://www.softwolves.pp.se/
--
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