Re: [PATCH] t5562: skip if NO_CURL is enabled

2018-11-21 Thread Max Kirillov
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.

2018-11-21 Thread Mgr Georg Black
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

2018-11-21 Thread Elijah Newren
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

2018-11-21 Thread Elijah Newren
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

2018-11-21 Thread Elijah Newren
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

2018-11-21 Thread Elijah Newren
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

2018-11-21 Thread Elijah Newren
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

2018-11-21 Thread Elijah Newren
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

2018-11-21 Thread Elijah Newren
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

2018-11-21 Thread Elijah Newren
[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)

2018-11-21 Thread Stephen P. Smith
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

2018-11-21 Thread Junio C Hamano
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

2018-11-21 Thread Junio C Hamano
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

2018-11-21 Thread Denton Liu
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)

2018-11-21 Thread Junio C Hamano
"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

2018-11-21 Thread Carlo Arenas
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

2018-11-21 Thread Junio C Hamano
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)

2018-11-21 Thread Stephen P. Smith
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

2018-11-21 Thread Carlo Arenas
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

2018-11-21 Thread Max Kirillov
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

2018-11-21 Thread Johannes Schindelin
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

2018-11-21 Thread :COMPENSATION AGENTt
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

2018-11-21 Thread Thomas Braun
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

2018-11-21 Thread Thomas Braun
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

2018-11-21 Thread Thomas Braun
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

2018-11-21 Thread Thomas Braun
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

2018-11-21 Thread Marc Strapetz

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)

2018-11-21 Thread Johannes Schindelin
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

2018-11-21 Thread Bryan Turner
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

2018-11-21 Thread Stefan Xenos
>   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

2018-11-21 Thread Jeff King
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

2018-11-21 Thread Jeff King
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

2018-11-21 Thread Jamie Jackson
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

2018-11-21 Thread Phillip Wood

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

2018-11-21 Thread Jamie Jackson
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

2018-11-21 Thread Jamie Jackson
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)

2018-11-21 Thread Derrick Stolee

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

2018-11-21 Thread Konstantin Khomoutov
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

2018-11-21 Thread Christian Couder
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

2018-11-21 Thread Jeff King
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

2018-11-21 Thread Jeff King
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

2018-11-21 Thread Nickolai Belakovski
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

2018-11-21 Thread Johannes Schindelin
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

2018-11-21 Thread Nickolai Belakovski
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

2018-11-21 Thread Jeff King
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

2018-11-21 Thread Marc Gonzalez
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

2018-11-21 Thread davidasare70005
>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

2018-11-21 Thread Junio C Hamano
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

2018-11-21 Thread Jamie Jackson
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

2018-11-21 Thread Jeff King
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

2018-11-21 Thread Marc Strapetz
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

2018-11-21 Thread Phillip Wood
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

2018-11-21 Thread Carlo Arenas
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

2018-11-21 Thread Derrick Stolee

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

2018-11-21 Thread Derrick Stolee

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'

2018-11-21 Thread Derrick Stolee

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)

2018-11-21 Thread Ævar Arnfjörð Bjarmason


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

2018-11-21 Thread Junio C Hamano
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

2018-11-21 Thread Ævar Arnfjörð Bjarmason


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)

2018-11-21 Thread Junio C Hamano
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