* Re: [PATCH 3/3] abbrev: auto size the default abbreviation
From: Junio C Hamano @ 2016-11-02 1:33 UTC (permalink / raw)
To: Jeff King; +Cc: git, Linus Torvalds
In-Reply-To: <20161003222701.za5njew33rqc5b6g@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Sep 30, 2016 at 05:19:37PM -0700, Junio C Hamano wrote:
>
>> Introduce a mechanism, where we estimate the number of objects in
>> the repository upon the first request to abbreviate an object name
>> with the default setting and come up with a sane default for the
>> repository. Based on the expectation that we would see collision in
>> a repository with 2^(2N) objects when using object names shortened
>> to first N bits, use sufficient number of hexdigits to cover the
>> number of objects in the repository. Each hexdigit (4-bits) we add
>> to the shortened name allows us to have four times (2-bits) as many
>> objects in the repository.
I was idly browsing the draft release notes and then documentation
and noticed that, even though the new default is to auto-scale,
there is no mention of that in the documentation and there is no way
to explicitly ask for auto-scaling.
I wonder if we want to have something like this. I actually am
inclined to drop the change to config.c and remove the new mention
of "auto" in the documentation.
Documentation/config.txt | 9 +++++----
config.c | 14 ++++++++++----
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a0ab66aae7..b02f8a4025 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -783,10 +783,11 @@ core.sparseCheckout::
linkgit:git-read-tree[1] for more information.
core.abbrev::
- Set the length object names are abbreviated to. If unspecified,
- many commands abbreviate to 7 hexdigits, which may not be enough
- for abbreviated object names to stay unique for sufficiently long
- time.
+ Set the length object names are abbreviated to. If
+ unspecified or set to "auto", an appropriate value is
+ computed based on the approximate number of packed objects
+ in your repository, which hopefully is enough for
+ abbreviated object names to stay unique for some time.
add.ignoreErrors::
add.ignore-errors (deprecated)::
diff --git a/config.c b/config.c
index 83fdecb1bc..c363cca4a9 100644
--- a/config.c
+++ b/config.c
@@ -834,10 +834,16 @@ static int git_default_core_config(const char *var, const char *value)
}
if (!strcmp(var, "core.abbrev")) {
- int abbrev = git_config_int(var, value);
- if (abbrev < minimum_abbrev || abbrev > 40)
- return -1;
- default_abbrev = abbrev;
+ if (!value)
+ return config_error_nonbool(var);
+ if (!strcasecmp(value, "auto"))
+ default_abbrev = -1;
+ else {
+ int abbrev = git_config_int(var, value);
+ if (abbrev < minimum_abbrev || abbrev > 40)
+ return -1;
+ default_abbrev = abbrev;
+ }
return 0;
}
^ permalink raw reply related
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Jacob Keller @ 2016-11-02 1:21 UTC (permalink / raw)
To: Stefan Beller
Cc: Jeff King, Junio C Hamano, Michael Haggerty, git@vger.kernel.org
In-Reply-To: <CAGZ79ka6un7nHaNk3F8yp3vFSnB-iGapqLcZ-ZC3EvcKE4DMNQ@mail.gmail.com>
On Tue, Nov 1, 2016 at 2:10 PM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Nov 1, 2016 at 1:59 PM, Jeff King <peff@peff.net> wrote:
>> On Tue, Nov 01, 2016 at 01:56:34PM -0700, Junio C Hamano wrote:
>>
>>> > As of -rc0, we have both --indent-heuristic and --compaction-heuristic
>>> > (along with matching config), and they are mutually exclusive.
>>> >
>>> > In [1], Stefan suggested just replacing the compaction heuristic
>>> > entirely with the new one (and you seemed to agree). If we're going to
>>> > do that, it makes sense to do so before the release, so that we don't
>>> > get stuck supporting --indent-heuristic forever.
>>>
>>> You meant "compaction" in the last part? I think it is probably a
>>> good idea.
>>
>> I thought the plan mentioned in the mail I linked was to keep the indent
>> heuristic, but simply _call_ it the compaction heuristic. IOW, to swap
>> out the implementation under the hood for something we know is better.
>
> AFAICT Michaels design is better in every aspect than what I did initially,
> so it supersedes the work I did there. I would support the swap in names.
>
Agreed, it's much better than the original idea, and results in better
diffs in every single case we could find.
>>
>> We've already released a version with --compaction-heuristic, so we are
>> stuck keeping it forever either way.
>
> IIRC the release notes specifically noted this flag to be experimental and
> may be removed in future versions.
I agree, I think that we specifically spelled out that this might go
away, and so I don't think we're stuck supporting it forever. We don't
even really need a deprecation time frame either.
>
> When not doing the swap of the implementation, but rather remove the
> experimental feature of compaction-heuristic and introducing a *new*
> experimental --indent-heuristic, this may drive the point across that
> these names are actually experimental.
I think we should swap names as "compaction heuristic" is more generic.
Thanks,
Jake
^ permalink raw reply
* should git-subtree ignore submodules?
From: Kirill Katsnelson @ 2016-11-01 19:41 UTC (permalink / raw)
To: git, apenwarr
"git-subtree add" fails to add the subtree into a repository with
submodules, reporting that the worktree is not clean, while `git status`
says that everything is clean (including submodules). I tracked the
problem down to the command "git index HEAD" returning all submodules as
having the M status:
$ git diff-index HEAD
:160000 160000 d3812c9318c4d0336897fd2d666be908fa1a7953
d3812c9318c4d0336897fd2d666be908fa1a7953 M ext/grpc
<snip more submodules>
$ git --version
git version 2.9.2.windows.1
I worked around the problem in my local copy of git-subtree shell script
by adding "--ignore-submodules=all" to the two invocations of `git
diff-index` in the ensure_clean() function (direct link
<https://github.com/git/git/blob/next/contrib/subtree/git-subtree.sh#L586>).
I am wondering, is this a defect in git-subtree? To my understanding,
the command should not care about submodules more than ensuring their
worktree is not in the way of new prefix, and that's a separate check.
So *even if* the submodule is modified, this should not be a show
stopper for "git-subtree add". Or am I missing some subtleties?
-kkm
^ permalink raw reply
* Re: [PATCH v1 05/19] update-index: warn in case of split-index incoherency
From: Christian Couder @ 2016-11-01 23:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: Duy Nguyen, Git Mailing List,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <xmqqlgx3owbg.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 1, 2016 at 8:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>>> index b75ea03..a14dbf2 100644
>>> --- a/builtin/update-index.c
>>> +++ b/builtin/update-index.c
>>> @@ -1098,12 +1098,21 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>>> }
>>>
>>> if (split_index > 0) {
>>> + if (git_config_get_split_index() == 0)
>>> + warning("core.splitIndex is set to false; "
>>> + "remove or change it, if you really want to "
>>> + "enable split index");
>>
>> Wrap this string and the one below with _() so they can be translated.
>
> True.
>
> I further wonder if a natural reaction from users after seeing this
> message is "I do want to--what else would I use that option to run
> you for? Just do as you are told, instead of telling me what to
> do!". Is this warning really a good idea, or shouldn't these places
> be setting the configuration?
In 435ec090ec (config: add core.untrackedCache, 2016-01-27) we decided
to just use warning() after discussing if we should instead set the
configuration.
^ permalink raw reply
* Re: Submodules: two confusing situations
From: Stefan Beller @ 2016-11-01 22:56 UTC (permalink / raw)
To: David Turner; +Cc: git@vger.kernel.org
In-Reply-To: <6533af94ebd74952b75e51c5609d8c20@exmbdft7.ad.twosigma.com>
On Tue, Nov 1, 2016 at 3:13 PM, David Turner <David.Turner@twosigma.com> wrote:
> Consider the following two cases:
>
> We have commit X and commit Y. X is an ancestor of Y.
>
> We're at X and doing get checkout Y -- or we're at Y and we're doing
> git checkout X.
>
> Case 1: Between X and Y, we have deleted a submodule.
> In order to move from X to Y, git removes the submodule
> from the working tree.
>
> Case 2: Between X and Y, we have instead added a submodule. In order
> to move from Y to X (that is, the opposite direction), git *does not*
> remove the submodule from the tree; instead, it gives a warning and
> leaves the submodule behind.
>
> I don't understand why these two cases are not symmetric.
Because you are using git-submodule.sh that is only an approximation of
what is supposed to happen. ("git checkout [X | Y] && git submodule update"
I'd guess).
"git submodule update" only *updates* the submodules and doesn't *delete*
them at all. I think this originated from the historic behavior of
having the submodules
.git directory inside the submodule and not in the superprojects .git/module.
When having the .git dir inside the submodule, you cannot delete the submodule
as you'd potentially loose information (local commits) from the submodule.
I am currently working on implementing a flag --recurse-submodules for
git-checkout
that would be symmetrical from X -> Y and back, but that feature hasn't seen
the mailing list yet as I discovered yet another bug locally.
I think I found a reviewer though. ;)
The problem with doing a propoer checkout, i.e. update and deletion of
a submodule,
you need to be sure the submodule in its state can go away:
* no untracked files that are lost (except for gitignored files)
* clean index in the submodule and
* the submodule points at the recorded sha1 (i.e. there are no
commits that would be dangling)
>
> Perhaps relatedly, consider the attached shell-script, which I have not yet made into a full git test because I'm not sure I understand the issues fully.
>
> It creates three commits:
>
> Commit 1 adds a submodule
> Commit 2 removes that submodule and re-adds it into a subdirectory (sub1 to sub1/sub1).
> Commit 3 adds an unrelated file.
>
> Then it checks out commit 1 (first deinitializing the submodule to avoid case 2 above), and attempts to cherry-pick commit 3. This seems like it ought to work, based on my understanding of cherry-pick. But in fact it gives a conflict on the stuff from commit 2 (which is unrelated to commit 3).
That sounds like a bug indeed.
>
> This is confusing to me, and looks like a bug. What am I missing?
>
^ permalink raw reply
* Submodules: two confusing situations
From: David Turner @ 2016-11-01 22:13 UTC (permalink / raw)
To: git@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]
Consider the following two cases:
We have commit X and commit Y. X is an ancestor of Y.
We're at X and doing get checkout Y -- or we're at Y and we're doing
git checkout X.
Case 1: Between X and Y, we have deleted a submodule.
In order to move from X to Y, git removes the submodule
from the working tree.
Case 2: Between X and Y, we have instead added a submodule. In order
to move from Y to X (that is, the opposite direction), git *does not*
remove the submodule from the tree; instead, it gives a warning and
leaves the submodule behind.
I don't understand why these two cases are not symmetric.
--
Perhaps relatedly, consider the attached shell-script, which I have not yet made into a full git test because I'm not sure I understand the issues fully.
It creates three commits:
Commit 1 adds a submodule
Commit 2 removes that submodule and re-adds it into a subdirectory (sub1 to sub1/sub1).
Commit 3 adds an unrelated file.
Then it checks out commit 1 (first deinitializing the submodule to avoid case 2 above), and attempts to cherry-pick commit 3. This seems like it ought to work, based on my understanding of cherry-pick. But in fact it gives a conflict on the stuff from commit 2 (which is unrelated to commit 3).
This is confusing to me, and looks like a bug. What am I missing?
[-- Attachment #2: submodule-merge.sh --]
[-- Type: application/octet-stream, Size: 1021 bytes --]
#!/bin/bash
set -euo pipefail
mkdir demo
cd demo
git init main
(
git init sub1 &&
cd sub1 &&
dd if=/dev/urandom of=f bs=40 count=1 &&
git add f &&
git commit -m f
)
(
cd main &&
git submodule add ../sub1 sub1 &&
> sub2 && git add sub2 &&
git commit -m 'add both submodules' &&
git tag start
)
#make a commit that replaces sub1 in a l->d transition
(
cd main &&
git rm sub1 &&
mkdir sub1 &&
git submodule add ../sub1 sub1/sub1 &&
git commit -m 'f to d' &&
#And another commit that just adds a random file.
>foo &&
git add foo &&
git commit -m 'foo' &&
git tag l-to-d
# Now we want to get back to the start state
git submodule deinit sub1/sub1 &&
git checkout -b sub2 start &&
# Finally, we want to cherry-pick an innocuous-looking commit from a branch
# where we have previously made the l->d change
# This should not fail, but does.
git cherry-pick l-to-d
)
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Junio C Hamano @ 2016-11-01 22:06 UTC (permalink / raw)
To: Stefan Beller
Cc: Jeff King, Michael Haggerty, Jacob Keller, git@vger.kernel.org
In-Reply-To: <CAGZ79kZHajTxFRbOftH==UAXhbH7RSA_jYWO-aQXhW2aSRdUFA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> -diff.indentHeuristic::
>> diff.compactionHeuristic::
>> Set one of these options to `true` to enable one of two
>> experimental heuristics that shift diff hunk boundaries to
>
> We would need to reword this as well, as there will be only one heuristic left?
>> + } else if (flags & XDF_COMPACTION_HEURISTIC) {
>> /*
>> * Indent heuristic: a group of pure add/delete lines
>
> This comment may need adjustment as well (though we could go without)
Thanks. I've queued this as "SQUASH???" on top.
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 0c79e48d9d..39fff3aef9 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -172,9 +172,8 @@ diff.tool::
include::mergetools-diff.txt[]
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/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 6cb96219cb..31ff519232 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -908,7 +908,9 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
}
} else if (flags & XDF_COMPACTION_HEURISTIC) {
/*
- * Indent heuristic: a group of pure add/delete lines
+ * Heuristic based on the indentation level.
+ *
+ * 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"
^ permalink raw reply related
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Jeff King @ 2016-11-01 21:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, Stefan Beller, Jacob Keller, git
In-Reply-To: <xmqqvaw6nac0.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 01, 2016 at 02:45:19PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > And here is _with_ renaming. I think "compaction heuristics" is a
> > much better phrase to call "heuristics used during the diff hunk
> > compaction process" without specifying how that heuristics work
> > (like taking hints in the indentation levels). If we are to retire
> > one while keeping the other, compaction-heuristics should be the
> > name we give and keep for the surviving one.
> >
> > I have not much confidence in the conversion result, though.
>
> I'll queue it on 'pu' for now, before I go offline for a few days,
> with the following log message:
Thanks. I think this is good, with the exception of the "One of these"
bits that remains in diff-config.txt.
I think Michael is OK with this approach based on previous discussions,
but we should get his Ack, too.
-Peff
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Junio C Hamano @ 2016-11-01 21:45 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Haggerty, Stefan Beller, Jacob Keller, git
In-Reply-To: <xmqqzilinanp.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> And here is _with_ renaming. I think "compaction heuristics" is a
> much better phrase to call "heuristics used during the diff hunk
> compaction process" without specifying how that heuristics work
> (like taking hints in the indentation levels). If we are to retire
> one while keeping the other, compaction-heuristics should be the
> name we give and keep for the surviving one.
>
> I have not much confidence in the conversion result, though.
I'll queue it on 'pu' for now, before I go offline for a few days,
with the following log message:
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>
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Stefan Beller @ 2016-11-01 21:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Michael Haggerty, Jacob Keller, git@vger.kernel.org
In-Reply-To: <xmqqzilinanp.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 1, 2016 at 2:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> -diff.indentHeuristic::
> diff.compactionHeuristic::
> Set one of these options to `true` to enable one of two
> experimental heuristics that shift diff hunk boundaries to
We would need to reword this as well, as there will be only one heuristic left?
> --- a/xdiff/xdiffi.c
> +++ b/xdiff/xdiffi.c
> @@ -906,22 +906,7 @@ 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) {
> + } else if (flags & XDF_COMPACTION_HEURISTIC) {
> /*
> * Indent heuristic: a group of pure add/delete lines
This comment may need adjustment as well (though we could go without)
> * implies two splits, one between the end of the "before"
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Junio C Hamano @ 2016-11-01 21:38 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Haggerty, Stefan Beller, Jacob Keller, git
In-Reply-To: <xmqqh97qoqq2.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> If it involves renaming and swapping, however, we may be talking
> about a change that we cannot deliver with confidence in less than a
> week before -rc1, which sounds, eh, suboptimal.
>
> FWIW, here is how the removal of compaction without renaming looks
> like.
And here is _with_ renaming. I think "compaction heuristics" is a
much better phrase to call "heuristics used during the diff hunk
compaction process" without specifying how that heuristics work
(like taking hints in the indentation levels). If we are to retire
one while keeping the other, compaction-heuristics should be the
name we give and keep for the surviving one.
I have not much confidence in the conversion result, though.
Documentation/diff-config.txt | 1 -
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 | 17 +-------------
8 files changed, 26 insertions(+), 66 deletions(-)
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 58f4bd6afa..0c79e48d9d 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -171,7 +171,6 @@ 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
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..6cb96219cb 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -906,22 +906,7 @@ 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) {
+ } else if (flags & XDF_COMPACTION_HEURISTIC) {
/*
* Indent heuristic: a group of pure add/delete lines
* implies two splits, one between the end of the "before"
^ permalink raw reply related
* Re: [PATCH v2 1/5] trailer: be stricter in parsing separators
From: Junio C Hamano @ 2016-11-01 21:26 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, christian.couder
In-Reply-To: <0b3e70fe-5e1f-7aaa-4c3d-88f30cb15008@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
>>> Hmph. The optional whitespace is to allow for what kind of line?
>>>
>>> It is not for "Signed off by:" that is a misspelt "Signed-off-by:";
>>> it may not hurt but I do not think of a case that would be useful
>>> offhand.
>
> This is to allow trailers of the form "Fix #42" (mentioned in the
> git-interpret-trailers documentation and also in the test).
That piece of information belongs to the in-code comment for the
function, I would think. The comment that describes what it does
(i.e. "token with optional spaces and separator") can be read from
the code easily, but the reason why the code implements that
behaviour cannot.
Thanks.
^ permalink raw reply
* Re: Git issue
From: Junio C Hamano @ 2016-11-01 21:23 UTC (permalink / raw)
To: Jeff King; +Cc: Philip Oakley, Halde, Faiz, git
In-Reply-To: <20161101210310.sqrhvviry7iyyjrm@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Nov 01, 2016 at 08:50:23PM -0000, Philip Oakley wrote:
>
>> > From "git help update-index":
>> >
>> > --[no-]assume-unchanged
>> > When this flag is specified, the object names recorded for
>> > the paths are not updated. Instead, this option sets/unsets
>> > the "assume unchanged" bit for the paths. ...
>
> I think the "Instead" is "we are not doing the usual update-index thing
> of reading the new data from disk; instead, we are _just_ setting the
> bit". Perhaps that can be spelled out more clearly, but I think just
> dropping "Instead" is a step backwards.
I tend to agree; the biggest problem with this part of the
description I think is that it starts by saying what it does not do,
which may help people who expect the command to do its usual thing
but otherwise is a secondary information. How about ripping it out
and moving it after the primary description, i.e.
Set or unset the "assume unchanged" bit for the paths,
without changing the object names recorded for them, in the
index.
>> Given the number of misrepresentations (on the web) of what the bit does,
>> and the ongoing misunderstandings of users it does feel like the man page
>> article could be refreshed to be more assertive about the users promise, and
>> Git's cautions.
>
> I dunno. I know this has long been a source of confusion, but I
> specifically dug in the docs to see what we had, and I thought what I
> quoted above was pretty clear. That has "only" been around for about 2
> years, and is fighting against other mis-advice on the Internet, though.
> So I'm not sure if it is badly worded, or if people simply do not see
> it.
I share the same reaction, and I find that the update in ccadb25f73
("doc: make clear --assume-unchanged's user contract", 2014-12-06)
that introduced the "the user promises not to change and allows Git
to assume" is sufficiently clear.
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Stefan Beller @ 2016-11-01 21:10 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Michael Haggerty, Jacob Keller,
git@vger.kernel.org
In-Reply-To: <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net>
On Tue, Nov 1, 2016 at 1:59 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 01, 2016 at 01:56:34PM -0700, Junio C Hamano wrote:
>
>> > As of -rc0, we have both --indent-heuristic and --compaction-heuristic
>> > (along with matching config), and they are mutually exclusive.
>> >
>> > In [1], Stefan suggested just replacing the compaction heuristic
>> > entirely with the new one (and you seemed to agree). If we're going to
>> > do that, it makes sense to do so before the release, so that we don't
>> > get stuck supporting --indent-heuristic forever.
>>
>> You meant "compaction" in the last part? I think it is probably a
>> good idea.
>
> I thought the plan mentioned in the mail I linked was to keep the indent
> heuristic, but simply _call_ it the compaction heuristic. IOW, to swap
> out the implementation under the hood for something we know is better.
AFAICT Michaels design is better in every aspect than what I did initially,
so it supersedes the work I did there. I would support the swap in names.
>
> We've already released a version with --compaction-heuristic, so we are
> stuck keeping it forever either way.
IIRC the release notes specifically noted this flag to be experimental and
may be removed in future versions.
When not doing the swap of the implementation, but rather remove the
experimental feature of compaction-heuristic and introducing a *new*
experimental --indent-heuristic, this may drive the point across that
these names are actually experimental.
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Junio C Hamano @ 2016-11-01 21:05 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Haggerty, Stefan Beller, Jacob Keller, git
In-Reply-To: <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> We've already released a version with --compaction-heuristic, so we are
> stuck keeping it forever either way.
While I do not necessarily agree with this conclusion (we've already
released it labelled as experimental; can't we freely take it
away?), that may be of lessor impact. I like it better.
If it involves renaming and swapping, however, we may be talking
about a change that we cannot deliver with confidence in less than a
week before -rc1, which sounds, eh, suboptimal.
FWIW, here is how the removal of compaction without renaming looks
like.
Documentation/diff-heuristic-options.txt | 2 --
builtin/blame.c | 3 +--
diff.c | 23 +++--------------------
git-add--interactive.perl | 3 ---
xdiff/xdiff.h | 3 +--
xdiff/xdiffi.c | 15 ---------------
6 files changed, 5 insertions(+), 44 deletions(-)
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..f45416a6c3 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2597,7 +2597,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
* 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),
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..f9be87fdfb 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -906,21 +906,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
^ permalink raw reply related
* [PATCH] doc: update-index --assume-unchanged
From: Philip Oakley @ 2016-11-01 21:04 UTC (permalink / raw)
To: git; +Cc: philipoakley, fhalde, peff, gitster
In-Reply-To: <7CE3166CFD244DAABF554451E8B0800F@PhilipOakley>
Be more assertive about the User promise, and Git's defensiveness
of file changes.
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
Here is a full patch for
https://public-inbox.org/git/7CE3166CFD244DAABF554451E8B0800F@PhilipOakley/
---
Documentation/git-update-index.txt | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 7386c93..4ec1711 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -85,10 +85,10 @@ OPTIONS
--[no-]assume-unchanged::
When this flag is specified, the object names recorded
- for the paths are not updated. Instead, this option
+ for the paths are not updated. This option
sets/unsets the "assume unchanged" bit for the
paths. When the "assume unchanged" bit is on, the user
- promises not to change the file and allows Git to assume
+ *promises* not to change the file and allows Git to assume
that the working tree file matches what is recorded in
the index. If you want to change the working tree file,
you need to unset the bit to tell Git. This is
@@ -301,17 +301,23 @@ $ git ls-files -s
Using ``assume unchanged'' bit
------------------------------
-Many operations in Git depend on your filesystem to have an
+Many operations in Git depend on your filesystem having a fast and
efficient `lstat(2)` implementation, so that `st_mtime`
information for working tree files can be cheaply checked to see
if the file contents have changed from the version recorded in
the index file. Unfortunately, some filesystems have
inefficient `lstat(2)`. If your filesystem is one of them, you
-can set "assume unchanged" bit to paths you have not changed to
-cause Git not to do this check. Note that setting this bit on a
-path does not mean Git will check the contents of the file to
-see if it has changed -- it makes Git to omit any checking and
-assume it has *not* changed. When you make changes to working
+can set "assume unchanged" bit to *paths you have not changed* to
+cause Git not to do this check.
+
+Note that setting this bit on a
+path does not mean Git will never check the contents of the file to
+see if it has changed. Though normally it makes Git to omit any checking to
+assume it has not changed.
+Commands which may overwrite local changes (pull/merge etc) are
+likely to check if the contents have changed
+
+If you make desired changes to working
tree files, you have to explicitly tell Git about it by dropping
"assume unchanged" bit, either before or after you modify them.
--
2.9.0.windows.1.323.g0305acf
^ permalink raw reply related
* Re: Git issue
From: Jeff King @ 2016-11-01 21:03 UTC (permalink / raw)
To: Philip Oakley; +Cc: Halde, Faiz, git
In-Reply-To: <7CE3166CFD244DAABF554451E8B0800F@PhilipOakley>
On Tue, Nov 01, 2016 at 08:50:23PM -0000, Philip Oakley wrote:
> > From "git help update-index":
> >
> > --[no-]assume-unchanged
> > When this flag is specified, the object names recorded for
> > the paths are not updated. Instead, this option sets/unsets
> > the "assume unchanged" bit for the paths. When the "assume
> > unchanged" bit is on, the user promises not to change the
> > file and allows Git to assume that the working tree file
> > matches what is recorded in the index. If you want to change
> > the working tree file, you need to unset the bit to tell Git.
> > This is sometimes helpful when working with a big project on
> > a filesystem that has very slow lstat(2) system call (e.g.
> > cifs).
> >
> > Git will fail (gracefully) in case it needs to modify this
> > file in the index e.g. when merging in a commit; thus, in
> > case the assumed-untracked file is changed upstream, you will
> > need to handle the situation manually.
> >
>
> The whole section (including the ones above this quote) are often confused
> between the promises of the user, and the alleged promises of Git. Even in
> the quote above the "Instead" probably shouldn't be there.
I think the "Instead" is "we are not doing the usual update-index thing
of reading the new data from disk; instead, we are _just_ setting the
bit". Perhaps that can be spelled out more clearly, but I think just
dropping "Instead" is a step backwards.
> Given the number of misrepresentations (on the web) of what the bit does,
> and the ongoing misunderstandings of users it does feel like the man page
> article could be refreshed to be more assertive about the users promise, and
> Git's cautions.
I dunno. I know this has long been a source of confusion, but I
specifically dug in the docs to see what we had, and I thought what I
quoted above was pretty clear. That has "only" been around for about 2
years, and is fighting against other mis-advice on the Internet, though.
So I'm not sure if it is badly worded, or if people simply do not see
it.
-Peff
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Jeff King @ 2016-11-01 20:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, Stefan Beller, Jacob Keller, git
In-Reply-To: <xmqqlgx2or5p.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 01, 2016 at 01:56:34PM -0700, Junio C Hamano wrote:
> > As of -rc0, we have both --indent-heuristic and --compaction-heuristic
> > (along with matching config), and they are mutually exclusive.
> >
> > In [1], Stefan suggested just replacing the compaction heuristic
> > entirely with the new one (and you seemed to agree). If we're going to
> > do that, it makes sense to do so before the release, so that we don't
> > get stuck supporting --indent-heuristic forever.
>
> You meant "compaction" in the last part? I think it is probably a
> good idea.
I thought the plan mentioned in the mail I linked was to keep the indent
heuristic, but simply _call_ it the compaction heuristic. IOW, to swap
out the implementation under the hood for something we know is better.
We've already released a version with --compaction-heuristic, so we are
stuck keeping it forever either way.
> I'd vote for just removing compaction-heuristic while keeping the
> indent-heuristic with experimental label and knobs and keeping it
> off by default for a while.
So the matching variant of that plan would be to drop the internals of
compaction-heuristic, swap in the new heuristic instead, and then drop
all of the --indent-heuristic. It remains off by default, but we may
flip that in a later release.
-Peff
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Junio C Hamano @ 2016-11-01 20:56 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Haggerty, Stefan Beller, Jacob Keller, git
In-Reply-To: <20161101203637.3jr73wwpfal4brho@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Oct 31, 2016 at 02:49:42PM -0700, Junio C Hamano wrote:
>
>> * Output from "git diff" can be made easier to read by selecting
>> which lines are common and which lines are added/deleted
>> intelligently when the lines before and after the changed section
>> are the same. A command line option is added to help with the
>> experiment to find a good heuristics.
>
> I'm not sure we ever resolved all the discussion around options and
> defaults here.
>
> As of -rc0, we have both --indent-heuristic and --compaction-heuristic
> (along with matching config), and they are mutually exclusive.
>
> In [1], Stefan suggested just replacing the compaction heuristic
> entirely with the new one (and you seemed to agree). If we're going to
> do that, it makes sense to do so before the release, so that we don't
> get stuck supporting --indent-heuristic forever.
You meant "compaction" in the last part? I think it is probably a
good idea.
I'd vote for just removing compaction-heuristic while keeping the
indent-heuristic with experimental label and knobs and keeping it
off by default for a while.
Thanks for bringing it up.
^ permalink raw reply
* Re: [PATCH v2 1/5] trailer: be stricter in parsing separators
From: Jonathan Tan @ 2016-11-01 20:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, christian.couder
In-Reply-To: <xmqqr36vndgn.fsf@gitster.mtv.corp.google.com>
On 11/01/2016 01:37 PM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Jonathan Tan <jonathantanmy@google.com> writes:
>>
>>> Currently, a line is interpreted to be a trailer line if it contains a
>>> separator. Make parsing stricter by requiring the text on the left of
>>> the separator, if not the empty string, to be of the "<token><optional
>>> whitespace>" form.
>>
>> Hmph. The optional whitespace is to allow for what kind of line?
>>
>> It is not for "Signed off by:" that is a misspelt "Signed-off-by:";
>> it may not hurt but I do not think of a case that would be useful
>> offhand.
This is to allow trailers of the form "Fix #42" (mentioned in the
git-interpret-trailers documentation and also in the test).
> Other than this "Hmph" (which is not an objection---just something
> that the reviewer did not understand), the rest looked good to me.
>
> Will re-queue.
>
> Thanks.
Thanks!
^ permalink raw reply
* Re: Git issue
From: Philip Oakley @ 2016-11-01 20:50 UTC (permalink / raw)
To: Jeff King, Halde, Faiz; +Cc: git
In-Reply-To: <20161101174526.e2tilsriz2fqaru3@sigill.intra.peff.net>
From: "Jeff King" <peff@peff.net>
> On Tue, Nov 01, 2016 at 10:28:57AM +0000, Halde, Faiz wrote:
>
>> I frequently use the following command to ignore changes done in a file
>>
>> git update-index --assume-unchanged somefile
>>
>> Now when I do a pull from my remote branch and say the file 'somefile'
>> was changed locally and in remote, git will abort the merge saying I
>> need to commit my changes of 'somefile'.
>>
>> But isn't the whole point of the above command to ignore the changes
>> within the file?
>
> No. The purpose of --assume-unchanged is to promise git that you will
> not change the file, so that it may skip checking the file contents in
> some cases as an optimization.
True. However...
>
> From "git help update-index":
>
> --[no-]assume-unchanged
> When this flag is specified, the object names recorded for
> the paths are not updated. Instead, this option sets/unsets
> the "assume unchanged" bit for the paths. When the "assume
> unchanged" bit is on, the user promises not to change the
> file and allows Git to assume that the working tree file
> matches what is recorded in the index. If you want to change
> the working tree file, you need to unset the bit to tell Git.
> This is sometimes helpful when working with a big project on
> a filesystem that has very slow lstat(2) system call (e.g.
> cifs).
>
> Git will fail (gracefully) in case it needs to modify this
> file in the index e.g. when merging in a commit; thus, in
> case the assumed-untracked file is changed upstream, you will
> need to handle the situation manually.
>
The whole section (including the ones above this quote) are often confused
between the promises of the user, and the alleged promises of Git. Even in
the quote above the "Instead" probably shouldn't be there.
Given the number of misrepresentations (on the web) of what the bit does,
and the ongoing misunderstandings of users it does feel like the man page
article could be refreshed to be more assertive about the users promise, and
Git's cautions.
My quick rough working on a more assertive update..
-- >8 --
----------------------
Documentation/git-update-index.txt ----------------------
index 7386c93..4ec1711 100644
@@ -84,12 +84,12 @@ OPTIONS
Set the execute permissions on the updated files.
--[no-]assume-unchanged::
When this flag is specified, the object names recorded
- for the paths are not updated. Instead, this option
+ for the paths are not updated. This option
sets/unsets the "assume unchanged" bit for the
paths. When the "assume unchanged" bit is on, the user
- promises not to change the file and allows Git to assume
+ *promises* not to change the file and allows Git to assume
that the working tree file matches what is recorded in
the index. If you want to change the working tree file,
you need to unset the bit to tell Git. This is
sometimes helpful when working with a big project on a
@@ -300,19 +300,25 @@ $ git ls-files -s
Using ``assume unchanged'' bit
------------------------------
-Many operations in Git depend on your filesystem to have an
+Many operations in Git depend on your filesystem having a fast and
efficient `lstat(2)` implementation, so that `st_mtime`
information for working tree files can be cheaply checked to see
if the file contents have changed from the version recorded in
the index file. Unfortunately, some filesystems have
inefficient `lstat(2)`. If your filesystem is one of them, you
-can set "assume unchanged" bit to paths you have not changed to
-cause Git not to do this check. Note that setting this bit on a
-path does not mean Git will check the contents of the file to
-see if it has changed -- it makes Git to omit any checking and
-assume it has *not* changed. When you make changes to working
+can set "assume unchanged" bit to *paths you have not changed* to
+cause Git not to do this check.
+
+Note that setting this bit on a
+path does not mean Git will never check the contents of the file to
+see if it has changed. Though normally it makes Git to omit any checking to
+assume it has not changed.
+Commands which may overwrite local changes (pull/merge etc) are
+likely to check if the contents have changed
+
+If you make desired changes to working
tree files, you have to explicitly tell Git about it by dropping
"assume unchanged" bit, either before or after you modify them.
In order to set "assume unchanged" bit, use `--assume-unchanged`
---
Philip
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Stefan Beller @ 2016-11-01 20:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Linux Kernel
In-Reply-To: <xmqq1sywrxxl.fsf@gitster.mtv.corp.google.com>
> * "git clone --resurse-submodules --reference $path $URL" is a way to
"recurse"
^ permalink raw reply
* Re: [PATCH v2 1/5] trailer: be stricter in parsing separators
From: Junio C Hamano @ 2016-11-01 20:37 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, christian.couder
In-Reply-To: <xmqqvaw7ndow.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> Currently, a line is interpreted to be a trailer line if it contains a
>> separator. Make parsing stricter by requiring the text on the left of
>> the separator, if not the empty string, to be of the "<token><optional
>> whitespace>" form.
>
> Hmph. The optional whitespace is to allow for what kind of line?
>
> It is not for "Signed off by:" that is a misspelt "Signed-off-by:";
> it may not hurt but I do not think of a case that would be useful
> offhand.
Other than this "Hmph" (which is not an objection---just something
that the reviewer did not understand), the rest looked good to me.
Will re-queue.
Thanks.
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Jeff King @ 2016-11-01 20:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, Stefan Beller, Jacob Keller, git
In-Reply-To: <xmqq1sywrxxl.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 31, 2016 at 02:49:42PM -0700, Junio C Hamano wrote:
> * Output from "git diff" can be made easier to read by selecting
> which lines are common and which lines are added/deleted
> intelligently when the lines before and after the changed section
> are the same. A command line option is added to help with the
> experiment to find a good heuristics.
I'm not sure we ever resolved all the discussion around options and
defaults here.
As of -rc0, we have both --indent-heuristic and --compaction-heuristic
(along with matching config), and they are mutually exclusive.
In [1], Stefan suggested just replacing the compaction heuristic
entirely with the new one (and you seemed to agree). If we're going to
do that, it makes sense to do so before the release, so that we don't
get stuck supporting --indent-heuristic forever.
We also discussed making this the default (e.g., in [2]). Did we want to
do that before the release? If so, it would have been nice to flip it on
during -rc0 to get more exposure, but perhaps -rc1 would not be too
late. That's less critical than resolving the indent/compaction thing,
though it potentially means we could rip out the config entirely and
just leave the command-line option (as an escape hatch and debugging
tool).
-Peff
[1] http://public-inbox.org/git/CAGZ79kaMSkRfkBng_Epq+2T_q--VkKQ6-m=M_jPkzeYcxuDKWA@mail.gmail.com/
[2] http://public-inbox.org/git/CA+P7+xrsz27Hhk12dwDrUEpn0L7R8F5z-XASS2JkoY6sqK7u5A@mail.gmail.com/
^ permalink raw reply
* Re: [PATCH v2 1/5] trailer: be stricter in parsing separators
From: Junio C Hamano @ 2016-11-01 20:32 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, christian.couder
In-Reply-To: <c7db0aafb543845382e1835e3704273d3596e6bb.1478028700.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> Currently, a line is interpreted to be a trailer line if it contains a
> separator. Make parsing stricter by requiring the text on the left of
> the separator, if not the empty string, to be of the "<token><optional
> whitespace>" form.
Hmph. The optional whitespace is to allow for what kind of line?
It is not for "Signed off by:" that is a misspelt "Signed-off-by:";
it may not hurt but I do not think of a case that would be useful
offhand.
> (The find_separator function distinguishes the no-separator case from
> the separator-starts-line case because some callers of this function
> need such a distinction.)
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> trailer.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/trailer.c b/trailer.c
> index f0ecde2..0ee634f 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -563,15 +563,26 @@ static int token_matches_item(const char *tok, struct arg_item *item, int tok_le
> }
>
> /*
> - * Return the location of the first separator in line, or -1 if there is no
> - * separator.
> + * If the given line is of the form
> + * "<token><optional whitespace><separator>..." or "<separator>...", return the
> + * location of the separator. Otherwise, return -1.
> */
> static int find_separator(const char *line, const char *separators)
> {
> - int loc = strcspn(line, separators);
> - if (!line[loc])
> - return -1;
> - return loc;
> + int whitespace_found = 0;
> + const char *c;
> + for (c = line; *c; c++) {
> + if (strchr(separators, *c))
> + return c - line;
> + if (!whitespace_found && (isalnum(*c) || *c == '-'))
> + continue;
> + if (c != line && (*c == ' ' || *c == '\t')) {
> + whitespace_found = 1;
> + continue;
> + }
> + break;
> + }
> + return -1;
> }
>
> /*
^ 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