Repo with only one file
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> + 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
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
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
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
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
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
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
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
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
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
> 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
> 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
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
--- 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
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"
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
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
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
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
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
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
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
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.
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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)
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()
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
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