Git development
 help / color / mirror / Atom feed
* [PATCH 4/9] Make branch creation atomic
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


This patch adds an optional create_at parameter to Series.init(), to
pass a git.branch_create() parameter if we want it to be called.  This
parameter can be None, so the test for False has to be explicit.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/branch.py |    6 +-----
 stgit/stack.py           |    8 +++++---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index be501a8..c7561a8 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -125,12 +125,8 @@ def func(parser, options, args):
         tree_id = None
         if len(args) == 2:
             tree_id = git_id(args[1])
-
-        if git.branch_exists(args[0]):
-            raise CmdException, 'Branch "%s" already exists' % args[0]
         
-        stack.Series(args[0]).init()
-        git.create_branch(args[0], tree_id)
+        stack.Series(args[0]).init(create_at = tree_id)
 
         print 'Branch "%s" created.' % args[0]
         return
diff --git a/stgit/stack.py b/stgit/stack.py
index c14e029..19bb62a 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -426,7 +426,7 @@ class Series:
         """
         return os.path.isdir(self.__patch_dir)
 
-    def init(self):
+    def init(self, create_at=False):
         """Initialises the stgit series
         """
         bases_dir = os.path.join(self.__base_dir, 'refs', 'bases')
@@ -438,6 +438,9 @@ class Series:
         if os.path.exists(self.__base_file):
             raise StackException, self.__base_file + ' already exists'
 
+        if (create_at!=False):
+            git.create_branch(self.__name, create_at)
+
         os.makedirs(self.__patch_dir)
 
         if not os.path.isdir(bases_dir):
@@ -509,8 +512,7 @@ class Series:
         """Clones a series
         """
         base = read_string(self.get_base_file())
-        git.create_branch(target_series, tree_id = base)
-        Series(target_series).init()
+        Series(target_series).init(create_at = base)
         new_series = Series(target_series)
 
         # generate an artificial description file

^ permalink raw reply related

* [PATCH 5/9] Correctly handle refs/patches on series rename
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


When renaming a series, the refs/patches dir was not moved, and by
chance a new one was created by the repository-upgrade code, but that
left the old one behind as cruft (which the safety checks added in a
former patch now detects).

Also adds a regression test to assert that nothing by the old name
is left behind.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/stack.py           |    2 ++
 t/t1001-branch-rename.sh |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/stgit/stack.py b/stgit/stack.py
index 19bb62a..975ac21 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -505,6 +505,8 @@ class Series:
             os.rename(self.__series_dir, to_stack.__series_dir)
         if os.path.exists(self.__base_file):
             os.rename(self.__base_file, to_stack.__base_file)
+        if os.path.exists(self.__refs_dir):
+            os.rename(self.__refs_dir, to_stack.__refs_dir)
 
         self.__init__(to_name)
 
diff --git a/t/t1001-branch-rename.sh b/t/t1001-branch-rename.sh
new file mode 100755
index 0000000..28da15c
--- /dev/null
+++ b/t/t1001-branch-rename.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Branch renames.
+
+Exercises branch renaming commands.
+'
+
+. ./test-lib.sh
+
+test_expect_success \
+    'Create an stgit branch from scratch' \
+    'stg init &&
+     stg branch -c foo &&
+     stg new p1 -m "p1"
+'
+
+test_expect_failure \
+    'Rename the current stgit branch' \
+    'stg branch -r foo bar
+'
+
+test_expect_success \
+    'Rename an stgit branch' \
+    'stg branch -c buz &&
+     stg branch -r foo bar &&
+     test -z `find .git -name foo | tee /dev/stderr`
+'
+
+test_done

^ permalink raw reply related

* [PATCH 7/9] Test that pulls a patch creating a file that got modified afterwards
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


This demonstrates an issue wite has bitten me more than once: the stg
branch adds a file in one patch, and modifies it in a later patch; then all
patches get integrated in upstream tree, and at "stg pull" time, stgit
believes there is a conflict, even one the patches are exactly the same.

"stg push" apparently does not consider patches one by one, or it
would have noticed that the patches were "already applied".  It reports:

 Pushing patch "p1"...Error: File "file2" added in branches but different
 The merge failed during "push". Use "refresh" after fixing the conflicts
 stg push: GIT index merging failed (possible conflicts)

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 t/t1200-push-modified.sh |   51 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/t/t1200-push-modified.sh b/t/t1200-push-modified.sh
new file mode 100755
index 0000000..18a4df2
--- /dev/null
+++ b/t/t1200-push-modified.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Exercise pushing patches applied upstream.
+
+Especially, consider the case of a patch that adds a file,
+while a subsequent one modifies it.
+'
+
+. ./test-lib.sh
+
+# don't need this repo, but better not drop it, see t1100
+#rm -rf .git
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'Clone tree and setup changes' \
+    "stg clone foo bar &&
+     (cd bar && stg new p1 -m p1
+      printf 'a\nc\n' > file && stg add file && stg refresh &&
+      stg new p2 -m p2
+      printf 'a\nb\nc\n' > file && stg refresh
+     )
+"
+
+test_expect_success \
+    'Port those patches to orig tree' \
+    "(cd foo &&
+      GIT_DIR=../bar/.git git-format-patch --stdout bases/master..HEAD |
+      git-am -3 -k
+     )
+"
+
+test_expect_success \
+    'Pull to sync with parent, preparing for the problem' \
+    "(cd bar && stg pop --all &&
+      stg pull
+     )
+"
+
+test_expect_success \
+    'Now attempt to push those patches applied upstream' \
+    "(cd bar && stg push --all
+     )
+"
+
+test_done

^ permalink raw reply related

* [PATCH 3/9] Add a couple of safety checks to series creation
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


- we must check first whether the operation can complete, instead of
  bombing out halfway.  That means checking that nothing will prevent
  the creation of stgit data (note that calling is_initialised() is
  not enough, as it does not catch a bogus file), which is tested by
  the 3 first couple of testcases, and fixed in stack.py

- creating the git branch unconditionally before knowing whether
  creation of the stgit stuff can be completed is a problem as well:
  being atomic would be much much better.  To emulate atomicity (which
  comeds in the next patch), this patch does a somewhat dirty hack to
  branch.py: we first attempt to create the stgit stuff, and if that
  succeeds, we create the git branch: it is much easier to do at first
  a quick check that the latter will succeed.  Testcase 7/8 ensure
  that such a safety check has not been forgotten.

- when git already reports a problem with that head we would like to
  create, we should catch it.  Testcase 9/10 creates such a situation,
  and the fix to git.py allows to catch the error spit out by
  git-rev-parse.  I cannot tell why the stderr lines were not included
  by the Popen3 object.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/branch.py |    5 ++-
 stgit/git.py             |    4 ++
 stgit/stack.py           |    7 +++-
 t/t1000-branch-create.sh |   82 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 3 deletions(-)

diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index c4b5945..be501a8 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -126,8 +126,11 @@ def func(parser, options, args):
         if len(args) == 2:
             tree_id = git_id(args[1])
 
-        git.create_branch(args[0], tree_id)
+        if git.branch_exists(args[0]):
+            raise CmdException, 'Branch "%s" already exists' % args[0]
+        
         stack.Series(args[0]).init()
+        git.create_branch(args[0], tree_id)
 
         print 'Branch "%s" created.' % args[0]
         return
diff --git a/stgit/git.py b/stgit/git.py
index d75b54e..8523455 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -272,9 +272,11 @@ def rev_parse(git_id):
 def branch_exists(branch):
     """Existence check for the named branch
     """
-    for line in _output_lines(['git-rev-parse', '--symbolic', '--all']):
+    for line in _output_lines('git-rev-parse --symbolic --all 2>&1'):
         if line.strip() == branch:
             return True
+        if re.compile('[ |/]'+branch+' ').search(line):
+            raise GitException, 'Bogus branch: %s' % line
     return False
 
 def create_branch(new_branch, tree_id = None):
diff --git a/stgit/stack.py b/stgit/stack.py
index f4d7490..c14e029 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -431,8 +431,13 @@ class Series:
         """
         bases_dir = os.path.join(self.__base_dir, 'refs', 'bases')
 
-        if self.is_initialised():
+        if os.path.exists(self.__patch_dir):
             raise StackException, self.__patch_dir + ' already exists'
+        if os.path.exists(self.__refs_dir):
+            raise StackException, self.__refs_dir + ' already exists'
+        if os.path.exists(self.__base_file):
+            raise StackException, self.__base_file + ' already exists'
+
         os.makedirs(self.__patch_dir)
 
         if not os.path.isdir(bases_dir):
diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh
new file mode 100755
index 0000000..f0c2367
--- /dev/null
+++ b/t/t1000-branch-create.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Branch operations.
+
+Exercises the "stg branch" commands.
+'
+
+. ./test-lib.sh
+
+stg init
+
+test_expect_failure \
+    'Try to create an stgit branch with a spurious refs/patches/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/refs/patches/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/patches/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+test_expect_failure \
+    'Try to create an stgit branch with a spurious patches/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/patches/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/patches/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+test_expect_failure \
+    'Try to create an stgit branch with a spurious refs/bases/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/refs/bases/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/bases/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+
+test_expect_failure \
+    'Try to create an stgit branch with an existing git branch by that name' \
+    'find .git -name foo | xargs rm -rf &&
+     cp .git/refs/heads/master .git/refs/heads/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/heads/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+
+test_expect_failure \
+    'Try to create an stgit branch with an invalid refs/heads/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/refs/heads/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/heads/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+test_done

^ permalink raw reply related

* [PATCH 6/9] Fix a seriously bad interaction between .git caching and repo cloning
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


Testcase 2 exhibits a problem with caching the location of .git while
cloning a repository.  Since basedir.get() is called before the clone is
built, a value may get stored in the cache if we are within a
git-controlled tree already.  Then when constructing the object for the
clone, a bogus .git is used, which can lead, when running tests in t/trash,
to corruption of the stgit .git repository.

Testcase 1 does not show any problem by chance, because since we have a
./.git prepared for use by the testsuite, value ".git" get cached, and it
happens that this value will be still valid after chdir'ing into the
newborn clone.

Clearing the cache at the appropriate place fixes the problem.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/basedir.py        |    6 ++++++
 stgit/commands/clone.py |    3 +++
 t/t1100-clone-under.sh  |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/stgit/basedir.py b/stgit/basedir.py
index c394572..81f2b40 100644
--- a/stgit/basedir.py
+++ b/stgit/basedir.py
@@ -42,3 +42,9 @@ def get():
             __base_dir = __output('git-rev-parse --git-dir 2> /dev/null')
 
     return __base_dir
+
+def clear_cache():
+    """Clear the cached location of .git
+    """
+    global __base_dir
+    __base_dir = None
diff --git a/stgit/commands/clone.py b/stgit/commands/clone.py
index 9ad76a6..455dd6e 100644
--- a/stgit/commands/clone.py
+++ b/stgit/commands/clone.py
@@ -51,6 +51,9 @@ def func(parser, options, args):
     os.chdir(local_dir)
     git.checkout(tree_id = 'HEAD')
 
+    # be sure to forget any cached value for .git, since we're going
+    # to work on a brand new repository
+    basedir.clear_cache()
     stack.Series().init()
 
     print 'done'
diff --git a/t/t1100-clone-under.sh b/t/t1100-clone-under.sh
new file mode 100755
index 0000000..c86ef61
--- /dev/null
+++ b/t/t1100-clone-under.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Check cloning in a repo subdir
+
+Check that "stg clone" works in a subdir of a git tree.
+This ensures (to some point) that a clone within a tree does
+not corrupt the enclosing repo.
+
+This test must be run before any tests making use of clone.
+'
+
+. ./test-lib.sh
+
+# Here we are in a repo, we have a ./.git
+# Do not get rid of it, or a bug may bite out stgit repo hard
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'stg clone right inside a git tree' \
+    "stg clone foo bar"
+
+# now work in a subdir
+mkdir sub
+mv foo sub
+cd sub
+
+test_expect_success \
+    'stg clone deeper under a git tree' \
+    "stg clone foo bar"
+
+test_done

^ permalink raw reply related

* [PATCH 2/9] Add list of bugs to TODO
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


Since there is no formal place to register bugs, other than the git
ml, I have added a couple of them, some of them already mentionned on
the ml but still around.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 TODO |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/TODO b/TODO
index d97ffd1..a13e511 100644
--- a/TODO
+++ b/TODO
@@ -7,6 +7,7 @@ The TODO list until 1.0:
 - man page
 - code execution allowed from templates
 - more regression tests
+- stg help should probably pipe through the $PAGER
 - release 1.0
 
 
@@ -17,3 +18,17 @@ The future, when time allows or if someo
   synchronising with other patches (diff format or in other
   repositories)
 - write bash-completion script for the StGIT commands
+- support for branches with / in names
+  (ml: "Handle branch names with slashes")
+- "pull" argument should default to a sane value, "origin" is wrong in
+  many cases
+
+Bugs:
+
+- the following commands break in subdirs:
+  - refresh (ml: "Running StGIT in subdirectories")
+- "stg show" on empty patch shows previous patch
+- "stg add" is accepted when no patch is applied, then any push says
+  one must refresh first, which is obviously wrong
+- "stg add" on files already added should print a notice, so that the
+  user can catch a possible typo

^ permalink raw reply related

* [PATCH 1/9] Add a testsuite framework copied from git-core
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


See git's t/README for details on how to use this framework.

There is no integration yet in the toplevel Makefile, I'll let
python masters take care of this.  Use "make -C t" to run the
tests for now.

A patch-naming policy should be defined for stgit, since the
git one does not apply.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 TODO             |    2 -
 t/Makefile       |   25 ++++++
 t/README         |  210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t0000-dummy.sh |   17 ++++
 t/test-lib.sh    |  203 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 456 insertions(+), 1 deletions(-)

diff --git a/TODO b/TODO
index e5affe0..d97ffd1 100644
--- a/TODO
+++ b/TODO
@@ -6,7 +6,7 @@ The TODO list until 1.0:
 - debian package support
 - man page
 - code execution allowed from templates
-- regression tests
+- more regression tests
 - release 1.0
 
 
diff --git a/t/Makefile b/t/Makefile
new file mode 100644
index 0000000..d5d7b6f
--- /dev/null
+++ b/t/Makefile
@@ -0,0 +1,25 @@
+# Run tests
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+#GIT_TEST_OPTS=--verbose --debug
+SHELL_PATH ?= $(SHELL)
+TAR ?= $(TAR)
+
+# Shell quote;
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+
+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
+
+all: $(T) clean
+
+$(T):
+	@echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+
+clean:
+	rm -fr trash
+
+.PHONY: $(T) clean
+.NOPARALLEL:
+
diff --git a/t/README b/t/README
new file mode 100644
index 0000000..d88bad2
--- /dev/null
+++ b/t/README
@@ -0,0 +1,210 @@
+Core GIT Tests
+==============
+
+This directory holds many test scripts for core GIT tools.  The
+first part of this short document describes how to run the tests
+and read their output.
+
+When fixing the tools or adding enhancements, you are strongly
+encouraged to add tests in this directory to cover what you are
+trying to fix or enhance.  The later part of this short document
+describes how your test scripts should be organized.
+
+The mechanism that powers this testsuite is directly imported from the
+Core GIT Tests, in directory t/ of the git repository.  Files are base
+on Core GIT version 1.3.0.rc4.g5069.
+
+
+Running Tests
+-------------
+
+The easiest way to run tests is to say "make -C t".  This runs all
+the tests.
+
+    *** t0000-basic.sh ***
+    *   ok 1: .git/objects should be empty after git-init-db in an empty repo.
+    *   ok 2: .git/objects should have 256 subdirectories.
+    *   ok 3: git-update-index without --add should fail adding.
+    ...
+    *   ok 23: no diff after checkout and git-update-index --refresh.
+    * passed all 23 test(s)
+    *** t0100-environment-names.sh ***
+    *   ok 1: using old names should issue warnings.
+    *   ok 2: using old names but having new names should not issue warnings.
+    ...
+
+Or you can run each test individually from command line, like
+this:
+
+    $ sh ./t3001-ls-files-killed.sh
+    *   ok 1: git-update-index --add to add various paths.
+    *   ok 2: git-ls-files -k to show killed files.
+    *   ok 3: validate git-ls-files -k output.
+    * passed all 3 test(s)
+
+You can pass --verbose (or -v), --debug (or -d), and --immediate
+(or -i) command line argument to the test.
+
+--verbose::
+	This makes the test more verbose.  Specifically, the
+	command being run and their output if any are also
+	output.
+
+--debug::
+	This may help the person who is developing a new test.
+	It causes the command defined with test_debug to run.
+
+--immediate::
+	This causes the test to immediately exit upon the first
+	failed test.
+
+
+Naming Tests
+------------
+
+The test files are named as:
+
+	tNNNN-commandname-details.sh
+
+where N is a decimal digit.
+
+Here is a proposal for numbering, loosely based on the Core GIT
+numbering conventions.
+
+First two digit tells the particular command we are testing:
+
+	00 - stgit itself
+	10 - branch
+	11 - clone
+	12 - push
+
+Third and fourth digit (optionally) tells the particular switch or
+group of switches we are testing.
+
+If you create files under t/ directory (i.e. here) that is not
+the top-level test script, never name the file to match the above
+pattern.  The Makefile here considers all such files as the
+top-level test script and tries to run all of them.  A care is
+especially needed if you are creating a common test library
+file, similar to test-lib.sh, because such a library file may
+not be suitable for standalone execution.
+
+
+Writing Tests
+-------------
+
+The test script is written as a shell script.  It should start
+with the standard "#!/bin/sh" with copyright notices, and an
+assignment to variable 'test_description', like this:
+
+	#!/bin/sh
+	#
+	# Copyright (c) 2005 Junio C Hamano
+	#
+
+	test_description='xxx test (option --frotz)
+
+	This test registers the following structure in the cache
+	and tries to run git-ls-files with option --frotz.'
+
+
+Source 'test-lib.sh'
+--------------------
+
+After assigning test_description, the test script should source
+test-lib.sh like this:
+
+	. ./test-lib.sh
+
+This test harness library does the following things:
+
+ - If the script is invoked with command line argument --help
+   (or -h), it shows the test_description and exits.
+
+ - Creates an empty test directory with an empty .git/objects
+   database and chdir(2) into it.  This directory is 't/trash'
+   if you must know, but I do not think you care.
+
+ - Defines standard test helper functions for your scripts to
+   use.  These functions are designed to make all scripts behave
+   consistently when command line arguments --verbose (or -v),
+   --debug (or -d), and --immediate (or -i) is given.
+
+
+End with test_done
+------------------
+
+Your script will be a sequence of tests, using helper functions
+from the test harness library.  At the end of the script, call
+'test_done'.
+
+
+Test harness library
+--------------------
+
+There are a handful helper functions defined in the test harness
+library for your script to use.
+
+ - test_expect_success <message> <script>
+
+   This takes two strings as parameter, and evaluates the
+   <script>.  If it yields success, test is considered
+   successful.  <message> should state what it is testing.
+
+   Example:
+
+	test_expect_success \
+	    'git-write-tree should be able to write an empty tree.' \
+	    'tree=$(git-write-tree)'
+
+ - test_expect_failure <message> <script>
+
+   This is the opposite of test_expect_success.  If <script>
+   yields success, test is considered a failure.
+
+   Example:
+
+	test_expect_failure \
+	    'git-update-index without --add should fail adding.' \
+	    'git-update-index should-be-empty'
+
+ - test_debug <script>
+
+   This takes a single argument, <script>, and evaluates it only
+   when the test script is started with --debug command line
+   argument.  This is primarily meant for use during the
+   development of a new test script.
+
+ - test_done
+
+   Your test script must have test_done at the end.  Its purpose
+   is to summarize successes and failures in the test script and
+   exit with an appropriate error code.
+
+
+Tips for Writing Tests
+----------------------
+
+As with any programming projects, existing programs are the best
+source of the information.  However, do _not_ emulate
+t0000-basic.sh when writing your tests.  The test is special in
+that it tries to validate the very core of GIT.  For example, it
+knows that there will be 256 subdirectories under .git/objects/,
+and it knows that the object ID of an empty tree is a certain
+40-byte string.  This is deliberately done so in t0000-basic.sh
+because the things the very basic core test tries to achieve is
+to serve as a basis for people who are changing the GIT internal
+drastically.  For these people, after making certain changes,
+not seeing failures from the basic test _is_ a failure.  And
+such drastic changes to the core GIT that even changes these
+otherwise supposedly stable object IDs should be accompanied by
+an update to t0000-basic.sh.
+
+However, other tests that simply rely on basic parts of the core
+GIT working properly should not have that level of intimate
+knowledge of the core GIT internals.  If all the test scripts
+hardcoded the object IDs like t0000-basic.sh does, that defeats
+the purpose of t0000-basic.sh, which is to isolate that level of
+validation in one place.  Your test also ends up needing
+updating when such a change to the internal happens, so do _not_
+do it and leave the low level of validation to t0000-basic.sh.
diff --git a/t/t0000-dummy.sh b/t/t0000-dummy.sh
new file mode 100755
index 0000000..1a9bd87
--- /dev/null
+++ b/t/t0000-dummy.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Dummy test.
+
+Only to test the testing environment.
+'
+
+. ./test-lib.sh
+
+test_expect_success \
+    'check stgit can be run' \
+    'stg version'
+
+test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
new file mode 100755
index 0000000..6339c54
--- /dev/null
+++ b/t/test-lib.sh
@@ -0,0 +1,203 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+# Copyright (c) 2006 Yann Dirson - tuning for stgit
+#
+
+# For repeatability, reset the environment to known value.
+LANG=C
+LC_ALL=C
+PAGER=cat
+TZ=UTC
+export LANG LC_ALL PAGER TZ
+unset AUTHOR_DATE
+unset AUTHOR_EMAIL
+unset AUTHOR_NAME
+unset COMMIT_AUTHOR_EMAIL
+unset COMMIT_AUTHOR_NAME
+unset GIT_ALTERNATE_OBJECT_DIRECTORIES
+unset GIT_AUTHOR_DATE
+GIT_AUTHOR_EMAIL=author@example.com
+GIT_AUTHOR_NAME='A U Thor'
+unset GIT_COMMITTER_DATE
+GIT_COMMITTER_EMAIL=committer@example.com
+GIT_COMMITTER_NAME='C O Mitter'
+unset GIT_DIFF_OPTS
+unset GIT_DIR
+unset GIT_EXTERNAL_DIFF
+unset GIT_INDEX_FILE
+unset GIT_OBJECT_DIRECTORY
+unset SHA1_FILE_DIRECTORIES
+unset SHA1_FILE_DIRECTORY
+export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
+export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
+
+# Each test should start with something like this, after copyright notices:
+#
+# test_description='Description of this test...
+# This test checks if command xyzzy does the right thing...
+# '
+# . ./test-lib.sh
+
+error () {
+	echo "* error: $*"
+	trap - exit
+	exit 1
+}
+
+say () {
+	echo "* $*"
+}
+
+test "${test_description}" != "" ||
+error "Test script did not set test_description."
+
+while test "$#" -ne 0
+do
+	case "$1" in
+	-d|--d|--de|--deb|--debu|--debug)
+		debug=t; shift ;;
+	-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
+		immediate=t; shift ;;
+	-h|--h|--he|--hel|--help)
+		echo "$test_description"
+		exit 0 ;;
+	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
+		verbose=t; shift ;;
+	*)
+		break ;;
+	esac
+done
+
+exec 5>&1
+if test "$verbose" = "t"
+then
+	exec 4>&2 3>&1
+else
+	exec 4>/dev/null 3>/dev/null
+fi
+
+test_failure=0
+test_count=0
+
+trap 'echo >&5 "FATAL: Unexpected exit with code $?"; exit 1' exit
+
+
+# You are not expected to call test_ok_ and test_failure_ directly, use
+# the text_expect_* functions instead.
+
+test_ok_ () {
+	test_count=$(expr "$test_count" + 1)
+	say "  ok $test_count: $@"
+}
+
+test_failure_ () {
+	test_count=$(expr "$test_count" + 1)
+	test_failure=$(expr "$test_failure" + 1);
+	say "FAIL $test_count: $1"
+	shift
+	echo "$@" | sed -e 's/^/	/'
+	test "$immediate" = "" || { trap - exit; exit 1; }
+}
+
+
+test_debug () {
+	test "$debug" = "" || eval "$1"
+}
+
+test_run_ () {
+	eval >&3 2>&4 "$1"
+	eval_ret="$?"
+	return 0
+}
+
+test_expect_failure () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-expect-failure"
+	say >&3 "expecting failure: $2"
+	test_run_ "$2"
+	if [ "$?" = 0 -a "$eval_ret" != 0 ]
+	then
+		test_ok_ "$1"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+test_expect_success () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-expect-success"
+	say >&3 "expecting success: $2"
+	test_run_ "$2"
+	if [ "$?" = 0 -a "$eval_ret" = 0 ]
+	then
+		test_ok_ "$1"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+test_expect_code () {
+	test "$#" = 3 ||
+	error "bug in the test script: not 3 parameters to test-expect-code"
+	say >&3 "expecting exit code $1: $3"
+	test_run_ "$3"
+	if [ "$?" = 0 -a "$eval_ret" = "$1" ]
+	then
+		test_ok_ "$2"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+# Most tests can use the created repository, but some amy need to create more.
+# Usage: test_create_repo <directory>
+test_create_repo () {
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 parameter to test-create-repo"
+	owd=`pwd`
+	repo="$1"
+	mkdir "$repo"
+	cd "$repo" || error "Cannot setup test environment"
+	git-init-db 2>/dev/null ||
+	error "cannot run git-init-db -- have you installed git-core?"
+	mv .git/hooks .git/hooks-disabled
+	echo "empty start" |
+	git-commit-tree `git-write-tree` >.git/refs/heads/master 2>/dev/null ||
+	error "cannot run git-commit -- is your git-core funtionning?"
+	cd "$owd"
+}
+
+test_done () {
+	trap - exit
+	case "$test_failure" in
+	0)
+		# We could:
+		# cd .. && rm -fr trash
+		# but that means we forbid any tests that use their own
+		# subdirectory from calling test_done without coming back
+		# to where they started from.
+		# The Makefile provided will clean this test area so
+		# we will leave things as they are.
+
+		say "passed all $test_count test(s)"
+		exit 0 ;;
+
+	*)
+		say "failed $test_failure among $test_count test(s)"
+		exit 1 ;;
+
+	esac
+}
+
+# Test the binaries we have just built.  The tests are kept in
+# t/ subdirectory and are run in trash subdirectory.
+PATH=$(pwd)/..:$PATH
+export PATH
+
+
+# Test repository
+test=trash
+rm -fr "$test"
+test_create_repo $test
+cd "$test"

^ permalink raw reply related

* [PATCH 0/9] Add a testsuite to stgit (take 3), and more
From: Yann Dirson @ 2006-04-16 10:41 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

This new revision of my testsuite patches include:

- cleanup of the repo-initialization stuff in test-lib.sh: now
test_create_repo creates an empty commit to start with, so that HEAD
points to an existing master branch.  This makes test_stg_init
useless, rather let tests can "stg init" directly.

- steal the git t/README as well, and adapt it.  Proposes a numbering
convention for stgit testcases.

- activation and refining of the testcase which I had left commented
out in t1000, and a fix for the bug it shows

- fix for a nasty caching issue in stg clone, and associated test

- 2 testcases showing problems with detection of applied patches in
"push" after a pull when our patches were integrated upstream.  No fix
here, though, so you'll see t1200 and t1201 failing.


And not directly related to the testsuite:
- more bugs listed in TODO
- support for per-user templates for mail and export, in ~/.stgit/templates

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

^ permalink raw reply

* Re: Help please :-)
From: Paolo Ciarrocchi @ 2006-04-16  9:30 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Git Mailing List
In-Reply-To: <20060415191039.GC27689@pasky.or.cz>

On 4/15/06, Petr Baudis <pasky@suse.cz> wrote:
>   Hello,
>
> Dear diary, on Sat, Apr 15, 2006 at 06:08:01PM CEST, I got a letter
> where Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> said that...
> > I'm used to keep updated my linux tree with cg-status,
> > I did that this morning but now I see the following:
> > paolo@Italia:~/linux-2.6$ cg-status
> > Heads:
> >    >master      2c5362007bc0a46461a9d94958cdd53bb027004c
> >   R origin      2c5362007bc0a46461a9d94958cdd53bb027004c
> >
> > ? arch/i386/kernel/smpboot.c.rej
> > ? drivers/md/dm-stripe.c.rej
> > ? drivers/net/chelsio/sge.c.rej
> > ? drivers/net/e100.c.rej
> > ? drivers/net/e1000/e1000_main.c.rej
> > ? fs/9p/vfs_dir.c.rej
> > ? fs/nfsctl.c.rej
> > ? kernel/fork.c.rej
> > ? kernel/posix-timers.c.rej
> > ? kernel/timer.c.rej
> > ? mm/memory.c.rej
> > ? mm/mempolicy.c.rej
> > ? mm/swap.c.rej
> > ? net/ieee80211/ieee80211_crypt_ccmp.c.rej
> > ? net/ieee80211/ieee80211_rx.c.rej
> > ? scripts/kconfig/lkc_defs.h
> > ? scripts/mod/modpost.c.rej
> > paolo@Italia:~/linux-2.6$ cg-diff
> >
> > I'm a bit lost, the tree is correctly updated, no error message but
> > why I see all these .rej?
>
>   you apparently had local changes in your working tree, did cg-update
> and then the local changes conflicted with the new changes in Linus'
> tree. cg-update should have told you further details.

Can this be due an interrupted cg-update?
However, cg-update did not complain.

> > And how can I fix this problem?
> > git reset and cg-reset don't help...
>
>   cg-clean can remove files not recognized by git.

Thanks!

Ciao,
--
Paolo
http://paolociarrocchi.googlepages.com

^ permalink raw reply

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-16  8:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604151016220.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Btw, I can certainly understand if you don't want to do this before 1.3.x. 
> Since there's no actual user-visible advantage to it, it's probably worth 
> dropping for now.

Well, I bit the bullet, fixed-up the remaining issues I found in
rev-list in your grand unified version, reverted the revert, and
ported the changes for log/whatchanged/show.

For now this lives in the "next" branch and _will_ not graduate
before 1.3.0, of course.

^ permalink raw reply

* Re: [RFC/PATCH] pager: do not fork a pager if environment variable PAGER is set to NONE
From: Johannes Schindelin @ 2006-04-16  2:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4q0udzwg.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sat, 15 Apr 2006, Junio C Hamano wrote:

> A somewhat related topic; I often set PAGER=cat when I do not
> want the --[More]-- prompt and I thing many Emacs users do this.
> It might also be good to detect it and omit piping in such a
> case, but that is independent, so if you are going to do this as
> well, please make it a separate patch.

I was not quite sure if PAGER=cat could be taken as "user does not want 
any pager to be fork()ed", but you are probably right: PAGER=cat means 
that stdout is forwarded to stdout, i.e. we are better off not fork()ing 
and calling "cat":

---
[PATCH] pager: do not fork a pager if PAGER=cat

This helps debugging tremendously.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 pager.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

4a617a436bba9288cec5d3918d02c5b0e652df98
diff --git a/pager.c b/pager.c
index 1364e15..86e93b3 100644
--- a/pager.c
+++ b/pager.c
@@ -5,9 +5,8 @@ #include "cache.h"
  * something different on Windows, for example.
  */
 
-static void run_pager(void)
+static void run_pager(const char *prog)
 {
-	const char *prog = getenv("PAGER");
 	if (!prog)
 		prog = "less";
 	setenv("LESS", "-S", 0);
@@ -16,10 +15,11 @@ static void run_pager(void)
 
 void setup_pager(void)
 {
+	const char *prog = getenv("PAGER");
 	pid_t pid;
 	int fd[2];
 
-	if (!isatty(1))
+	if (!isatty(1) || (prog != NULL && !strcmp(prog, "cat")))
 		return;
 	if (pipe(fd) < 0)
 		return;
@@ -43,6 +43,6 @@ void setup_pager(void)
 	close(fd[0]);
 	close(fd[1]);
 
-	run_pager();
+	run_pager(prog);
 	exit(255);
 }
-- 
1.3.0.rc4.gb6b20-dirty

^ permalink raw reply related

* Re: git-svn and Author files question
From: Junio C Hamano @ 2006-04-16  2:22 UTC (permalink / raw)
  To: git
In-Reply-To: <m2ejzys2ns.fsf@ziti.fhcrc.org>

Seth Falcon <sethfalcon@gmail.com> writes:

> I'm a bit confused about why I have to do this.  Is there a way around
> this?  Or perhaps a way to force a bogus email address based on svn
> user name?

I think this is a reasonable request.  IIRC, the original
foreign SCM interface (git-cvsimport) did "foobar <foobar>"
if there is no mapping available, so that might be a good
example to follow.

^ permalink raw reply

* Re: [RFC/PATCH] pager: do not fork a pager if environment variable PAGER is set to NONE
From: Junio C Hamano @ 2006-04-16  2:20 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604160357460.31461@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Sat, 15 Apr 2006, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > This helps debugging tremendously.
>> >
>> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>> 
>> I like what this wants to do.  I am not so sure PAGER=NONE is a
>> good convention, however.
>
> I am sure it is not.
>
> One solution would be to introduce yet another command line option 
> "--no-pager", but I find that ugly.
>
> Another solution would be to check if the environment variable NO_PAGER is 
> set.
>
> Wishes?

Honestly, undecided.  Not that I think people would do
PAGER=NONE to mean a program /usr/bin/NONE.

Right now, the following does not say anything:

	$ PAGER= git log

but I do not think that is a reasonably behaviour, either.  So
detecting PAGER is set to an empty string (not "nonexistence" -
the current behaviour of defaulting to "less" in such a case is
reasonable) is probably a better alternative.

A somewhat related topic; I often set PAGER=cat when I do not
want the --[More]-- prompt and I thing many Emacs users do this.
It might also be good to detect it and omit piping in such a
case, but that is independent, so if you are going to do this as
well, please make it a separate patch.

^ permalink raw reply

* [FYI] generated completions
From: Johannes Schindelin @ 2006-04-16  2:05 UTC (permalink / raw)
  To: git

Hi,

I just played around with command line completion in bash, mainly to 
distract me from the work I have to do.

This is a shell script which generates a completion script from the 
documentation. It is very dumb at the moment, but at least a start. I am 
not likely to continue with this in the near future, so everyone who's 
interested: go wild.

--- generate-completions.sh ---
#!/bin/sh

cat << EOF
function gitcomplete ()
{
	case "\$COMP_LINE" in
EOF

cmds=""
for file in Documentation/git-*.txt; do
	cmd=$(expr $file : "Documentation/git-\(.*\).txt")
	cmds="$cmds $cmd"
	echo "	git\ $cmd\ *)"
	echo '		case "$3" in'

	# get all lines which begin with '-' or '<' and end with '::',
	# because they likely contain a command line option
	sed -n "s/^\([-<].*\)::/\1/p" < $file | { \
		opts=""
		while read line; do
			# some lines have the form '-a|--all'
			opt1=$(expr "$line" : '^\([^ |<]*\)')
			opt2=$(expr "$line" : '|\([^ |<]*\)')
			opts="$opts $opt1 $opt2"
			# this is an example how to handle non-simple args:
			# '-m <msg>' means: if the second-last arg is '-m',
			# the current arg is the message.
			test "$(expr "$line" : ".*<msg>")" != 0 &&
				echo "		'$opt1')" &&
				echo '			COMPREPLY=(\"a \"z);;'
		done
		echo "		*)"
		echo "			COMPREPLY=(\$(compgen -W \"$opts\" -- \$2));;"
	}
	echo "		esac;;"
done

cat << EOF
	git\ *\ *)
		COMPREPLY=();;
	git\ *)
		COMPREPLY=(\$(compgen -W '$cmds' -- \$2));;
	esac
}

complete -o default -F gitcomplete git
EOF
--- cut ---

^ permalink raw reply

* Re: [RFC/PATCH] pager: do not fork a pager if environment variable PAGER is set to NONE
From: Johannes Schindelin @ 2006-04-16  2:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwtdqef6u.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sat, 15 Apr 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > This helps debugging tremendously.
> >
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> I like what this wants to do.  I am not so sure PAGER=NONE is a
> good convention, however.

I am sure it is not.

One solution would be to introduce yet another command line option 
"--no-pager", but I find that ugly.

Another solution would be to check if the environment variable NO_PAGER is 
set.

Wishes?

Ciao,
Dscho

^ permalink raw reply

* Re: git-svn and Author files question
From: Seth Falcon @ 2006-04-16  1:57 UTC (permalink / raw)
  To: git
In-Reply-To: <20060415215850.GB20468@hand.yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:
> There were some embarassing bugs between the git-svn in rc1 and rc2.
> Current versions should work.  Rutger was right about the file format,
> same as the other importers.

Thanks for the explanation (and thanks to Rutger as well for the
reply!).

I managed to get git-svn fetch to work by specifying an author file
with -A.

I'm a bit confused about why I have to do this.  Is there a way around
this?  Or perhaps a way to force a bogus email address based on svn
user name?

[ok, maybe I'm less confused than I'm letting on: the email
requirement is, I think, because that's what suits the main git
"customers".  If I was using git for a project's primary SCM, I would
have no problem with this.  In fact, we started using email addresses
as user names in svn a long time ago.  

But it seems to me that (1) using git to track a project that uses a
less featureful SCM is way cool, and (2) having to manually muck with
an authors file is kinda uncool.]

> Sorry about so long to reply to questions this week, left hand/wrist is
> wrecked.

Hope it heals quickly!


Thanks for listening and again for the help,

+ seth

^ permalink raw reply

* Re: Support "git cmd --help" syntax
From: Junio C Hamano @ 2006-04-16  1:24 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604151402470.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> I'd have expected that to be an improvement. You can always get the usage 
> message with "git commit --huh?", so it's not like you've lost anything.

Fair enough.  I'd say I am convinced now.

^ permalink raw reply

* Re: git-svn and Author files question
From: Eric Wong @ 2006-04-15 21:58 UTC (permalink / raw)
  To: Seth Falcon; +Cc: git
In-Reply-To: <m21wvzx5e6.fsf@ziti.fhcrc.org>

Seth Falcon <sethfalcon@gmail.com> wrote:
> Hi all,
> 
> I've been using git to manually track changes to a project that uses
> svn as its primary SCM.
> 
> git-svn looks like it can help me streamline my workflow, but I'm
> getting stuck with the following:
> 
>     mkdir foo
>     cd foo
>     git-svn init $URL  <--- the svn URL
>     git-svn fetch
>     Author: dfcimm3 not defined in  file
> 
> :-(
> 
> Can someone point me to the file and the place that describes what I
> should put in it?  There are many committers to the svn project.  I'm
> hoping that I will not have to enumerate all of their names in some
> file.
> 
> I'm using git version 1.3.0.rc1.g40e9, and BTW, enjoying it very much.

There were some embarassing bugs between the git-svn in rc1 and rc2.
Current versions should work.  Rutger was right about the file format,
same as the other importers.

Sorry about so long to reply to questions this week, left hand/wrist is
wrecked.

@Pavel, Junio: I should have doc updates coming in hopefully in the next
few days, unless anybody wants to help :)

-- 
Eric Wong

^ permalink raw reply

* [PATCH] diff --stat: do not do its own three-dashes.
From: Junio C Hamano @ 2006-04-15 21:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

I missed that "git-diff-* --stat" spits out three-dash separator
on its own without being asked.  Remove it.

When we output commit log followed by diff, perhaps --patch-with-stat,
for downstream consumer, we _would_ want the three-dash between
the message and the diff material, but that logic belongs to the
caller, not diff generator.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---
 * Now the next task is to make "git log" not to do its own
   commit formatting, but let the log_tree_commit() do it, just
   like we do in diff-tree --pretty.  This would open the door
   for us to (optionally) omit log message when there is no
   diff, and honor the --max-count by counting what we actually
   output.

   When that happens, the printf() I moved tentatively to
   cmd_log() should become part of the commit formatting
   log_tree_commit() does, perhaps with --pretty=format-patch.

 diff.c |    2 --
 git.c  |    4 +++-
 2 files changed, 3 insertions(+), 3 deletions(-)

6f4780f9dfd3bc6b23f9ea66b3d49577e0a0c2f9
diff --git a/diff.c b/diff.c
index f1b672d..cda8d20 100644
--- a/diff.c
+++ b/diff.c
@@ -245,8 +245,6 @@ static void show_stats(struct diffstat_t
 	if (data->nr == 0)
 		return;
 
-	printf("---\n");
-
 	for (i = 0; i < data->nr; i++) {
 		struct diffstat_file *file = data->files[i];
 
diff --git a/git.c b/git.c
index 78ed403..61265a8 100644
--- a/git.c
+++ b/git.c
@@ -388,8 +388,10 @@ static int cmd_log(int argc, const char 
 		pretty_print_commit(commit_format, commit, ~0, buf,
 				    LOGSIZE, abbrev);
 		printf("%s\n", buf);
-		if (do_diff)
+		if (do_diff) {
+			printf("---\n");
 			log_tree_commit(&opt, commit);
+		}
 		shown = 1;
 		free(commit->buffer);
 		commit->buffer = NULL;
-- 
1.3.0.rc4.g5a48-dirty

^ permalink raw reply related

* Re: Support "git cmd --help" syntax
From: Linus Torvalds @ 2006-04-15 21:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsloeef0k.fsf@assigned-by-dhcp.cox.net>



On Sat, 15 Apr 2006, Junio C Hamano wrote:
> 
> Tried "git commit --help" with and without the patch?

I get a usage message without the patch.

With the patch, I get the man-page.

IOW, I don't see your point.

I'd have expected that to be an improvement. You can always get the usage 
message with "git commit --huh?", so it's not like you've lost anything.

Now, if you have _not_ done

	make prefix=/usr/share install-doc

or similar, then yes, you get something less than helpful (along the lines 
of "No manual entry for git-commit"), and I agree that we could/should 
improve on that somehow. 

			Linus

^ permalink raw reply

* Re: Tentative built-in "git show"
From: Linus Torvalds @ 2006-04-15 20:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3bgefxkp.fsf@assigned-by-dhcp.cox.net>



On Sat, 15 Apr 2006, Junio C Hamano wrote:
> 
> I sometimes do "git show -4" myself, and wondered why defaulting
> to "-n 1"is insufficient

I detest -n1 for a variety of reasons, and giving multiple refs is just 
one of them.

The reason I like "--no-walk" is that it's conceptually much stronger, and 
more efficient. Remember how we tried to optimize "-1" (as used by gitweb) 
by adding magic special cases to get_revision()? And how they always ended
up being problematic because of how they interact with all the other 
options?

So we don't do it any more, and as a result, "git-rev-list -1 HEAD" will 
actually walk all the parents too.

In contrast, "--no-walk" just does exactly what you tell it: don't start 
walking the parents. It's kind of equivalent to -1 with one argument, but 
in the presense of path limiting it actually does the RightThing(tm), 
unlike -1.

Try this as a replacement for "git show":

	git log -1 kernel/
	git log --no-walk kernel/

on the current kernel source tree to see the difference. "--no-walk" gives 
the right answer (for some definition of "right"). While -1 gives a 
totally random answer that has nothing to do with the current HEAD (except 
that it's _reachable_ from the current HEAD).

So I think "-n 1" is fundamentally incompatible with "git show". Git 
"show" is "show _this_ commit". While "-1" fundamentally means "show the 
_first_ commit when you walk the tree". Which is really really 
fundamentally different.

In contrast "--no-walk" tells you exactly what it is about. Don't walk the 
tree. Show _this_ commit.

(Now, the reason I said 'for _some_ definition of "right"' is that the 
question about path limiting in the absense of commit walking is somewhat 
ambiguous. The current "git show" shows the commit regardless. My 
suggested patch will potentially prune the commit away, and if the 
specified commit does not touch the path, no commit is shown at all. 
Both make sense. While "git-rev-list -1" does _not_ make sense)

			Linus

^ permalink raw reply

* Re: Support "git cmd --help" syntax
From: Junio C Hamano @ 2006-04-15 20:54 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604151054380.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> However, as anybody who has ever used CVS or some similar devil-spawn 
> program, it's confusing as h*ll when options before the sub-command act 
> differently from options after the sub-command, so this quick hack just 
> makes it acceptable to do "git cmd --help" instead, and get the exact same 
> result.
>
> It may be hacky, but it's simple and does the trick.

Tried "git commit --help" with and without the patch?

I am not sure if we want to "help" migrants from CVS or some
similar program by shooting ourselves in the foot.

^ permalink raw reply

* Re: [RFC/PATCH] pager: do not fork a pager if environment variable PAGER is set to NONE
From: Junio C Hamano @ 2006-04-15 20:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604151516150.6563@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> This helps debugging tremendously.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

I like what this wants to do.  I am not so sure PAGER=NONE is a
good convention, however.

^ permalink raw reply

* Re: Tentative built-in "git show"
From: Junio C Hamano @ 2006-04-15 19:28 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604151203060.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> This uses the "--no-walk" flag that I never actually implemented (but I'm 
> sure I mentioned it) to make "git show" be essentially the same thing as 
> "git whatchanged --no-walk".

I sometimes do "git show -4" myself, and wondered why defaulting
to "-n 1"is insufficient, but if you do something like this to
check the tip of each branch:

	git show master next pu

this may make sense.

Otherwise, it feels to me that (I haven't had caffeine nor
nicotine yet, so I need to re-think this later) log, show and
whatchanged are the same program in disguise with different set
of defaults.

        log		--pretty
        show		--pretty -n1 -p
        whatchanged	--pretty -r

... which implies that some of them might become historical
curiosity and no real reason to teach about each of them in the
tutorial.

^ permalink raw reply

* Re: Git web broken, or Linus' kernel tree in odd state
From: Junio C Hamano @ 2006-04-15 19:15 UTC (permalink / raw)
  To: Tony Luck; +Cc: git, Linus Torvalds
In-Reply-To: <12c511ca0604151155j40c68a21qf684f77d2476605b@mail.gmail.com>

"Tony Luck" <tony.luck@intel.com> writes:

> Git web is showing the "master" tag four commits back from HEAD on
> the summary page for Linus' kernel tree, which is unusual (isn't it?).

I suspect it is related to this:

http://article.gmane.org/gmane.comp.version-control.git/14366

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox