Re: Is support for 10.8 dropped?
On Sun, Apr 22, 2018 at 1:15 AM, Igor Korot wrote: > MyMac:git-2.17.0 igorkorot$ cat config.mak > NO_GETTEXT=Yes > NO_OPENSSL=Yes > > MyMac:dbhandler igorkorot$ /Users/igorkorot/git-2.17.0/git pull > fatal: unable to access > 'https://github.com/oneeyeman1/dbhandler.git/': error:1407742E:SSL > routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version > MyMac:dbhandler igorkorot$ Try re-building with OpenSSL enabled (remove NO_OPENSSL from config.make). You may need to build/install OpenSSL yourself to get this to work.
Re: Is support for 10.8 dropped?
Hi, Sorry for the long delay. Been busy with life things. I built git from sources. The build was successful. [code] MyMac:git-2.17.0 igorkorot$ cat config.mak NO_GETTEXT=Yes NO_OPENSSL=Yes [/code] Next I tried to do git pull for my project: [code] MyMac:dbhandler igorkorot$ /Users/igorkorot/git-2.17.0/git pull fatal: unable to access 'https://github.com/oneeyeman1/dbhandler.git/': error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version MyMac:dbhandler igorkorot$ [/code] Thank you. On Wed, Apr 11, 2018 at 5:44 AM, Eric Sunshine wrote: > On Wed, Apr 11, 2018 at 12:28 AM, Igor Korot wrote: Is there a way to check for OpenSSL presence? >>> >>> Not sure what you're asking. Are you asking how to determine if you >>> already have OpenSSL built on your machine? >> >> Yes, that's what I was asking... > > Easiest way to determine it is to try to compile Git without setting > NO_OPENSSL. > >> This is what I got trying to do just "make": >> >> MyMac:git-2.17.0 igorkorot$ make >> * new build flags >> CC credential-store.o >> In file included from credential-store.c:1: >> In file included from ./cache.h:9: >> ./gettext.h:17:11: fatal error: 'libintl.h' file not found >> # include >> ^ >> 1 error generated. > > This is because you don't have gettext installed. You should be able > to set NO_GETTEXT to work around this. > >> And I am also confused. Which file am I suppose to modify here? >> >> MyMac:git-2.17.0 igorkorot$ ls -la conf* >> -rwxr-xr-x@ 1 igorkorot staff 74461 Apr 2 10:13 config.c >> -rwxr-xr-x@ 1 igorkorot staff 9888 Apr 2 10:13 config.h >> -rwxr-xr-x@ 1 igorkorot staff540 Apr 2 10:13 config.mak.in >> -rwxr-xr-x@ 1 igorkorot staff 16940 Apr 2 10:13 config.mak.uname >> -rwxr-xr-x@ 1 igorkorot staff 37509 Apr 2 10:13 configure.ac >> -rw-r--r-- 1 igorkorot staff 37500 Apr 8 18:52 configure.ac+ > > You should create a new file named "config.mak". For instance, the > content of your config.mak file might be: > > NO_GETTEXT=Yes
Re: [PATCH v4 05/11] replace: introduce --convert-graft-file
On Sat, Apr 21, 2018 at 5:48 AM, Johannes Schindelin
wrote:
> This option is intended to help with the transition away from the
> now-deprecated graft file.
>
> Signed-off-by: Johannes Schindelin
> ---
> diff --git a/builtin/replace.c b/builtin/replace.c
> @@ -454,6 +455,38 @@ static int create_graft(int argc, const char **argv, int
> force)
> +static int convert_graft_file(int force)
> +{
> + const char *graft_file = get_graft_file();
> + FILE *fp = fopen_or_warn(graft_file, "r");
> + struct strbuf buf = STRBUF_INIT, err = STRBUF_INIT;
> + struct argv_array args = ARGV_ARRAY_INIT;
> +
> + if (!fp)
> + return -1;
> +
> + while (strbuf_getline(&buf, fp) != EOF) {
> + if (*buf.buf == '#')
> + continue;
> +
> + argv_array_split(&args, buf.buf);
> + if (args.argc && create_graft(args.argc, args.argv, force))
> + strbuf_addf(&err, "\n\t%s", buf.buf);
> + argv_array_clear(&args);
> + }
> +
> + strbuf_release(&buf);
> + argv_array_clear(&args);
This argv_array_clear() is redundant, isn't it?
> + if (!err.len)
> + return unlink_or_warn(graft_file);
> +
> + warning(_("could not convert the following graft(s):\n%s"), err.buf);
> + strbuf_release(&err);
> +
> + return -1;
> +}
Re: [PATCH 0/6] Compute and consume generation numbers
Jakub Narebski writes: > Derrick Stolee writes: >> On 4/11/2018 3:32 PM, Jakub Narebski wrote: > >>> What would you suggest as a good test that could imply performance? The >>> Google Colab notebook linked to above includes a function to count >>> number of commits (nodes / vertices in the commit graph) walked, >>> currently in the worst case scenario. >> >> The two main questions to consider are: >> >> 1. Can X reach Y? > > That is easy to do. The function generic_is_reachable() does > that... though using direct translation of the pseudocode for > "Algorithm 3: Reachable" from FELINE paper, which is recursive and > doesn't check if vertex was already visited was not good idea for large > graphs such as Linux kernel commit graph, oops. That is why > generic_is_reachable_large() was created. [...] >> And the thing to measure is a commit count. If possible, it would be >> good to count commits walked (commits whose parent list is enumerated) >> and commits inspected (commits that were listed as a parent of some >> walked commit). Walked commits require a commit parse -- albeit from >> the commit-graph instead of the ODB now -- while inspected commits >> only check the in-memory cache. [...] >> >> For git.git and Linux, I like to use the release tags as tests. They >> provide a realistic view of the linear history, and maintenance >> releases have their own history from the major releases. > > Hmmm... testing for v4.9-rc5..v4.9 in Linux kernel commit graphs, the > FELINE index does not bring any improvements over using just level > (generation number) filter. But that may be caused by narrowing od > commit DAG around releases. > > I try do do the same between commits in wide part, with many commits > with the same level (same generation number) both for source and for > target commit. Though this may be unfair to level filter, though... > > > Note however that FELINE index is not unabiguous, like generation > numbers are (modulo decision whether to start at 0 or at 1); it depends > on the topological ordering chosen for the X elements. One can now test reachability on git.git repository; there is a form where one can plug source and destination revisions at https://colab.research.google.com/drive/1V-U7_slu5Z3s5iEEMFKhLXtaxSu5xyzg#scrollTo=svNUnSA9O_NK&line=2&uniqifier=1 I have tried the case that is quite unfair to the generation numbers filter, namely the check between one of recent tags, and one commit that shares generation number among largest number of other commits. Here level = generation number-1 (as it starts at 0 for root commit, not 1). The results are: * src = 468165c1d = v2.17.0 * dst = 66d2e04ec = v2.0.5-5-g66d2e04ec * 468165c1d has level 18418 which it shares with 6 commits * 66d2e04ec has level 14776 which it shares with 93 commits * gen(468165c1d) - gen(66d2e04ec) = 3642 algorithm | access | walk | maxdepth | visited | level-f | FELINE-f | ---+-++--+-+--+---+ naive | 48865 | 39599 | 244 | 9200| | | level | 3086 | 2492 | 113 | 528| 285 | | FELINE | 283 | 216 | 68 |0| | 25 | lev+FELINE | 282 | 215 | 68 |0| 5 | 24 | ---+-++--+-+--+---+ lev+FEL+mpi|79 |59 | 21 |0| 0 | 0 | Here we have: * 'naive' implementation means simple DFS walk, without any filters (cut-offs) * 'level' means using levels / generation numbers based negative-cut filter * 'FELINE' means using FELINE index based negative-cut filter * 'lev+FELINE' means combining generation numbers filter with FELINE filter * 'mpi' means min-post [smanning-tree] intervals for positive-cut filter; note that the code does not walk the path after cut, but it is easy to do The stats have the following meaning: * 'access' means accessing the node * 'walk' is actual walking the node * 'maxdepth' is maximum depth of the stack used for DFS * 'level-f' and 'FELINE-f' is number of times levels filter or FELINE filter were used for negative-cut; note that those are not disjoint; node can be rejected by both level filter and FELINE filter For v2.17.0 and v2.17.0-rc2 the numbers are much less in FELINE favor: the results are the same, with 5 commits accessed and 6 walked compared to 61574 accessed in naive algorithm. The git.git commit graph has 53128 nodes and 66124 edges, 4 tips / heads (different child-less commits) and 9 roots, and has average clustering coefficient 0.000409217. P.S. Would it be better to move the discussion about possible extensions to the commit-graph in the form of new chunks (topological order, FELINE index, min-post intervals, bloom filter for changed files, etc.) be moved into separate thread? -- Jakub Narębski
Re: [RFC PATCH v10 32.5/36] unpack_trees: fix memory corruption with split_index when src != dst
On Sat, Apr 21, 2018 at 12:37 PM, Elijah Newren wrote: > Currently, all callers of unpack_trees() set o->src_index == o->dst_index. > Since we create a temporary index in o->result, then discard o->dst_index > and overwrite it with o->result, when o->src_index == o->dst_index it is > safe to just reuse o->src_index's split_index for o->result. However, > o->src_index and o->dst_index are specified separately in order to allow > callers to have these be different. In such a case, reusing > o->src_index's split_index for o->result will cause the split_index to be > shared. If either index then has entries replaced or removed, it will > result in the other index referring to free()'d memory. > > Signed-off-by: Elijah Newren > --- Also, I probably shouldn't have made this look like part of my series (by marking it as "RFC PATCH v10 32.5/36"). It doesn't depend on my series and is an independently valuable bugfix, though to avoid breaking SZEDER and other split_index users, this patch should probably go in before my series does.
[RFC PATCH v10 32.5/36] unpack_trees: fix memory corruption with split_index when src != dst
Currently, all callers of unpack_trees() set o->src_index == o->dst_index.
Since we create a temporary index in o->result, then discard o->dst_index
and overwrite it with o->result, when o->src_index == o->dst_index it is
safe to just reuse o->src_index's split_index for o->result. However,
o->src_index and o->dst_index are specified separately in order to allow
callers to have these be different. In such a case, reusing
o->src_index's split_index for o->result will cause the split_index to be
shared. If either index then has entries replaced or removed, it will
result in the other index referring to free()'d memory.
Signed-off-by: Elijah Newren
---
I still haven't wrapped my head around the split_index stuff entirely, so
it's possible that
- the performance optimization isn't even valid when src == dst. Could
the original index be different enough from the result that we don't
want its split_index?
- there's a better, more performant fix or there is some way to actually
share a split_index between two independent index_state objects.
However, with this fix, all the tests pass both normally and under
GIT_TEST_SPLIT_INDEX=DareISayYes. Without this patch, when
GIT_TEST_SPLIT_INDEX is set, my directory rename detection series will fail
several tests, as reported by SZEDER.
unpack-trees.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 79fd97074e..b670415d4c 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1284,9 +1284,20 @@ int unpack_trees(unsigned len, struct tree_desc *t,
struct unpack_trees_options
o->result.timestamp.sec = o->src_index->timestamp.sec;
o->result.timestamp.nsec = o->src_index->timestamp.nsec;
o->result.version = o->src_index->version;
- o->result.split_index = o->src_index->split_index;
- if (o->result.split_index)
+ if (!o->src_index->split_index) {
+ o->result.split_index = NULL;
+ } else if (o->src_index == o->dst_index) {
+ /*
+* o->dst_index (and thus o->src_index) will be discarded
+* and overwritten with o->result at the end of this function,
+* so just use src_index's split_index to avoid having to
+* create a new one.
+*/
+ o->result.split_index = o->src_index->split_index;
o->result.split_index->refcount++;
+ } else {
+ o->result.split_index = init_split_index(&o->result);
+ }
hashcpy(o->result.sha1, o->src_index->sha1);
o->merge_size = len;
mark_all_ce_unused(o->src_index);
--
2.17.0.296.gaac25b4b81
Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary
On Sat, Apr 21, 2018 at 06:54:08PM +0200, Nguyễn Thái Ngọc Duy wrote:
> Changes:
>
> - remove the deprecated column in command-list.txt. My change break it
> anyway if anyone uses it.
> - fix up failed tests that I marked in the RFC and kinda forgot about it.
> - fix bashisms in generate-cmdlist.sh
> - fix segfaul in "git help"
Sorry I forgot the interdiff
diff --git a/command-list.txt b/command-list.txt
index 0809a19184..1835f1a928 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -9,7 +9,7 @@ history grow, mark and tweak your common history
remote collaborate (see also: git help workflows)
### command list (do not change this line)
-# command name category [deprecated] [common]
+# command name category[common]
git-add mainporcelain worktree
git-am mainporcelain
git-annotateancillaryinterrogators
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 9f17703aa7..7d17ca23f6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -835,19 +835,23 @@ __git_complete_strategy ()
}
__git_commands () {
- if test -n "${GIT_TESTING_COMMAND_COMPLETION:-}"
+ if test -n "$GIT_TESTING_COMPLETION"
then
- printf "%s" "${GIT_TESTING_COMMAND_COMPLETION}"
+ case "$1" in
+ porcelain)
+ printf "%s" "$GIT_TESTING_PORCELAIN_COMMAND_LIST";;
+ all)
+ printf "%s" "$GIT_TESTING_ALL_COMMAND_LIST";;
+ esac
else
git --list-cmds=$1
fi
}
-__git_list_all_commands ()
+__git_list_commands ()
{
local i IFS=" "$'\n'
- local category=${1-all}
- for i in $(__git_commands $category)
+ for i in $(__git_commands $1)
do
case $i in
*--*) : helper pattern;;
@@ -860,14 +864,14 @@ __git_all_commands=
__git_compute_all_commands ()
{
test -n "$__git_all_commands" ||
- __git_all_commands=$(__git_list_all_commands)
+ __git_all_commands=$(__git_list_commands all)
}
__git_porcelain_commands=
__git_compute_porcelain_commands ()
{
test -n "$__git_porcelain_commands" ||
- __git_porcelain_commands=$(__git_list_all_commands porcelain)
+ __git_porcelain_commands=$(__git_list_commands porcelain)
}
# Lists all set config variables starting with the given section prefix,
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index e35f3e357b..86d59419b3 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -36,7 +36,7 @@ sed -n '
' "$1"
printf '};\n\n'
-echo "#define GROUP_NONE 0xff /* no common group */"
+echo "#define GROUP_NONE 0xff"
n=0
while read grp
do
@@ -45,15 +45,6 @@ do
done <"$grps"
echo
-echo '/*'
-printf 'static const char *cmd_categories[] = {\n'
-category_list "$1" |
-while read category; do
- printf '\t\"'$category'\",\n'
-done
-printf '\tNULL\n};\n\n'
-echo '*/'
-
n=0
category_list "$1" |
while read category; do
@@ -68,10 +59,11 @@ sort |
while read cmd category tags
do
if [ "$category" = guide ]; then
- name=${cmd/git}
+ prefix=git
else
- name=${cmd/git-}
+ prefix=git-
fi
+ name=$(echo $cmd | sed "s/^${prefix}//")
sed -n '
/^NAME/,/'"$cmd"'/H
${
diff --git a/help.c b/help.c
index a44f4a113e..88127fdd6f 100644
--- a/help.c
+++ b/help.c
@@ -201,7 +201,8 @@ static void extract_common_cmds(struct cmdname_help
**p_common_cmds,
for (i = 0; i < ARRAY_SIZE(command_list); i++) {
const struct cmdname_help *cmd = command_list + i;
- if (cmd->category != CAT_mainporcelain)
+ if (cmd->category != CAT_mainporcelain ||
+ cmd->group == GROUP_NONE)
continue;
common_cmds[nr++] = *cmd;
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index fd2a7f27dc..53208ab20e 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -25,6 +25,15 @@ test_expect_success "setup" '
EOF
'
+# make sure to exercise these code paths, the output is a bit tricky
+# to verify
+test_expect_success 'basic help commands' '
+ git help >/dev/null &&
+ git help -a >/dev/null &&
+ git help -g >/dev/null &&
+ git help -av >/dev/null
+'
+
test_expect_success "works for commands and guides by default" '
configure_help &&
git help status &&
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 4bfd26ddf9..5a23a46fcf 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -13,7 +13,7 @@ complete ()
return 0
}
-# Be careful when updating this list:
+# Be careful when updating
[PATCH v3 4/6] git.c: implement --list-cmds=porcelain
This is useful for git-completion.bash because it needs this set of
commands. Right now we have to maintain a separate command category in
there.
Note that the current completion script incorrectly classifies
filter-branch as porcelain and t9902 tests this behavior. We keep it
this way in t9902 because this test does not really care which
particular command is porcelain or plubmbing, they're just names.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
contrib/completion/git-completion.bash | 106 +++--
git.c | 2 +
help.c | 12 +++
help.h | 1 +
t/t9902-completion.sh | 6 +-
5 files changed, 31 insertions(+), 96 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index a5f13ade20..7d17ca23f6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -835,18 +835,23 @@ __git_complete_strategy ()
}
__git_commands () {
- if test -n "${GIT_TESTING_COMMAND_COMPLETION:-}"
+ if test -n "$GIT_TESTING_COMPLETION"
then
- printf "%s" "${GIT_TESTING_COMMAND_COMPLETION}"
+ case "$1" in
+ porcelain)
+ printf "%s" "$GIT_TESTING_PORCELAIN_COMMAND_LIST";;
+ all)
+ printf "%s" "$GIT_TESTING_ALL_COMMAND_LIST";;
+ esac
else
- git --list-cmds=all
+ git --list-cmds=$1
fi
}
-__git_list_all_commands ()
+__git_list_commands ()
{
local i IFS=" "$'\n'
- for i in $(__git_commands)
+ for i in $(__git_commands $1)
do
case $i in
*--*) : helper pattern;;
@@ -859,101 +864,14 @@ __git_all_commands=
__git_compute_all_commands ()
{
test -n "$__git_all_commands" ||
- __git_all_commands=$(__git_list_all_commands)
-}
-
-__git_list_porcelain_commands ()
-{
- local i IFS=" "$'\n'
- __git_compute_all_commands
- for i in $__git_all_commands
- do
- case $i in
- *--*) : helper pattern;;
- applymbox): ask gittus;;
- applypatch) : ask gittus;;
- archimport) : import;;
- cat-file) : plumbing;;
- check-attr) : plumbing;;
- check-ignore) : plumbing;;
- check-mailmap): plumbing;;
- check-ref-format) : plumbing;;
- checkout-index) : plumbing;;
- column) : internal helper;;
- commit-tree) : plumbing;;
- count-objects): infrequent;;
- credential) : credentials;;
- credential-*) : credentials helper;;
- cvsexportcommit) : export;;
- cvsimport): import;;
- cvsserver): daemon;;
- daemon) : daemon;;
- diff-files) : plumbing;;
- diff-index) : plumbing;;
- diff-tree): plumbing;;
- fast-import) : import;;
- fast-export) : export;;
- fsck-objects) : plumbing;;
- fetch-pack) : plumbing;;
- fmt-merge-msg): plumbing;;
- for-each-ref) : plumbing;;
- hash-object) : plumbing;;
- http-*) : transport;;
- index-pack) : plumbing;;
- init-db) : deprecated;;
- local-fetch) : plumbing;;
- ls-files) : plumbing;;
- ls-remote): plumbing;;
- ls-tree) : plumbing;;
- mailinfo) : plumbing;;
- mailsplit): plumbing;;
- merge-*) : plumbing;;
- mktree) : plumbing;;
- mktag): plumbing;;
- pack-objects) : plumbing;;
- pack-redundant) : plumbing;;
- pack-refs): plumbing;;
- parse-remote) : plumbing;;
- patch-id) : plumbing;;
- prune): plumbing;;
- prune-packed) : plumbing;;
- quiltimport) : import;;
- read-tree): plumbing;;
- receive-pack) : plumbing;;
- remote-*) : transport;;
- rerere) : plumbing;;
- rev-list) : plumbing;;
- rev-parse): plumbing;;
- runstatus): plumbing;;
- sh-setup) : internal;;
- shell): daemon;;
- show-ref) : plumbing;;
- sen
[PATCH v3 5/6] help: add "-a --verbose" to list all commands with synopsis
This lists all recognized commands [1] by category. The group order
follows closely git.txt.
[1] We may actually show commands that are not built (e.g. if you set
NO_PERL you don't have git-instaweb but it's still listed here). I
ignore the problem because on Linux a git package could be split
anyway. The "git-core" package may not contain git-instaweb even if
it's built because it may end up in a separate package. We can't know
anyway.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git-help.txt | 4 ++-
builtin/help.c | 7
help.c | 69 ++
help.h | 1 +
t/t0012-help.sh| 9 +
5 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 40d328a4b3..a40fc38d8b 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
SYNOPSIS
[verse]
-'git help' [-a|--all] [-g|--guide]
+'git help' [-a|--all [--verbose]] [-g|--guide]
[-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]
DESCRIPTION
@@ -42,6 +42,8 @@ OPTIONS
--all::
Prints all the available commands on the standard output. This
option overrides any given command or guide name.
+ When used with `--verbose` print description for all recognized
+ commands.
-g::
--guides::
diff --git a/builtin/help.c b/builtin/help.c
index 598867cfea..0e0af8426a 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -36,6 +36,7 @@ static const char *html_path;
static int show_all = 0;
static int show_guides = 0;
+static int verbose;
static unsigned int colopts;
static enum help_format help_format = HELP_FORMAT_NONE;
static int exclude_guides;
@@ -48,6 +49,7 @@ static struct option builtin_help_options[] = {
HELP_FORMAT_WEB),
OPT_SET_INT('i', "info", &help_format, N_("show info page"),
HELP_FORMAT_INFO),
+ OPT__VERBOSE(&verbose, N_("print command description")),
OPT_END(),
};
@@ -463,6 +465,11 @@ int cmd_help(int argc, const char **argv, const char
*prefix)
if (show_all) {
git_config(git_help_config, NULL);
+ if (verbose) {
+ setup_pager();
+ list_all_cmds_help();
+ return 0;
+ }
printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
load_command_list("git-", &main_cmds, &other_cmds);
list_commands(colopts, &main_cmds, &other_cmds);
diff --git a/help.c b/help.c
index 6688653f99..ae5281601a 100644
--- a/help.c
+++ b/help.c
@@ -285,6 +285,75 @@ void list_porcelain_cmds(void)
}
}
+static int cmd_category_cmp(const void *elem1, const void *elem2)
+{
+ const struct cmdname_help *e1 = elem1;
+ const struct cmdname_help *e2 = elem2;
+
+ if (e1->category < e2->category)
+ return -1;
+ if (e1->category > e2->category)
+ return 1;
+ return strcmp(e1->name, e2->name);
+}
+
+static void list_commands_by_category(int cat, struct cmdname_help *cmds,
+ int nr, int longest)
+{
+ int i;
+
+ for (i = 0; i < nr; i++) {
+ struct cmdname_help *cmd = cmds + i;
+
+ if (cmd->category != cat)
+ continue;
+
+ printf(" %s ", cmd->name);
+ mput_char(' ', longest - strlen(cmd->name));
+ puts(_(cmd->help));
+ }
+}
+
+void list_all_cmds_help(void)
+{
+ int i, longest = 0;
+ int nr = ARRAY_SIZE(command_list);
+ struct cmdname_help *cmds = command_list;
+
+ for (i = 0; i < nr; i++) {
+ struct cmdname_help *cmd = cmds + i;
+
+ if (longest < strlen(cmd->name))
+ longest = strlen(cmd->name);
+ }
+
+ QSORT(cmds, nr, cmd_category_cmp);
+
+ printf("%s\n\n", _("Main Porcelain Commands"));
+ list_commands_by_category(CAT_mainporcelain, cmds, nr, longest);
+
+ printf("\n%s\n\n", _("Ancillary Commands / Manipulators"));
+ list_commands_by_category(CAT_ancillarymanipulators, cmds, nr, longest);
+
+ printf("\n%s\n\n", _("Ancillary Commands / Interrogators"));
+ list_commands_by_category(CAT_ancillaryinterrogators, cmds, nr,
longest);
+
+ printf("\n%s\n\n", _("Interacting with Others"));
+ list_commands_by_category(CAT_foreignscminterface, cmds, nr, longest);
+
+ printf("\n%s\n\n", _("Low-level Commands / Manipulators"));
+ list_commands_by_category(CAT_plumbingmanipulators, cmds, nr, longest);
+
+ printf("\n%s\n\n", _("Low-level Commands / Interrogators"));
+ list_commands_by_category(CAT_plumbinginterrogators, cmds, nr, longest);
+
+ printf("\n%s\n\n", _("Low-level Commands / Synching Repositories"
[PATCH v3 6/6] help: use command-list.txt for the source of guides
The help command currently hard codes the list of guides and their
summary in C. Let's move this list to command-list.txt. This lets us
extract summary lines from Documentation/git*.txt. This also
potentially lets us lists guides in git.txt, but I'll leave that for
now.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/gitattributes.txt | 2 +-
Documentation/gitmodules.txt| 2 +-
Documentation/gitrevisions.txt | 2 +-
builtin/help.c | 32
command-list.txt| 8
generate-cmdlist.sh | 6 +-
help.c | 22 ++
help.h | 1 +
8 files changed, 39 insertions(+), 36 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 1094fe2b5b..083c2f380d 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -3,7 +3,7 @@ gitattributes(5)
NAME
-gitattributes - defining attributes per path
+gitattributes - Defining attributes per path
SYNOPSIS
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index db5d47eb19..4d63def206 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -3,7 +3,7 @@ gitmodules(5)
NAME
-gitmodules - defining submodule properties
+gitmodules - Defining submodule properties
SYNOPSIS
diff --git a/Documentation/gitrevisions.txt b/Documentation/gitrevisions.txt
index 27dec5b91d..1f6cceaefb 100644
--- a/Documentation/gitrevisions.txt
+++ b/Documentation/gitrevisions.txt
@@ -3,7 +3,7 @@ gitrevisions(7)
NAME
-gitrevisions - specifying revisions and ranges for Git
+gitrevisions - Specifying revisions and ranges for Git
SYNOPSIS
diff --git a/builtin/help.c b/builtin/help.c
index 0e0af8426a..5727fb5e51 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -402,38 +402,6 @@ static void show_html_page(const char *git_cmd)
open_html(page_path.buf);
}
-static struct {
- const char *name;
- const char *help;
-} common_guides[] = {
- { "attributes", N_("Defining attributes per path") },
- { "everyday", N_("Everyday Git With 20 Commands Or So") },
- { "glossary", N_("A Git glossary") },
- { "ignore", N_("Specifies intentionally untracked files to ignore") },
- { "modules", N_("Defining submodule properties") },
- { "revisions", N_("Specifying revisions and ranges for Git") },
- { "tutorial", N_("A tutorial introduction to Git (for version 1.5.1 or
newer)") },
- { "workflows", N_("An overview of recommended workflows with Git") },
-};
-
-static void list_common_guides_help(void)
-{
- int i, longest = 0;
-
- for (i = 0; i < ARRAY_SIZE(common_guides); i++) {
- if (longest < strlen(common_guides[i].name))
- longest = strlen(common_guides[i].name);
- }
-
- puts(_("The common Git guides are:\n"));
- for (i = 0; i < ARRAY_SIZE(common_guides); i++) {
- printf(" %s ", common_guides[i].name);
- mput_char(' ', longest - strlen(common_guides[i].name));
- puts(_(common_guides[i].help));
- }
- putchar('\n');
-}
-
static const char *check_git_cmd(const char* cmd)
{
char *alias;
diff --git a/command-list.txt b/command-list.txt
index 2c81d8db74..1835f1a928 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -149,3 +149,11 @@ gitweb
ancillaryinterrogators
git-whatchanged ancillaryinterrogators
git-worktreemainporcelain
git-write-tree plumbingmanipulators
+gitattributes guide
+giteveryday guide
+gitglossary guide
+gitignore guide
+gitmodules guide
+gitrevisionsguide
+gittutorial guide
+gitworkflowsguide
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 05722b1392..86d59419b3 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -58,7 +58,11 @@ command_list "$1" |
sort |
while read cmd category tags
do
- prefix=git-
+ if [ "$category" = guide ]; then
+ prefix=git
+ else
+ prefix=git-
+ fi
name=$(echo $cmd | sed "s/^${prefix}//")
sed -n '
/^NAME/,/'"$cmd"'/H
diff --git a/help.c b/help.c
index ae5281601a..88127fdd6f 100644
--- a/help.c
+++ b/help.c
@@ -314,6 +314,28 @@ static void list_commands_by_category(int cat, struct
cmdname_help *cmds,
}
}
+void list_common_guides_help(void)
+{
+ int i, longest = 0;
+ int nr = ARRAY_SIZE(command_list);
+ struct cmdname_help *cmds = command_list;
+
+ QSORT(cmds, nr, cmd_catego
[PATCH v3 0/6] Keep all info in command-list.txt in git binary
Changes: - remove the deprecated column in command-list.txt. My change break it anyway if anyone uses it. - fix up failed tests that I marked in the RFC and kinda forgot about it. - fix bashisms in generate-cmdlist.sh - fix segfaul in "git help" Nguyễn Thái Ngọc Duy (6): git.c: convert --list-*builtins to --list-cmds=* git.c: implement --list-cmds=all and use it in git-completion.bash generate-cmdlist.sh: keep all information in common-cmds.h git.c: implement --list-cmds=porcelain help: add "-a --verbose" to list all commands with synopsis help: use command-list.txt for the source of guides Documentation/git-help.txt | 4 +- Documentation/gitattributes.txt| 2 +- Documentation/gitmodules.txt | 2 +- Documentation/gitrevisions.txt | 2 +- builtin/help.c | 39 ++ command-list.txt | 10 +- contrib/completion/git-completion.bash | 108 ++-- generate-cmdlist.sh| 57 ++--- git.c | 16 ++- help.c | 164 - help.h | 4 + t/t0012-help.sh| 11 +- t/t9902-completion.sh | 6 +- 13 files changed, 263 insertions(+), 162 deletions(-) -- 2.17.0.367.g5dd2e386c3
[PATCH v3 3/6] generate-cmdlist.sh: keep all information in common-cmds.h
common-cmds.h is used to extract the list of common commands (by
group) and a one-line summary of each command. Some information is
dropped, for example command category or summary of other commands.
Update generate-cmdlist.sh to keep all the information. The extra info
will be used shortly.
The "deprecated" column is dropped from command-list.txt since it's
not really used and the new parsing code can't deal with it. When we
do need deprecated attribute, we could add it back and adjust
generate-cmdlist.sh at the same time.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
command-list.txt| 2 +-
generate-cmdlist.sh | 53 +++--
help.c | 43 +++-
3 files changed, 75 insertions(+), 23 deletions(-)
diff --git a/command-list.txt b/command-list.txt
index a1fad28fd8..2c81d8db74 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -9,7 +9,7 @@ history grow, mark and tweak your common history
remote collaborate (see also: git help workflows)
### command list (do not change this line)
-# command name category [deprecated] [common]
+# command name category[common]
git-add mainporcelain worktree
git-am mainporcelain
git-annotateancillaryinterrogators
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index eeea4b67ea..05722b1392 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -1,17 +1,30 @@
#!/bin/sh
+# Don't let locale affect this script.
+LC_ALL=C
+LANG=C
+export LC_ALL LANG
+
+command_list () {
+ sed '1,/^### command list/d;/^#/d' "$1"
+}
+
+category_list () {
+ command_list "$1" | awk '{print $2;}' | sort | uniq
+}
+
echo "/* Automatically generated by generate-cmdlist.sh */
struct cmdname_help {
- char name[16];
+ char name[32];
char help[80];
- unsigned char group;
+ unsigned int category;
+ unsigned int group;
};
static const char *common_cmd_groups[] = {"
grps=grps$$.tmp
-match=match$$.tmp
-trap "rm -f '$grps' '$match'" 0 1 2 3 15
+trap "rm -f '$grps'" 0 1 2 3 15
sed -n '
1,/^### common groups/b
@@ -23,28 +36,36 @@ sed -n '
' "$1"
printf '};\n\n'
+echo "#define GROUP_NONE 0xff"
n=0
-substnum=
while read grp
do
- echo "^git-..*[ ]$grp"
- substnum="$substnum${substnum:+;}s/[]$grp/$n/"
+ echo "#define GROUP_${grp:-NONE} $n"
n=$(($n+1))
-done <"$grps" >"$match"
+done <"$grps"
+echo
+
+n=0
+category_list "$1" |
+while read category; do
+ echo "#define CAT_$category $n"
+ n=$(($n+1))
+done
+echo
-printf 'static struct cmdname_help common_cmds[] = {\n'
-grep -f "$match" "$1" |
-sed 's/^git-//' |
+printf 'static struct cmdname_help command_list[] = {\n'
+command_list "$1" |
sort |
-while read cmd tags
+while read cmd category tags
do
- tag=$(echo "$tags" | sed "$substnum; s/[^0-9]//g")
+ prefix=git-
+ name=$(echo $cmd | sed "s/^${prefix}//")
sed -n '
- /^NAME/,/git-'"$cmd"'/H
+ /^NAME/,/'"$cmd"'/H
${
x
- s/.*git-'"$cmd"' - \(.*\)/ {"'"$cmd"'", N_("\1"),
'$tag'},/
+ s/.*'"$cmd"' - \(.*\)/ {"'"$name"'", N_("\1"),
CAT_'$category', GROUP_'${tags:-NONE}' },/
p
- }' "Documentation/git-$cmd.txt"
+ }' "Documentation/$cmd.txt"
done
echo "};"
diff --git a/help.c b/help.c
index e155c39870..e63006c333 100644
--- a/help.c
+++ b/help.c
@@ -190,6 +190,28 @@ void list_commands(unsigned int colopts,
}
}
+static void extract_common_cmds(struct cmdname_help **p_common_cmds,
+ int *p_nr)
+{
+ int i, nr = 0;
+ struct cmdname_help *common_cmds;
+
+ ALLOC_ARRAY(common_cmds, ARRAY_SIZE(command_list));
+
+ for (i = 0; i < ARRAY_SIZE(command_list); i++) {
+ const struct cmdname_help *cmd = command_list + i;
+
+ if (cmd->category != CAT_mainporcelain ||
+ cmd->group == GROUP_NONE)
+ continue;
+
+ common_cmds[nr++] = *cmd;
+ }
+
+ *p_common_cmds = common_cmds;
+ *p_nr = nr;
+}
+
static int cmd_group_cmp(const void *elem1, const void *elem2)
{
const struct cmdname_help *e1 = elem1;
@@ -206,17 +228,21 @@ void list_common_cmds_help(void)
{
int i, longest = 0;
int current_grp = -1;
+ int nr = 0;
+ struct cmdname_help *common_cmds;
+
+ extract_common_cmds(&common_cmds, &nr);
- for (i = 0; i < ARRAY_SIZE(common_cmds); i++) {
+ for (i = 0; i < nr; i++) {
if (longest < strlen(common_cmds[i].name))
longest = strlen(common_cmds[i].name);
}
- QSORT(common_cmd
[PATCH v3 2/6] git.c: implement --list-cmds=all and use it in git-completion.bash
Signed-off-by: Nguyễn Thái Ngọc Duy
---
contrib/completion/git-completion.bash | 2 +-
git.c | 2 ++
help.c | 18 ++
help.h | 1 +
4 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 3556838759..a5f13ade20 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -839,7 +839,7 @@ __git_commands () {
then
printf "%s" "${GIT_TESTING_COMMAND_COMPLETION}"
else
- git help -a|egrep '^ [a-zA-Z0-9]'
+ git --list-cmds=all
fi
}
diff --git a/git.c b/git.c
index 28bfa96d87..64f67e7f7f 100644
--- a/git.c
+++ b/git.c
@@ -228,6 +228,8 @@ static int handle_options(const char ***argv, int *argc,
int *envchanged)
list_builtins(0, '\n');
else if (!strcmp(cmd, "parseopt"))
list_builtins(NO_PARSEOPT, ' ');
+ else if (!strcmp(cmd, "all"))
+ list_all_cmds();
else
die("unsupported command listing type '%s'",
cmd);
exit(0);
diff --git a/help.c b/help.c
index 60071a9bea..e155c39870 100644
--- a/help.c
+++ b/help.c
@@ -228,6 +228,24 @@ void list_common_cmds_help(void)
}
}
+void list_all_cmds(void)
+{
+ struct cmdnames main_cmds, other_cmds;
+ int i;
+
+ memset(&main_cmds, 0, sizeof(main_cmds));
+ memset(&other_cmds, 0, sizeof(other_cmds));
+ load_command_list("git-", &main_cmds, &other_cmds);
+
+ for (i = 0; i < main_cmds.cnt; i++)
+ puts(main_cmds.names[i]->name);
+ for (i = 0; i < other_cmds.cnt; i++)
+ puts(other_cmds.names[i]->name);
+
+ clean_cmdnames(&main_cmds);
+ clean_cmdnames(&other_cmds);
+}
+
int is_in_cmdlist(struct cmdnames *c, const char *s)
{
int i;
diff --git a/help.h b/help.h
index b21d7c94e8..0bf29f8dc5 100644
--- a/help.h
+++ b/help.h
@@ -17,6 +17,7 @@ static inline void mput_char(char c, unsigned int num)
}
extern void list_common_cmds_help(void);
+extern void list_all_cmds(void);
extern const char *help_unknown_cmd(const char *cmd);
extern void load_command_list(const char *prefix,
struct cmdnames *main_cmds,
--
2.17.0.367.g5dd2e386c3
[PATCH v3 1/6] git.c: convert --list-*builtins to --list-cmds=*
Even if these are hidden options, let's make them a bit more generic
since we're introducing more listing types shortly.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
contrib/completion/git-completion.bash | 2 +-
git.c | 12 +++-
t/t0012-help.sh| 2 +-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index a757073945..3556838759 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3049,7 +3049,7 @@ __git_complete_common () {
__git_cmds_with_parseopt_helper=
__git_support_parseopt_helper () {
test -n "$__git_cmds_with_parseopt_helper" ||
- __git_cmds_with_parseopt_helper="$(__git
--list-parseopt-builtins)"
+ __git_cmds_with_parseopt_helper="$(__git --list-cmds=parseopt)"
case " $__git_cmds_with_parseopt_helper " in
*" $1 "*)
diff --git a/git.c b/git.c
index 3a89893712..28bfa96d87 100644
--- a/git.c
+++ b/git.c
@@ -223,11 +223,13 @@ static int handle_options(const char ***argv, int *argc,
int *envchanged)
}
(*argv)++;
(*argc)--;
- } else if (!strcmp(cmd, "--list-builtins")) {
- list_builtins(0, '\n');
- exit(0);
- } else if (!strcmp(cmd, "--list-parseopt-builtins")) {
- list_builtins(NO_PARSEOPT, ' ');
+ } else if (skip_prefix(cmd, "--list-cmds=", &cmd)) {
+ if (!strcmp(cmd, "builtins"))
+ list_builtins(0, '\n');
+ else if (!strcmp(cmd, "parseopt"))
+ list_builtins(NO_PARSEOPT, ' ');
+ else
+ die("unsupported command listing type '%s'",
cmd);
exit(0);
} else {
fprintf(stderr, _("unknown option: %s\n"), cmd);
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 487b92a5de..fd2a7f27dc 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -50,7 +50,7 @@ test_expect_success "--help does not work for guides" "
"
test_expect_success 'generate builtin list' '
- git --list-builtins >builtins
+ git --list-cmds=builtins >builtins
'
while read builtin
--
2.17.0.367.g5dd2e386c3
Re: [PATCH v8 06/16] sequencer: introduce the `merge` command
On 21/04/18 11:33, Johannes Schindelin wrote:
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 labeled commits. This patch adds the `merge`
command, with the following syntax:
The two patches seem to have been fused together in this series.
If the reset command fails because it would overwrite untracked files it
says
error: Untracked working tree file 'b' would be overwritten by merge.
Followed by the hint to edit the todo file. Saying 'merge' rather
'reset' is possibly confusing to users. Perhaps it could call
setup_unpack_trees_porcelain(), though that would need to be extended to
handle 'reset'. Also it currently refuses to overwrite ignored files
which is either annoying or safe depending on one's point of view.
Best Wishes
Phillip
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 | 6 +
sequencer.c| 407 -
2 files changed, 406 insertions(+), 7 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index e1b865f43f2..ccd5254d1c9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -162,6 +162,12 @@ 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
+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 01443e0f245..35fcacbdf0f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -23,6 +23,8 @@
#include "hashmap.h"
#include "notes-utils.h"
#include "sigchain.h"
+#include "unpack-trees.h"
+#include "worktree.h"
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -120,6 +122,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).
@@ -244,18 +253,34 @@ 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 (is_rebase_i(opts) &&
+ strbuf_read_file(&buf, 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)
+
Re: [PATCH 3/3] rebase --skip: clean up commit message after a failedfixup/squash
On 20/04/18 13:18, Johannes Schindelin wrote:
During a series of fixup/squash commands, the interactive rebase builds
up a commit message with comments. This will be presented to the user in
the editor if at least one of those commands was a `squash`.
However, if the last of these fixup/squash commands fails with merge
conflicts, and if the user then decides to skip it (or resolve it to a
clean worktree and then continue the rebase), the current code fails to
clean up the commit message.
Thanks for taking the time to track this down and fix it.
This commit fixes that behavior.
The diff is best viewed with --color-moved.
Signed-off-by: Johannes Schindelin
---
sequencer.c| 36
t/t3418-rebase-continue.sh | 2 +-
2 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index a9c3bc26f84..f067b7b24c5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2781,17 +2781,12 @@ static int continue_single_pick(void)
static int commit_staged_changes(struct replay_opts *opts)
{
- unsigned int flags = ALLOW_EMPTY | EDIT_MSG;
+ unsigned int flags = ALLOW_EMPTY | EDIT_MSG, is_fixup = 0, is_clean;
if (has_unstaged_changes(1))
return error(_("cannot rebase: You have unstaged changes."));
- if (!has_uncommitted_changes(0)) {
- const char *cherry_pick_head = git_path_cherry_pick_head();
- if (file_exists(cherry_pick_head) && unlink(cherry_pick_head))
- return error(_("could not remove CHERRY_PICK_HEAD"));
- return 0;
- }
+ is_clean = !has_uncommitted_changes(0);
if (file_exists(rebase_path_amend())) {
struct strbuf rev = STRBUF_INIT;
@@ -2804,16 +2799,41 @@ static int commit_staged_changes(struct replay_opts
*opts)
if (get_oid_hex(rev.buf, &to_amend))
return error(_("invalid contents: '%s'"),
rebase_path_amend());
- if (oidcmp(&head, &to_amend))
+ if (!is_clean && oidcmp(&head, &to_amend))
return error(_("\nYou have uncommitted changes in your "
"working tree. Please, commit them\n"
"first and then run 'git rebase "
"--continue' again."));
+ if (is_clean && !oidcmp(&head, &to_amend)) {
Looking at pick_commits() it only writes to rebase_path_amend() if there
are conflicts, not if the command has been rescheduled so this is safe.
+ strbuf_reset(&rev);
+ /*
+* Clean tree, but we may need to finalize a
+* fixup/squash chain. A failed fixup/squash leaves the
+* file amend-type in rebase-merge/; It is okay if that
+* file is missing, in which case there is no such
+* chain to finalize.
+*/
+ read_oneliner(&rev, rebase_path_amend_type(), 0);
+ if (!strcmp("squash", rev.buf))
+ is_fixup = TODO_SQUASH;
+ else if (!strcmp("fixup", rev.buf)) {
+ is_fixup = TODO_FIXUP;
+ flags = (flags & ~EDIT_MSG) | CLEANUP_MSG;
I was going to say this should probably be (flags & ~(EDIT_MSG |
VERIFY_MSG)) but for some reason VERIFY_MSG isn't set here - I wonder if
it should be as I think it's set elsewhere when we edit the message.
+ }
+ }
strbuf_release(&rev);
flags |= AMEND_MSG;
}
+ if (is_clean && !is_fixup) {
+ const char *cherry_pick_head = git_path_cherry_pick_head();
+
+ if (file_exists(cherry_pick_head) && unlink(cherry_pick_head))
+ return error(_("could not remove CHERRY_PICK_HEAD"));
+ return 0;
+ }
+
if (run_git_commit(rebase_path_message(), opts, flags))
If a squash command has been skipped, then rebase_path_message() still
contains the message of the skipped commit. If it passed NULL instead
then the user would get to edit the previous version of the squash
message without the skipped commit message in it.
Also I think we only want to re-commit if the skipped squash/fixup was
preceded by another squash/fixup. If the user skips the first
squash/fixup in a chain then HEAD has the commit message from the
original pick so does not need amending. The first patch could perhaps
avoid writing rebase_path_amend_type() in that case by reading the
squash message and checking the message count is greater than two.
Best Wishes
Phillip
return error(_("could not commit staged changes."));
unlink(rebase_path_amend());
diff --git a/t/t34
[no subject]
I didn't get any reply from you concerning my last email.. Br LiN __ Sky Silk, http://aknet.kz
[PATCH v2 3/3] Avoid multiple PREFIX definitions
From: Philip Oakley
The short and sweet PREFIX can be confused when used in many places.
Rename both usages to better describe their purpose. EXEC_CMD_PREFIX is
used in full to disambiguate it from the nearby GIT_EXEC_PATH.
The PREFIX in sideband.c, while nominally independant of the exec_cmd
PREFIX, does reside within libgit[1], so the definitions would clash
when taken together with a PREFIX given on the command line for use by
exec_cmd.c.
Noticed when compiling Git for Windows using MSVC/Visual Studio [1] which
reports the conflict beteeen the command line definition and the
definition in sideband.c within the libgit project.
[1] the libgit functions are brought into a single sub-project
within the Visual Studio construction script provided in contrib,
and hence uses a single command for both exec_cmd.c and sideband.c.
Signed-off-by: Philip Oakley
Signed-off-by: Johannes Schindelin
---
Makefile | 2 +-
exec-cmd.c | 4 ++--
sideband.c | 10 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 111e93d3bea..49cec672242 100644
--- a/Makefile
+++ b/Makefile
@@ -2271,7 +2271,7 @@ exec-cmd.sp exec-cmd.s exec-cmd.o: EXTRA_CPPFLAGS = \
'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
'-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \
'-DBINDIR="$(bindir_relative_SQ)"' \
- '-DPREFIX="$(prefix_SQ)"'
+ '-DFALLBACK_RUNTIME_PREFIX="$(prefix_SQ)"'
builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX
builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \
diff --git a/exec-cmd.c b/exec-cmd.c
index 3b0a039083a..02d31ee8971 100644
--- a/exec-cmd.c
+++ b/exec-cmd.c
@@ -48,7 +48,7 @@ static const char *system_prefix(void)
!(prefix = strip_path_suffix(executable_dirname, GIT_EXEC_PATH)) &&
!(prefix = strip_path_suffix(executable_dirname, BINDIR)) &&
!(prefix = strip_path_suffix(executable_dirname, "git"))) {
- prefix = PREFIX;
+ prefix = FALLBACK_RUNTIME_PREFIX;
trace_printf("RUNTIME_PREFIX requested, "
"but prefix computation failed. "
"Using static fallback '%s'.\n", prefix);
@@ -243,7 +243,7 @@ void git_resolve_executable_dir(const char *argv0)
*/
static const char *system_prefix(void)
{
- return PREFIX;
+ return FALLBACK_RUNTIME_PREFIX;
}
/*
diff --git a/sideband.c b/sideband.c
index 6d7f943e438..325bf0e974a 100644
--- a/sideband.c
+++ b/sideband.c
@@ -13,7 +13,7 @@
* the remote died unexpectedly. A flush() concludes the stream.
*/
-#define PREFIX "remote: "
+#define DISPLAY_PREFIX "remote: "
#define ANSI_SUFFIX "\033[K"
#define DUMB_SUFFIX ""
@@ -49,7 +49,7 @@ int recv_sideband(const char *me, int in_stream, int out)
switch (band) {
case 3:
strbuf_addf(&outbuf, "%s%s%s", outbuf.len ? "\n" : "",
- PREFIX, buf + 1);
+ DISPLAY_PREFIX, buf + 1);
retval = SIDEBAND_REMOTE_ERROR;
break;
case 2:
@@ -67,7 +67,7 @@ int recv_sideband(const char *me, int in_stream, int out)
int linelen = brk - b;
if (!outbuf.len)
- strbuf_addstr(&outbuf, PREFIX);
+ strbuf_addstr(&outbuf, DISPLAY_PREFIX);
if (linelen > 0) {
strbuf_addf(&outbuf, "%.*s%s%c",
linelen, b, suffix, *brk);
@@ -81,8 +81,8 @@ int recv_sideband(const char *me, int in_stream, int out)
}
if (*b)
- strbuf_addf(&outbuf, "%s%s",
- outbuf.len ? "" : PREFIX, b);
+ strbuf_addf(&outbuf, "%s%s", outbuf.len ?
+ "" : DISPLAY_PREFIX, b);
break;
case 1:
write_or_die(out, buf + 1, len);
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v2 1/3] gettext: avoid initialization if the locale dir is not present
The runtime of a simple `git.exe version` call on Windows is currently
dominated by the gettext setup, adding a whopping ~150ms to the ~210ms
total.
Given that this cost is added to each and every git.exe invocation goes
through common-main's invocation of git_setup_gettext(), and given that
scripts have to call git.exe dozens, if not hundreds, of times, this is
a substantial performance penalty.
This is particularly pointless when considering that Git for Windows
ships without localization (to keep the installer's size to a bearable
~34MB): all that time setting up gettext is for naught.
To be clear, Git for Windows *needs* to be compiled with localization,
for the following reasons:
- to allow users to copy add-on localization in case they want it, and
- to fix the nasty error message
BUG: your vsnprintf is broken (returned -1)
by using libgettext's override of vsnprintf() that does not share the
behavior of msvcrt.dll's version of vsnprintf().
So let's be smart about it and skip setting up gettext if the locale
directory is not even present.
Since localization might be missing for not-yet-supported locales, this
will not break anything.
Signed-off-by: Johannes Schindelin
---
gettext.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gettext.c b/gettext.c
index baba28343c3..701355d66e7 100644
--- a/gettext.c
+++ b/gettext.c
@@ -163,6 +163,9 @@ void git_setup_gettext(void)
if (!podir)
podir = system_path(GIT_LOCALE_PATH);
+ if (!is_directory(podir))
+ return;
+
bindtextdomain("git", podir);
setlocale(LC_MESSAGES, "");
setlocale(LC_TIME, "");
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v2 0/3] Some add-on patches on top of dj/runtime-prefix
We carried a slightly different version of the git_setup_gettext() patch (which took care of releasing the buffer allocated by system_path()), and we carry two more patches that touch the same area, so now that dj/runtime-prefix hit `next`, it seems a good time to contribute those, too. Changes since v1: - clarified in v1 why we cannot simply force users to recompile with NO_GETTEXT instead. Johannes Schindelin (2): gettext: avoid initialization if the locale dir is not present git_setup_gettext: plug memory leak Philip Oakley (1): Avoid multiple PREFIX definitions Makefile | 2 +- exec-cmd.c | 4 ++-- gettext.c | 10 +- sideband.c | 10 +- 4 files changed, 17 insertions(+), 9 deletions(-) base-commit: b46fe60e1d7235603a29499822493bd3791195da Published-As: https://github.com/dscho/git/releases/tag/runtime-prefix-addons-v2 Fetch-It-Via: git fetch https://github.com/dscho/git runtime-prefix-addons-v2 -- 2.17.0.windows.1.15.gaa56ade3205
[PATCH v2 2/3] git_setup_gettext: plug memory leak
The system_path() function returns a freshly-allocated string. We need
to release it.
Signed-off-by: Johannes Schindelin
---
gettext.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/gettext.c b/gettext.c
index 701355d66e7..7272771c8e4 100644
--- a/gettext.c
+++ b/gettext.c
@@ -159,18 +159,23 @@ static void init_gettext_charset(const char *domain)
void git_setup_gettext(void)
{
const char *podir = getenv(GIT_TEXT_DOMAIN_DIR_ENVIRONMENT);
+ char *p = NULL;
if (!podir)
- podir = system_path(GIT_LOCALE_PATH);
+ podir = p = system_path(GIT_LOCALE_PATH);
- if (!is_directory(podir))
+ if (!is_directory(podir)) {
+ free(p);
return;
+ }
bindtextdomain("git", podir);
setlocale(LC_MESSAGES, "");
setlocale(LC_TIME, "");
init_gettext_charset("git");
textdomain("git");
+
+ free(p);
}
/* return the number of columns of string 's' in current locale */
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v8 16/16] rebase -i --rebase-merges: add a section to the man page
The --rebase-merges mode is probably not half as intuitive to use as its inventor hopes, so let's document it some. Signed-off-by: Johannes Schindelin --- Documentation/git-rebase.txt | 135 +++ 1 file changed, 135 insertions(+) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 841cf9cf385..3b996e46d6a 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -399,6 +399,8 @@ inserted and dropped at will. It is currently only possible to recreate the merge commits using the `recursive` merge strategy; Different merge strategies can be used only via explicit `exec git merge -s [...]` commands. ++ +See also REBASING MERGES below. -p:: --preserve-merges:: @@ -797,6 +799,139 @@ The ripple effect of a "hard case" recovery is especially bad: 'everyone' downstream from 'topic' will now have to perform a "hard case" recovery too! +REBASING MERGES +- + +The interactive rebase command was originally designed to handle +individual patch series. As such, it makes sense to exclude merge +commits from the todo list, as the developer may have merged the +then-current `master` while working on the branch, only to rebase +all the commits onto `master` eventually (skipping the merge +commits). + +However, there are legitimate reasons why a developer may want to +recreate merge commits: to keep the branch structure (or "commit +topology") when working on multiple, inter-related branches. + +In the following example, the developer works on a topic branch that +refactors the way buttons are defined, and on another topic branch +that uses that refactoring to implement a "Report a bug" button. The +output of `git log --graph --format=%s -5` may look like this: + + +* Merge branch 'report-a-bug' +|\ +| * Add the feedback button +* | Merge branch 'refactor-button' +|\ \ +| |/ +| * Use the Button class for all buttons +| * Extract a generic Button class from the DownloadButton one + + +The developer might want to rebase those commits to a newer `master` +while keeping the branch topology, for example when the first topic +branch is expected to be integrated into `master` much earlier than the +second one, say, to resolve merge conflicts with changes to the +DownloadButton class that made it into `master`. + +This rebase can be performed using the `--rebase-merges` option. +It will generate a todo list looking like this: + + +label onto + +# Branch: refactor-button +reset onto +pick 123456 Extract a generic Button class from the DownloadButton one +pick 654321 Use the Button class for all buttons +label refactor-button + +# Branch: report-a-bug +reset refactor-button # Use the Button class for all buttons +pick abcdef Add the feedback button +label report-a-bug + +reset onto +merge -C a1b2c3 refactor-button # Merge 'refactor-button' +merge -C 6f5e4d report-a-bug # Merge 'report-a-bug' + + +In contrast to a regular interactive rebase, there are `label`, `reset` +and `merge` commands in addition to `pick` ones. + +The `label` command associates a label with the current HEAD when that +command is executed. These labels are created as worktree-local refs +(`refs/rewritten/`) that will be deleted when the rebase +finishes. That way, rebase operations in multiple worktrees linked to +the same repository do not interfere with one another. If the `label` +command fails, it is rescheduled immediately, with a helpful message how +to proceed. + +The `reset` command resets the HEAD, index and worktree to the specified +revision. It is isimilar to an `exec git reset --hard `, but +refuses to overwrite untracked files. If the `reset` command fails, it is +rescheduled immediately, with a helpful message how to edit the todo list +(this typically happens when a `reset` command was inserted into the todo +list manually and contains a typo). + +The `merge` command will merge the specified revision into whatever is +HEAD at that time. With `-C `, the commit message of +the specified merge commit will be used. When the `-C` is changed to +a lower-case `-c`, the message will be opened in an editor after a +successful merge so that the user can edit the message. + +If a `merge` command fails for any reason other than merge conflicts (i.e. +when the merge operation did not even start), it is rescheduled immediately. + +At this time, the `merge` command will *always* use the `recursive` +merge strategy, with no way to choose a different one. To work around +this, an `exec` command can be used to call `git merge` explicitly, +using the fact that the labels are worktree-local refs (the ref +`refs/rewritten/onto` would correspond to the label `onto`, for example). + +Note: the first command (`label onto`) labels the revision onto which +the commits are rebased; The name `onto` is just a convention, as a nod +to the `--onto` option. + +It is also possible to introduce completely new merge commits
[PATCH v8 15/16] rebase -i: introduce --rebase-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 --rebase-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 --rebase-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-merges.sh | 18 ++
7 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 34e0f6a69c1..841cf9cf385 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -379,7 +379,7 @@ rebase.instructionFormat. A customized instruction format
will automatically
have the long commit hash prepended to the format.
-r::
---rebase-merges::
+--rebase-merges[=(rebase-cousins|no-rebase-cousins)]::
By default, a rebase will simply drop merge commits and only rebase
the non-merge commits. With this option, it will try to preserve
the branching structure within the commits that are to be rebased,
@@ -387,6 +387,11 @@ have the long commit hash prepended to the format.
or contained manual amendments, then they will have to be re-applied
manually.
+
+By default, or when `no-rebase-cousins` was specified, commits which do not
+have `` as direct ancestor will keep their original branch point.
+If the `rebase-cousins` mode is turned on, such commits are instead rebased
+onto `` (or ``, if specified).
++
This mode is similar in spirit to `--preserve-merges`, but in contrast to
that option works well in interactive rebases: commits can be reordered,
inserted and dropped at will.
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 781782e7272..f7c2a5fdc81 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, rebase_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,
@@ -25,6 +25,8 @@ int cmd_rebase__helper(int argc, const char **argv, const
char *prefix)
OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message,
N_("allow commits with empty messages")),
OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge
commits")),
+ OPT_BOOL(0, "rebase-cousins", &rebase_cousins,
+N_("keep original branch points of cousins")),
OPT_CMDMODE(0, "continue", &command, N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", &command, N_("abort rebase"),
@@ -59,8 +61,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 |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
+ flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
+ if (rebase_cousins >= 0 && !rebase_merges)
+ warning(_("--[no-]rebase-cousins has no effect without "
+ "--rebase-merges"));
+
if (command == CONTINUE && argc == 1)
return !!sequencer_continue(&opts);
if (command == ABORT && argc == 1)
diff --git a/git-rebase--interac
[PATCH v8 14/16] pull: accept --rebase=merges to recreate the branch topology
Similar to the `preserve` mode simply passing the `--preserve-merges`
option to the `rebase` command, the `merges` mode simply passes the
`--rebase-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 | 6 +-
builtin/pull.c | 14 ++
builtin/remote.c | 18 ++
contrib/completion/git-completion.bash | 2 +-
5 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2659153cb37..d6bcb5dcb67 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 `merges`, pass the `--rebase-merges` option to 'git rebase'
+so that the local merge commits are included in the rebase (see
+linkgit:git-rebase[1] for details).
++
When preserve, also pass `--preserve-merges` along to 'git rebase'
so that locally committed merge commits will not be flattened
by running 'git pull'.
@@ -2617,6 +2621,10 @@ pull.rebase::
pull" is run. See "branch..rebase" for setting this on a
per-branch basis.
+
+When `merges`, pass the `--rebase-merges` option to 'git rebase'
+so that the local merge commits are included in the rebase (see
+linkgit:git-rebase[1] for details).
++
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..4e0ad6fd8e0 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -101,13 +101,17 @@ Options related to merging
include::merge-options.txt[]
-r::
---rebase[=false|true|preserve|interactive]::
+--rebase[=false|true|merges|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 `merges`, rebase using `git rebase --rebase-merges` so that
+the local merge commits are included in the rebase (see
+linkgit:git-rebase[1] for details).
++
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 e32d6cd5b4c..70b44146ce4 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -27,14 +27,16 @@ enum rebase_type {
REBASE_FALSE = 0,
REBASE_TRUE,
REBASE_PRESERVE,
+ REBASE_MERGES,
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.
+ * "merges", returns REBASE_MERGES. 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, "merges"))
+ return REBASE_MERGES;
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", &opt_rebase,
- "false|true|preserve|interactive",
+ "false|true|merges|preserve|interactive",
N_("incorporate changes by rebasing rather than merging"),
PARSE_OPT_OPTARG, parse_opt_rebase },
OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
@@ -800,7 +804,9 @@ static int run_rebase(const struct object_id *curr_head,
argv_push_verbosity(&args);
/* Options passed to git-rebase */
- if (opt_rebase == REBASE_PRESERVE)
+ if (opt_rebase == REBASE_MERGES)
+ argv_array_push(&args, "--rebase-merges");
+ else if (opt_rebase == REBASE_PRESERVE)
argv_array_push(&args, "--preserve-merges");
else if (opt_rebase == REBA
[PATCH v8 13/16] rebase --rebase-merges: avoid "empty merges"
The `git merge` command does not allow merging commits that are already
reachable from HEAD: `git merge HEAD^`, for example, will report that we
are already up to date and not change a thing.
In an interactive rebase, such a merge could occur previously, e.g. when
competing (or slightly modified) versions of a patch series were applied
upstream, and the user had to `git rebase --skip` all of the local
commits, and the topic branch becomes "empty" as a consequence.
Let's teach the todo command `merge` to behave the same as `git merge`.
Seeing as it requires some low-level trickery to create such merges with
Git's commands in the first place, we do not even have to bother to
introduce an option to force `merge` to create such merge commits.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 7 +++
t/t3430-rebase-merges.sh | 8
2 files changed, 15 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index f3b4fe4d75f..443a0a0ee87 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2803,6 +2803,13 @@ static int do_merge(struct commit *commit, const char
*arg, int arg_len,
write_message("no-ff", 5, git_path_merge_mode(), 0);
bases = get_merge_bases(head_commit, merge_commit);
+ if (bases && !oidcmp(&merge_commit->object.oid,
+&bases->item->object.oid)) {
+ ret = 0;
+ /* skip merging an ancestor of HEAD */
+ goto leave_merge;
+ }
+
for (j = bases; j; j = j->next)
commit_list_insert(j->item, &reversed);
free_commit_list(bases);
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index e9c5dc1cd95..1628c8dcc20 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -215,4 +215,12 @@ test_expect_success 'post-rewrite hook and fixups work for
merges' '
test_cmp expect actual
'
+test_expect_success 'refuse to merge ancestors of HEAD' '
+ echo "merge HEAD^" >script-from-scratch &&
+ test_config -C wt sequence.editor \""$PWD"/replace-editor.sh\" &&
+ before="$(git rev-parse HEAD)" &&
+ git rebase -i HEAD &&
+ test_cmp_rev HEAD $before
+'
+
test_done
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v8 12/16] sequencer: handle post-rewrite for merge commands
In the previous patches, we implemented the basic functionality of the `git rebase -i --rebase-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 hooks do not need to handle them. Signed-off-by: Johannes Schindelin --- sequencer.c | 5 - t/t3430-rebase-merges.sh | 25 + 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index 1e17a11ca32..f3b4fe4d75f 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3062,7 +3062,10 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) item->arg, item->arg_len, item->flags, opts)) < 0) reschedule = 1; - else if (res > 0) + else if (item->commit) + record_in_rewritten(&item->commit->object.oid, + peek_command(todo_list, 1)); + if (res > 0) /* failed with merge conflicts */ return error_with_patch(item->commit, item->arg, diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 96853784ec0..e9c5dc1cd95 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -190,4 +190,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 -r 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.17.0.windows.1.15.gaa56ade3205
[PATCH v8 11/16] 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-merges.sh | 14 ++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 8b7a77fe5ee..f61ec58d1df 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-merges.sh b/t/t3430-rebase-merges.sh
index 5f0febb9970..96853784ec0 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -176,4 +176,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.17.0.windows.1.15.gaa56ade3205
[PATCH v8 10/16] rebase --rebase-merges: add test for --keep-empty
From: Phillip Wood If there are empty commits on the left hand side of $upstream...HEAD then the empty commits on the right hand side that we want to keep are being pruned. Signed-off-by: Phillip Wood --- t/t3421-rebase-topology-linear.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index 68fe2003ef5..fbae5dab7e2 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -217,6 +217,7 @@ test_run_rebase success '' test_run_rebase failure -m test_run_rebase failure -i test_run_rebase failure -p +test_run_rebase success --rebase-merges # m # / -- 2.17.0.windows.1.15.gaa56ade3205
[PATCH v8 08/16] rebase-helper --make-script: introduce a flag to rebase 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 --rebase-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 series.
As a special, hard-coded label, all merge-rebasing 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 | 351 ++-
sequencer.h | 1 +
3 files changed, 353 insertions(+), 3 deletions(-)
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index ad074705bb5..781782e7272 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, rebase_merges = 0;
int abbreviate_commands = 0;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
@@ -24,6 +24,7 @@ int cmd_rebase__helper(int argc, const char **argv, const
char *prefix)
OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty
commits")),
OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message,
N_("allow commits with empty messages")),
+ OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge
commits")),
OPT_CMDMODE(0, "continue", &command, N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", &command, N_("abort rebase"),
@@ -57,6 +58,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 |= rebase_merges ? TODO_LIST_REBASE_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 5944d3a34eb..1e17a11ca32 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -25,6 +25,8 @@
#include "sigchain.h"
#include "unpack-trees.h"
#include "worktree.h"
+#include "oidmap.h"
+#include "oidset.h"
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -3436,6 +3438,343 @@ void append_signoff(struct strbuf *msgbuf, int
ignore_footer, unsigned flag)
strbuf_release(&sob);
}
+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(&state->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(&state->buf);
+
[PATCH v8 09/16] rebase: introduce the --rebase-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 `--rebase-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 | 20 ++- contrib/completion/git-completion.bash | 2 +- git-rebase--interactive.sh | 1 + git-rebase.sh | 6 + t/t3430-rebase-merges.sh | 179 + 5 files changed, 206 insertions(+), 2 deletions(-) create mode 100755 t/t3430-rebase-merges.sh diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 3277ca14327..34e0f6a69c1 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -378,6 +378,23 @@ 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. +-r:: +--rebase-merges:: + By default, a rebase will simply drop merge commits and only rebase + the non-merge commits. With this option, it will try to preserve + the branching structure within the commits that are to be rebased, + by recreating the merge commits. If a merge commit resolved any merge + or contained manual amendments, then they will have to be re-applied + manually. ++ +This mode is similar in spirit to `--preserve-merges`, but in contrast to +that option works well in interactive rebases: commits can be reordered, +inserted and dropped at will. ++ +It is currently only possible to recreate the merge commits using the +`recursive` merge strategy; Different merge strategies can be used only via +explicit `exec git merge -s [...]` commands. + -p:: --preserve-merges:: Recreate merge commits instead of flat
[PATCH v8 07/16] sequencer: fast-forward `merge` commands, if possible
Just like with regular `pick` commands, if we are trying to rebase 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 | 33 -
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 35fcacbdf0f..5944d3a34eb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2680,7 +2680,7 @@ static int do_merge(struct commit *commit, const char
*arg, int arg_len,
struct commit *head_commit, *merge_commit, *i;
struct commit_list *bases, *j, *reversed = NULL;
struct merge_options o;
- int merge_arg_len, oneline_offset, ret;
+ int merge_arg_len, oneline_offset, can_fast_forward, ret;
static struct lock_file lock;
const char *p;
@@ -2765,6 +2765,37 @@ static int do_merge(struct commit *commit, const char
*arg, int arg_len,
}
}
+ /*
+* If HEAD is not identical to the first parent of the original merge
+* commit, we cannot fast-forward.
+*/
+ can_fast_forward = opts->allow_ff && commit && commit->parents &&
+ !oidcmp(&commit->parents->item->object.oid,
+ &head_commit->object.oid);
+
+ /*
+* If the merge head is different from the original one, we cannot
+* fast-forward.
+*/
+ if (can_fast_forward) {
+ struct commit_list *second_parent = commit->parents->next;
+
+ if (second_parent && !second_parent->next &&
+ oidcmp(&merge_commit->object.oid,
+ &second_parent->item->object.oid))
+ can_fast_forward = 0;
+ }
+
+ if (can_fast_forward && commit->parents->next &&
+ !commit->parents->next->next &&
+ !oidcmp(&commit->parents->next->item->object.oid,
+ &merge_commit->object.oid)) {
+ rollback_lock_file(&lock);
+ ret = fast_forward_to(&commit->object.oid,
+ &head_commit->object.oid, 0, opts);
+ goto leave_merge;
+ }
+
write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ,
git_path_merge_head(), 0);
write_message("no-ff", 5, git_path_merge_mode(), 0);
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v8 06/16] 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 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 | 6 +
sequencer.c| 407 -
2 files changed, 406 insertions(+), 7 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index e1b865f43f2..ccd5254d1c9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -162,6 +162,12 @@ 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
+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 01443e0f245..35fcacbdf0f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -23,6 +23,8 @@
#include "hashmap.h"
#include "notes-utils.h"
#include "sigchain.h"
+#include "unpack-trees.h"
+#include "worktree.h"
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -120,6 +122,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).
@@ -244,18 +253,34 @@ 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 (is_rebase_i(opts) &&
+ strbuf_read_file(&buf, 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(&dir, get_dir(opts));
- remove_dir_recursively(&dir, 0);
- strbuf_release(&dir);
+ strbuf_reset(&buf);
+ strbuf_addstr(&buf, get_dir(opts));
+ remove_dir_recursively(&buf, 0);
+ strbuf_release(&buf);
return 0;
[PATCH v8 05/16] git-rebase--interactive: clarify arguments
From: Stefan Beller
Up to now each command took a commit as its first argument and ignored
the rest of the line (usually the subject of the commit)
Now that we are about to introduce commands that take different
arguments, clarify each command by giving the argument list.
Signed-off-by: Stefan Beller
Signed-off-by: Johannes Schindelin
---
git-rebase--interactive.sh | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 50323fc2735..e1b865f43f2 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.17.0.windows.1.15.gaa56ade3205
[PATCH v8 04/16] sequencer: offer helpful advice when a command was rescheduled
Previously, we did that just magically, and potentially left some users
quite puzzled. Let's err on the safe side instead, telling the user what
is happening, and how they are supposed to continue.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 16
1 file changed, 16 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index 3d0a45ab25a..01443e0f245 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2555,6 +2555,17 @@ static const char *reflog_message(struct replay_opts
*opts,
return buf.buf;
}
+static const char rescheduled_advice[] =
+N_("Could not execute the todo command\n"
+"\n"
+"%.*s"
+"\n"
+"It has been rescheduled; To edit the command before continuing, please\n"
+"edit the todo list first:\n"
+"\n"
+"git rebase --edit-todo\n"
+"git rebase --continue\n");
+
static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
{
int res = 0;
@@ -2600,6 +2611,11 @@ static int pick_commits(struct todo_list *todo_list,
struct replay_opts *opts)
opts, is_final_fixup(todo_list));
if (is_rebase_i(opts) && res < 0) {
/* Reschedule */
+ advise(_(rescheduled_advice),
+ get_item_line_length(todo_list,
+ todo_list->current),
+ get_item_line(todo_list,
+todo_list->current));
todo_list->current--;
if (save_todo(todo_list, opts))
return -1;
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v8 03/16] sequencer: refactor how original todo list lines are accessed
Previously, we did a lot of arithmetic gymnastics to get at the line in
the todo list (as stored in todo_list.buf). This might have been fast,
but only in terms of execution speed, not in terms of developer time.
Let's refactor this to make it a lot easier to read, and hence to
reason about the correctness of the code. It is not performance-critical
code anyway.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 60 -
1 file changed, 36 insertions(+), 24 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 1ee70d843c1..3d0a45ab25a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1870,6 +1870,23 @@ static int count_commands(struct todo_list *todo_list)
return count;
}
+static int get_item_line_offset(struct todo_list *todo_list, int index)
+{
+ return index < todo_list->nr ?
+ todo_list->items[index].offset_in_buf : todo_list->buf.len;
+}
+
+static const char *get_item_line(struct todo_list *todo_list, int index)
+{
+ return todo_list->buf.buf + get_item_line_offset(todo_list, index);
+}
+
+static int get_item_line_length(struct todo_list *todo_list, int index)
+{
+ return get_item_line_offset(todo_list, index + 1)
+ - get_item_line_offset(todo_list, index);
+}
+
static ssize_t strbuf_read_file_or_whine(struct strbuf *sb, const char *path)
{
int fd;
@@ -2244,29 +2261,27 @@ static int save_todo(struct todo_list *todo_list,
struct replay_opts *opts)
fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
if (fd < 0)
return error_errno(_("could not lock '%s'"), todo_path);
- offset = next < todo_list->nr ?
- todo_list->items[next].offset_in_buf : todo_list->buf.len;
+ offset = get_item_line_offset(todo_list, next);
if (write_in_full(fd, todo_list->buf.buf + offset,
todo_list->buf.len - offset) < 0)
return error_errno(_("could not write to '%s'"), todo_path);
if (commit_lock_file(&todo_lock) < 0)
return error(_("failed to finalize '%s'"), todo_path);
- if (is_rebase_i(opts)) {
- const char *done_path = rebase_path_done();
- int fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
- int prev_offset = !next ? 0 :
- todo_list->items[next - 1].offset_in_buf;
+ if (is_rebase_i(opts) && next > 0) {
+ const char *done = rebase_path_done();
+ int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666);
+ int ret = 0;
- if (fd >= 0 && offset > prev_offset &&
- write_in_full(fd, todo_list->buf.buf + prev_offset,
- offset - prev_offset) < 0) {
- close(fd);
- return error_errno(_("could not write to '%s'"),
- done_path);
- }
- if (fd >= 0)
- close(fd);
+ if (fd < 0)
+ return 0;
+ if (write_in_full(fd, get_item_line(todo_list, next - 1),
+ get_item_line_length(todo_list, next - 1))
+ < 0)
+ ret = error_errno(_("could not write to '%s'"), done);
+ if (close(fd) < 0)
+ ret = error_errno(_("failed to finalize '%s'"), done);
+ return ret;
}
return 0;
}
@@ -3297,8 +3312,7 @@ int skip_unnecessary_picks(void)
oid = &item->commit->object.oid;
}
if (i > 0) {
- int offset = i < todo_list.nr ?
- todo_list.items[i].offset_in_buf : todo_list.buf.len;
+ int offset = get_item_line_offset(&todo_list, i);
const char *done_path = rebase_path_done();
fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
@@ -3478,12 +3492,10 @@ int rearrange_squash(void)
continue;
while (cur >= 0) {
- int offset = todo_list.items[cur].offset_in_buf;
- int end_offset = cur + 1 < todo_list.nr ?
- todo_list.items[cur + 1].offset_in_buf :
- todo_list.buf.len;
- char *bol = todo_list.buf.buf + offset;
- char *eol = todo_list.buf.buf + end_offset;
+ const char *bol =
+ get_item_line(&todo_list, cur);
+ const char *eol =
+ get_item_line(&todo_list, cur + 1);
/* replace 'pick', by 'fixup' or 'squash' */
command = todo_list.items[cur].command
[PATCH v8 02/16] 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 096e6d241e0..1ee70d843c1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3393,7 +3393,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.17.0.windows.1.15.gaa56ade3205
[PATCH v8 01/16] sequencer: avoid using errno clobbered by rollback_lock_file()
As pointed out in a review of the `--rebase-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 | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 667f35ebdff..096e6d241e0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -345,12 +345,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(&msg_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(&msg_file);
- return error_errno(_("could not write eol to '%s'"), filename);
+ return -1;
}
if (commit_lock_file(&msg_file) < 0)
return error(_("failed to finalize '%s'"), filename);
@@ -2119,9 +2121,9 @@ static int save_head(const char *head)
written = write_in_full(fd, buf.buf, buf.len);
strbuf_release(&buf);
if (written < 0) {
+ error_errno(_("could not write to '%s'"), git_path_head_file());
rollback_lock_file(&head_lock);
- return error_errno(_("could not write to '%s'"),
- git_path_head_file());
+ return -1;
}
if (commit_lock_file(&head_lock) < 0)
return error(_("failed to finalize '%s'"),
git_path_head_file());
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v8 00/16] rebase -i: offer to recreate commit topology by rebasing merges
Junio, I think this is now ready for `next`. Thank you for your patience and help with this. Once upon a time, I dreamed of an interactive rebase that would not linearize all patches and drop all merge commits, 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 --rebase-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 --rebase-merges. And then one to allow for rebasing merge commits in a smarter way (this one will need a bit more work, though, as it can result in very complicated, nested merge conflicts *very* easily). Changes since v7: - Touched up all the documentation (it was a mistake to copy-edit the --preserve-merges description, for example). - Disentangled the rescheduling of label/reset/merge from the one of the pick/fixup/squash code path (thanks Phillip!). - When the merge failed, we now write out .git/rebase-merge/patch. - An `exec git cherry-pick` or `exec git revert` will no longer mess with refs/rewritten/ in sequencer_remove_state() (d'oh). Johannes Schindelin (14): sequencer: avoid using errno clobbered by rollback_lock_file() sequencer: make rearrange_squash() a bit more obvious sequencer: refactor how original todo list lines are accessed sequencer: offer helpful advice when a command was rescheduled sequencer: introduce the `merge` command sequencer: fast-forward `merge` commands, if possible rebase-helper --make-script: introduce a flag to rebase merges rebase: introduce the --rebase-merges option sequencer: make refs generated by the `label` command worktree-local sequencer: handle post-rewrite for merge commands rebase --rebase-merges: avoid "empty merges" pull: accept --rebase=merges to recreate the branch topology rebase -i: introduce --rebase-merges=[no-]rebase-cousins rebase -i --rebase-merges: add a section to the man page Phillip Wood (1): rebase --rebase-merges: add test for --keep-empty Stefan Beller (1): git-rebase--interactive: clarify arguments Documentation/config.txt | 8 + Documentation/git-pull.txt | 6 +- Documentation/git-rebase.txt | 160 - builtin/pull.c | 14 +- builtin/rebase--helper.c | 13 +- builtin/remote.c | 18 +- contrib/completion/git-completion.bash | 4 +- git-rebase--interactive.sh | 22 +- git-rebase.sh | 16 + refs.c | 3 +- sequencer.c| 891 +++-- sequencer.h| 7 + t/t3421-rebase-topology-linear.sh | 1 + t/t3430-rebase-merges.sh | 244 +++ 14 files changed, 1347 insertions(+), 60 deletions(-) create mode 100755 t/t3430-rebase-merges.sh base-commit: fe0a9eaf31dd0c349ae4308498c33a5c3794b293 Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v8 Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v8 Interdiff vs v7: diff --git a/Documentation/config.txt b/Documentation/config.txt index da46f154bb3..d6bcb5dcb67 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1059,8 +1059,8 @@ branch..rebase:: branch-specific manner. + When `merges`, pass the `--rebase-merges` option to 'git rebase' -so that locally committed merge commits will not be flattened -by running 'git pull'. +so that the local merge commits are included in the rebase (see +linkgit:git-rebase[1] for details). + When preserve, also pass `--preserve-merges` along to 'git rebase' so that locally committed merge commits will not be flattened @@ -2622,8 +2622,8 @@ pull.rebase:: per-branch basis. + When `merges`, pass the
Re: [PATCH 1/3] gettext: avoid initialization if the locale dir is not present
Hi Ævar, On Sat, 21 Apr 2018, Ævar Arnfjörð Bjarmason wrote: > I still stand by the observation that the "why don't we" isn't clear at > all from your patch's commit message and that should be fixed. It > doesn't mention any of the actual reasons for the change which have been > revealed in this follow-up thread. Fair enough. Will fix, Dscho
[PATCH v3 0/4] Colorize push errors
To help users discern large chunks of white text (when the push
succeeds) from large chunks of white text (when the push fails), let's
add some color to the latter.
The original contribution came in via Pull Request from the Git for
Windows project:
https://github.com/git-for-windows/git/pull/1429
I almost forgot to submit v3. Guess it's a good thing I have a project
management system with tickets I can move through a story board...
Changes since v2:
- removed a bogus "squash! ..." from the commit message of 2/4
- removed the unnecessary here-doc from the test added in 4/4
- renamed color.advice.advice to color.advice.hint in 2/4 and 4/4
Johannes Schindelin (3):
color: introduce support for colorizing stderr
Add a test to verify that push errors are colorful
Document the new color.* settings to colorize push errors/hints
Ryan Dammrose (1):
push: colorize errors
Documentation/config.txt | 28
advice.c | 49 ++--
builtin/push.c | 44 -
color.c| 20 +++-
color.h| 4 ++-
config.c | 2 +-
t/t5541-http-push-smart.sh | 12 +++
transport.c| 67 +-
8 files changed, 211 insertions(+), 15 deletions(-)
base-commit: fe0a9eaf31dd0c349ae4308498c33a5c3794b293
Published-As: https://github.com/dscho/git/releases/tag/colorize-push-errors-v3
Fetch-It-Via: git fetch https://github.com/dscho/git colorize-push-errors-v3
Interdiff vs v2:
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1fef60a7301..2cea0c6c899 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1095,7 +1095,7 @@ color.advice::
are used only when the error output goes to a terminal. If
unset, then the value of `color.ui` is used (`auto` by default).
-color.advice.advice::
+color.advice.hint::
Use customized color for hints.
color.branch::
diff --git a/advice.c b/advice.c
index 2121c1811fd..89fda1de55b 100644
--- a/advice.c
+++ b/advice.c
@@ -36,7 +36,7 @@ static int parse_advise_color_slot(const char *slot)
{
if (!strcasecmp(slot, "reset"))
return ADVICE_COLOR_RESET;
- if (!strcasecmp(slot, "advice"))
+ if (!strcasecmp(slot, "hint"))
return ADVICE_COLOR_HINT;
return -1;
}
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 1c2be98dc2b..a2af693068f 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -379,12 +379,6 @@ test_expect_success 'push status output scrubs password' '
test_expect_success 'colorize errors/hints' '
cd "$ROOT_PATH"/test_repo_clone &&
- cat >exp <<-EOF &&
- To http://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git
- ! [rejected]origin/master^ -> master
(non-fast-forward)
- error: failed to push some refs to
'\''http://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git'\''
- EOF
-
test_must_fail git -c color.transport=always -c color.advice=always \
-c color.push=always \
push origin origin/master^:master 2>act &&
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v3 1/4] color: introduce support for colorizing stderr
So far, we only ever asked whether stdout wants to be colorful. In the
upcoming patches, we will want to make push errors more prominent, which
are printed to stderr, though.
So let's refactor the want_color() function into a want_color_fd()
function (which expects to be called with fd == 1 or fd == 2 for stdout
and stderr, respectively), and then define the macro `want_color()` to
use the want_color_fd() function.
And then also add a macro `want_color_stderr()`, for convenience and
for documentation.
Signed-off-by: Johannes Schindelin
---
color.c | 20 +++-
color.h | 4 +++-
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/color.c b/color.c
index f277e72e4ce..c6c6c4f580f 100644
--- a/color.c
+++ b/color.c
@@ -319,18 +319,20 @@ int git_config_colorbool(const char *var, const char
*value)
return GIT_COLOR_AUTO;
}
-static int check_auto_color(void)
+static int check_auto_color(int fd)
{
- if (color_stdout_is_tty < 0)
- color_stdout_is_tty = isatty(1);
- if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {
+ static int color_stderr_is_tty = -1;
+ int *is_tty_p = fd == 1 ? &color_stdout_is_tty : &color_stderr_is_tty;
+ if (*is_tty_p < 0)
+ *is_tty_p = isatty(fd);
+ if (*is_tty_p || (fd == 1 && pager_in_use() && pager_use_color)) {
if (!is_terminal_dumb())
return 1;
}
return 0;
}
-int want_color(int var)
+int want_color_fd(int fd, int var)
{
/*
* NEEDSWORK: This function is sometimes used from multiple threads, and
@@ -339,15 +341,15 @@ int want_color(int var)
* is listed in .tsan-suppressions for the time being.
*/
- static int want_auto = -1;
+ static int want_auto[3] = { -1, -1, -1 };
if (var < 0)
var = git_use_color_default;
if (var == GIT_COLOR_AUTO) {
- if (want_auto < 0)
- want_auto = check_auto_color();
- return want_auto;
+ if (want_auto[fd] < 0)
+ want_auto[fd] = check_auto_color(fd);
+ return want_auto[fd];
}
return var;
}
diff --git a/color.h b/color.h
index cd0bcedd084..5b744e1bc68 100644
--- a/color.h
+++ b/color.h
@@ -88,7 +88,9 @@ int git_config_colorbool(const char *var, const char *value);
* Return a boolean whether to use color, where the argument 'var' is
* one of GIT_COLOR_UNKNOWN, GIT_COLOR_NEVER, GIT_COLOR_ALWAYS, GIT_COLOR_AUTO.
*/
-int want_color(int var);
+int want_color_fd(int fd, int var);
+#define want_color(colorbool) want_color_fd(1, (colorbool))
+#define want_color_stderr(colorbool) want_color_fd(2, (colorbool))
/*
* Translate a Git color from 'value' into a string that the terminal can
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v3 3/4] Add a test to verify that push errors are colorful
This actually only tests whether the push errors/hints are colored if the respective color.* config settings are `always`, but in the regular case they default to `auto` (in which case we color the messages when stderr is connected to an interactive terminal), therefore these tests should suffice. Signed-off-by: Johannes Schindelin --- t/t5541-http-push-smart.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh index 21340e89c96..a2af693068f 100755 --- a/t/t5541-http-push-smart.sh +++ b/t/t5541-http-push-smart.sh @@ -377,5 +377,17 @@ test_expect_success 'push status output scrubs password' ' grep "^To $HTTPD_URL/smart/test_repo.git" status ' +test_expect_success 'colorize errors/hints' ' + cd "$ROOT_PATH"/test_repo_clone && + test_must_fail git -c color.transport=always -c color.advice=always \ + -c color.push=always \ + push origin origin/master^:master 2>act && + test_decode_color decoded && + test_i18ngrep ".*rejected.*" decoded && + test_i18ngrep "error: failed to push some refs" decoded && + test_i18ngrep "hint: " decoded && + test_i18ngrep ! "^hint: " decoded +' + stop_httpd test_done -- 2.17.0.windows.1.15.gaa56ade3205
[PATCH v3 4/4] Document the new color.* settings to colorize push errors/hints
Let's make it easier for users to find out how to customize these colors. Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 28 1 file changed, 28 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 2659153cb37..2cea0c6c899 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1088,6 +1088,16 @@ clean.requireForce:: A boolean to make git-clean do nothing unless given -f, -i or -n. Defaults to true. +color.advice:: + A boolean to enable/disable color in hints (e.g. when a push + failed, see `advice.*` for a list). May be set to `always`, + `false` (or `never`) or `auto` (or `true`), in which case colors + are used only when the error output goes to a terminal. If + unset, then the value of `color.ui` is used (`auto` by default). + +color.advice.hint:: + Use customized color for hints. + color.branch:: A boolean to enable/disable color in the output of linkgit:git-branch[1]. May be set to `always`, @@ -1190,6 +1200,15 @@ color.pager:: A boolean to enable/disable colored output when the pager is in use (default is true). +color.push:: + A boolean to enable/disable color in push errors. May be set to + `always`, `false` (or `never`) or `auto` (or `true`), in which + case colors are used only when the error output goes to a terminal. + If unset, then the value of `color.ui` is used (`auto` by default). + +color.push.error:: + Use customized color for push errors. + color.showBranch:: A boolean to enable/disable color in the output of linkgit:git-show-branch[1]. May be set to `always`, @@ -1218,6 +1237,15 @@ color.status.:: status short-format), or `unmerged` (files which have unmerged changes). +color.transport:: + A boolean to enable/disable color when pushes are rejected. May be + set to `always`, `false` (or `never`) or `auto` (or `true`), in which + case colors are used only when the error output goes to a terminal. + If unset, then the value of `color.ui` is used (`auto` by default). + +color.transport.rejected:: + Use customized color when a push was rejected. + color.ui:: This variable determines the default value for variables such as `color.diff` and `color.grep` that control the use of color -- 2.17.0.windows.1.15.gaa56ade3205
[PATCH v3 2/4] push: colorize errors
From: Ryan Dammrose
This is an attempt to resolve an issue I experience with people that are
new to Git -- especially colleagues in a team setting -- where they miss
that their push to a remote location failed because the failure and
success both return a block of white text.
An example is if I push something to a remote repository and then a
colleague attempts to push to the same remote repository and the push
fails because it requires them to pull first, but they don't notice
because a success and failure both return a block of white text. They
then continue about their business, thinking it has been successfully
pushed.
This patch colorizes the errors and hints (in red and yellow,
respectively) so whenever there is a failure when pushing to a remote
repository that fails, it is more noticeable.
[jes: fixed a couple bugs, added the color.{advice,push,transport}
settings, refactored to use want_color_stderr().]
Signed-off-by: Ryan Dammrose ryandammr...@gmail.com
Signed-off-by: Johannes Schindelin
---
advice.c | 49 ++--
builtin/push.c | 44 -
config.c | 2 +-
transport.c| 67 +-
4 files changed, 157 insertions(+), 5 deletions(-)
diff --git a/advice.c b/advice.c
index 406efc183ba..89fda1de55b 100644
--- a/advice.c
+++ b/advice.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "config.h"
+#include "color.h"
int advice_push_update_rejected = 1;
int advice_push_non_ff_current = 1;
@@ -20,6 +21,33 @@ int advice_add_embedded_repo = 1;
int advice_ignored_hook = 1;
int advice_waiting_for_editor = 1;
+static int advice_use_color = -1;
+static char advice_colors[][COLOR_MAXLEN] = {
+ GIT_COLOR_RESET,
+ GIT_COLOR_YELLOW, /* HINT */
+};
+
+enum color_advice {
+ ADVICE_COLOR_RESET = 0,
+ ADVICE_COLOR_HINT = 1,
+};
+
+static int parse_advise_color_slot(const char *slot)
+{
+ if (!strcasecmp(slot, "reset"))
+ return ADVICE_COLOR_RESET;
+ if (!strcasecmp(slot, "hint"))
+ return ADVICE_COLOR_HINT;
+ return -1;
+}
+
+static const char *advise_get_color(enum color_advice ix)
+{
+ if (want_color_stderr(advice_use_color))
+ return advice_colors[ix];
+ return "";
+}
+
static struct {
const char *name;
int *preference;
@@ -59,7 +87,10 @@ void advise(const char *advice, ...)
for (cp = buf.buf; *cp; cp = np) {
np = strchrnul(cp, '\n');
- fprintf(stderr, _("hint: %.*s\n"), (int)(np - cp), cp);
+ fprintf(stderr, _("%shint: %.*s%s\n"),
+ advise_get_color(ADVICE_COLOR_HINT),
+ (int)(np - cp), cp,
+ advise_get_color(ADVICE_COLOR_RESET));
if (*np)
np++;
}
@@ -68,9 +99,23 @@ void advise(const char *advice, ...)
int git_default_advice_config(const char *var, const char *value)
{
- const char *k;
+ const char *k, *slot_name;
int i;
+ if (!strcmp(var, "color.advice")) {
+ advice_use_color = git_config_colorbool(var, value);
+ return 0;
+ }
+
+ if (skip_prefix(var, "color.advice.", &slot_name)) {
+ int slot = parse_advise_color_slot(slot_name);
+ if (slot < 0)
+ return 0;
+ if (!value)
+ return config_error_nonbool(var);
+ return color_parse(value, advice_colors[slot]);
+ }
+
if (!skip_prefix(var, "advice.", &k))
return 0;
diff --git a/builtin/push.c b/builtin/push.c
index 013c20d6164..ac3705370e1 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -12,12 +12,40 @@
#include "submodule.h"
#include "submodule-config.h"
#include "send-pack.h"
+#include "color.h"
static const char * const push_usage[] = {
N_("git push [] [ [...]]"),
NULL,
};
+static int push_use_color = -1;
+static char push_colors[][COLOR_MAXLEN] = {
+ GIT_COLOR_RESET,
+ GIT_COLOR_RED, /* ERROR */
+};
+
+enum color_push {
+ PUSH_COLOR_RESET = 0,
+ PUSH_COLOR_ERROR = 1
+};
+
+static int parse_push_color_slot(const char *slot)
+{
+ if (!strcasecmp(slot, "reset"))
+ return PUSH_COLOR_RESET;
+ if (!strcasecmp(slot, "error"))
+ return PUSH_COLOR_ERROR;
+ return -1;
+}
+
+static const char *push_get_color(enum color_push ix)
+{
+ if (want_color_stderr(push_use_color))
+ return push_colors[ix];
+ return "";
+}
+
static int thin = 1;
static int deleterefs;
static const char *receivepack;
@@ -337,8 +365,11 @@ static int push_with_options(struct transport *transport,
int flags)
fprintf(stderr, _("Pushing to %s\n"), transport->url);
err = transport_push(transport, refspec_nr, refspec, flags,
[PATCH v4 09/11] technical/shallow: describe the relationship with replace refs
Now that grafts are deprecated, we should start to assume that readers have no idea what grafts are. So it makes more sense to describe the "shallow" feature in terms of replace refs. Suggested-by: Eric Sunshine Signed-off-by: Johannes Schindelin --- Documentation/technical/shallow.txt | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Documentation/technical/shallow.txt b/Documentation/technical/shallow.txt index 5183b154229..b3ff23c25f6 100644 --- a/Documentation/technical/shallow.txt +++ b/Documentation/technical/shallow.txt @@ -9,14 +9,17 @@ these commits have no parents. * The basic idea is to write the SHA-1s of shallow commits into -$GIT_DIR/shallow, and handle its contents like the contents -of $GIT_DIR/info/grafts (with the difference that shallow -cannot contain parent information). - -This information is stored in a new file instead of grafts, or -even the config, since the user should not touch that file -at all (even throughout development of the shallow clone, it -was never manually edited!). +$GIT_DIR/shallow, and handle its contents similar to replace +refs (with the difference that shallow does not actually +create those replace refs) and very much like the deprecated +graft file (with the difference that shallow commits will +always have their parents grafted away, not replaced by +different parents). + +This information is stored in a special-purpose file because the +user should not touch that file at all (even throughout +development of the shallow clone, it was never manually +edited!). Each line contains exactly one SHA-1. When read, a commit_graft will be constructed, which has nr_parent < 0 to make it easier -- 2.17.0.windows.1.15.gaa56ade3205
[PATCH v4 11/11] Remove obsolete script to convert grafts to replace refs
The functionality is now implemented as `git replace
--convert-graft-file`.
Signed-off-by: Johannes Schindelin
---
contrib/convert-grafts-to-replace-refs.sh | 28 ---
1 file changed, 28 deletions(-)
delete mode 100755 contrib/convert-grafts-to-replace-refs.sh
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
deleted file mode 100755
index 0cbc917b8cf..000
--- a/contrib/convert-grafts-to-replace-refs.sh
+++ /dev/null
@@ -1,28 +0,0 @@
-#!/bin/sh
-
-# You should execute this script in the repository where you
-# want to convert grafts to replace refs.
-
-GRAFTS_FILE="${GIT_DIR:-.git}/info/grafts"
-
-. $(git --exec-path)/git-sh-setup
-
-test -f "$GRAFTS_FILE" || die "Could not find graft file: '$GRAFTS_FILE'"
-
-grep '^[^# ]' "$GRAFTS_FILE" |
-while read definition
-do
- if test -n "$definition"
- then
- echo "Converting: $definition"
- git replace --graft $definition ||
- die "Conversion failed for: $definition"
- fi
-done
-
-mv "$GRAFTS_FILE" "$GRAFTS_FILE.bak" ||
- die "Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'"
-
-echo "Success!"
-echo "All the grafts in '$GRAFTS_FILE' have been converted to replace refs!"
-echo "The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'"
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v4 10/11] technical/shallow: describe why shallow cannot use replace refs
It is tempting to do away with commit_graft altogether (in the long haul), now that grafts are deprecated. However, the shallow feature needs a couple of things that the replace refs cannot fulfill. Let's point that out in the documentation. Signed-off-by: Johannes Schindelin --- Documentation/technical/shallow.txt | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/technical/shallow.txt b/Documentation/technical/shallow.txt index b3ff23c25f6..cb79181c2bb 100644 --- a/Documentation/technical/shallow.txt +++ b/Documentation/technical/shallow.txt @@ -25,6 +25,13 @@ Each line contains exactly one SHA-1. When read, a commit_graft will be constructed, which has nr_parent < 0 to make it easier to discern from user provided grafts. +Note that the shallow feature could not be changed easily to +use replace refs: a commit containing a `mergetag` is not allowed +to be replaced, not even by a root commit. Such a commit can be +made shallow, though. Also, having a `shallow` file explicitly +listing all the commits made shallow makes it a *lot* easier to +do shallow-specific things such as to deepen the history. + Since fsck-objects relies on the library to read the objects, it honours shallow commits automatically. -- 2.17.0.windows.1.15.gaa56ade3205
[PATCH v4 08/11] filter-branch: stop suggesting to use grafts
The graft file is deprecated now, so let's use replace refs in the example in filter-branch's man page instead. Suggested-by: Eric Sunshine Signed-off-by: Johannes Schindelin --- Documentation/git-filter-branch.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt index b634043183b..1d4d2f86045 100644 --- a/Documentation/git-filter-branch.txt +++ b/Documentation/git-filter-branch.txt @@ -288,7 +288,7 @@ git filter-branch --parent-filter \ or even simpler: --- -echo "$commit-id $graft-id" >> .git/info/grafts +git replace --graft $commit-id $graft-id git filter-branch $graft-id..HEAD --- -- 2.17.0.windows.1.15.gaa56ade3205
[PATCH v4 07/11] Deprecate support for .git/info/grafts
The grafts feature was a convenient way to "stitch together" ancient
history to the fresh start of linux.git.
Its implementation is, however, not up to Git's standards, as there are
too many ways where it can lead to surprising and unwelcome behavior.
For example, when pushing from a repository with active grafts, it is
possible to miss commits that have been "grafted out", resulting in a
broken state on the other side.
Also, the grafts feature is limited to "rewriting" commits' list of
parents, it cannot replace anything else.
The much younger feature implemented as `git replace` set out to remedy
those limitations and dangerous bugs.
Seeing as `git replace` is pretty mature by now (since 4228e8bc98
(replace: add --graft option, 2014-07-19) it can perform the graft
file's duties), it is time to deprecate support for the graft file, and
to retire it eventually.
Signed-off-by: Johannes Schindelin
Reviewed-by: Stefan Beller
---
advice.c | 2 ++
advice.h | 1 +
commit.c | 10 ++
t/t6001-rev-list-graft.sh | 9 +
4 files changed, 22 insertions(+)
diff --git a/advice.c b/advice.c
index 406efc183ba..4411704fd45 100644
--- a/advice.c
+++ b/advice.c
@@ -19,6 +19,7 @@ int advice_rm_hints = 1;
int advice_add_embedded_repo = 1;
int advice_ignored_hook = 1;
int advice_waiting_for_editor = 1;
+int advice_graft_file_deprecated = 1;
static struct {
const char *name;
@@ -42,6 +43,7 @@ static struct {
{ "addembeddedrepo", &advice_add_embedded_repo },
{ "ignoredhook", &advice_ignored_hook },
{ "waitingforeditor", &advice_waiting_for_editor },
+ { "graftfiledeprecated", &advice_graft_file_deprecated },
/* make this an alias for backward compatibility */
{ "pushnonfastforward", &advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 70568fa7922..9f5064e82a8 100644
--- a/advice.h
+++ b/advice.h
@@ -21,6 +21,7 @@ extern int advice_rm_hints;
extern int advice_add_embedded_repo;
extern int advice_ignored_hook;
extern int advice_waiting_for_editor;
+extern int advice_graft_file_deprecated;
int git_default_advice_config(const char *var, const char *value);
__attribute__((format (printf, 1, 2)))
diff --git a/commit.c b/commit.c
index 2952ec987c5..451d3ce8dfe 100644
--- a/commit.c
+++ b/commit.c
@@ -12,6 +12,7 @@
#include "prio-queue.h"
#include "sha1-lookup.h"
#include "wt-status.h"
+#include "advice.h"
static struct commit_extra_header *read_commit_extra_header_lines(const char
*buf, size_t len, const char **);
@@ -176,6 +177,15 @@ static int read_graft_file(const char *graft_file)
struct strbuf buf = STRBUF_INIT;
if (!fp)
return -1;
+ if (advice_graft_file_deprecated)
+ advise(_("Support for /info/grafts is deprecated\n"
+"and will be removed in a future Git version.\n"
+"\n"
+"Please use \"git replace --convert-graft-file\"\n"
+"to convert the grafts into replace refs.\n"
+"\n"
+"Turn this message off by running\n"
+"\"git config advice.graftFileDeprecated false\""));
while (!strbuf_getwholeline(&buf, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
struct commit_graft *graft = read_graft_line(&buf);
diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 05ddc69cf2a..7504ba47511 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -110,4 +110,13 @@ do
"
done
+
+test_expect_success 'show advice that grafts are deprecated' '
+ git show HEAD 2>err &&
+ test_i18ngrep "git replace" err &&
+ test_config advice.graftFileDeprecated false &&
+ git show HEAD 2>err &&
+ test_i18ngrep ! "git replace" err
+'
+
test_done
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v4 06/11] Add a test for `git replace --convert-graft-file`
The proof, as the saying goes, lies in the pudding. So here is a regression test that not only demonstrates what the option is supposed to accomplish, but also demonstrates that it does accomplish it. Signed-off-by: Johannes Schindelin --- t/t6050-replace.sh | 20 1 file changed, 20 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index c630aba657e..bed86a0af3d 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -444,4 +444,24 @@ test_expect_success GPG '--graft on a commit with a mergetag' ' git replace -d $HASH10 ' +test_expect_success '--convert-graft-file' ' + : add and convert graft file && + printf "%s\n%s %s\n\n# comment\n%s\n" \ + $(git rev-parse HEAD^^ HEAD^ HEAD^^ HEAD^2) \ + >.git/info/grafts && + git replace --convert-graft-file && + test_path_is_missing .git/info/grafts && + + : verify that the history is now "grafted" && + git rev-list HEAD >out && + test_line_count = 4 out && + + : create invalid graft file and verify that it is not deleted && + test_when_finished "rm -f .git/info/grafts" && + echo $EMPTY_BLOB $EMPTY_TREE >.git/info/grafts && + test_must_fail git replace --convert-graft-file 2>err && + test_i18ngrep "$EMPTY_BLOB $EMPTY_TREE" err && + test_i18ngrep "$EMPTY_BLOB $EMPTY_TREE" .git/info/grafts +' + test_done -- 2.17.0.windows.1.15.gaa56ade3205
[PATCH v4 04/11] replace: "libify" create_graft() and callees
File this away as yet another patch in the "libification" category.
As with all useful functions, in the next commit we want to use
create_graft() from a higher-level function where it would be
inconvenient if the called function simply die()s: if there is a
problem, we want to let the user know how to proceed, and the callee
simply has no way of knowing what to say.
Signed-off-by: Johannes Schindelin
---
builtin/replace.c | 135 --
1 file changed, 84 insertions(+), 51 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index e345a5a0f1c..f63df405fd0 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -79,9 +79,9 @@ static int list_replace_refs(const char *pattern, const char
*format)
else if (!strcmp(format, "long"))
data.format = REPLACE_FORMAT_LONG;
else
- die("invalid replace format '%s'\n"
- "valid formats are 'short', 'medium' and 'long'\n",
- format);
+ return error("invalid replace format '%s'\n"
+"valid formats are 'short', 'medium' and 'long'\n",
+format);
for_each_replace_ref(show_reference, (void *)&data);
@@ -134,7 +134,7 @@ static int delete_replace_ref(const char *name, const char
*ref,
return 0;
}
-static void check_ref_valid(struct object_id *object,
+static int check_ref_valid(struct object_id *object,
struct object_id *prev,
struct strbuf *ref,
int force)
@@ -142,12 +142,13 @@ static void check_ref_valid(struct object_id *object,
strbuf_reset(ref);
strbuf_addf(ref, "%s%s", git_replace_ref_base, oid_to_hex(object));
if (check_refname_format(ref->buf, 0))
- die("'%s' is not a valid ref name.", ref->buf);
+ return error("'%s' is not a valid ref name.", ref->buf);
if (read_ref(ref->buf, prev))
oidclr(prev);
else if (!force)
- die("replace ref '%s' already exists", ref->buf);
+ return error("replace ref '%s' already exists", ref->buf);
+ return 0;
}
static int replace_object_oid(const char *object_ref,
@@ -161,28 +162,31 @@ static int replace_object_oid(const char *object_ref,
struct strbuf ref = STRBUF_INIT;
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
+ int res = 0;
obj_type = oid_object_info(object, NULL);
repl_type = oid_object_info(repl, NULL);
if (!force && obj_type != repl_type)
- die("Objects must be of the same type.\n"
- "'%s' points to a replaced object of type '%s'\n"
- "while '%s' points to a replacement object of type '%s'.",
- object_ref, type_name(obj_type),
- replace_ref, type_name(repl_type));
+ return error("Objects must be of the same type.\n"
+"'%s' points to a replaced object of type '%s'\n"
+"while '%s' points to a replacement object of "
+"type '%s'.",
+object_ref, type_name(obj_type),
+replace_ref, type_name(repl_type));
- check_ref_valid(object, &prev, &ref, force);
+ if (check_ref_valid(object, &prev, &ref, force))
+ return -1;
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, repl, &prev,
0, NULL, &err) ||
ref_transaction_commit(transaction, &err))
- die("%s", err.buf);
+ res = error("%s", err.buf);
ref_transaction_free(transaction);
strbuf_release(&ref);
- return 0;
+ return res;
}
static int replace_object(const char *object_ref, const char *replace_ref, int
force)
@@ -190,9 +194,11 @@ static int replace_object(const char *object_ref, const
char *replace_ref, int f
struct object_id object, repl;
if (get_oid(object_ref, &object))
- die("Failed to resolve '%s' as a valid ref.", object_ref);
+ return error("Failed to resolve '%s' as a valid ref.",
+object_ref);
if (get_oid(replace_ref, &repl))
- die("Failed to resolve '%s' as a valid ref.", replace_ref);
+ return error("Failed to resolve '%s' as a valid ref.",
+replace_ref);
return replace_object_oid(object_ref, &object, replace_ref, &repl,
force);
}
@@ -202,7 +208,7 @@ static int replace_object(const char *object_ref, const
char *replace_ref, int f
* If "raw" is true, then the object's raw contents are printed according to
* "type". Otherwise, we pretty-print the contents for human
[PATCH v4 05/11] replace: introduce --convert-graft-file
This option is intended to help with the transition away from the
now-deprecated graft file.
Signed-off-by: Johannes Schindelin
---
Documentation/git-replace.txt | 11 ++---
builtin/replace.c | 44 ++-
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index e5c57ae6ef4..246dc9943c2 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -11,6 +11,7 @@ SYNOPSIS
'git replace' [-f]
'git replace' [-f] --edit
'git replace' [-f] --graft [...]
+'git replace' [-f] --convert-graft-file
'git replace' -d ...
'git replace' [--format=] [-l []]
@@ -87,9 +88,13 @@ OPTIONS
content as except that its parents will be
[...] instead of 's parents. A replacement ref
is then created to replace with the newly created
- commit. See contrib/convert-grafts-to-replace-refs.sh for an
- example script based on this option that can convert grafts to
- replace refs.
+ commit. Use `--convert-graft-file` to convert a
+ `$GIT_DIR/info/grafts` file and use replace refs instead.
+
+--convert-graft-file::
+ Creates graft commits for all entries in `$GIT_DIR/info/grafts`
+ and deletes that file upon success. The purpose is to help users
+ with transitioning off of the now-deprecated graft file.
-l ::
--list ::
diff --git a/builtin/replace.c b/builtin/replace.c
index f63df405fd0..6b3e0301e90 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -20,6 +20,7 @@ static const char * const git_replace_usage[] = {
N_("git replace [-f] "),
N_("git replace [-f] --edit "),
N_("git replace [-f] --graft [...]"),
+ N_("git replace [-f] --convert-graft-file"),
N_("git replace -d ..."),
N_("git replace [--format=] [-l []]"),
NULL
@@ -454,6 +455,38 @@ static int create_graft(int argc, const char **argv, int
force)
return replace_object_oid(old_ref, &old_oid, "replacement", &new_oid,
force);
}
+static int convert_graft_file(int force)
+{
+ const char *graft_file = get_graft_file();
+ FILE *fp = fopen_or_warn(graft_file, "r");
+ struct strbuf buf = STRBUF_INIT, err = STRBUF_INIT;
+ struct argv_array args = ARGV_ARRAY_INIT;
+
+ if (!fp)
+ return -1;
+
+ while (strbuf_getline(&buf, fp) != EOF) {
+ if (*buf.buf == '#')
+ continue;
+
+ argv_array_split(&args, buf.buf);
+ if (args.argc && create_graft(args.argc, args.argv, force))
+ strbuf_addf(&err, "\n\t%s", buf.buf);
+ argv_array_clear(&args);
+ }
+
+ strbuf_release(&buf);
+ argv_array_clear(&args);
+
+ if (!err.len)
+ return unlink_or_warn(graft_file);
+
+ warning(_("could not convert the following graft(s):\n%s"), err.buf);
+ strbuf_release(&err);
+
+ return -1;
+}
+
int cmd_replace(int argc, const char **argv, const char *prefix)
{
int force = 0;
@@ -465,6 +498,7 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
MODE_DELETE,
MODE_EDIT,
MODE_GRAFT,
+ MODE_CONVERT_GRAFT_FILE,
MODE_REPLACE
} cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
@@ -472,6 +506,7 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
OPT_CMDMODE('d', "delete", &cmdmode, N_("delete replace refs"),
MODE_DELETE),
OPT_CMDMODE('e', "edit", &cmdmode, N_("edit existing object"),
MODE_EDIT),
OPT_CMDMODE('g', "graft", &cmdmode, N_("change a commit's
parents"), MODE_GRAFT),
+ OPT_CMDMODE(0, "convert-graft-file", &cmdmode, N_("convert
existing graft file"), MODE_CONVERT_GRAFT_FILE),
OPT_BOOL_F('f', "force", &force, N_("replace the ref if it
exists"),
PARSE_OPT_NOCOMPLETE),
OPT_BOOL(0, "raw", &raw, N_("do not pretty-print contents for
--edit")),
@@ -494,7 +529,8 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
if (force &&
cmdmode != MODE_REPLACE &&
cmdmode != MODE_EDIT &&
- cmdmode != MODE_GRAFT)
+ cmdmode != MODE_GRAFT &&
+ cmdmode != MODE_CONVERT_GRAFT_FILE)
usage_msg_opt("-f only makes sense when writing a replacement",
git_replace_usage, options);
@@ -527,6 +563,12 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
git_replace_usage, options);
return create_graft(argc, argv, force);
+ case MODE_CONVERT_GRAFT_FILE:
+ if (argc != 0)
+ usage_msg_opt("--convert-graft-file takes no argument",
+
[PATCH v4 02/11] commit: Let the callback of for_each_mergetag return on error
This is yet another patch to be filed under the keyword "libification".
There is one subtle change in behavior here, where a `git log` that has
been asked to show the mergetags would now stop reporting the mergetags
upon the first failure, whereas previously, it would have continued to the
next mergetag, if any.
In practice, that change should not matter, as it is 1) uncommon to
perform octopus merges using multiple tags as merge heads, and 2) when the
user asks to be shown those tags, they really should be there.
Signed-off-by: Johannes Schindelin
---
builtin/replace.c | 8
commit.c | 8 +---
commit.h | 4 ++--
log-tree.c| 13 +++--
4 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 935647be6bd..245d3f4164e 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -345,7 +345,7 @@ struct check_mergetag_data {
const char **argv;
};
-static void check_one_mergetag(struct commit *commit,
+static int check_one_mergetag(struct commit *commit,
struct commit_extra_header *extra,
void *data)
{
@@ -368,20 +368,20 @@ static void check_one_mergetag(struct commit *commit,
if (get_oid(mergetag_data->argv[i], &oid) < 0)
die(_("Not a valid object name: '%s'"),
mergetag_data->argv[i]);
if (!oidcmp(&tag->tagged->oid, &oid))
- return; /* found */
+ return 0; /* found */
}
die(_("original commit '%s' contains mergetag '%s' that is discarded; "
"use --edit instead of --graft"), ref, oid_to_hex(&tag_oid));
}
-static void check_mergetags(struct commit *commit, int argc, const char **argv)
+static int check_mergetags(struct commit *commit, int argc, const char **argv)
{
struct check_mergetag_data mergetag_data;
mergetag_data.argc = argc;
mergetag_data.argv = argv;
- for_each_mergetag(check_one_mergetag, commit, &mergetag_data);
+ return for_each_mergetag(check_one_mergetag, commit, &mergetag_data);
}
static int create_graft(int argc, const char **argv, int force)
diff --git a/commit.c b/commit.c
index ca474a7c112..2952ec987c5 100644
--- a/commit.c
+++ b/commit.c
@@ -1288,17 +1288,19 @@ struct commit_extra_header
*read_commit_extra_headers(struct commit *commit,
return extra;
}
-void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
+int for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
{
struct commit_extra_header *extra, *to_free;
+ int res = 0;
to_free = read_commit_extra_headers(commit, NULL);
- for (extra = to_free; extra; extra = extra->next) {
+ for (extra = to_free; !res && extra; extra = extra->next) {
if (strcmp(extra->key, "mergetag"))
continue; /* not a merge tag */
- fn(commit, extra, data);
+ res = fn(commit, extra, data);
}
free_commit_extra_headers(to_free);
+ return res;
}
static inline int standard_header_field(const char *field, size_t len)
diff --git a/commit.h b/commit.h
index 0fb8271665c..9000895ad91 100644
--- a/commit.h
+++ b/commit.h
@@ -291,10 +291,10 @@ extern const char *find_commit_header(const char *msg,
const char *key,
/* Find the end of the log message, the right place for a new trailer. */
extern int ignore_non_trailer(const char *buf, size_t len);
-typedef void (*each_mergetag_fn)(struct commit *commit, struct
commit_extra_header *extra,
+typedef int (*each_mergetag_fn)(struct commit *commit, struct
commit_extra_header *extra,
void *cb_data);
-extern void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void
*data);
+extern int for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void
*data);
struct merge_remote_desc {
struct object *obj; /* the named object, could be a tag */
diff --git a/log-tree.c b/log-tree.c
index d1c0bedf244..f3a51a6e726 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -488,9 +488,9 @@ static int is_common_merge(const struct commit *commit)
&& !commit->parents->next->next);
}
-static void show_one_mergetag(struct commit *commit,
- struct commit_extra_header *extra,
- void *data)
+static int show_one_mergetag(struct commit *commit,
+struct commit_extra_header *extra,
+void *data)
{
struct rev_info *opt = (struct rev_info *)data;
struct object_id oid;
@@ -502,7 +502,7 @@ static void show_one_mergetag(struct commit *commit,
hash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &oid);
tag = lookup_tag(&oid);
if (!tag)
- return; /* error message already given */
+
[PATCH v4 03/11] replace: avoid using die() to indicate a bug
We have the BUG() macro for that purpose.
Signed-off-by: Johannes Schindelin
---
builtin/replace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 245d3f4164e..e345a5a0f1c 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -501,6 +501,6 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
return list_replace_refs(argv[0], format);
default:
- die("BUG: invalid cmdmode %d", (int)cmdmode);
+ BUG("invalid cmdmode %d", (int)cmdmode);
}
}
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v4 01/11] argv_array: offer to split a string by whitespace
This is a simple function that will interpret a string as a whitespace
delimited list of values, and add those values into the array.
Note: this function does not (yet) offer to split by arbitrary delimiters,
or keep empty values in case of runs of whitespace, or de-quote Unix shell
style. All fo this functionality can be added later, when and if needed.
Signed-off-by: Johannes Schindelin
---
argv-array.c | 20
argv-array.h | 2 ++
2 files changed, 22 insertions(+)
diff --git a/argv-array.c b/argv-array.c
index 5d370fa3366..cb5bcd2c064 100644
--- a/argv-array.c
+++ b/argv-array.c
@@ -64,6 +64,26 @@ void argv_array_pop(struct argv_array *array)
array->argc--;
}
+void argv_array_split(struct argv_array *array, const char *to_split)
+{
+ while (isspace(*to_split))
+ to_split++;
+ for (;;) {
+ const char *p = to_split;
+
+ if (!*p)
+ break;
+
+ while (*p && !isspace(*p))
+ p++;
+ argv_array_push_nodup(array, xstrndup(to_split, p - to_split));
+
+ while (isspace(*p))
+ p++;
+ to_split = p;
+ }
+}
+
void argv_array_clear(struct argv_array *array)
{
if (array->argv != empty_argv) {
diff --git a/argv-array.h b/argv-array.h
index 29056e49a12..750c30d2f2c 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -19,6 +19,8 @@ LAST_ARG_MUST_BE_NULL
void argv_array_pushl(struct argv_array *, ...);
void argv_array_pushv(struct argv_array *, const char **);
void argv_array_pop(struct argv_array *);
+/* Splits by whitespace; does not handle quoted arguments! */
+void argv_array_split(struct argv_array *, const char *);
void argv_array_clear(struct argv_array *);
const char **argv_array_detach(struct argv_array *);
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v4 00/11] Deprecate .git/info/grafts
It is fragile, as there is no way for the revision machinery to say "but now I want to traverse the graph ignoring the graft file" e.g. when pushing commits to a remote repository (which, as a consequence, can miss commits). And we already have a better solution with `git replace --graft [...]`. Changes since v3: - The argv_array_split() declaration now has a clear comment indicating that it does not perform any "un-quoting" but goes purely by whitespace. - Fixed t6050 under GETTEXT_POISON. Johannes Schindelin (11): argv_array: offer to split a string by whitespace commit: Let the callback of for_each_mergetag return on error replace: avoid using die() to indicate a bug replace: "libify" create_graft() and callees replace: introduce --convert-graft-file Add a test for `git replace --convert-graft-file` Deprecate support for .git/info/grafts filter-branch: stop suggesting to use grafts technical/shallow: describe the relationship with replace refs technical/shallow: describe why shallow cannot use replace refs Remove obsolete script to convert grafts to replace refs Documentation/git-filter-branch.txt | 2 +- Documentation/git-replace.txt | 11 +- Documentation/technical/shallow.txt | 24 ++- advice.c | 2 + advice.h | 1 + argv-array.c | 20 +++ argv-array.h | 2 + builtin/replace.c | 189 +++--- commit.c | 18 ++- commit.h | 4 +- contrib/convert-grafts-to-replace-refs.sh | 28 log-tree.c| 13 +- t/t6001-rev-list-graft.sh | 9 ++ t/t6050-replace.sh| 20 +++ 14 files changed, 236 insertions(+), 107 deletions(-) delete mode 100755 contrib/convert-grafts-to-replace-refs.sh base-commit: fe0a9eaf31dd0c349ae4308498c33a5c3794b293 Published-As: https://github.com/dscho/git/releases/tag/deprecate-grafts-v4 Fetch-It-Via: git fetch https://github.com/dscho/git deprecate-grafts-v4 Interdiff vs v3: diff --git a/argv-array.h b/argv-array.h index c7c397695df..750c30d2f2c 100644 --- a/argv-array.h +++ b/argv-array.h @@ -19,6 +19,7 @@ LAST_ARG_MUST_BE_NULL void argv_array_pushl(struct argv_array *, ...); void argv_array_pushv(struct argv_array *, const char **); void argv_array_pop(struct argv_array *); +/* Splits by whitespace; does not handle quoted arguments! */ void argv_array_split(struct argv_array *, const char *); void argv_array_clear(struct argv_array *); const char **argv_array_detach(struct argv_array *); diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 8a3ee7c3db9..bed86a0af3d 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -460,8 +460,8 @@ test_expect_success '--convert-graft-file' ' test_when_finished "rm -f .git/info/grafts" && echo $EMPTY_BLOB $EMPTY_TREE >.git/info/grafts && test_must_fail git replace --convert-graft-file 2>err && - grep "$EMPTY_BLOB $EMPTY_TREE" err && - grep "$EMPTY_BLOB $EMPTY_TREE" .git/info/grafts + test_i18ngrep "$EMPTY_BLOB $EMPTY_TREE" err && + test_i18ngrep "$EMPTY_BLOB $EMPTY_TREE" .git/info/grafts ' test_done -- 2.17.0.windows.1.15.gaa56ade3205
Re: [PATCH v3 06/11] Add a test for `git replace --convert-graft-file`
Hi Gábor, On Sat, 21 Apr 2018, SZEDER Gábor wrote: > > The proof, as the saying goes, lies in the pudding. So here is a > > regression test that not only demonstrates what the option is supposed to > > accomplish, but also demonstrates that it does accomplish it. > > > > Signed-off-by: Johannes Schindelin > > --- > > t/t6050-replace.sh | 20 > > 1 file changed, 20 insertions(+) > > > > diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh > > index c630aba657e..8a3ee7c3db9 100755 > > --- a/t/t6050-replace.sh > > +++ b/t/t6050-replace.sh > > @@ -444,4 +444,24 @@ test_expect_success GPG '--graft on a commit with a > > mergetag' ' > > git replace -d $HASH10 > > ' > > > > +test_expect_success '--convert-graft-file' ' > > + : add and convert graft file && > > + printf "%s\n%s %s\n\n# comment\n%s\n" \ > > + $(git rev-parse HEAD^^ HEAD^ HEAD^^ HEAD^2) \ > > + >.git/info/grafts && > > + git replace --convert-graft-file && > > + test_path_is_missing .git/info/grafts && > > + > > + : verify that the history is now "grafted" && > > + git rev-list HEAD >out && > > + test_line_count = 4 out && > > + > > + : create invalid graft file and verify that it is not deleted && > > + test_when_finished "rm -f .git/info/grafts" && > > + echo $EMPTY_BLOB $EMPTY_TREE >.git/info/grafts && > > + test_must_fail git replace --convert-graft-file 2>err && > > + grep "$EMPTY_BLOB $EMPTY_TREE" err && > > This should be 'test_i18ngrep'. Apparently this error message is > translated, and, consequently, the check fails in a GETTEXT_POISON > build. Sure enough, you're right! I thought it would not be translated, what with being an parameter to a formatted string... Will fix, Dscho
Re: [PATCH v3 01/11] argv_array: offer to split a string by whitespace
Hi Stefan,
On Fri, 20 Apr 2018, Stefan Beller wrote:
> On Fri, Apr 20, 2018 at 3:20 PM, Johannes Schindelin
> wrote:
> > This is a simple function that will interpret a string as a whitespace
> > delimited list of values, and add those values into the array.
> >
> > Note: this function does not (yet) offer to split by arbitrary delimiters,
> > or keep empty values in case of runs of whitespace, or de-quote Unix shell
> > style. All fo this functionality can be added later, when and if needed.
> >
> > Signed-off-by: Johannes Schindelin
> > ---
> > argv-array.c | 20
> > argv-array.h | 1 +
> > 2 files changed, 21 insertions(+)
> >
> > diff --git a/argv-array.c b/argv-array.c
> > index 5d370fa3366..cb5bcd2c064 100644
> > --- a/argv-array.c
> > +++ b/argv-array.c
> > @@ -64,6 +64,26 @@ void argv_array_pop(struct argv_array *array)
> > array->argc--;
> > }
> >
> > +void argv_array_split(struct argv_array *array, const char *to_split)
> > +{
> > + while (isspace(*to_split))
> > + to_split++;
> > + for (;;) {
> > + const char *p = to_split;
> > +
> > + if (!*p)
> > + break;
> > +
> > + while (*p && !isspace(*p))
> > + p++;
> > + argv_array_push_nodup(array, xstrndup(to_split, p -
> > to_split));
> > +
> > + while (isspace(*p))
> > + p++;
> > + to_split = p;
> > + }
> > +}
>
> The code looks correct to me.
>
> Though this seems so low level, that I find it hard to accept
> to implement yet another low level split function.
> Would reuse of strbuf_split or string_list_split make sense?
>
> Looking at the user in patch 5 you really want to have the
> argv array, though, so it cannot be pushed to an even higher
> abstraction layer and be solved there. You really want a
> string -> argv array split, which would mean we'd have to
> do the split via string -> {strbufs, stringlist} and then perform
> a conversion from that to argv array and both conversions
> look ugly as we'd need to iterate their specific data structure
> and push each element itself again.
Maybe we could reconcile all of this by introducing yet another layer of
indirection? I am wary of this, as I tend to think that layers of
indirection make things tedious to debug.
But I could imagine that we could introduce something like
typedef int (*split_string_fn)(const char *item, size_t len, void *data);
int split_string(const char *string, int delimiter, int maxsplit,
split_string_fn fn, void *fn_data);
And then argv_array_split() would be implemented like this:
struct argv_array_split_data {
struct argv_array *array;
};
static int argv_array_split_fn(const char *item, size_t len, void *data)
{
struct argv_array *array =
((struct argv_array_split_data *)data)->array;
argv_array_push_nodup(array, xstrndup(item, len));
return 0;
}
void argv_array_split(struct argv_array *array, const char *string,
int delimiter, int maxsplit)
{
struct argv_array_split_data data;
data.array = array;
split_string(string, delimiter, maxsplit, argv_array_split_fn, &data);
}
Possible? Yes. Desirable? Dunno. Looks like a lot of effort for little
gain so far.
> So I guess we rather implement it yet another time.
>
> Looking at their function declarations:
>
> /*
> * lots of very good comments for string list splitting
> */
> int string_list_split(struct string_list *list, const char *string,
> int delim, int maxsplit);
>
> /*
> * lots of very good comments for strbuf splitting
> */
> static inline struct strbuf **strbuf_split(const struct strbuf *sb,
>int terminator)
>
> I find they have comments in common as well as the
> terminator. In the commit message you defer the
> implementation of a terminating symbol, as we
> can do it later. Also the isspace takes more than one
> delimiter, which the others do not.
>
> I am debating myself if I want to ask for a comment, as
> argv-array.h is very short for now and it would be consistent
> not to add a comment.
That was my thinking.
And I thought further: once argv_array_split() becomes more complex, we
would need to consider again whether we want that level of indirection
(and consolidation), and then also whether we need a comment.
On the other hand, argv_array's name suggests that it handles
command-lines, so I think you are correct that at least a little comment
is needed to state that this does not handle quoted arguments.
Ciao,
Dscho
Please Let My Situation Touch Your Heart
-- Hello My Dear Calvary Greetings in the name of the ALMIGHTY I am Mrs Lillian Allan from Switzerland I am married to Late Mr.Allan Joseph who is a wealthy business man here in Burkina Faso we were married for many years without a child before he died after a brief illness Before his sudden death we where devoted christian When my late husband was alive he deposited the sum of Six Million Two Hundred United State Dollars ($6.200.000.00) in one of the prime bank here in Burkina Faso, Presently this money is still with the Bank, I am very sick from Kidney cancer that i may not last till the next two months according to my doctor so now i decided to donate this money to a honest individual who will use it to work for Almighty, orphans, widow and maintenance of church to fulfill the vow i and my late husband made to Almighty, and i have chosen you after praying. I want this money to be use as i have said since i do not have any child to inherit it and our relatives are all unbelievers and i don't want our hard earn money to be used in ungodly way so you will take 40% of the fund for your effort and use the remaining as i stated, as soon as i read from you i will give you more details on how to achieve it, as i don't know what tomorrow will result, i wish you the best in life. Please Always remember me in your prayers. Yours Sister, Mrs Lillian Allan. Please Let My Prayer Touch Your Heart.
Re: [PATCH 5/6] builtin/grep.c: show column numbers via --column-number
On 21 April 2018 at 05:45, Taylor Blau wrote: > This commit teaches 'git-grep(1)' a new option, '--column-number'. This > option builds upon previous commits to show the column number of the > first match on a non-context line. > > For example: > > $ git grep -mn example | head -n3 > .clang-format:51:14:# myFunction(foo, bar, baz); > .clang-format:64:7:# int foo(); > .clang-format:75:8:# void foo() "example" vs "foo" :-) > Now that configuration variables such as grep.columnNumber and > color.grep.columnNumber have a visible effect, we document them in this > patch as well. > > Signed-off-by: Taylor Blau > --- One thing I've noted is that your messages are light on the imperative "do this", preferring "this commit does" or "we do". That said, I found myself following along quite well in what they're saying. > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -1159,6 +1159,8 @@ color.grep.:: > function name lines (when using `-p`) > `linenumber`;; > line number prefix (when using `-n`) > +`columnnumber`;; > + column number prefix (when using `-m`) > `match`;; > matching text (same as setting `matchContext` and `matchSelected`) > `matchContext`;; > @@ -1708,6 +1710,9 @@ gitweb.snapshot:: > grep.lineNumber:: > If set to true, enable `-n` option by default. > > +grep.columnNumber:: > + If set to true, enable `-m` option by default. > + > grep.patternType:: > Set the default matching behavior. Using a value of 'basic', > 'extended', > 'fixed', or 'perl' will enable the `--basic-regexp`, > `--extended-regexp`, You're doing what the immediate neighbours are doing, but the end result is a bit inconsistent: columnnumber vs columnNumber. I think the ideal end-game is columnNumber, lineNumber, and so on. Maybe use "columnNumber" consistently since you're introducing it and want it to be perfect from the start ;-) and if that leaves "linenumber" looking too inconsistent, then change it while at it? (I'm not suggesting changing all foobar to fooBar while at it...) Martin
Re: [PATCH 3/6] grep.[ch]: teach columnnum, color_columnno to grep_opt
On 21 April 2018 at 05:45, Taylor Blau wrote:
> diff --git a/grep.c b/grep.c
> index 29bc799ecf..7872a5d868 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -95,6 +95,10 @@ int grep_config(const char *var, const char *value, void
> *cb)
> opt->linenum = git_config_bool(var, value);
> return 0;
> }
> + if (!strcmp(var, "grep.columnnumber")) {
> + opt->columnnum = git_config_bool(var, value);
> + return 0;
> + }
>
> if (!strcmp(var, "grep.fullname")) {
> opt->relative = !git_config_bool(var, value);
> @@ -111,6 +115,8 @@ int grep_config(const char *var, const char *value, void
> *cb)
> color = opt->color_function;
> else if (!strcmp(var, "color.grep.linenumber"))
> color = opt->color_lineno;
> + else if (!strcmp(var, "color.grep.columnumber"))
> + color = opt->color_columnno;
missing 'n' (s/umnum/umnnum/). Those characters are almost perfectly
designed for hiding precisely this mistake. ;-)
Re: [PATCH 1/3] gettext: avoid initialization if the locale dir is not present
On Fri, Apr 20 2018, Johannes Schindelin wrote:
> Hi Ævar,
>
> On Fri, 20 Apr 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> On Fri, Apr 20 2018, Martin Ågren wrote:
>>
>> > On 20 April 2018 at 11:59, Ævar Arnfjörð Bjarmason
>> > wrote:
>> >>
>> >> On Fri, Apr 20 2018, Johannes Schindelin wrote:
>> >>
>> >>> The runtime of a simple `git.exe version` call on Windows is currently
>> >>> dominated by the gettext setup, adding a whopping ~150ms to the ~210ms
>> >>> total.
>> >>>
>> >>> Given that this cost is added to each and every git.exe invocation goes
>> >>> through common-main's invocation of git_setup_gettext(), and given that
>> >>> scripts have to call git.exe dozens, if not hundreds, of times, this is
>> >>> a substantial performance penalty.
>> >>>
>> >>> This is particularly pointless when considering that Git for Windows
>> >>> ships without localization (to keep the installer's size to a bearable
>> >>> ~34MB): all that time setting up gettext is for naught.
>> >>>
>> >>> So let's be smart about it and skip setting up gettext if the locale
>> >>> directory is not even present.
>> >
>> >> If you don't ship git for windows with gettext or a podir, then compile
>> >> with NO_GETTEXT, then we won't even compile this function you're
>> >> patching here. See line 30 and beyond of gettext.h.
>> >>
>> >> Are you instead compiling git for windows with gettext support with an
>> >> optional add-on package for i18n, so then this podir conditional would
>> >> pass?
>> >>
>> >> In any case, if this is actually the desired behavior I think it's worth
>> >> clearly explaining the interaction with NO_GETTEXT in the commit
>> >> message, and if you *actually* don't want NO_GETTEXT I think it's useful
>> >> to guard this with something like MAYBE_GETTEXT on top, so this code
>> >> doesn't unintentionally hide installation errors on other
>> >> platforms. I.e. something like:
>> >>
>> >> int have_podir = is_directory(podir);
>> >> if (!have_podir)
>> >> #ifdef MAYBE_GETTEXT
>> >> return;
>> >> #else
>> >> BUG("Broken installation, can't find '%s'!", podir);
>> >> #endif
>> >
>> > Is it fair to say that such a broken installation would function more or
>> > less well before and after Dscho's patch, and that your approach would
>> > render such an installation quite useless?
>>
>> Yes my thown out there for the purposes of discussion suggestion may
>> break things for Dscho, or not. I'm just saying that we should document
>> *what* the use-case is.
>
> I think you underestimate the scope of your willful breakage. It is not
> just "break things for Dscho". It is breaking things for every single last
> user of Git for Windows. If we ever do that, I will make sure to announce
> that together with your name and your postal address on it.
>
>> Because it's not just important to massage the code so it works now, but
>> to document what the use-case is, so someone down the line who things
>> "oh why are we doing that" has a clear answer.
>
> Let's face it: if gettext would ever fail to work in case of a missing
> podir, then it would fail for every installation using a locale for which
> we just happen not to have any translation.
>
> So we know that your patch would not only break things, but break things
> totally without reason!
>
>> > Do we have some other similar
>> > cases where we go BUG("I could not find a resource. I could manage
>> > fairly well without it, but you seemed to want it when you installed
>> > me.")? I wonder what other well-respected software do if they can't find
>> > their translations.
>>
>> E.g. my recent 1aca69c019 ("perl Git::LoadCPAN: emit better errors under
>> NO_PERL_CPAN_FALLBACKS", 2018-03-03) does similar things, we *could*
>> carry on in certain cases instead of dying there (or not, depending on
>> the codepath).
>>
>> But in general, I don't think there's any reasonable use-cases for git
>> needing to repair from a broken or semi-broken installation that aren't
>> so specific that they can be explicitly declared, e.g. in this
>> hypothetical MAYBE_GETTEXT case.
>
> Ævar, you need to understand one thing, and you need to understand it
> right now: the vast majority of Git users will not compile their Git. Not.
> Ever. Really, I am not kidding. They use whatever built version they can
> get most conveniently.
>
> It is even more true on Windows, where it may be easier to build Git for
> Windows now (what with all the work I put into the Git for Windows SDK),
> but it is still an expensive endeavor.
>
> And that is even assuming that every Git user is at liberty to build their
> own software, which is completely untrue in a large chunk of our business.
>
> So in order to give users who want localization what they want, without
> burdening users who do not want localization, we use the exact same build
> of Git for Windows for both, and the entire difference is that one comes
> with the podir, and the other without.
>
> Okay, I am lying, at th
[PATCH v3 4/4] rebase --skip: clean up commit message after a failed fixup/squash
During a series of fixup/squash commands, the interactive rebase builds
up a commit message with comments. This will be presented to the user in
the editor if at least one of those commands was a `squash`.
However, if the last of these fixup/squash commands fails with merge
conflicts, and if the user then decides to skip it (or resolve it to a
clean worktree and then continue the rebase), the current code fails to
clean up the commit message.
This commit fixes that behavior.
The diff is best viewed with --color-moved.
Signed-off-by: Johannes Schindelin
---
sequencer.c| 36
t/t3418-rebase-continue.sh | 2 +-
2 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 9a85b705a84..b8b72fd540f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2790,17 +2790,12 @@ static int continue_single_pick(void)
static int commit_staged_changes(struct replay_opts *opts)
{
- unsigned int flags = ALLOW_EMPTY | EDIT_MSG;
+ unsigned int flags = ALLOW_EMPTY | EDIT_MSG, is_fixup = 0, is_clean;
if (has_unstaged_changes(1))
return error(_("cannot rebase: You have unstaged changes."));
- if (!has_uncommitted_changes(0)) {
- const char *cherry_pick_head = git_path_cherry_pick_head();
- if (file_exists(cherry_pick_head) && unlink(cherry_pick_head))
- return error(_("could not remove CHERRY_PICK_HEAD"));
- return 0;
- }
+ is_clean = !has_uncommitted_changes(0);
if (file_exists(rebase_path_amend())) {
struct strbuf rev = STRBUF_INIT;
@@ -2813,16 +2808,41 @@ static int commit_staged_changes(struct replay_opts
*opts)
if (get_oid_hex(rev.buf, &to_amend))
return error(_("invalid contents: '%s'"),
rebase_path_amend());
- if (oidcmp(&head, &to_amend))
+ if (!is_clean && oidcmp(&head, &to_amend))
return error(_("\nYou have uncommitted changes in your "
"working tree. Please, commit them\n"
"first and then run 'git rebase "
"--continue' again."));
+ if (is_clean && !oidcmp(&head, &to_amend)) {
+ strbuf_reset(&rev);
+ /*
+* Clean tree, but we may need to finalize a
+* fixup/squash chain. A failed fixup/squash leaves the
+* file amend-type in rebase-merge/; It is okay if that
+* file is missing, in which case there is no such
+* chain to finalize.
+*/
+ read_oneliner(&rev, rebase_path_amend_type(), 0);
+ if (!strcmp("squash", rev.buf))
+ is_fixup = TODO_SQUASH;
+ else if (!strcmp("fixup", rev.buf)) {
+ is_fixup = TODO_FIXUP;
+ flags = (flags & ~EDIT_MSG) | CLEANUP_MSG;
+ }
+ }
strbuf_release(&rev);
flags |= AMEND_MSG;
}
+ if (is_clean && !is_fixup) {
+ const char *cherry_pick_head = git_path_cherry_pick_head();
+
+ if (file_exists(cherry_pick_head) && unlink(cherry_pick_head))
+ return error(_("could not remove CHERRY_PICK_HEAD"));
+ return 0;
+ }
+
if (run_git_commit(rebase_path_message(), opts, flags))
return error(_("could not commit staged changes."));
unlink(rebase_path_amend());
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 6ddf952b7b9..693f92409ec 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -88,7 +88,7 @@ test_expect_success 'rebase passes merge strategy options
correctly' '
git rebase --continue
'
-test_expect_failure '--skip after failed fixup cleans commit message' '
+test_expect_success '--skip after failed fixup cleans commit message' '
test_when_finished "test_might_fail git rebase --abort" &&
git checkout -b with-conflicting-fixup &&
test_commit wants-fixup &&
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v3 3/4] sequencer: leave a tell-tale when a fixup/squash failed
In the upcoming patch to clean up fixup/squash commit messages even when
skipping a final fixup/squash that failed with merge conflicts, we will
need to have some indicator what happened.
As we need to remove the message-fixup and message-squash files upon
failure, we cannot use those. So let's just write an explicit amend-type
file, containing either `fixup` or `squash`. The absence of that file
indicates that we were not in the middle of a fixup or squash when merge
conflicts were happening.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index dc482e76a28..9a85b705a84 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -106,6 +106,13 @@ static GIT_PATH_FUNC(rebase_path_author_script,
"rebase-merge/author-script")
* command is processed, this file is deleted.
*/
static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
+/*
+ * If there was a merge conflict in a fixup/squash series, we need to
+ * record the type so that a `git rebase --skip` can clean up the commit
+ * message as appropriate. This file will contain that type (`fixup` or
+ * `squash`), and not exist otherwise.
+ */
+static GIT_PATH_FUNC(rebase_path_amend_type, "rebase-merge/amend-type")
/*
* When we stop at a given patch via the "edit" command, this file contains
* the abbreviated commit name of the corresponding patch.
@@ -2400,10 +2407,21 @@ static int error_with_patch(struct commit *commit,
static int error_failed_squash(struct commit *commit,
struct replay_opts *opts, int subject_len, const char *subject)
{
+ const char *amend_type = "squash";
+
if (rename(rebase_path_squash_msg(), rebase_path_message()))
return error(_("could not rename '%s' to '%s'"),
rebase_path_squash_msg(), rebase_path_message());
- unlink(rebase_path_fixup_msg());
+
+ if (file_exists(rebase_path_fixup_msg())) {
+ unlink(rebase_path_fixup_msg());
+ amend_type = "fixup";
+ }
+ if (write_message(amend_type, strlen(amend_type),
+ rebase_path_amend_type(), 0))
+ return error(_("could not write '%s'"),
+rebase_path_amend_type());
+
unlink(git_path_merge_msg());
if (copy_file(git_path_merge_msg(), rebase_path_message(), 0666))
return error(_("could not copy '%s' to '%s'"),
@@ -2580,6 +2598,7 @@ static int pick_commits(struct todo_list *todo_list,
struct replay_opts *opts)
unlink(rebase_path_author_script());
unlink(rebase_path_stopped_sha());
unlink(rebase_path_amend());
+ unlink(rebase_path_amend_type());
delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
}
if (item->command <= TODO_SQUASH) {
@@ -2807,6 +2826,7 @@ static int commit_staged_changes(struct replay_opts *opts)
if (run_git_commit(rebase_path_message(), opts, flags))
return error(_("could not commit staged changes."));
unlink(rebase_path_amend());
+ unlink(rebase_path_amend_type());
return 0;
}
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v3 1/4] rebase -i: demonstrate bugs with fixup!/squash! commit messages
When multiple fixup/squash commands are processed and the last one causes merge conflicts and is skipped, we leave the "This is a combination of ..." comments in the commit message. Noticed by Eric Sunshine. This regression test also demonstrates that we rely on the localized version of # This is a combination of commits to contain the in ASCII, which breaks under GETTEXT_POISON. Signed-off-by: Johannes Schindelin --- t/t3418-rebase-continue.sh | 22 ++ 1 file changed, 22 insertions(+) diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index 9214d0bb511..6ddf952b7b9 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -88,6 +88,28 @@ test_expect_success 'rebase passes merge strategy options correctly' ' git rebase --continue ' +test_expect_failure '--skip after failed fixup cleans commit message' ' + test_when_finished "test_might_fail git rebase --abort" && + git checkout -b with-conflicting-fixup && + test_commit wants-fixup && + test_commit "fixup! wants-fixup" wants-fixup.t 1 wants-fixup-1 && + test_commit "fixup! wants-fixup" wants-fixup.t 2 wants-fixup-2 && + test_commit "fixup! wants-fixup" wants-fixup.t 3 wants-fixup-3 && + test_must_fail env FAKE_LINES="1 fixup 2 fixup 4" \ + git rebase -i HEAD~4 && + + : now there is a conflict, and comments in the commit message && + git show HEAD >out && + test_i18ngrep "This is a combination of" out && + + : skip and continue && + git rebase --skip && + + : now the comments in the commit message should have been cleaned up && + git show HEAD >out && + test_i18ngrep ! "This is a combination of" out +' + test_expect_success 'setup rerere database' ' rm -fr .git/rebase-* && git reset --hard commit-new-file-F3-on-topic-branch && -- 2.17.0.windows.1.15.gaa56ade3205
[PATCH v3 0/4] rebase -i: avoid stale "# This is a combination of" in commit messages
Eric Sunshine pointed out that I had such a commit message in
https://public-inbox.org/git/CAPig+cRrS0_nYJJY=o6cbov630snqhpv5qgrqdd8mw-syzn...@mail.gmail.com/
and I went on a hunt to figure out how the heck this happened.
Turns out that if there is a fixup/squash chain where the *last* command
fails with merge conflicts, and we either --skip ahead or resolve the
conflict to a clean tree and then --continue, our code does not do a
final cleanup.
Contrary to my initial gut feeling, this bug was not introduced by my
rewrite in C of the core parts of rebase -i, but it looks to me as if
that bug was with us for a very long time (at least the --skip part).
The developer (read: user of rebase -i) in me says that we would want to
fast-track this, but the author of rebase -i in me says that we should
be cautious and cook this in `next` for a while.
Fixes since v2 (thanks, Stefan!):
- Fixed commit message of 2/4: "Thisis" -> "This is".
- Reinstated the order where the `message-squash` file is renamed to
`message` first, and only if that succeeded, we delete the
`message-fixup` file.
Johannes Schindelin (4):
rebase -i: demonstrate bugs with fixup!/squash! commit messages
rebase -i: Handle "combination of commits" with GETTEXT_POISON
sequencer: leave a tell-tale when a fixup/squash failed
rebase --skip: clean up commit message after a failed fixup/squash
sequencer.c| 94 --
t/t3418-rebase-continue.sh | 22 +
2 files changed, 93 insertions(+), 23 deletions(-)
base-commit: fe0a9eaf31dd0c349ae4308498c33a5c3794b293
Published-As:
https://github.com/dscho/git/releases/tag/clean-msg-after-fixup-continue-v3
Fetch-It-Via: git fetch https://github.com/dscho/git
clean-msg-after-fixup-continue-v3
Interdiff vs v2:
diff --git a/sequencer.c b/sequencer.c
index 881503a6463..b8b72fd540f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2409,6 +2409,10 @@ static int error_failed_squash(struct commit *commit,
{
const char *amend_type = "squash";
+ if (rename(rebase_path_squash_msg(), rebase_path_message()))
+ return error(_("could not rename '%s' to '%s'"),
+ rebase_path_squash_msg(), rebase_path_message());
+
if (file_exists(rebase_path_fixup_msg())) {
unlink(rebase_path_fixup_msg());
amend_type = "fixup";
@@ -2418,9 +2422,6 @@ static int error_failed_squash(struct commit *commit,
return error(_("could not write '%s'"),
rebase_path_amend_type());
- if (rename(rebase_path_squash_msg(), rebase_path_message()))
- return error(_("could not rename '%s' to '%s'"),
- rebase_path_squash_msg(), rebase_path_message());
unlink(git_path_merge_msg());
if (copy_file(git_path_merge_msg(), rebase_path_message(), 0666))
return error(_("could not copy '%s' to '%s'"),
--
2.17.0.windows.1.15.gaa56ade3205
[PATCH v3 2/4] rebase -i: Handle "combination of commits" with GETTEXT_POISON
We previously relied on the localized versions of
# This is a combination of commits
(which we write into the commit messages during fixup/squash chains)
to contain as ASCII.
This is not true in general, and certainly not in GETTEXT_POISON, as
demonstrated by the regression test we just introduced in t3418.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 36 ++--
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 667f35ebdff..dc482e76a28 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1343,19 +1343,18 @@ static int update_squash_messages(enum todo_command
command,
eol = strchrnul(buf.buf, '\n');
if (buf.buf[0] != comment_line_char ||
(p += strcspn(p, "0123456789\n")) == eol)
- return error(_("unexpected 1st line of squash message:"
- "\n\n\t%.*s"),
-(int)(eol - buf.buf), buf.buf);
- count = strtol(p, NULL, 10);
-
- if (count < 1)
- return error(_("invalid 1st line of squash message:\n"
- "\n\t%.*s"),
-(int)(eol - buf.buf), buf.buf);
+ count = -1;
+ else
+ count = strtol(p, NULL, 10);
strbuf_addf(&header, "%c ", comment_line_char);
- strbuf_addf(&header,
- _("This is a combination of %d commits."), ++count);
+ if (count < 1)
+ strbuf_addf(&header, _("This is a combination of "
+ "several commits."));
+ else
+ strbuf_addf(&header,
+ _("This is a combination of %d commits."),
+ ++count);
strbuf_splice(&buf, 0, eol - buf.buf, header.buf, header.len);
strbuf_release(&header);
} else {
@@ -1398,13 +1397,22 @@ static int update_squash_messages(enum todo_command
command,
if (command == TODO_SQUASH) {
unlink(rebase_path_fixup_msg());
strbuf_addf(&buf, "\n%c ", comment_line_char);
- strbuf_addf(&buf, _("This is the commit message #%d:"), count);
+ if (count < 2)
+ strbuf_addf(&buf, _("This is the next commit "
+ "message:"));
+ else
+ strbuf_addf(&buf, _("This is the commit message #%d:"),
+ count);
strbuf_addstr(&buf, "\n\n");
strbuf_addstr(&buf, body);
} else if (command == TODO_FIXUP) {
strbuf_addf(&buf, "\n%c ", comment_line_char);
- strbuf_addf(&buf, _("The commit message #%d will be skipped:"),
- count);
+ if (count < 2)
+ strbuf_addf(&buf, _("The next commit message will be "
+ "skipped:"));
+ else
+ strbuf_addf(&buf, _("The commit message #%d will be "
+ "skipped:"), count);
strbuf_addstr(&buf, "\n\n");
strbuf_add_commented_lines(&buf, body, strlen(body));
} else
--
2.17.0.windows.1.15.gaa56ade3205
Re: [PATCH v2 3/4] sequencer: leave a tell-tale when a fixup/squash failed
Hi Stefan,
On Fri, 20 Apr 2018, Stefan Beller wrote:
> > static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
> > +/*
> > + * If there was a merge conflict in a fixup/squash series, we need to
> > + * record the type so that a `git rebase --skip` can clean up the commit
> > + * message as appropriate. This file will contain that type (`fixup` or
> > + * `squash`), and not exist otherwise.
> > + */
>
> Thanks for the documentation here, is there some other high level doc that
> describes all things to know about the internals of the rebase-merge dir
> or is this the definitive guide?
>
> > +static GIT_PATH_FUNC(rebase_path_amend_type, "rebase-merge/amend-type")
> > /*
> > * When we stop at a given patch via the "edit" command, this file contains
> > * the abbreviated commit name of the corresponding patch.
> > @@ -2400,10 +2407,20 @@ static int error_with_patch(struct commit *commit,
> > static int error_failed_squash(struct commit *commit,
> > struct replay_opts *opts, int subject_len, const char *subject)
> > {
> > + const char *amend_type = "squash";
> > +
> > + if (file_exists(rebase_path_fixup_msg())) {
> > + unlink(rebase_path_fixup_msg());
> > + amend_type = "fixup";
> > + }
> > + if (write_message(amend_type, strlen(amend_type),
> > + rebase_path_amend_type(), 0))
> > + return error(_("could not write '%s'"),
> > +rebase_path_amend_type());
>
> Do we want to wait with unlinking rebase_path_fixup_msg()
> until after we are sure there is no error returned?
Actually until after the rename() of `rebase_path_squash_msg()` succeeded,
you are right. I had changed the behavior unintentionally.
> I first thought so as to preserve the state as before, but
> then it only signals the amend type. But we're downgrading the
> amend type from "squash" to "fixup", which means that if
> this error happens and the user just retries the git command
> we'll end up with a "fixup", i.e. not opening their editor?
I am actually more worried about the rename() call failing... ;-) I
changed the order back to where it was before.
Thanks,
Dscho
Re: [PATCH v2 2/4] rebase -i: Handle "combination of commits" with GETTEXT_POISON
Hi Stefan, On Fri, 20 Apr 2018, Stefan Beller wrote: > On Fri, Apr 20, 2018 at 2:07 PM, Johannes Schindelin > wrote: > > We previously relied on the localized versions of > > > > # This is a combination of commits > > > > (which we write into the commit messages during fixup/squash chains) > > to contain as ASCII. > > > > Thisis not true in general, and certainly not in GETTEXT_POISON, as > > This is > > Apart from this typo, this patch looks good. Ouch. Fixed, locally. Ciao, Dscho
Re: [PATCH v3] fast-export: fix regression skipping some merge-commits
On 21 April 2018 at 05:43, Junio C Hamano wrote: > but I do not think the updated "fix" below is better. It might be > just aesthetics and I suspect I won't find it as disturbing if we > could push with > > object_array_push(commits, (struct object *)commit); > > or something that is more clearly symmetric to object_array_pop(). > The "Queue again" comment is needed only because use of "add" > highlights the lack of symmetry. > > With add_object_array(), it looks somewhat more odd than your > previous > > peek it to check; > if (it should not be molested) > return; > pop to mark it consumed; > consume it; > > sequence, in which peek() and pop() were more obviously related > operations on the same "array" object. > > And I do not think it is a good idea to introduce _push() only for > symmetry (it would merely be a less capable version of add whose > name is spelled differently). Hence my preference for peek-check-pop > over pop-oops-push-again-but-push-spelled-as-add. > > Not worth a reroll, though. I just wanted to spread better design > sense to contributors ;-) Thanks for your wise words. :-) One thing that just occurred to me is that if the original site where we `add_object_array()` all objects starts adding a non-NULL `name` for some reason, then we need to remember to do the same with this new caller. I suspect that at that time, at the latest, we will be switching to peek-check-pop. Thanks for sharing your thoughts. Martin
