* Re: [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Junio C Hamano @ 2016-11-22 7:07 UTC (permalink / raw)
To: Stefan Beller
Cc: Brandon Williams, Jonathan Nieder, git@vger.kernel.org,
Jens Lehmann, Heiko Voigt
In-Reply-To: <CAGZ79kb_4wWs_90AfsT932iPWbCXf6yRq875JUxoRZjUcsBW5A@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> So I guess we should test a bit more extensively, maybe
>
> git status >expect
> git submodule embedgitdirs
> git status >actual
> test_cmp expect actual
> # further testing via
> test -f ..
> test -d ..
Something along that line. "status should succeed" does not tell
the readers what kind of breakage the test is expecting to protect
us from. If we are expecting a breakage in embed-git-dirs would
somehow corrupt an existing submodule, which would lead to "status"
that is run in the superproject report the submodule differently,
then comparing output before and after the operation may be a
reasonable test. Going there to the submodule working tree and
checking the health of the repository (of the submodule) may be
another sensible test.
>> In the
>> extreme, if the failed "git submodule" command did
>>
>> rm -fr .git ?* && git init
>>
>> wouldn't "git status" still succeed?
>
> In that particular case you'd get
> $ git status
> fatal: Not a git repository (or any parent up to mount point ....)
Even with "&& git init"? Or you forgot that part?
^ permalink raw reply
* [PATCH 0/3] Minor fixes on 'git worktree'
From: Nguyễn Thái Ngọc Duy @ 2016-11-22 10:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
This fixes two things:
- make sure the first item is always the main worktree even if we
fail to retrieve some info
- keep 'worktree list' order stable (which in turn fixes the random
failure on my 'worktree-move' series
Nguyễn Thái Ngọc Duy (3):
worktree.c: zero new 'struct worktree' on allocation
get_worktrees() must return main worktree as first item even on error
worktree list: keep the list sorted
builtin/worktree.c | 26 ++++++++++++++++++++++----
worktree.c | 20 ++++----------------
2 files changed, 26 insertions(+), 20 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* [PATCH 1/3] worktree.c: zero new 'struct worktree' on allocation
From: Nguyễn Thái Ngọc Duy @ 2016-11-22 10:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161122100046.8341-1-pclouds@gmail.com>
This keeps things a bit simpler when we add more fields, knowing that
default values are always zero.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
worktree.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/worktree.c b/worktree.c
index f7869f8..f7c1b5e 100644
--- a/worktree.c
+++ b/worktree.c
@@ -91,16 +91,11 @@ static struct worktree *get_main_worktree(void)
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
- worktree = xmalloc(sizeof(struct worktree));
+ worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
- worktree->id = NULL;
worktree->is_bare = is_bare;
- worktree->head_ref = NULL;
worktree->is_detached = is_detached;
- worktree->is_current = 0;
add_head_info(&head_ref, worktree);
- worktree->lock_reason = NULL;
- worktree->lock_reason_valid = 0;
done:
strbuf_release(&path);
@@ -138,16 +133,11 @@ static struct worktree *get_linked_worktree(const char *id)
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
- worktree = xmalloc(sizeof(struct worktree));
+ worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
worktree->id = xstrdup(id);
- worktree->is_bare = 0;
- worktree->head_ref = NULL;
worktree->is_detached = is_detached;
- worktree->is_current = 0;
add_head_info(&head_ref, worktree);
- worktree->lock_reason = NULL;
- worktree->lock_reason_valid = 0;
done:
strbuf_release(&path);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 2/3] get_worktrees() must return main worktree as first item even on error
From: Nguyễn Thái Ngọc Duy @ 2016-11-22 10:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161122100046.8341-1-pclouds@gmail.com>
This is required by git-worktree.txt, stating that the main worktree is
the first line (especially in --porcelain mode when we can't just change
behavior at will).
There's only one case when get_worktrees() may skip main worktree, when
parse_ref() fails. Update the code so that we keep first item as main
worktree and return something sensible in this case:
- In user-friendly mode, since we're not constraint by anything,
returning "(error)" should do the job (we already show "(detached
HEAD)" which is not machine-friendly). Actually errors should be
printed on stderr by parse_ref() (*)
- In plumbing mode, we do not show neither 'bare', 'detached' or
'branch ...', which is possible by the format description if I read
it right.
Careful readers may realize that when the local variable "head_ref" in
get_main_worktree() is emptied, add_head_info() will do nothing to
wt->head_sha1. But that's ok because head_sha1 is zero-ized in the
previous patch.
(*) Well, it does not. But it's supposed to be a stop gap
implementation until refs we can reuse code, to parse "ref: " stuff
in HEAD, from resolve_refs_unsafe(). Now may be the time since
refs refactoring is mostly done.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 8 +++++---
worktree.c | 6 ++----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 5c4854d..b835b91 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -388,7 +388,7 @@ static void show_worktree_porcelain(struct worktree *wt)
printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
if (wt->is_detached)
printf("detached\n");
- else
+ else if (wt->head_ref)
printf("branch %s\n", wt->head_ref);
}
printf("\n");
@@ -406,10 +406,12 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
else {
strbuf_addf(&sb, "%-*s ", abbrev_len,
find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));
- if (!wt->is_detached)
+ if (wt->is_detached)
+ strbuf_addstr(&sb, "(detached HEAD)");
+ else if (wt->head_ref)
strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0));
else
- strbuf_addstr(&sb, "(detached HEAD)");
+ strbuf_addstr(&sb, "(error)");
}
printf("%s\n", sb.buf);
diff --git a/worktree.c b/worktree.c
index f7c1b5e..a674efa 100644
--- a/worktree.c
+++ b/worktree.c
@@ -89,7 +89,7 @@ static struct worktree *get_main_worktree(void)
strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
- goto done;
+ strbuf_reset(&head_ref);
worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
@@ -97,7 +97,6 @@ static struct worktree *get_main_worktree(void)
worktree->is_detached = is_detached;
add_head_info(&head_ref, worktree);
-done:
strbuf_release(&path);
strbuf_release(&worktree_path);
strbuf_release(&head_ref);
@@ -173,8 +172,7 @@ struct worktree **get_worktrees(void)
list = xmalloc(alloc * sizeof(struct worktree *));
- if ((list[counter] = get_main_worktree()))
- counter++;
+ list[counter++] = get_main_worktree();
strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
dir = opendir(path.buf);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 3/3] worktree list: keep the list sorted
From: Nguyễn Thái Ngọc Duy @ 2016-11-22 10:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161122100046.8341-1-pclouds@gmail.com>
It makes it easier to write tests for. But it should also be good for
the user since locating a worktree by eye would be easier once they
notice this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index b835b91..80ccc51 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -434,6 +434,13 @@ static void measure_widths(struct worktree **wt, int *abbrev, int *maxlen)
}
}
+static int compare_worktree(const void *a_, const void *b_)
+{
+ const struct worktree *const *a = a_;
+ const struct worktree *const *b = b_;
+ return fspathcmp((*a)->path, (*b)->path);
+}
+
static int list(int ac, const char **av, const char *prefix)
{
int porcelain = 0;
@@ -448,11 +455,20 @@ static int list(int ac, const char **av, const char *prefix)
usage_with_options(worktree_usage, options);
else {
struct worktree **worktrees = get_worktrees();
- int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;
+ int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i, nr;
if (!porcelain)
measure_widths(worktrees, &abbrev, &path_maxlen);
+ for (i = nr = 0; worktrees[i]; i++)
+ nr++;
+
+ /*
+ * don't sort the first item (main worktree), which will
+ * always be the first
+ */
+ QSORT(worktrees + 1, nr - 1, compare_worktree);
+
for (i = 0; worktrees[i]; i++) {
if (porcelain)
show_worktree_porcelain(worktrees[i]);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [PATCH 2/3] stripspace: respect repository config
From: Duy Nguyen @ 2016-11-22 10:10 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Git Mailing List, Junio C Hamano, Ralf Thielow, Taufiq Hoven
In-Reply-To: <5567548295222401fab10d3f2901c1787afbfd07.1479737858.git.johannes.schindelin@gmx.de>
On Mon, Nov 21, 2016 at 9:18 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> When eff80a9 (Allow custom "comment char", 2013-01-16) taught the
> `stripspace` command to respect the config setting `core.commentChar`,
> it forgot that this variable may be defined in .git/config.
>
> So when rebasing interactively with a commentChar defined in the current
> repository's config, the help text at the bottom of the edit script
> potentially used an incorrect comment character. This was not only
> funny-looking, but also resulted in tons of warnings like this one:
>
> Warning: the command isn't recognized in the following line
> - #
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> builtin/stripspace.c | 4 +++-
> t/t0030-stripspace.sh | 2 +-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/stripspace.c b/builtin/stripspace.c
> index 15e716e..1e62a00 100644
> --- a/builtin/stripspace.c
> +++ b/builtin/stripspace.c
> @@ -44,8 +44,10 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
> if (argc)
> usage_with_options(stripspace_usage, options);
>
> - if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
> + if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
> + setup_git_directory_gently(NULL);
> git_config(git_default_config, NULL);
> + }
This conditional config file reading is a trap for similar bugs to
happen again. Is there any reason we should not just mark the command
RUN_SETUP_GENTLY in git.c and call git_config() here unconditionally?
>
> if (strbuf_read(&buf, 0, 1024) < 0)
> die_errno("could not read the input");
--
Duy
^ permalink raw reply
* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Duy Nguyen @ 2016-11-22 10:31 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Git Mailing List, Junio C Hamano, Ralf Thielow, Taufiq Hoven
In-Reply-To: <9ef529a4fbb60990a91d7bbfdd49c6d20d49e442.1479737858.git.johannes.schindelin@gmx.de>
On Mon, Nov 21, 2016 at 9:18 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> When 84c9dc2 (commit: allow core.commentChar=auto for character auto
> selection, 2014-05-17) extended the core.commentChar functionality to
> allow for the value 'auto', it forgot that rebase -i was already taught to
> handle core.commentChar,
I think this is 180bad3 (rebase -i: respect core.commentchar -
2013-02-11) and a couple followup fixes. My bad.
> and in turn forgot to let rebase -i handle that
> new value gracefully.
>
> Reported by Taufiq Hoven.
Another report in the same area: a merge conflict always writes the
"Conflicts:" section in the commit message with '#' as comment char
when core.commentChar is auto. I've known this for a while, but it has
not bitten me hard enough to do something about it,
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> git-rebase--interactive.sh | 13 +++++++++++--
> t/t3404-rebase-interactive.sh | 2 +-
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index ca994c5..6bb740c 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -93,8 +93,17 @@ eval '
> GIT_CHERRY_PICK_HELP="$resolvemsg"
> export GIT_CHERRY_PICK_HELP
>
> -comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
> -: ${comment_char:=#}
> +comment_char=$(git config --get core.commentchar 2>/dev/null)
> +case "$comment_char" in
> +''|auto)
> + comment_char=#
My first thought was "this kinda defeats the purpose of auto, doesn't
it". But 'auto' here is irrelevant because we control the leading
character of all lines in the todo file. 'auto' makes little sense in
this context, falling back to any comment char would do.
So, ack (after you fix the $(comment_char other people have mentioned,
of course).
--
Duy
^ permalink raw reply
* Re: [PATCH v1 12/19] Documentation/config: add splitIndex.maxPercentChange
From: Duy Nguyen @ 2016-11-22 10:35 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git,
Christian Couder
In-Reply-To: <CAP8UFD1mun7wz2WqV8GCj6MODVjP2mPEBRGbJTf_ypiGCgtb9Q@mail.gmail.com>
On Fri, Nov 18, 2016 at 9:34 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Mon, Nov 7, 2016 at 10:38 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> (sorry I got sick in the last few weeks and could not respond to this earlier)
>
> (Yeah, I have also been sick during the last few weeks.)
>
>> On Mon, Nov 7, 2016 at 4:44 AM, Christian Couder
>> <christian.couder@gmail.com> wrote:
>>> Le 6 nov. 2016 09:16, "Junio C Hamano" <gitster@pobox.com> a écrit :
>>>>
>>>> Christian Couder <christian.couder@gmail.com> writes:
>>>>
>>>> > I think it is easier for user to be able to just set core.splitIndex
>>>> > to true to enable split-index.
>>>>
>>>> You can have that exact benefit by making core.splitIndex to
>>>> bool-or-more. If your default is 20%, take 'true' as if the user
>>>> specified 20% and take 'false' as if the user specified 100% (or is
>>>> it 0%? I do not care about the details but you get the point).
>>
>>> Then if we ever add 'auto' and the user wants for example 10% instead of the
>>> default 20%, we will have to make it accept things like "auto,10".
>
> (Sorry for writing the above on my phone which added HTML, so that it
> didn't reach the list.)
>
>> In my opinion, "true" _is_ auto, which is a way to say "I trust you to
>> do the right thing, just re-split the index when it makes sense", "no"
>> is disabled of course. If the user wants to be specific, just write
>> "10" or some other percentage.(and either 0 or 100 would mean enable
>> split-index but do not re-split automatically, let _me_ do it when I
>> want it)
>
> The meaning of a future "auto" option for "core.splitIndex" could be
> "use the split-index feature only if the number of entries in whole
> index is greater than 10000 (by default)".
Well.. with the "just re-split the index when it makes sense" part,
the user entrusts git to do something sensible in all cases, and going
with absolute numbers might not be the best way, I think. It's big
responsibility :)
> If there is no difference between "true" and "auto" then, when users
> who have "core.splitIndex=true" will migrate to the git version that
> adds the "auto" feature, their repos with under 10000 entires will not
> use the split-index feature anymore. These users may then be annoyed
> that the behavior has been switched under them, and that the
> split-index feature is not always used despite having
> "core.splitIndex=true" in their config.
--
Duy
^ permalink raw reply
* Re: [PATCH 31/35] pathspec: allow querying for attributes
From: Duy Nguyen @ 2016-11-22 10:41 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Brandon Williams, Git Mailing List
In-Reply-To: <20161110203428.30512-32-sbeller@google.com>
On Fri, Nov 11, 2016 at 3:34 AM, Stefan Beller <sbeller@google.com> wrote:
> @@ -139,7 +140,8 @@ static size_t common_prefix_len(const struct pathspec *pathspec)
> PATHSPEC_LITERAL |
> PATHSPEC_GLOB |
> PATHSPEC_ICASE |
> - PATHSPEC_EXCLUDE);
> + PATHSPEC_EXCLUDE |
> + PATHSPEC_ATTR);
Hmm.. common_prefix_len() has always been a bit relaxing and can cover
more than needed. It's for early pruning. Exact pathspec matching
_will_ be done later anyway.
Is that obvious? I'm wondering if we need to add a line or two in the
big comment code before this statement. I'm thinking it is and we
probably don't need more comments...
--
Duy
^ permalink raw reply
* [PATCH] merge-recursive.c: use QSORT macro
From: Nguyễn Thái Ngọc Duy @ 2016-11-22 12:30 UTC (permalink / raw)
To: git; +Cc: René Scharfe, Nguyễn Thái Ngọc Duy
This is the follow up of rs/qsort series, merged in b8688ad (Merge
branch 'rs/qsort' - 2016-10-10), where coccinelle was used to do
automatic transformation.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
coccinelle missed this place, understandably, because it can't know
that
sizeof(*entries->items)
is the same as
sizeof(*df_name_compare.items)
without some semantic analysis.
merge-recursive.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 9041c2f..2d4dca9 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -451,7 +451,7 @@ static void record_df_conflict_files(struct merge_options *o,
string_list_append(&df_sorted_entries, next->string)->util =
next->util;
}
- qsort(df_sorted_entries.items, entries->nr, sizeof(*entries->items),
+ QSORT(df_sorted_entries.items, entries->nr,
string_list_df_name_compare);
string_list_clear(&o->df_conflict_file_set, 1);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [PATCH v1 12/19] Documentation/config: add splitIndex.maxPercentChange
From: Christian Couder @ 2016-11-22 13:13 UTC (permalink / raw)
To: Duy Nguyen
Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git,
Christian Couder
In-Reply-To: <CACsJy8BazeR=4tz3q2f35x=fCfp-Ld9LJz0mQh_CZoR_iXKEEQ@mail.gmail.com>
On Tue, Nov 22, 2016 at 11:35 AM, Duy Nguyen <pclouds@gmail.com> wrote:
[...]
>>> In my opinion, "true" _is_ auto, which is a way to say "I trust you to
>>> do the right thing, just re-split the index when it makes sense", "no"
>>> is disabled of course. If the user wants to be specific, just write
>>> "10" or some other percentage.(and either 0 or 100 would mean enable
>>> split-index but do not re-split automatically, let _me_ do it when I
>>> want it)
>>
>> The meaning of a future "auto" option for "core.splitIndex" could be
>> "use the split-index feature only if the number of entries in whole
>> index is greater than 10000 (by default)".
>
> Well.. with the "just re-split the index when it makes sense" part,
> the user entrusts git to do something sensible in all cases,
That's an interpretation of what "core.splitIndex=true" could mean,
but there could be users who trust Git to re-split when it makes
sense, but who do want to use the split-index on all theirs repos even
the small ones or who just don't trust Git to choose when it might be
better to use it or not.
Yeah, a typical git user would most of the time just trust Git for all
those things, but on the other hand there are companies out there that
are willing to tweak many configuration options to get the better
possible behavior for them.
In fact I am working on this for Booking.com, and if we find out later
that we would gain something significant, like performance
improvements or configuration simplification, by adding "auto" and/or
other configuration variables to tweak more split-index related
things, we might very well post patch series to do that.
So if we now mix things up just to avoid one more configuration
option, we could very well make things harder to develop, to
configure, to parse and to understand later, so it is not a trade off
worth making.
> and going
> with absolute numbers might not be the best way, I think. It's big
> responsibility :)
About going with absolute number, yeah I am not sure at all it is the
best way, but this was just part of an example to try to explain what
I am saying above.
^ permalink raw reply
* Re: [PATCH v1 12/19] Documentation/config: add splitIndex.maxPercentChange
From: Duy Nguyen @ 2016-11-22 13:20 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git,
Christian Couder
In-Reply-To: <CAP8UFD2VFQs2X14omWEy-iJzEeTEeEOa+fxMEq3HTQK3rFyuCg@mail.gmail.com>
On Tue, Nov 22, 2016 at 8:13 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> So if we now mix things up just to avoid one more configuration
> option, we could very well make things harder to develop, to
> configure, to parse and to understand later, so it is not a trade off
> worth making.
OK since we're still in the early phase of determining how to
re-split, I guess going with elaborate, precise configuration is
better, even if it's more config options. Once we know better, maybe
we can decide a good default cover-all "auto" then (or maybe not
because we find out no shoe fits all).
--
Duy
^ permalink raw reply
* Re: [PATCH v6 01/16] Git.pm: add subroutines for commenting lines
From: Vasco Almeida @ 2016-11-22 14:10 UTC (permalink / raw)
To: git
Cc: Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar,
Junio C Hamano
In-Reply-To: <20161111124541.8216-2-vascomalmeida@sapo.pt>
A Sex, 11-11-2016 às 11:45 -0100, Vasco Almeida escreveu:
> +=item comment_lines ( STRING [, STRING... ])
> +
> +Comments lines following core.commentchar configuration.
> +
> +=cut
> +
> +sub comment_lines {
> + my $comment_line_char = config("core.commentchar") || '#';
> + return prefix_lines("$comment_line_char ", @_);
> +}
> +
In light of the recent "Fix problems with rebase -i when
core.commentchar is defined" [1], I realized that this patch does not
handle the 'auto' value of core.commentchat configuration variable.
I propose to do the patch below in the next re-roll.
[1] http://www.mail-archive.com/git@vger.kernel.org/msg107818.html
-- >8 --
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 3a6d846..8d33634 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1073,6 +1073,7 @@ sub edit_hunk_manually {
my $is_reverse = $patch_mode_flavour{IS_REVERSE};
my ($remove_plus, $remove_minus) = $is_reverse ? ('-', '+') : ('+', '-');
my $comment_line_char = Git::config("core.commentchar") || '#';
+ $comment_line_char = '#' if ($comment_line_char eq 'auto');
print $fh Git::comment_lines sprintf(__ <<EOF, $remove_minus, $remove_plus, $comment_line_char),
---
To remove '%s' lines, make them ' ' lines (context).
diff --git a/perl/Git.pm b/perl/Git.pm
index 69cd1dd..47b5899 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1459,6 +1459,7 @@ Comments lines following core.commentchar configuration.
sub comment_lines {
my $comment_line_char = config("core.commentchar") || '#';
+ $comment_line_char = '#' if ($comment_line_char eq 'auto');
return prefix_lines("$comment_line_char ", @_);
}
^ permalink raw reply related
* v2.11 new diff heuristic?
From: Robert Dailey @ 2016-11-22 14:42 UTC (permalink / raw)
To: Git
The release notes mention a new heuristic for diff:
* Output from "git diff" can be made easier to read by selecting
which lines are common and which lines are added/deleted
intelligently when the lines before and after the changed section
are the same. A command line option is added to help with the
experiment to find a good heuristics.
However, it lacks information on exactly how to use this new feature.
I dug into the git diff documentation here:
https://git-scm.com/docs/git-diff
It mentions a "--compaction-heuristic" option. Is this the new
heuristic outlined by the release notes? If not, which is it? Is the
compaction heuristic compatible with the histogram diff algorithm? Is
there a config option to turn this on all the time? For that matter,
is this something I can keep on all the time or is it only useful in
certain situations?
There's still so much more about this feature I would like to know.
^ permalink raw reply
* [GIT PULL] l10n updates for 2.11.0 round 2
From: Jiang Xin @ 2016-11-22 14:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Alexander Shopov, Alex Henrie, Ralf Thielow, Jean-Noël Avila,
Marco Paolone, Changwoo Ryu, Vasco Almeida, Dimitriy Ryazantcev,
Peter Krefting, Trần Ngọc Quân, Jiang Xin,
Git List
Hi Junio,
The following changes since commit 1310affe024fba407bff55dbe65cd6d670c8a32d:
Git 2.11-rc2 (2016-11-17 13:47:36 -0800)
are available in the git repository at:
git://github.com/git-l10n/git-po tags/l10n-2.11.0-rnd2
for you to fetch changes up to 275588f93eedd8d7a556c38b75944b858e704dce:
l10n: Fixed typo of git fetch-pack command (2016-11-22 22:24:59 +0800)
----------------------------------------------------------------
l10n-2.11.0-rnd2
----------------------------------------------------------------
Changwoo Ryu (1):
l10n: ko.po: Update Korean translation
Dimitriy Ryazantcev (1):
l10n: ru.po: update Russian translation
Jean-Noel Avila (1):
l10n: fr.po v2.11.0_rnd1
Jiang Xin (8):
l10n: git.pot: v2.11.0 round 1 (209 new, 53 removed)
Merge branch 'russian-l10n' of https://github.com/DJm00n/git-po-ru
Merge branch 'ko/merge-l10n' of https://github.com/changwoo/git-l10n-ko
Merge branch 'fr_v2.11.0_rnd1' of git://github.com/jnavila/git
l10n: zh_CN: for git v2.11.0 l10n round 1
Merge branch 'master' of git://github.com/git-l10n/git-po
l10n: git.pot: v2.11.0 round 2 (1 new, 1 removed)
l10n: Fixed typo of git fetch-pack command
Peter Krefting (1):
l10n: sv.po: Update Swedish translation (2913t0f0u)
Trần Ngọc Quân (1):
l10n: vi.po: Updated translation to v2.11.0 (2913t)
Vasco Almeida (1):
l10n: pt_PT: update Portuguese translation
jfbu (1):
l10n: fr.po fix grammar mistakes
po/fr.po | 8916 +++++++++++++++++++++++++++++++++--------------------------
po/git.pot | 6664 ++++++++++++++++++++++++--------------------
po/ko.po | 8729 +++++++++++++++++++++++++++++++--------------------------
po/pt_PT.po | 8794 ++++++++++++++++++++++++++++++++--------------------------
po/ru.po | 4429 +++++++++++++++++------------
po/sv.po | 8709 +++++++++++++++++++++++++++++++--------------------------
po/vi.po | 8697 +++++++++++++++++++++++++++++++--------------------------
po/zh_CN.po | 8689 +++++++++++++++++++++++++++++++--------------------------
8 files changed, 35318 insertions(+), 28309 deletions(-)
--
Jiang Xin
^ permalink raw reply
* Re: [GIT PULL] l10n updates for 2.11.0 round 2
From: Jiang Xin @ 2016-11-22 15:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Alexander Shopov, Alex Henrie, Ralf Thielow, Jean-Noël Avila,
Marco Paolone, Changwoo Ryu, Vasco Almeida, Dimitriy Ryazantcev,
Peter Krefting, Trần Ngọc Quân, Jiang Xin,
Git List
In-Reply-To: <CANYiYbGtfZKshmfYxHM0sL2Z67=3KTF0kixP1OjumRw87e9XkA@mail.gmail.com>
Git 2.11.0-rc2 introduced a very small l10n update. Because the
update windows will be
closed tomorrow, I made a batch updates. See commit:
* https://github.com/git-l10n/git-po/commit/275588f93
I have a reduced diff for this commit using a custom diff driver, see:
* https://gist.github.com/jiangxin/6384b1e865249228e00385fab84ef3f3
2016-11-22 22:59 GMT+08:00 Jiang Xin <worldhello.net@gmail.com>:
> Hi Junio,
>
> The following changes since commit 1310affe024fba407bff55dbe65cd6d670c8a32d:
>
> Git 2.11-rc2 (2016-11-17 13:47:36 -0800)
>
> are available in the git repository at:
>
> git://github.com/git-l10n/git-po tags/l10n-2.11.0-rnd2
--
Jiang Xin
^ permalink raw reply
* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Johannes Schindelin @ 2016-11-22 16:04 UTC (permalink / raw)
To: Johannes Sixt
Cc: git, Junio C Hamano, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <975c0002-8afe-efa6-d967-72f2a0d21169@kdbg.org>
Hi Hannes,
On Mon, 21 Nov 2016, Johannes Sixt wrote:
> Am 21.11.2016 um 15:18 schrieb Johannes Schindelin:
> > -comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
> > -: ${comment_char:=#}
> > +comment_char=$(git config --get core.commentchar 2>/dev/null)
> > +case "$comment_char" in
> > +''|auto)
> > + comment_char=#
> > + ;;
> > +?)
> > + ;;
> > +*)
> > + comment_char=$(comment_char | cut -c1)
>
> comment_char is a command? Did you mean
>
> comment_char=$(echo "$comment_char" | cut -c1)
D'oh.
> It could be written without forking a process:
>
> comment_char=${comment_char%${comment_char#?}}
>
> (aka "remove from the end what remains after removing the first character")
I was considering this, actually, but it is rather unreadable. Better
rewrite it in C, to begin with.
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Johannes Schindelin @ 2016-11-22 16:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, git, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <xmqqbmx8k8c0.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 21 Nov 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> > index 29e91d8..202ac07 100755
> > --- a/t/t0030-stripspace.sh
> > +++ b/t/t0030-stripspace.sh
> > @@ -432,6 +432,13 @@ test_expect_success '-c with changed comment char' '
> > test_cmp expect actual
> > '
> >
> > +test_expect_failure '-c with comment char defined in .git/config' '
> > + test_config core.commentchar = &&
> > + printf "= foo\n" >expect &&
> > + printf "foo" | git stripspace -c >actual &&
>
> We'd want "\n" on this printf to match the one before as well,
True.
> The analysis of the log message in [2/3] is wrong and needs
> updating, though.
Thanks for following up on that, and for fixing the commit message.
> > +test_expect_failure 'rebase -i respects core.commentchar=auto' '
> > + test_config core.commentchar auto &&
> > + write_script copy-edit-script.sh <<-\EOF &&
> > + cp "$1" edit-script
> > + EOF
> > + test_set_editor "$(pwd)/copy-edit-script.sh" &&
> > + test_when_finished "git rebase --abort || :" &&
> > + git rebase -i HEAD^ &&
> > + grep "^#" edit-script &&
>
> This was added for debugging that was forgotten?
No, this was me trying to ensure that the comment character '#' *is* used.
As opposed to somehow testing any edit script that contains no commented
lines whatsoever.
But if you don't care, I won't, either.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/3] rebase -i: highlight problems with core.commentchar
From: Johannes Schindelin @ 2016-11-22 16:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20161121190514.18574-1-gitster@pobox.com>
Hi Junio,
On Mon, 21 Nov 2016, Junio C Hamano wrote:
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index 29e91d861c..c1f6411eb2 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -432,6 +432,15 @@ test_expect_success '-c with changed comment char' '
> test_cmp expect actual
> '
>
> +test_expect_failure '-c with comment char defined in .git/config' '
> + test_config core.commentchar = &&
> + printf "= foo\n" >expect &&
> + printf "foo" | (
Could I ask you to sneak in a \n here?
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH 2/3] stripspace: respect repository config
From: Johannes Schindelin @ 2016-11-22 16:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20161121190514.18574-2-gitster@pobox.com>
Hi Junio,
On Mon, 21 Nov 2016, Junio C Hamano wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The way "git stripspace" reads the configuration was not quite
> correct, in that it forgot to probe for a possibly existing
> repository (note: stripspace is designed to be usable outside the
> repository as well) before doing so. Due to this, .git/config was
> read only when the command was run from the top-level of the working
> tree.
>
> A recent change b9605bc4f2 ("config: only read .git/config from
> configured repos", 2016-09-12) stopped reading the repository-local
> configuration file ".git/config" unless the repository discovery
> process is done, and ".git/config" is no longer read even when run
> from the top-level, which exposed the bug even more.
>
> When rebasing interactively with a commentChar defined in the
> current repository's config, the help text at the bottom of the edit
> script potentially used an incorrect comment character. This was not
> only funny-looking, but also resulted in tons of warnings like this
> one:
>
> Warning: the command isn't recognized in the following line
> - #
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thanks for the corrected commit message!
Dscho
^ permalink raw reply
* Re: [PATCH 2/3] stripspace: respect repository config
From: Johannes Schindelin @ 2016-11-22 16:13 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano, Ralf Thielow, Taufiq Hoven
In-Reply-To: <CACsJy8D5oBR+vo2B+Ro2Q4SX0CG3jME4Gfs1_6AohccmpNvD0A@mail.gmail.com>
Hi Duy,
On Tue, 22 Nov 2016, Duy Nguyen wrote:
> On Mon, Nov 21, 2016 at 9:18 PM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> > When eff80a9 (Allow custom "comment char", 2013-01-16) taught the
> > `stripspace` command to respect the config setting `core.commentChar`,
> > it forgot that this variable may be defined in .git/config.
> >
> > So when rebasing interactively with a commentChar defined in the current
> > repository's config, the help text at the bottom of the edit script
> > potentially used an incorrect comment character. This was not only
> > funny-looking, but also resulted in tons of warnings like this one:
> >
> > Warning: the command isn't recognized in the following line
> > - #
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > builtin/stripspace.c | 4 +++-
> > t/t0030-stripspace.sh | 2 +-
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/stripspace.c b/builtin/stripspace.c
> > index 15e716e..1e62a00 100644
> > --- a/builtin/stripspace.c
> > +++ b/builtin/stripspace.c
> > @@ -44,8 +44,10 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
> > if (argc)
> > usage_with_options(stripspace_usage, options);
> >
> > - if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
> > + if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
> > + setup_git_directory_gently(NULL);
> > git_config(git_default_config, NULL);
> > + }
>
> This conditional config file reading is a trap for similar bugs to
> happen again. Is there any reason we should not just mark the command
> RUN_SETUP_GENTLY in git.c and call git_config() here unconditionally?
As I plan to slip these patches into Git for Windows v2.11.0, i.e. making
this a last-minute hot fix, I want to err on the side of caution. So I'd
rather keep this conditional (it might regress on the performance front,
or something).
Ciao,
Dscho
^ permalink raw reply
* [PATCH 0/2] Show Git Mailing List: a builtin difftool
From: Johannes Schindelin @ 2016-11-22 17:01 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
I have been working on the builtin difftool for a little over a week,
for two reasons:
1. Perl is really not native on Windows. Not only is there a performance
penalty to be paid just for running Perl scripts, we also have to deal
with the fact that users may have different Perl installations, with
different options, and some other Perl installation may decide to set
PERL5LIB globally, wreaking havoc with Git for Windows' Perl (which we
have to use because almost all other Perl distributions lack the
Subversion bindings we need for `git svn`).
2. Perl makes for a rather large reason that Git for Windows' installer
weighs in with >30MB. While one Perl script less does not relieve us
of that burden, it is one step in the right direction.
This pair of patches serves two purposes: to ask for reviews, and to
show what I plan to release as part of Git for Windows v2.11.0 (which is
due this Thursday, if Git v2.11.0 is released tomorrow, as planned).
The second patch really only explains how I will make sure that the
builtin difftool will only affect users who want to opt in to testing.
Johannes Schindelin (2):
difftool: add the builtin
difftool: add a feature flag for the builtin vs scripted version
.gitignore | 2 +
Makefile | 1 +
builtin.h | 1 +
builtin/builtin-difftool.c | 680 +++++++++++++++++++++++++++++++++++++++++++++
git-difftool.perl | 7 +
git.c | 21 ++
6 files changed, 712 insertions(+)
create mode 100644 builtin/builtin-difftool.c
base-commit: 1310affe024fba407bff55dbe65cd6d670c8a32d
Published-As: https://github.com/dscho/git/releases/tag/builtin-difftool-v1
Fetch-It-Via: git fetch https://github.com/dscho/git builtin-difftool-v1
--
2.10.1.583.g721a9e0
^ permalink raw reply
* [PATCH 1/2] difftool: add the builtin
From: Johannes Schindelin @ 2016-11-22 17:01 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1479834051.git.johannes.schindelin@gmx.de>
This adds a builtin difftool that represents a conversion of the current
Perl script version of the difftool.
The motivation is that Perl scripts are not at all native on Windows,
and that `git difftool` therefore is pretty slow on that platform, when
there is no good reason for it to be slow.
In addition, Perl does not really have access to Git's internals. That
means that any script will always have to jump through unnecessary
hoops.
The current version of the builtin difftool does not, however, make full
use of the internals but instead chooses to spawn a couple of Git
processes, still, to make for an easier conversion. There remains a lot
of room for improvement, left for a later date.
Note: the original difftool is still called by `git difftool`. To get the
new, experimental version, call `git builtin-difftool`. The reason: this
new, experimental, builtin difftool will be shipped as part of Git for
Windows v2.11.0, to allow for easier large-scale testing.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.gitignore | 1 +
Makefile | 1 +
builtin.h | 1 +
builtin/builtin-difftool.c | 680 +++++++++++++++++++++++++++++++++++++++++++++
git.c | 1 +
5 files changed, 684 insertions(+)
create mode 100644 builtin/builtin-difftool.c
diff --git a/.gitignore b/.gitignore
index 05cb58a..4f54531 100644
--- a/.gitignore
+++ b/.gitignore
@@ -51,6 +51,7 @@
/git-diff-tree
/git-difftool
/git-difftool--helper
+/git-builtin-difftool
/git-describe
/git-fast-export
/git-fast-import
diff --git a/Makefile b/Makefile
index f53fcc9..f764174 100644
--- a/Makefile
+++ b/Makefile
@@ -888,6 +888,7 @@ BUILTIN_OBJS += builtin/diff-files.o
BUILTIN_OBJS += builtin/diff-index.o
BUILTIN_OBJS += builtin/diff-tree.o
BUILTIN_OBJS += builtin/diff.o
+BUILTIN_OBJS += builtin/builtin-difftool.o
BUILTIN_OBJS += builtin/fast-export.o
BUILTIN_OBJS += builtin/fetch-pack.o
BUILTIN_OBJS += builtin/fetch.o
diff --git a/builtin.h b/builtin.h
index b9122bc..409a61e 100644
--- a/builtin.h
+++ b/builtin.h
@@ -60,6 +60,7 @@ extern int cmd_diff_files(int argc, const char **argv, const char *prefix);
extern int cmd_diff_index(int argc, const char **argv, const char *prefix);
extern int cmd_diff(int argc, const char **argv, const char *prefix);
extern int cmd_diff_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_builtin_difftool(int argc, const char **argv, const char *prefix);
extern int cmd_fast_export(int argc, const char **argv, const char *prefix);
extern int cmd_fetch(int argc, const char **argv, const char *prefix);
extern int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
diff --git a/builtin/builtin-difftool.c b/builtin/builtin-difftool.c
new file mode 100644
index 0000000..9feefcd
--- /dev/null
+++ b/builtin/builtin-difftool.c
@@ -0,0 +1,680 @@
+/*
+ * "git difftool" builtin command
+ *
+ * This is a wrapper around the GIT_EXTERNAL_DIFF-compatible
+ * git-difftool--helper script.
+ *
+ * This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.
+ * The GIT_DIFF* variables are exported for use by git-difftool--helper.
+ *
+ * Any arguments that are unknown to this script are forwarded to 'git diff'.
+ *
+ * Copyright (C) 2016 Johannes Schindelin
+ */
+#include "cache.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "run-command.h"
+#include "argv-array.h"
+#include "strbuf.h"
+#include "lockfile.h"
+
+static char *diff_gui_tool;
+static int trust_exit_code;
+
+static const char * const builtin_difftool_usage[] = {
+ N_("git add [<options>] [--] <pathspec>..."),
+ NULL
+};
+
+static int difftool_config(const char *var, const char *value, void *cb)
+{
+ if (!strcmp(var, "diff.guitool")) {
+ diff_gui_tool = xstrdup(value);
+ return 0;
+ }
+
+ if (!strcmp(var, "difftool.trustexitcode")) {
+ trust_exit_code = git_config_bool(var, value);
+ return 0;
+ }
+
+ return git_default_config(var, value, cb);
+}
+
+static int print_tool_help(void)
+{
+ const char *argv[] = { "mergetool", "--tool-help=diff", NULL };
+ return run_command_v_opt(argv, RUN_GIT_CMD);
+}
+
+static int parse_index_info(char *p, int *mode1, int *mode2,
+ struct object_id *oid1, struct object_id *oid2,
+ char *status)
+{
+ if (*p != ':')
+ return error("expected ':', got '%c'", *p);
+ *mode1 = (int)strtol(p + 1, &p, 8);
+ if (*p != ' ')
+ return error("expected ' ', got '%c'", *p);
+ *mode2 = (int)strtol(p + 1, &p, 8);
+ if (*p != ' ')
+ return error("expected ' ', got '%c'", *p);
+ if (get_oid_hex(++p, oid1))
+ return error("expected object ID, got '%s'", p + 1);
+ p += GIT_SHA1_HEXSZ;
+ if (*p != ' ')
+ return error("expected ' ', got '%c'", *p);
+ if (get_oid_hex(++p, oid2))
+ return error("expected object ID, got '%s'", p + 1);
+ p += GIT_SHA1_HEXSZ;
+ if (*p != ' ')
+ return error("expected ' ', got '%c'", *p);
+ *status = *++p;
+ if (!status || p[1])
+ return error("unexpected trailer: '%s'", p);
+ return 0;
+}
+
+/*
+ * Remove any trailing slash from $workdir
+ * before starting to avoid double slashes in symlink targets.
+ */
+static void add_path(struct strbuf *buf, size_t base_len, const char *path)
+{
+ strbuf_setlen(buf, base_len);
+ if (buf->len && buf->buf[buf->len - 1] != '/')
+ strbuf_addch(buf, '/');
+ strbuf_addstr(buf, path);
+}
+
+/*
+ * Determine whether we can simply reuse the file in the worktree.
+ */
+static int use_wt_file(const char *workdir, const char *name,
+ struct object_id *oid)
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct stat st;
+ int use = 0;
+
+ strbuf_addstr(&buf, workdir);
+ add_path(&buf, buf.len, name);
+
+ if (!lstat(buf.buf, &st) && !S_ISLNK(st.st_mode)) {
+ struct object_id wt_oid;
+ int fd = open(buf.buf, O_RDONLY);
+
+ if (!index_fd(wt_oid.hash, fd, &st, OBJ_BLOB, name, 0)) {
+ if (is_null_oid(oid)) {
+ oidcpy(oid, &wt_oid);
+ use = 1;
+ } else if (!oidcmp(oid, &wt_oid))
+ use = 1;
+ }
+ }
+
+ strbuf_release(&buf);
+
+ return use;
+}
+
+struct working_tree_entry {
+ struct hashmap_entry entry;
+ char path[FLEX_ARRAY];
+};
+
+static int working_tree_entry_cmp(struct working_tree_entry *a,
+ struct working_tree_entry *b, void *keydata)
+{
+ return strcmp(a->path, b->path);
+}
+
+/*
+ * The `left` and `right` entries hold paths for the symlinks hashmap,
+ * and a SHA-1 surrounded by brief text for submodules.
+ */
+struct pair_entry {
+ struct hashmap_entry entry;
+ char left[PATH_MAX], right[PATH_MAX];
+ const char path[FLEX_ARRAY];
+};
+
+static int pair_cmp(struct pair_entry *a, struct pair_entry *b, void *keydata)
+{
+ return strcmp(a->path, b->path);
+}
+
+static void add_left_or_right(struct hashmap *map, const char *path,
+ const char *content, int is_right)
+{
+ struct pair_entry *e, *existing;
+
+ FLEX_ALLOC_STR(e, path, path);
+ hashmap_entry_init(e, strhash(path));
+ existing = hashmap_get(map, e, NULL);
+ if (existing) {
+ free(e);
+ e = existing;
+ } else {
+ e->left[0] = e->right[0] = '\0';
+ hashmap_add(map, e);
+ }
+ strcpy(is_right ? e->right : e->left, content);
+}
+
+struct path_entry {
+ struct hashmap_entry entry;
+ char path[FLEX_ARRAY];
+};
+
+int path_entry_cmp(struct path_entry *a, struct path_entry *b, void *key)
+{
+ return strcmp(a->path, key ? key : b->path);
+}
+
+static void changed_files(struct hashmap *result, const char *index_path,
+ const char *workdir)
+{
+ struct child_process update_index = CHILD_PROCESS_INIT;
+ struct child_process diff_files = CHILD_PROCESS_INIT;
+ struct strbuf index_env = STRBUF_INIT, buf = STRBUF_INIT;
+ const char *git_dir = absolute_path(get_git_dir()), *env[] = {
+ NULL, NULL
+ };
+ FILE *fp;
+
+ strbuf_addf(&index_env, "GIT_INDEX_FILE=%s", index_path);
+ env[0] = index_env.buf;
+
+ argv_array_pushl(&update_index.args,
+ "--git-dir", git_dir, "--work-tree", workdir,
+ "update-index", "--really-refresh", "-q",
+ "--unmerged", NULL);
+ update_index.no_stdin = 1;
+ update_index.no_stdout = 1;
+ update_index.no_stderr = 1;
+ update_index.git_cmd = 1;
+ update_index.use_shell = 0;
+ update_index.clean_on_exit = 1;
+ update_index.dir = workdir;
+ update_index.env = env;
+ /* Ignore any errors of update-index */
+ run_command(&update_index);
+
+ argv_array_pushl(&diff_files.args,
+ "--git-dir", git_dir, "--work-tree", workdir,
+ "diff-files", "--name-only", "-z", NULL);
+ diff_files.no_stdin = 1;
+ diff_files.git_cmd = 1;
+ diff_files.use_shell = 0;
+ diff_files.clean_on_exit = 1;
+ diff_files.out = -1;
+ diff_files.dir = workdir;
+ diff_files.env = env;
+ if (start_command(&diff_files))
+ die("could not obtain raw diff");
+ fp = xfdopen(diff_files.out, "r");
+ while (!strbuf_getline_nul(&buf, fp)) {
+ struct path_entry *entry;
+ FLEX_ALLOC_STR(entry, path, buf.buf);
+ hashmap_entry_init(entry, strhash(buf.buf));
+ hashmap_add(result, entry);
+ }
+ if (finish_command(&diff_files))
+ die("diff-files did not exit properly");
+ strbuf_release(&index_env);
+ strbuf_release(&buf);
+}
+
+#include "dir.h"
+
+static NORETURN void exit_cleanup(const char *tmpdir, int exit_code)
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addstr(&buf, tmpdir);
+ remove_dir_recursively(&buf, 0);
+ if (exit_code)
+ warning(_("failed: %d"), exit_code);
+ exit(exit_code);
+}
+
+static int ensure_leading_directories(char *path)
+{
+ switch (safe_create_leading_directories(path)) {
+ case SCLD_OK:
+ case SCLD_EXISTS:
+ return 0;
+ default:
+ return error(_("could not create leading directories "
+ "of '%s'"), path);
+ }
+}
+
+static int run_dir_diff(const char *extcmd, int symlinks,
+ int argc, const char **argv)
+{
+ char tmpdir[PATH_MAX];
+ struct strbuf info = STRBUF_INIT, lpath = STRBUF_INIT;
+ struct strbuf rpath = STRBUF_INIT, buf = STRBUF_INIT;
+ struct strbuf ldir = STRBUF_INIT, rdir = STRBUF_INIT;
+ struct strbuf wtdir = STRBUF_INIT;
+ size_t ldir_len, rdir_len, wtdir_len;
+ struct cache_entry *ce = xcalloc(1, sizeof(ce) + PATH_MAX + 1);
+ const char *workdir, *tmp;
+ int ret = 0, i;
+ FILE *fp;
+ struct hashmap working_tree_dups, submodules, symlinks2;
+ struct hashmap_iter iter;
+ struct pair_entry *entry;
+ enum object_type type;
+ unsigned long size;
+ struct index_state wtindex;
+ struct checkout lstate, rstate;
+ int rc, flags = RUN_GIT_CMD, err = 0;
+ struct child_process child = CHILD_PROCESS_INIT;
+ const char *helper_argv[] = { "difftool--helper", NULL, NULL, NULL };
+ struct hashmap wt_modified, tmp_modified;
+ int indices_loaded = 0;
+
+ setup_work_tree();
+ workdir = get_git_work_tree();
+
+ /* Setup temp directories */
+ tmp = getenv("TMPDIR");
+ sprintf(tmpdir, "%s/git-difftool.XXXXXX", tmp ? tmp : "/tmp");
+ if (!mkdtemp(tmpdir))
+ return error("could not create temporary directory");
+ strbuf_addf(&ldir, "%s/left/", tmpdir);
+ strbuf_addf(&rdir, "%s/right/", tmpdir);
+ strbuf_addstr(&wtdir, workdir);
+ if (!wtdir.len || !is_dir_sep(wtdir.buf[wtdir.len - 1]))
+ strbuf_addch(&wtdir, '/');
+ mkdir(ldir.buf, 0777);
+ mkdir(rdir.buf, 0777);
+
+ memset(&wtindex, 0, sizeof(wtindex));
+
+ memset(&lstate, 0, sizeof(lstate));
+ lstate.base_dir = ldir.buf;
+ lstate.base_dir_len = ldir.len;
+ lstate.force = 1;
+ memset(&rstate, 0, sizeof(rstate));
+ rstate.base_dir = rdir.buf;
+ rstate.base_dir_len = rdir.len;
+ rstate.force = 1;
+
+ ldir_len = ldir.len;
+ rdir_len = rdir.len;
+ wtdir_len = wtdir.len;
+
+ hashmap_init(&working_tree_dups,
+ (hashmap_cmp_fn)working_tree_entry_cmp, 0);
+ hashmap_init(&submodules, (hashmap_cmp_fn)pair_cmp, 0);
+ hashmap_init(&symlinks2, (hashmap_cmp_fn)pair_cmp, 0);
+
+ child.no_stdin = 1;
+ child.git_cmd = 1;
+ child.use_shell = 0;
+ child.clean_on_exit = 1;
+ child.out = -1;
+ argv_array_pushl(&child.args, "diff", "--raw", "--no-abbrev", "-z",
+ NULL);
+ for (i = 0; i < argc; i++)
+ argv_array_push(&child.args, argv[i]);
+ if (start_command(&child))
+ die("could not obtain raw diff");
+ fp = xfdopen(child.out, "r");
+
+ /* Build index info for left and right sides of the diff */
+ while (!strbuf_getline_nul(&info, fp)) {
+ int lmode, rmode;
+ struct object_id loid, roid;
+ char status;
+ const char *src_path, *dst_path;
+ size_t src_path_len, dst_path_len;
+
+ if (starts_with(info.buf, "::"))
+ die(N_("combined diff formats('-c' and '--cc') are "
+ "not supported in\n"
+ "directory diff mode('-d' and '--dir-diff')."));
+
+ if (parse_index_info(info.buf, &lmode, &rmode, &loid, &roid,
+ &status))
+ break;
+ if (strbuf_getline_nul(&lpath, fp))
+ break;
+ src_path = lpath.buf;
+ src_path_len = lpath.len;
+
+ if (status != 'C' && status != 'R') {
+ dst_path = src_path;
+ dst_path_len = src_path_len;
+ } else {
+ if (strbuf_getline_nul(&rpath, fp))
+ break;
+ dst_path = rpath.buf;
+ dst_path_len = rpath.len;
+ }
+
+ if (S_ISGITLINK(lmode) || S_ISGITLINK(rmode)) {
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "Subproject commit %s",
+ oid_to_hex(&loid));
+ add_left_or_right(&submodules, src_path, buf.buf, 0);
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "Subproject commit %s",
+ oid_to_hex(&roid));
+ if (!oidcmp(&loid, &roid))
+ strbuf_addstr(&buf, "-dirty");
+ add_left_or_right(&submodules, dst_path, buf.buf, 1);
+ continue;
+ }
+
+ if (S_ISLNK(lmode)) {
+ char *content = read_sha1_file(loid.hash, &type, &size);
+ add_left_or_right(&symlinks2, src_path, content, 0);
+ free(content);
+ }
+
+ if (S_ISLNK(rmode)) {
+ char *content = read_sha1_file(roid.hash, &type, &size);
+ add_left_or_right(&symlinks2, dst_path, content, 1);
+ free(content);
+ }
+
+ if (lmode && status != 'C') {
+ ce->ce_mode = lmode;
+ oidcpy(&ce->oid, &loid);
+ strcpy(ce->name, src_path);
+ ce->ce_namelen = src_path_len;
+ if (checkout_entry(ce, &lstate, NULL))
+ return error("could not write '%s'", src_path);
+ }
+
+ if (rmode) {
+ struct working_tree_entry *entry;
+
+ /* Avoid duplicate working_tree entries */
+ FLEX_ALLOC_STR(entry, path, dst_path);
+ hashmap_entry_init(entry, strhash(dst_path));
+ if (hashmap_get(&working_tree_dups, entry, NULL)) {
+ free(entry);
+ continue;
+ }
+ hashmap_add(&working_tree_dups, entry);
+
+ if (!use_wt_file(workdir, dst_path, &roid)) {
+ ce->ce_mode = rmode;
+ oidcpy(&ce->oid, &roid);
+ strcpy(ce->name, dst_path);
+ ce->ce_namelen = dst_path_len;
+ if (checkout_entry(ce, &rstate, NULL))
+ return error("could not write '%s'",
+ dst_path);
+ } else if (!is_null_oid(&roid)) {
+ /*
+ * Changes in the working tree need special
+ * treatment since they are not part of the
+ * index.
+ */
+ struct cache_entry *ce2 =
+ make_cache_entry(rmode, roid.hash,
+ dst_path, 0, 0);
+ ce_mode_from_stat(ce2, rmode);
+
+ add_index_entry(&wtindex, ce2,
+ ADD_CACHE_JUST_APPEND);
+
+ add_path(&wtdir, wtdir_len, dst_path);
+ add_path(&rdir, rdir_len, dst_path);
+ if (ensure_leading_directories(rdir.buf))
+ return error("could not create "
+ "directory for '%s'",
+ dst_path);
+ if (symlinks) {
+ if (symlink(wtdir.buf, rdir.buf)) {
+ ret = error_errno("could not symlink '%s' to '%s'", wtdir.buf, rdir.buf);
+ goto finish;
+ }
+ } else {
+ struct stat st;
+ if (stat(wtdir.buf, &st))
+ st.st_mode = 0644;
+ if (copy_file(rdir.buf, wtdir.buf,
+ st.st_mode)) {
+ ret = error("could not copy '%s' to '%s'", wtdir.buf, rdir.buf);
+ goto finish;
+ }
+ }
+ }
+ }
+ }
+ if (finish_command(&child)) {
+ ret = error("error occurred running diff --raw");
+ goto finish;
+ }
+
+ /*
+ * Changes to submodules require special treatment.This loop writes a
+ * temporary file to both the left and right directories to show the
+ * change in the recorded SHA1 for the submodule.
+ */
+ hashmap_iter_init(&submodules, &iter);
+ while ((entry = hashmap_iter_next(&iter))) {
+ if (*entry->left) {
+ add_path(&ldir, ldir_len, entry->path);
+ ensure_leading_directories(ldir.buf);
+ write_file(ldir.buf, "%s", entry->left);
+ }
+ if (*entry->right) {
+ add_path(&rdir, rdir_len, entry->path);
+ ensure_leading_directories(rdir.buf);
+ write_file(rdir.buf, "%s", entry->right);
+ }
+ }
+
+ /*
+ * Symbolic links require special treatment.The standard "git diff"
+ * shows only the link itself, not the contents of the link target.
+ * This loop replicates that behavior.
+ */
+ hashmap_iter_init(&symlinks2, &iter);
+ while ((entry = hashmap_iter_next(&iter))) {
+ if (*entry->left) {
+ add_path(&ldir, ldir_len, entry->path);
+ ensure_leading_directories(ldir.buf);
+ write_file(ldir.buf, "%s", entry->left);
+ }
+ if (*entry->right) {
+ add_path(&rdir, rdir_len, entry->path);
+ ensure_leading_directories(rdir.buf);
+ write_file(rdir.buf, "%s", entry->right);
+ }
+ }
+
+ strbuf_release(&buf);
+
+ strbuf_setlen(&ldir, ldir_len);
+ helper_argv[1] = ldir.buf;
+ strbuf_setlen(&rdir, rdir_len);
+ helper_argv[2] = rdir.buf;
+
+ if (extcmd) {
+ helper_argv[0] = extcmd;
+ flags = 0;
+ } else
+ setenv("GIT_DIFFTOOL_DIRDIFF", "true", 1);
+ rc = run_command_v_opt(helper_argv, flags);
+
+ /*
+ * If the diff includes working copy files and those
+ * files were modified during the diff, then the changes
+ * should be copied back to the working tree.
+ * Do not copy back files when symlinks are used and the
+ * external tool did not replace the original link with a file.
+ *
+ * These hashes are loaded lazily since they aren't needed
+ * in the common case of --symlinks and the difftool updating
+ * files through the symlink.
+ */
+ hashmap_init(&wt_modified, (hashmap_cmp_fn)path_entry_cmp,
+ wtindex.cache_nr);
+ hashmap_init(&tmp_modified, (hashmap_cmp_fn)path_entry_cmp,
+ wtindex.cache_nr);
+
+ for (i = 0; i < wtindex.cache_nr; i++) {
+ struct hashmap_entry dummy;
+ const char *name = wtindex.cache[i]->name;
+ struct stat st;
+
+ add_path(&rdir, rdir_len, name);
+ if (lstat(rdir.buf, &st))
+ continue;
+
+ if ((symlinks && S_ISLNK(st.st_mode)) || !S_ISREG(st.st_mode))
+ continue;
+
+ if (!indices_loaded) {
+ static struct lock_file lock;
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "%s/wtindex", tmpdir);
+ if (hold_lock_file_for_update(&lock, buf.buf, 0) < 0 ||
+ write_locked_index(&wtindex, &lock, COMMIT_LOCK)) {
+ ret = error("could not write %s", buf.buf);
+ rollback_lock_file(&lock);
+ goto finish;
+ }
+ changed_files(&wt_modified, buf.buf, workdir);
+ strbuf_setlen(&rdir, rdir_len);
+ changed_files(&tmp_modified, buf.buf, rdir.buf);
+ add_path(&rdir, rdir_len, name);
+ indices_loaded = 1;
+ }
+
+ hashmap_entry_init(&dummy, strhash(name));
+ if (hashmap_get(&tmp_modified, &dummy, name)) {
+ add_path(&wtdir, wtdir_len, name);
+ if (hashmap_get(&wt_modified, &dummy, name)) {
+ warning(_("both files modified: '%s' and '%s'."),
+ wtdir.buf, rdir.buf);
+ warning(_("working tree file has been left."));
+ warning("");
+ err = 1;
+ } else if (unlink(wtdir.buf) ||
+ copy_file(wtdir.buf, rdir.buf, st.st_mode))
+ warning_errno(_("could not copy '%s' to '%s'"),
+ rdir.buf, wtdir.buf);
+ }
+ }
+
+ if (err) {
+ warning(_("temporary files exist in '%s'."), tmpdir);
+ warning(_("you may want to cleanup or recover these."));
+ exit(1);
+ } else
+ exit_cleanup(tmpdir, rc);
+
+finish:
+ free(ce);
+ strbuf_release(&ldir);
+ strbuf_release(&rdir);
+ strbuf_release(&wtdir);
+ strbuf_release(&buf);
+
+ return ret;
+}
+
+static int run_file_diff(int prompt, int argc, const char **argv)
+{
+ struct argv_array args = ARGV_ARRAY_INIT;
+ const char *env[] = {
+ "GIT_PAGER=", "GIT_EXTERNAL_DIFF=git-difftool--helper", NULL,
+ NULL
+ };
+ int ret = 0, i;
+
+ if (prompt > 0)
+ env[2] = "GIT_DIFFTOOL_PROMPT=true";
+ else if (!prompt)
+ env[2] = "GIT_DIFFTOOL_NO_PROMPT=true";
+
+ argv_array_push(&args, "diff");
+ for (i = 0; i < argc; i++)
+ argv_array_push(&args, argv[i]);
+ ret = run_command_v_opt_cd_env(args.argv, RUN_GIT_CMD, NULL, env);
+ exit(ret);
+}
+
+int cmd_builtin_difftool(int argc, const char ** argv, const char * prefix)
+{
+ int use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0,
+ tool_help = 0;
+ static char *difftool_cmd = NULL, *extcmd = NULL;
+
+ struct option builtin_difftool_options[] = {
+ OPT_BOOL('g', "gui", &use_gui_tool,
+ N_("use `diff.guitool` instead of `diff.tool`")),
+ OPT_BOOL('d', "dir-diff", &dir_diff,
+ N_("perform a full-directory diff")),
+ { OPTION_SET_INT, 'y', "no-prompt", &prompt, NULL,
+ N_("do not prompt before launching a diff tool"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
+ { OPTION_SET_INT, 0, "prompt", &prompt, NULL, NULL,
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN,
+ NULL, 1 },
+ OPT_BOOL(0, "symlinks", &symlinks,
+ N_("use symlinks in dir-diff mode")),
+ OPT_STRING('t', "tool", &difftool_cmd, N_("<tool>"),
+ N_("use the specified diff tool")),
+ OPT_BOOL(0, "tool-help", &tool_help,
+ N_("print a list of diff tools that may be used with "
+ "`--tool`")),
+ OPT_BOOL(0, "trust-exit-code", &trust_exit_code,
+ N_("make 'git-difftool' exit when an invoked diff "
+ "tool returns a non - zero exit code")),
+ OPT_STRING('x', "extcmd", &extcmd, N_("<command>"),
+ N_("specify a custom command for viewing diffs")),
+ OPT_END()
+ };
+
+ symlinks = has_symlinks;
+
+ git_config(difftool_config, NULL);
+
+ argc = parse_options(argc, argv, prefix, builtin_difftool_options,
+ builtin_difftool_usage, PARSE_OPT_KEEP_UNKNOWN |
+ PARSE_OPT_KEEP_DASHDASH);
+
+ if (tool_help)
+ return print_tool_help();
+
+ if (use_gui_tool && diff_gui_tool && *diff_gui_tool)
+ setenv("GIT_DIFF_TOOL", diff_gui_tool, 1);
+ else if (difftool_cmd) {
+ if (*difftool_cmd)
+ setenv("GIT_DIFF_TOOL", difftool_cmd, 1);
+ else
+ die(_("no <tool> given for --tool=<tool>"));
+ }
+
+ if (extcmd) {
+ if (*extcmd)
+ setenv("GIT_DIFFTOOL_EXTCMD", extcmd, 1);
+ else
+ die(_("no <cmd> given for --extcmd=<cmd>"));
+ }
+
+ setenv("GIT_DIFFTOOL_TRUST_EXIT_CODE",
+ trust_exit_code ? "true" : "false", 1);
+
+ /*
+ * In directory diff mode, 'git-difftool--helper' is called once
+ * to compare the a / b directories.In file diff mode, 'git diff'
+ * will invoke a separate instance of 'git-difftool--helper' for
+ * each file that changed.
+ */
+ if (dir_diff)
+ return run_dir_diff(extcmd, symlinks, argc, argv);
+ return run_file_diff(prompt, argc, argv);
+}
diff --git a/git.c b/git.c
index efa1059..eaa0f67 100644
--- a/git.c
+++ b/git.c
@@ -424,6 +424,7 @@ static struct cmd_struct commands[] = {
{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
{ "diff-index", cmd_diff_index, RUN_SETUP },
{ "diff-tree", cmd_diff_tree, RUN_SETUP },
+ { "builtin-difftool", cmd_builtin_difftool, RUN_SETUP | NEED_WORK_TREE },
{ "fast-export", cmd_fast_export, RUN_SETUP },
{ "fetch", cmd_fetch, RUN_SETUP },
{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
--
2.10.1.583.g721a9e0
^ permalink raw reply related
* [PATCH 2/2] difftool: add a feature flag for the builtin vs scripted version
From: Johannes Schindelin @ 2016-11-22 17:01 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1479834051.git.johannes.schindelin@gmx.de>
The popular difftool command was just converted into a builtin, for
better performance on Windows as well as to reduce the number of Perl
scripts (so that we may, in the very long run, be able to ship Git for
Windows without any Perl interpreter at all).
However, it would be sloppy practice to simply switch over from the
tried-and-tested (albeit slow) scripted version to the spanking new
builtin version that has not seen a whole lot of real-world testing.
So let's add a feature flag.
If the file `use-builtin-difftool` exists in Git's exec path, Git will
now automagically use the builtin version of the difftool, without
requiring the user to call `git builtin-difftool <args>`. This comes in
particularly handy when the difftool command is used from within
scripts.
If the file `use-builtin-difftool` is absent from Git's exec path, which
is the default, Git will use the scripted version as before.
The original idea was to use an environment variable
GIT_USE_BUILTIN_DIFFTOOL, but the test suite resets those variables, and
we do want to use that feature flag to run the tests with, and without,
the feature flag.
Besides, the plan is to add an opt-in flag in Git for Windows'
installer. If we implemented the feature flag as an environment
variable, we would have to modify the user's environment, in order to
make the builtin difftool the default when called from Git Bash, Git CMD
or third-party tools.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.gitignore | 1 +
git-difftool.perl | 7 +++++++
git.c | 20 ++++++++++++++++++++
3 files changed, 28 insertions(+)
diff --git a/.gitignore b/.gitignore
index 4f54531..91bfd09 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,4 @@
+/use-builtin-difftool
/GIT-BUILD-OPTIONS
/GIT-CFLAGS
/GIT-LDFLAGS
diff --git a/git-difftool.perl b/git-difftool.perl
index a5790d0..28e47d8 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -23,6 +23,13 @@ use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
+if (-e Git::exec_path() . '/use-builtin-difftool') {
+ unshift(@ARGV, "builtin-difftool");
+ unshift(@ARGV, "git");
+ exec(@ARGV);
+ die("Could not execute builtin difftool");
+}
+
sub usage
{
my $exitcode = shift;
diff --git a/git.c b/git.c
index eaa0f67..7a0df7a 100644
--- a/git.c
+++ b/git.c
@@ -2,6 +2,7 @@
#include "exec_cmd.h"
#include "help.h"
#include "run-command.h"
+#include "dir.h"
const char git_usage_string[] =
"git [--version] [--help] [-C <path>] [-c name=value]\n"
@@ -542,6 +543,22 @@ static void strip_extension(const char **argv)
#define strip_extension(cmd)
#endif
+static int use_builtin_difftool(void)
+{
+ static int initialized, use;
+
+ if (!initialized) {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(&buf, "%s/%s", git_exec_path(),
+ "use-builtin-difftool");
+ use = file_exists(buf.buf);
+ strbuf_release(&buf);
+ initialized = 1;
+ }
+
+ return use;
+}
+
static void handle_builtin(int argc, const char **argv)
{
struct argv_array args = ARGV_ARRAY_INIT;
@@ -551,6 +568,9 @@ static void handle_builtin(int argc, const char **argv)
strip_extension(argv);
cmd = argv[0];
+ if (!strcmp("difftool", cmd) && use_builtin_difftool())
+ cmd = "builtin-difftool";
+
/* Turn "git cmd --help" into "git help --exclude-guides cmd" */
if (argc > 1 && !strcmp(argv[1], "--help")) {
int i;
--
2.10.1.583.g721a9e0
^ permalink raw reply related
* Re: [PATCH 2/3] stripspace: respect repository config
From: Junio C Hamano @ 2016-11-22 17:10 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Duy Nguyen, Git Mailing List, Ralf Thielow, Taufiq Hoven
In-Reply-To: <alpine.DEB.2.20.1611221712480.3746@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> This conditional config file reading is a trap for similar bugs to
>> happen again. Is there any reason we should not just mark the command
>> RUN_SETUP_GENTLY in git.c and call git_config() here unconditionally?
>
> As I plan to slip these patches into Git for Windows v2.11.0, i.e. making
> this a last-minute hot fix, I want to err on the side of caution.
So do I. As a hot-fix, I'd prefer the patch I queued yesterday.
I think we want to audit the ones without RUN_SETUP* in the command
table in order to hunt for regression aka "a fix revealed a bug that
was covered by .git/config accidentally getting read when run from
the top-level of the working tree", though. We may find unreported
breakages that we may have to fix.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox