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

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

[Junio C Hamano]

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

[PATCH v2 0/8] Support coverage testing with GCC/gcov

[Thomas Rast]

Junio C Hamano wrote:
> [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

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

[PATCH v2 1/8] Support coverage testing with GCC/gcov

[Thomas Rast]

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).

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

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(-)

diff --git a/Makefile b/Makefile
index b040a96..c32881b 100644
--- a/Makefile
+++ b/Makefile
@@ -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

[PATCH v2 2/8] Test that diff can read from stdin

[Thomas Rast]

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Same as v1.

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

diff --git a/t/t4002-diff-basic.sh b/t/t4002-diff-basic.sh
index cc3681f..18695ce 100755
--- a/t/t4002-diff-basic.sh
+++ b/t/t4002-diff-basic.sh
@@ -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

[PATCH v2 3/8] Test diff --dirstat functionality

[Thomas Rast]

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

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

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

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 9c70902..0289336 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -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
diff --git a/t/t4013/diff.diff_--dirstat_master~1_master~2 b/t/t4013/diff.diff_--dirstat_master~1_master~2
new file mode 100644
index 0000000..b672e1c
--- /dev/null
+++ b/t/t4013/diff.diff_--dirstat_master~1_master~2
@@ -0,0 +1,3 @@
+$ git diff --dirstat master~1 master~2
+  40.0% dir/
+$
-- 
1.6.2.rc1.266.gce6c4

[PATCH v2 4/8] Test log --graph

[Thomas Rast]

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.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Same as v1.

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

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 7b976ee..93966f7 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -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 <author@example.com>
+| |
+| |     Merge branch 'side'
+| |
+| * commit side
+| | Author: A U Thor <author@example.com>
+| |
+| |     side-2
+| |
+| * commit tags/side-1
+| | Author: A U Thor <author@example.com>
+| |
+| |     side-1
+| |
+* | commit master~1
+| | Author: A U Thor <author@example.com>
+| |
+| |     Second
+| |
+* | commit master~2
+| | Author: A U Thor <author@example.com>
+| |
+| |     sixth
+| |
+* | commit master~3
+| | Author: A U Thor <author@example.com>
+| |
+| |     fifth
+| |
+* | commit master~4
+|/  Author: A U Thor <author@example.com>
+|
+|       fourth
+|
+* commit tags/side-1~1
+| Author: A U Thor <author@example.com>
+|
+|     third
+|
+* commit tags/side-1~2
+| Author: A U Thor <author@example.com>
+|
+|     second
+|
+* commit tags/side-1~3
+  Author: A U Thor <author@example.com>
+
+      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

[PATCH v2 5/8] Test fsck a bit harder

[Thomas Rast]

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.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

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 <tagger@example.com> 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(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 4597af0..a22632f 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -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 <tagger@example.com> 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 <tagger@example.com> 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

[PATCH v2 6/8] Test log --decorate

[Thomas Rast]

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

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

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 0289336..ef16f69 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -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
diff --git a/t/t4013/diff.log_--decorate_--all b/t/t4013/diff.log_--decorate_--all
new file mode 100644
index 0000000..12da8ac
--- /dev/null
+++ b/t/t4013/diff.log_--decorate_--all
@@ -0,0 +1,34 @@
+$ git log --decorate --all
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (refs/heads/master)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (refs/heads/initial)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$
-- 
1.6.2.rc1.266.gce6c4

[PATCH v2 7/8] Test rev-list --parents/--children

[Thomas Rast]

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

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

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index ef16f69..9cd5a6e 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -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
diff --git a/t/t4013/diff.rev-list_--children_HEAD b/t/t4013/diff.rev-list_--children_HEAD
new file mode 100644
index 0000000..e7f17d5
--- /dev/null
+++ b/t/t4013/diff.rev-list_--children_HEAD
@@ -0,0 +1,7 @@
+$ git rev-list --children HEAD
+59d314ad6f356dd08601a4cd5e530381da3e3c64
+c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a 59d314ad6f356dd08601a4cd5e530381da3e3c64
+9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 59d314ad6f356dd08601a4cd5e530381da3e3c64
+1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+444ac553ac7612cc88969031b02b3767fb8a353a 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+$
diff --git a/t/t4013/diff.rev-list_--parents_HEAD b/t/t4013/diff.rev-list_--parents_HEAD
new file mode 100644
index 0000000..65d2a80
--- /dev/null
+++ b/t/t4013/diff.rev-list_--parents_HEAD
@@ -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

[PATCH v2 8/8] Test git-patch-id

[Thomas Rast]

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

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Same as v1.

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

diff --git a/t/t4203-patch-id.sh b/t/t4203-patch-id.sh
new file mode 100755
index 0000000..04f7bae
--- /dev/null
+++ b/t/t4203-patch-id.sh
@@ -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

Re: [PATCH v2 0/8] Support coverage testing with GCC/gcov

[Johannes Schindelin]

Hi,

On Thu, 19 Feb 2009, Thomas Rast wrote:

> Junio C Hamano wrote:
> > [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
> 
> 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.)

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

Re: [PATCH v2 0/8] Support coverage testing with GCC/gcov

[Sverre Rabbelier]

Heya,

On Thu, Feb 19, 2009 at 12:13, Thomas Rast <trast@student.ethz.ch> wrote:
> I noticed two small things that I'd like to fix before this goes
> 'next', so here they are:

<snip>

> Apart from that it's the same.

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

Re: [PATCH v2 0/8] Support coverage testing with GCC/gcov

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

Johannes Schindelin wrote:
> On Thu, 19 Feb 2009, Thomas Rast wrote:
> > 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.)
> 
> 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

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH v3] Test fsck a bit harder

[Thomas Rast]

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.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

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 <tagger@example.com> 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 <tagger@example.com> 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(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 4597af0..628db19 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -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 <tagger@example.com> 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 <tagger@example.com> 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

Re: [PATCH v3] Test fsck a bit harder

[Johannes Sixt]

Thomas Rast schrieb:
> +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 &&

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

> +	sha="$(dirname $new)$(basename $new)"

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

note the '&&'!

> +	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) &&

Any particular reason not to use

	test_must_fail git fsck 2>out &&

here?

> +	grep "$sha.*corrupt" out &&
> +	rm -f .git/objects/$new &&
> +	git update-ref -d refs/heads/bogus &&
> +	git read-tree -u --reset HEAD
> +'

Shouldn't the cleanup be outside the test_expect_success so that later 
tests work even if this one fails? (Ditto for subsequent tests.)

> +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 &&

	test_must_fail git fsck > out 2>&1 &&

> +	grep "missing commit ffffffffffffffffffffffffffffffffffffffff" out &&
...
> +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 &&

	test_must_fail git fsck > out 2>&1 &&

> +	grep "Object.*is a blob, not a commit" out &&
...

-- Hannes

[PATCH v4] Test fsck a bit harder

[Thomas Rast]

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.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Johannes Sixt wrote:
> Any particular reason not to use
> 
> 	test_must_fail git fsck 2>out &&
> 
> here?

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.

> Shouldn't the cleanup be outside the test_expect_success so that later 
> tests work even if this one fails? (Ditto for subsequent tests.)

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(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 4597af0..0906b1d 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -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 <tagger@example.com> 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 <tagger@example.com> 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

Re: [PATCH v4] Test fsck a bit harder

[Johannes Sixt]

On Samstag, 21. Februar 2009, Thomas Rast wrote:
> 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.

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

[PATCH v5] Test fsck a bit harder

[Thomas Rast]

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.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Sorry for taking so long to fix this.

Johannes Sixt wrote:
> 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 &&

You're right.

> 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.

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(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 4597af0..d4a83a1 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -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 <tagger@example.com> 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 <tagger@example.com> 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

Re: [PATCH v5] Test fsck a bit harder

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Johannes Sixt wrote:
>> 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 &&
>
> You're right.

And you still write this after the lesson?

> +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 <tagger@example.com> 1234567890 -0000
> +
> +This is an invalid tag.
> +EOF

Technically speaking, the tag is perfectly valid.  It just points to a
commit you do not have.

> +cat > wrong-tag <<EOF
> +object $(echo blob | git hash-object -w --stdin)
> +type commit
> +tag wrong
> +tagger T A Gger <tagger@example.com> 1234567890 -0000
> +
> +This is an invalid tag.
> +EOF

This is indeed invalid.

> +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

I'd rather want to see test_expect_success around this "rm", and replace
that rm with something more git-ish, like "tag -d" or "update-ref -d".

By the way, is it just me or would we want to reword the subject of the
commit to tame it a little bit?