What's cooking in git.git

What's cooking in git.git

[Junio C Hamano]

This is still a draft but only to show the highlights on new topics.

* bb/remote-get-url (2015-08-05) 1 commit
 - remote: add get-url subcommand

 "git remote" learned "get-url" subcommand to show the URL for a
 given remote name used for fetching and pushing.

 Expecting a reroll.


* dt/untracked-subdir (2015-08-05) 2 commits
 - DONTMERGE: Waiting for an Ack from Duy
 - untracked-cache: fix subdirectory handling
 (this branch uses dt/untracked-sparse.)

 This seems to break some tests.



* cb/open-noatime-clear-errno (2015-08-05) 1 commit
 - git_open_noatime: return with errno=0 on success

 Breaks build; need to tweak.


* jk/guess-repo-name-regression-fix (2015-08-05) 2 commits
 - clone: use computed length in guess_dir_name
 - clone: add tests for output directory
 (this branch is used by ps/guess-repo-name-at-root.)

 "git clone $URL" in recent releases of Git contains a regression in
 the code that invents a new repository name incorrectly based on
 the $URL.  This has been corrected.

 May need some tests from SQUASH??? on ps/guess-repo-name-at-root
 squashed into this, but otherwise looks good.


* ps/guess-repo-name-at-root (2015-08-05) 4 commits
 - SQUASH???
 - clone: abort if no dir name could be guessed
 - clone: do not use port number as dir name
 - clone: do not include authentication data in guessed dir
 (this branch uses jk/guess-repo-name-regression-fix.)

 "git clone $URL", when cloning from a site whose sole purpose is to
 host a single repository (hence, no path after <scheme>://<site>/),
 tried to used the site name as the new repository name, but did
 not remove username or password when <site> part was of the form
 <user>@<pass>:<host>.  The code is taught to redact these.


* jk/notes-merge-config (2015-08-05) 4 commits
 - SQUASH???
 - notes: add notes.merge option to select default strategy
 - notes: add tests for --commit/--abort/--strategy exclusivity
 - notes: document cat_sort_uniq rewriteMode

 "git notes merge" can be told with "--strategy=<how>" option how to
 automatically handle conflicts; this can now be configured by
 setting notes.merge configuration variable.

 The last step to add more specific notes.$ref.merge looked
 questionable.

 Waiting for reroll.


* mk/submodule-gitdir-path (2015-08-05) 2 commits
 - path: implement common_dir handling in git_path_submodule()
 - submodule refactor: use git_path_submodule() in add_submodule_odb()

 The submodule code has been taught to work better with separate
 work trees created via "git worktree add".


* mm/pull-upload-pack (2015-07-30) 1 commit
 - pull.sh: quote $upload_pack when passing it to git-fetch

 "git pull" in recent releases of Git has a regression in the code
 that allows custom path to the --upload-pack=<program>.  This has
 been corrected.

 Will merge to 'maint'.

 This should have already become irrelevant in 'master' with "git
 pull" getting rewritten in C.


* ps/t1509-chroot-test-fixup (2015-08-05) 2 commits
 - tests: fix cleanup after tests in t1509-root-worktree
 - tests: fix broken && chains in t1509-root-worktree

 t1509 test that requires a dedicated VM environment had some
 bitrot, which has been corrected.

 Will merge to 'next'.


* pt/am-builtin-options (2015-08-05) 4 commits
 - am: let --signoff override --no-signoff
 - am: let command-line options override saved options
 - squash! test_terminal: redirect child process' stdin to a pty
 - test_terminal: redirect child process' stdin to a pty
 (this branch uses pt/am-builtin.)

 After "git am --opt1" stops, running "git am --opt2" pays attention
 to "--opt2" only for the patch that caused the original invocation
 to stop.


* sb/remove-get-pathspec (2015-08-03) 1 commit
 - builtin/mv: remove get_pathspec()

 Expecting a reroll ($gmane/275224).


* sb/submodule-helper (2015-08-05) 1 commit
 - submodule: implement `module_list` as a builtin helper

 The beginning of "git submodule" rewritten in C.


* tb/complete-rebase-i-edit-todo (2015-08-05) 1 commit
 - completion: offer '--edit-todo' during interactive rebase

 Comments?


* jk/negative-hiderefs (2015-07-28) 2 commits
 - refs: support negative transfer.hideRefs
 - docs/config.txt: reorder hideRefs config

 Allow negative !ref entry in multi-value transfer.hideRefs
 configuration to say "don't hide this one".

 An update to test coming?

Re: What's cooking in git.git

[David Turner]

On Wed, 2015-08-05 at 15:55 -0700, Junio C Hamano wrote:
> * dt/untracked-subdir (2015-08-05) 2 commits
>  - DONTMERGE: Waiting for an Ack from Duy
>  - untracked-cache: fix subdirectory handling
>  (this branch uses dt/untracked-sparse.)
> 
>  This seems to break some tests.

All tests pass for me locally.  What's broken for you?

Re: What's cooking in git.git

[Junio C Hamano]

David Turner <dturner@twopensource.com> writes:

> On Wed, 2015-08-05 at 15:55 -0700, Junio C Hamano wrote:
>> * dt/untracked-subdir (2015-08-05) 2 commits
>>  - DONTMERGE: Waiting for an Ack from Duy
>>  - untracked-cache: fix subdirectory handling
>>  (this branch uses dt/untracked-sparse.)
>> 
>>  This seems to break some tests.
>
> All tests pass for me locally.  What's broken for you?


*** prove ***
t7063-status-untracked-cache.sh .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 3/30 subtests

Test Summary Report
-------------------
t7063-status-untracked-cache.sh (Wstat: 256 Tests: 30 Failed: 3)
  Failed tests:  28-30
  Non-zero exit status: 1
Files=1, Tests=30, 27 wallclock secs ( 0.04 usr  0.01 sys +  0.15 cusr  0.67 csys =  0.87 CPU)
Result: FAIL

$ sh t7063-status-untracked-cache -i -v

ends like so:

...
node creation: 2
gitignore invalidation: 0
directory invalidation: 1
opendir: 3
EOF
        test_cmp ../trace.expect ../trace

strace: invalid option -- 'k'
usage: strace [-CdffhiqrtttTvVxxy] [-I n] [-e expr]...
              [-a column] [-o file] [-s strsize] [-P path]...
              -p pid... / [-D] [-E var=val]... [-u username] PROG [ARGS]
   or: strace -c[df] [-I n] [-e expr]... [-O overhead] [-S sortby]
              -p pid... / [-D] [-E var=val]... [-u username] PROG [ARGS]
-c -- count time, calls, and errors for each syscall and report summary
-C -- like -c but also print regular output
-d -- enable debug output to stderr
-D -- run tracer process as a detached grandchild, not as parent
-f -- follow forks, -ff -- with output into separate files
-i -- print instruction pointer at time of syscall
-q -- suppress messages about attaching, detaching, etc.
-r -- print relative timestamp, -t -- absolute timestamp, -tt -- with usecs
-T -- print time spent in each syscall
-v -- verbose mode: print unabbreviated argv, stat, termios, etc. args
-x -- print non-ascii strings in hex, -xx -- print all strings in hex
-y -- print paths associated with file descriptor arguments
-h -- print help message, -V -- print version
-a column -- alignment COLUMN for printing syscall results (default 40)
-b execve -- detach on this syscall
-e expr -- a qualifying expression: option=[!]all or option=[!]val1[,val2]...
   options: trace, abbrev, verbose, raw, signal, read, write
-I interruptible --
   1: no signals are blocked
   2: fatal signals are blocked while decoding syscall (default)
   3: fatal signals are always blocked (default if '-o FILE PROG')
   4: fatal signals and SIGTSTP (^Z) are always blocked
      (useful to make 'strace -o FILE PROG' not stop on ^Z)
-o file -- send trace output to FILE instead of stderr
-O overhead -- set overhead for tracing syscalls to OVERHEAD usecs
-p pid -- trace process with process id PID, may be repeated
-s strsize -- limit length of print strings to STRSIZE chars (default 32)
-S sortby -- sort syscall counts by: time, calls, name, nothing (default time)
-u username -- run command as username handling setuid and/or setgid
-E var=val -- put var=val in the environment for command
-E var -- remove var from the environment for command
-P path -- trace accesses to path
not ok 28 - test sparse status with untracked cache and subdir

Re: What's cooking in git.git

[Jeff King]

On Wed, Aug 05, 2015 at 03:55:23PM -0700, Junio C Hamano wrote:

> * jk/negative-hiderefs (2015-07-28) 2 commits
>  - refs: support negative transfer.hideRefs
>  - docs/config.txt: reorder hideRefs config
> 
>  Allow negative !ref entry in multi-value transfer.hideRefs
>  configuration to say "don't hide this one".
> 
>  An update to test coming?

Thanks for reminding me. I think we just want to squash this in to the
tip commit:

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index afde495..7756100 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -150,7 +150,8 @@ do
 		git config --add $configsection.hiderefs "!refs/tags/magic" &&
 		git config --add $configsection.hiderefs refs/tags/magic/one &&
 		git ls-remote . >actual &&
-		verbose grep refs/tags/magic/two actual
+		verbose grep refs/tags/magic/two actual &&
+		! grep refs/tags/magic/one actual
 	'
 
 done

-Peff

PS I don't recall the outcome of our last discussion on the "verbose"
   test function. Here it makes debug output for the "grep" above more
   readable when it fails. But it also looks weird not to have the
   matching negative one for the final grep (which could be
   test_must_fail in this case, but we do not usually apply that to
   non-git commands). If you would prefer to strip out the "verbose"
   (from here and the test just below) while squashing, I am OK with
   that.

[PATCH 0/2] ./t5512-*.sh -x complaints

[Jeff King]

On Thu, Aug 06, 2015 at 12:55:35AM -0400, Jeff King wrote:

> PS I don't recall the outcome of our last discussion on the "verbose"
>    test function. Here it makes debug output for the "grep" above more
>    readable when it fails. But it also looks weird not to have the
>    matching negative one for the final grep (which could be
>    test_must_fail in this case, but we do not usually apply that to
>    non-git commands). If you would prefer to strip out the "verbose"
>    (from here and the test just below) while squashing, I am OK with
>    that.

Here's a squashable patch for that, in case it is easier (on top of the
previous squash; I am happy to just send a re-rolled patch if you'd
prefer):

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 7756100..aadaac5 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -150,7 +150,7 @@ do
 		git config --add $configsection.hiderefs "!refs/tags/magic" &&
 		git config --add $configsection.hiderefs refs/tags/magic/one &&
 		git ls-remote . >actual &&
-		verbose grep refs/tags/magic/two actual &&
+		grep refs/tags/magic/two actual &&
 		! grep refs/tags/magic/one actual
 	'
 
@@ -160,7 +160,7 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs'
 	test_config uploadpack.hiderefs refs/tags &&
 	test_config transfer.hiderefs "!refs/tags/magic" &&
 	git ls-remote . >actual &&
-	verbose grep refs/tags/magic actual
+	grep refs/tags/magic actual
 '
 
 test_done

I think the last discussion did end up with "eh, now that we have -x it
is simpler to just use that". Of course, then I tried to _use_ -x and
found some small niggles.  So here are fixes for those:

  [1/2]: test-lib: turn off "-x" tracing during chain-lint check
  [2/2]: test-lib: disable trace when test is not verbose

-Peff

[PATCH 1/2] test-lib: turn off "-x" tracing during chain-lint check

[Jeff King]

Now that GIT_TEST_CHAIN_LINT is on by default, running:

    ./t0000-basic.sh -x --verbose-only=1

starts with:

    expecting success:
            find .git/objects -type f -print >should-be-empty &&
            test_line_count = 0 should-be-empty

    + exit 117
    error: last command exited with $?=117
    + find .git/objects -type f -print
    + test_line_count = 0 should-be-empty
    + test 3 != 3
    + wc -l
    + test 0 = 0
    ok 1 - .git/objects should be empty after git init in an empty repo

This is confusing, as the "exit 117" line and the error line
(which is printed in red, no less!) are not part of the test
at all, but are rather in the separate chain-lint test_eval.
Let's unset the "trace" variable when eval-ing the chain
lint check, which avoids this.

Note that we cannot just do a one-shot variable like:

    trace= test_eval ...

as the behavior of one-shot variables for function calls
is not portable.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/test-lib.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index cea6cda..374bfcb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -571,12 +571,17 @@ test_run_ () {
 	expecting_failure=$2
 
 	if test "${GIT_TEST_CHAIN_LINT:-1}" != 0; then
+		# turn off tracing for this test-eval, as it simply creates
+		# confusing noise in the "-x" output
+		trace_tmp=$trace
+		trace=
 		# 117 is magic because it is unlikely to match the exit
 		# code of other programs
 		test_eval_ "(exit 117) && $1"
 		if test "$?" != 117; then
 			error "bug in the test script: broken &&-chain: $1"
 		fi
+		trace=$trace_tmp
 	fi
 
 	setup_malloc_check
-- 
2.5.0.148.g63828c1

[PATCH 2/2] test-lib: disable trace when test is not verbose

[Jeff King]

The "-x" test-script option turns on the shell's "-x"
tracing, which can help show why a particular test is
failing. Unfortunately, this can create false negatives in
some tests if they invoke a shell function with its stderr
redirected. t5512.10 is such a test, as it does:

    test_must_fail git ls-remote refs*master >actual 2>&1 &&
    test_cmp exp actual

The "actual" file gets the "-x" trace for the test_must_fail
function, which prevents it from matching the expected
output.

There's no way to avoid this without managing the
trace flag inside each sub-function, which isn't really a
workable solution. But unless you specifically care about
t5512.10, we can work around it by enabling tracing only for
the specific tests we want.

You can already do:

    ./t5512-ls-remote.sh -x --verbose-only=16

to see the trace only for a specific test. But that doesn't
_disable_ the tracing in the other tests; it just sends it
to /dev/null. However, there's no point in generating a
trace that the user won't see, so we can simply disable
tracing whenever it doesn't have a matching verbose flag.

The normal case of just "./t5512-ls-remote.sh -x" stays the
same, as "-x" already implies "--verbose" (and
"--verbose-only" overrides "--verbose", which is why this
works at all). And for our test, we need only check
$verbose, as maybe_setup_verbose will have already
set that flag based on the $verbose_only list).

Signed-off-by: Jeff King <peff@peff.net>
---
 t/test-lib.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 374bfcb..16c4d7b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -531,6 +531,10 @@ maybe_setup_valgrind () {
 	fi
 }
 
+want_trace () {
+	test "$trace" = t && test "$verbose" = t
+}
+
 # This is a separate function because some tests use
 # "return" to end a test_expect_success block early
 # (and we want to make sure we run any cleanup like
@@ -538,7 +542,7 @@ maybe_setup_valgrind () {
 test_eval_inner_ () {
 	# Do not add anything extra (including LF) after '$*'
 	eval "
-		test \"$trace\" = t && set -x
+		want_trace && set -x
 		$*"
 }
 
@@ -554,7 +558,7 @@ test_eval_ () {
 	{
 		test_eval_inner_ "$@" </dev/null >&3 2>&4
 		test_eval_ret_=$?
-		if test "$trace" = t
+		if want_trace
 		then
 			set +x
 			if test "$test_eval_ret_" != 0
-- 
2.5.0.148.g63828c1

Re: What's cooking in git.git

[Johannes Schindelin]

Hi Junio,

On 2015-08-06 00:55, Junio C Hamano wrote:

> * sb/submodule-helper (2015-08-05) 1 commit
>  - submodule: implement `module_list` as a builtin helper
> 
>  The beginning of "git submodule" rewritten in C.

I am really looking forward to that, with my Windows performance hat firmly on my head.

> * tb/complete-rebase-i-edit-todo (2015-08-05) 1 commit
>  - completion: offer '--edit-todo' during interactive rebase
> 
>  Comments?

The problem fixed by this one has bugged me plenty of times. I often need to edit the TODO e.g. when I managed to get the reordering wrong and want to insert a `pick <commit>` later in the `git-rebase-todo` and then call `git rebase --skip`.

Read: I am very much in favor of fast-tracking this topic.

Thanks,
Dscho

Re: What's cooking in git.git

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> David Turner <dturner@twopensource.com> writes:
>
>> On Wed, 2015-08-05 at 15:55 -0700, Junio C Hamano wrote:
>>> * dt/untracked-subdir (2015-08-05) 2 commits
>>>  - DONTMERGE: Waiting for an Ack from Duy
>>>  - untracked-cache: fix subdirectory handling
>>>  (this branch uses dt/untracked-sparse.)
>>> 
>>>  This seems to break some tests.
>>
>> All tests pass for me locally.  What's broken for you?
>
> *** prove ***
> t7063-status-untracked-cache.sh .. Dubious, test returned 1 (wstat 256, 0x100)
> Failed 3/30 subtests
>
> Test Summary Report
> -------------------
> t7063-status-untracked-cache.sh (Wstat: 256 Tests: 30 Failed: 3)
>   Failed tests:  28-30
>   Non-zero exit status: 1
> Files=1, Tests=30, 27 wallclock secs ( 0.04 usr  0.01 sys +  0.15 cusr  0.67 csys =  0.87 CPU)
> Result: FAIL

So it turns out that you run "strace -k -o /tmp/tr" and never look
at /tmp/tr; I am guessing that this was a remnant of your debugging
while developing the test, sign of lack of proofreading before
sending the patch out, perhaps?

I didn't carefully read the remainder of the patch, but at the
minimum this needs to be squashed in.  They may be a reroll with
some more tweaks from you after other people comment on it, so I'll
keep this separate and squashable.

Thanks.

 t/t7063-status-untracked-cache.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index bb0e2a9..ca8dc3a 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -486,7 +486,7 @@ test_expect_success 'test sparse status with untracked cache and subdir' '
 	avoid_racy &&
 	: >../trace &&
 	GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \
-	strace -k -o /tmp/st git status --porcelain >../status.actual &&
+	git status --porcelain >../status.actual &&
 	cat >../status.expect <<EOF &&
  M done/two
 ?? .gitignore

Re: What's cooking in git.git

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> PS I don't recall the outcome of our last discussion on the "verbose"
>    test function. Here it makes debug output for the "grep" above more
>    readable when it fails. But it also looks weird not to have the
>    matching negative one for the final grep (which could be
>    test_must_fail in this case, but we do not usually apply that to
>    non-git commands). If you would prefer to strip out the "verbose"
>    (from here and the test just below) while squashing, I am OK with
>    that.

Yeah, let's do that then.