* 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: 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: [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: 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
* [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: 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
* 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: [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
* [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
* [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
* 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
* 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 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 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] 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] 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: What's cooking in git.git (Dec 2016, #07; Thu, 22)
From: Lars Schneider @ 2016-12-23 23:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Luke Diamand, Stefan Beller
In-Reply-To: <xmqqzijnehgb.fsf@gitster.mtv.corp.google.com>
> On 22 Dec 2016, at 23:18, Junio C Hamano <gitster@pobox.com> wrote:
>
> Here are the topics that have been cooking. Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'. The ones marked with '.' do not appear in any of
> the integration branches, but I am still holding onto them.
>
> Even though I try not to do two "What's cooking" report back to back,
> I wanted to push out a few topics that we want to have in 'master'
> soonish on 'next' before things really quiet and slow down due to
> year-end holidays.
>
> You can find the changes described here in the integration branches
> of the repositories listed at
>
> http://git-blame.blogspot.com/p/git-public-repositories.html
>
> Here are the summaries:
>
> Will merge to 'master'.
> + jc/push-default-explicit 10-31/11-01 #2
> + sb/submodule-config-cleanup 11-22/11-23 #3
> + va/i18n-perl-scripts 12-14/12-19 #16
> + cp/merge-continue 12-15/12-19 #4
> + bw/transport-protocol-policy 12-15/12-19 #6
> + ls/filter-process 12-18/12-19 #2
> + ld/p4-compare-dir-vs-symlink 12-18/12-20 #1
> + jk/difftool-in-subdir 12-11/12-21 #4
> + sb/submodule-embed-gitdir 12-12/12-21 #6
> + gv/p4-multi-path-commit-fix 12-19/12-21 #1
> + mk/mingw-winansi-ttyname-termination-fix 12-20/12-21 #1
> + lt/shortlog-by-committer 12-20/12-21 #3
> + va/i18n-even-more 12-20/12-22 #1
> + ls/p4-lfs 12-20/12-22 #1
> + js/mingw-isatty 12-22/12-22 #3
> + bw/realpath-wo-chdir 12-22/12-22 #5
> + bw/grep-recurse-submodules 12-22/12-22 #7
>
> Will merge to 'next'.
> - jc/git-open-cloexec 11-02 #3
> - ls/p4-path-encoding 12-18 #1
Please hold it. Luke [1] made a good point and I need some time to think it through.
[1] http://public-inbox.org/git/CAE5ih7-=bD_ZoL5pFYfD2Qvy-XE24V_cgge0XoAvuoTK02EDfg@mail.gmail.com/
--
Unrelated to my topic:
"next" seems to generate a small error on macOS. Probably introduced in
"worktree: check if a submodule uses worktrees" (1a248cf)
worktree.c:423:9: error: variable 'ret' is used uninitialized whenever 'while' loop exits because its condition is false [-Werror,-Wsometimes-uninitialized]
while ((d = readdir(dir)) != NULL) {
^~~~~~~~~~~~~~~~~~~~~~~~~~
worktree.c:431:9: note: uninitialized use occurs here
return ret;
^~~
worktree.c:423:9: note: remove the condition if it is always true
while ((d = readdir(dir)) != NULL) {
^~~~~~~~~~~~~~~~~~~~~~~~~~
1
worktree.c:390:9: note: initialize the variable 'ret' to silence this warning
int ret;
^
= 0
1 error generated.
More: https://s3.amazonaws.com/archive.travis-ci.org/jobs/186186597/log.txt
- Lars
^ permalink raw reply
* [PATCH v3 0/2] pack bitmaps and incremental packing
From: David Turner @ 2016-12-24 1:08 UTC (permalink / raw)
To: git, peff; +Cc: David Turner
Cleaned up the first patch's test code.
Decided to remove the unnecessary checks from the second patch.
David Turner (2):
auto gc: don't write bitmaps for incremental repacks
repack: die on incremental + write-bitmap-index
builtin/gc.c | 9 ++++++++-
builtin/repack.c | 9 +++++++++
t/t5310-pack-bitmaps.sh | 5 +++--
t/t6500-gc.sh | 25 +++++++++++++++++++++++++
4 files changed, 45 insertions(+), 3 deletions(-)
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply
* [PATCH v3 2/2] repack: die on incremental + write-bitmap-index
From: David Turner @ 2016-12-24 1:08 UTC (permalink / raw)
To: git, peff; +Cc: David Turner
In-Reply-To: <1482541735-21346-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 | 8 +++-----
2 files changed, 12 insertions(+), 5 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..424bec7 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -118,12 +118,10 @@ 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 &&
- find .git/objects/pack -name "*.bitmap" >actual &&
- test_cmp expect actual
+ test_must_fail git repack -d 2>err &&
+ test_i18ngrep "Incremental repacks are incompatible with bitmap" err
'
test_expect_success 'incremental repack can disable bitmaps' '
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply related
* [PATCH v3 1/2] auto gc: don't write bitmaps for incremental repacks
From: David Turner @ 2016-12-24 1:08 UTC (permalink / raw)
To: git, peff; +Cc: David Turner
In-Reply-To: <1482541735-21346-1-git-send-email-dturner@twosigma.com>
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 | 25 +++++++++++++++++++++++++
2 files changed, 33 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..def2aca 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -43,4 +43,29 @@ 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 | sort >existing_packs &&
+ test_commit 523 &&
+ test_commit 790 &&
+
+ git gc --auto 2>err &&
+ test_i18ngrep ! "^warning:" err &&
+ ls .git/objects/pack/ | grep -v bitmap | sort >post_packs &&
+ comm --output-delimiter , -1 -3 existing_packs post_packs >new &&
+ comm --output-delimiter , -2 -3 existing_packs post_packs >del &&
+ test_line_count = 0 del && # No packs are deleted
+ test_line_count = 2 new # There is one new pack and its .idx
+'
+
+
test_done
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply related
* Re: [PATCH v3 1/2] auto gc: don't write bitmaps for incremental repacks
From: Jeff King @ 2016-12-24 2:57 UTC (permalink / raw)
To: David Turner; +Cc: git
In-Reply-To: <1482541735-21346-2-git-send-email-dturner@twosigma.com>
On Fri, Dec 23, 2016 at 08:08:54PM -0500, David Turner wrote:
> + git gc --auto 2>err &&
> + test_i18ngrep ! "^warning:" err &&
> + ls .git/objects/pack/ | grep -v bitmap | sort >post_packs &&
I'm not sure why you omit the bitmap here. It shouldn't change, right?
In that case it should not be mentioned by comm, and not impact the
counts you check later (and in the off chance that a new .bitmap is
created, we'd want to know and fail the test, I would think).
> + comm --output-delimiter , -1 -3 existing_packs post_packs >new &&
> + comm --output-delimiter , -2 -3 existing_packs post_packs >del &&
I don't think --output-delimiter will be portable beyond GNU comm.
> + test_line_count = 0 del && # No packs are deleted
> + test_line_count = 2 new # There is one new pack and its .idx
This logic makes sense (and is much nicer than the previous round).
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #07; Thu, 22)
From: Duy Nguyen @ 2016-12-24 10:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqd1giedxr.fsf@gitster.mtv.corp.google.com>
On Sat, Dec 24, 2016 at 12:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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.
Looks good. Merge it! :-D
--
Duy
^ permalink raw reply
* [PATCH v2] log --graph: customize the graph lines with config log.graphColors
From: Nguyễn Thái Ngọc Duy @ 2016-12-24 11:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161220123929.15329-1-pclouds@gmail.com>
If you have a 256 colors terminal (or one with true color support), then
the predefined 12 colors seem limited. On the other hand, you don't want
to draw graph lines with every single color in this mode because the two
colors could look extremely similar. This option allows you to hand pick
the colors you want.
Even with standard terminal, if your background color is neither black
or white, then the graph line may match your background and become
hidden. You can exclude your background color (or simply the colors you
hate) with this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Sounds like the good first step should be something like this instead
of jumping straight to generating a new color palette automatically.
It's not hard to create a script that generate this config value
based on some jump calculation, if you don't want to manually picking
colors.
Documentation/config.txt | 4 ++++
graph.c | 36 ++++++++++++++++++++++++++++++++++--
2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d51182a..4f26c2a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2033,6 +2033,10 @@ log.follow::
i.e. it cannot be used to follow multiple files and does not work well
on non-linear history.
+log.graphColors::
+ A list of colors, separated by commas, that can be used to draw
+ history lines in `git log --graph`.
+
log.showRoot::
If true, the initial commit will be shown as a big creation event.
This is equivalent to a diff against an empty tree.
diff --git a/graph.c b/graph.c
index d4e8519..9c58fd1 100644
--- a/graph.c
+++ b/graph.c
@@ -79,6 +79,39 @@ static void graph_show_line_prefix(const struct diff_options *diffopt)
static const char **column_colors;
static unsigned short column_colors_max;
+static void set_column_colors_by_config(void)
+{
+ static char **colors;
+ static int colors_max, colors_alloc;
+ char *string = NULL;
+ const char *end, *start;
+
+ if (git_config_get_string("log.graphcolors", &string)) {
+ graph_set_column_colors(column_colors_ansi,
+ column_colors_ansi_max);
+ return;
+ }
+
+ start = string;
+ end = string + strlen(string);
+ while (start < end) {
+ const char *comma = strchrnul(start, ',');
+ char color[COLOR_MAXLEN];
+
+ if (!color_parse_mem(start, comma - start, color)) {
+ ALLOC_GROW(colors, colors_max + 1, colors_alloc);
+ colors[colors_max++] = xstrdup(color);
+ } else
+ warning(_("ignore invalid color '%.*s'"),
+ (int)(comma - start), start);
+ start = comma + 1;
+ }
+ free(string);
+ ALLOC_GROW(colors, colors_max + 1, colors_alloc);
+ colors[colors_max] = xstrdup(GIT_COLOR_RESET);
+ graph_set_column_colors((const char **)colors, colors_max);
+}
+
void graph_set_column_colors(const char **colors, unsigned short colors_max)
{
column_colors = colors;
@@ -239,8 +272,7 @@ struct git_graph *graph_init(struct rev_info *opt)
struct git_graph *graph = xmalloc(sizeof(struct git_graph));
if (!column_colors)
- graph_set_column_colors(column_colors_ansi,
- column_colors_ansi_max);
+ set_column_colors_by_config();
graph->commit = NULL;
graph->revs = opt;
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Michael Haggerty @ 2016-12-24 12:55 UTC (permalink / raw)
To: Junio C Hamano, Jeff King
Cc: Jacob Keller, Jacob Keller, Git mailing list, Norbert Kiesel
In-Reply-To: <xmqq8tr6e46o.fsf@gitster.mtv.corp.google.com>
On 12/23/2016 10:17 PM, 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
>
> 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.
The whole patch looks good to me. Thanks for taking care of this.
Michael
^ permalink raw reply
* Re: [RFC/PATCH] add diffstat information to rebase
From: Duy Nguyen @ 2016-12-25 0:52 UTC (permalink / raw)
To: Stefan Beller; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <20161222221327.7354-1-sbeller@google.com>
On Fri, Dec 23, 2016 at 5:13 AM, Stefan Beller <sbeller@google.com> wrote:
> *Ideally* I would rather have a different formatting, e.g. maybe:
>
> $ git checkout remotes/origin/js/sequencer-wo-die
> $ git rebase -i --new-magic v2.10.0-rc2^
>
> which produces:
>
> pick d5cb9cbd64 Git 2.10-rc2 | Documentation/RelNo.., GIT-VERSION-GEN -..(+5, -1)
> ...
> pick dbfad033d4 sequencer: do not die() in do_pick_commit() | sequencer.c - do_pick_commit (+7, -6)
> pick 4ef3d8f033 sequencer: lib'ify write_message() | sequencer.c - write_message, do_pick_com..(+11, -9)
> ...
> ...
> pick 88d5a271b0 sequencer: lib'ify save_opts() | sequencer.c - save_opts + sequencer_pick..(+20, -20)
> pick 0e408fc347 sequencer: lib'ify fast_forward_to() | sequencer.c - fast_forward_to (+1, -1)
> pick 55f5704da6 sequencer: lib'ify checkout_fast_forward() | sequencer.c - checkout_fast_forward (+6, -3)
> pick 49fb937e9a sequencer: ensure to release the lock when w... | sequencer.c - read_and_refresh_cache (+6, -3)
Instead of guessing the changes based on diffstat, you could add the
actual commit message (besides the subject line) after that '|' for
fixup! and squash! commits (and it's probably should be "#" instead of
"!"). Then you could just describe what you have changed in the commit
messages. If you don't use autosquash, you'll need some way to mark
"to-be-merged" commits though, so that you don't put all commit
messages here.
--
Duy
^ 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