Re: [PATCH] t5562: skip if NO_CURL is enabled
On Wed, Nov 21, 2018 at 05:04:25PM -0800, Carlo Arenas wrote: > the error that gets eventually to stderr in the caller comes from > get_packet_data, who is trying to read 4 bytes and gets 0. > when looking at the trace (obtained with ktrace) Yes too early close of the input data is the thing which triggers the "remote end hung up unexpectedly" message. > I see there is no > longer any other process running, do you mean git receive-pack? This is strange, all its parents should be waiting for it to exit. > the last child of it is long gone with an error as shown by : > > 9255 1 git-http-backend CALL close(1) ... > 9255 1 git-http-backend CALL write(2,0xbfb2a604,0x36) > 9255 1 git-http-backend GIO fd 2 wrote 54 bytes >"fatal: request ended in the middle of the gzip stream\n" This should be some other test than push_plain, some of the gzip related ones. Are there other tests failing? > 9255 1 git-http-backend RET write 54/0x36 > 9255 1 git-http-backend CALL write(1,0xb781f0e0,0x94) > 9255 1 git-http-backend RET write -1 errno 9 Bad file descriptor This is interesting. http-backend for some reason closes its stdout. Here it then tries to write there something. I have not seen it in my push_plain run. Maybe it worth redirecting instead to stderr, to avoid losing some diagnostics? > > not sure how it got into that state, though > > Carlo
How to propagate critical fixs from master to develope branch.
Hello everyone.I red git manual but I can't figure out how to propagate critical change from master branch to long live develop branch. I red chapter about rebasing that I think could solve it but at the end of this chapter is written that it's not recommended for pubic repositories. I don't know how to do it without merging develop branch to master. I'll appreciate list of orders very much. :-) Thanks for any info or link. GB
[PATCH v3 3/7] am, rebase--merge: do not overlook --skip'ed commits with post-rewrite
The post-rewrite hook is supposed to be invoked for each rewritten commit. The fact that a commit was selected and processed by the rebase operation (even though when we hit an error a user said it had no more useful changes), suggests we should write an entry for it. In particular, let's treat it as an empty commit trivially squashed into its parent. This brings the rebase--am and rebase--merge backends in sync with the behavior of the interactive rebase backend. Signed-off-by: Elijah Newren --- builtin/am.c | 9 + git-rebase--merge.sh | 2 ++ t/t5407-post-rewrite-hook.sh | 3 +++ 3 files changed, 14 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 8f27f3375b..af9d034838 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2000,6 +2000,15 @@ static void am_skip(struct am_state *state) if (clean_index(, )) die(_("failed to clean index")); + if (state->rebasing) { + FILE *fp = xfopen(am_path(state, "rewritten"), "a"); + + assert(!is_null_oid(>orig_commit)); + fprintf(fp, "%s ", oid_to_hex(>orig_commit)); + fprintf(fp, "%s\n", oid_to_hex()); + fclose(fp); + } + am_next(state); am_load(state); am_run(state, 0); diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh index aa2f2f0872..91250cbaed 100644 --- a/git-rebase--merge.sh +++ b/git-rebase--merge.sh @@ -121,6 +121,8 @@ continue) skip) read_state git rerere clear + cmt="$(cat "$state_dir/cmt.$msgnum")" + echo "$cmt $(git rev-parse HEAD^0)" >> "$state_dir/rewritten" msgnum=$(($msgnum + 1)) while test "$msgnum" -le "$end" do diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index 6426ec8991..a4a5903cba 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -78,6 +78,7 @@ test_expect_success 'git rebase --skip' ' git rebase --continue && echo rebase >expected.args && cat >expected.data <<-EOF && + $(git rev-parse C) $(git rev-parse HEAD^) $(git rev-parse D) $(git rev-parse HEAD) EOF verify_hook_input @@ -91,6 +92,7 @@ test_expect_success 'git rebase --skip the last one' ' echo rebase >expected.args && cat >expected.data <<-EOF && $(git rev-parse E) $(git rev-parse HEAD) + $(git rev-parse F) $(git rev-parse HEAD) EOF verify_hook_input ' @@ -120,6 +122,7 @@ test_expect_success 'git rebase -m --skip' ' git rebase --continue && echo rebase >expected.args && cat >expected.data <<-EOF && + $(git rev-parse C) $(git rev-parse HEAD^) $(git rev-parse D) $(git rev-parse HEAD) EOF verify_hook_input -- 2.20.0.rc1.7.g58371d377a
[PATCH v3 6/7] rebase: define linearization ordering and enforce it
Ever since commit 3f213981e44a ("add tests for rebasing merged history", 2013-06-06), t3425 has had tests which included the rebasing of merged history and whose order of applied commits was checked. Unfortunately, the tests expected different behavior depending on which backend was in use. Implementing these checks was the following four lines (including the TODO message) which were repeated verbatim three times in t3425: #TODO: make order consistent across all flavors of rebase test_run_rebase success 'e n o' '' test_run_rebase success 'e n o' -m test_run_rebase success 'n o e' -i As part of the effort to reduce differences between the rebase backends so that users get more uniform behavior, let's define the correct behavior and modify the different backends so they all get the right answer. It turns out that the difference in behavior here is entirely due to topological sorting; since some backends require topological sorting (particularly when --rebase-merges is specified), require it for all modes. Modify the am and merge backends to implement this. Performance Considerations: I was unable to measure any appreciable performance difference with this change. Trying to control the run-to-run variation was difficult; I eventually found a headless beefy box that I could ssh into, which seemed to help. Using git.git, I ran the following testcase: $ git reset --hard v2.20.0-rc1~2 $ time git rebase --quiet v2.20.0-rc0~16 I first ran once to warm any disk caches, then ran five subsequent runs and recorded the times of those five. I observed the following results for the average time: Before this change: "real" timing: 1.340s (standard deviation: 0.040s) "user" timing: 1.050s (standard deviation: 0.041s) "sys" timing: 0.270s (standard deviation: 0.011s) After this change: "real" timing: 1.327s (standard deviation: 0.065s) "user" timing: 1.031s (standard deviation: 0.061s) "sys" timing: 0.280s (standard deviation: 0.014s) Measurements aside, I would expect the timing for walking revisions to be dwarfed by the work involved in creating and applying patches, so this isn't too surprising. Further, while somewhat counter-intuitive, it is possible that turning on topological sorting is actually a performance improvement: by way of comparison, turning on --topo-order made fast-export faster (see https://public-inbox.org/git/20090211135640.ga19...@coredump.intra.peff.net/). Signed-off-by: Elijah Newren --- git-rebase--am.sh | 2 +- git-rebase--merge.sh | 2 +- t/t3425-rebase-topology-merges.sh | 15 ++- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/git-rebase--am.sh b/git-rebase--am.sh index 99b8c17787..6416716ee6 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -36,7 +36,7 @@ rm -f "$GIT_DIR/rebased-patches" git format-patch -k --stdout --full-index --cherry-pick --right-only \ --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \ - --pretty=mboxrd \ + --pretty=mboxrd --topo-order \ $git_format_patch_opt \ "$revisions" ${restrict_revision+^$restrict_revision} \ >"$GIT_DIR/rebased-patches" diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh index 91250cbaed..ced38bb3a6 100644 --- a/git-rebase--merge.sh +++ b/git-rebase--merge.sh @@ -143,7 +143,7 @@ write_basic_state rm -f "$(git rev-parse --git-path REBASE_HEAD)" msgnum=0 -for cmt in $(git rev-list --reverse --no-merges "$revisions") +for cmt in $(git rev-list --topo-order --reverse --no-merges "$revisions") do msgnum=$(($msgnum + 1)) echo "$cmt" > "$state_dir/cmt.$msgnum" diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh index 5f892e33d7..fd8efe84fe 100755 --- a/t/t3425-rebase-topology-merges.sh +++ b/t/t3425-rebase-topology-merges.sh @@ -70,9 +70,8 @@ test_run_rebase () { test_linear_range "\'"$expected"\'" d.. " } -#TODO: make order consistent across all flavors of rebase -test_run_rebase success 'e n o' '' -test_run_rebase success 'e n o' -m +test_run_rebase success 'n o e' '' +test_run_rebase success 'n o e' -m test_run_rebase success 'n o e' -i test_run_rebase () { @@ -87,9 +86,8 @@ test_run_rebase () { test_linear_range "\'"$expected"\'" c.. " } -#TODO: make order consistent across all flavors of rebase -test_run_rebase success 'd e n o' '' -test_run_rebase success 'd e n o' -m +test_run_rebase success 'd n o e' '' +test_run_rebase success 'd n o e' -m test_run_rebase success 'd n o e' -i test_run_rebase () { @@ -104,9 +102,8 @@ test_run_rebase () { test_linear_range "\'"$expected"\'" c.. " } -#TODO: make order consistent across all flavors of rebase -test_run_rebase success 'd e n o' '' -test_run_rebase success 'd e n o' -m +test_run_rebase success 'd n o e' '' +test_run_rebase success 'd n o e' -m
[PATCH v3 7/7] rebase: Implement --merge via the interactive machinery
As part of an ongoing effort to make rebase have more uniform behavior, modify the merge backend to behave like the interactive one, by re-implementing it on top of the latter. Interactive rebases are implemented in terms of cherry-pick rather than the merge-recursive builtin, but cherry-pick also calls into the recursive merge machinery by default and can accept special merge strategies and/or special strategy options. As such, there really is not any need for having both git-rebase--merge and git-rebase--interactive anymore. Delete git-rebase--merge.sh and instead implement it in builtin/rebase.c. This results in a few deliberate but small user-visible changes: * The progress output is modified (see t3406 and t3420 for examples) * A few known test failures are now fixed (see t3421) * bash-prompt during a rebase --merge is now REBASE-i instead of REBASE-m. Reason: The prompt is a reflection of the backend in use; this allows users to report an issue to the git mailing list with the appropriate backend information, and allows advanced users to know where to search for relevant control files. (see t9903) testcase modification notes: t3406: --interactive and --merge had slightly different progress output while running; adjust a test to match the new expectation t3420: these test precise output while running, but rebase--am, rebase--merge, and rebase--interactive all were built on very different commands (am, merge-recursive, cherry-pick), so the tests expected different output for each type. Now we expect --merge and --interactive to have the same output. t3421: --interactive fixes some bugs in --merge! Wahoo! t9903: --merge uses the interactive backend so the prompt expected is now REBASE-i. Signed-off-by: Elijah Newren --- .gitignore| 1 - Documentation/git-rebase.txt | 17 +-- Makefile | 1 - builtin/rebase.c | 14 +-- git-legacy-rebase.sh | 43 git-rebase--merge.sh | 166 -- t/t3406-rebase-message.sh | 7 +- t/t3420-rebase-autostash.sh | 78 ++ t/t3421-rebase-topology-linear.sh | 10 +- t/t9903-bash-prompt.sh| 2 +- 10 files changed, 42 insertions(+), 297 deletions(-) delete mode 100644 git-rebase--merge.sh diff --git a/.gitignore b/.gitignore index 0d77ea5894..910b1d2d2f 100644 --- a/.gitignore +++ b/.gitignore @@ -124,7 +124,6 @@ /git-rebase--am /git-rebase--common /git-rebase--interactive -/git-rebase--merge /git-rebase--preserve-merges /git-receive-pack /git-reflog diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 80793bad8d..8211f9357c 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -504,15 +504,7 @@ See also INCOMPATIBLE OPTIONS below. INCOMPATIBLE OPTIONS -git-rebase has many flags that are incompatible with each other, -predominantly due to the fact that it has three different underlying -implementations: - - * one based on linkgit:git-am[1] (the default) - * one based on git-merge-recursive (merge backend) - * one based on linkgit:git-cherry-pick[1] (interactive backend) - -Flags only understood by the am backend: +The following options: * --committer-date-is-author-date * --ignore-date @@ -520,15 +512,12 @@ Flags only understood by the am backend: * --ignore-whitespace * -C -Flags understood by both merge and interactive backends: +are incompatible with the following options: * --merge * --strategy * --strategy-option * --allow-empty-message - -Flags only understood by the interactive backend: - * --[no-]autosquash * --rebase-merges * --preserve-merges @@ -539,7 +528,7 @@ Flags only understood by the interactive backend: * --edit-todo * --root when used in combination with --onto -Other incompatible flag pairs: +In addition, the following pairs of options are incompatible: * --preserve-merges and --interactive * --preserve-merges and --signoff diff --git a/Makefile b/Makefile index 1a44c811aa..82e1eb1a4a 100644 --- a/Makefile +++ b/Makefile @@ -628,7 +628,6 @@ SCRIPT_LIB += git-parse-remote SCRIPT_LIB += git-rebase--am SCRIPT_LIB += git-rebase--common SCRIPT_LIB += git-rebase--preserve-merges -SCRIPT_LIB += git-rebase--merge SCRIPT_LIB += git-sh-setup SCRIPT_LIB += git-sh-i18n diff --git a/builtin/rebase.c b/builtin/rebase.c index f1f449801b..39ed88040a 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -122,7 +122,7 @@ static void imply_interactive(struct rebase_options *opts, const char *option) case REBASE_PRESERVE_MERGES: break; case REBASE_MERGE: - /* we silently *upgrade* --merge to --interactive if needed */ + /* we now implement --merge via --interactive */ default: opts->type =
[PATCH v3 4/7] git-rebase, sequencer: extend --quiet option for the interactive machinery
While 'quiet' and 'interactive' may sound like antonyms, the interactive machinery actually has logic that implements several interactive_rebase=implied cases (--exec, --keep-empty, --rebase-merges) which won't pop up an editor. The rewrite of interactive rebase in C added a quiet option, though it only turns stats off. Since we want to make the interactive machinery also take over for git-rebase--merge, it should fully implement the --quiet option. git-rebase--interactive was already somewhat quieter than git-rebase--merge and git-rebase--am, possibly because cherry-pick has just traditionally been quieter. As such, we only drop a few informational messages -- "Rebasing (n/m)" and "Successfully rebased..." Also, for simplicity, remove the differences in how quiet and verbose options were recorded. Having one be signalled by the presence of a "verbose" file in the state_dir, while the other was signalled by the contents of a "quiet" file was just weirdly inconsistent. (This inconsistency pre-dated the rewrite into C.) Make them consistent by having them both key off the presence of the file. Signed-off-by: Elijah Newren --- builtin/rebase.c | 5 + git-legacy-rebase.sh | 2 +- git-rebase--common.sh | 2 +- sequencer.c | 23 +-- sequencer.h | 1 + 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 5ece134ae6..f1f449801b 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -185,10 +185,7 @@ static int read_basic_state(struct rebase_options *opts) if (get_oid(buf.buf, >orig_head)) return error(_("invalid orig-head: '%s'"), buf.buf); - strbuf_reset(); - if (read_one(state_dir_path("quiet", opts), )) - return -1; - if (buf.len) + if (file_exists(state_dir_path("quiet", opts))) opts->flags &= ~REBASE_NO_QUIET; else opts->flags |= REBASE_NO_QUIET; diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh index 0a747eb76c..d01eef9876 100755 --- a/git-legacy-rebase.sh +++ b/git-legacy-rebase.sh @@ -113,7 +113,7 @@ read_basic_state () { else orig_head=$(cat "$state_dir"/head) fi && - GIT_QUIET=$(cat "$state_dir"/quiet) && + test -f "$state_dir"/quiet && GIT_QUIET=t test -f "$state_dir"/verbose && verbose=t test -f "$state_dir"/strategy && strategy="$(cat "$state_dir"/strategy)" test -f "$state_dir"/strategy_opts && diff --git a/git-rebase--common.sh b/git-rebase--common.sh index 7e39d22871..dc18c682fa 100644 --- a/git-rebase--common.sh +++ b/git-rebase--common.sh @@ -10,7 +10,7 @@ write_basic_state () { echo "$head_name" > "$state_dir"/head-name && echo "$onto" > "$state_dir"/onto && echo "$orig_head" > "$state_dir"/orig-head && - echo "$GIT_QUIET" > "$state_dir"/quiet && + test t = "$GIT_QUIET" && : > "$state_dir"/quiet test t = "$verbose" && : > "$state_dir"/verbose test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy test -n "$strategy_opts" && echo "$strategy_opts" > \ diff --git a/sequencer.c b/sequencer.c index e1a4dd15f1..bc25615050 100644 --- a/sequencer.c +++ b/sequencer.c @@ -150,6 +150,7 @@ static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt") static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head") static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose") +static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") static GIT_PATH_FUNC(rebase_path_signoff, "rebase-merge/signoff") static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name") static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto") @@ -157,7 +158,6 @@ static GIT_PATH_FUNC(rebase_path_autostash, "rebase-merge/autostash") static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy") static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts") static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate") -static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") static int git_sequencer_config(const char *k, const char *v, void *cb) { @@ -2357,6 +2357,9 @@ static int read_populate_opts(struct replay_opts *opts) if (file_exists(rebase_path_verbose())) opts->verbose = 1; + if (file_exists(rebase_path_quiet())) + opts->quiet = 1; + if (file_exists(rebase_path_signoff())) { opts->allow_ff = 0; opts->signoff = 1; @@ -2424,9 +2427,6 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, if (quiet) write_file(rebase_path_quiet(), "%s\n", quiet); - else -
[PATCH v3 2/7] t5407: add a test demonstrating how interactive handles --skip differently
The post-rewrite hook is documented as being invoked by commands that rewrite commits such as commit --amend and rebase, and that it will be called for each rewritten commit. Apparently, the three backends handled --skip'ed commits differently: am: treat the skipped commit as though it weren't rewritten merge: same as 'am' backend interactive: treat skipped commits as having been rewritten to empty (view them as an empty fixup to their parent) For now, just add a testcase documenting the different behavior (use --keep to force usage of the interactive machinery even though we have no empty commits). A subsequent commit will remove the inconsistency in --skip handling. Signed-off-by: Elijah Newren --- t/t5407-post-rewrite-hook.sh | 31 +++ 1 file changed, 31 insertions(+) diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index 9b2a274c71..6426ec8991 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -125,6 +125,37 @@ test_expect_success 'git rebase -m --skip' ' verify_hook_input ' +test_expect_success 'git rebase with implicit use of interactive backend' ' + git reset --hard D && + clear_hook_input && + test_must_fail git rebase --keep --onto A B && + echo C > foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data <<-EOF && + $(git rev-parse C) $(git rev-parse HEAD^) + $(git rev-parse D) $(git rev-parse HEAD) + EOF + verify_hook_input +' + +test_expect_success 'git rebase --skip with implicit use of interactive backend' ' + git reset --hard D && + clear_hook_input && + test_must_fail git rebase --keep --onto A B && + test_must_fail git rebase --skip && + echo D > foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data <<-EOF && + $(git rev-parse C) $(git rev-parse HEAD^) + $(git rev-parse D) $(git rev-parse HEAD) + EOF + verify_hook_input +' + . "$TEST_DIRECTORY"/lib-rebase.sh set_fake_editor -- 2.20.0.rc1.7.g58371d377a
[PATCH v3 5/7] git-legacy-rebase: simplify unnecessary triply-nested if
The git-legacy-rebase.sh script previously had code of the form: if git_am_opt: if interactive: if incompatible_opts: show_error_about_interactive_and_am_incompatibilities if rebase-merge: if incompatible_opts show_error_about_merge_and_am_incompatibilities which was a triply nested if. However, the first conditional (git_am_opt) and third (incompatible_opts) were somewhat redundant: the latter condition was a strict subset of the former. Simplify this by moving the innermost conditional to the outside, allowing us to remove the test on git_am_opt entirely and giving us the following form: if incomptable_opts: if interactive: show_error_about_interactive_and_am_incompatibilities if rebase-merge: show_error_about_merge_and_am_incompatibilities Signed-off-by: Elijah Newren --- git-legacy-rebase.sh | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh index d01eef9876..425189bde1 100755 --- a/git-legacy-rebase.sh +++ b/git-legacy-rebase.sh @@ -501,21 +501,17 @@ then git_format_patch_opt="$git_format_patch_opt --progress" fi -if test -n "$git_am_opt"; then - incompatible_opts=$(echo " $git_am_opt " | \ - sed -e 's/ -q / /g' -e 's/^ \(.*\) $/\1/') +incompatible_opts=$(echo " $git_am_opt " | \ + sed -e 's/ -q / /g' -e 's/^ \(.*\) $/\1/') +if test -n "$incompatible_opts" +then if test -n "$interactive_rebase" then - if test -n "$incompatible_opts" - then - die "$(gettext "error: cannot combine am options with interactive options")" - fi + die "$(gettext "error: cannot combine am options with interactive options")" fi - if test -n "$do_merge"; then - if test -n "$incompatible_opts" - then - die "$(gettext "error: cannot combine am options with merge options")" - fi + if test -n "$do_merge" + then + die "$(gettext "error: cannot combine am options with merge options")" fi fi -- 2.20.0.rc1.7.g58371d377a
[PATCH v3 1/7] rebase: fix incompatible options error message
In commit f57696802c30 ("rebase: really just passthru the `git am` options", 2018-11-14), the handling of `git am` options was simplified dramatically (and an option parsing bug was fixed), but it introduced a small regression in the error message shown when options only understood by separate backends were used: $ git rebase --keep --ignore-whitespace fatal: error: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options (.git/rebase-apply/applying) $ git rebase --merge --ignore-whitespace fatal: error: cannot combine merge options (--merge, --strategy, --strategy-option) with am options (.git/rebase-apply/applying) Note that in both cases, the list of "am options" is ".git/rebase-apply/applying", which makes no sense. Since the lists of backend-specific options is documented pretty thoroughly in the rebase man page (in the "Incompatible Options" section, with multiple links throughout the document), and since I expect this list to change over time, just simplify the error message. Signed-off-by: Elijah Newren --- builtin/rebase.c | 11 --- git-legacy-rebase.sh | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 5b3e5baec8..5ece134ae6 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1202,14 +1202,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) break; if (is_interactive() && i >= 0) - die(_("error: cannot combine interactive options " - "(--interactive, --exec, --rebase-merges, " - "--preserve-merges, --keep-empty, --root + " - "--onto) with am options (%s)"), buf.buf); + die(_("error: cannot combine am options " + "with interactive options")); if (options.type == REBASE_MERGE && i >= 0) - die(_("error: cannot combine merge options (--merge, " - "--strategy, --strategy-option) with am options " - "(%s)"), buf.buf); + die(_("error: cannot combine am options " + "with merge options ")); } if (options.signoff) { diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh index b97ffdc9dd..0a747eb76c 100755 --- a/git-legacy-rebase.sh +++ b/git-legacy-rebase.sh @@ -508,13 +508,13 @@ if test -n "$git_am_opt"; then then if test -n "$incompatible_opts" then - die "$(gettext "error: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options ($incompatible_opts)")" + die "$(gettext "error: cannot combine am options with interactive options")" fi fi if test -n "$do_merge"; then if test -n "$incompatible_opts" then - die "$(gettext "error: cannot combine merge options (--merge, --strategy, --strategy-option) with am options ($incompatible_opts)")" + die "$(gettext "error: cannot combine am options with merge options")" fi fi fi -- 2.20.0.rc1.7.g58371d377a
[PATCH v3 0/7] Reimplement rebase --merge via interactive machinery
[Important: Patch 1 fixes a (minor) regression in 2.20 relative to 2.19...but it also modifies a translated string. I'm not sure what the right step we want to take there is. If you want me to submit it separately and also resubmit the rest of this series to depend on the separated first patch, let me know.] This series continues the work of making rebase more self-consistent by removing inconsistencies between different backends. In particular, this series focuses on making the merge machinery behave like the interactive machinery (though two differences between the am and interactive backends are also fixed along the way), and ultimately removes the merge backend in favor of reimplementing the relevant options on top of the interactive machinery. Differences since v2 (full range-diff below): - Addressed feedback from both Phillip and Dscho - Added five new patches; the biggest change is still the final patch but it should be a little easier to review now. - Added a patch fixing a very recent regression in the incompatible options error message - Documented and fixed the inconsistency in how different rebase backends handle --skip relative to the post-rewrite hook - Added a patch defining the linearization order and enforcing it, getting rid of an age-old TODO - Added a patch which took a slightly confusing diff hunk from the previous final patch, and give it its own commit message explaining why the drop from a triply-nested if block to a doubly-nested if-block still keeps all necessary checks in place. - Rebased to latest master Elijah Newren (7): rebase: fix incompatible options error message t5407: add a test demonstrating how interactive handles --skip differently am, rebase--merge: do not overlook --skip'ed commits with post-rewrite git-rebase, sequencer: extend --quiet option for the interactive machinery git-legacy-rebase: simplify unnecessary triply-nested if rebase: define linearization ordering and enforce it rebase: Implement --merge via the interactive machinery .gitignore| 1 - Documentation/git-rebase.txt | 17 +--- Makefile | 1 - builtin/am.c | 9 ++ builtin/rebase.c | 24 ++--- git-legacy-rebase.sh | 57 +-- git-rebase--am.sh | 2 +- git-rebase--common.sh | 2 +- git-rebase--merge.sh | 164 -- sequencer.c | 23 +++-- sequencer.h | 1 + t/t3406-rebase-message.sh | 7 +- t/t3420-rebase-autostash.sh | 78 ++ t/t3421-rebase-topology-linear.sh | 10 +- t/t3425-rebase-topology-merges.sh | 15 ++- t/t5407-post-rewrite-hook.sh | 34 +++ t/t9903-bash-prompt.sh| 2 +- 17 files changed, 114 insertions(+), 333 deletions(-) delete mode 100644 git-rebase--merge.sh -: -- > 1: 2f4bdd1980 rebase: fix incompatible options error message -: -- > 2: cc33a8ccc1 t5407: add a test demonstrating how interactive handles --skip differently -: -- > 3: f5838ef763 am, rebase--merge: do not overlook --skip'ed commits with post-rewrite 1: bf0acd9b27 ! 4: 50dc863d9f git-rebase, sequencer: extend --quiet option for the interactive machinery @@ -13,7 +13,7 @@ git-rebase--interactive was already somewhat quieter than git-rebase--merge and git-rebase--am, possibly because cherry-pick has just traditionally been quieter. As such, we only drop a few -informational messages -- "Rebasing (n/m)" and "Succesfully rebased..." +informational messages -- "Rebasing (n/m)" and "Successfully rebased..." Also, for simplicity, remove the differences in how quiet and verbose options were recorded. Having one be signalled by the presence of a -: -- > 5: 35cf552f27 git-legacy-rebase: simplify unnecessary triply-nested if -: -- > 6: 2a3d8ff1c1 rebase: define linearization ordering and enforce it 2: cd0ccab680 ! 7: 58371d377a rebase: Implement --merge via git-rebase--interactive @@ -1,35 +1,37 @@ Author: Elijah Newren -rebase: Implement --merge via git-rebase--interactive +rebase: Implement --merge via the interactive machinery -Interactive rebases are implemented in terms of cherry-pick rather than -the merge-recursive builtin, but cherry-pick also calls into the recursive -merge machinery by default and can accept special merge strategies and/or -special strategy options. As such, there really is not any need for -having both git-rebase--merge and git-rebase--interactive anymore. +As part of an ongoing effort to make rebase have more uniform behavior, +modify the merge backend to behave like the interactive one, by +re-implementing it on top
Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)
On Wednesday, November 21, 2018 6:06:13 PM MST Junio C Hamano wrote: > "Stephen P. Smith" writes: > > On Wednesday, November 21, 2018 2:00:16 AM MST Junio C Hamano wrote: > >> [Stalled] > >> > >> * lt/date-human (2018-07-09) 1 commit > >> > >> - Add 'human' date format > >> > >> A new date format "--date=human" that morphs its output depending > >> on how far the time is from the current time has been introduced. > >> "--date=auto" can be used to use this new format when the output is > >> goint to the pager or to the terminal and otherwise the default > >> format. > > > > What needs to be done with this patch to move it along? > > In a random order as they come to my mind: > > - Support by people other than the original author; I was trying to decide if you were wanting someone besides Linus to carry this through (knowing that he was unlikely to do so). The idea seems reasonable to me. I don't mind doing so if neither of you object. > > - Deciding what to call this (i.e. Linus's personal preference >would not be the only 'human' style, but we may declare it is >good enough as "a" human format, not "the" human format); > > - Some mechanism (either technical or documentation) to prevent >endless stream of "I like human output, but I'd tweak it slightly >this way" updates in the future; > > - Doc; > > - Command line completion; > > - Tests; > > - Waiting for the end of feature freeze before the upcoming >release. > > There may be others, but without all of the above, I'd feel a bit > uncomfortable. > I wouldn't think this would go in now since the patch has been stalled for months. It is also on the pu branch. > > I see that both Linus and Junio have signed the patch. > > That does not assert that the code is desirable thing to add. It > just says we made sure that we legally have rights to include it, if > we want to. Picking up someones stalled patch is one thing, picking up Linus' patch is in a different league.
Re: [PATCH v1 2/2] log -S: Add test which searches in binary files
Thomas Braun writes: > The -S option of log looks for differences that changes the > number of occurrences of the specified string (i.e. addition/deletion) > in a file. s/-S /-S/ and s/the specified string/the specified block of text/ would make it more in line with how Documentation/gitdiffcore.txt explains it. The original discussion from early 2017 also explains with a pointer why the primary mode of -S is not but is . > diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh > index 42cc8afd8b..d430f6f2f9 100755 > --- a/t/t4209-log-pickaxe.sh > +++ b/t/t4209-log-pickaxe.sh > @@ -128,4 +128,15 @@ test_expect_success 'log -G looks into binary files with > textconv filter' ' > test_cmp actual expected > ' > > +test_expect_success 'log -S looks into binary files' ' > + rm -rf .git && > + git init && Same comment as the one for 1/2 applies here. > + printf "a\0b" >data.bin && > + git add data.bin && > + git commit -m "message" && > + git log -S a >actual && > + git log >expected && > + test_cmp actual expected > +' > + > test_done Other than these, I think both patches look sensible. Thanks for resurrecting the old topic and reigniting it.
Re: [PATCH v1 1/2] log -G: Ignore binary files
Thomas Braun writes: > The -G option of log looks for the differences whose patch text > contains added/removed lines that match regex. > > The concept of differences only makes sense for text files, therefore > we need to ignore binary files when searching with -G as well. > > Signed-off-by: Thomas Braun > --- > Documentation/gitdiffcore.txt | 2 +- > diffcore-pickaxe.c| 5 + > t/t4209-log-pickaxe.sh| 22 ++ > 3 files changed, 28 insertions(+), 1 deletion(-) OK. > diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt > index c0a60f3158..059ddd3431 100644 > --- a/Documentation/gitdiffcore.txt > +++ b/Documentation/gitdiffcore.txt > @@ -242,7 +242,7 @@ textual diff has an added or a deleted line that matches > the given > regular expression. This means that it will detect in-file (or what > rename-detection considers the same file) moves, which is noise. The > implementation runs diff twice and greps, and this can be quite > -expensive. > +expensive. Binary files without textconv filter are ignored. OK. > diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c > index 69fc55ea1e..8c2558b07d 100644 > --- a/diffcore-pickaxe.c > +++ b/diffcore-pickaxe.c > @@ -144,6 +144,11 @@ static int pickaxe_match(struct diff_filepair *p, struct > diff_options *o, > textconv_two = get_textconv(o->repo->index, p->two); > } > > + if ((o->pickaxe_opts & DIFF_PICKAXE_KIND_G) && > + ((!textconv_one && diff_filespec_is_binary(o->repo, p->one)) || > + (!textconv_two && diff_filespec_is_binary(o->repo, p->two > + return 0; > + > /* >* If we have an unmodified pair, we know that the count will be the >* same and don't even have to load the blobs. Unless textconv is in Shouldn't this new test come after the existing optimization, which allows us to leave without loading the blob contents (which is needed once you call diff_filespec_is_binary())? > diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh > index 844df760f7..42cc8afd8b 100755 > --- a/t/t4209-log-pickaxe.sh > +++ b/t/t4209-log-pickaxe.sh > @@ -106,4 +106,26 @@ test_expect_success 'log -S --no-textconv (missing > textconv tool)' ' > rm .gitattributes > ' > > +test_expect_success 'log -G ignores binary files' ' > + rm -rf .git && > + git init && Please never never ever do the above two unless you are writing a test that checks low-level repository details. If you want a clean history that has specific lineage of commits without getting affected by commits that have been made by the previous test pieces, it is OK to "checkout --orphan" to create an empty history to work with. > + printf "a\0b" >data.bin && > + git add data.bin && > + git commit -m "message" && > + git log -G a >result && > + test_must_be_empty result > +' > + > +test_expect_success 'log -G looks into binary files with textconv filter' ' > + rm -rf .git && > + git init && > + echo "* diff=bin" > .gitattributes && > + printf "a\0b" >data.bin && > + git add data.bin && > + git commit -m "message" && > + git -c diff.bin.textconv=cat log -G a >actual && > + git log >expected && > + test_cmp actual expected > +' > + > test_done
Re: [PATCH v4 0/2] Fix scissors bug during merge conflict
On Wed, Nov 21, 2018 at 06:38:20PM +0900, Junio C Hamano wrote: > Denton Liu writes: > > > Changes since V3: > > * Add patch to cleanup 'merge --squash c3 with c7' test in t7600 > > * Use test_i18ncmp instead of test_cmp to pass GETTEXT_POISON tests > > Queued. Thanks, both. I just realised that there is a slight problem with the proposed change. When we do a merge and there are no merge conflicts, at the end of the merge, we get dropped into an editor with this text: Merge branch 'master' into new # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. Note that in git-merge, the cleanup only removes commented lines and this cannot be configured to be scissors or whatever else. I think that the fact that it's not configurable isn't a problem; most hardcore commit message editing happens in git-commit anyway. However, since we taught git-merge the --cleanup option, this might be misleading for the end-user since they would expect the MERGE_MSG to be cleaned up as specified. I see two resolutions for this. We can either rename --cleanup more precisely so users won't be confused (perhaps something like --merge-conflict-scissors but a lot more snappy) or we can actually make git-merge respect the cleanup option and post-process the message according to the specified cleanup rule. I would personally think the first option is better than the second but I'd like to hear your thoughts. Thanks!
Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)
"Stephen P. Smith" writes: > On Wednesday, November 21, 2018 2:00:16 AM MST Junio C Hamano wrote: >> [Stalled] >> >> * lt/date-human (2018-07-09) 1 commit >> - Add 'human' date format >> >> A new date format "--date=human" that morphs its output depending >> on how far the time is from the current time has been introduced. >> "--date=auto" can be used to use this new format when the output is >> goint to the pager or to the terminal and otherwise the default >> format. > > What needs to be done with this patch to move it along? In a random order as they come to my mind: - Support by people other than the original author; - Deciding what to call this (i.e. Linus's personal preference would not be the only 'human' style, but we may declare it is good enough as "a" human format, not "the" human format); - Some mechanism (either technical or documentation) to prevent endless stream of "I like human output, but I'd tweak it slightly this way" updates in the future; - Doc; - Command line completion; - Tests; - Waiting for the end of feature freeze before the upcoming release. There may be others, but without all of the above, I'd feel a bit uncomfortable. > I see that both Linus and Junio have signed the patch. That does not assert that the code is desirable thing to add. It just says we made sure that we legally have rights to include it, if we want to.
Re: [PATCH] t5562: skip if NO_CURL is enabled
On Wed, Nov 21, 2018 at 2:49 PM Max Kirillov wrote: > > On Wed, Nov 21, 2018 at 04:02:04AM -0800, Carlo Arenas wrote: > > for some tracing, it would seem that it gets 0 when > > trying to read 4 bytes from what I think is a pipe that connects to a > > child that has been gone already for a while. > > Could you clarify it? I'm afraid I don't understand. the error that gets eventually to stderr in the caller comes from get_packet_data, who is trying to read 4 bytes and gets 0. when looking at the trace (obtained with ktrace) I see there is no longer any other process running, the last child of it is long gone with an error as shown by : 9255 1 git-http-backend CALL close(1) 9255 1 git-http-backend RET close 0 9255 1 git-http-backend CALL read(0,0xbfb2bb14,0) 9255 1 git-http-backend GIO fd 0 read 0 bytes "" 9255 1 git-http-backend RET read 0 9255 1 git-http-backend CALL write(2,0xbfb2a604,0x36) 9255 1 git-http-backend GIO fd 2 wrote 54 bytes "fatal: request ended in the middle of the gzip stream\n" 9255 1 git-http-backend RET write 54/0x36 9255 1 git-http-backend CALL write(1,0xb781f0e0,0x94) 9255 1 git-http-backend RET write -1 errno 9 Bad file descriptor not sure how it got into that state, though Carlo
Re: [PATCH 1/5] eoie: default to not writing EOIE section
Jeff King writes: > Yes, there are two ways to write this. With a conditional to initialize > and return or to return the default, as we have here: > >> > >+ if (!git_config_get_bool("index.recordendofindexentries", )) >> > >+ return val; >> > >+ return 0; > > Or initialize the default ahead of time, and rely on the function not to > modify it when the entry is missing: > > int val = 0; > git_config_get_bool("index.whatever", ); > return val; > > I think either is perfectly fine, but since I also had to look at it > twice to make sure it was doing the right thing, I figured it is worth > mentioning as a possible style/convention thing we may want to decide > on. I too think either is fine, and both rely on the git_config_get_*() to modify the value return only when it sees that it is set. I'd choose the latter when the default value is simple, as the reader does not have to even know what the return value from the git_config_get_*() function means to follow what is going on. On the other hand, the former (i.e. the original by Jonathan) is more flexible, and it makes it possible to write a piece of code, which computes a default that is expensive to build only when necessary, in the most natural way. The readers do need to be aware of how the functin signals "I didn't get anything" with its return value, though. I do not mind standardizing on the latter, though. A caller with an expensive default can initialize val to an impossible "sentinel" value that signals the fact that git_config_get_*() did not get anything, as long as the type has a natural sentinel (like -1 for a bool to signal "unset"), and code that comes either immediately after git_config_get_*() or much much later in the control flow can check for the sentinel to see if it needs to compute the expensive default.
Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)
On Wednesday, November 21, 2018 2:00:16 AM MST Junio C Hamano wrote: > [Stalled] > > * lt/date-human (2018-07-09) 1 commit > - Add 'human' date format > > A new date format "--date=human" that morphs its output depending > on how far the time is from the current time has been introduced. > "--date=auto" can be used to use this new format when the output is > goint to the pager or to the terminal and otherwise the default > format. What needs to be done with this patch to move it along? I see that both Linus and Junio have signed the patch.
Re: [PATCH] t5562: skip if NO_CURL is enabled
On Wed, Nov 21, 2018 at 2:49 PM Max Kirillov wrote: > > Should I install bash for it to work? I cannot say I understand what the > message is about. yes, you need to install bash and use SHELL_PATH=/usr/pkg/bin/bash; PERL_PATH=/usr/pkg/bin/perl for the perl script Carlo
Re: [PATCH] t5562: skip if NO_CURL is enabled
On Wed, Nov 21, 2018 at 04:02:04AM -0800, Carlo Arenas wrote: > for some tracing, it would seem that it gets 0 when > trying to read 4 bytes from what I think is a pipe that connects to a > child that has been gone already for a while. Could you clarify it? I'm afraid I don't understand. Meanwhile, I've been staring at code and so far don't have any assumption where it could fail. Except basic things like something is wrong with forking or reading/writing pipes, but then it would have bigger consequences. Also, I tried to look at it with NetBSD but cannot get past error, while running tests: > ./test-lib.sh: 327: Syntax error: Bad substitution There is the following code there: - if test -z "$test_untraceable" || { test -n "$BASH_VERSION" && { test ${BASH_VERSINFO[0]} -gt 4 || { # line 327 test ${BASH_VERSINFO[0]} -eq 4 && test ${BASH_VERSINFO[1]} -ge 1 - Should I install bash for it to work? I cannot say I understand what the message is about.
Re: [PATCH 0/1] rebase: warn about the correct tree's OID
Hi Junio, On Wed, 21 Nov 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" > writes: > > > A quick fix for a recent topic. Not overly critical, but I would deem this > > v2.20.0-rc1 material. > > > > Johannes Schindelin (1): > > rebase: warn about the correct tree's OID > > > > Yup, it is kind of embarrasing that nobody caught it, but at the > same time, this typo is at so tiny level that I would not be > surprised if it survived for many years. TBH I am quite mortified that it slipped through *my* multiple pre-contribution reviews. > Will apply. Thanks. Ciao, Dscho
International credit settlement
International credit settlement Office of the director of operations World Bank united state of America. Attention : This Is To Officially Inform You That We Have Verified Your Contact Inheritance File Presently On My Desk, And I Found Out That You Have Not Received Your Payment Due To Your Lack Of Co- Operation And Not Fulfilling The Obligations Giving To You In Respect To Your Contract /Inheritance Payment. Secondly, You Are Hereby Advice To Stop Dealing With Some Non- Officials In The Bank As This Is An Illegal Act And Will Have To Stop If You So Wish To Receive Your Payment Immediately. After The Board Meeting Held At Our Headquarters, We Have Resolved In Finding A Solution To Your Problem, And As You May Know, We Have Arranged Your Payment Through Our Swift Card Payment Center In Europe, America And Asia Pacific, Which Is Then Instruction Given By Our President. This Card Center Will Send You An ATM Card Which You Will Use To Withdraw Your Money In Any Part Of The World, But The Maximum Is ($15,000.00) Fifteen Thousand Us Dollars Per Transaction. So, If You Like To Receive Your Fund This Way, $15,000 Usd For You To Withdraw For A Day And Each Transaction Is $5,000usd Minimum Which You Have To Withdraw $15,000 Usd For One Working Day. Also Be Informed That The Total Amount In The Swift ATM Card Is $14.6 Million Usd. Contact:COMPENSATION AGENT Email:DavegibsonEsq@gmail.comcom (1) Your Full Name: (2) Your Address Where You Want the Payment Centre to Send Your ATM Card.: (3) Phone and Fax Number: (4) Age and Occupation: (5) Your Nearest International Air Port in Your City Of Residence: We Shall Be Expecting To Receive Your Information, You Have To Stop Any Further Communication With Anybody Or Office On This Regards, Do Not Hesitate To Contact Me For More Details And Direction, And Please Do Update Me With Any New Development. Thanks for Your Co-Operation.
[PATCH v1 2/2] log -S: Add test which searches in binary files
The -S option of log looks for differences that changes the number of occurrences of the specified string (i.e. addition/deletion) in a file. Add a test to ensure that we keep looking into binary files with -S as changing that would break backwards compatibility in unexpected ways. Signed-off-by: Thomas Braun --- t/t4209-log-pickaxe.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh index 42cc8afd8b..d430f6f2f9 100755 --- a/t/t4209-log-pickaxe.sh +++ b/t/t4209-log-pickaxe.sh @@ -128,4 +128,15 @@ test_expect_success 'log -G looks into binary files with textconv filter' ' test_cmp actual expected ' +test_expect_success 'log -S looks into binary files' ' + rm -rf .git && + git init && + printf "a\0b" >data.bin && + git add data.bin && + git commit -m "message" && + git log -S a >actual && + git log >expected && + test_cmp actual expected +' + test_done -- 2.19.0.271.gfe8321ec05.dirty
[PATCH 0/2] Teach log -G to ignore binary files
Based on the previous discussion in [1] I've prepared patches which teach log -G to ignore binary files. log -S keeps its behaviour but got a test to ensure that. Feedback welcome! [1]: https://public-inbox.org/git/7a0992eb-adb9-a7a1-cfaa-3384bc4d3...@virtuell-zuhause.de/ Thomas Braun (2): log -G: Ignore binary files log -S: Add test which searches in binary files Documentation/gitdiffcore.txt | 2 +- diffcore-pickaxe.c| 5 + t/t4209-log-pickaxe.sh| 21 + 3 files changed, 27 insertions(+), 1 deletion(-) -- 2.19.0.271.gfe8321ec05.dirty
[PATCH v1 1/2] log -G: Ignore binary files
The -G option of log looks for the differences whose patch text contains added/removed lines that match regex. The concept of differences only makes sense for text files, therefore we need to ignore binary files when searching with -G as well. Signed-off-by: Thomas Braun --- Documentation/gitdiffcore.txt | 2 +- diffcore-pickaxe.c| 5 + t/t4209-log-pickaxe.sh| 22 ++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt index c0a60f3158..059ddd3431 100644 --- a/Documentation/gitdiffcore.txt +++ b/Documentation/gitdiffcore.txt @@ -242,7 +242,7 @@ textual diff has an added or a deleted line that matches the given regular expression. This means that it will detect in-file (or what rename-detection considers the same file) moves, which is noise. The implementation runs diff twice and greps, and this can be quite -expensive. +expensive. Binary files without textconv filter are ignored. When `-S` or `-G` are used without `--pickaxe-all`, only filepairs that match their respective criterion are kept in the output. When diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index 69fc55ea1e..8c2558b07d 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -144,6 +144,11 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o, textconv_two = get_textconv(o->repo->index, p->two); } + if ((o->pickaxe_opts & DIFF_PICKAXE_KIND_G) && + ((!textconv_one && diff_filespec_is_binary(o->repo, p->one)) || +(!textconv_two && diff_filespec_is_binary(o->repo, p->two + return 0; + /* * If we have an unmodified pair, we know that the count will be the * same and don't even have to load the blobs. Unless textconv is in diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh index 844df760f7..42cc8afd8b 100755 --- a/t/t4209-log-pickaxe.sh +++ b/t/t4209-log-pickaxe.sh @@ -106,4 +106,26 @@ test_expect_success 'log -S --no-textconv (missing textconv tool)' ' rm .gitattributes ' +test_expect_success 'log -G ignores binary files' ' + rm -rf .git && + git init && + printf "a\0b" >data.bin && + git add data.bin && + git commit -m "message" && + git log -G a >result && + test_must_be_empty result +' + +test_expect_success 'log -G looks into binary files with textconv filter' ' + rm -rf .git && + git init && + echo "* diff=bin" > .gitattributes && + printf "a\0b" >data.bin && + git add data.bin && + git commit -m "message" && + git -c diff.bin.textconv=cat log -G a >actual && + git log >expected && + test_cmp actual expected +' + test_done -- 2.19.0.271.gfe8321ec05.dirty
[PATCH 0/2] Teach log -G to ignore binary files
Based on the previous discussion in [1] I've prepared patches which teach log -G to ignore binary files. log -S keeps its behaviour but got a test to ensure that. Feedback welcome! [1]: https://public-inbox.org/git/7a0992eb-adb9-a7a1-cfaa-3384bc4d3...@virtuell-zuhause.de/ PS: This is the (possibly missing) cover letter.
Re: pathspec: problems with too long command line
On 21.11.2018 14:37, Junio C Hamano wrote: Jeff King writes: On Wed, Nov 21, 2018 at 10:23:34AM +0100, Marc Strapetz wrote: From our GUI client we are invoking git operations on a possibly large set of files. ... command line length, especially on Windows [1] and OSX [2]. To workaround this problem we are currently splitting up such operations by invoking multiple git commands. This works well for some commands (like add), but doesn't work well for others (like commit). Quite a few commands take --stdin, which can be used to send pathspecs (and often other stuff) without size limits. I don't think either "commit" or "add" does, but that might be another route. A GUI client, like your server, should not be using end-user facing Porcelain commands like "add" and "commit" anyway. In the standard "update-index" followed by "write-tree" followed-by "commit-tree" followed by "update-ref" sequence, the only thing that needs to take pathspec is the update-index step, and it already does take --stdin. In our case it's a desktop client. We didn't consider using plumbing commands in general but only in cases where no appropriate high level commands exist. One reason for this decision was definitely a lack of our understanding in the beginning (which is no excuse anymore :). Another reason is that our users have quite frequently requested to see invoked Git commands, for their own understanding and learning. I think this argument remains valid. A third reason is to reduce process invocations. Although we are quite experienced Git users now, one final reason may be that it's still desirable to rely on additional validation in high level commands. Summed up, I would prefer to find a solution which allows to stick with "git add"s, "git commit"s, "git checkout"s, ... and providing --stdin-paths alternatively to would be a good solution from a GUI client developer's perspective. I'm probably too biased to see whether it will be beneficial to standalone Git, too? I'm slightly nervous at a pathspec that starts reading arbitrary files, because I suspect there may be interesting ways to abuse it for services which expose Git. E.g., if I have a web service which can show the history of a file, I might take a $file parameter from the client and run "git rev-list -- $file" (handling shell quoting, of course). That's OK now, but with the proposed pathspec magic, a malicious user could ask for ":(from-file=/etc/passwd)" or whatever. In any case, I share your gut feeling that this should not be a magic pathspec, but should instead be "--stdin[-paths]", for command line parsing's sanity. Catchng random strings that begin with double dash as fishy is much simpler and more robust than having to tell if a string that is a risky or a benign magic pathspec. These are interesting points. Then --stdin[-paths] is definitely the better choice. -Marc
js/vsts-ci, was Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)
Hi Junio, On Wed, 21 Nov 2018, Junio C Hamano wrote: > * js/vsts-ci (2018-10-16) 13 commits > . travis: fix skipping tagged releases > . README: add a build badge (status of the Azure Pipelines build) > . tests: record more stderr with --write-junit-xml in case of failure > . tests: include detailed trace logs with --write-junit-xml upon failure > . git-p4: use `test_atexit` to kill the daemon > . git-daemon: use `test_atexit` in the tests > . tests: introduce `test_atexit` > . ci: add a build definition for Azure DevOps > . ci/lib.sh: add support for Azure Pipelines > . tests: optionally write results as JUnit-style .xml > . test-date: add a subcommand to measure times in shell scripts > . ci/lib.sh: encapsulate Travis-specific things > . ci: rename the library of common functions > > Prepare to run test suite on Azure DevOps. > > Ejected out of 'pu', as doing so seems to help other topics get > tested at TravisCI. > > https://travis-ci.org/git/git/builds/452713184 is a sample of a > build whose tests on 4 hang (with this series in). Ejecting it > gave us https://travis-ci.org/git/git/builds/452778963 which still > shows breakages from other topics not yet in 'next', but at least > the tests do not stall. Sorry about that. FWIW my current plan is to work a bit more on the Windows phase (to make it faster), and to split out the `test_atexit` patches (because they cause those hangs). I still think it is the right thing to do, but I lack the time to take care of it within the next weeks. Instead, I will try to run even the Windows phase in --verbose-log mode so that the --junit-xml code can pick up the verbose logs right away (read: no more re-running upon test failures). Hopefully this won't cause a speed regression. Ciao, Dscho
Re: Git for Windows v2.20.0-rc0, was Re: [ANNOUNCE] Git v2.20.0-rc0
On Wed, Nov 21, 2018 at 6:20 AM Jeff King wrote: > > On Tue, Nov 20, 2018 at 03:17:07PM -0800, Bryan Turner wrote: > > > I've run 2.20.0-rc0 through the test matrix for Bitbucket Server on > > both Linux and Windows, and the only failures were related to this > > change: > > > > * "git branch -l " used to be a way to ask a reflog to be > >created while creating a new branch, but that is no longer the > >case. It is a short-hand for "git branch --list " now. > > > > Since this is an intentional change I suspect there's nothing to do > > for it but patch Bitbucket Server and move on, but I'll confess it's a > > little frustrating that the option was deprecated in 2.19 and then > > immediately removed in the next minor release. Such a > > backwards-incompatible change seems far more apt for a major release, > > a perspective that's reinforced by having the change follow such a > > brief deprecation period--2.19.0 was only tagged September 10th (in my > > timezone), so it's been less than 3 months. (Looking at the git branch > > documentation for 2.18.0 [1] shows nothing about this deprecation; the > > messaging first appears in 2.19.0 [2]. To be honest, I didn't even > > realize it was deprecated until now, when it's gone--our test suite is > > automated, so the deprecation warning was not visible.) > > We did go a bit faster than usual, under the assumption that nobody's > really using "-l". It has been the default since 2006. > > Can you tell us a little more about your invocation? Our invocation is... A little difficult to nail down, if I'm honest. Bitbucket Server code does not use "git branch -l" anywhere in its _shipping_ code, only in its _test_ code. But that test code exists because Bitbucket Server provides a Java API [1][2] which allows third-party developers to easily build arbitrary Git commands to invoke for their own functionality. Setting `GitBranchCreateBuilder.reflog(true)` will trigger adding "-l" to the assembled "git branch" command. I've changed the code now so that it will use "--create-reflog" instead; however, because many of the Bitbucket Server add-ons on Marketplace [3], whether free or paid, are not open source, and because there are a significant number of in-house plugins that are not listed there, it's difficult to know how many might be affected. If I were to hazard a guess it would be _none_, but I've been surprised before. The end result is that the net impact is hard to predict--especially because Git on the server would need to be upgraded to 2.20. (In case you're curious why we used shorthand options, it's because of our Windows support. While "git branch" commands rarely, if ever, get very long, as a general rule we use shorthand options where they exist to keep our command lines shorter, to allow passing more options without hitting the hard limit (generally 32K) imposed by Windows--something we _have_ had issues with on other commands. For commands like "git diff", where it's not possible to pass in paths via stdin, every character matters.) To try and protect against the unexpected, we have a Supported Platforms [4] page which lists Git versions that we've _explicitly tested_ with Bitbucket Server. 2.20 won't be marked tested until a future release, so the majority of installs will not use it with older versions of the system--but there's always that subset who ignore the documentation. (Since we do text parsing on Git output, subtle breakages do happen from time to time.) I would say it's _unlikely_ that we'll hear of any installations where all the conditions are met for this to come up: - Git 2.20 - Bitbucket Server (without fixes) - Third-party add-on using `reflog(true)` It's really just that a) I was caught off guard by the change (my own fault for not reading the 2.19 announcement more carefully) and b) it's impossible for me to say with _certainty_ that it won't be an issue. I'd imagine that latter point is true of the change in general, though (it's not really possible to know what scripts it might break, and that's going to be true regardless of when the change actually gets released), and I'd agree that that shouldn't hold Git back from making useful improvements. Thanks for your time! Bryan [1] https://docs.atlassian.com/bitbucket-server/javadoc/5.16.0/git-api/reference/com/atlassian/bitbucket/scm/git/command/GitScmCommandBuilder.html [2] https://docs.atlassian.com/bitbucket-server/javadoc/5.16.0/git-api/reference/com/atlassian/bitbucket/scm/git/command/branch/GitBranchCreateBuilder.html [3] https://marketplace.atlassian.com/addons/app/bitbucket [4] https://confluence.atlassian.com/bitbucketserver/supported-platforms-776640981.html#Supportedplatforms-dvcsDVCS > > We still have time to avoid the change for this release. And this early > testing of master and release candidates is wonderful exactly to get > this kind of feedback before it's too late. > > -Peff
Re: [PATCH] technical doc: add a design doc for the evolve command
> I don't have a strong opinion about whether this would go in the > design doc. I suppose the doc could have an "implementation plan" > section describing temporary stopping points on the way to the final > result, but it's not necessary to include that. As long as this is something I'm just doing for fun and nobody needs to coordinate anything with me, I was planning to just document the endpoint and then work on whatever seems interesting at any given moment. Of course, if I found a job/team that would let me do this as my day job, I'd be more willing to commit to deliverables. - Stefan On Tue, Nov 20, 2018 at 5:33 PM Jonathan Nieder wrote: > > Stefan Xenos wrote: > > Jonathan Nieder wrote: > > >> putting it in the commit message is a way to > >> experiment with the workflow without changing the object format > > > > As long as we're talking about a temporary state of affairs for users > > that have opted in, and we're explicit about the fact that future > > versions of git won't understand the change graphs that are produced > > during that temporary state of affairs, I'm fine with using the commit > > message. We can move it to the header prior to enabling the feature by > > default. > > Yay! I think that addresses both my and Ævar's concerns. Also, if > you run into an issue that requires changing the object format > earlier, that's fine and we can handle the situation when it comes. > > I don't have a strong opinion about whether this would go in the > design doc. I suppose the doc could have an "implementation plan" > section describing temporary stopping points on the way to the final > result, but it's not necessary to include that. > > Thanks for the quick and thoughtful replies. > > Jonathan
Re: [PATCH 1/5] eoie: default to not writing EOIE section
On Tue, Nov 20, 2018 at 02:21:51PM +0100, SZEDER Gábor wrote: > On Tue, Nov 20, 2018 at 08:06:16AM -0500, Ben Peart wrote: > > >diff --git a/read-cache.c b/read-cache.c > > >index 4ca81286c0..1e9c772603 100644 > > >--- a/read-cache.c > > >+++ b/read-cache.c > > >@@ -2689,6 +2689,15 @@ void update_index_if_able(struct index_state > > >*istate, struct lock_file *lockfile > > > rollback_lock_file(lockfile); > > > } > > >+static int record_eoie(void) > > >+{ > > >+ int val; > > > > I believe you are going to want to initialize val to 0 here as it is on the > > stack so is not guaranteed to be zero. > > The git_config_get_bool() call below will initialize it anyway. Yes, there are two ways to write this. With a conditional to initialize and return or to return the default, as we have here: > > >+ if (!git_config_get_bool("index.recordendofindexentries", )) > > >+ return val; > > >+ return 0; Or initialize the default ahead of time, and rely on the function not to modify it when the entry is missing: int val = 0; git_config_get_bool("index.whatever", ); return val; I think either is perfectly fine, but since I also had to look at it twice to make sure it was doing the right thing, I figured it is worth mentioning as a possible style/convention thing we may want to decide on. -Peff
Re: [RFC 2/2] exclude-promisor-objects: declare when option is allowed
On Mon, Oct 22, 2018 at 06:13:42PM -0700, Matthew DeVore wrote: > diff --git a/builtin/prune.c b/builtin/prune.c > index 41230f8215..11284d0bf3 100644 > --- a/builtin/prune.c > +++ b/builtin/prune.c > @@ -120,6 +120,7 @@ int cmd_prune(int argc, const char **argv, const char > *prefix) > save_commit_buffer = 0; > read_replace_refs = 0; > ref_paranoia = 1; > + revs.allow_exclude_promisor_objects_opt = 1; > repo_init_revisions(the_repository, , prefix); > > argc = parse_options(argc, argv, prefix, options, prune_usage, 0); I think this line is in the wrong place. The very first thing repo_init_revisions() will do is memset() the revs struct to all-zeroes, so it cannot possibly be doing anything. Normally it would need to go after init_revisions() but before setup_revisions(), but we don't seem to call the latter at all in builtin/prune.c. Which makes sense, because you cannot pass options to influence the reachability traversal. So I don't think we need to care about this flag at all here. Speaking of which, would this flag work better as a field in setup_revision_opt, which is passed to setup_revisions()? The intent seem to be to influence how we parse command-line arguments, and that's where other similar flags are (e.g., assume_dashdash). -Peff
Re: Fwd: "show-ignore" problem after svn-git clone
I think I've got it now; maybe I just needed to sleep on it. It's happier if I use the whole URL for trunk in the -T parameter. I'll see how the rest of it plays out, but the `git svn show-ignore --id=origin/trunk` command is working now. On Wed, Nov 21, 2018 at 10:45 AM Jamie Jackson wrote: > > By the way, my goal is to pull in trunk (only) at first, and possibly > pull in certain branches (later) on an as-needed basis. I'll need to > sync the Git repo with SVN for a while, until we permanently switch to > Git (and put SVN in read-only). > On Wed, Nov 21, 2018 at 10:38 AM Jamie Jackson wrote: > > > > ICF2008571:eclipse-workspace jjackson$ git svn clone \ > > > -r 95115:HEAD https://mydomain.com/svn/HUD/onecpd \ > > > -T trunk \ > > > --no-metadata \ > > > -A ~/eclipse-workspace/scraps/git_migration/users.txt \ > > > hudx-git-migration > > ICF2008571:eclipse-workspace jjackson$ cd hudx-git-migration/ > > > > ICF2008571:hudx-git-migration jjackson$ git svn show-ignore > > fatal: bad revision 'HEAD' > > rev-list --first-parent --pretty=medium HEAD --: command returned error: 128 > > > > ICF2008571:hudx-git-migration jjackson$ git svn show-ignore > > --id=origin/trunk > > fatal: bad revision 'HEAD' > > rev-list --first-parent --pretty=medium HEAD --: command returned error: 128 > > > > ICF2008571:hudx-git-migration jjackson$ cat .git/config > > [core] > > repositoryformatversion = 0 > > filemode = true > > bare = false > > logallrefupdates = true > > ignorecase = true > > precomposeunicode = true > > [svn-remote "svn"] > > noMetadata = 1 > > url = https://mydomain.com/svn/HUD > > fetch = onecpd/trunk:refs/remotes/origin/trunk > > [svn] > > authorsfile = > > /Users/jjackson/eclipse-workspace/scraps/git_migration/users.txt > > > > On Wed, Nov 21, 2018 at 9:44 AM Konstantin Khomoutov > > wrote: > > > > > > On Wed, Nov 21, 2018 at 08:37:03AM -0500, Jamie Jackson wrote: > > > > > > > I'm brand new to svn-git and I'm having a problem right out of the > > > > gate. I suspect I need a different ID, but I have no clue how to get > > > > it. > > > > > > > > Here's the failed attempt: > > > > https://gist.github.com/jamiejackson/57e90302802f4990b36dfe28c3c71d13 > > > > > > Please post the content supposedly available at that link in your mail > > > message, inline. Otherwise it's impossibly to sensibly comment on it. > > >
Re: [PATCH v1 9/9] diff --color-moved-ws: handle blank lines
On 20/11/2018 18:05, Stefan Beller wrote: On Fri, Nov 16, 2018 at 3:04 AM Phillip Wood wrote: From: Phillip Wood When using --color-moved-ws=allow-indentation-change allow lines with the same indentation change to be grouped across blank lines. For now this only works if the blank lines have been moved as well, not for blocks that have just had their indentation changed. This completes the changes to the implementation of --color-moved=allow-indentation-change. Running git diff --color-moved=allow-indentation-change v2.18.0 v2.19.0 now takes 5.0s. This is a saving of 41% from 8.5s for the optimized version of the previous implementation and 66% from the original which took 14.6s. Signed-off-by: Phillip Wood --- Notes: Changes since rfc: - Split these changes into a separate commit. - Detect blank lines when processing the indentation rather than parsing each line twice. - Tweaked the test to make it harder as suggested by Stefan. - Added timing data to the commit message. diff.c | 34 --- t/t4015-diff-whitespace.sh | 41 ++ 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/diff.c b/diff.c index 89559293e7..072b5bced6 100644 --- a/diff.c +++ b/diff.c @@ -792,9 +792,11 @@ static void moved_block_clear(struct moved_block *b) memset(b, 0, sizeof(*b)); } +#define INDENT_BLANKLINE INT_MIN Answering my question from the previous patch: This is why we need to keep the indents signed. This patch looks quite nice to read along. The whole series looks good to me. Thanks Do we need to update the docs in any way? I'm not sure, at the moment it does not make any promises about the exact behavior of --color-moved-ws=allow-indentation-change, we could change it to be more explicit but I'm not sure it's worth it. Thanks for looking over these patches, I'll post a reroll soon based on your comments. Phillip Thanks, Stefan
Re: Fwd: "show-ignore" problem after svn-git clone
By the way, my goal is to pull in trunk (only) at first, and possibly pull in certain branches (later) on an as-needed basis. I'll need to sync the Git repo with SVN for a while, until we permanently switch to Git (and put SVN in read-only). On Wed, Nov 21, 2018 at 10:38 AM Jamie Jackson wrote: > > ICF2008571:eclipse-workspace jjackson$ git svn clone \ > > -r 95115:HEAD https://mydomain.com/svn/HUD/onecpd \ > > -T trunk \ > > --no-metadata \ > > -A ~/eclipse-workspace/scraps/git_migration/users.txt \ > > hudx-git-migration > ICF2008571:eclipse-workspace jjackson$ cd hudx-git-migration/ > > ICF2008571:hudx-git-migration jjackson$ git svn show-ignore > fatal: bad revision 'HEAD' > rev-list --first-parent --pretty=medium HEAD --: command returned error: 128 > > ICF2008571:hudx-git-migration jjackson$ git svn show-ignore --id=origin/trunk > fatal: bad revision 'HEAD' > rev-list --first-parent --pretty=medium HEAD --: command returned error: 128 > > ICF2008571:hudx-git-migration jjackson$ cat .git/config > [core] > repositoryformatversion = 0 > filemode = true > bare = false > logallrefupdates = true > ignorecase = true > precomposeunicode = true > [svn-remote "svn"] > noMetadata = 1 > url = https://mydomain.com/svn/HUD > fetch = onecpd/trunk:refs/remotes/origin/trunk > [svn] > authorsfile = /Users/jjackson/eclipse-workspace/scraps/git_migration/users.txt > > On Wed, Nov 21, 2018 at 9:44 AM Konstantin Khomoutov wrote: > > > > On Wed, Nov 21, 2018 at 08:37:03AM -0500, Jamie Jackson wrote: > > > > > I'm brand new to svn-git and I'm having a problem right out of the > > > gate. I suspect I need a different ID, but I have no clue how to get > > > it. > > > > > > Here's the failed attempt: > > > https://gist.github.com/jamiejackson/57e90302802f4990b36dfe28c3c71d13 > > > > Please post the content supposedly available at that link in your mail > > message, inline. Otherwise it's impossibly to sensibly comment on it. > >
Re: Fwd: "show-ignore" problem after svn-git clone
ICF2008571:eclipse-workspace jjackson$ git svn clone \ > -r 95115:HEAD https://mydomain.com/svn/HUD/onecpd \ > -T trunk \ > --no-metadata \ > -A ~/eclipse-workspace/scraps/git_migration/users.txt \ > hudx-git-migration ICF2008571:eclipse-workspace jjackson$ cd hudx-git-migration/ ICF2008571:hudx-git-migration jjackson$ git svn show-ignore fatal: bad revision 'HEAD' rev-list --first-parent --pretty=medium HEAD --: command returned error: 128 ICF2008571:hudx-git-migration jjackson$ git svn show-ignore --id=origin/trunk fatal: bad revision 'HEAD' rev-list --first-parent --pretty=medium HEAD --: command returned error: 128 ICF2008571:hudx-git-migration jjackson$ cat .git/config [core] repositoryformatversion = 0 filemode = true bare = false logallrefupdates = true ignorecase = true precomposeunicode = true [svn-remote "svn"] noMetadata = 1 url = https://mydomain.com/svn/HUD fetch = onecpd/trunk:refs/remotes/origin/trunk [svn] authorsfile = /Users/jjackson/eclipse-workspace/scraps/git_migration/users.txt On Wed, Nov 21, 2018 at 9:44 AM Konstantin Khomoutov wrote: > > On Wed, Nov 21, 2018 at 08:37:03AM -0500, Jamie Jackson wrote: > > > I'm brand new to svn-git and I'm having a problem right out of the > > gate. I suspect I need a different ID, but I have no clue how to get > > it. > > > > Here's the failed attempt: > > https://gist.github.com/jamiejackson/57e90302802f4990b36dfe28c3c71d13 > > Please post the content supposedly available at that link in your mail > message, inline. Otherwise it's impossibly to sensibly comment on it. >
Git Test Coverage Report (Wednesday Nov 21)
Here is today's test report. Thanks, -Stolee [1] https://dev.azure.com/git/git/_build/results?buildId=271=logs --- pu: c4a21f043160e02a25755bbf43e4d2fa0b9766aa jch: 29ea8ddbcef3ec1c79fffa23cc5751a45344754c next: 68bc7959f8dc2d629c09be1a52f1b95b977b3a13 master: bb75be6cb916297f271c846f2f9caa3daaaec718 master@{1}: d166e6afe5f257217836ef24a73764eba390c58d Uncovered code in 'pu' not in 'jch' -- builtin/blame.c c4a21f0431 builtin/blame.c 200) repo_unuse_commit_buffer(the_repository, commit, message); 74e8221b52 builtin/blame.c 928) blame_date_width = sizeof("Thu Oct 19 16:00"); 74e8221b52 builtin/blame.c 929) break; builtin/describe.c c4a21f0431 builtin/describe.c 257) repo_parse_commit(the_repository, p); builtin/pack-objects.c c4a21f0431 builtin/pack-objects.c 2834) if (!repo_has_object_file(the_repository, >oid) && is_promisor_object(>oid)) builtin/remote.c b7f4e371e7 builtin/remote.c 1551) die(_("--save-to-push cannot be used with other options")); b7f4e371e7 builtin/remote.c 1575) die(_("--save-to-push can only be used when only one url is defined")); date.c 74e8221b52 113) die("Timestamp too large for this system: %"PRItime, time); 74e8221b52 216) if (tm->tm_mon == human_tm->tm_mon) { 74e8221b52 217) if (tm->tm_mday > human_tm->tm_mday) { 74e8221b52 219) } else if (tm->tm_mday == human_tm->tm_mday) { 74e8221b52 220) hide.date = hide.wday = 1; 74e8221b52 221) } else if (tm->tm_mday + 5 > human_tm->tm_mday) { 74e8221b52 223) hide.date = 1; 74e8221b52 231) gettimeofday(, NULL); 74e8221b52 232) show_date_relative(time, tz, , buf); 74e8221b52 233) return; 74e8221b52 246) hide.seconds = 1; 74e8221b52 247) hide.tz |= !hide.date; 74e8221b52 248) hide.wday = hide.time = !hide.year; 74e8221b52 262) strbuf_rtrim(buf); 74e8221b52 287) gettimeofday(, NULL); 74e8221b52 290) human_tz = local_time_tzoffset(now.tv_sec, _tm); 74e8221b52 886) static int auto_date_style(void) 74e8221b52 888) return (isatty(1) || pager_in_use()) ? DATE_HUMAN : DATE_NORMAL; 74e8221b52 909) return DATE_HUMAN; 74e8221b52 911) return auto_date_style(); fast-import.c c4a21f0431 2793) buf = repo_read_object_file(the_repository, oid, , ); c4a21f0431 2899) buf = repo_read_object_file(the_repository, oid, , fsck.c c4a21f0431 858) repo_unuse_commit_buffer(the_repository, commit, buffer); c4a21f0431 878) repo_read_object_file(the_repository, c4a21f0431 879) >object.oid, , ); http-push.c c4a21f0431 1635) if (!repo_has_object_file(the_repository, _oid)) c4a21f0431 1642) if (!repo_has_object_file(the_repository, _ref->old_oid)) negotiator/default.c c4a21f0431 71) if (repo_parse_commit(the_repository, commit)) protocol.c 24c10f7473 37) die(_("Unrecognized protocol version")); 24c10f7473 39) die(_("Unrecognized protocol_version")); remote-curl.c 24c10f7473 403) return 0; revision.c c4a21f0431 726) if (repo_parse_commit(the_repository, p) < 0) sequencer.c c4a21f0431 1643) repo_unuse_commit_buffer(the_repository, head_commit, c4a21f0431 3914) repo_unuse_commit_buffer(the_repository, sha1-array.c bba406749a 91) oidcpy([dst], [src]); strbuf.c 10a40f5700 397) return 0; submodule.c e2419f7e30 1376) strbuf_release(); 7454fe5cb6 1499) struct get_next_submodule_task *task = task_cb; 7454fe5cb6 1503) get_next_submodule_task_release(task); 7454fe5cb6 1530) return 0; 7454fe5cb6 1534) goto out; 7454fe5cb6 1549) return 0; tree.c c4a21f0431 110) if (repo_parse_commit(the_repository, commit)) wrapper.c 5efde212fc 70) die("Out of memory, malloc failed (tried to allocate %" PRIuMAX " bytes)", 5efde212fc 73) error("Out of memory, malloc failed (tried to allocate %" PRIuMAX " bytes)", Commits introducing uncovered code: Anders Waldenborg 10a40f570: strbuf: separate callback for strbuf_expand:ing literals Denton Liu b7f4e371e: remote: add --save-to-push option to git remote set-url Josh Steadmon 24c10f747: protocol: advertise multiple supported versions Junio C Hamano c4a21f043: treewide: apply cocci patch Linus Torvalds 74e8221b5: Add 'human' date format Martin Koegler 5efde212f: zlib.c: use size_t for size Stefan Beller 7454fe5cb: fetch: try fetching submodules if needed objects were not fetched Stefan Beller bba406749: sha1-array: provide oid_array_filter Stefan Beller e2419f7e3: submodule: migrate get_next_submodule to use repository structs Uncovered code in 'jch' not in 'next' apply.c 0f086e6dca 3355) if (checkout_entry(ce, , NULL, NULL) || 0f086e6dca 3356) lstat(ce->name, st)) builtin/branch.c 0ecb1fc726 builtin/branch.c 456) die(_("could not resolve HEAD")); 0ecb1fc726 builtin/branch.c 462) die(_("HEAD (%s) points outside of refs/heads/"), refname); builtin/pull.c b19eee9066 647) argv_array_push(, opt_cleanup); hex.c 47edb64997 93) char *sha1_to_hex_r(char *buffer, const unsigned char *sha1) 47edb64997 95) return hash_to_hex_algop_r(buffer,
Re: Fwd: "show-ignore" problem after svn-git clone
On Wed, Nov 21, 2018 at 08:37:03AM -0500, Jamie Jackson wrote: > I'm brand new to svn-git and I'm having a problem right out of the > gate. I suspect I need a different ID, but I have no clue how to get > it. > > Here's the failed attempt: > https://gist.github.com/jamiejackson/57e90302802f4990b36dfe28c3c71d13 Please post the content supposedly available at that link in your mail message, inline. Otherwise it's impossibly to sensibly comment on it.
[ANNOUNCE] Git Rev News edition 45
Hi everyone, The 45th edition of Git Rev News is now published: https://git.github.io/rev_news/2018/11/21/edition-45/ Thanks a lot to the contributors: Elijah Newren, Luca Milanesio, Derrick Stolee and Johannes Schindelin! Enjoy, Christian, Jakub, Markus and Gabriel.
Re: Git for Windows v2.20.0-rc0, was Re: [ANNOUNCE] Git v2.20.0-rc0
On Tue, Nov 20, 2018 at 03:17:07PM -0800, Bryan Turner wrote: > I've run 2.20.0-rc0 through the test matrix for Bitbucket Server on > both Linux and Windows, and the only failures were related to this > change: > > * "git branch -l " used to be a way to ask a reflog to be >created while creating a new branch, but that is no longer the >case. It is a short-hand for "git branch --list " now. > > Since this is an intentional change I suspect there's nothing to do > for it but patch Bitbucket Server and move on, but I'll confess it's a > little frustrating that the option was deprecated in 2.19 and then > immediately removed in the next minor release. Such a > backwards-incompatible change seems far more apt for a major release, > a perspective that's reinforced by having the change follow such a > brief deprecation period--2.19.0 was only tagged September 10th (in my > timezone), so it's been less than 3 months. (Looking at the git branch > documentation for 2.18.0 [1] shows nothing about this deprecation; the > messaging first appears in 2.19.0 [2]. To be honest, I didn't even > realize it was deprecated until now, when it's gone--our test suite is > automated, so the deprecation warning was not visible.) We did go a bit faster than usual, under the assumption that nobody's really using "-l". It has been the default since 2006. Can you tell us a little more about your invocation? We still have time to avoid the change for this release. And this early testing of master and release candidates is wonderful exactly to get this kind of feedback before it's too late. -Peff
Re: [PATCH v2 1/2] ref-filter: add worktree atom
On Wed, Nov 21, 2018 at 03:05:04PM +0100, Nickolai Belakovski wrote: > I think if we move to making this atom just store worktree path, that > needs to be implemented as a hashmap of refname->wtpath, which would > also solve this string_list issue, correct? Just making sure I'm not > missing something before I submit another patch. string_list has a "util" field, so you actually _can_ use it to create a mapping. I do think a hashmap is a little more obvious. OTOH, the hashmap API is a little tricky; if we are going to add a "strmap" API soon, it may be simpler to just use a string_list now and convert to strmap when it is a available. -Peff
Re: [PATCH v2 2/2] branch: Mark and colorize a branch differently if it is checked out in a linked worktree
OK, I see 3 votes for cyan and 4-5 people participating in the thread, so I'll make it cyan in the next revision. On Tue, Nov 13, 2018 at 3:49 PM Jeff King wrote: > > On Mon, Nov 12, 2018 at 06:07:18PM +, Rafael Ascensão wrote: > > > On Mon, Nov 12, 2018 at 07:14:23AM -0500, Jeff King wrote: > > > just adding a bunch of color variants. It would be nice if we could just > > > do this with a run-time parse_color("bold red") or whatever, but we use > > > these as static initializers. > > > > I suggested those colors, but now, I think this needs to be > > configurable. > > I think they are configurable in that patch, since it provides > "worktree" as a n entry in color_branch_slots. But yeah, every color we > add needs to be configurable, and this is really just about defaults. > > > I suggested using green and dim green as the obvious theoretical choice > > but after using it for a while I found out that both shades are way too > > similar, making it really hard to tell by glancing at the output, > > especially when they're not side by side. > > > > If we continue with two dual green approach, current branch needs to be > > at least bold. But I'm not sure if it's enough. > > > > I've been trying some other colors, and cyan feels neutral-ish. > > Yeah, cyan seems pretty reasonable to me. > > -Peff
Re: [PATCH 1/1] legacy-rebase: backport -C and --whitespace= checks
Hi Junio, On Wed, 21 Nov 2018, Junio C Hamano wrote: > Carlo Arenas writes: > > > Tested-by: Carlo Marcelo Arenas Belón > > > > the C version prepends: "fatal: " unlike the shell version for both > > error messages > > In addition, "Invalid whitespace option" says 'bad', not > '--whitespace=bad', in the builtin version. > > I think the following would address both issues. Yes! Thank you. Can you squash it in? Thanks, Dscho > > > git-legacy-rebase.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh > index ced0635326..b97ffdc9dd 100755 > --- a/git-legacy-rebase.sh > +++ b/git-legacy-rebase.sh > @@ -340,7 +340,7 @@ do > warn|nowarn|error|error-all) > ;; # okay, known whitespace option > *) > - die "Invalid whitespace option: '${1%*=}'" > + die "fatal: Invalid whitespace option: '${1#*=}'" > ;; > esac > ;; > @@ -358,7 +358,7 @@ do > force_rebase=t > ;; > -C*[!0-9]*) > - die "switch \`C' expects a numerical value" > + die "fatal: switch \`C' expects a numerical value" > ;; > -C*) > git_am_opt="$git_am_opt $1" > > >
Re: [PATCH v2 1/2] ref-filter: add worktree atom
I think if we move to making this atom just store worktree path, that needs to be implemented as a hashmap of refname->wtpath, which would also solve this string_list issue, correct? Just making sure I'm not missing something before I submit another patch. On Tue, Nov 13, 2018 at 2:38 AM Junio C Hamano wrote: > > Jeff King writes: > > >> I wonder if "The worktree at /local/src/wt1 has this branch checked > >> out" is something the user of %(worktree) atom, or a variant thereof > >> e.g. "%(worktree:detailed)", may want to learn, but because that > >> information is lost when this function returns, such an enhancement > >> cannot be done without fixing this funciton. > > > > Hmm. I think for the purposes of this series we could jump straight to > > converting %(worktree) to mean "the path of the worktree for which this > > branch is HEAD, or the empty string otherwise". > > > > Then the caller from git-branch (or anybody wanting to emulate it) could > > still do: > > > > %(if)%(worktree)%(then)+ %(refname)%(end) > > > > As a bonus, the decision to use "+" becomes a lot easier. It is no > > longer a part of the format language that we must promise forever, but > > simply a porcelain decision by git-branch. > > Yeah, thanks for following through the thought process to the > logical conclusion. If a branch is multply checked out, which is a > condition "git worktree" and "git checkout" ought to prevent from > happening, we could leave the result unspecified but a non-empty > string, or something like that. > > > file-global data-structure storing the worktree info once (in an ideal > > world, it would be part of a "struct ref_format" that uses no global > > variables, but that is not how the code is structured today). > > Yes, I agree that would be the ideal longer-term direction to move > this code in. > > >> > + } else if (!strcmp(name, "worktree")) { > >> > + if (string_list_has_string(>u.worktree_heads, > >> > ref->refname)) > >> > >> I thought we were moving towards killing the use of string_list as a > >> look-up table, as we do not want to see thoughtless copy such > >> a code from parts of the code that are not performance critical to a > >> part. Not very satisfying. > >> > >> I think we can let this pass, and later add a wrapper around > >> hashmap that is meant to only be used to replace string-list > >> used for this exact purpose, i.e. key is a string, and there > >> is no need to iterate over the existing elements in any > >> sorted order. Optionally, we can limit the look up to only > >> checking for existence, if it makes the code for the wrapper > >> simpler. > > > > This came up over in another thread yesterday, too. So yeah, perhaps we > > should move on that (I am OK punting on it for this series and > > converting it later, though). > > FWIW, I am OK punting and leaving, too.
Re: git grep prints colon instead of slash
On Wed, Nov 21, 2018 at 02:54:34PM +0100, Marc Gonzalez wrote: > If I specify the branch to explore, git grep prints a colon instead of > a slash in the path: > > $ git grep arm,coresight-tmc master:Documentation/devicetree > master:Documentation/devicetree:bindings/arm/coresight.txt: > "arm,coresight-tmc", "arm,primecell"; > master:Documentation/devicetree:bindings/arm/coresight.txt: > compatible = "arm,coresight-tmc", "arm,primecell"; >^ > HERE > > There is no such issue when the branch is not specified: > > $ git grep arm,coresight-tmc Documentation/devicetree > Documentation/devicetree/bindings/arm/coresight.txt: > "arm,coresight-tmc", "arm,primecell"; > Documentation/devicetree/bindings/arm/coresight.txt:compatible = > "arm,coresight-tmc", "arm,primecell"; > ^ > NO ISSUE > > > Is this expected behavior? > The spurious colon prevents one from simply copy/pasting the output. There's lots of discussion in this thread from last year: https://public-inbox.org/git/20170119150347.3484-1-stefa...@redhat.com/ Notably: git grep arm,coresight-tmc master -- Documentation/devicetree will do what you want. -Peff
git grep prints colon instead of slash
Hello, I'm using an older version of git, so this particular issue might have already been fixed. $ git --version git version 2.17.1 If I specify the branch to explore, git grep prints a colon instead of a slash in the path: $ git grep arm,coresight-tmc master:Documentation/devicetree master:Documentation/devicetree:bindings/arm/coresight.txt: "arm,coresight-tmc", "arm,primecell"; master:Documentation/devicetree:bindings/arm/coresight.txt: compatible = "arm,coresight-tmc", "arm,primecell"; ^ HERE There is no such issue when the branch is not specified: $ git grep arm,coresight-tmc Documentation/devicetree Documentation/devicetree/bindings/arm/coresight.txt: "arm,coresight-tmc", "arm,primecell"; Documentation/devicetree/bindings/arm/coresight.txt:compatible = "arm,coresight-tmc", "arm,primecell"; ^ NO ISSUE Is this expected behavior? The spurious colon prevents one from simply copy/pasting the output. Regards.
From Barr
>From Barr This is Barr Philip Twite reaching you from the United Kingdom further to my previous email notice. I have not received your response till this date. Kindly forward to me the required information to engender further discuss. Information should include; your full Names Address Telephone number Private e-mail Urgent response solicited. Kind regards, Philip Twite REPLY TO uk1...@mail.com.tr
Re: pathspec: problems with too long command line
Jeff King writes: > On Wed, Nov 21, 2018 at 10:23:34AM +0100, Marc Strapetz wrote: > >> From our GUI client we are invoking git operations on a possibly large set >> of files. ... >> command line length, especially on Windows [1] and OSX [2]. To workaround >> this problem we are currently splitting up such operations by invoking >> multiple git commands. This works well for some commands (like add), but >> doesn't work well for others (like commit). > Quite a few commands take --stdin, which can be used to send pathspecs > (and often other stuff) without size limits. I don't think either > "commit" or "add" does, but that might be another route. A GUI client, like your server, should not be using end-user facing Porcelain commands like "add" and "commit" anyway. In the standard "update-index" followed by "write-tree" followed-by "commit-tree" followed by "update-ref" sequence, the only thing that needs to take pathspec is the update-index step, and it already does take --stdin. In any case, I share your gut feeling that this should not be a magic pathspec, but should instead be "--stdin[-paths]", for command line parsing's sanity. Catchng random strings that begin with double dash as fishy is much simpler and more robust than having to tell if a string that is a risky or a benign magic pathspec.
Fwd: "show-ignore" problem after svn-git clone
I'm brand new to svn-git and I'm having a problem right out of the gate. I suspect I need a different ID, but I have no clue how to get it. Here's the failed attempt: https://gist.github.com/jamiejackson/57e90302802f4990b36dfe28c3c71d13 What am I doing wrong? Thanks, Jamie
Re: pathspec: problems with too long command line
On Wed, Nov 21, 2018 at 10:23:34AM +0100, Marc Strapetz wrote: > From our GUI client we are invoking git operations on a possibly large set > of files. This may result in pathspecs which are exceeding the maximum > command line length, especially on Windows [1] and OSX [2]. To workaround > this problem we are currently splitting up such operations by invoking > multiple git commands. This works well for some commands (like add), but > doesn't work well for others (like commit). > > A possible solution could be to add another patchspec magic word which will > read paths from a file instead of command line. A similar approach can be > found in Mercurial with its "listfile:" pattern [3]. > > Does that sound reasonable? If so, we should be able to provide a > corresponding patch. Quite a few commands take --stdin, which can be used to send pathspecs (and often other stuff) without size limits. I don't think either "commit" or "add" does, but that might be another route. I'm slightly nervous at a pathspec that starts reading arbitrary files, because I suspect there may be interesting ways to abuse it for services which expose Git. E.g., if I have a web service which can show the history of a file, I might take a $file parameter from the client and run "git rev-list -- $file" (handling shell quoting, of course). That's OK now, but with the proposed pathspec magic, a malicious user could ask for ":(from-file=/etc/passwd)" or whatever. I dunno. Maybe that is overly paranoid, and certainly servers like that are a subset of users. And perhaps such servers should be specifying GIT_LITERAL_PATHSPECS=1 anyway. -Peff
pathspec: problems with too long command line
From our GUI client we are invoking git operations on a possibly large set of files. This may result in pathspecs which are exceeding the maximum command line length, especially on Windows [1] and OSX [2]. To workaround this problem we are currently splitting up such operations by invoking multiple git commands. This works well for some commands (like add), but doesn't work well for others (like commit). A possible solution could be to add another patchspec magic word which will read paths from a file instead of command line. A similar approach can be found in Mercurial with its "listfile:" pattern [3]. Does that sound reasonable? If so, we should be able to provide a corresponding patch. -Marc [1] https://blogs.msdn.microsoft.com/oldnewthing/20031210-00/?p=41553/ [2] https://serverfault.com/questions/69430 [3] https://www.mercurial-scm.org/repo/hg/help/patterns
Re: [PATCH] technical doc: add a design doc for the evolve command
Hi Stefan On 20/11/2018 20:24, Stefan Xenos wrote: >> If a merge has been cherry-picked we cannot update it as we don't record >> which parent was used for the pick, however it is probably not a problem >> in practice - I think it is unusual to amend merges. > > I've read and reread that sentence several times and don't fully > understand it. Could you elaborate? Sorry if I wasn't very clear. To cherry-pick (or revert) a merge commit one has to specify a parent of the commit being picked with -m for cherry-pick to use as the merge base for the three way merge that creates the new commit. If the original merge is updated then evolve wont know which parent to use as the merge base when evolving the cherry-picked version of the merge as the parent is not recorded in the meta commit. > It sounds scary, though. With the evolve command, amending merges will > need to be supported. Evolving a merge should be fine, I was just referring to merges that have been cherry-picked. Best Wishes Phillip (Thanks for your reply to my other message, I'm still digesting it at the moment, once I've done that and found the references to mercurial using commit obsolescence information in rebase and pull I'll reply.) > If you create a merge and then amend one of its > parent commits, the evolve command will need to rebase the merge and > point one or both parents to the replacement instead. > > - Stefan > On Tue, Nov 20, 2018 at 5:03 AM Phillip Wood > wrote: >> >> On 15/11/2018 00:55, sxe...@google.com wrote: >>> From: Stefan Xenos >>> >>> +Obsolescence across cherry-picks >>> + >>> +By default the evolve command will treat cherry-picks and squash merges as >>> being >>> +completely separate from the original. Further amendments to the original >>> commit >>> +will have no effect on the cherry-picked copy. However, this behavior may >>> not be >>> +desirable in all circumstances. >>> + >>> +The evolve command may at some point support an option to look for cases >>> where >>> +the source of a cherry-pick or squash merge has itself been amended, and >>> +automatically apply that same change to the cherry-picked copy. In such >>> cases, >>> +it would traverse origin edges rather than ignoring them, and would treat a >>> +commit with origin edges as being obsolete if any of its origins were >>> obsolete. >> >> If a merge has been cherry-picked we cannot update it as we don't record >> which parent was used for the pick, however it is probably not a problem >> in practice - I think it is unusual to amend merges. >> >> Best Wishes >> >> Phillip
Re: [PATCH] t5562: skip if NO_CURL is enabled
FWIW the issue goes away when more than 1 CPU is used in NetBSD 8,0 (32-bit) and for some tracing, it would seem that it gets 0 when trying to read 4 bytes from what I think is a pipe that connects to a child that has been gone already for a while. Carlo
Re: [PATCH v2 1/1] Use size_t instead of 'unsigned long' for data in memory
On 11/20/2018 12:04 AM, tbo...@web.de wrote: From: Torsten Bögershausen Currently the length of data which is stored in memory is stored in "unsigned long" at many places in the code base. This is OK when both "unsigned long" and size_t are 32 bits, (and is OK when both are 64 bits). On a 64 bit Windows system am "unsigned long" is 32 bit, and that may be too short to measure the size of objects in memory, a size_t is the natural choice. Is this change enough to store 4GB files on Windows? Or is there more to do? Thanks for all the comments on V1. Changes since V1: - Make the motivation somewhat clearer in the commit message - Rebase to the November 19 pu What we really need for this patch to fly are this branches: mk/use-size-t-in-zlib tb/print-size-t-with-uintmax-format I believe communicating these direct dependencies is the correct way to go, and the rebase you mentioned is unnecessary (instead, test with a merge). Hopefully the patch applies on top of a merge of those branches. @@ -529,7 +530,7 @@ static void *unpack_raw_entry(struct object_entry *obj, } static void *unpack_data(struct object_entry *obj, -int (*consume)(const unsigned char *, unsigned long, void *), +int (*consume)(const unsigned char *, size_t, void *), void *cb_data) This is the only instance that is not paired directly with a variable name like "size", "sz", or "len". However, it appears to be correct from context. Thanks for this! Reviewed-by: Derrick Stolee
Re: [PATCH 2/2] commit-graph: don't call write_graph_chunk_large_edges() unnecessarily
On 11/20/2018 8:26 PM, SZEDER Gábor wrote: write_graph_chunk_data(f, GRAPH_OID_LEN, commits.list, commits.nr); - write_graph_chunk_large_edges(f, commits.list, commits.nr); + if (num_large_edges) + write_graph_chunk_large_edges(f, commits.list, commits.nr); This is clearly correct, and the tests in t5318-commit-graph.sh would catch a dropped (or additional) large/extra edge chunk. Thanks, -Stolee
Re: [PATCH 1/2] commit-graph: rename 'num_extra_edges' variable to 'num_large_edges'
On 11/20/2018 10:29 PM, Junio C Hamano wrote: SZEDER Gábor writes: I rename these variables to 'num_large_edges', because the commit graph file format speaks about the 'Large Edge List' chunk. However, I do find that the term 'extra' makes much more sense Would it make sense to do the rename in the other direction? I tend to agree that "large edge" is a misnomer. I agree with you both. "Extra" is better. Thanks, -Stolee
Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)
On Wed, Nov 21 2018, Junio C Hamano wrote: > * jk/loose-object-cache (2018-11-13) 9 commits > (merged to 'next' on 2018-11-18 at 276691a21b) > + fetch-pack: drop custom loose object cache > + sha1-file: use loose object cache for quick existence check > + object-store: provide helpers for loose_objects_cache > + sha1-file: use an object_directory for the main object dir > + handle alternates paths the same as the main object dir > + sha1_file_name(): overwrite buffer instead of appending > + rename "alternate_object_database" to "object_directory" > + submodule--helper: prefer strip_suffix() to ends_with() > + fsck: do not reuse child_process structs > > Code clean-up with optimization for the codepath that checks > (non-)existence of loose objects. > > Will cook in 'next'. I think as noted in https://public-inbox.org/git/e5148b8c-9a3a-5d2e-ac8c-3e536c0f2...@web.de/ that we should hold off the [89]/9 of this series due to the performance regressions this introduces in some cases (while fixing other cases). I hadn't had time to follow up on that, and figured it could wait until post-2.20 for a re-roll.
Re: [PATCH v4 0/2] Fix scissors bug during merge conflict
Denton Liu writes: > Changes since V3: > * Add patch to cleanup 'merge --squash c3 with c7' test in t7600 > * Use test_i18ncmp instead of test_cmp to pass GETTEXT_POISON tests Queued. Thanks, both.
Re: [PATCH 5/5] index: offer advice for unknown index extensions
On Wed, Nov 21 2018, Jonathan Nieder wrote: > Junio C Hamano wrote: > >> This series has a strong smell of pushing back by the >> toolsmiths who refuse to promptly upgrade to help their users, and >> that is why I do not feel entirely happy with this series. > > Last reply, I promise. :) > > This sentence might have the key to the misunderstanding. Let me say > a little more about where this showed up in the internal deployment > here, to clarify things a little. > > At Google we deploy snapshots of the "next" branch approximately > weekly so that we can find problems early before they affect a > published release. We rely on the ability to roll back quickly when a > problem is discovered, and we might care more about compatibility than > some others because of that. > > A popular tool within Google has a bundled copy of Git (also a > snapshot of the "next" branch, but from a few weeks prior) and when we > deployed Git with the EOIE and IEOT extensions, users of that tool > very quickly reported the mysterious message. > > That said, the maintainers of that tool did not complain at all, so > hopefully I can allay your worries about toolsmiths pushing back. > Once the problem reached my attention (a few days later than I would > have liked it to), the Git team at Google knew that we could not roll > back and were certainly alarmed about what that means about our > ability to cope with other problems should we need to. But we were > able to quickly update that popular tool --- no issue. > > Instead, we ran into a number of other users running into the same > problem, when sharing repositories between machines using sshfs, etc. > That, plus the aforementioned inability to roll back Git if we need > to, meant that this was a serious issue so we quickly addressed it in > the internal installation. > > In general, we haven't had much trouble getting people to use Git > 2.19.1 or newer. So the problem here does not have to do with users > being slow to upgrade. > > Instead, it's simply that upgrading Git should not cause the older, > widely deployed version of Git to complain about the repositories it > acts on. That's a recipe for difficult debugging situations, it can > lead to people upgrading less quickly and reporting bugs later, and > all in all it's a bad situation to be in. I've used tools like > Subversion that would upgrade repositories so they are unusable by the > previous version and experienced all of these problems. > > So I consider it important *to Git upstream* to handle this well in > the Git 2.20 release. We can flip the default soon after, even as > soon as 2.21. > > Moreover, I am not the only one who ran into this --- e.g. from [1], > 2018-10-19: > > 17:10 jrnieder: Yes, I noticed that annoyance myself. ;) > 17:11 Yeah, I saw that message a few times and was slightly > annoyed as well. > > Now, a meta point. Throughout this discussion, I have been hoping for > some acknowledgement of the problem --- e.g. an "I am sympathetic to > what you are trying to do, but ". I wasn't able to find that, and > that is part of what contributed to the feeling of not being heard. > > Thanks for your patient explanations, and hope that helps, > Jonathan I think it makes total sense to fix this. I had not spotted this myself since I tend to just roll forward and only use one version of git on one system, but fixing this makes sense.
What's cooking in git.git (Nov 2018, #06; Wed, 21)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. The group of last-minute fixes marked as "Will merge to 'master'" hopefully will be part of -rc1. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * ab/dynamic-gettext-poison (2018-11-09) 2 commits (merged to 'next' on 2018-11-18 at 13247a3be6) + Makefile: ease dynamic-gettext-poison transition + i18n: make GETTEXT_POISON a runtime option Our testing framework uses a special i18n "poisoned localization" feature to find messages that ought to stay constant but are incorrectly marked to be translated. This feature has been made into a runtime option (it used to be a compile-time option). * ab/range-diff-no-patch (2018-11-14) 3 commits (merged to 'next' on 2018-11-17 at c42e0891d0) + range-diff: make diff option behavior (e.g. --stat) consistent + range-diff: fix regression in passing along diff options + range-diff doc: add a section about output stability The "--no-patch" option, which can be used to get a high-level overview without the actual line-by-line patch difference shown, of the "range-diff" command was earlier broken, which has been corrected. * ab/rebase-in-c-escape-hatch (2018-11-16) 2 commits (merged to 'next' on 2018-11-17 at a01be221c7) + tests: add a special setup where rebase.useBuiltin is off + rebase doc: document rebase.useBuiltin The recently merged "rebase in C" has an escape hatch to use the scripted version when necessary, but it hasn't been documented, which has been corrected. * ag/p3400-force-checkout (2018-11-12) 1 commit (merged to 'next' on 2018-11-17 at 87ff48d52a) + p3400: replace calls to `git checkout -b' by `git checkout -B' Perf test tweak. * cb/notes-freeing-always-null-fix (2018-11-13) 1 commit (merged to 'next' on 2018-11-17 at 47aeec5fc9) + builtin/notes: remove unnecessary free Code cleanup. * dd/poll-dot-h (2018-11-14) 1 commit (merged to 'next' on 2018-11-18 at b6745f3308) + git-compat-util: prefer poll.h to sys/poll.h A build update. * ds/push-squelch-ambig-warning (2018-11-07) 1 commit (merged to 'next' on 2018-11-18 at 5e8b3db129) + pack-objects: ignore ambiguous object warnings "git push" used to check ambiguities between object-names and refnames while processing the list of refs' old and new values, which was unnecessary (as it knew that it is feeding raw object names). This has been optimized out. * ds/reachable-topo-order (2018-11-02) 7 commits (merged to 'next' on 2018-11-13 at 4155d01aee) + t6012: make rev-list tests more interesting + revision.c: generation-based topo-order algorithm + commit/revisions: bookkeeping before refactoring + revision.c: begin refactoring --topo-order logic + test-reach: add rev-list tests + test-reach: add run_three_modes method + prio-queue: add 'peek' operation The revision walker machinery learned to take advantage of the commit generation numbers stored in the commit-graph file. * jk/close-duped-fd-before-unlock-for-bundle (2018-11-17) 1 commit (merged to 'next' on 2018-11-17 at 2fe598284e) + bundle: dup() output descriptor closer to point-of-use When "git bundle" aborts due to an empty commit ranges (i.e. resulting in an empty pack), it left a file descriptor to an lockfile open, which resulted in leftover lockfile on Windows where you cannot remove a file with an open file descriptor. This has been corrected. * jk/curl-ldflags (2018-11-05) 1 commit (merged to 'next' on 2018-11-13 at d1387a3aa0) + build: link with curl-defined linker flags The way -lcurl library gets linked has been simplified by taking advantage of the fact that we can just ask curl-config command how. * jk/unused-parameter-fixes (2018-11-06) 14 commits (merged to 'next' on 2018-11-13 at 8d3625b4ae) + midx: double-check large object write loop + assert NOARG/NONEG behavior of parse-options callbacks + parse-options: drop OPT_DATE() + apply: return -1 from option callback instead of calling exit(1) + cat-file: report an error on multiple --batch options + tag: mark "--message" option with NONEG + show-branch: mark --reflog option as NONEG + format-patch: mark "--no-numbered" option with NONEG + status: mark --find-renames option with NONEG + cat-file: mark batch options with NONEG + pack-objects: mark index-version option as NONEG + ls-files: mark exclude options as NONEG + am: handle --no-patch-format option + apply: mark include/exclude options as NONEG Various functions have been audited for "-Wunused-parameter" warnings and bugs in them got fixed. * jk/verify-sig-merge-into-void