What's cooking in git.git (Feb 2009, #06; Wed, 18)

Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the branches, but I am still holding onto them.

The topics list the commits in reverse chronological order. The topics meant to be merged to the maintenance series have "maint-" in their names.

The master branch is slightly past 1.6.2-rc1.

As an experiment, 'next' and 'pu' stay open during this release freeze; new topics are accepted as time permits, but people are much more strongly encouraged to find issues on what is already on 'master', and patches to fix them always takes precedence over what is still on 'next' and will never graduate until 1.6.2 final.

----------------------------------------------------------------
[New Topics]

* fc/parseopt-config (Tue Feb 17 15:52:56 2009 +0200) 11 commits . config: Don't allow extra arguments for -e or -l
. config: Disallow multiple variable types
. config: Disallow multiple config file locations
. config: Disallow --getcolor* and other actions used together . config: Use parseopt
. config: Reorganize get_color*
. config: Trivial rename in preparation for parseopt . ??? config: Make git_config() more flexible
. config: Cleanup editor action
. config: style clean-ups
- config: Add new option to open an editor.

Builds on top of "config -e" change.

* mv/parseopt-ls-files (Tue Feb 17 15:27:11 2009 +0100) 2 commits
- parse-opt: migrate builtin-ls-files.
- Turn the flags in struct dir_struct into a single variable

----------------------------------------------------------------
[Stalled and may need help and prodding to go forward]

* ns/stash-keep (Thu Feb 12 06:25:14 2009 +0900) 1 commit
- stash: --keep option just saves

Do we want to keep this one?

* lh/submodule-tree-traversal (Sun Jan 25 01:52:06 2009 +0100) 1 commit
- archive.c: add support for --submodules[=(all|checkedout)]

Discussion stalled on the submodule selection criteria.

* jc/blame (Wed Jun 4 22:58:40 2008 -0700) 2 commits
+ blame: show "previous" information in --porcelain/--incremental format
+ git-blame: refactor code to emit "porcelain format" output

This gives Porcelains (like gitweb) the information on the commit _before_ the one that the final blame is laid on, which should save them one rev-parse to dig further. The line number in the "previous" information may need refining, and sanity checking code for reference counting may need to be resurrected before this can move forward.

I thought recent tig discussion may blow new life into it, but is this unneeded? If so I'd rather revert it (or discard after 1.6.2).

* db/foreign-scm (Sun Jan 11 15:12:10 2009 -0500) 3 commits
- Support fetching from foreign VCSes
- Add specification of git-vcs helpers
- Add "vcs" config option in remotes

The "spec" did not seem quite well cooked yet, but in the longer term I think something like this to allow interoperating with other SCMs as if the other end is a native git repository is a very worthy goal.

* cc/replace (Mon Feb 2 06:13:06 2009 +0100) 11 commits
- builtin-replace: use "usage_msg_opt" to give better error messages
- parse-options: add new function "usage_msg_opt"
- builtin-replace: teach "git replace" to actually replace
- Add new "git replace" command
- environment: add global variable to disable replacement
- mktag: call "check_sha1_signature" with the replacement sha1
- replace_object: add a test case
- object: call "check_sha1_signature" with the replacement sha1
- sha1_file: add a "read_sha1_file_repl" function
- replace_object: add mechanism to replace objects found in "refs/replace/"
- refs: add a "for_each_replace_ref" function

I think the code is much cleaner than the first round, but I am not convinced it is doing the right thing in the connectivity traverser. Independent review sorely needed.

* sc/gitweb-category (Fri Dec 12 00:45:12 2008 +0100) 3 commits
- gitweb: Optional grouping of projects by category
- gitweb: Split git_project_list_body in two functions
- gitweb: Modularized git_get_project_description to be more generic

Design discussion between Jakub and Sebastien seems to have stalled, but Jakub seems to be taking this over.

* jc/fsck (Fri Jan 30 02:33:47 2009 -0800) 4 commits
- fsck: three levels of validation
- verify-pack: add --quick
- verify_pack(): allow a quicker verification for a pack with version 2 idx
- pack-check.c: minor formatting fix to match coding style

J6t has a good point that if this had any value then medium level should replace the default. I am tempted to actually dropping this as a failed experiment.

----------------------------------------------------------------
[Reverted]

* mh/unify-color (Fri Jan 23 01:25:23 2009 -0800) 3 commits ? Revert previous two commits
? move the color variables to color.c
? handle color.ui at a central place

This broke git-format-patch badly.

* js/rebase-error-a-bit-more-verbose (Sun Feb 8 21:22:18 2009 -0800) 2 commits ? Revert "rebase: explain why when the HEAD could not be detached" ? rebase: explain why when the HEAD could not be detached

This turned out to be unnecessary.

* rs/maint-1.6.0-windows-ceiling (Sat Feb 7 12:40:40 2009 -0800) 2 commits ? Revert "fix t1504 on Windows"
? fix t1504 on Windows

I'm giving a fresh start to J6t's series which contains this.

* lh/reverted-submodule-tree-traversal (Sun Jan 25 18:39:55 2009 -0800) 4 commits ? Revert round #1 of the series
? builtin-ls-tree: enable traversal of submodules
? archive.c: enable traversal of submodules
? tree.c: add support for traversal of submodules

I'm giving a fresh start to Lars's second iteration.

----------------------------------------------------------------
[Will merge to 'next' soon]

* tr/gcov (Sun Feb 15 23:25:45 2009 +0100) 8 commits
- Test git-patch-id
- Test rev-list --parents/--children
- Test log --decorate
- Test fsck a bit harder
- Test log --graph
- Test diff --dirstat functionality
- Test that diff can read from stdin
- Support coverage testing with GCC/gcov

----------------------------------------------------------------
[Ready for 'master' after 1.6.2]

* jw/imap-preformatted-html (Thu Feb 12 08:58:12 2009 -0600) 1 commit
+ imap.preformattedHTML to tell Thunderbird to send non-flowed text

The patch text should be identical to Jeremy's "Virtual Patch", except that the configuration variable was renamed per list discussion.

* jw/format-patch-attach (Thu Feb 12 09:51:55 2009 -0600) 1 commit
+ Enable setting attach as the default in .gitconfig for git-format- patch.

* sr/force-rebase (Fri Feb 13 23:48:01 2009 +0100) 1 commit
+ Teach rebase to rebase even if upstream is up to date

* fg/exclude-bq (Tue Feb 10 15:20:17 2009 +0100) 1 commit
+ Support "\" in non-wildcard exclusion entries

* dm/add-i-edit-abort (Thu Feb 12 00:19:41 2009 -0500) 1 commit
+ add -i: revisit hunk on editor failure

* tp/completion (Wed Feb 11 13:03:26 2009 -0500) 4 commits
+ completion: More fixes to prevent unbound variable errors
+ completion: Better __git_ps1 support when not in working directory
+ completion: Use consistent if [...] convention, not "test"
+ completion: For consistency, change "git rev-parse" to __gitdir calls

* js/branch-symref (Wed Feb 18 19:14:59 2009 +0100) 2 commits
+ Avoid segfault with 'git branch' when the HEAD is detached
+ builtin-branch: improve output when displaying remote branches

* al/ansi-color (Fri Feb 13 22:53:41 2009 +0100) 2 commits
+ builtin-branch.c: Rename branch category color names
+ Clean up use of ANSI color sequences

* kb/checkout-optim (Wed Feb 18 23:18:03 2009 +0100) 10 commits
+ check_updates(): effective removal of cache entries marked CE_REMOVE
+ lstat_cache(): print a warning if doing ping-pong between cache types
+ show_patch_diff(): remove a call to fstat()
+ write_entry(): use fstat() instead of lstat() when file is open
+ write_entry(): cleanup of some duplicated code
+ create_directories(): remove some memcpy() and strchr() calls
+ unlink_entry(): introduce schedule_dir_for_removal()
+ lstat_cache(): swap func(length, string) into func(string, length)
+ lstat_cache(): generalise longest_match_lstat_cache()
+ lstat_cache(): small cleanup and optimisation

* js/valgrind (Thu Feb 5 22:03:00 2009 +0100) 9 commits
+ valgrind: do not require valgrind 3.4.0 or newer
+ test-lib: avoid assuming that templates/ are in the GIT_EXEC_PATH
+ Tests: let --valgrind imply --verbose and --tee
+ Add a script to coalesce the valgrind outputs
+ t/Makefile: provide a 'valgrind' target
+ test-lib.sh: optionally output to test-results/$TEST.out, too
+ Valgrind support: check for more than just programming errors
+ valgrind: ignore ldso and more libz errors
+ Add valgrind support in test scripts

----------------------------------------------------------------
[Actively cooking]

* jc/add-p-unquote (Mon Feb 16 22:43:43 2009 -0800) 1 commit
+ git-add -i/-p: learn to unwrap C-quoted paths

* js/send-email (Sat Feb 14 23:32:15 2009 -0500) 3 commits
+ send-email: --suppress-cc improvements
+ send-email: handle multiple Cc addresses when reading mbox message
+ send-email: allow send-email to run outside a repo

* sg/rerere-cleanup (Sat Feb 14 23:21:04 2009 +0100) 1 commit
+ rerere: remove duplicated functions

* js/notes (Wed Feb 18 11:17:27 2009 -0800) 14 commits
+ tests: fix "export var=val"
+ notes: refuse to edit notes outside refs/notes/
+ t3301: use test_must_fail instead of !
+ t3301: fix confusing quoting in test for valid notes ref
+ notes: use GIT_EDITOR and core.editor over VISUAL/EDITOR
+ notes: only clean up message file when editing
+ handle empty notes gracefully
+ git notes show: test empty notes
+ git-notes: fix printing of multi-line notes
+ notes: fix core.notesRef documentation
+ Add an expensive test for git-notes
+ Speed up git notes lookup
+ Add a script to edit/inspect notes
+ Introduce commit notes

Earlier part was merged to master and then reverted there. I probably should rebase this after 1.6.2 to keep my sanity.

* fc/config-editor (Sat Feb 7 23:53:00 2009 +0200) 1 commit
- config: Add new option to open an editor.

* js/remote-set-head (Sat Feb 14 05:30:30 2009 -0500) 5 commits
- builtin-remote: better handling of multiple remote HEADs
- builtin-remote: add set-head subcommand
- builtin-remote: teach show to display remote HEAD
- builtin-remote: move duplicated cleanup code its own function
- builtin-clone: move locate_head() to remote.c so it can be re-used

* jk/head-lookup (Sun Feb 15 01:18:18 2009 -0500) 5 commits
- remote: use exact HEAD lookup if it is available
- remote: refactor guess_remote_head
- refactor find_refs_by_name to accept const list
- add basic http clone/fetch tests
- test scripts: refactor start_httpd helper

Builds on top of Jay Soffian's js/remote-set-head topic. How well do these two work in practice in the real world? Success stories?

----------------------------------------------------------------
[On Hold]

* jc/deny-delete-current-1.7.0 (Mon Feb 9 00:19:46 2009 -0800) 1 commit
- receive-pack: default receive.denyDeleteCurrent to refuse

* jc/refuse-push-to-current-1.7.0 (Wed Feb 11 02:28:03 2009 -0800) 1 commit
- Refuse updating the current branch in a non-bare repository via push

This is for 1.7.0.

* jc/commit-assume-also-during-merge (Thu Jan 22 22:21:49 2009 -0800) 3 commits
- git commit: pathspec without -i/-o implies -i semantics during a merge
- builtin-commit: shorten eye-sore overlong lines
- Add "partial commit" tests during a conflicted merge

This was only meant as a weatherballoon to help facilitate discussion.

* jc/merge-convert (Mon Jan 26 16:45:01 2009 -0800) 1 commit
- git-merge-file: allow converting the results for the work tree

This is a feature waiting for a user. I do not need it in 1.6.2

We did not give scripted Porcelains a way to say "this temporary file I am using for merging is for this path, so use the core.autocrlf and attributes rules for that final path". Instead, merge-file simply wrote out the data in the canonical repository representation.

rerere has the same issue, but it is a lot worse. It reads the three files (preimage, postimage and thisimage) from the work tree in the work tree representation, merges them without converting them to the canonical representation first but inserts the conflict markers with the canonical representation and writes the resulting mess out. It needs to be fixed to read with convert_to_git(), merge them while they are still in the canonical representation and possibly add conflict markers, and then write the results out after convert_to_working_tree(). It also needs to write in binary mode as well.

----------------------------------------------------------------
[Perhaps will discard]

* jk/renamelimit (Sat May 3 13:58:42 2008 -0700) 1 commit . diff: enable "too large a rename" warning when -M/-C is explicitly asked for

* jc/stripspace (Sun Mar 9 00:30:35 2008 -0800) 6 commits . git-am --forge: add Signed-off-by: line for the author . git-am: clean-up Signed-off-by: lines
. stripspace: add --log-clean option to clean up signed-off-by:
lines
. stripspace: use parse_options()
. Add "git am -s" test
. git-am: refactor code to add signed-off-by line for the committer

* jc/post-simplify (Fri Aug 15 01:34:51 2008 -0700) 2 commits . revision --simplify-merges: incremental simplification . revision --simplify-merges: prepare for incremental simplification

I noticed two small things that I'd like to fix before this goes 'next', so here they are:

* [1/8] Support coverage testing with GCC/gcov

Changed it so the compilation (but not the testing) uses the same -j flags as the caller, so that compilation can be done in parallel. (It's rather minor compared to the slow testing with optimizations turned off, but still.)

* [5/8] Test fsck a bit harder

Fixed an invocation of hash-objects that lacked --stdin. I also added some debugging cats to make it easier to fix the grep invocation when fsck actually starts printing errors for these corruptions.

Apart from that it's the same.

Thanks!

Thomas Rast (8):
Support coverage testing with GCC/gcov
Test that diff can read from stdin
Test diff --dirstat functionality
Test log --graph
Test fsck a bit harder
Test log --decorate
Test rev-list --parents/--children
Test git-patch-id

 Makefile                                      |   24 ++++
 t/t1450-fsck.sh                               |   67 +++++++++++
 t/t4002-diff-basic.sh                         |    8 ++
 t/t4013-diff-various.sh                       |    5 +
 t/t4013/diff.diff_--dirstat_master~1_master~2 |    3 +
 t/t4013/diff.log_--decorate_--all             |   34 ++++++
 t/t4013/diff.rev-list_--children_HEAD         |    7 +
 t/t4013/diff.rev-list_--parents_HEAD          |    7 +
 t/t4202-log.sh                                |  148 +++++++++++++++++++++++++
 t/t4203-patch-id.sh                           |   38 +++++++
 10 files changed, 341 insertions(+), 0 deletions(-)
 create mode 100644 t/t4013/diff.diff_--dirstat_master~1_master~2
 create mode 100644 t/t4013/diff.log_--decorate_--all
 create mode 100644 t/t4013/diff.rev-list_--children_HEAD
 create mode 100644 t/t4013/diff.rev-list_--parents_HEAD
 create mode 100755 t/t4203-patch-id.sh

With gcc's --coverage option, we can perform automatic coverage data collection for the test suite.

Add a new Makefile target 'coverage' that scraps all previous coverage results, recompiles git with the required compiler/linker flags (in addition to any flags you specify manually), then runs the test suite and compiles a report.

The compilation must be done with all optimizations disabled, since inlined functions (and for line-by-line coverage, also optimized branches/loops) break coverage tracking.

The tests are run serially (with -j1). The coverage code should theoretically allow concurrent access to its data files, but the author saw random test failures. Obviously this could be improved.

The report currently consists of a list of functions that were never executed during the tests, which is written to 'coverage-untested-functions'. Once this list becomes reasonably short, we would also want to look at branches that were never taken.

Currently only toplevel *.c files are considered. It would be nice to at least include xdiff, but --coverage did not save data to subdirectories on the system used to write this (gcc 4.3.2).

---

Let the 'all' sub-build of 'coverage' run with the caller's -j setting so that it can parallelize.

Interdiff:

--- a/Makefile
+++ b/Makefile
@@ -21,8 +21,9 @@
+COVERAGE_LDFLAGS = $(CFLAGS) -O0 -lgcov
+
+coverage-build: coverage-clean
++ $(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" all
+ $(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
-+ -j1 all test
++ -j1 test
+
+coverage-report:
+ gcov -b *.c

 Makefile |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

index b040a96..c32881b 100644
@@ -1640,3 +1640,27 @@ check-docs::
 check-builtins::
 	./check-builtins.sh
 
+### Test suite coverage testing
+#
+.PHONY: coverage coverage-clean coverage-build coverage-report
+
+coverage:
+	$(MAKE) coverage-build
+	$(MAKE) coverage-report
+
+coverage-clean:
+	rm -f *.gcda *.gcno
+
+COVERAGE_CFLAGS = $(CFLAGS) -O0 -ftest-coverage -fprofile-arcs
+COVERAGE_LDFLAGS = $(CFLAGS)  -O0 -lgcov
+
+coverage-build: coverage-clean
+	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" all
+	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
+		-j1 test
+
+coverage-report:
+	gcov -b *.c
+	grep '^function.*called 0 ' *.c.gcov \
+		| sed -e 's/\([^:]*\)\.gcov: *function \([^ ]*\) called.*/\1: \2/' \
+		| tee coverage-untested-functions

--
1.6.2.rc1.266.gce6c4

---

Same as v1.

 t/t4002-diff-basic.sh |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

index cc3681f..18695ce 100755
@@ -258,4 +258,12 @@ test_expect_success \
     git diff-tree -r -R $tree_A $tree_B >.test-b &&
     cmp -s .test-a .test-b'
 
+test_expect_success \
+    'diff can read from stdin' \
+    'test_must_fail git diff --no-index -- MN - < NN |
+        grep -v "^index" | sed "s#/-#/NN#" >.test-a &&
+    test_must_fail git diff --no-index -- MN NN |
+        grep -v "^index" >.test-b &&
+    test_cmp .test-a .test-b'
+
 test_done

1.6.2.rc1.266.gce6c4

This is only a very rudimentary test, but it was untested before.

---

Same as v1.

 t/t4013-diff-various.sh                       |    1 +
 t/t4013/diff.diff_--dirstat_master~1_master~2 |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)
 create mode 100644 t/t4013/diff.diff_--dirstat_master~1_master~2

index 9c70902..0289336 100755
@@ -268,6 +268,7 @@ diff --no-index --name-status dir2 dir
 diff --no-index --name-status -- dir2 dir
 diff --no-index dir dir3
 diff master master^ side
+diff --dirstat master~1 master~2
 EOF
 
 test_done

new file mode 100644
index 0000000..b672e1c
@@ -0,0 +1,3 @@
+$ git diff --dirstat master~1 master~2
+  40.0% dir/
+$

1.6.2.rc1.266.gce6c4

So far there were no tests checking that log --graph actually works.

Note that the tests strip trailing whitespace, as the current --graph emits trailing whitespace on lines that do not contain anything but graph lines.

---

Same as v1.

 t/t4202-log.sh |  148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 148 insertions(+), 0 deletions(-)

index 7b976ee..93966f7 100755
@@ -134,5 +134,153 @@ test_expect_success 'log --grep -i' '
 	test_cmp expect actual
 '
 
+cat > expect <<EOF
+* Second
+* sixth
+* fifth
+* fourth
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'simple log --graph' '
+	git log --graph --pretty=tformat:%s >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'set up merge history' '
+	git checkout -b side HEAD~4 &&
+	test_commit side-1 1 1 &&
+	test_commit side-2 2 2 &&
+	git checkout master &&
+	git merge side
+'
+
+cat > expect <<\EOF
+*   Merge branch 'side'
+|\
+| * side-2
+| * side-1
+* | Second
+* | sixth
+* | fifth
+* | fourth
+|/
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge' '
+	git log --graph --date-order --pretty=tformat:%s |
+		sed "s/ *$//" >actual &&
+	test_cmp expect actual
+'
+
+cat > expect <<\EOF
+*   commit master
+|\  Merge: A B
+| | Author: A U Thor <[email protected]>
+| |
+| |     Merge branch 'side'
+| |
+| * commit side
+| | Author: A U Thor <[email protected]>
+| |
+| |     side-2
+| |
+| * commit tags/side-1
+| | Author: A U Thor <[email protected]>
+| |
+| |     side-1
+| |
+* | commit master~1
+| | Author: A U Thor <[email protected]>
+| |
+| |     Second
+| |
+* | commit master~2
+| | Author: A U Thor <[email protected]>
+| |
+| |     sixth
+| |
+* | commit master~3
+| | Author: A U Thor <[email protected]>
+| |
+| |     fifth
+| |
+* | commit master~4
+|/  Author: A U Thor <[email protected]>
+|
+|       fourth
+|
+* commit tags/side-1~1
+| Author: A U Thor <[email protected]>
+|
+|     third
+|
+* commit tags/side-1~2
+| Author: A U Thor <[email protected]>
+|
+|     second
+|
+* commit tags/side-1~3
+  Author: A U Thor <[email protected]>
+
+      initial
+EOF
+
+test_expect_success 'log --graph with full output' '
+	git log --graph --date-order --pretty=short |
+		git name-rev --name-only --stdin |
+		sed "s/Merge:.*/Merge: A B/;s/ *$//" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'set up more tangled history' '
+	git checkout -b tangle HEAD~6 &&
+	test_commit tangle-a tangle-a a &&
+	git merge master~3 &&
+	git merge side~1 &&
+	git checkout master &&
+	git merge tangle
+'
+
+cat > expect <<\EOF
+*   Merge branch 'tangle'
+|\
+| *   Merge branch 'side' (early part) into tangle
+| |\
+| * \   Merge branch 'master' (early part) into tangle
+| |\ \
+| * | | tangle-a
+* | | |   Merge branch 'side'
+|\ \ \ \
+| * | | | side-2
+| | | |/
+| | |/|
+| |/| |
+| * | | side-1
+* | | | Second
+* | | | sixth
+| | |/
+| |/|
+|/| |
+* | | fifth
+* | | fourth
+|/ /
+* | third
+|/
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge' '
+	git log --graph --date-order --pretty=tformat:%s |
+		sed "s/ *$//" >actual &&
+	test_cmp expect actual
+'
+
 test_done
 

1.6.2.rc1.266.gce6c4

git-fsck, of all tools, has very few tests. This adds some more:
* a corrupted object;
* a branch pointing to a non-commit;
* a tag pointing to a nonexistent object;
* and a tag pointing to an object of a type other than what the tag itself claims.

Only the first two are caught. At least the third probably should, too, but currently slips through.

---

Fixed a missing --stdin when calling git-hash-object. I also added some debugging 'cat out' to make it easier to fix the 'grep' invocation when fsck actually starts printing errors for these corruptions.

--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
-@@ -28,4 +28,69 @@ test_expect_success 'loose objects borrowed from alternate are not missing' ' +@@ -28,4 +28,71 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '

          )
    '

@@ -79,12 +80,13 @@

   +      tag=$(git hash-object -w --stdin < invalid-tag) &&
   +      echo $tag > .git/refs/tags/invalid &&
   +      git fsck --tags 2>out &&
  ++      cat out &&
   +      grep "could not load tagged object" out &&
   +      rm .git/refs/tags/invalid
   +'
   +
   +cat > wrong-tag <<EOF
  -+object $(echo blob | git hash-object -w)
  ++object $(echo blob | git hash-object -w --stdin)
   +type commit
   +tag wrong
   +tagger T A Gger <[email protected]> 1234567890 -0000
  @@ -96,6 +98,7 @@
   +      tag=$(git hash-object -w --stdin < wrong-tag) &&
   +      echo $tag > .git/refs/tags/wrong &&
   +      git fsck --tags 2>out &&
  ++      cat out &&
   +      grep "some sane error message" out &&
   +      rm .git/refs/tags/wrong

+'

 t/t1450-fsck.sh |   67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 67 insertions(+), 0 deletions(-)

index 4597af0..a22632f 100755
@@ -28,4 +28,71 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '
 	)
 '
 
+# Corruption tests follow.  Make sure to remove all traces of the
+# specific corruption you test afterwards, lest a later test trip over
+# it.
+
+test_expect_success 'object with bad sha1' '
+	sha=$(echo blob | git hash-object -w --stdin) &&
+	echo $sha &&
+	old=$(echo $sha | sed "s+^..+&/+") &&
+	new=$(dirname $old)/ffffffffffffffffffffffffffffffffffffff &&
+	sha="$(dirname $new)$(basename $new)"
+	mv .git/objects/$old .git/objects/$new &&
+	git update-index --add --cacheinfo 100644 $sha foo &&
+	tree=$(git write-tree) &&
+	cmt=$(echo bogus | git commit-tree $tree) &&
+	git update-ref refs/heads/bogus $cmt &&
+	(git fsck 2>out; true) &&
+	grep "$sha.*corrupt" out &&
+	rm -f .git/objects/$new &&
+	git update-ref -d refs/heads/bogus &&
+	git read-tree -u --reset HEAD
+'
+
+test_expect_success 'branch pointing to non-commit' '
+	git rev-parse HEAD^{tree} > .git/refs/heads/invalid &&
+	git fsck 2>out &&
+	grep "not a commit" out &&
+	git update-ref -d refs/heads/invalid
+'
+
+cat > invalid-tag <<EOF
+object ffffffffffffffffffffffffffffffffffffffff
+type commit
+tag invalid
+tagger T A Gger <[email protected]> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_failure 'tag pointing to nonexistent' '
+	tag=$(git hash-object -w --stdin < invalid-tag) &&
+	echo $tag > .git/refs/tags/invalid &&
+	git fsck --tags 2>out &&
+	cat out &&
+	grep "could not load tagged object" out &&
+	rm .git/refs/tags/invalid
+'
+
+cat > wrong-tag <<EOF
+object $(echo blob | git hash-object -w --stdin)
+type commit
+tag wrong
+tagger T A Gger <[email protected]> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_failure 'tag pointing to something else than its type' '
+	tag=$(git hash-object -w --stdin < wrong-tag) &&
+	echo $tag > .git/refs/tags/wrong &&
+	git fsck --tags 2>out &&
+	cat out &&
+	grep "some sane error message" out &&
+	rm .git/refs/tags/wrong
+'
+
+
+
 test_done

1.6.2.rc1.266.gce6c4

git-fsck, of all tools, has very few tests. This adds some more:
* a corrupted object;
* a branch pointing to a non-commit;
* a tag pointing to a nonexistent object;
* and a tag pointing to an object of a type other than what the tag itself claims.

---

I investigated the test failures with v2, and it turns out the problem was in my use of hash-object. When correctly passing '-t tag' for the tags, fsck does detect the breakage. So here's an updated version of this test. Interdiff:

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 628db19..a22632f 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -66,13 +66,13 @@ tagger T A Gger <[email protected]> 1234567890 -0000
This is an invalid tag.
EOF

-test_expect_success 'tag pointing to nonexistent' '
- tag=$(git hash-object -t tag -w --stdin < invalid-tag) &&
+test_expect_failure 'tag pointing to nonexistent' '
+ tag=$(git hash-object -w --stdin < invalid-tag) &&

          echo $tag > .git/refs/tags/invalid &&
  -       git fsck 2>&1 | tee out &&
  -       grep "missing commit ffffffffffffffffffffffffffffffffffffffff" out &&
  -       rm .git/refs/tags/invalid &&
  -       rm -f .git/objects/$(echo $tag | sed "s#^..#&/#")
  +       git fsck --tags 2>out &&
  +       cat out &&
  +       grep "could not load tagged object" out &&
  +       rm .git/refs/tags/invalid

'

cat > wrong-tag <<EOF
@@ -84,11 +84,12 @@ tagger T A Gger <[email protected]> 1234567890 -0000
This is an invalid tag.
EOF

-test_expect_success 'tag pointing to something else than its type' '
- tag=$(git hash-object -t tag -w --stdin < wrong-tag) &&
+test_expect_failure 'tag pointing to something else than its type' '
+ tag=$(git hash-object -w --stdin < wrong-tag) &&

          echo $tag > .git/refs/tags/wrong &&
  -       git fsck 2>&1 | tee out &&
  -       grep "Object.*is a blob, not a commit" out &&
  +       git fsck --tags 2>out &&
  +       cat out &&
  +       grep "some sane error message" out &&

rm .git/refs/tags/wrong
'

 t/t1450-fsck.sh |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 66 insertions(+), 0 deletions(-)

index 4597af0..628db19 100755
@@ -28,4 +28,70 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '
 	)
 '
 
+# Corruption tests follow.  Make sure to remove all traces of the
+# specific corruption you test afterwards, lest a later test trip over
+# it.
+
+test_expect_success 'object with bad sha1' '
+	sha=$(echo blob | git hash-object -w --stdin) &&
+	echo $sha &&
+	old=$(echo $sha | sed "s+^..+&/+") &&
+	new=$(dirname $old)/ffffffffffffffffffffffffffffffffffffff &&
+	sha="$(dirname $new)$(basename $new)"
+	mv .git/objects/$old .git/objects/$new &&
+	git update-index --add --cacheinfo 100644 $sha foo &&
+	tree=$(git write-tree) &&
+	cmt=$(echo bogus | git commit-tree $tree) &&
+	git update-ref refs/heads/bogus $cmt &&
+	(git fsck 2>out; true) &&
+	grep "$sha.*corrupt" out &&
+	rm -f .git/objects/$new &&
+	git update-ref -d refs/heads/bogus &&
+	git read-tree -u --reset HEAD
+'
+
+test_expect_success 'branch pointing to non-commit' '
+	git rev-parse HEAD^{tree} > .git/refs/heads/invalid &&
+	git fsck 2>out &&
+	grep "not a commit" out &&
+	git update-ref -d refs/heads/invalid
+'
+
+cat > invalid-tag <<EOF
+object ffffffffffffffffffffffffffffffffffffffff
+type commit
+tag invalid
+tagger T A Gger <[email protected]> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_success 'tag pointing to nonexistent' '
+	tag=$(git hash-object -t tag -w --stdin < invalid-tag) &&
+	echo $tag > .git/refs/tags/invalid &&
+	git fsck 2>&1 | tee out &&
+	grep "missing commit ffffffffffffffffffffffffffffffffffffffff" out &&
+	rm .git/refs/tags/invalid &&
+	rm -f .git/objects/$(echo $tag | sed "s#^..#&/#")
+'
+
+cat > wrong-tag <<EOF
+object $(echo blob | git hash-object -w --stdin)
+type commit
+tag wrong
+tagger T A Gger <[email protected]> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_success 'tag pointing to something else than its type' '
+	tag=$(git hash-object -t tag -w --stdin < wrong-tag) &&
+	echo $tag > .git/refs/tags/wrong &&
+	git fsck 2>&1 | tee out &&
+	grep "Object.*is a blob, not a commit" out &&
+	rm .git/refs/tags/wrong
+'
+
+
+
 test_done

1.6.2.rc1.310.g364d6

	new=${old%/*}/ffffffffffffffffffffffffffffffffffffff &&

	sha=${new%/*}${new##*/} &&

	test_must_fail git fsck 2>out &&

	test_must_fail git fsck > out 2>&1 &&

...

	test_must_fail git fsck > out 2>&1 &&

git-fsck, of all tools, has very few tests. This adds some more:
* a corrupted object;
* a branch pointing to a non-commit;
* a tag pointing to a nonexistent object;
* and a tag pointing to an object of a type other than what the tag itself claims.

Some of the involved shell programming is due to Johannes Sixt.

---

I thought I had seen instances where it notices something wrong, but still exits with success.

I checked again, and apparently I was dreaming. I changed the tests to use

test_must_fail git fsck 2>&1 | tee out

instead, which both checks the exit status and makes the tests more verbose with -v.

Indeed. I have a tendency to try and wrap as much as possible into the tests so that it's visible with -v, but here I clearly overshot.

I also used all your shell programming suggestions, thanks a lot.

 t/t1450-fsck.sh |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 69 insertions(+), 0 deletions(-)

index 4597af0..0906b1d 100755
@@ -28,4 +28,73 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '
 	)
 '
 
+# Corruption tests follow.  Make sure to remove all traces of the
+# specific corruption you test afterwards, lest a later test trip over
+# it.
+
+test_expect_success 'object with bad sha1' '
+	sha=$(echo blob | git hash-object -w --stdin) &&
+	echo $sha &&
+	old=$(echo $sha | sed "s+^..+&/+") &&
+	new=${old%/*}/ffffffffffffffffffffffffffffffffffffff &&
+	sha=${new%/*}${new##*/} &&
+	mv .git/objects/$old .git/objects/$new &&
+	git update-index --add --cacheinfo 100644 $sha foo &&
+	tree=$(git write-tree) &&
+	cmt=$(echo bogus | git commit-tree $tree) &&
+	git update-ref refs/heads/bogus $cmt &&
+	test_must_fail git fsck 2>&1 | tee out &&
+	grep "$sha.*corrupt" out
+'
+
+rm -f .git/objects/$new
+git update-ref -d refs/heads/bogus
+git read-tree -u --reset HEAD
+
+test_expect_success 'branch pointing to non-commit' '
+	git rev-parse HEAD^{tree} > .git/refs/heads/invalid &&
+	git fsck 2>&1 | tee out &&
+	grep "not a commit" out
+'
+
+git update-ref -d refs/heads/invalid
+
+cat > invalid-tag <<EOF
+object ffffffffffffffffffffffffffffffffffffffff
+type commit
+tag invalid
+tagger T A Gger <[email protected]> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_success 'tag pointing to nonexistent' '
+	tag=$(git hash-object -t tag -w --stdin < invalid-tag) &&
+	echo $tag > .git/refs/tags/invalid &&
+	test_must_fail git fsck 2>&1 | tee out &&
+	grep "missing commit ffffffffffffffffffffffffffffffffffffffff" out
+'
+
+rm .git/refs/tags/invalid
+rm -f .git/objects/$(echo $tag | sed "s#^..#&/#")
+
+cat > wrong-tag <<EOF
+object $(echo blob | git hash-object -w --stdin)
+type commit
+tag wrong
+tagger T A Gger <[email protected]> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_success 'tag pointing to something else than its type' '
+	tag=$(git hash-object -t tag -w --stdin < wrong-tag) &&
+	echo $tag > .git/refs/tags/wrong &&
+	test_must_fail git fsck 2>&1 | tee out &&
+	grep "Object.*is a blob, not a commit" out
+'
+
+rm .git/refs/tags/wrong
+
+
 test_done

1.6.2.rc1.310.ga3b4a

This is wrong: It does not test the exist status. In a pipeline, the shell looks only at the exit status of the last command. You really want this as

test_must_fail git fsck >out 2>&1 &&

If you want to have it more verbose, add 'cat out &&'. But IMHO that is overengineered. If the test detects a regression in the future, it is easy to inspect the file out if necessary.

-- Hannes

git-fsck, of all tools, has very few tests. This adds some more:
* a corrupted object;
* a branch pointing to a non-commit;
* a tag pointing to a nonexistent object;
* and a tag pointing to an object of a type other than what the tag itself claims.

Some of the involved shell programming is due to Johannes Sixt.

---

Sorry for taking so long to fix this.

You're right.

I usually try to write the tests such that the cause of failure should be reasonably obvious from the verbose output. But I don't really care enough to insert stray cats either...

 t/t1450-fsck.sh |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 69 insertions(+), 0 deletions(-)

index 4597af0..d4a83a1 100755
@@ -28,4 +28,73 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '
 	)
 '
 
+# Corruption tests follow.  Make sure to remove all traces of the
+# specific corruption you test afterwards, lest a later test trip over
+# it.
+
+test_expect_success 'object with bad sha1' '
+	sha=$(echo blob | git hash-object -w --stdin) &&
+	echo $sha &&
+	old=$(echo $sha | sed "s+^..+&/+") &&
+	new=${old%/*}/ffffffffffffffffffffffffffffffffffffff &&
+	sha=${new%/*}${new##*/} &&
+	mv .git/objects/$old .git/objects/$new &&
+	git update-index --add --cacheinfo 100644 $sha foo &&
+	tree=$(git write-tree) &&
+	cmt=$(echo bogus | git commit-tree $tree) &&
+	git update-ref refs/heads/bogus $cmt &&
+	test_must_fail git fsck >out 2>&1 &&
+	grep "$sha.*corrupt" out
+'
+
+rm -f .git/objects/$new
+git update-ref -d refs/heads/bogus
+git read-tree -u --reset HEAD
+
+test_expect_success 'branch pointing to non-commit' '
+	git rev-parse HEAD^{tree} > .git/refs/heads/invalid &&
+	git fsck 2>&1 | tee out &&
+	grep "not a commit" out
+'
+
+git update-ref -d refs/heads/invalid
+
+cat > invalid-tag <<EOF
+object ffffffffffffffffffffffffffffffffffffffff
+type commit
+tag invalid
+tagger T A Gger <[email protected]> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_success 'tag pointing to nonexistent' '
+	tag=$(git hash-object -t tag -w --stdin < invalid-tag) &&
+	echo $tag > .git/refs/tags/invalid &&
+	test_must_fail git fsck >out 2>&1 &&
+	grep "missing commit ffffffffffffffffffffffffffffffffffffffff" out
+'
+
+rm .git/refs/tags/invalid
+rm -f .git/objects/$(echo $tag | sed "s#^..#&/#")
+
+cat > wrong-tag <<EOF
+object $(echo blob | git hash-object -w --stdin)
+type commit
+tag wrong
+tagger T A Gger <[email protected]> 1234567890 -0000
+
+This is an invalid tag.
+EOF
+
+test_expect_success 'tag pointing to something else than its type' '
+	tag=$(git hash-object -t tag -w --stdin < wrong-tag) &&
+	echo $tag > .git/refs/tags/wrong &&
+	test_must_fail git fsck >out 2>&1 &&
+	grep "Object.*is a blob, not a commit" out
+'
+
+rm .git/refs/tags/wrong
+
+
 test_done

1.6.2.rc2.289.g2fa25

And you still write this after the lesson?

Hi,

I strongly disagree that it is minor. _Especially_ since the tests are slow, -j makes a huge difference.

And also without making the tests slower, -j makes a dramatic difference here:

	$ /usr/bin/time make -j50 test
	...
	60.41user 213.92system 2:49.34elapsed 162%CPU
	(0avgtext+0avgdata 0maxresident)k
	160inputs+401944outputs
	(0major+37204627minor)pagefaults 0swaps

	$ /usr/bin/time make test
	...
	88.08user 314.96system 7:28.28elapsed 89%CPU
	(0avgtext+0avgdata 0maxresident)k
	0inputs+401008outputs
	(0major+37223584minor)pagefaults 0swaps

Ciao,
Dscho

That's not the point.

I agree that for the test suite, -j makes a huge impact, since it takes so long. But even though the docs claim it should be possible, I've been getting "random" test failures when compiled with coverage support, that went away with -j1. So the tests still run with -j1, as with the first version of the series.

However, the first version used a single make invocation for the'all' and 'test' targets, which of course also means that the _compilation_ ('all') happens with -j1. That's not necessary as far as I can see, and I split them into separate commands in v2, where 'all' doesn't pass a -j option any more (so that the caller's choice remains in effect).

What I meant by the above remark is that the maybe 30 seconds you gain by compiling in parallel are a very minor gain when compared to the enormous running time of the _sequential_ test suite _without optimizations_, which feels like 15-20min here, I haven't even timed it. (I always mount t/ on a tmpfs which does away with the fsync() bottleneck and lets me do ordinary test runs, with NO_SVN_TESTS, in under 3min.)

Thomas Rast
trast@{inf,student}.ethz.ch

Heya,

<snip>

I think in this case, where it is the second iteration of the series, with only few, and minor changes to the series as a whole it's better to just send the updates as reply to the original version?

Cheers,

Sverre Rabbelier

---

Same as v1.

 t/t4013-diff-various.sh           |    1 +
 t/t4013/diff.log_--decorate_--all |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)
 create mode 100644 t/t4013/diff.log_--decorate_--all

index 0289336..ef16f69 100755
@@ -207,6 +207,7 @@ log --root -c --patch-with-stat --summary master
 log --root --cc --patch-with-stat --summary master
 log -SF master
 log -SF -p master
+log --decorate --all
 
 whatchanged master
 whatchanged -p master

new file mode 100644
index 0000000..12da8ac
@@ -0,0 +1,34 @@
+$ git log --decorate --all
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (refs/heads/master)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <[email protected]>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
+Author: A U Thor <[email protected]>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <[email protected]>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <[email protected]>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (refs/heads/initial)
+Author: A U Thor <[email protected]>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$

1.6.2.rc1.266.gce6c4

---

Same as v1.

 t/t4013-diff-various.sh               |    3 +++
 t/t4013/diff.rev-list_--children_HEAD |    7 +++++++
 t/t4013/diff.rev-list_--parents_HEAD  |    7 +++++++
 3 files changed, 17 insertions(+), 0 deletions(-)
 create mode 100644 t/t4013/diff.rev-list_--children_HEAD
 create mode 100644 t/t4013/diff.rev-list_--parents_HEAD

index ef16f69..9cd5a6e 100755
@@ -209,6 +209,9 @@ log -SF master
 log -SF -p master
 log --decorate --all
 
+rev-list --parents HEAD
+rev-list --children HEAD
+
 whatchanged master
 whatchanged -p master
 whatchanged --root master

new file mode 100644
index 0000000..e7f17d5
@@ -0,0 +1,7 @@
+$ git rev-list --children HEAD
+59d314ad6f356dd08601a4cd5e530381da3e3c64
+c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a 59d314ad6f356dd08601a4cd5e530381da3e3c64
+9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 59d314ad6f356dd08601a4cd5e530381da3e3c64
+1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+444ac553ac7612cc88969031b02b3767fb8a353a 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+$

new file mode 100644
index 0000000..65d2a80
@@ -0,0 +1,7 @@
+$ git rev-list --parents HEAD
+59d314ad6f356dd08601a4cd5e530381da3e3c64 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a 444ac553ac7612cc88969031b02b3767fb8a353a
+9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 444ac553ac7612cc88969031b02b3767fb8a353a
+444ac553ac7612cc88969031b02b3767fb8a353a
+$

1.6.2.rc1.266.gce6c4

So far, git-patch-id was untested. Add some simple checks for output format and patch (in)equality.

---

Same as v1.

 t/t4203-patch-id.sh |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100755 t/t4203-patch-id.sh

new file mode 100755
index 0000000..04f7bae
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='git patch-id'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit initial foo a &&
+	test_commit first foo b &&
+	git checkout -b same HEAD^ &&
+	test_commit same-msg foo b &&
+	git checkout -b notsame HEAD^ &&
+	test_commit notsame-msg foo c
+'
+
+test_expect_success 'patch-id output is well-formed' '
+	git log -p -1 | git patch-id > output &&
+	grep "^[a-f0-9]\{40\} $(git rev-parse HEAD)$" output
+'
+
+get_patch_id () {
+	git log -p -1 "$1" | git patch-id |
+		sed "s# .*##" > patch-id_"$1"
+}
+
+test_expect_success 'patch-id detects equality' '
+	get_patch_id master &&
+	get_patch_id same &&
+	test_cmp patch-id_master patch-id_same
+'
+
+test_expect_success 'patch-id detects inequality' '
+	get_patch_id master &&
+	get_patch_id notsame &&
+	! test_cmp patch-id_master patch-id_notsame
+'
+
+test_done

1.6.2.rc1.266.gce6c4