Re: [PATCH v3 42/42] git-completion.bash: add GIT_COMPLETION_OPTIONS=all config
On Fri, Feb 9, 2018 at 6:02 AM, Nguyễn Thái Ngọc Duywrote: > 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
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
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
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
From: Stefan BellerUp 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
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
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
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
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
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
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
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()
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
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
Hi Sergey, On Fri, 9 Feb 2018, Sergey Organov wrote: > Johannes Schindelinwrites: > > [...] > > > 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
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
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
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]
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
Duy Nguyenwrites: > 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)
Duy Nguyenwrites: >> 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
Hi Junio, On Tue, 23 Jan 2018, Junio C Hamano wrote: > Johannes Schindelinwrites: > > > 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
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
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
On Thu, Feb 08, 2018 at 12:29:53PM -0800, Junio C Hamano wrote: > Jonathan Tanwrites: > > > 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
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
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
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
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
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
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
On Fri, Feb 9, 2018 at 7:08 PM, Duy Nguyenwrote: > 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
On Thu, Feb 8, 2018 at 1:06 AM, Stefan Bellerwrote: >> 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
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)
On Thu, Feb 8, 2018 at 1:19 AM, Junio C Hamanowrote: > 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
On Thu, Feb 8, 2018 at 2:00 AM, Stefan Bellerwrote: >> + >> +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
On Thu, Feb 8, 2018 at 3:45 AM, Jeff Kingwrote: > 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
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)
On Sat, Feb 10, 2018 at 1:37 AM, Ævar Arnfjörð Bjarmasonwrote: > > 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
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
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
On Sat, Feb 10, 2018 at 4:31 AM, Torsten Bögershausenwrote: > 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
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
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
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