* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Jacob Keller @ 2016-12-23 22:23 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Michael Haggerty, Jacob Keller, Git mailing list,
Norbert Kiesel
In-Reply-To: <20161223222145.vkf6mjvs5t7ag3od@sigill.intra.peff.net>
On Fri, Dec 23, 2016 at 2:21 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Dec 23, 2016 at 01:17:03PM -0800, Junio C Hamano wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > I guess both you and Michael are in favor of just removing compaction
>> > variant without any renames, so let me prepare a reroll and queue
>> > that instead. We can flip the default perhaps one release later.
>>
>> -- >8 --
>> Subject: [PATCH] diff: retire "compaction" heuristics
>
> Looks good to me from a cursory read.
>
> Thanks.
>
> -Peff
Same. This is more obviously correct since we didn't have to change a
bunch of references to INDENT_HEURISTIC. I agree that the name does
not make sense now, but if our goal is to make it default with a
disable option, I think that we shouldn't worry too much about the
naming.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Jeff King @ 2016-12-23 22:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jacob Keller, Michael Haggerty, Jacob Keller, Git mailing list,
Norbert Kiesel
In-Reply-To: <xmqq8tr6e46o.fsf@gitster.mtv.corp.google.com>
On Fri, Dec 23, 2016 at 01:17:03PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > I guess both you and Michael are in favor of just removing compaction
> > variant without any renames, so let me prepare a reroll and queue
> > that instead. We can flip the default perhaps one release later.
>
> -- >8 --
> Subject: [PATCH] diff: retire "compaction" heuristics
Looks good to me from a cursory read.
Thanks.
-Peff
^ permalink raw reply
* Re: [PATCH v2 2/2] repack: die on incremental + write-bitmap-index
From: Jeff King @ 2016-12-23 22:20 UTC (permalink / raw)
To: David Turner; +Cc: git, novalis
In-Reply-To: <1482522215-13401-2-git-send-email-dturner@twosigma.com>
On Fri, Dec 23, 2016 at 02:43:35PM -0500, David Turner wrote:
> The bitmap index only works for single packs, so requesting an
> incremental repack with bitmap indexes makes no sense.
OK. I doubt this will come up much after the git-gc change from the
first patch. And it should already be printing a warning in this case
anyway[1]. But I do not mind turning the warning into a hard failure; it
may make things more obvious for the user. The only weird thing is that
the bitmap option may come from the config, so this effectively means
you can never run "git repack -d" in a repo with bitmap config. I'm not
sure if anybody cares or not, with the new git-gc patch.
[1] Technically "repack -d" could actually create bitmaps (with no
warning) in a repo that has no existing packs. But that's probably
an uninteresting corner case (the user should say "-ad" there, and
the "-a" ends up being a noop).
> @@ -206,6 +212,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> if (pack_kept_objects < 0)
> pack_kept_objects = write_bitmaps;
>
> + if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
> + die(incremental_bitmap_conflict_error);
I double-checked that ALL_INTO_ONE covers both -A and -a, which I think
should be sufficient here.
> -test_expect_success 'incremental repack cannot create bitmaps' '
> +test_expect_success 'incremental repack fails when bitmaps are requested' '
> test_commit more-1 &&
> find .git/objects/pack -name "*.bitmap" >expect &&
> - git repack -d &&
> + test_must_fail git repack -d 2>err &&
> + test_i18ngrep "Incremental repacks are incompatible with bitmap" err &&
> find .git/objects/pack -name "*.bitmap" >actual &&
> test_cmp expect actual
> '
The final `find` is probably overkill now after we know the command
failed and reported the error. But it does not hurt to be thorough. :)
-Peff
^ permalink raw reply
* Re: [PATCH v2 1/2] auto gc: don't write bitmaps for incremental repacks
From: Jeff King @ 2016-12-23 22:12 UTC (permalink / raw)
To: David Turner; +Cc: git, novalis
In-Reply-To: <1482522215-13401-1-git-send-email-dturner@twosigma.com>
On Fri, Dec 23, 2016 at 02:43:34PM -0500, David Turner wrote:
> When git gc --auto does an incremental repack of loose objects, we do
> not expect to be able to write a bitmap; it is very likely that
> objects in the new pack will have references to objects outside of the
> pack. So we shouldn't try to write a bitmap, because doing so will
> likely issue a warning.
Makes sense. Another reason is that the bitmap-reading code only handles
a single bitmap. So it makes sense only to generate one during the
all-in-one repack. I don't know if that is worth mentioning.
> Signed-off-by: David Turner <dturner@twosigma.com>
> Signed-off-by: Jeff King <peff@peff.net>
I don't remember if I signed off the original, but just for the record,
my signoff here is valid.
-Peff
^ permalink raw reply
* Re: [PATCH v2 1/2] auto gc: don't write bitmaps for incremental repacks
From: Junio C Hamano @ 2016-12-23 21:27 UTC (permalink / raw)
To: David Turner; +Cc: git, peff, novalis
In-Reply-To: <1482522215-13401-1-git-send-email-dturner@twosigma.com>
David Turner <dturner@twosigma.com> writes:
> +test_expect_success 'auto gc with too many loose objects does not attempt to create bitmaps' '
> + test_config gc.auto 3 &&
> + test_config gc.autodetach false &&
> + test_config pack.writebitmaps true &&
> + # We need to create two object whose sha1s start with 17
> + # since this is what git gc counts. As it happens, these
> + # two blobs will do so.
> + test_commit 263 &&
> + test_commit 410 &&
> + # Our first gc will create a pack; our second will create a second pack
> + git gc --auto &&
> + ls .git/objects/pack |grep -v bitmap >existing_packs &&
Missing SP (compare with the second invocation of the same).
> + test_commit 523 &&
> + test_commit 790 &&
> +
> + git gc --auto 2>err &&
> + test_i18ngrep ! "^warning:" err &&
> + ls .git/objects/pack/ | grep -v bitmap >post_packs &&
> + ! test_cmp existing_packs post_packs
This does not look good for two reasons. test_cmp tries to
highlight test breakage by being verbose when two files are
different while keeping quiet when they are the same. Running it
with "!" does not change its expectation. This undesirable effect
should be visible when this test is run with "-v" option.
Another is that this only tests if the set of packs before and after
are different. I think you are expecting that the second invocation
will create a new one while leaving the old one intact but this test
will not catch a breakage if the second repack instead created just
one pack replacing the old one.
> +'
> +
> +
> test_done
Thanks.
^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Junio C Hamano @ 2016-12-23 21:17 UTC (permalink / raw)
To: Jeff King
Cc: Jacob Keller, Michael Haggerty, Jacob Keller, Git mailing list,
Norbert Kiesel
In-Reply-To: <xmqqh95uedzu.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> I guess both you and Michael are in favor of just removing compaction
> variant without any renames, so let me prepare a reroll and queue
> that instead. We can flip the default perhaps one release later.
-- >8 --
Subject: [PATCH] diff: retire "compaction" heuristics
When a patch inserts a block of lines, whose last lines are the
same as the existing lines that appear before the inserted block,
"git diff" can choose any place between these existing lines as the
boundary between the pre-context and the added lines (adjusting the
end of the inserted block as appropriate) to come up with variants
of the same patch, and some variants are easier to read than others.
We have been trying to improve the choice of this boundary, and Git
2.11 shipped with an experimental "compaction-heuristic". Since
then another attempt to improve the logic further resulted in a new
"indent-heuristic" logic. It is agreed that the latter gives better
result overall, and the former outlived its usefulness.
Retire "compaction", and keep "indent" as an experimental feature.
The latter hopefully will be turned on by default in a future
release, but that should be done as a separate step.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/diff-config.txt | 6 ++----
Documentation/diff-heuristic-options.txt | 2 --
builtin/blame.c | 5 ++---
diff.c | 23 +++-------------------
git-add--interactive.perl | 3 ---
xdiff/xdiff.h | 3 +--
xdiff/xdiffi.c | 33 --------------------------------
7 files changed, 8 insertions(+), 67 deletions(-)
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 58f4bd6afa..d8570f2a75 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -172,10 +172,8 @@ diff.tool::
include::mergetools-diff.txt[]
diff.indentHeuristic::
-diff.compactionHeuristic::
- Set one of these options to `true` to enable one of two
- experimental heuristics that shift diff hunk boundaries to
- make patches easier to read.
+ Set this option to `true` to enable experimental heuristics
+ that shift diff hunk boundaries to make patches easier to read.
diff.algorithm::
Choose a diff algorithm. The variants are as follows:
diff --git a/Documentation/diff-heuristic-options.txt b/Documentation/diff-heuristic-options.txt
index 36cb549df9..d4f3d95505 100644
--- a/Documentation/diff-heuristic-options.txt
+++ b/Documentation/diff-heuristic-options.txt
@@ -1,7 +1,5 @@
--indent-heuristic::
--no-indent-heuristic::
---compaction-heuristic::
---no-compaction-heuristic::
These are to help debugging and tuning experimental heuristics
(which are off by default) that shift diff hunk boundaries to
make patches easier to read.
diff --git a/builtin/blame.c b/builtin/blame.c
index 4ddfadb71f..ab54a5c1f4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2596,8 +2596,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
* and are only included here to get included in the "-h"
* output:
*/
- { OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental indent-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
- { OPTION_LOWLEVEL_CALLBACK, 0, "compaction-heuristic", NULL, NULL, N_("Use an experimental blank-line-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
+ { OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find better match"), XDF_NEED_MINIMAL),
OPT_STRING('S', NULL, &revs_file, N_("file"), N_("Use revisions from <file> instead of calling git-rev-list")),
@@ -2645,7 +2644,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
}
parse_done:
no_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);
- xdl_opts |= revs.diffopt.xdl_opts & (XDF_COMPACTION_HEURISTIC | XDF_INDENT_HEURISTIC);
+ xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;
DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
argc = parse_options_end(&ctx);
diff --git a/diff.c b/diff.c
index 8981477c43..741ce8c68d 100644
--- a/diff.c
+++ b/diff.c
@@ -28,7 +28,6 @@
static int diff_detect_rename_default;
static int diff_indent_heuristic; /* experimental */
-static int diff_compaction_heuristic; /* experimental */
static int diff_rename_limit_default = 400;
static int diff_suppress_blank_empty;
static int diff_use_color_default = -1;
@@ -223,16 +222,8 @@ void init_diff_ui_defaults(void)
int git_diff_heuristic_config(const char *var, const char *value, void *cb)
{
- if (!strcmp(var, "diff.indentheuristic")) {
+ if (!strcmp(var, "diff.indentheuristic"))
diff_indent_heuristic = git_config_bool(var, value);
- if (diff_indent_heuristic)
- diff_compaction_heuristic = 0;
- }
- if (!strcmp(var, "diff.compactionheuristic")) {
- diff_compaction_heuristic = git_config_bool(var, value);
- if (diff_compaction_heuristic)
- diff_indent_heuristic = 0;
- }
return 0;
}
@@ -3380,8 +3371,6 @@ void diff_setup(struct diff_options *options)
options->xdl_opts |= diff_algorithm;
if (diff_indent_heuristic)
DIFF_XDL_SET(options, INDENT_HEURISTIC);
- else if (diff_compaction_heuristic)
- DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
options->orderfile = diff_order_file_cfg;
@@ -3876,16 +3865,10 @@ int diff_opt_parse(struct diff_options *options,
DIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL);
else if (!strcmp(arg, "--ignore-blank-lines"))
DIFF_XDL_SET(options, IGNORE_BLANK_LINES);
- else if (!strcmp(arg, "--indent-heuristic")) {
+ else if (!strcmp(arg, "--indent-heuristic"))
DIFF_XDL_SET(options, INDENT_HEURISTIC);
- DIFF_XDL_CLR(options, COMPACTION_HEURISTIC);
- } else if (!strcmp(arg, "--no-indent-heuristic"))
- DIFF_XDL_CLR(options, INDENT_HEURISTIC);
- else if (!strcmp(arg, "--compaction-heuristic")) {
- DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
+ else if (!strcmp(arg, "--no-indent-heuristic"))
DIFF_XDL_CLR(options, INDENT_HEURISTIC);
- } else if (!strcmp(arg, "--no-compaction-heuristic"))
- DIFF_XDL_CLR(options, COMPACTION_HEURISTIC);
else if (!strcmp(arg, "--patience"))
options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
else if (!strcmp(arg, "--histogram"))
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index ee3d812695..5a55d80b9d 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -46,7 +46,6 @@
my $diff_algorithm = $repo->config('diff.algorithm');
my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');
-my $diff_compaction_heuristic = $repo->config_bool('diff.compactionheuristic');
my $diff_filter = $repo->config('interactive.difffilter');
my $use_readkey = 0;
@@ -753,8 +752,6 @@ sub parse_diff {
}
if ($diff_indent_heuristic) {
splice @diff_cmd, 1, 0, "--indent-heuristic";
- } elsif ($diff_compaction_heuristic) {
- splice @diff_cmd, 1, 0, "--compaction-heuristic";
}
if (defined $patch_mode_revision) {
push @diff_cmd, get_diff_reference($patch_mode_revision);
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 8db16d4ae6..b090ad8eac 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -41,8 +41,7 @@ extern "C" {
#define XDF_IGNORE_BLANK_LINES (1 << 7)
-#define XDF_COMPACTION_HEURISTIC (1 << 8)
-#define XDF_INDENT_HEURISTIC (1 << 9)
+#define XDF_INDENT_HEURISTIC (1 << 8)
#define XDL_EMIT_FUNCNAMES (1 << 0)
#define XDL_EMIT_FUNCCONTEXT (1 << 2)
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 760fbb6db7..93a65680a1 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -400,11 +400,6 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,
}
-static int is_blank_line(xrecord_t *rec, long flags)
-{
- return xdl_blankline(rec->ptr, rec->size, flags);
-}
-
static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags)
{
return (rec1->ha == rec2->ha &&
@@ -821,7 +816,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
struct xdlgroup g, go;
long earliest_end, end_matching_other;
long groupsize;
- unsigned int blank_lines;
group_init(xdf, &g);
group_init(xdfo, &go);
@@ -846,13 +840,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
*/
end_matching_other = -1;
- /*
- * Boolean value that records whether there are any blank
- * lines that could be made to be the last line of this
- * group.
- */
- blank_lines = 0;
-
/* Shift the group backward as much as possible: */
while (!group_slide_up(xdf, &g, flags))
if (group_previous(xdfo, &go))
@@ -869,11 +856,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
/* Now shift the group forward as far as possible: */
while (1) {
- if (!blank_lines)
- blank_lines = is_blank_line(
- xdf->recs[g.end - 1],
- flags);
-
if (group_slide_down(xdf, &g, flags))
break;
if (group_next(xdfo, &go))
@@ -906,21 +888,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
if (group_previous(xdfo, &go))
xdl_bug("group sync broken sliding to match");
}
- } else if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) {
- /*
- * Compaction heuristic: if it is possible to shift the
- * group to make its bottom line a blank line, do so.
- *
- * As we already shifted the group forward as far as
- * possible in the earlier loop, we only need to handle
- * backward shifts, not forward ones.
- */
- while (!is_blank_line(xdf->recs[g.end - 1], flags)) {
- if (group_slide_up(xdf, &g, flags))
- xdl_bug("blank line disappeared");
- if (group_previous(xdfo, &go))
- xdl_bug("group sync broken sliding to blank line");
- }
} else if (flags & XDF_INDENT_HEURISTIC) {
/*
* Indent heuristic: a group of pure add/delete lines
--
2.11.0-448-g09546ed716
^ permalink raw reply related
* [PATCH v2 1/2] auto gc: don't write bitmaps for incremental repacks
From: David Turner @ 2016-12-23 19:43 UTC (permalink / raw)
To: git, peff, novalis; +Cc: David Turner
When git gc --auto does an incremental repack of loose objects, we do
not expect to be able to write a bitmap; it is very likely that
objects in the new pack will have references to objects outside of the
pack. So we shouldn't try to write a bitmap, because doing so will
likely issue a warning.
This warning was making its way into gc.log. When the gc.log was
present, future auto gc runs would refuse to run.
Patch by Jeff King.
Bug report, test, and commit message by David Turner.
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/gc.c | 9 ++++++++-
t/t6500-gc.sh | 22 ++++++++++++++++++++++
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 069950d..331f219 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -191,6 +191,11 @@ static void add_repack_all_option(void)
}
}
+static void add_repack_incremental_option(void)
+{
+ argv_array_push(&repack, "--no-write-bitmap-index");
+}
+
static int need_to_gc(void)
{
/*
@@ -208,7 +213,9 @@ static int need_to_gc(void)
*/
if (too_many_packs())
add_repack_all_option();
- else if (!too_many_loose_objects())
+ else if (too_many_loose_objects())
+ add_repack_incremental_option();
+ else
return 0;
if (run_hook_le(NULL, "pre-auto-gc", NULL))
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 5d7d414..b83a08c 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -43,4 +43,26 @@ test_expect_success 'gc is not aborted due to a stale symref' '
)
'
+test_expect_success 'auto gc with too many loose objects does not attempt to create bitmaps' '
+ test_config gc.auto 3 &&
+ test_config gc.autodetach false &&
+ test_config pack.writebitmaps true &&
+ # We need to create two object whose sha1s start with 17
+ # since this is what git gc counts. As it happens, these
+ # two blobs will do so.
+ test_commit 263 &&
+ test_commit 410 &&
+ # Our first gc will create a pack; our second will create a second pack
+ git gc --auto &&
+ ls .git/objects/pack |grep -v bitmap >existing_packs &&
+ test_commit 523 &&
+ test_commit 790 &&
+
+ git gc --auto 2>err &&
+ test_i18ngrep ! "^warning:" err &&
+ ls .git/objects/pack/ | grep -v bitmap >post_packs &&
+ ! test_cmp existing_packs post_packs
+'
+
+
test_done
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply related
* [PATCH v2 2/2] repack: die on incremental + write-bitmap-index
From: David Turner @ 2016-12-23 19:43 UTC (permalink / raw)
To: git, peff, novalis; +Cc: David Turner
In-Reply-To: <1482522215-13401-1-git-send-email-dturner@twosigma.com>
The bitmap index only works for single packs, so requesting an
incremental repack with bitmap indexes makes no sense.
Signed-off-by: David Turner <dturner@twosigma.com>
---
builtin/repack.c | 9 +++++++++
t/t5310-pack-bitmaps.sh | 5 +++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 80dd06b..9c3dd09 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -18,6 +18,12 @@ static const char *const git_repack_usage[] = {
NULL
};
+static const char incremental_bitmap_conflict_error[] = N_(
+"Incremental repacks are incompatible with bitmap indexes. Use \n"
+"--no-write-bitmap-index or disable the pack.writebitmaps configuration."
+);
+
+
static int repack_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "repack.usedeltabaseoffset")) {
@@ -206,6 +212,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
if (pack_kept_objects < 0)
pack_kept_objects = write_bitmaps;
+ if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
+ die(incremental_bitmap_conflict_error);
+
packdir = mkpathdup("%s/pack", get_object_directory());
packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid());
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index b4c7a6f..e9a2771 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -118,10 +118,11 @@ test_expect_success 'fetch (partial bitmap)' '
test_cmp expect actual
'
-test_expect_success 'incremental repack cannot create bitmaps' '
+test_expect_success 'incremental repack fails when bitmaps are requested' '
test_commit more-1 &&
find .git/objects/pack -name "*.bitmap" >expect &&
- git repack -d &&
+ test_must_fail git repack -d 2>err &&
+ test_i18ngrep "Incremental repacks are incompatible with bitmap" err &&
find .git/objects/pack -name "*.bitmap" >actual &&
test_cmp expect actual
'
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply related
* Re: [PATCH] mingw: add a regression test for pushing to UNC paths
From: Johannes Sixt @ 2016-12-23 18:04 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <9fb8a9f405b19db087379ea5bbad80c3fbac8e4e.1482513055.git.johannes.schindelin@gmx.de>
Am 23.12.2016 um 18:11 schrieb Johannes Schindelin:
> Let's make sure that it does not regress again, by introducing a test
> that uses so-called "administrative shares": disk volumes are
> automatically shared under certain circumstances, e.g. the C: drive is
> shared as \\localhost\c$.
Clever!
> +test_expect_success setup '
> + test_commit initial
> +'
> +
> +test_expect_success clone '
> + git clone "file://$UNCPATH" clone
> +'
> +
> +test_expect_success push '
> + (
> + cd clone &&
> + git checkout -b to-push &&
> + test_commit to-push &&
> + git push origin HEAD
> + )
> +'
> +
> +test_done
Wouldn't at a minimum
test_expect_success 'check push result' '
git rev-parse to-push
'
be a good idea to make sure that the pushed commit actually arrived?
-- Hannes
^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Junio C Hamano @ 2016-12-23 17:45 UTC (permalink / raw)
To: Jeff King
Cc: Jacob Keller, Michael Haggerty, Jacob Keller, Git mailing list,
Norbert Kiesel
In-Reply-To: <20161223161917.4a352c2wzerj5uyz@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Dec 23, 2016 at 12:12:13AM -0800, Jacob Keller wrote:
>
>> I actually would prefer that we just say "this is the default now" and
>> provide some knob "no-indent-heuristic" or "no-compaction-heuristic"
>> and go with that, I think, since I am pretty sure we're all in
>> agreement that the heuristic is an improvement in almost every case,
>> certainly all the ones we've found. It's at least not worse in any
>> case I've seen, and is usually better.
>>
>> Thoughts? I don't have a super strong opinion about which name we went
>> with for the knob.
>
> Yes, I think we should also make --indent-heuristic the default. That's
> technically orthogonal to the name, but I agree the name becomes a lot
> less important when it is just on by default.
Yes, I agree that will be the endgame, but one step at the time, and
also removing one of them is orthogonal that we would want to do
sooner rather than later. So the step represented by the patch
under discussion is the first one among the two towards the final
state.
I guess both you and Michael are in favor of just removing compaction
variant without any renames, so let me prepare a reroll and queue
that instead. We can flip the default perhaps one release later.
Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #07; Thu, 22)
From: Junio C Hamano @ 2016-12-23 17:46 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8CEKj6Lbn++NHhyB7J8HBrMW4F37SHi2soCH1z=RJz4GQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Fri, Dec 23, 2016 at 5:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Will merge to 'next'.
>> - nd/config-misc-fixes 12-22 #3
>
> Hold it. You made a comment about rollback lockfile on uninitialized
> variable or something but I haven't time to really look at it yet.
The fix for it is squashed in to the version queued. Please double
check by fetching from me and comparing it with what you sent out.
Thanks.
^ permalink raw reply
* [PATCH] mingw: add a regression test for pushing to UNC paths
From: Johannes Schindelin @ 2016-12-23 17:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
On Windows, there are "UNC paths" to access network (AKA shared)
folders, of the form \\server\sharename\directory. This provides a
convenient way for Windows developers to share their Git repositories
without having to have a dedicated server.
Git for Windows v2.11.0 introduced a regression where pushing to said
UNC paths no longer works, although fetching and cloning still does, as
reported here: https://github.com/git-for-windows/git/issues/979
This regression was fixed in 7814fbe3f1 (normalize_path_copy(): fix
pushing to //server/share/dir on Windows, 2016-12-14).
Let's make sure that it does not regress again, by introducing a test
that uses so-called "administrative shares": disk volumes are
automatically shared under certain circumstances, e.g. the C: drive is
shared as \\localhost\c$. The test needs to be skipped if the current
directory is inaccessible via said administrative share, of course.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/unc-paths-test-v1
Fetch-It-Via: git fetch https://github.com/dscho/git unc-paths-test-v1
t/t5580-clone-push-unc.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
create mode 100755 t/t5580-clone-push-unc.sh
diff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh
new file mode 100755
index 0000000000..e06d230724
--- /dev/null
+++ b/t/t5580-clone-push-unc.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='various UNC path tests (Windows-only)'
+. ./test-lib.sh
+
+if ! test_have_prereq MINGW; then
+ skip_all='skipping UNC path tests, requires Windows'
+ test_done
+fi
+
+UNCPATH="$(pwd)"
+case "$UNCPATH" in
+[A-Z]:*)
+ # Use administrative share e.g. \\localhost\C$\git-sdk-64\usr\src\git
+ # (we use forward slashes here because MSYS2 and Git accept them, and
+ # they are easier on the eyes)
+ UNCPATH="//localhost/${UNCPATH%%:*}\$/${UNCPATH#?:}"
+ test -d "$UNCPATH" || {
+ skip_all='could not access administrative share; skipping'
+ test_done
+ }
+ ;;
+*)
+ skip_all='skipping UNC path tests, cannot determine current path as UNC'
+ test_done
+ ;;
+esac
+
+test_expect_success setup '
+ test_commit initial
+'
+
+test_expect_success clone '
+ git clone "file://$UNCPATH" clone
+'
+
+test_expect_success push '
+ (
+ cd clone &&
+ git checkout -b to-push &&
+ test_commit to-push &&
+ git push origin HEAD
+ )
+'
+
+test_done
base-commit: 1ede815b8d1562f46b7aa5d55af084a3cad2ecf8
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* Re: git 2.11.0 error when pushing to remote located on a windows share
From: Johannes Schindelin @ 2016-12-23 17:04 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: thomas.attwood, peff, git
In-Reply-To: <91983cb6-eed8-987d-bdda-c0fe55a9d139@web.de>
[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]
Hi,
On Tue, 6 Dec 2016, Torsten Bögershausen wrote:
> On 2016-12-05 12:05, thomas.attwood@stfc.ac.uk wrote:
> > On Sun, 4 Dec 2016 08:09:14 +0000, Torsten Bögershausen wrote:
> >> There seems to be another issue, which may or may not being related:
> >> https://github.com/git-for-windows/git/issues/979
> >
> > I think this is the same issue. I've posted my trace command output there as
> > It might be more appropriate:
> > https://github.com/git-for-windows/git/issues/979#issuecomment-264816175
> >
> Thanks for the trace.
> I think that the problem comes from the "cwd", when a UNC name is used.
>
> cd //SERVER/share/somedir
> does not work under Windows, the is no chance to change into that directory.
> Does anybody know out of his head why and since when we change the directory
> like this ?
> Or "git bisect" may help.
For the record, Hannes Sixt came up with a patch that fixes #979, and
which already made it into Git for Windows' `master`.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Jeff King @ 2016-12-23 16:19 UTC (permalink / raw)
To: Jacob Keller
Cc: Junio C Hamano, Michael Haggerty, Jacob Keller, Git mailing list,
Norbert Kiesel
In-Reply-To: <CA+P7+xrWsCkABzpSkYJ4fb2_JijmUx=Sf4Hgsr6Z+k=_GogE_Q@mail.gmail.com>
On Fri, Dec 23, 2016 at 12:12:13AM -0800, Jacob Keller wrote:
> I actually would prefer that we just say "this is the default now" and
> provide some knob "no-indent-heuristic" or "no-compaction-heuristic"
> and go with that, I think, since I am pretty sure we're all in
> agreement that the heuristic is an improvement in almost every case,
> certainly all the ones we've found. It's at least not worse in any
> case I've seen, and is usually better.
>
> Thoughts? I don't have a super strong opinion about which name we went
> with for the knob.
Yes, I think we should also make --indent-heuristic the default. That's
technically orthogonal to the name, but I agree the name becomes a lot
less important when it is just on by default.
-Peff
^ permalink raw reply
* Re: [PATCH v2 1/3] mingw: adjust is_console() to work with stdin
From: Beat Bolli @ 2016-12-23 12:51 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Pranit Bauva, Johannes Sixt
In-Reply-To: <alpine.DEB.2.20.1612231024400.155951@virtualbox>
Hi Dscho
On 2016-12-23 10:30, Johannes Schindelin wrote:
> Hi Beat,
>
> On Fri, 23 Dec 2016, Beat Bolli wrote:
>
>> On 22.12.16 18:08, Johannes Schindelin wrote:
>> > diff --git a/compat/winansi.c b/compat/winansi.c
>> > index cb725fb02f..590d61cb1b 100644
>> > --- a/compat/winansi.c
>> > +++ b/compat/winansi.c
>> > @@ -84,6 +84,7 @@ static void warn_if_raster_font(void)
>> > static int is_console(int fd)
>> > {
>> > CONSOLE_SCREEN_BUFFER_INFO sbi;
>> > + DWORD mode;
>>
>> Nit: can we move this definition into the block below where it's used?
>>
>> > HANDLE hcon;
>> >
>> > static int initialized = 0;
>> > @@ -98,7 +99,10 @@ static int is_console(int fd)
>> > return 0;
>> >
>> > /* check if its a handle to a console output screen buffer */
>> > - if (!GetConsoleScreenBufferInfo(hcon, &sbi))
>> > + if (!fd) {
>>
>> Right here:
>> + DWORD mode;
>
> By that reasoning, the CONSOLE_SCREEN_BUFFER_INFO declaration that has
> function-wide scope should also move below:
>
>> > + if (!GetConsoleMode(hcon, &mode))
>> > + return 0;
>
> Right here.
>
>> > + } else if (!GetConsoleScreenBufferInfo(hcon, &sbi))
>> > return 0;
>> >
>> > /* initialize attributes */
>
> As the existing code followed a different convention, so does my patch.
>
> If you choose to submit a change that moved the `mode` declaration to
> narrow its scope, please also move the `sbi` declaration for
> consistency.
It's probably not worth it. It just jumped at me when reading the patch,
and, writing much C++ recently, it looked weird to have a definition so
far away from the single use of the variable.
Cheers,
Beat
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #07; Thu, 22)
From: Duy Nguyen @ 2016-12-23 10:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqzijnehgb.fsf@gitster.mtv.corp.google.com>
On Fri, Dec 23, 2016 at 5:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Will merge to 'next'.
> - nd/config-misc-fixes 12-22 #3
Hold it. You made a comment about rollback lockfile on uninitialized
variable or something but I haven't time to really look at it yet.
--
Duy
^ permalink raw reply
* Re: [PATCH v2 1/3] mingw: adjust is_console() to work with stdin
From: Johannes Schindelin @ 2016-12-23 9:30 UTC (permalink / raw)
To: Beat Bolli; +Cc: git, Junio C Hamano, Pranit Bauva, Johannes Sixt
In-Reply-To: <fc3e0d9c-86ea-4a62-6b70-b9cdd67f581a@drbeat.li>
Hi Beat,
On Fri, 23 Dec 2016, Beat Bolli wrote:
> On 22.12.16 18:08, Johannes Schindelin wrote:
> > diff --git a/compat/winansi.c b/compat/winansi.c
> > index cb725fb02f..590d61cb1b 100644
> > --- a/compat/winansi.c
> > +++ b/compat/winansi.c
> > @@ -84,6 +84,7 @@ static void warn_if_raster_font(void)
> > static int is_console(int fd)
> > {
> > CONSOLE_SCREEN_BUFFER_INFO sbi;
> > + DWORD mode;
>
> Nit: can we move this definition into the block below where it's used?
>
> > HANDLE hcon;
> >
> > static int initialized = 0;
> > @@ -98,7 +99,10 @@ static int is_console(int fd)
> > return 0;
> >
> > /* check if its a handle to a console output screen buffer */
> > - if (!GetConsoleScreenBufferInfo(hcon, &sbi))
> > + if (!fd) {
>
> Right here:
> + DWORD mode;
By that reasoning, the CONSOLE_SCREEN_BUFFER_INFO declaration that has
function-wide scope should also move below:
> > + if (!GetConsoleMode(hcon, &mode))
> > + return 0;
Right here.
> > + } else if (!GetConsoleScreenBufferInfo(hcon, &sbi))
> > return 0;
> >
> > /* initialize attributes */
As the existing code followed a different convention, so does my patch.
If you choose to submit a change that moved the `mode` declaration to
narrow its scope, please also move the `sbi` declaration for consistency.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC/PATCH] add diffstat information to rebase
From: Jacob Keller @ 2016-12-23 8:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Git mailing list
In-Reply-To: <xmqqtw9vegjr.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 22, 2016 at 2:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> $ git rebase -i HEAD^^
>>
>> pick 2eaa3f532c Third batch for 2.12
>> # Documentation/RelNotes/2.12.0.txt | 40 +++++++++++++++++++++++++++++++++++++++
>> # 1 file changed, 40 insertions(+)
>> pick 3170a3a57b add information to rebase
>> # git-rebase--interactive.sh | 2 ++
>> # 1 file changed, 2 insertions(+)
>>
>> # Rebase 2eaa3f532c..3170a3a57b onto 2eaa3f532c (1 command)
>> #
>> # Commands:
>> # p, pick = use commit
>> # r, reword = use commit, but edit the commit message
>> # e, edit = use commit, but stop for amending
>>
>> I am not completely satisfied with the result, as I initially wished these
>> information would just appear in line after the commit subject, but this
>> comes close. Maybe the last line also needs to be dropped.
>
> This is an interesting and thought-provoking idea ;-).
>
> In practice, you would probably be touching the same file over and
> over again in the series you are rebasing, when you are doing "many
> miniscule commits recording experiments and dead ends, with an
> intention to clean it up later", and by definition, your subject
> lines are useless series of "oops", "fix", etc. The subject and
> list of filenames would probably not make a good "summary" of the
> changes for each commit.
>
> Stepping back a bit, right now, when the user asks "git commit" to
> supply material to help writing a good commit message, we punt on
> mechanically generating a good summary and instead just show output
> of "diff --cached". If we can come up with a way to mechanically
> generate a concise summary for the purpose of annotating "rebase -i"
> instruction, we probably can reuse that and append it at the end of
> the log editor "git commit" spawns when it is run without "-v".
>
I really like this idea, though I'm not sure exactly what a good
heuristic would be? A short summary of all diff "function headers"
could be valuable, as could file names. It would obviously be a
heuristic of some kind.
> Also, this makes me wonder if the ideal endgame might be to depend
> on the current "rebase -i" UI as little as possible.
>
I think this type of short summary could be valuable in lots of places yes.
> "rebase -i" is "interactive" only to the extent that you can
> interact in your text editor the order and the fashion in which the
> changes are applied. If we instead teach either gitk or tig to
> easily allow you to "tick" each commit you see in their UI and
> generate the instruction used by the sequencer, and feed that and
> actually drive the sequencer to execute it (perhaps inside a
> temporary/throwaway working tree) while you are still in gitk or tig
> and reload the UI dynamically to let you view the result, the
> overall user experience would become a lot more "interactive".
>
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Jacob Keller @ 2016-12-23 8:12 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Michael Haggerty, Jacob Keller, Git mailing list,
Norbert Kiesel
In-Reply-To: <20161223072201.zw2lwkdcs6qmb4rp@sigill.intra.peff.net>
On Thu, Dec 22, 2016 at 11:22 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Dec 22, 2016 at 01:12:12PM -0800, Junio C Hamano wrote:
>
>> Jacob Keller <jacob.keller@gmail.com> writes:
>>
>> > I don't think we have too many config options that interact in this
>> > way, so I understand that "last writing of a particular configuration"
>> > makes sense, but interactions between configs is something that would
>> > have never occurred to me. I'll send a patch to drop the compaction
>> > heuristic since I think we're all 100% in agreement that it is
>> > superseded by the new configuration (as no case has been shown where
>> > the new one is worse than compaction, and most show it to be better).
>>
>> If I recall correctly, we agreed that we'll drop the implementation
>> of compaction, but use the name --compaction-heuristics to trigger
>> the new and improved "indent heuristics":
>>
>> <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net>
>
> FWIW, I was swayed in the other direction by later messages in the
> thread. Especially your noting that the "compaction" name has always
> been labeled experimental, and Michael's argument in:
>
> http://public-inbox.org/git/8dbbd28b-af60-5e66-ae27-d7cddca233dc@alum.mit.edu/
>
> I.e., we could keep calling it "--indent-heuristic", and probably drop
> the other heuristic entirely as a failed experiment.
>
> I can live with it either way, but since I am being quoted as the source
> of the suggestion, I feel like that's an invitation to add my 2 cents. :)
>
> Liberal quoting below since I am adding Michael to the cc list.
>
> -Peff
>
I actually would prefer that we just say "this is the default now" and
provide some knob "no-indent-heuristic" or "no-compaction-heuristic"
and go with that, I think, since I am pretty sure we're all in
agreement that the heuristic is an improvement in almost every case,
certainly all the ones we've found. It's at least not worse in any
case I've seen, and is usually better.
Thoughts? I don't have a super strong opinion about which name we went
with for the knob.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Jeff King @ 2016-12-23 7:22 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael Haggerty, Jacob Keller, Jacob Keller, Git mailing list,
Norbert Kiesel
In-Reply-To: <xmqqinqbfz2r.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 22, 2016 at 01:12:12PM -0800, Junio C Hamano wrote:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
> > I don't think we have too many config options that interact in this
> > way, so I understand that "last writing of a particular configuration"
> > makes sense, but interactions between configs is something that would
> > have never occurred to me. I'll send a patch to drop the compaction
> > heuristic since I think we're all 100% in agreement that it is
> > superseded by the new configuration (as no case has been shown where
> > the new one is worse than compaction, and most show it to be better).
>
> If I recall correctly, we agreed that we'll drop the implementation
> of compaction, but use the name --compaction-heuristics to trigger
> the new and improved "indent heuristics":
>
> <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net>
FWIW, I was swayed in the other direction by later messages in the
thread. Especially your noting that the "compaction" name has always
been labeled experimental, and Michael's argument in:
http://public-inbox.org/git/8dbbd28b-af60-5e66-ae27-d7cddca233dc@alum.mit.edu/
I.e., we could keep calling it "--indent-heuristic", and probably drop
the other heuristic entirely as a failed experiment.
I can live with it either way, but since I am being quoted as the source
of the suggestion, I feel like that's an invitation to add my 2 cents. :)
Liberal quoting below since I am adding Michael to the cc list.
-Peff
> So let's do this.
>
> -- >8 --
> Subject: [PATCH] diff: retire the original experimental "compaction" heuristics
>
> This retires the experimental "compaction" heuristics but with a
> twist. It removes the mention of "indent" heuristics, which was a
> competing experiment, from everywhere, guts the core logic of the
> original "compaction" heuristics out and replaces it with the logic
> used by the "indent" heuristics.
>
> The externally visible effect of this change is that people who have
> been experimenting by setting diff.compactionHeuristic configuration
> or giving the command line option --compaction-heuristic will start
> getting the behaviour based on the improved heuristics that used to
> be called "indent" heuristics.
>
> Suggested-by: Jeff King <peff@peff.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/diff-config.txt | 6 ++---
> Documentation/diff-heuristic-options.txt | 2 --
> builtin/blame.c | 3 +--
> diff.c | 25 ++++-----------------
> git-add--interactive.perl | 5 +----
> t/t4061-diff-indent.sh | 38 ++++++++++++++++----------------
> xdiff/xdiff.h | 1 -
> xdiff/xdiffi.c | 37 +++----------------------------
> 8 files changed, 30 insertions(+), 87 deletions(-)
>
> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
> index 58f4bd6afa..39fff3aef9 100644
> --- a/Documentation/diff-config.txt
> +++ b/Documentation/diff-config.txt
> @@ -171,11 +171,9 @@ diff.tool::
>
> include::mergetools-diff.txt[]
>
> -diff.indentHeuristic::
> diff.compactionHeuristic::
> - Set one of these options to `true` to enable one of two
> - experimental heuristics that shift diff hunk boundaries to
> - make patches easier to read.
> + Set this option to `true` to enable experimental heuristics
> + that shift diff hunk boundaries to make patches easier to read.
>
> diff.algorithm::
> Choose a diff algorithm. The variants are as follows:
> diff --git a/Documentation/diff-heuristic-options.txt b/Documentation/diff-heuristic-options.txt
> index 36cb549df9..3cb024aa22 100644
> --- a/Documentation/diff-heuristic-options.txt
> +++ b/Documentation/diff-heuristic-options.txt
> @@ -1,5 +1,3 @@
> ---indent-heuristic::
> ---no-indent-heuristic::
> --compaction-heuristic::
> --no-compaction-heuristic::
> These are to help debugging and tuning experimental heuristics
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 4ddfadb71f..395d4011fb 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2596,7 +2596,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
> * and are only included here to get included in the "-h"
> * output:
> */
> - { OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental indent-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
> { OPTION_LOWLEVEL_CALLBACK, 0, "compaction-heuristic", NULL, NULL, N_("Use an experimental blank-line-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
>
> OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find better match"), XDF_NEED_MINIMAL),
> @@ -2645,7 +2644,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
> }
> parse_done:
> no_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);
> - xdl_opts |= revs.diffopt.xdl_opts & (XDF_COMPACTION_HEURISTIC | XDF_INDENT_HEURISTIC);
> + xdl_opts |= revs.diffopt.xdl_opts & XDF_COMPACTION_HEURISTIC;
> DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
> argc = parse_options_end(&ctx);
>
> diff --git a/diff.c b/diff.c
> index 8981477c43..f1b01f5b1e 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -27,7 +27,6 @@
> #endif
>
> static int diff_detect_rename_default;
> -static int diff_indent_heuristic; /* experimental */
> static int diff_compaction_heuristic; /* experimental */
> static int diff_rename_limit_default = 400;
> static int diff_suppress_blank_empty;
> @@ -223,16 +222,8 @@ void init_diff_ui_defaults(void)
>
> int git_diff_heuristic_config(const char *var, const char *value, void *cb)
> {
> - if (!strcmp(var, "diff.indentheuristic")) {
> - diff_indent_heuristic = git_config_bool(var, value);
> - if (diff_indent_heuristic)
> - diff_compaction_heuristic = 0;
> - }
> - if (!strcmp(var, "diff.compactionheuristic")) {
> + if (!strcmp(var, "diff.compactionheuristic"))
> diff_compaction_heuristic = git_config_bool(var, value);
> - if (diff_compaction_heuristic)
> - diff_indent_heuristic = 0;
> - }
> return 0;
> }
>
> @@ -3378,9 +3369,7 @@ void diff_setup(struct diff_options *options)
> options->use_color = diff_use_color_default;
> options->detect_rename = diff_detect_rename_default;
> options->xdl_opts |= diff_algorithm;
> - if (diff_indent_heuristic)
> - DIFF_XDL_SET(options, INDENT_HEURISTIC);
> - else if (diff_compaction_heuristic)
> + if (diff_compaction_heuristic)
> DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
>
> options->orderfile = diff_order_file_cfg;
> @@ -3876,15 +3865,9 @@ int diff_opt_parse(struct diff_options *options,
> DIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL);
> else if (!strcmp(arg, "--ignore-blank-lines"))
> DIFF_XDL_SET(options, IGNORE_BLANK_LINES);
> - else if (!strcmp(arg, "--indent-heuristic")) {
> - DIFF_XDL_SET(options, INDENT_HEURISTIC);
> - DIFF_XDL_CLR(options, COMPACTION_HEURISTIC);
> - } else if (!strcmp(arg, "--no-indent-heuristic"))
> - DIFF_XDL_CLR(options, INDENT_HEURISTIC);
> - else if (!strcmp(arg, "--compaction-heuristic")) {
> + else if (!strcmp(arg, "--compaction-heuristic"))
> DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
> - DIFF_XDL_CLR(options, INDENT_HEURISTIC);
> - } else if (!strcmp(arg, "--no-compaction-heuristic"))
> + else if (!strcmp(arg, "--no-compaction-heuristic"))
> DIFF_XDL_CLR(options, COMPACTION_HEURISTIC);
> else if (!strcmp(arg, "--patience"))
> options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index ee3d812695..642cce1ac6 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -45,7 +45,6 @@
> my $normal_color = $repo->get_color("", "reset");
>
> my $diff_algorithm = $repo->config('diff.algorithm');
> -my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');
> my $diff_compaction_heuristic = $repo->config_bool('diff.compactionheuristic');
> my $diff_filter = $repo->config('interactive.difffilter');
>
> @@ -751,9 +750,7 @@ sub parse_diff {
> if (defined $diff_algorithm) {
> splice @diff_cmd, 1, 0, "--diff-algorithm=${diff_algorithm}";
> }
> - if ($diff_indent_heuristic) {
> - splice @diff_cmd, 1, 0, "--indent-heuristic";
> - } elsif ($diff_compaction_heuristic) {
> + if ($diff_compaction_heuristic) {
> splice @diff_cmd, 1, 0, "--compaction-heuristic";
> }
> if (defined $patch_mode_revision) {
> diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
> index 556450609b..30f809d0d3 100755
> --- a/t/t4061-diff-indent.sh
> +++ b/t/t4061-diff-indent.sh
> @@ -1,6 +1,6 @@
> #!/bin/sh
>
> -test_description='Test diff indent heuristic.
> +test_description='Test diff compaction heuristic.
>
> '
> . ./test-lib.sh
> @@ -157,28 +157,28 @@ test_expect_success 'diff: ugly spaces' '
> compare_diff spaces-expect out
> '
>
> -test_expect_success 'diff: nice spaces with --indent-heuristic' '
> - git diff --indent-heuristic old new -- spaces.txt >out-compacted &&
> +test_expect_success 'diff: nice spaces with --compaction-heuristic' '
> + git diff --compaction-heuristic old new -- spaces.txt >out-compacted &&
> compare_diff spaces-compacted-expect out-compacted
> '
>
> -test_expect_success 'diff: nice spaces with diff.indentHeuristic' '
> - git -c diff.indentHeuristic=true diff old new -- spaces.txt >out-compacted2 &&
> +test_expect_success 'diff: nice spaces with diff.compactionHeuristic' '
> + git -c diff.compactionHeuristic=true diff old new -- spaces.txt >out-compacted2 &&
> compare_diff spaces-compacted-expect out-compacted2
> '
>
> -test_expect_success 'diff: --no-indent-heuristic overrides config' '
> - git -c diff.indentHeuristic=true diff --no-indent-heuristic old new -- spaces.txt >out2 &&
> +test_expect_success 'diff: --no-compaction-heuristic overrides config' '
> + git -c diff.compactionHeuristic=true diff --no-compaction-heuristic old new -- spaces.txt >out2 &&
> compare_diff spaces-expect out2
> '
>
> -test_expect_success 'diff: --indent-heuristic with --patience' '
> - git diff --indent-heuristic --patience old new -- spaces.txt >out-compacted3 &&
> +test_expect_success 'diff: --compaction-heuristic with --patience' '
> + git diff --compaction-heuristic --patience old new -- spaces.txt >out-compacted3 &&
> compare_diff spaces-compacted-expect out-compacted3
> '
>
> -test_expect_success 'diff: --indent-heuristic with --histogram' '
> - git diff --indent-heuristic --histogram old new -- spaces.txt >out-compacted4 &&
> +test_expect_success 'diff: --compaction-heuristic with --histogram' '
> + git diff --compaction-heuristic --histogram old new -- spaces.txt >out-compacted4 &&
> compare_diff spaces-compacted-expect out-compacted4
> '
>
> @@ -187,8 +187,8 @@ test_expect_success 'diff: ugly functions' '
> compare_diff functions-expect out
> '
>
> -test_expect_success 'diff: nice functions with --indent-heuristic' '
> - git diff --indent-heuristic old new -- functions.c >out-compacted &&
> +test_expect_success 'diff: nice functions with --compaction-heuristic' '
> + git diff --compaction-heuristic old new -- functions.c >out-compacted &&
> compare_diff functions-compacted-expect out-compacted
> '
>
> @@ -197,18 +197,18 @@ test_expect_success 'blame: ugly spaces' '
> compare_blame spaces-expect out-blame
> '
>
> -test_expect_success 'blame: nice spaces with --indent-heuristic' '
> - git blame --indent-heuristic old..new -- spaces.txt >out-blame-compacted &&
> +test_expect_success 'blame: nice spaces with --compaction-heuristic' '
> + git blame --compaction-heuristic old..new -- spaces.txt >out-blame-compacted &&
> compare_blame spaces-compacted-expect out-blame-compacted
> '
>
> -test_expect_success 'blame: nice spaces with diff.indentHeuristic' '
> - git -c diff.indentHeuristic=true blame old..new -- spaces.txt >out-blame-compacted2 &&
> +test_expect_success 'blame: nice spaces with diff.compactionHeuristic' '
> + git -c diff.compactionHeuristic=true blame old..new -- spaces.txt >out-blame-compacted2 &&
> compare_blame spaces-compacted-expect out-blame-compacted2
> '
>
> -test_expect_success 'blame: --no-indent-heuristic overrides config' '
> - git -c diff.indentHeuristic=true blame --no-indent-heuristic old..new -- spaces.txt >out-blame2 &&
> +test_expect_success 'blame: --no-compaction-heuristic overrides config' '
> + git -c diff.compactionHeuristic=true blame --no-compaction-heuristic old..new -- spaces.txt >out-blame2 &&
> git blame old..new -- spaces.txt >out-blame &&
> compare_blame spaces-expect out-blame2
> '
> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
> index 8db16d4ae6..7423f77fc8 100644
> --- a/xdiff/xdiff.h
> +++ b/xdiff/xdiff.h
> @@ -42,7 +42,6 @@ extern "C" {
> #define XDF_IGNORE_BLANK_LINES (1 << 7)
>
> #define XDF_COMPACTION_HEURISTIC (1 << 8)
> -#define XDF_INDENT_HEURISTIC (1 << 9)
>
> #define XDL_EMIT_FUNCNAMES (1 << 0)
> #define XDL_EMIT_FUNCCONTEXT (1 << 2)
> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
> index 760fbb6db7..2131ea4920 100644
> --- a/xdiff/xdiffi.c
> +++ b/xdiff/xdiffi.c
> @@ -400,11 +400,6 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,
> }
>
>
> -static int is_blank_line(xrecord_t *rec, long flags)
> -{
> - return xdl_blankline(rec->ptr, rec->size, flags);
> -}
> -
> static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags)
> {
> return (rec1->ha == rec2->ha &&
> @@ -821,7 +816,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
> struct xdlgroup g, go;
> long earliest_end, end_matching_other;
> long groupsize;
> - unsigned int blank_lines;
>
> group_init(xdf, &g);
> group_init(xdfo, &go);
> @@ -846,13 +840,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
> */
> end_matching_other = -1;
>
> - /*
> - * Boolean value that records whether there are any blank
> - * lines that could be made to be the last line of this
> - * group.
> - */
> - blank_lines = 0;
> -
> /* Shift the group backward as much as possible: */
> while (!group_slide_up(xdf, &g, flags))
> if (group_previous(xdfo, &go))
> @@ -869,11 +856,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
>
> /* Now shift the group forward as far as possible: */
> while (1) {
> - if (!blank_lines)
> - blank_lines = is_blank_line(
> - xdf->recs[g.end - 1],
> - flags);
> -
> if (group_slide_down(xdf, &g, flags))
> break;
> if (group_next(xdfo, &go))
> @@ -906,24 +888,11 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
> if (group_previous(xdfo, &go))
> xdl_bug("group sync broken sliding to match");
> }
> - } else if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) {
> + } else if (flags & XDF_COMPACTION_HEURISTIC) {
> /*
> - * Compaction heuristic: if it is possible to shift the
> - * group to make its bottom line a blank line, do so.
> + * Heuristic based on the indentation level.
> *
> - * As we already shifted the group forward as far as
> - * possible in the earlier loop, we only need to handle
> - * backward shifts, not forward ones.
> - */
> - while (!is_blank_line(xdf->recs[g.end - 1], flags)) {
> - if (group_slide_up(xdf, &g, flags))
> - xdl_bug("blank line disappeared");
> - if (group_previous(xdfo, &go))
> - xdl_bug("group sync broken sliding to blank line");
> - }
> - } else if (flags & XDF_INDENT_HEURISTIC) {
> - /*
> - * Indent heuristic: a group of pure add/delete lines
> + * A group of pure add/delete lines
> * implies two splits, one between the end of the "before"
> * context and the start of the group, and another between
> * the end of the group and the beginning of the "after"
> --
> 2.11.0-448-g9a11f8a62b
>
^ permalink raw reply
* Re: [PATCH] git-svn: escape backslashes in refnames
From: Michael Fladischer @ 2016-12-23 7:06 UTC (permalink / raw)
To: Eric Wong; +Cc: git, Junio C Hamano
In-Reply-To: <20161223014202.GA8327@starla>
[-- Attachment #1.1: Type: text/plain, Size: 230 bytes --]
On 2016-12-23 02:42, Eric Wong wrote:
> Hi Michael, the patch below should fix things up.
Thank you Eric, I was able to successfully fetch the SVN tag with your
patch applied.
Cheers,
--
Michael Fladischer
Fladi.at
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] git-svn: escape backslashes in refnames
From: Eric Wong @ 2016-12-23 1:42 UTC (permalink / raw)
To: Michael Fladischer; +Cc: git, Junio C Hamano
In-Reply-To: <cb8cd9b1-9882-64d2-435d-40d0b2b82d59@fladi.at>
Hi Michael, the patch below should fix things up.
Junio: this should go to 'maint', pull request below.
----------------8<---------------
Subject: [PATCH] git-svn: escape backslashes in refnames
This brings git-svn refname escaping up-to-date with
commit a4c2e69936df8dd0b071b85664c6cc6a4870dd84
("Disallow '\' in ref names") from May 2009.
Reported-by: Michael Fladischer <michael@fladi.at>
Message-ID: <cb8cd9b1-9882-64d2-435d-40d0b2b82d59@fladi.at>
Signed-off-by: Eric Wong <e@80x24.org>
---
The following changes since commit a274e0a036ea886a31f8b216564ab1b4a3142f6c:
Sync with maint-2.10 (2016-12-05 11:25:47 -0800)
are available in the git repository at:
git://bogomips.org/git-svn.git svn-escape-backslash
for you to fetch changes up to 22af6fef9b6538c9e87e147a920be9509acf1ddd:
git-svn: escape backslashes in refnames (2016-12-23 01:37:36 +0000)
----------------------------------------------------------------
Eric Wong (1):
git-svn: escape backslashes in refnames
perl/Git/SVN.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 711d2687a3..98518f4ddb 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -490,7 +490,7 @@ sub refname {
#
# Additionally, % must be escaped because it is used for escaping
# and we want our escaped refname to be reversible
- $refname =~ s{([ \%~\^:\?\*\[\t])}{sprintf('%%%02X',ord($1))}eg;
+ $refname =~ s{([ \%~\^:\?\*\[\t\\])}{sprintf('%%%02X',ord($1))}eg;
# no slash-separated component can begin with a dot .
# /.* becomes /%2E*
--
EW
^ permalink raw reply related
* Re: Feature request: git rebase --no-edit --continue
From: Daniel Chumak @ 2016-12-22 23:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1612230006530.155951@virtualbox>
Uff, I forgot about the CC of my last reply.
>
> Why not
>
> git commit -C HEAD && git rebase --continue
>
> ?
>
> Ciao,
> Johannes
Thanks this is a cleaner solution. I guess because I was too fixed upon
knowing of the existence of the no-edit option from other git commands
that I forget about this, even though I am using it quite often.
^ permalink raw reply
* Re: [PATCH v2 3/3] mingw: replace isatty() hack
From: Johannes Schindelin @ 2016-12-22 23:18 UTC (permalink / raw)
To: Johannes Sixt
Cc: git, Jeff Hostetler, Junio C Hamano, Pranit Bauva, Beat Bolli
In-Reply-To: <04859cf9-e67a-28ab-ccb3-249687e696c8@kdbg.org>
Hi Hannes,
On Thu, 22 Dec 2016, Johannes Sixt wrote:
> Am 22.12.2016 um 22:37 schrieb Johannes Schindelin:
>
> > Would you have a suggestion how to rephrase the comment to make it
> > less confusing?
>
> Perhaps
>
> * This might close the cached console handle.
> * We must cache the live duplicate instead.
>
> Then update the cache before the dup2, because at this time all 3 of console,
> handle, and duplicate are live and cannot be recycled:
>
> if (console == handle)
> console = duplicate;
> dup2(new_fd, fd);
Good point. I reworded the comment slightly differently (see the interdiff
in v3 0/3), hope you don't mind.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 1/3] mingw: adjust is_console() to work with stdin
From: Junio C Hamano @ 2016-12-22 23:18 UTC (permalink / raw)
To: Beat Bolli; +Cc: Johannes Schindelin, git, Pranit Bauva, Johannes Sixt
In-Reply-To: <fc3e0d9c-86ea-4a62-6b70-b9cdd67f581a@drbeat.li>
Beat Bolli <dev+git@drbeat.li> writes:
>> @@ -84,6 +84,7 @@ static void warn_if_raster_font(void)
>> static int is_console(int fd)
>> {
>> CONSOLE_SCREEN_BUFFER_INFO sbi;
>> + DWORD mode;
>
> Nit: can we move this definition into the block below where it's used?
>
>> HANDLE hcon;
>>
>> static int initialized = 0;
>> @@ -98,7 +99,10 @@ static int is_console(int fd)
>> return 0;
>>
>> /* check if its a handle to a console output screen buffer */
>> - if (!GetConsoleScreenBufferInfo(hcon, &sbi))
>> + if (!fd) {
>
> Right here:
> + DWORD mode;
>
>> + if (!GetConsoleMode(hcon, &mode))
>> + return 0;
>> + } else if (!GetConsoleScreenBufferInfo(hcon, &sbi))
>> return 0;
As these patches have been already merged to 'next' a few hours ago
as part of today's integration cycle, please make any further
refinement (if necessary) as incremental updates.
Thanks.
^ 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