Re: [PATCH v3 42/42] git-completion.bash: add GIT_COMPLETION_OPTIONS=all config

[Eric Sunshine]

On Fri, Feb 9, 2018 at 6:02 AM, Nguyễn Thái Ngọc Duy  wrote:
> By default, some option names (mostly --force, scripting related or for
> internal use) are not completable for various reasons. When
> GIT_COMPLETION_OPTIONS is set to all, all options (except hidden ones)
> are completable.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> @@ -36,6 +36,10 @@
> +#
> +#   GIT_COMPLETION_OPTIONS
> +#
> +# When set to "all", complete all possible options
> @@ -303,7 +307,7 @@ __gitcomp_builtin ()
> if [ -z "$options" ]; then
> # leading and trailing spaces are significant to make
> # option removal work correctly.
> -   options=" $(__git ${cmd/_/ } --git-completion-helper) $incl "
> +   options=" $(__git ${cmd/_/ } 
> --git-completion-helper=$GIT_COMPLETION_OPTIONS) $incl "

This approach is rather different from what I had envisioned. Rather
than asking --git-completion-helper to do the filtering, my thought
was that it should unconditionally dump _all_ options but annotate
them, and then git-completion.bash can filter however it sees fit. For
instance, --git-completion-helper might annotate "dangerous" options
with a "!" ("!--force" or "--force!" or "--force:!" or whatever).

The benefit of this approach is that it more easily supports future
enhancements. For instance, options which only make sense in certain
contexts could be annotated to indicate such. An example are the
--continue, --abort, --skip options for git-rebase which only make
sense when a rebase session is active. One could imagine these options
being annotated something like this:

--abort:rebasing
--continue:rebasing
--skip:rebasing

where git-completion.bash understands the "rebasing" annotation as
meaning that these options only make sense when "rebase-apply" is
present. (In fact, the annotation could be more expressive, such as
"--abort:exists(rebase-apply)", but that might be overkill.)

[PATCH v3 06/12] sequencer: fast-forward merge commits, if possible

[Johannes Schindelin]

Just like with regular `pick` commands, if we are trying to recreate a
merge commit, we now test whether the parents of said commit match HEAD
and the commits to be merged, and fast-forward if possible.

This is not only faster, but also avoids unnecessary proliferation of
new objects.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index e577c213494..27d582479d1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2123,7 +2123,7 @@ static int do_merge(struct commit *commit, const char 
*arg, int arg_len,
struct commit *head_commit, *merge_commit, *i;
struct commit_list *common, *j, *reversed = NULL;
struct merge_options o;
-   int ret;
+   int can_fast_forward, ret;
static struct lock_file lock;
 
for (merge_arg_len = 0; merge_arg_len < arg_len; merge_arg_len++)
@@ -2191,6 +2191,14 @@ static int do_merge(struct commit *commit, const char 
*arg, int arg_len,
strbuf_release();
}
 
+   /*
+* If HEAD is not identical to the parent of the original merge commit,
+* we cannot fast-forward.
+*/
+   can_fast_forward = opts->allow_ff && commit && commit->parents &&
+   !oidcmp(>parents->item->object.oid,
+   _commit->object.oid);
+
strbuf_addf(_name, "refs/rewritten/%.*s", merge_arg_len, arg);
merge_commit = lookup_commit_reference_by_name(ref_name.buf);
if (!merge_commit) {
@@ -2204,6 +2212,17 @@ static int do_merge(struct commit *commit, const char 
*arg, int arg_len,
rollback_lock_file();
return -1;
}
+
+   if (can_fast_forward && commit->parents->next &&
+   !commit->parents->next->next &&
+   !oidcmp(>parents->next->item->object.oid,
+   _commit->object.oid)) {
+   strbuf_release(_name);
+   rollback_lock_file();
+   return fast_forward_to(>object.oid,
+  _commit->object.oid, 0, opts);
+   }
+
write_message(oid_to_hex(_commit->object.oid), GIT_SHA1_HEXSZ,
  git_path_merge_head(), 0);
write_message("no-ff", 5, git_path_merge_mode(), 0);
-- 
2.16.1.windows.1

[PATCH v3 08/12] rebase: introduce the --recreate-merges option

[Johannes Schindelin]

Once upon a time, this here developer thought: wouldn't it be nice if,
say, Git for Windows' patches on top of core Git could be represented as
a thicket of branches, and be rebased on top of core Git in order to
maintain a cherry-pick'able set of patch series?

The original attempt to answer this was: git rebase --preserve-merges.

However, that experiment was never intended as an interactive option,
and it only piggy-backed on git rebase --interactive because that
command's implementation looked already very, very familiar: it was
designed by the same person who designed --preserve-merges: yours truly.

Some time later, some other developer (I am looking at you, Andreas!
;-)) decided that it would be a good idea to allow --preserve-merges to
be combined with --interactive (with caveats!) and the Git maintainer
(well, the interim Git maintainer during Junio's absence, that is)
agreed, and that is when the glamor of the --preserve-merges design
started to fall apart rather quickly and unglamorously.

The reason? In --preserve-merges mode, the parents of a merge commit (or
for that matter, of *any* commit) were not stated explicitly, but were
*implied* by the commit name passed to the `pick` command.

This made it impossible, for example, to reorder commits. Not to mention
to flatten the branch topology or, deity forbid, to split topic branches
into two.

Alas, these shortcomings also prevented that mode (whose original
purpose was to serve Git for Windows' needs, with the additional hope
that it may be useful to others, too) from serving Git for Windows'
needs.

Five years later, when it became really untenable to have one unwieldy,
big hodge-podge patch series of partly related, partly unrelated patches
in Git for Windows that was rebased onto core Git's tags from time to
time (earning the undeserved wrath of the developer of the ill-fated
git-remote-hg series that first obsoleted Git for Windows' competing
approach, only to be abandoned without maintainer later) was really
untenable, the "Git garden shears" were born [*1*/*2*]: a script,
piggy-backing on top of the interactive rebase, that would first
determine the branch topology of the patches to be rebased, create a
pseudo todo list for further editing, transform the result into a real
todo list (making heavy use of the `exec` command to "implement" the
missing todo list commands) and finally recreate the patch series on
top of the new base commit.

That was in 2013. And it took about three weeks to come up with the
design and implement it as an out-of-tree script. Needless to say, the
implementation needed quite a few years to stabilize, all the while the
design itself proved itself sound.

With this patch, the goodness of the Git garden shears comes to `git
rebase -i` itself. Passing the `--recreate-merges` option will generate
a todo list that can be understood readily, and where it is obvious
how to reorder commits. New branches can be introduced by inserting
`label` commands and calling `merge `. And once this mode will
have become stable and universally accepted, we can deprecate the design
mistake that was `--preserve-merges`.

Link *1*:
https://github.com/msysgit/msysgit/blob/master/share/msysGit/shears.sh
Link *2*:
https://github.com/git-for-windows/build-extra/blob/master/shears.sh

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-rebase.txt   |   9 +-
 contrib/completion/git-completion.bash |   2 +-
 git-rebase--interactive.sh |   1 +
 git-rebase.sh  |   6 ++
 t/t3430-rebase-recreate-merges.sh  | 146 +
 5 files changed, 162 insertions(+), 2 deletions(-)
 create mode 100755 t/t3430-rebase-recreate-merges.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0d6..e9da7e26329 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,6 +368,12 @@ The commit list format can be changed by setting the 
configuration option
 rebase.instructionFormat.  A customized instruction format will automatically
 have the long commit hash prepended to the format.
 
+--recreate-merges::
+   Recreate merge commits instead of flattening the history by replaying
+   merges. Merge conflict resolutions or manual amendments to merge
+   commits are not recreated automatically, but have to be recreated
+   manually.
+
 -p::
 --preserve-merges::
Recreate merge commits instead of flattening the history by replaying
@@ -770,7 +776,8 @@ BUGS
 The todo list presented by `--preserve-merges --interactive` does not
 represent the topology of the revision graph.  Editing commits and
 rewording their commit messages should work fine, but attempts to
-reorder commits tend to produce counterintuitive results.
+reorder commits tend to produce counterintuitive results. Use
+--recreate-merges for a more faithful representation.
 
 For example, an attempt to rearrange
 

[PATCH v3 09/12] sequencer: make refs generated by the `label` command worktree-local

[Johannes Schindelin]

This allows for rebases to be run in parallel in separate worktrees
(think: interrupted in the middle of one rebase, being asked to perform
a different rebase, adding a separate worktree just for that job).

Signed-off-by: Johannes Schindelin 
---
 refs.c|  3 ++-
 t/t3430-rebase-recreate-merges.sh | 14 ++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 20ba82b4343..e8b84c189ff 100644
--- a/refs.c
+++ b/refs.c
@@ -600,7 +600,8 @@ int dwim_log(const char *str, int len, struct object_id 
*oid, char **log)
 static int is_per_worktree_ref(const char *refname)
 {
return !strcmp(refname, "HEAD") ||
-   starts_with(refname, "refs/bisect/");
+   starts_with(refname, "refs/bisect/") ||
+   starts_with(refname, "refs/rewritten/");
 }
 
 static int is_pseudoref_syntax(const char *refname)
diff --git a/t/t3430-rebase-recreate-merges.sh 
b/t/t3430-rebase-recreate-merges.sh
index 0073601a206..1a3e43d66ff 100755
--- a/t/t3430-rebase-recreate-merges.sh
+++ b/t/t3430-rebase-recreate-merges.sh
@@ -143,4 +143,18 @@ test_expect_success 'with a branch tip that was 
cherry-picked already' '
EOF
 '
 
+test_expect_success 'refs/rewritten/* is worktree-local' '
+   git worktree add wt &&
+   cat >wt/script-from-scratch <<-\EOF &&
+   label xyz
+   exec GIT_DIR=../.git git rev-parse --verify refs/rewritten/xyz >a || :
+   exec git rev-parse --verify refs/rewritten/xyz >b
+   EOF
+
+   test_config -C wt sequence.editor \""$PWD"/replace-editor.sh\" &&
+   git -C wt rebase -i HEAD &&
+   test_must_be_empty wt/a &&
+   test_cmp_rev HEAD "$(cat wt/b)"
+'
+
 test_done
-- 
2.16.1.windows.1

[PATCH v3 03/12] git-rebase--interactive: clarify arguments

[Johannes Schindelin]

From: Stefan Beller 

Up to now each command took a commit as its first argument and ignored
the rest of the line (usually the subject of the commit)

Now that we are about to introduce commands that take different
arguments, clarify each command by giving the argument list.

Signed-off-by: Stefan Beller 
Signed-off-by: Johannes Schindelin 
---
 git-rebase--interactive.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593a..fcedece1860 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -155,13 +155,13 @@ reschedule_last_action () {
 append_todo_help () {
gettext "
 Commands:
-p, pick = use commit
-r, reword = use commit, but edit the commit message
-e, edit = use commit, but stop for amending
-s, squash = use commit, but meld into previous commit
-f, fixup = like \"squash\", but discard this commit's log message
-x, exec = run command (the rest of the line) using shell
-d, drop = remove commit
+p, pick  = use commit
+r, reword  = use commit, but edit the commit message
+e, edit  = use commit, but stop for amending
+s, squash  = use commit, but meld into previous commit
+f, fixup  = like \"squash\", but discard this commit's log message
+x, exec  = run command (the rest of the line) using shell
+d, drop  = remove commit
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
-- 
2.16.1.windows.1

[PATCH v3 11/12] pull: accept --rebase=recreate to recreate the branch topology

[Johannes Schindelin]

Similar to the `preserve` mode simply passing the `--preserve-merges`
option to the `rebase` command, the `recreate` mode simply passes the
`--recreate-merges` option.

This will allow users to conveniently rebase non-trivial commit
topologies when pulling new commits, without flattening them.

Signed-off-by: Johannes Schindelin 
---
 Documentation/config.txt   |  8 
 Documentation/git-pull.txt |  5 -
 builtin/pull.c | 14 ++
 builtin/remote.c   |  2 ++
 contrib/completion/git-completion.bash |  2 +-
 5 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0e25b2c92b3..da41ab246dc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1058,6 +1058,10 @@ branch..rebase::
"git pull" is run. See "pull.rebase" for doing this in a non
branch-specific manner.
 +
+When recreate, also pass `--recreate-merges` along to 'git rebase'
+so that locally committed merge commits will not be flattened
+by running 'git pull'.
++
 When preserve, also pass `--preserve-merges` along to 'git rebase'
 so that locally committed merge commits will not be flattened
 by running 'git pull'.
@@ -2607,6 +2611,10 @@ pull.rebase::
pull" is run. See "branch..rebase" for setting this on a
per-branch basis.
 +
+When recreate, also pass `--recreate-merges` along to 'git rebase'
+so that locally committed merge commits will not be flattened
+by running 'git pull'.
++
 When preserve, also pass `--preserve-merges` along to 'git rebase'
 so that locally committed merge commits will not be flattened
 by running 'git pull'.
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index ce05b7a5b13..b4f9f057ea9 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -101,13 +101,16 @@ Options related to merging
 include::merge-options.txt[]
 
 -r::
---rebase[=false|true|preserve|interactive]::
+--rebase[=false|true|recreate|preserve|interactive]::
When true, rebase the current branch on top of the upstream
branch after fetching. If there is a remote-tracking branch
corresponding to the upstream branch and the upstream branch
was rebased since last fetched, the rebase uses that information
to avoid rebasing non-local changes.
 +
+When set to recreate, rebase with the `--recreate-merges` option passed
+to `git rebase` so that locally created merge commits will not be flattened.
++
 When set to preserve, rebase with the `--preserve-merges` option passed
 to `git rebase` so that locally created merge commits will not be flattened.
 +
diff --git a/builtin/pull.c b/builtin/pull.c
index 511dbbe0f6e..e33c84e0345 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -27,14 +27,16 @@ enum rebase_type {
REBASE_FALSE = 0,
REBASE_TRUE,
REBASE_PRESERVE,
+   REBASE_RECREATE,
REBASE_INTERACTIVE
 };
 
 /**
  * Parses the value of --rebase. If value is a false value, returns
  * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is
- * "preserve", returns REBASE_PRESERVE. If value is a invalid value, dies with
- * a fatal error if fatal is true, otherwise returns REBASE_INVALID.
+ * "recreate", returns REBASE_RECREATE. If value is "preserve", returns
+ * REBASE_PRESERVE. If value is a invalid value, dies with a fatal error if
+ * fatal is true, otherwise returns REBASE_INVALID.
  */
 static enum rebase_type parse_config_rebase(const char *key, const char *value,
int fatal)
@@ -47,6 +49,8 @@ static enum rebase_type parse_config_rebase(const char *key, 
const char *value,
return REBASE_TRUE;
else if (!strcmp(value, "preserve"))
return REBASE_PRESERVE;
+   else if (!strcmp(value, "recreate"))
+   return REBASE_RECREATE;
else if (!strcmp(value, "interactive"))
return REBASE_INTERACTIVE;
 
@@ -130,7 +134,7 @@ static struct option pull_options[] = {
/* Options passed to git-merge or git-rebase */
OPT_GROUP(N_("Options related to merging")),
{ OPTION_CALLBACK, 'r', "rebase", _rebase,
- "false|true|preserve|interactive",
+ "false|true|recreate|preserve|interactive",
  N_("incorporate changes by rebasing rather than merging"),
  PARSE_OPT_OPTARG, parse_opt_rebase },
OPT_PASSTHRU('n', NULL, _diffstat, NULL,
@@ -798,7 +802,9 @@ static int run_rebase(const struct object_id *curr_head,
argv_push_verbosity();
 
/* Options passed to git-rebase */
-   if (opt_rebase == REBASE_PRESERVE)
+   if (opt_rebase == REBASE_RECREATE)
+   argv_array_push(, "--recreate-merges");
+   else if (opt_rebase == REBASE_PRESERVE)
argv_array_push(, "--preserve-merges");
else if (opt_rebase == REBASE_INTERACTIVE)

[PATCH v3 10/12] sequencer: handle post-rewrite for merge commands

[Johannes Schindelin]

In the previous patches, we implemented the basic functionality of the
`git rebase -i --recreate-merges` command, in particular the `merge`
command to create merge commits in the sequencer.

The interactive rebase is a lot more these days, though, than a simple
cherry-pick in a loop. For example, it calls the post-rewrite hook (if
any) after rebasing with a mapping of the old->new commits.

This patch implements the post-rewrite handling for the `merge` command
we just introduced. The other commands that were added recently (`label`
and `reset`) do not create new commits, therefore post-rewrite do not
need to handle them.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c   |  7 +--
 t/t3430-rebase-recreate-merges.sh | 25 +
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 7cd091a9fd6..306ae014311 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2452,11 +2452,14 @@ static int pick_commits(struct todo_list *todo_list, 
struct replay_opts *opts)
else if (item->command == TODO_RESET)
res = do_reset(item->arg, item->arg_len, opts);
else if (item->command == TODO_MERGE ||
-item->command == TODO_MERGE_AND_EDIT)
+item->command == TODO_MERGE_AND_EDIT) {
res = do_merge(item->commit, item->arg, item->arg_len,
   item->command == TODO_MERGE_AND_EDIT ?
   EDIT_MSG | VERIFY_MSG : 0, opts);
-   else if (!is_noop(item->command))
+   if (item->commit)
+   record_in_rewritten(>commit->object.oid,
+   peek_command(todo_list, 1));
+   } else if (!is_noop(item->command))
return error(_("unknown command %d"), item->command);
 
todo_list->current++;
diff --git a/t/t3430-rebase-recreate-merges.sh 
b/t/t3430-rebase-recreate-merges.sh
index 1a3e43d66ff..35a61ce90bb 100755
--- a/t/t3430-rebase-recreate-merges.sh
+++ b/t/t3430-rebase-recreate-merges.sh
@@ -157,4 +157,29 @@ test_expect_success 'refs/rewritten/* is worktree-local' '
test_cmp_rev HEAD "$(cat wt/b)"
 '
 
+test_expect_success 'post-rewrite hook and fixups work for merges' '
+   git checkout -b post-rewrite &&
+   test_commit same1 &&
+   git reset --hard HEAD^ &&
+   test_commit same2 &&
+   git merge -m "to fix up" same1 &&
+   echo same old same old >same2.t &&
+   test_tick &&
+   git commit --fixup HEAD same2.t &&
+   fixup="$(git rev-parse HEAD)" &&
+
+   mkdir -p .git/hooks &&
+   test_when_finished "rm .git/hooks/post-rewrite" &&
+   echo "cat >actual" | write_script .git/hooks/post-rewrite &&
+
+   test_tick &&
+   git rebase -i --autosquash --recreate-merges HEAD^^^ &&
+   printf "%s %s\n%s %s\n%s %s\n%s %s\n" >expect $(git rev-parse \
+   $fixup^^2 HEAD^2 \
+   $fixup^^ HEAD^ \
+   $fixup^ HEAD \
+   $fixup HEAD) &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.16.1.windows.1

[PATCH v3 12/12] rebase -i: introduce --recreate-merges=[no-]rebase-cousins

[Johannes Schindelin]

This one is a bit tricky to explain, so let's try with a diagram:

C
  /   \
A - B - E - F
  \   /
D

To illustrate what this new mode is all about, let's consider what
happens upon `git rebase -i --recreate-merges B`, in particular to
the commit `D`. So far, the new branch structure would be:

   --- C' --
  / \
A - B -- E' - F'
  \/
D'

This is not really preserving the branch topology from before! The
reason is that the commit `D` does not have `B` as ancestor, and
therefore it gets rebased onto `B`.

This is unintuitive behavior. Even worse, when recreating branch
structure, most use cases would appear to want cousins *not* to be
rebased onto the new base commit. For example, Git for Windows (the
heaviest user of the Git garden shears, which served as the blueprint
for --recreate-merges) frequently merges branches from `next` early, and
these branches certainly do *not* want to be rebased. In the example
above, the desired outcome would look like this:

   --- C' --
  / \
A - B -- E' - F'
  \/
   -- D' --

Let's introduce the term "cousins" for such commits ("D" in the
example), and let's not rebase them by default, introducing the new
"rebase-cousins" mode for use cases where they should be rebased.

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-rebase.txt  |  7 ++-
 builtin/rebase--helper.c  |  9 -
 git-rebase--interactive.sh|  1 +
 git-rebase.sh | 12 +++-
 sequencer.c   |  4 
 sequencer.h   |  6 ++
 t/t3430-rebase-recreate-merges.sh | 23 +++
 7 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e9da7e26329..0e6d020d924 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,11 +368,16 @@ The commit list format can be changed by setting the 
configuration option
 rebase.instructionFormat.  A customized instruction format will automatically
 have the long commit hash prepended to the format.
 
---recreate-merges::
+--recreate-merges[=(rebase-cousins|no-rebase-cousins)]::
Recreate merge commits instead of flattening the history by replaying
merges. Merge conflict resolutions or manual amendments to merge
commits are not recreated automatically, but have to be recreated
manually.
++
+By default, or when `no-rebase-cousins` was specified, commits which do not
+have `` as direct ancestor keep their original branch point.
+If the `rebase-cousins` mode is turned on, such commits are rebased onto
+`` (or ``, if specified).
 
 -p::
 --preserve-merges::
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index a34ab5c0655..cea99cb3235 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,7 +13,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
unsigned flags = 0, keep_empty = 0, recreate_merges = 0;
-   int abbreviate_commands = 0;
+   int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
@@ -23,6 +23,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
OPT_BOOL(0, "keep-empty", _empty, N_("keep empty 
commits")),
OPT_BOOL(0, "recreate-merges", _merges, N_("recreate 
merge commits")),
+   OPT_BOOL(0, "rebase-cousins", _cousins,
+N_("keep original branch points of cousins")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -57,8 +59,13 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
flags |= recreate_merges ? TODO_LIST_RECREATE_MERGES : 0;
+   flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
+   if (rebase_cousins >= 0 && !recreate_merges)
+   warning(_("--[no-]rebase-cousins has no effect without "
+ "--recreate-merges"));
+
if (command == CONTINUE && argc == 1)
return !!sequencer_continue();
if (command == ABORT && argc == 1)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index cfe3a537ac2..e199fe1cca5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -903,6 +903,7 @@ if test t != 

[PATCH v3 07/12] rebase-helper --make-script: introduce a flag to recreate merges

[Johannes Schindelin]

The sequencer just learned new commands intended to recreate branch
structure (similar in spirit to --preserve-merges, but with a
substantially less-broken design).

Let's allow the rebase--helper to generate todo lists making use of
these commands, triggered by the new --recreate-merges option. For a
commit topology like this (where the HEAD points to C):

- A - B - C
\   /
  D

the generated todo list would look like this:

# branch D
pick 0123 A
label branch-point
pick 1234 D
label D

reset branch-point
pick 2345 B
merge -C 3456 D # C

To keep things simple, we first only implement support for merge commits
with exactly two parents, leaving support for octopus merges to a later
patch in this patch series.

As a special, hard-coded label, all merge-recreating todo lists start with
the command `label onto` so that we can later always refer to the revision
onto which everything is rebased.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase--helper.c |   4 +-
 sequencer.c  | 349 ++-
 sequencer.h  |   1 +
 3 files changed, 351 insertions(+), 3 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 7daee544b7b..a34ab5c0655 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,7 +12,7 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0;
+   unsigned flags = 0, keep_empty = 0, recreate_merges = 0;
int abbreviate_commands = 0;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
@@ -22,6 +22,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
OPT_BOOL(0, "keep-empty", _empty, N_("keep empty 
commits")),
+   OPT_BOOL(0, "recreate-merges", _merges, N_("recreate 
merge commits")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -55,6 +56,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
 
flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
+   flags |= recreate_merges ? TODO_LIST_RECREATE_MERGES : 0;
flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
if (command == CONTINUE && argc == 1)
diff --git a/sequencer.c b/sequencer.c
index 27d582479d1..7cd091a9fd6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -23,6 +23,8 @@
 #include "hashmap.h"
 #include "unpack-trees.h"
 #include "worktree.h"
+#include "oidmap.h"
+#include "oidset.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -2808,6 +2810,341 @@ void append_signoff(struct strbuf *msgbuf, int 
ignore_footer, unsigned flag)
strbuf_release();
 }
 
+struct labels_entry {
+   struct hashmap_entry entry;
+   char label[FLEX_ARRAY];
+};
+
+static int labels_cmp(const void *fndata, const struct labels_entry *a,
+ const struct labels_entry *b, const void *key)
+{
+   return key ? strcmp(a->label, key) : strcmp(a->label, b->label);
+}
+
+struct string_entry {
+   struct oidmap_entry entry;
+   char string[FLEX_ARRAY];
+};
+
+struct label_state {
+   struct oidmap commit2label;
+   struct hashmap labels;
+   struct strbuf buf;
+};
+
+static const char *label_oid(struct object_id *oid, const char *label,
+struct label_state *state)
+{
+   struct labels_entry *labels_entry;
+   struct string_entry *string_entry;
+   struct object_id dummy;
+   size_t len;
+   int i;
+
+   string_entry = oidmap_get(>commit2label, oid);
+   if (string_entry)
+   return string_entry->string;
+
+   /*
+* For "uninteresting" commits, i.e. commits that are not to be
+* rebased, and which can therefore not be labeled, we use a unique
+* abbreviation of the commit name. This is slightly more complicated
+* than calling find_unique_abbrev() because we also need to make
+* sure that the abbreviation does not conflict with any other
+* label.
+*
+* We disallow "interesting" commits to be labeled by a string that
+* is a valid full-length hash, to ensure that we always can find an
+* abbreviation for any uninteresting commit's names that does not
+* clash with any other label.
+*/
+   if (!label) {
+   char *p;
+
+   strbuf_reset(>buf);
+   

[PATCH v3 04/12] sequencer: introduce new commands to reset the revision

[Johannes Schindelin]

In the upcoming commits, we will teach the sequencer to recreate merges.
This will be done in a very different way from the unfortunate design of
`git rebase --preserve-merges` (which does not allow for reordering
commits, or changing the branch topology).

The main idea is to introduce new todo list commands, to support
labeling the current revision with a given name, resetting the current
revision to a previous state, and  merging labeled revisions.

This idea was developed in Git for Windows' Git garden shears (that are
used to maintain the "thicket of branches" on top of upstream Git), and
this patch is part of the effort to make it available to a wider
audience, as well as to make the entire process more robust (by
implementing it in a safe and portable language rather than a Unix shell
script).

This commit implements the commands to label, and to reset to, given
revisions. The syntax is:

label 
reset 

Internally, the `label ` command creates the ref
`refs/rewritten/`. This makes it possible to work with the labeled
revisions interactively, or in a scripted fashion (e.g. via the todo
list command `exec`).

These temporary refs are removed upon sequencer_remove_state(), so that
even a `git rebase --abort` cleans them up.

We disallow '#' as label because that character will be used as separator
in the upcoming `merge` command.

Later in this patch series, we will mark the `refs/rewritten/` refs as
worktree-local, to allow for interactive rebases to be run in parallel in
worktrees linked to the same repository.

Signed-off-by: Johannes Schindelin 
---
 git-rebase--interactive.sh |   2 +
 sequencer.c| 190 +++--
 2 files changed, 186 insertions(+), 6 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index fcedece1860..7e5281e74aa 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -162,6 +162,8 @@ s, squash  = use commit, but meld into previous 
commit
 f, fixup  = like \"squash\", but discard this commit's log message
 x, exec  = run command (the rest of the line) using shell
 d, drop  = remove commit
+l, label  = label current HEAD with a name
+t, reset  = reset HEAD to a label
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
diff --git a/sequencer.c b/sequencer.c
index 764ad43388f..8638086f667 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -21,6 +21,8 @@
 #include "log-tree.h"
 #include "wt-status.h"
 #include "hashmap.h"
+#include "unpack-trees.h"
+#include "worktree.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -116,6 +118,13 @@ static GIT_PATH_FUNC(rebase_path_stopped_sha, 
"rebase-merge/stopped-sha")
 static GIT_PATH_FUNC(rebase_path_rewritten_list, "rebase-merge/rewritten-list")
 static GIT_PATH_FUNC(rebase_path_rewritten_pending,
"rebase-merge/rewritten-pending")
+
+/*
+ * The path of the file listing refs that need to be deleted after the rebase
+ * finishes. This is used by the `label` command to record the need for 
cleanup.
+ */
+static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete")
+
 /*
  * The following files are written by git-rebase just after parsing the
  * command-line (and are only consumed, not modified, by the sequencer).
@@ -195,18 +204,33 @@ static const char *gpg_sign_opt_quoted(struct replay_opts 
*opts)
 
 int sequencer_remove_state(struct replay_opts *opts)
 {
-   struct strbuf dir = STRBUF_INIT;
+   struct strbuf buf = STRBUF_INIT;
int i;
 
+   if (strbuf_read_file(, rebase_path_refs_to_delete(), 0) > 0) {
+   char *p = buf.buf;
+   while (*p) {
+   char *eol = strchr(p, '\n');
+   if (eol)
+   *eol = '\0';
+   if (delete_ref("(rebase -i) cleanup", p, NULL, 0) < 0)
+   warning(_("could not delete '%s'"), p);
+   if (!eol)
+   break;
+   p = eol + 1;
+   }
+   }
+
free(opts->gpg_sign);
free(opts->strategy);
for (i = 0; i < opts->xopts_nr; i++)
free(opts->xopts[i]);
free(opts->xopts);
 
-   strbuf_addstr(, get_dir(opts));
-   remove_dir_recursively(, 0);
-   strbuf_release();
+   strbuf_reset();
+   strbuf_addstr(, get_dir(opts));
+   remove_dir_recursively(, 0);
+   strbuf_release();
 
return 0;
 }
@@ -769,6 +793,8 @@ enum todo_command {
TODO_SQUASH,
/* commands that do something else than handling a single commit */
TODO_EXEC,
+   TODO_LABEL,
+   TODO_RESET,
/* commands that do nothing but are counted for reporting progress */
TODO_NOOP,
TODO_DROP,
@@ -787,6 +813,8 @@ static struct {
{ 'f', "fixup" },
{ 's', 

[PATCH v3 05/12] sequencer: introduce the `merge` command

[Johannes Schindelin]

This patch is part of the effort to reimplement `--preserve-merges` with
a substantially improved design, a design that has been developed in the
Git for Windows project to maintain the dozens of Windows-specific patch
series on top of upstream Git.

The previous patch implemented the `label` and `reset` commands to label
commits and to reset to a labeled commits. This patch adds the `merge`
command, with the following syntax:

merge [-C ]  # 

The  parameter in this instance is the *original* merge commit,
whose author and message will be used for the merge commit that is about
to be created.

The  parameter refers to the (possibly rewritten) revision to
merge. Let's see an example of a todo list:

label onto

# Branch abc
reset onto
pick deadbeef Hello, world!
label abc

reset onto
pick cafecafe And now for something completely different
merge -C baaabaaa abc # Merge the branch 'abc' into master

To edit the merge commit's message (a "reword" for merges, if you will),
use `-c` (lower-case) instead of `-C`; this convention was borrowed from
`git commit` that also supports `-c` and `-C` with similar meanings.

To create *new* merges, i.e. without copying the commit message from an
existing commit, simply omit the `-C ` parameter (which will
open an editor for the merge message):

merge abc

This comes in handy when splitting a branch into two or more branches.

Note: this patch only adds support for recursive merges, to keep things
simple. Support for octopus merges will be added later in a separate
patch series, support for merges using strategies other than the
recursive merge is left for the future.

Signed-off-by: Johannes Schindelin 
---
 git-rebase--interactive.sh |   4 ++
 sequencer.c| 158 +
 2 files changed, 162 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 7e5281e74aa..9d9d91f25e3 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -164,6 +164,10 @@ x, exec  = run command (the rest of the line) 
using shell
 d, drop  = remove commit
 l, label  = label current HEAD with a name
 t, reset  = reset HEAD to a label
+m, merge [-C  | -c ]  [# ]
+.   create a merge commit using the original merge commit's
+.   message (or the oneline, if no original merge commit was
+.   specified). Use -c  to reword the commit message.
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
diff --git a/sequencer.c b/sequencer.c
index 8638086f667..e577c213494 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -795,6 +795,8 @@ enum todo_command {
TODO_EXEC,
TODO_LABEL,
TODO_RESET,
+   TODO_MERGE,
+   TODO_MERGE_AND_EDIT,
/* commands that do nothing but are counted for reporting progress */
TODO_NOOP,
TODO_DROP,
@@ -815,6 +817,8 @@ static struct {
{ 'x', "exec" },
{ 'l', "label" },
{ 't', "reset" },
+   { 'm', "merge" },
+   { 0, "merge" }, /* MERGE_AND_EDIT */
{ 0,   "noop" },
{ 'd', "drop" },
{ 0,   NULL }
@@ -1317,6 +1321,21 @@ static int parse_insn_line(struct todo_item *item, const 
char *bol, char *eol)
return 0;
}
 
+   if (item->command == TODO_MERGE) {
+   if (skip_prefix(bol, "-C", ))
+   bol += strspn(bol, " \t");
+   else if (skip_prefix(bol, "-c", )) {
+   bol += strspn(bol, " \t");
+   item->command = TODO_MERGE_AND_EDIT;
+   } else {
+   item->command = TODO_MERGE_AND_EDIT;
+   item->commit = NULL;
+   item->arg = bol;
+   item->arg_len = (int)(eol - bol);
+   return 0;
+   }
+   }
+
end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
saved = *end_of_object_name;
*end_of_object_name = '\0';
@@ -2096,6 +2115,134 @@ static int do_reset(const char *name, int len, struct 
replay_opts *opts)
return ret;
 }
 
+static int do_merge(struct commit *commit, const char *arg, int arg_len,
+   int run_commit_flags, struct replay_opts *opts)
+{
+   int merge_arg_len;
+   struct strbuf ref_name = STRBUF_INIT;
+   struct commit *head_commit, *merge_commit, *i;
+   struct commit_list *common, *j, *reversed = NULL;
+   struct merge_options o;
+   int ret;
+   static struct lock_file lock;
+
+   for (merge_arg_len = 0; merge_arg_len < arg_len; merge_arg_len++)
+   if (isspace(arg[merge_arg_len]))
+   break;
+
+   if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
+   return -1;
+
+   head_commit = lookup_commit_reference_by_name("HEAD");
+   

[PATCH v3 00/12] rebase -i: offer to recreate merge commits

[Johannes Schindelin]

Once upon a time, I dreamt of an interactive rebase that would not
flatten branch structure, but instead recreate the commit topology
faithfully.

My original attempt was --preserve-merges, but that design was so
limited that I did not even enable it in interactive mode.

Subsequently, it *was* enabled in interactive mode, with the predictable
consequences: as the --preserve-merges design does not allow for
specifying the parents of merge commits explicitly, all the new commits'
parents are defined *implicitly* by the previous commit history, and
hence it is *not possible to even reorder commits*.

This design flaw cannot be fixed. Not without a complete re-design, at
least. This patch series offers such a re-design.

Think of --recreate-merges as "--preserve-merges done right". It
introduces new verbs for the todo list, `label`, `reset` and `merge`.
For a commit topology like this:

A - B - C
  \   /
D

the generated todo list would look like this:

# branch D
pick 0123 A
label branch-point
pick 1234 D
label D

reset branch-point
pick 2345 B
merge -C 3456 D # C

There are more patches in the pipeline, based on this patch series, but
left for later in the interest of reviewable patch series: one mini
series to use the sequencer even for `git rebase -i --root`, and another
one to add support for octopus merges to --recreate-merges.

Changes since v2:

- fixed the incorrect comment for rebase_path_refs_to_delete.

- we now error out properly if read_cache_unmerged() fails.

- if there are unresolved merge conflicts, the `reset` command now errors out
  (even if the current design should not allow for such a scenario to occur).

- a diff hunk that was necessary to support `bud` was dropped from 2/10.

- changed all `rollback_lock_file(); return error_errno(...);` patterns to
  first show the errors (i.e. using the correct errno). This added 1/11.

- The temporary refs are now also cleaned up upon `git rebase --abort`.

- Reworked the entire patch series to support

merge -C   # 

  instead of the previous `merge   `.

- Dropped the octopus part of the description of the `merge` command in
  the usage at the bottom of the todo list, as it is subject to change.

- The autosquash handling was not elegant, and cuddled into the same
  commit as the post-rewrite changes. Now, the autosquash handling is a
  lot more elegant, and a separate introductory patch (as it arguably
  improves the current code on its own).


Johannes Schindelin (11):
  sequencer: avoid using errno clobbered by rollback_lock_file()
  sequencer: make rearrange_squash() a bit more obvious
  sequencer: introduce new commands to reset the revision
  sequencer: introduce the `merge` command
  sequencer: fast-forward merge commits, if possible
  rebase-helper --make-script: introduce a flag to recreate merges
  rebase: introduce the --recreate-merges option
  sequencer: make refs generated by the `label` command worktree-local
  sequencer: handle post-rewrite for merge commands
  pull: accept --rebase=recreate to recreate the branch topology
  rebase -i: introduce --recreate-merges=[no-]rebase-cousins

Stefan Beller (1):
  git-rebase--interactive: clarify arguments

 Documentation/config.txt   |   8 +
 Documentation/git-pull.txt |   5 +-
 Documentation/git-rebase.txt   |  14 +-
 builtin/pull.c |  14 +-
 builtin/rebase--helper.c   |  13 +-
 builtin/remote.c   |   2 +
 contrib/completion/git-completion.bash |   4 +-
 git-rebase--interactive.sh |  22 +-
 git-rebase.sh  |  16 +
 refs.c |   3 +-
 sequencer.c| 736 -
 sequencer.h|   7 +
 t/t3430-rebase-recreate-merges.sh  | 208 ++
 13 files changed, 1021 insertions(+), 31 deletions(-)
 create mode 100755 t/t3430-rebase-recreate-merges.sh


base-commit: 5be1f00a9a701532232f57958efab4be8c959a29
Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v3
Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v3

Interdiff vs v2:
 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index 5e21e4cf269..e199fe1cca5 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -164,10 +164,10 @@ x, exec  = run command (the rest of the line) 
using shell
  d, drop  = remove commit
  l, label  = label current HEAD with a name
  t, reset  = reset HEAD to a label
 -m, merge  (  | \"...\" ) []
 +m, merge [-C  | -c ]  [# ]
  .   create a merge commit using the original merge commit's
 -.   message (or the oneline, if "-" is given). Use a quoted
 -.   list of commits to be merged for octopus merges.
 +.   message (or the oneline, if no original 

[PATCH v3 01/12] sequencer: avoid using errno clobbered by rollback_lock_file()

[Johannes Schindelin]

As pointed out in a review of the `--recreate-merges` patch series,
`rollback_lock_file()` clobbers errno. Therefore, we have to report the
error message that uses errno before calling said function.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 4d3f60594cb..114db3b2775 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -296,12 +296,14 @@ static int write_message(const void *buf, size_t len, 
const char *filename,
if (msg_fd < 0)
return error_errno(_("could not lock '%s'"), filename);
if (write_in_full(msg_fd, buf, len) < 0) {
+   error_errno(_("could not write to '%s'"), filename);
rollback_lock_file(_file);
-   return error_errno(_("could not write to '%s'"), filename);
+   return -1;
}
if (append_eol && write(msg_fd, "\n", 1) < 0) {
+   error_errno(_("could not write eol to '%s'"), filename);
rollback_lock_file(_file);
-   return error_errno(_("could not write eol to '%s'"), filename);
+   return -1;
}
if (commit_lock_file(_file) < 0) {
rollback_lock_file(_file);
@@ -1584,16 +1586,17 @@ static int save_head(const char *head)
 
fd = hold_lock_file_for_update(_lock, git_path_head_file(), 0);
if (fd < 0) {
+   error_errno(_("could not lock HEAD"));
rollback_lock_file(_lock);
-   return error_errno(_("could not lock HEAD"));
+   return -1;
}
strbuf_addf(, "%s\n", head);
written = write_in_full(fd, buf.buf, buf.len);
strbuf_release();
if (written < 0) {
+   error_errno(_("could not write to '%s'"), git_path_head_file());
rollback_lock_file(_lock);
-   return error_errno(_("could not write to '%s'"),
-  git_path_head_file());
+   return -1;
}
if (commit_lock_file(_lock) < 0) {
rollback_lock_file(_lock);
-- 
2.16.1.windows.1

[PATCH v3 02/12] sequencer: make rearrange_squash() a bit more obvious

[Johannes Schindelin]

There are some commands that have to be skipped from rearranging by virtue
of not handling any commits.

However, the logic was not quite obvious: it skipped commands based on
their position in the enum todo_command.

Instead, let's make it explicit that we skip all commands that do not
handle any commit. With one exception: the `drop` command, because it,
well, drops the commit and is therefore not eligible to rearranging.

Note: this is a bit academic at the moment because the only time we call
`rearrange_squash()` is directly after generating the todo list, when we
have nothing but `pick` commands anyway.

However, the upcoming `merge` command *will* want to be handled by that
function, and it *can* handle commits.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 114db3b2775..764ad43388f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2890,7 +2890,7 @@ int rearrange_squash(void)
struct subject2item_entry *entry;
 
next[i] = tail[i] = -1;
-   if (item->command >= TODO_EXEC) {
+   if (!item->commit || item->command == TODO_DROP) {
subjects[i] = NULL;
continue;
}
-- 
2.16.1.windows.1

Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Johannes Schindelin]

Hi Sergey,

On Fri, 9 Feb 2018, Sergey Organov wrote:

> Johannes Schindelin  writes:
> 
> [...]
> 
> > With this patch, the goodness of the Git garden shears comes to `git
> > rebase -i` itself. Passing the `--recreate-merges` option will generate
> > a todo list that can be understood readily, and where it is obvious
> > how to reorder commits. New branches can be introduced by inserting
> > `label` commands and calling `merge -  `. And once this
> > mode has become stable and universally accepted, we can deprecate the
> > design mistake that was `--preserve-merges`.
> 
> This doesn't explain why you introduced this new --recreate-merges. Why
> didn't you rather fix --preserve-merges to generate and use new todo
> list format?

Because that would of course break existing users of --preserve-merges.

So why not --preserve-merges=v2? Because that would force me to maintain
--preserve-merges forever. And I don't want to.

> It doesn't seem likely that todo list created by one Git version is to
> be ever used by another, right?

No. But by scripts based on `git rebase -p`.

> Is there some hidden reason here? Some tools outside of Git that use old
> todo list format, maybe?

Exactly.

I did mention such a tool: the Git garden shears:

https://github.com/git-for-windows/build-extra/blob/master/shears.sh

Have a look at it. It will inform the discussion.

> Then, if new option indeed required, please look at the resulting manual:
> 
> --recreate-merges::
>   Recreate merge commits instead of flattening the history by replaying
>   merges. Merge conflict resolutions or manual amendments to merge
>   commits are not preserved.
> 
> -p::
> --preserve-merges::
>   Recreate merge commits instead of flattening the history by replaying
>   commits a merge commit introduces. Merge conflict resolutions or manual
>   amendments to merge commits are not preserved.

As I stated in the cover letter, there are more patches lined up after
this patch series.

Have a look at https://github.com/git/git/pull/447, especially the latest
commit in there which is an early version of the deprecation I intend to
bring about.

Also, please refrain from saying things like... "Don't you think ..."

If you don't like the wording, I wold much more appreciate it if a better
alternative was suggested.

> Don't you think more explanations are needed there in the manual on
> why do we have 2 separate options with almost the same yet subtly
> different description? Is this subtle difference even important? How?
> 
> I also have trouble making sense of "Recreate merge commits instead of
> flattening the history by replaying merges." Is it " commits by replaying merges> instead of " or is it
> rather " instead of  replaying merges>?

The documentation of the --recreate-merges option is not meant to explain
the difference to --preserve-merges. It is meant to explain the difference
to regular `git rebase -i`, which flattens the commit history into a
single branch without merge commits (in fact, all merge commits are simply
ignored).

And I would rather not start to describe the difference between
--recreate-merges and --preserve-merges because I want to deprecate the
latter, and describing the difference as I get the sense is your wish
would simply mean more work because it would have to be added and then
removed again.

If you still think it would be a good idea to describe the difference
between --recreate-merges and --preserve-merges, then please provide a
suggestion, preferably in the form of a patch, that adds appropriate
paragraphs to *both* options' documentation, so that your proposal can be
discussed properly.

Ciao,
Johannes

Re: [PATCH v2 02/10] sequencer: introduce new commands to reset therevision

[Johannes Schindelin]

Hi Phillip,

On Wed, 31 Jan 2018, Phillip Wood wrote:

> On 31/01/18 13:21, Johannes Schindelin wrote:
> > 
> > On Tue, 30 Jan 2018, Stefan Beller wrote:
> > 
> >> On Mon, Jan 29, 2018 at 2:54 PM, Johannes Schindelin
> >>  wrote:
> >>> @@ -116,6 +118,13 @@ static GIT_PATH_FUNC(rebase_path_stopped_sha, 
> >>> "rebase-merge/stopped-sha")
> >>>  static GIT_PATH_FUNC(rebase_path_rewritten_list, 
> >>> "rebase-merge/rewritten-list")
> >>>  static GIT_PATH_FUNC(rebase_path_rewritten_pending,
> >>> "rebase-merge/rewritten-pending")
> >>> +
> >>> +/*
> >>> + * The path of the file listing refs that need to be deleted after the 
> >>> rebase
> >>> + * finishes. This is used by the `merge` command.
> >>> + */
> > 
> > Whoops. The comment "This is used by the `merge` command`" is completely
> > wrong. Will fix.
> > 
> >> So this file contains (label -> commit),
> > 
> > Only `label`. No `commit`.
> > 
> >> which is appended in do_label, it uses refs to store the commits in
> >> refs/rewritten.  We do not have to worry about the contents of that file
> >> getting too long, or label re-use, because the directory containing all
> >> these helper files will be deleted upon successful rebase in
> >> `sequencer_remove_state()`.
> > 
> > Yes.
> >
> It might be a good idea to have 'git rebase --abort' delete the refs as
> well as the file though

That makes sense. I made it so.

Ciao,
Dscho

Re: [PATCH v2 02/10] sequencer: introduce new commands to reset the revision

[Johannes Schindelin]

Hi Eric,

On Tue, 30 Jan 2018, Eric Sunshine wrote:

> On Mon, Jan 29, 2018 at 5:54 PM, Johannes Schindelin
>  wrote:
> > [...]
> > This commit implements the commands to label, and to reset to, given
> > revisions. The syntax is:
> >
> > label 
> > reset 
> > [...]
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -1253,7 +1266,8 @@ static int parse_insn_line(struct todo_item *item, 
> > const char *bol, char *eol)
> > if (skip_prefix(bol, todo_command_info[i].str, )) {
> > item->command = i;
> > break;
> > -   } else if (bol[1] == ' ' && *bol == todo_command_info[i].c) 
> > {
> > +   } else if ((bol + 1 == eol || bol[1] == ' ') &&
> > +  *bol == todo_command_info[i].c) {
> 
> This adds support for commands which have no arguments, however, now
> that the "bud" command has been retired, this can go away too, right?

Good point. Fixed.

> > bol++;
> > item->command = i;
> > break;
> > @@ -1919,6 +1934,144 @@ static int do_exec(const char *command_line)
> > +static int safe_append(const char *filename, const char *fmt, ...)
> > +{
> > +   va_list ap;
> > +   struct lock_file lock = LOCK_INIT;
> > +   int fd = hold_lock_file_for_update(, filename, 0);
> > +   struct strbuf buf = STRBUF_INIT;
> > +
> > +   if (fd < 0)
> > +   return error_errno(_("could not lock '%s'"), filename);
> 
> Minor: unable_to_lock_message() can provide a more detailed
> explanation of the failure.

That is true. Due to its awkward signature (returning void, using a
strbuf), it would add a whopping 4 lines, too.

There is a better solution, though, adding only one line: passing
LOCK_REPORT_ON_ERROR as flag to hold_lock_file_for_update().

> > +
> > +   if (strbuf_read_file(, filename, 0) < 0 && errno != ENOENT)
> > +   return error_errno(_("could not read '%s'"), filename);
> > +   strbuf_complete(, '\n');
> > +   va_start(ap, fmt);
> > +   strbuf_vaddf(, fmt, ap);
> > +   va_end(ap);
> 
> Would it make sense to also
> 
> strbuf_complete(, '\n')
> 
> here, as well, to be a bit more robust against lazy callers?

I'd rather not make that assumption. It *may* be true that the current
sole user wants the last line of the file to end in a newline. I try to
design my code for maximum reusability, though. And who is to say whether
my next use case for the safe_append() function wants the semantics you
suggest, if it wants to append less than entire lines at a time, maybe?
Let's not optimize prematurely, okay?

> > +
> > +   if (write_in_full(fd, buf.buf, buf.len) < 0) {
> > +   rollback_lock_file();
> > +   return error_errno(_("could not write to '%s'"), filename);
> 
> Reading lockfile.h & tempfile.c, I see that rollback_lock_file()
> clobbers write_in_full()'s errno before error_errno() is called.

True. Fixed.

I also fixed the code from where I copy-edited this pattern (increasing
the patch series by yet another patch).

> > +   }
> > +   if (commit_lock_file() < 0) {
> > +   rollback_lock_file();
> > +   return error(_("failed to finalize '%s'"), filename);
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static int do_reset(const char *name, int len)
> > +{
> > +   [...]
> > +   strbuf_addf(_name, "refs/rewritten/%.*s", len, name);
> > +   if (get_oid(ref_name.buf, ) &&
> > +   get_oid(ref_name.buf + strlen("refs/rewritten/"), )) {
> > +   error(_("could not read '%s'"), ref_name.buf);
> 
> Checking my understanding: The two get_oid() calls allow the argument
> to 'reset' to be a label created with the 'label' command or any other
> way to name an object, right? If so, then I wonder if the error
> invocation should instead be:
> 
> error(_("could not read '%.*s'"), len, name);

I would rather give the preferred form: refs/rewritten/.

The main reason this code falls back to getting the OID of ``
directly is to support the `no-rebase-cousins` code: in that mode, topic
branches may be based on commits other than the one labeled `onto`, but
the original, unchanged one. In this case, we have no way of labeling the
base commit, and therefore use a unique abbreviation of that base commit's
OID.

But this is really a very special use case, and the more common use case
should be the one using refs/rewritten/.

Ciao,
Dscho

Question about rebasing - unexpected result, can't figure out why

[Jonas Thiem]

Hi *,

I did some experimenting to understand rebasing better. I'm trying to
follow ProGit 2nd edition. I used the repository example from here:

https://git-scm.com/book/en/v2/Git-Branching-Rebasing#rbdiag_e

I did a short test setup and did a few rebases, expecting a certain
result from my limited understanding of rebases. Sadly, I got something
else, and I can't figure out why.

== Setup ==

The repository is present in the attached archive in folder:
./rebase-experiment/repo-before-rebases/

The graph is as in the linked book example above.
I set up the commits (named same as in the book) in the following way:

C1: adds file C1.txt
C2: adds file C2.txt
C3: adds file C3.txt
C4: adds file C4.txt
C5: adds file C5.txt
C6: adds file C6.txt
C8: deletes file C3.txt (following C3 which added it)
C9: adds file C9.txt
C10: adds file C10.txt

Then I did the following rebases:

git checkout client && git rebase master
git checkout master && git merge client
git checkout server && git rebase master
git checkout master && git merge server

The resulting repository is present in the attached archive in folder:
./rebase-experiment/repo-after-rebases/

== Expected result ==

The result I expected on the master branch after the rebases was:

C1 -> c2 -> C3 -> C8 -> C9 -> C3 -> C4 -> C10 (with C3.txt present /
readded at the end)

The actual result present in ./rebase-experiment/repo-after-rebases/ is:

C1 -> c2 -> C3 -> C8 -> C9 -> C4 -> C10 (with C3.txt not present / not
readded at the end)

== Why did I expect that ==

Of course after the client rebase, C3.txt should be gone (since it's
gone at the original last commit of the client branch).

But since it still exists in the server branch at the final commit,
after rebasing & reapplying that onto the master, shouldn't it be put
back in? Also, I would expect C3 -> C4 -> C10 as the complete chain of
commits of the remaining server branch to be attached at the end, not
just C4 -> C10...

Does git somehow detect C3 would be applied twice? Doesn't the commit
hash of C3 change after it is rebased? So how exactly would it figure
that out? I'm really unsure about what is going on.

Regards,
Jonas Thiem


rebase-experiment.tar.gz
Description: application/gzip

[no subject]

[Emile Kenold]

I need a good and honest person to claim the sum of seven million
British Pound which i inherited from my late husband. My name is Mrs.
Emile Kenold, presently i am in a very critical health condition in
which I sleep every night without knowing my fate day by day. I am a
widow suffering from long time cancer.

I decided to entrust this fund to a very honest and God fearing person
that will use it for Charity works, orphanages, widows and also build
schools for less privileged ones that will be named after my late
husband if possible.

I took this decision because I do not have any child who will inherit
this money after I die. Please I want your sincere and urgent answer
to know if you will be able to execute this project, and I will give
you more information on how the fund will be transferred to your bank
account. I am waiting for your reply.

Re: [RFC PATCH 3/3] fsck: Check HEAD of other worktrees as well

[Junio C Hamano]

Duy Nguyen  writes:

> On Fri, Feb 09, 2018 at 03:13:30PM -0800, Elijah Newren wrote:
>> This takes advantage of the fact that "worktrees/$WORKTREE/HEAD" will
>> currently resolve as a ref, which may not be true in the future with
>> different ref backends.  I don't think it locks us in to supporting
>> resolving other worktree HEADs with this syntax, as I view the
>> user-visible error message as more of a pointer to a pathname that the
>> user will need to fix.  If the underlying ref storage changes, naturally
>> both this code and the hint may need to change to match.
>
> I'm leaning more about something like this patch below (which does not
> even build, just to demonstrate). A couple points:
>
> - Instead of doing the hacky refs worktrees/foo/HEAD, we get the
>   correct ref store for each worktree
> - We have an API for getting all (non-broken) worktrees. I realize for
>   fsck, we may even want to examine semi-broken worktrees as well, but
>   get_worktrees() can take a flag to accomplish that if needed.

Very sensible.  

When I saw that "fsck" thing, the first thing I wondered was "wait,
how are we doing prune and repack while making sure objects
reachable only from HEAD in other worktrees are not lost?  we must
already have an API that gives us where the refs of them are stored
in".

Thanks for a quick response for course correction.

Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)

[Junio C Hamano]

Duy Nguyen  writes:

>> Yup, this is about giving summary in a compact way, not about giving
>> a compact stat information.  I agree with all the above reasoning,
>> and that is why I said that your "compact-summary" was a good way to
>> refer to the feature.
>
> OK I'll wait for a few days. If there's no comment, I'll go with
> --stat=compact-summary.

Sorry, but that is not what I agreed with you on ;-) I meant to say
that your earlier --compact-summary made sense.  I do not think
"compact summary" as a value of "--stat" makes any sense.  It's not
like this new one is one of the ways we offer to present "stat"
differently and compared to other existing ways this is to give the
stat in a compactly summarized fashion.  If this were a value to
"--summary", then "--summary=in-stat" might make sense, in that this
is not about how we show the "stat" information but about how/where
"summary" information is shown.

Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Johannes Schindelin]

Hi Junio,

On Tue, 23 Jan 2018, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> > index 8a861c1e0d6..1d061373288 100644
> > --- a/Documentation/git-rebase.txt
> > +++ b/Documentation/git-rebase.txt
> > @@ -368,6 +368,11 @@ The commit list format can be changed by setting the 
> > configuration option
> >  rebase.instructionFormat.  A customized instruction format will 
> > automatically
> >  have the long commit hash prepended to the format.
> >  
> > +--recreate-merges::
> > +   Recreate merge commits instead of flattening the history by replaying
> > +   merges. Merge conflict resolutions or manual amendments to merge
> > +   commits are not preserved.
> > +
> 
> It is sensible to postpone tackling "evil merges" in this initial
> iteration of the series, and "manual amendments ... not preserved"
> is a reasonable thing to document.  But do we want to say a bit more
> about conflicting merges?  "conflict resolutions ... not preserved"
> sounds as if it does not stop and instead record the result with
> conflict markers without even letting rerere to kick in, which
> certainly is not the impression you wanted to give to the readers.
> 
> I am imagining that it will stop and give control back to the end
> user just like a conflicted "pick" would, and allow "rebase
> --continue" to record resolution from the working tree, and just
> like conflicted "pick", it would allow rerere() to help end users
> recall previous resolution.

This is my current version:

--recreate-merges[=(rebase-cousins|no-rebase-cousins)]::
Recreate merge commits instead of flattening the history by replaying
merges. Merge conflict resolutions or manual amendments to merge
commits are not recreated automatically, but have to be recreated
manually.

Ciao,
Dscho

Re: Fetch-hooks

[Leo Gaspard]

On 02/10/2018 01:21 PM, Jeff King wrote:
> On Sat, Feb 10, 2018 at 01:37:20AM +0100, Leo Gaspard wrote:
> 
>>> Yeah, tag-following may be a little tricky, because it usually wants to
>>> write to refs/tags/. One workaround would be to have your config look
>>> like this:
>>>
>>>   [remote "origin"]
>>>   fetch = +refs/heads/*:refs/quarantine/origin/heads/*
>>>   fetch = +refs/tags/*:refs/quarantine/origin/tags/*
>>>   tagOpt = --no-tags
>>>
>>> That's not exactly the same thing, because it would fetch all tags, not
>>> just those that point to the history on the branches. But in most
>>> repositories and workflows the distinction doesn't matter.
>>
>> Hmm... apart from the implementation complexity (of which I have no
>> idea), is there an argument against the idea of adding a
>> remote..fetchTagsTo refmap similar to remote..fetch but used
>> every time a tag is fetched? (well, maybe not exactly similar to
>> remote..fetch because we know the source is going to be
>> refs/tags/*; so just having the part of .fetch past the ':' would be
>> more like what's needed I guess)
> 
> I don't think it would be too hard to implement, and is at least
> logically consistent with the way we handle tags.
> 
> If we were designing from scratch, I do think this would all be helped
> immensely by having more separation of refs fetched from remotes. There
> was a proposal in the v1.8 era to fetch everything for a remote,
> including tags, into "refs/remotes/origin/heads/",
> "refs/remotes/origin/tags/", etc. And then we'd teach the lookup side to
> look for tags in each of the remote-tag namespaces.
> 
> I still think that would be a good direction to go, but it would be a
> big project (which is why the original stalled).

Hmm... would this also drown the remote..fetch map? Also, I think
this behavior could be emulated with fetch and fetchTagsTo, and it would
look like:
[remote "my-remote"]
fetch = +refs/heads/*:refs/remotes/my-remote/heads/*
fetchTagsTo = refs/remotes/my-remote/tags/*
The remaining issue being to teach the lookup side to look for tags in
all the remote-tag namespaces (and the fact it's a breaking change).

That said, actually I just noticed an issue in the “add a
remote..fetch option to fetch to refs/quarantine then have the
post-fetch hook do the work”: it means if I run `git pull`, then:
 1. The remote references will be pulled to refs/quarantine/...
 2. The post-fetch hook will copy the accepted ones to refs/remotes/...
 3. The `git merge FETCH_HEAD` called by pull will merge FETCH_HEAD into
local branches... and so merge from refs/quarantine.

A solution would be to also update FETCH_HEAD in the post-fetch hook,
but then we're back to the issue raised by Junio after the “*HOWEVER*”
of [1]: the hook writer has to maintain consistency between the “copied”
references and FETCH_HEAD.

So, when thinking about it, I'm back to thinking the proper hook
interface should be the one of the tweak-fetch hook, but its
implementation should make it not go crazy on remote servers. And so
that the implementation should do all this refs/quarantine wizardry
inside git itself.

So basically what I'm getting at at the moment is that git fetch should:
 1. fetch all the references to refs/real-remotes//{insert here a
copy of the refs/ tree of }
 2. figure out what the “expected” names for these references will by,
by looking at remote..fetch (and at whether --tags was passed)
 3. for each “expected” name,
 1. if a tweak-fetch hook is present, call it with the
refs/real-remotes/... refname and the “expected end-name” from
remote..fetch
 2. based on the hook's result, potentially to move the “expected
end-name” to another commit than the one referenced by refs/real-remotes/...
 3. move the “expected” name to the commit referenced in
refs/real-remotes

Which would make the tweak-fetch hook interface simpler (though more
restrictive, but I don't have any real use case for the other change
possibilities) than it is now:
 1. feed the hook with lines of
“refs/real-remotes/my-remote/heads/my-branch
refs/remotes/my-remote/my-branch” (assuming remote.my-remote.fetch is
“normal”) or “refs/real-remotes/my-remote/tags/my-tag refs/tags/my-tag”
(if my-tag is being fetched from my-remote)
 2. read lines of “ refs/remotes/my-remote/my-branch”, that
will re-point my-branch to  instead of
refs/real-remotes/my-remote/heads/my-branch. In order to avoid any
issue, the hook is not allowed to pass as second output a reference that
was not passed as second input.

This way, the behavior of the tweak-fetch hook is reasonably preserved
(at least for my use case), and there is no additional load on the
servers thanks to the up-to-date references being stored in
refs/real-remotes//

Does this reasoning make any sense?


[1] https://marc.info/?l=git=132478296309094=2

Re: Fetch-hooks

[Leo Gaspard]

On 02/10/2018 02:33 AM, Leo Gaspard wrote:> I guess the very early part
of the discussion you're speaking of is what
> I was assuming after reading
> https://marc.info/?l=git=132478296309094=2
> 
> [...]
> 
> So the reason for a post-fetch in my opinion is the same as for a
> pre-push / pre-commit: not changing the user's workflow, while providing
> additional features.
Well, re-reading my email, it looks aggressive to me now... sorry about
that!

What I meant was basically, that in my mind pre-push or pre-commit are
also local-only things, and they are really useful in that they allow to
block the push or the commit if some conditions are not met (eg. block
commit with trailing whitespace, or block push of unsigned commits).

In pretty much the same way, what I'm really looking for is a way to
“block the fetch” (from a user-visible standpoint) -- like pre-push but
in the opposite direction. I hope such a goal is not an anti-pattern for
hook design?

In order to do this, I first tried updating this tweak-fetch hook patch
from late 2011, and then learned that it would cause too high a load on
servers. Then, while brainstorming another solution, this idea of a
notification-only post-fetch hook arose. But, as I noticed while writing
my previous answer, this suffers the same
the-hook-writer-must-correctly-update-FETCH_HEAD-concurrently-with-remote-branch
issue that you pointed out just after the “*HOWEVER*” in [1]. So that
solution is likely a bad solution too. I guess we'll continue the search
for a good solution in the side-thread with Peff, hoping to figure one out.

That being said about what I meant in my last email, obviously you're
the one who has the say on what goes in or not! And if it's an
anti-feature I'd much rather know it now than after spending a few
nights coding :)

So, what do you think about this use case I'm thinking of? (ie.
“blocking the fetch” like pre-push “blocks the push” and pre-commit
“blocks the commit”?)


[1] https://marc.info/?l=git=132478296309094=2

Re: [PATCH] docs/interpret-trailers: fix agreement error

[brian m. carlson]

On Thu, Feb 08, 2018 at 12:29:53PM -0800, Junio C Hamano wrote:
> Jonathan Tan  writes:
> 
> > On Thu,  8 Feb 2018 02:56:14 +
> > "brian m. carlson"  wrote:
> >
> >>  Existing trailers are extracted from the input message by looking for
> >> -a group of one or more lines that (i) are all trailers, or (ii) contains 
> >> at
> >> -least one Git-generated or user-configured trailer and consists of at
> >> +a group of one or more lines that (i) are all trailers, or (ii) contain at
> >> +least one Git-generated or user-configured trailer and consist of at
> >>  least 25% trailers.
> >>  The group must be preceded by one or more empty (or whitespace-only) 
> >> lines.
> >>  The group must either be at the end of the message or be the last
> >
> > Ah, good catch. Maybe "a group of one or more lines that (i) consists of all
> > trailers, or (ii) contains ..."?
> 
> Your version reads better perhaps because it talks about "a group"
> without placing undue stress on the fact that the member of the
> group are usually multiple---I guess it is better over Brian's?

I'm happy to make the change to be all singular instead.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: Crash when clone includes magic filenames on Windows

[Philip Oakley]

From: "Philip Oakley" 

From: "Jeffrey Walton" 

Hi Everyone,

I'm seeing this issue on Windows: https://pastebin.com/YfB25E4T . It
seems the filename AUX is the culprit. Also see
https://blogs.msdn.microsoft.com/oldnewthing/20031022-00/?p=42073 .
(Thanks to Milleneumbug on Stack Overflow).

I did not name the file, someone else did. I doubt the filename will be
changed.

Searching is not turning up much information:
https://www.google.com/search?q=git+"magic+filenames"+windows

Does anyone know how to sidestep the issue on Windows?

Jeff


This comes up on the Git-for-Windows (GfW) issues fairly often
https://github.com/git-for-windows/git/issues.

The fetch part of the clone is sucessful, but the final checkout step
fails when the AUX (or any other prohibited filename - that's proper
cabkward compatibility for you) is to be checked out then the file system
(FS) refuses and the checkout 'fails. You do however have the full repo
locally.

The trick is probably then to set up a sparse checkout so the AUX is never
included on the FS.

However it is an open 'up-for-grabs' project to add such a check in GfW.

Philip

One option maybe to extend the $GIT_DIR/info/sparse-checkout capability and
add a specific $GIT_DIR/info/never-sparse-checkout file that could carry the
complement (files & dirs) options that are platform applicable (no AUX, no
COM1, no colons, etc.;-), so that it does not conflict with the users'
regular sparse checkout selection in $GIT_DIR/info/sparse-checkout. It's
probably easier to understand that way.
--
Philip

[PATCH] check-ignore: fix mix of directories and other file types

[René Scharfe]

In check_ignore(), the first pathspec item determines the dtype for any
subsequent ones.  That means that a pathspec matching a regular file can
prevent following pathspecs from matching directories, which makes no
sense.  Fix that by determining the dtype for each pathspec separately,
by passing the value DT_UNKNOWN to last_exclude_matching() each time.

Signed-off-by: Rene Scharfe 
---
 builtin/check-ignore.c |  3 ++-
 t/t0008-ignores.sh | 20 
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 3e280b9c7a..ec9a959e08 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -72,7 +72,7 @@ static int check_ignore(struct dir_struct *dir,
 {
const char *full_path;
char *seen;
-   int num_ignored = 0, dtype = DT_UNKNOWN, i;
+   int num_ignored = 0, i;
struct exclude *exclude;
struct pathspec pathspec;
 
@@ -104,6 +104,7 @@ static int check_ignore(struct dir_struct *dir,
full_path = pathspec.items[i].match;
exclude = NULL;
if (!seen[i]) {
+   int dtype = DT_UNKNOWN;
exclude = last_exclude_matching(dir, _index,
full_path, );
}
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index d27f438bf4..54a4703ef1 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -775,6 +775,26 @@ test_expect_success PIPE 'streaming support for --stdin' '
echo "$response" | grep "^::two"
 '
 
+test_expect_success 'existing file and directory' '
+   test_when_finished "rm one" &&
+   test_when_finished "rmdir top-level-dir" &&
+   >one &&
+   mkdir top-level-dir &&
+   git check-ignore one top-level-dir >actual &&
+   grep one actual &&
+   grep top-level-dir actual
+'
+
+test_expect_success 'existing directory and file' '
+   test_when_finished "rm one" &&
+   test_when_finished "rmdir top-level-dir" &&
+   >one &&
+   mkdir top-level-dir &&
+   git check-ignore top-level-dir one >actual &&
+   grep one actual &&
+   grep top-level-dir actual
+'
+
 
 #
 # test whitespace handling
-- 
2.16.1

Re: [RFC PATCH 3/3] fsck: Check HEAD of other worktrees as well

[Jeff King]

On Sat, Feb 10, 2018 at 04:59:52PM +0700, Duy Nguyen wrote:

> On Fri, Feb 09, 2018 at 03:13:30PM -0800, Elijah Newren wrote:
> > This takes advantage of the fact that "worktrees/$WORKTREE/HEAD" will
> > currently resolve as a ref, which may not be true in the future with
> > different ref backends.  I don't think it locks us in to supporting
> > resolving other worktree HEADs with this syntax, as I view the
> > user-visible error message as more of a pointer to a pathname that the
> > user will need to fix.  If the underlying ref storage changes, naturally
> > both this code and the hint may need to change to match.
> 
> I'm leaning more about something like this patch below (which does not
> even build, just to demonstrate). A couple points:
> 
> - Instead of doing the hacky refs worktrees/foo/HEAD, we get the
>   correct ref store for each worktree
> - We have an API for getting all (non-broken) worktrees. I realize for
>   fsck, we may even want to examine semi-broken worktrees as well, but
>   get_worktrees() can take a flag to accomplish that if needed.
> - As you can see, I print %p from the ref store instead of something
>   human friendly. This is something I was stuck at last time. I need
>   better ref store description (or even the worktree name)

Yeah, I think this is the right approach. We know about worktrees
internally, and we should be able to iterate over their ref stores, even
if we have no way to succinctly name the resulting ref.

> - This ref_name() function makes me think if we should have an
>   extended sha1 syntax for accessing per-worktree refs from a
>   different worktree, something like HEAD@{worktree:foo} to resolve to
>   worktrees/foo/HEAD. Then we have a better description here that can
>   actually be used from command line, as a regular ref, if needed.

Yeah, I agree this would be very useful. I peeked at how bad it would be
to hanlde this. The parsing is pretty easy in get_oid_basic(), but you'd
have to plumb through the ref_store in quite a few places:

 - interpret_nth_prior_checkout() would probably want to use the
   worktree's HEAD (for "@{worktree:foo}@{-1}")

 - dwim_ref() would need to know about the ref store

 - that implies that substitute_branch_name() knows about it, too

So it may turn into a big job. But I think it's moving in the right
direction. And it may not be the end of the world if all features do not
work from day one (e.g., if HEAD@{worktree:foo} works, but
HEAD@{worktree:foo}@{upstream} does not yet, with plans to eventually
make that work).

-Peff

Re: Fetch-hooks

[Jeff King]

On Sat, Feb 10, 2018 at 01:37:20AM +0100, Leo Gaspard wrote:

> > Yeah, tag-following may be a little tricky, because it usually wants to
> > write to refs/tags/. One workaround would be to have your config look
> > like this:
> > 
> >   [remote "origin"]
> >   fetch = +refs/heads/*:refs/quarantine/origin/heads/*
> >   fetch = +refs/tags/*:refs/quarantine/origin/tags/*
> >   tagOpt = --no-tags
> > 
> > That's not exactly the same thing, because it would fetch all tags, not
> > just those that point to the history on the branches. But in most
> > repositories and workflows the distinction doesn't matter.
> 
> Hmm... apart from the implementation complexity (of which I have no
> idea), is there an argument against the idea of adding a
> remote..fetchTagsTo refmap similar to remote..fetch but used
> every time a tag is fetched? (well, maybe not exactly similar to
> remote..fetch because we know the source is going to be
> refs/tags/*; so just having the part of .fetch past the ':' would be
> more like what's needed I guess)

I don't think it would be too hard to implement, and is at least
logically consistent with the way we handle tags.

If we were designing from scratch, I do think this would all be helped
immensely by having more separation of refs fetched from remotes. There
was a proposal in the v1.8 era to fetch everything for a remote,
including tags, into "refs/remotes/origin/heads/",
"refs/remotes/origin/tags/", etc. And then we'd teach the lookup side to
look for tags in each of the remote-tag namespaces.

I still think that would be a good direction to go, but it would be a
big project (which is why the original stalled).

> The issue with your solution is that if the user runs 'git fetch
> --tags', he will get the (potentially compromised) tags directly in his
> refs/tags/.

True.

-Peff

Re: Bug? Error during commit

[Jeff King]

On Sat, Feb 10, 2018 at 05:21:05PM +0700, Duy Nguyen wrote:

> > But it doesn't seem to work:
> >
> >   $ yes | head -c $((1024*1024*1024 - 10*1024*1024)) >file
> >   $ git add file
> >   $ git commit -m one
> >   $ yes | head -c $((1024*1024*1024)) >file
> >   $ git commit -am two
> >   fatal: unable to generate diffstat for file
> >
> > What's weird is that if I run "git show --stat" on the same commit, it
> > works! So there's something about how commit invokes the diff that
> > doesn't let the big-file check kick in.
> 
> I have a flashback about checking big_file_threshold in this code. I
> vaguely remember doing something in this code but not sure if it was
> merged.
> 
> I finally found 6bf3b81348 (diff --stat: mark any file larger than
> core.bigfilethreshold binary - 2014-08-16) so it's merged after all,
> but this commit is about --stat apparently ;-)

The confounding factor here is actually the break-detection that
"commit" does. After running the "commit" above (which does succeed
despite the "fatal", since that happens only as part of the diff summary
we print), you can replicate the problem with "git show -B --stat".

The break-detection code unconditionally loads the file data. Then when
the stat code later checks whether it's binary, it just uses the data
as-is. So that leads me to a few questions / lines of thought:

  1. When we're checking binary-ness, we shouldn't assume if the data
 is loaded that the size is sane. We should check it against
 big_file_threshold.

  2. Should break detection (and probably inexact rename detection) skip
 files that are over big_file_threshold?

 I think our code is capable of actually performing these operations
 on large files (at least on systems with 64-bit ulong; I'd be
 willing to bet you get funny results due to integer overflow on
 32-bit systems or on 64-bit Windows). But I'm not sure it's doing
 anybody any good. And it's way faster not to. For example, here's
 "git show" before and after the patch below (running on the repo
 from my example):

  $ time git show --oneline -B --stat | cat
  fatal: unable to generate diffstat for file
  883cbdc two
  
  real  0m10.153s
  user  0m9.929s
  sys   0m0.224s
  
  peff@sigill:~/tmp [master]
  $ time git.compile show --oneline -B --stat | cat
  883cbdc two
   file | Bin 1063256064 -> 1073741824 bytes
   1 file changed, 0 insertions(+), 0 deletions(-)
  
  real  0m0.008s
  user  0m0.002s
  sys   0m0.010s

 We can produce the useful answer in a fraction of the time, since
 we don't even need to load the blob content. The downside is that
 in theory we could break-detect these, and then find renames. So I
 guess it comes down to the philosophy of core.bigfilethreshold: how
 much effort are we willing to put into such files on the off chance
 that they produce a useful output?

 If we go this route, then in theory the fix in (1) becomes
 redundant, though I'd probably do both just as a
 belt-and-suspenders thing.

  3. To what degree should we override the user's config to treat these
 files as binary. If I set core.bigfilethreshold to "10G", or if I
 use gitattributes to mark the file as non-binary, then we're still
 going to feed it to xdiff (which is going to choke and die).

 Should we override these when we approach MAX_DIFF_SIZE, and just
 treat the file as binary? Should we barf and tell the user to fix
 their config?

 I guess I argued for overriding attributes earlier, and that
 probably ought to be independent of core.bigfilethreshold. Perhaps
 we should issue a warning in that case, to let the user know their
 config is nonsense.

> > I think we probably ought to consider anything over big_file_threshold
> > to be binary, no matter what. Possibly even if the user gave us a
> > .gitattribute that says "no really, this is text". Because that 1GB
> > limit is a hard limit that the code can't cope with; our options are
> > either to generate a binary diff or to die.
> 
> Agreed. While at there perhaps we need to improve this "unable to
> generate diffstat" message a bit too.

One reason the message is so vague is that we don't actually have any
kind of error code. Though really the only reason for xdiff to fail is
due to file size. So we could perhaps offer some advice there. But if we
do all the things I suggested above, then we'd automatically handle all
the cases we know about.

so my hope was that we would make it impossible to trigger this message.
In which case it maybe ought to be a BUG(). :)

Here's the patch to make "show -B --stat" work. I'll give some more
thought to whether this is a good idea and prepare a series.

One downside is that in the common case it causes us to look up each
object twice (once to get its size, and then again to load 

[PATCH] t0002: simplify error checking

[Jeff King]

On Fri, Feb 09, 2018 at 02:30:39PM -0500, Jeff King wrote:

> Yes, I think so, but we may want to avoid this anti-pattern (since
> usually "! test_i18ngrep" is a sign of something wrong. It seems like
> these tests are doing more manual reporting work than is necessary, and
> could just be relying on helpers to report errors.
> 
> Something like the patch below, though I'm not sure if we'd want to
> leave it as "grep" (if applying on master), or have "test_i18ngrep" in
> the preimage (if basing on top of Alexander's patch).

Here's a version suitable for applying to master as an independent
cleanup. It will conflict with Alexander's patch, but the resolution is
pretty easy (take my side, but s/grep/test_i18ngrep/). I'm happy to do
it on top of his if that's easier.

-- >8 --
Subject: [PATCH] t0002: simplify error checking

This ancient test script does a lot of manual checking of
test conditions with "if" blocks. We can simplify this
by relying on helpers like test_must_fail.

Note that a failing "grep" call here won't produce any
verbose output, but that's OK. These days we rely on "-x" to
tell us about such commands. And in addition, these greps
are soon to be converted to test_i18ngrep (which is itself
soon learning to be more verbose).

Signed-off-by: Jeff King 
---
 t/t0002-gitfile.sh | 53 +-
 1 file changed, 10 insertions(+), 43 deletions(-)

diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index 9670e8cbe6..fb8d094117 100755
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -10,15 +10,6 @@ objpath() {
echo "$1" | sed -e 's|\(..\)|\1/|'
 }
 
-objck() {
-   p=$(objpath "$1")
-   if test ! -f "$REAL/objects/$p"
-   then
-   echo "Object not found: $REAL/objects/$p"
-   false
-   fi
-}
-
 test_expect_success 'initial setup' '
REAL="$(pwd)/.real" &&
mv .git "$REAL"
@@ -26,30 +17,14 @@ test_expect_success 'initial setup' '
 
 test_expect_success 'bad setup: invalid .git file format' '
echo "gitdir $REAL" >.git &&
-   if git rev-parse 2>.err
-   then
-   echo "git rev-parse accepted an invalid .git file"
-   false
-   fi &&
-   if ! grep "Invalid gitfile format" .err
-   then
-   echo "git rev-parse returned wrong error"
-   false
-   fi
+   test_must_fail git rev-parse 2>.err &&
+   grep "Invalid gitfile format" .err
 '
 
 test_expect_success 'bad setup: invalid .git file path' '
echo "gitdir: $REAL.not" >.git &&
-   if git rev-parse 2>.err
-   then
-   echo "git rev-parse accepted an invalid .git file path"
-   false
-   fi &&
-   if ! grep "Not a git repository" .err
-   then
-   echo "git rev-parse returned wrong error"
-   false
-   fi
+   test_must_fail git rev-parse 2>.err &&
+   grep "Not a git repository" .err
 '
 
 test_expect_success 'final setup + check rev-parse --git-dir' '
@@ -60,7 +35,7 @@ test_expect_success 'final setup + check rev-parse --git-dir' 
'
 test_expect_success 'check hash-object' '
echo "foo" >bar &&
SHA=$(cat bar | git hash-object -w --stdin) &&
-   objck $SHA
+   test_path_is_file "$REAL/objects/$(objpath $SHA)"
 '
 
 test_expect_success 'check cat-file' '
@@ -69,29 +44,21 @@ test_expect_success 'check cat-file' '
 '
 
 test_expect_success 'check update-index' '
-   if test -f "$REAL/index"
-   then
-   echo "Hmm, $REAL/index exists?"
-   false
-   fi &&
+   test_path_is_missing "$REAL/index" &&
rm -f "$REAL/objects/$(objpath $SHA)" &&
git update-index --add bar &&
-   if ! test -f "$REAL/index"
-   then
-   echo "$REAL/index not found"
-   false
-   fi &&
-   objck $SHA
+   test_path_is_file "$REAL/index" &&
+   test_path_is_file "$REAL/objects/$(objpath $SHA)"
 '
 
 test_expect_success 'check write-tree' '
SHA=$(git write-tree) &&
-   objck $SHA
+   test_path_is_file "$REAL/objects/$(objpath $SHA)"
 '
 
 test_expect_success 'check commit-tree' '
SHA=$(echo "commit bar" | git commit-tree $SHA) &&
-   objck $SHA
+   test_path_is_file "$REAL/objects/$(objpath $SHA)"
 '
 
 test_expect_success 'check rev-list' '
-- 
2.16.1.464.gc4bae515b7

Re: [PATCH v2 1/3] worktree: improve message when creating a new worktree

[Duy Nguyen]

On Fri, Feb 9, 2018 at 7:08 PM, Duy Nguyen  wrote:
> On Fri, Feb 9, 2018 at 6:27 PM, Thomas Gummerer  wrote:
>> This would loose the information about the identifier of the worktree,
>> but from a coarse look at the man page it doesn't seem like we
>> advertise that widely
>>
>> ...
>>
>> So given that maybe it would even be better to hide the part about the
>> identifier, as it seems more like an implementation detail than
>> relevant to the end user?
>
> Exactly. I'd rather hide it. I haven't found any good reason that a
> user needs to know these IDs unless they poke into $GIT_DIR/worktrees,
> but they should not need to.

Just FYI. I mentioned elsewhere [1] the possibility of a new extended
ref syntax to refer to e.g. HEAD from a different worktree. Such
syntax may make use of this id (which also means we might need to give
the user control over these ids and not just always auto generate
them).

But that's for the future. If/When that thing comes, we can worry
about displaying id here then. For now I still think it's ok to hide
it.

[1] https://public-inbox.org/git/20180210095952.GA9035@ash/T/#u
-- 
Duy

Re: [RFC PATCH 000/194] Moving global state into the repository object

[Duy Nguyen]

On Thu, Feb 8, 2018 at 1:06 AM, Stefan Beller  wrote:
>> Something else.. maybe "struct repository *" should not be a universal
>> function argument to avoid global states. Like sha1_file.c is mostly about 
>> the
>> object store, and I see that you added object store struct (nice!).
>> These sha1 related function should take the object_store* (which
>> probably is a combination of both packed and loose stores since they
>> depend on each other), not repository*. This way if a function needs
>> both access to object store and ref store, we can see the two
>> dependencies from function arguments.
>
> I tried that in the beginning and it blew up a couple of times when I realized
> that I forgot to pass through one of these dependencies.
> Maybe we can go to the repository and shrink the scope in a follow up?

I think it's a good thing to do. We need to make these implicit
dependencies explicit sooner or later.

Also, perhaps at the earlier steps you don't need to add everything to
the_respository yet. You can have the_object_store, the_parsed_object
(and we already have get_main_ref_store()), then use them internally
without touching the API, which helps reduce code change. For example,
read_sha1_file() could be converted to take "struct object_store *"
all the way

void *read_sha1_file_extended(const unsigned char *sha1,
  enum object_type *type,
  unsigned long *size,
  int lookup_replace)
{
struct object_store *store = _object_store;
...
// more access to "store"
}

When every piece is in place, the API change can be made be removing
that "store = _object_store" line and make "store" an argument of
read_sha1_file().
-- 
Duy

Re: Crash when clone includes magic filenames on Windows

[Philip Oakley]

From: "Jeffrey Walton" 

Hi Everyone,

I'm seeing this issue on Windows: https://pastebin.com/YfB25E4T . It
seems the filename AUX is the culprit. Also see
https://blogs.msdn.microsoft.com/oldnewthing/20031022-00/?p=42073 .
(Thanks to Milleneumbug on Stack Overflow).

I did not name the file, someone else did. I doubt the filename will be 
changed.


Searching is not turning up much information:
https://www.google.com/search?q=git+"magic+filenames"+windows

Does anyone know how to sidestep the issue on Windows?

Jeff

This comes up on the Git-for-Windows (GfW) issues fairly often 
https://github.com/git-for-windows/git/issues.


The fetch part of the clone is sucessful, but the final checkout step fails 
when the AUX (or any other prohibited filename - that's proper cabkward 
compatibility for you) is to be checked out then the file system (FS) 
refuses and the checkout 'fails. You do however have the full repo locally.


The trick is probably then to set up a sparse checkout so the AUX is never 
included on the FS.


However it is an open 'up-for-grabs' project to add such a check in GfW.

Philip 

Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)

[Duy Nguyen]

On Thu, Feb 8, 2018 at 1:19 AM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> ...
>> Then we still need to decide the new keyword for this feature, I feel
>> compact is a bit too vague (I read --stat=compact as "it's compact
>> stat", not "stat with compact summary"), so perhaps
>> --stat=compact-summary, or just --stat=summary?
>
> Yup, this is about giving summary in a compact way, not about giving
> a compact stat information.  I agree with all the above reasoning,
> and that is why I said that your "compact-summary" was a good way to
> refer to the feature.

OK I'll wait for a few days. If there's no comment, I'll go with
--stat=compact-summary.
-- 
Duy

Re: [PATCH v3 22/35] upload-pack: support shallow requests

[Duy Nguyen]

On Thu, Feb 8, 2018 at 2:00 AM, Stefan Beller  wrote:
>> +
>> +deepen-relative
>> +   Requests that the semantics of the "deepen" command be changed
>> +   to indicate that the depth requested is relative to the clients
>> +   current shallow boundary, instead of relative to the remote
>> +   refs.
>> +
>> +deepen-since 
>> +   Requests that the shallow clone/fetch should be cut at a
>> +   specific time, instead of depth.  Internally it's equivalent of
>> +   doing "rev-list --max-age=". Cannot be used with
>> +   "deepen".
>> +
>> +deepen-not 
>> +   Requests that the shallow clone/fetch should be cut at a
>> +   specific revision specified by '', instead of a depth.
>> +   Internally it's equivalent of doing "rev-list --not ".
>> +   Cannot be used with "deepen", but can be used with
>> +   "deepen-since".
>
> What happens if those are given in combination?

It should be described in the old protocol document or I did a bad job
documenting it. Some of these can be combined (I think it's AND logic
from rev-list point of view), with the exception of --depth which does
not use rev-list underneath and cannot be combined with the others.
-- 
Duy

Re: Bug? Error during commit

[Duy Nguyen]

On Thu, Feb 8, 2018 at 3:45 AM, Jeff King  wrote:
> On Mon, Feb 05, 2018 at 08:59:52PM +0700, Duy Nguyen wrote:
>
>> On Mon, Feb 5, 2018 at 8:48 PM, Andreas Kalz  wrote:
>> > Hello,
>> >
>> > I am using git frequently and usually it is running great.
>> >
>> > I read to write to this eMail address regarding problems and possible bugs.
>> > I am using git version 2.16.1.windows.2 / 64 Bit and during commit the 
>> > following error message comes up:
>> > e:\Internet>git commit -m 2018-01-27
>> > fatal: unable to generate diffstat for 
>> > Thunderbird/andreas-kalz.de/Mail/pop.gmx.net/Inbox
>> > [master f74cf30] 2018-01-27
>> >
>> > I also tried this before with an older git version with same problem.
>> >
>> > Can you help me with this problem please? Thanks in advance.
>>
>> I think if you add -q to that "git commit" command, diffstat is not
>> generated and you can get past that. If that particular commit can be
>> published in public, it'll help us find out why diffstat could not be
>> generated.
>
> I think that's the first time I've seen that particular error. :)
>
> I think the only reason that xdiff would report failure is if malloc()
> failed, or if one of the files exceeds MAX_XDIFF_SIZE, which is ~1GB.
> I think we'd usually avoid doing a text diff on anything over
> core.bigFileThreshold, though.
>
> But it doesn't seem to work:
>
>   $ yes | head -c $((1024*1024*1024 - 10*1024*1024)) >file
>   $ git add file
>   $ git commit -m one
>   $ yes | head -c $((1024*1024*1024)) >file
>   $ git commit -am two
>   fatal: unable to generate diffstat for file
>
> What's weird is that if I run "git show --stat" on the same commit, it
> works! So there's something about how commit invokes the diff that
> doesn't let the big-file check kick in.

I have a flashback about checking big_file_threshold in this code. I
vaguely remember doing something in this code but not sure if it was
merged.

I finally found 6bf3b81348 (diff --stat: mark any file larger than
core.bigfilethreshold binary - 2014-08-16) so it's merged after all,
but this commit is about --stat apparently ;-)

> It looks like the logic in diff_filespec_is_binary() will only check
> big_file_threshold if we haven't already loaded the contents into RAM.
> So "commit" does that, but "diff" is more careful about not loading the
> file contents.
>
> I think we probably ought to consider anything over big_file_threshold
> to be binary, no matter what. Possibly even if the user gave us a
> .gitattribute that says "no really, this is text". Because that 1GB
> limit is a hard limit that the code can't cope with; our options are
> either to generate a binary diff or to die.

Agreed. While at there perhaps we need to improve this "unable to
generate diffstat" message a bit too.
-- 
Duy

Hello Beautiful

[jack]

Good day dear, i hope this mail meets you well? my name is Jack, from the U.S. 
I know this may seem inappropriate so i ask for your forgiveness but i wish to 
get to know you better, if I may be so bold. I consider myself an easy-going 
man, adventurous, honest and fun loving person but I am currently looking for a 
relationship in which I will feel loved. I promise to answer any question that 
you may want to ask me...all i need is just your attention and the chance to 
know you more.

Please tell me more about yourself, if you do not mind. Hope to hear back from 
you soon.

Jack.

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Duy Nguyen]

On Sat, Feb 10, 2018 at 1:37 AM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>
>> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>>  - update-index doc: note a fixed bug in the untracked cache
>>  - dir.c: fix missing dir invalidation in untracked code
>>  - dir.c: avoid stat() in valid_cached_dir()
>>  - status: add a failing test showing a core.untrackedCache bug
>>
>>  Some bugs around "untracked cache" feature have been fixed.
>>
>>  Will merge to 'next'.
>
> I think you / Nguyễn may not have seen my
> <87d11omi2o@evledraar.gmail.com>
> (https://public-inbox.org/git/87d11omi2o@evledraar.gmail.com/)

I have. But since you wrote "I haven't found... yet", I assumed you
were still on it. You didn't give me much info to follow anyway.

> As noted there I think it's best to proceed without the "dir.c: stop
> ignoring opendir[...]" patch.
>
> It's going to be a bad regression in 2.17 if it ends up spewing pagefuls
> of warnings in previously working repos if the UC is on.

"previously working" is a strong word when opendir() starts to
complain. Sure we can suppress/ignore the error messages but I don't
think it's a healthy attitude. Unreported bugs can't be fixed.

We could perhaps stop reporting after we have printed like ... 5 lines
or so? That keeps the noise level down a bit and probably still give
enough indicator to at least repair the cache.
-- 
Duy

Re: [RFC PATCH 3/3] fsck: Check HEAD of other worktrees as well

[Duy Nguyen]

On Fri, Feb 09, 2018 at 03:13:30PM -0800, Elijah Newren wrote:
> This takes advantage of the fact that "worktrees/$WORKTREE/HEAD" will
> currently resolve as a ref, which may not be true in the future with
> different ref backends.  I don't think it locks us in to supporting
> resolving other worktree HEADs with this syntax, as I view the
> user-visible error message as more of a pointer to a pathname that the
> user will need to fix.  If the underlying ref storage changes, naturally
> both this code and the hint may need to change to match.

I'm leaning more about something like this patch below (which does not
even build, just to demonstrate). A couple points:

- Instead of doing the hacky refs worktrees/foo/HEAD, we get the
  correct ref store for each worktree
- We have an API for getting all (non-broken) worktrees. I realize for
  fsck, we may even want to examine semi-broken worktrees as well, but
  get_worktrees() can take a flag to accomplish that if needed.
- As you can see, I print %p from the ref store instead of something
  human friendly. This is something I was stuck at last time. I need
  better ref store description (or even the worktree name)
- This ref_name() function makes me think if we should have an
  extended sha1 syntax for accessing per-worktree refs from a
  different worktree, something like HEAD@{worktree:foo} to resolve to
  worktrees/foo/HEAD. Then we have a better description here that can
  actually be used from command line, as a regular ref, if needed.

-- 8< --
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 4132034170..73cfcbc07a 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -16,6 +16,7 @@
 #include "streaming.h"
 #include "decorate.h"
 #include "packfile.h"
+#include "worktree.h"
 
 #define REACHABLE 0x0001
 #define SEEN  0x0002
@@ -451,17 +452,39 @@ static int fsck_handle_ref(const char *refname, const 
struct object_id *oid,
return 0;
 }
 
-static int fsck_head_link(const char **head_points_at,
+static const char *ref_name(struct ref_store *refs, const char *name)
+{
+   static struct strbuf sb = STRBUF_INIT;
+
+   if (!refs)
+   return name;
+   strbuf_reset();
+   strbuf_addf(, "%s (from %p)", name, refs);
+   return sb.buf;
+}
+
+static int fsck_head_link(struct ref_store *refs,
+ const char **head_points_at,
  struct object_id *head_oid);
 
 static void get_default_heads(void)
 {
const char *head_points_at;
struct object_id head_oid;
+   struct worktree **worktrees, **wt;
 
-   fsck_head_link(_points_at, _oid);
+   fsck_head_link(NULL, _points_at, _oid);
if (head_points_at && !is_null_oid(_oid))
fsck_handle_ref("HEAD", _oid, 0, NULL);
+
+   worktrees = get_worktrees(0);
+   for (wt = worktrees; *wt; wt++) {
+   fsck_head_link(get_worktree_ref_store(*wt), _points_at, 
_oid);
+   if (head_points_at && !is_null_oid(_oid))
+   fsck_handle_ref(*wt, "HEAD", _oid, 0, NULL);
+   }
+   free_worktrees(wt);
+
for_each_rawref(fsck_handle_ref, NULL);
if (include_reflogs)
for_each_reflog(fsck_handle_reflog, NULL);
@@ -553,34 +576,36 @@ static void fsck_object_dir(const char *path)
stop_progress();
 }
 
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(struct ref_store *refs,
+ const char **head_points_at,
  struct object_id *head_oid)
 {
int null_is_error = 0;
 
if (verbose)
-   fprintf(stderr, "Checking HEAD link\n");
+   fprintf(stderr, "Checking %s link\n", ref_name(refs, "HEAD"));
 
-   *head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+   *head_points_at = refs_resolve_ref_unsafe(refs, "HEAD", 0, head_oid, 
NULL);
if (!*head_points_at) {
errors_found |= ERROR_REFS;
-   return error("Invalid HEAD");
+   return error("Invalid HEAD from ref-store %p", refs);
}
if (!strcmp(*head_points_at, "HEAD"))
/* detached HEAD */
null_is_error = 1;
else if (!starts_with(*head_points_at, "refs/heads/")) {
errors_found |= ERROR_REFS;
-   return error("HEAD points to something strange (%s)",
-*head_points_at);
+   return error("%s points to something strange (%s)",
+ref_name(refs, "HEAD"), *head_points_at);
}
if (is_null_oid(head_oid)) {
if (null_is_error) {
errors_found |= ERROR_REFS;
-   return error("HEAD: detached HEAD points at nothing");
+   return error("%s: detached HEAD points at nothing",
+ref_name(refs, "HEAD"));
}
-   fprintf(stderr, 

Re: [PATCH v6 5/7] convert: add 'working-tree-encoding' attribute

[Torsten Bögershausen]

On Fri, Feb 09, 2018 at 02:28:28PM +0100, lars.schnei...@autodesk.com wrote:
> From: Lars Schneider 
> 
> Git recognizes files encoded with ASCII or one of its supersets (e.g.
> UTF-8 or ISO-8859-1) as text files. All other encodings are usually
> interpreted as binary and consequently built-in Git text processing
> tools (e.g. 'git diff') as well as most Git web front ends do not
> visualize the content.
> 
> Add an attribute to tell Git what encoding the user has defined for a
> given file. If the content is added to the index, then Git converts the
> content to a canonical UTF-8 representation. On checkout Git will
> reverse the conversion.
> 
> Signed-off-by: Lars Schneider 
> ---
>  Documentation/gitattributes.txt  |  66 
>  convert.c| 157 -
>  convert.h|   1 +
>  sha1_file.c  |   2 +-
>  t/t0028-working-tree-encoding.sh | 210 
> +++
>  5 files changed, 434 insertions(+), 2 deletions(-)
>  create mode 100755 t/t0028-working-tree-encoding.sh
> 
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 30687de81a..4ecdcd4859 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -272,6 +272,72 @@ few exceptions.  Even though...
>catch potential problems early, safety triggers.
>  
>  
> +`working-tree-encoding`
> +^^^
> +
> +Git recognizes files encoded with ASCII or one of its supersets (e.g.
> +UTF-8 or ISO-8859-1) as text files.  All other encodings are usually
> +interpreted as binary and consequently built-in Git text processing
> +tools (e.g. 'git diff') as well as most Git web front ends do not
> +visualize the content.
> +
> +In these cases you can tell Git the encoding of a file in the working
> +directory with the `working-tree-encoding` attribute. If a file with this
> +attribute is added to Git, then Git reencodes the content from the
> +specified encoding to UTF-8. Finally, Git stores the UTF-8 encoded
> +content in its internal data structure (called "the index"). On checkout
> +the content is reencoded back to the specified encoding.
> +
> +Please note that using the `working-tree-encoding` attribute may have a
> +number of pitfalls:
> +
> +- Git clients that do not support the `working-tree-encoding` attribute

A client to Git ?
Or may be "third party Git implementations"

> +  will checkout the respective files UTF-8 encoded and not in the
> +  expected encoding. Consequently, these files will appear different
> +  which typically causes trouble. This is in particular the case for
> +  older Git versions and alternative Git implementations such as JGit
> +  or libgit2 (as of February 2018).
> +
> +- Reencoding content requires resources that might slow down certain
> +  Git operations (e.g 'git checkout' or 'git add').
> +
> +Use the `working-tree-encoding` attribute only if you cannot store a file
> +in UTF-8 encoding and if you want Git to be able to process the content
> +as text.
> +
> +As an example, use the following attributes if your '*.proj' files are
> +UTF-16 encoded with byte order mark (BOM) and you want Git to perform
> +automatic line ending conversion based on your platform.
> +
> +
> +*.proj   text working-tree-encoding=UTF-16
> +
> +
> +Use the following attributes if your '*.proj' files are UTF-16 little
> +endian encoded without BOM and you want Git to use Windows line endings
> +in the working directory. Please note, it is highly recommended to
> +explicitly define the line endings with `eol` if the `working-tree-encoding`
> +attribute is used to avoid ambiguity.
> +
> +
> +*.proj   working-tree-encoding=UTF-16LE text eol=CRLF
> +
> +
> +You can get a list of all available encodings on your platform with the
> +following command:

One question:
 +*.projtext working-tree-encoding=UTF-16
vs
*.proj  working-tree-encoding=UTF-16LE text eol=CRLF

Technically the order of attributes doesn't matter, but that is not what we
want to demonstrate here and now.
I would probably move the "text" attribute to the end of the line.
So that readers don't start to wonder if the order is important.

> +
> +
> +iconv --list
> +
> +
> +If you do not know the encoding of a file, then you can use the `file`
> +command to guess the encoding:
> +
> +
> +file foo.proj
> +
> +
> +
>  `ident`
>  ^^^
>  
> diff --git a/convert.c b/convert.c
> index b976eb968c..dc9e2db6b5 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -7,6 +7,7 @@
>  #include "sigchain.h"
>  #include "pkt-line.h"
>  #include "sub-process.h"
> +#include "utf8.h"
>  
>  /*
>   * convert.c - convert a file 

Re: Crash when clone includes magic filenames on Windows

[Jeffrey Walton]

On Sat, Feb 10, 2018 at 4:31 AM, Torsten Bögershausen  wrote:
> On Sat, Feb 10, 2018 at 03:55:58AM -0500, Jeffrey Walton wrote:
>> Hi Everyone,
>>
>> I'm seeing this issue on Windows: https://pastebin.com/YfB25E4T . It
>> seems the filename AUX is the culprit. Also see
>> https://blogs.msdn.microsoft.com/oldnewthing/20031022-00/?p=42073 .
>> (Thanks to Milleneumbug on Stack Overflow).
>>
>> I did not name the file, someone else did. I doubt the filename will be 
>> changed.
>>
>> Searching is not turning up much information:
>> https://www.google.com/search?q=git+"magic+filenames"+windows
>>
>> Does anyone know how to sidestep the issue on Windows?
>>
>> Jeff
>
> Thanks for the report.
>
> (Typically nobody (tm) here on the list opens a web-browser to look at 
> external
>  material, so here is a shortened version of the pastebin:)
>
> error: unable to create file 
> crypto_stream/lexv2/e/v2/schwabe/sparc-2/e/aux.c: No such file or directory
> error: unable to create file 
> crypto_stream/lexv2/e/v2/schwabe/sparc-2/e/aux.s: No such file or directory
> Segmentation fault:  99% (26526/26793)
>
> There are actually 2 problems:
> - The filenames named aux.c
>   It could be that git -c core.longpaths=true clone xxx
>   works, but I don't have a Windows box to test at the moment-

Thanks. This did not help.

> - The crash
>   Which Git version do you use?
>   It may be a good idea to report it here
>   https://github.com/git-for-windows/git

2.16.1-2

Thanks again.

Jeff

Re: Crash when clone includes magic filenames on Windows

[Torsten Bögershausen]

On Sat, Feb 10, 2018 at 03:55:58AM -0500, Jeffrey Walton wrote:
> Hi Everyone,
> 
> I'm seeing this issue on Windows: https://pastebin.com/YfB25E4T . It
> seems the filename AUX is the culprit. Also see
> https://blogs.msdn.microsoft.com/oldnewthing/20031022-00/?p=42073 .
> (Thanks to Milleneumbug on Stack Overflow).
> 
> I did not name the file, someone else did. I doubt the filename will be 
> changed.
> 
> Searching is not turning up much information:
> https://www.google.com/search?q=git+"magic+filenames"+windows
> 
> Does anyone know how to sidestep the issue on Windows?
> 
> Jeff

Thanks for the report.

(Typically nobody (tm) here on the list opens a web-browser to look at external
 material, so here is a shortened version of the pastebin:)

error: unable to create file crypto_stream/lexv2/e/v2/schwabe/sparc-2/e/aux.c: 
No such file or directory
error: unable to create file crypto_stream/lexv2/e/v2/schwabe/sparc-2/e/aux.s: 
No such file or directory
Segmentation fault:  99% (26526/26793)

There are actually 2 problems:
- The filenames named aux.c
  It could be that git -c core.longpaths=true clone xxx
  works, but I don't have a Windows box to test at the moment-

- The crash
  Which Git version do you use?
  It may be a good idea to report it here
  https://github.com/git-for-windows/git

Re: [PATCH v3 42/42] git-completion.bash: add GIT_COMPLETION_OPTIONS=all config

[Duy Nguyen]

On Fri, Feb 09, 2018 at 03:19:57PM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Feb 09 2018, Nguyễn Thái Ngọc Duy jotted:
> 
> > By default, some option names (mostly --force, scripting related or for
> > internal use) are not completable for various reasons. When
> > GIT_COMPLETION_OPTIONS is set to all, all options (except hidden ones)
> > are completable.
> >
> > Signed-off-by: Nguyễn Thái Ngọc Duy 
> > ---
> >  contrib/completion/git-completion.bash |  6 +-
> >  parse-options.c| 11 +++
> >  2 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/contrib/completion/git-completion.bash 
> > b/contrib/completion/git-completion.bash
> > index 0ddf40063b..0cfa489a8e 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -36,6 +36,10 @@
> >  #
> >  # When set to "1", do not include "DWIM" suggestions in git-checkout
> >  # completion (e.g., completing "foo" when "origin/foo" exists).
> > +#
> > +#   GIT_COMPLETION_OPTIONS
> > +#
> > +# When set to "all", complete all possible options
> 
> I was going to suggest some wording like:
> 
> When set to "all", include options considered unsafe such as --force
> in the completion.
> 
> However per your cover letter it's not just used for that:
> 
>  10 --force
>   4 --rerere-autoupdate
>   1 --unsafe-paths
>   1 --thin
>   1 --overwrite-ignore
>   1 --open-files-in-pager
>   1 --null
>   1 --ext-grep
>   1 --exit-code
>   1 --auto
> 
> I wonder if we shouldn't just make this only about --force, I don't see
> why "git grep --o" should only show --or but not
> --open-files-in-pager, and e.g. "git grep --" is already verbose so
> we're not saving much by excluding those.
> 
> Then this could just become:
> 
> GIT_COMPLETION_SHOWUNSAFEOPTIONS=1
> 
> Or other similar boolean variable, for consistency with all the "*SHOW*
> variables already in git-completion.bash.

No. You're asking for a second default. I'm not adding plenty of
GIT_COMPLETION_* variables for that. You either have all options, or
you convince people that --force should be part of the current
default.

Or you could push for a generic mechanism that allows you to customize
your own default. Something like the below patch could give you what
you want with:

GIT_COMPLETION_OPTIONS=all
GIT_COMPLETION_EXCLUDES=--open-files-in-pager # and some more
. /path/to/git-completion.bash

I'm not going to make a real patch for this since people may want to
ignore --foo in one command and complete --foo in others... I'm just
not interested in trying to cover all cases.

-- 8< --
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 0cfa489a8e..9ca0d80cd7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -40,6 +40,10 @@
 #   GIT_COMPLETION_OPTIONS
 #
 # When set to "all", complete all possible options
+#
+#   GIT_COMPLETION_EXCLUDES
+#
+# Exclude some options from the complete list
 
 case "$COMP_WORDBREAKS" in
 *:*) : great ;;
@@ -298,7 +302,7 @@ __gitcomp_builtin ()
# commands, e.g. "git remote add" becomes remote_add.
local cmd="$1"
local incl="$2"
-   local excl="$3"
+   local excl="$3 $GIT_COMPLETION_EXCLUDES"
 
local var=__gitcomp_builtin_"${cmd/-/_}"
local options
-- 8< --

Crash when clone includes magic filenames on Windows

[Jeffrey Walton]

Hi Everyone,

I'm seeing this issue on Windows: https://pastebin.com/YfB25E4T . It
seems the filename AUX is the culprit. Also see
https://blogs.msdn.microsoft.com/oldnewthing/20031022-00/?p=42073 .
(Thanks to Milleneumbug on Stack Overflow).

I did not name the file, someone else did. I doubt the filename will be changed.

Searching is not turning up much information:
https://www.google.com/search?q=git+"magic+filenames"+windows

Does anyone know how to sidestep the issue on Windows?

Jeff