Re: [PATCH] Rewritten fetch-pack.c:filter_refs() using starts_with() instead of memcmp()

[Eric Sunshine]

Thanks for the submission. Comments below to give you a taste of the
Git review process...

On Thu, Mar 20, 2014 at 6:04 AM, MustafaOrkunAcar
mustafaorkuna...@gmail.com wrote:
 Subject: Rewritten fetch-pack.c:filter_refs() using starts_with() instead of 
 memcmp()

Use imperative mood: Rewrite rather than Rewritten. Mention the
module or function you're touching at the start of the subject,
followed by a colon and space. For example:

Subject: filter_refs: replace memcmp() with starts_with()

 Hi, I have completed one of the microprojects -14th one: Change 
 fetch-pack.c:filter_refs() to use starts_with() instead of memcmp(). The 
 only line in the function filter_refs() containing memcmp() is changed with 
 starts_with(). I plan to apply for GSoC 2014. Any feedback is appreciated. 
 Thanks.

Wrap text to 65-70 characters.

This area above your sign-off is where you should explain the purpose
of the patch and justify the change. For a small one like this, you
shouldn't need more than one or two simple sentences.

 Signed-off-by: MustafaOrkunAcar mustafaorkuna...@gmail.com
 ---

This area below the --- line under your sign-off is for commentary
which won't likely be relevant to someone looking at the patch in the
project history months or years from now. Everything you wrote above
about GSoC and only one instance of memcmp() belongs here.

The patch itself looks reasonable. As suggested by the microproject,
were you able to find any other places in the project which could
benefit likewise? If so, perhaps include a few of them when you
resubmit.

  fetch-pack.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/fetch-pack.c b/fetch-pack.c
 index f061f1f..17823ab 100644
 --- a/fetch-pack.c
 +++ b/fetch-pack.c
 @@ -506,7 +506,7 @@ static void filter_refs(struct fetch_pack_args *args,
 int keep = 0;
 next = ref-next;

 -   if (!memcmp(ref-name, refs/, 5) 
 +   if (starts_with(ref-name, refs/) 
 check_refname_format(ref-name, 0))
 ; /* trash */
 else {
 --
 1.9.1.286.g5172cb3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] builtin/apply.c: fuzzy_matchlines:trying to fix some inefficiencies

[Jeff King]

On Thu, Mar 20, 2014 at 11:35:03AM +0200, George Papanikolaou wrote:

 Hi again guys,
 I forgot to add the signed-of line to the tiny patch I sent earlier for GSOC.
 Any ideas about the changes?
 Thanks...

You don't give any detail on the inefficiencies, or what specific
benchmark is made faster. Have you done any timings to show that there
is a measurable improvement? If so, can you share them in the commit
message?

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 03/28] Added test case for guilt delete -f.

[Per Cederqvist]

Ensure that the file really is deleted.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-026.out | 15 +++
 regression/t-026.sh  |  5 -
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/regression/t-026.out b/regression/t-026.out
index 3b9fb14..be50b48 100644
--- a/regression/t-026.out
+++ b/regression/t-026.out
@@ -29,3 +29,18 @@ f 7848194fd2e9ee0eb6589482900687d799d60a12  
.git/patches/master/series
 f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
 f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
 f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/status
+% guilt new delete-me
+% guilt pop
+All patches popped.
+% guilt delete -f delete-me
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 7848194fd2e9ee0eb6589482900687d799d60a12  .git/patches/master/series
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/status
diff --git a/regression/t-026.sh b/regression/t-026.sh
index 0ccdf85..e25d0df 100755
--- a/regression/t-026.sh
+++ b/regression/t-026.sh
@@ -20,4 +20,7 @@ cmd guilt delete add
 
 cmd list_files
 
-# FIXME: test delete -f
+cmd guilt new delete-me
+cmd guilt pop
+cmd guilt delete -f delete-me
+cmd list_files
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 00/28] Teach guilt import-commit how create legal patch names, and more

[Per Cederqvist]

I recently found myself sitting on a train with a computer in front of
me.  I tried to use guilt import-commit, which seemed to work, but
when I tried to guilt push the commits I had just imported I got
some errors.  It turned out that guilt import-commit had generated
invalid patch names.

I decided to fix the issue, and write a test case that demonstrated
the problem.

One thing led to another, and here I am, a few late nights at a hotel
and a return trip on the train later, submitting a patch series in 28
parts.  Sorry about the number of patches, but this is what happens
when you uncover a bug while writing a test case for the bug you
uncovered while writing a test case for your original problem.

The patch series consists of:

 - A number of fixes to the test suite.

 - A number of bug fixes which I hope are non-controversial.  Most of
   the fixes have test cases.

 - Changed behavior: guilt push when there is nothing more to push
   now uses exit status 1.  This makes it possible to write shell
   loops such as while guilt push; do make test||break; done.  Also,
   guilt pop when no patches are applied also uses exit status 1.
   (This aligns guilt push and guilt pop with how hg qpush and
   hg qpop has worked for several years.)

 - Changed behavior: by default, guilt no longer changes branch when
   you push a patch.  You need to do git config guilt.reusebranch
   false to re-enable that.  This patch sets the default value of
   guilt.reusebranch to true; it should in my opinion change to false
   a year or two after the next release.

A more detailed overview of the patches:

1. Some tests fail if git config guilt.diffstat true is in effect.

2-4. Some commands fail if run from a directory with spaces in its
 name.

5. guilt new had an overly restrictive argument parser.

6-8. guilt.diffstat could break guilt fold and guilt new.

9-10. The test suite did not test exit status properly.

11. Remove pointless redirections in the test suite.

12-13. guilt header tried to check if a patch existed, but the check
was broken.

14-16. Various parts of guilt tried to ensure that patch names were
   legal git branch names, but failed.

17-20. guilt graph failed when no patch was applied, and when a
   branch name contained a comma or a quote.

21-23. git config log.decorate short caused guilt import-commit,
   guilt patchbomb and guilt rebase to fail in various ways.

24. Patches may contain backslashes, but various informative messages
from guilt did backslash processing.

25-26. guilt push and guilt pop should fail when there is nothing
   to do.

27-28. These two commits were things I intended to contribute a while
   back, when contributing the Change git branch when patches are
   applied change (commit 67d3af63f422).  These commits makes
   that behavior optional, and it defaults to being disabled, so
   that you can use both Guilt v0.35 (and earlier) and the current
   Guilt code against the same repo.  These commits add some code
   complexity, and you might want to skip them if you think the
   current behavior is better.

This patch series is also available on
http://repo.or.cz/w/guilt/ceder.git in the oslo-2014 branch.  If you
already have a copy of guilt, you should be able to fetch that branch
with something like:

git remote add ceder http://repo.or.cz/r/guilt/ceder.git
git fetch ceder refs/heads/oslo-2014:refs/remotes/ceder/oslo-2014

A few of the regression/t-*.out files contain non-ASCII characters.  I
hope they survive the mail transfer; if not, please use the repo above
to fetch the commits.


Per Cederqvist (28):
  The tests should not fail if guilt.diffstat is set.
  Allow guilt delete -f to run from a dir which contains spaces.
  Added test case for guilt delete -f.
  Allow guilt import-commit to run from a dir which contains spaces.
  guilt new: Accept more than 4 arguments.
  Fix and simplify the do_get_patch function.
  Added test cases for guilt fold.
  Added more test cases for guilt new: empty patches.
  Test suite: properly check the exit status of commands.
  Run test_failed if the exit status of a test script is bad.
  test suite: remove pointless redirection.
  guilt header: more robust header selection.
  Check that guilt header '.*' fails.
  Use git check-ref-format to validate patch names.
  Produce legal patch names in guilt-import-commit.
  Fix backslash handling when creating names of imported patches.
  guilt graph no longer loops when no patches are applied.
  guilt-graph: Handle commas in branch names.
  Check that guilt graph works when working on a branch with a comma.
  guilt graph: Handle patch names containing quotes.
  The log.decorate setting should not influence import-commit.
  The log.decorate setting should not influence patchbomb.
  The log.decorate setting should not influence guilt rebase.
  disp no longer processes backslashes.
  guilt push now fails when there are no more patches to 

[GUILT 02/28] Allow guilt delete -f to run from a dir which contains spaces.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-delete | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guilt-delete b/guilt-delete
index 3e394f8..967ac10 100755
--- a/guilt-delete
+++ b/guilt-delete
@@ -49,7 +49,7 @@ series_remove_patch $patch
 
 guilt_hook delete $patch
 
-[ ! -z $force ]  rm -f $GUILT_DIR/$branch/$patch
+[ ! -z $force ]  rm -f $GUILT_DIR/$branch/$patch
 
 exit 0
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 26/28] guilt pop now fails when there are no more patches to pop.

[Per Cederqvist]

This is analogous to how guilt push now fails when there are no more
patches to push.  Like push, the --all argument still succeeds even
if there was no need to pop anything.

Updated the test suite.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-pop| 13 +++--
 regression/t-021.out |  2 ++
 regression/t-021.sh  |  6 ++
 regression/t-061.sh  |  7 ++-
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/guilt-pop b/guilt-pop
index f0e647f..208f868 100755
--- a/guilt-pop
+++ b/guilt-pop
@@ -48,10 +48,16 @@ fi
 
 patch=$1
 [ ! -z $all ]  patch=-a
+[ -z $patch ]  { patch=1; num=t; }
 
 if [ ! -s $applied ]; then
disp No patches applied.
-   exit 0
+   if [ $patch = -a ]
+   then
+   exit 0
+   else
+   exit 1
+   fi
 elif [ $patch = -a ]; then
# we are supposed to pop all patches
 
@@ -68,11 +74,6 @@ elif [ ! -z $num ]; then
# catch underflow
[ $eidx -lt 0 ]  eidx=0
[ $eidx -eq $sidx ]  die No patches requested to be removed.
-elif [ -z $patch ]; then
-   # we are supposed to pop only the current patch on the stack
-
-   sidx=`wc -l  $applied`
-   eidx=`expr $sidx - 1`
 else
# we're supposed to pop only up to a patch, make sure the patch is
# in the series
diff --git a/regression/t-021.out b/regression/t-021.out
index 9b42d9c..58be12f 100644
--- a/regression/t-021.out
+++ b/regression/t-021.out
@@ -287,6 +287,8 @@ index 000..8baef1b
 +++ b/def
 @@ -0,0 +1 @@
 +abc
+% guilt pop
+No patches applied.
 % guilt push --all
 Applying patch..modify
 Patch applied.
diff --git a/regression/t-021.sh b/regression/t-021.sh
index 614e870..e0d2dc1 100755
--- a/regression/t-021.sh
+++ b/regression/t-021.sh
@@ -23,6 +23,12 @@ guilt series | _tac | while read n ; do
 done
 
 #
+# pop when there is nothing to pop
+#
+
+shouldfail guilt pop
+
+#
 # push all
 #
 cmd guilt push --all
diff --git a/regression/t-061.sh b/regression/t-061.sh
index 1411baa..a9a4fea 100755
--- a/regression/t-061.sh
+++ b/regression/t-061.sh
@@ -48,7 +48,12 @@ cmd list_files
 
 for i in `seq 5`
 do
-   cmd guilt pop
+   if [ $i -ge 5 ]
+   then
+   shouldfail guilt pop
+   else
+   cmd guilt pop
+   fi
cmd git for-each-ref
cmd guilt push
cmd git for-each-ref
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 21/28] The log.decorate setting should not influence import-commit.

[Per Cederqvist]

Use --no-decorate in the call to git log that tries to read the commit
message to produce patch names.  Otherwise, if the user has set
log.decorate to short or full, the patch name will be less useful.

Modify the t-034.sh test case to demonstrate that this is needed.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-import-commit  | 2 +-
 regression/t-034.out | 2 ++
 regression/t-034.sh  | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/guilt-import-commit b/guilt-import-commit
index 6eb2f4e..703f034 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -26,7 +26,7 @@ disp About to begin conversion... 2
 disp Current head: `cat \$GIT_DIR\/refs/heads/\`git_branch\`` 2
 
 for rev in `git rev-list $rhash`; do
-   s=`git log --pretty=oneline -1 $rev | cut -c 42-`
+   s=`git log --no-decorate --pretty=oneline -1 $rev | cut -c 42-`
 
# Try to convert the first line of the commit message to a
# valid patch name.
diff --git a/regression/t-034.out b/regression/t-034.out
index bda4399..5d81bd4 100644
--- a/regression/t-034.out
+++ b/regression/t-034.out
@@ -232,6 +232,7 @@ Date:   Mon Jan 1 00:00:00 2007 +
 
 Signed-off-by: Commiter Name commiter@email
 % guilt init
+% git config log.decorate short
 % guilt import-commit base..HEAD
 About to begin conversion...
 Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541
@@ -259,6 +260,7 @@ Converting 45e81b51 as the_sequence_.lock-_is_forbidden
 Converting eebb76e9 as the_sequence_-._is_forbidden
 Done.
 Current head: d4850419ccc1146c7169f500725ce504b9774ed0
+% git config log.decorate no
 % guilt push -a
 Applying patch..the_sequence_-._is_forbidden.patch
 Patch applied.
diff --git a/regression/t-034.sh b/regression/t-034.sh
index 1055ddb..8179bc7 100755
--- a/regression/t-034.sh
+++ b/regression/t-034.sh
@@ -57,7 +57,9 @@ cmd git log
 
 # Import all the commits to guilt.
 cmd guilt init
+cmd git config log.decorate short
 cmd guilt import-commit base..HEAD
+cmd git config log.decorate no
 
 for patch in .git/patches/master/*.patch
 do
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 15/28] Produce legal patch names in guilt-import-commit.

[Per Cederqvist]

Try harder to create patch names that adhere to the rules in
git-check-ref-format(1) when deriving a patch name from the commit
message.  Verify that the derived name using git check-ref-format,
and as a final fallback simply use the patch name x (to ensure that
the code is future-proof in case new rules are added in the future).

Always append a .patch suffix to the patch name.

Added test cases.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-import-commit  |  22 +-
 regression/t-034.out | 567 +++
 regression/t-034.sh  |  72 +++
 3 files changed, 659 insertions(+), 2 deletions(-)
 create mode 100644 regression/t-034.out
 create mode 100755 regression/t-034.sh

diff --git a/guilt-import-commit b/guilt-import-commit
index 9488ded..a4119d6 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -28,19 +28,37 @@ disp Current head: `cat 
\$GIT_DIR\/refs/heads/\`git_branch\`` 2
 for rev in `git rev-list $rhash`; do
s=`git log --pretty=oneline -1 $rev | cut -c 42-`
 
+   # Try to convert the first line of the commit message to a
+   # valid patch name.
fname=`echo $s | sed -e s//and/g -e s/[ :]/_/g -e s,[/\\],-,g \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
-   -e 's/\?/-/g' | tr A-Z a-z`
+   -e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
+   -e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
+
+   if ! valid_patchname $fname
+   then
+   # Try harder to make it a legal commit name by
+   # removing all but a few safe characters.
+   fname=`echo $fname|tr -d -c _a-zA-Z0-9---/\\n`
+   fi
+   if ! valid_patchname $fname
+   then
+   # If we failed to derive a legal patch name, use the
+   # name x.  (If this happens, we likely have to
+   # append a suffix to make the name unique.)
+   fname=x
+   fi
 
disp Converting `echo $rev | cut -c 1-8` as $fname
 
mangle_prefix=1
fname_base=$fname
-   while [ -f $GUILT_DIR/$branch/$fname ]; do
+   while [ -f $GUILT_DIR/$branch/$fname.patch ]; do
fname=$fname_base-$mangle_prefix
mangle_prefix=`expr $mangle_prefix + 1`
disp Patch under that name exists...trying '$fname'
done
+   fname=$fname.patch
 
(
do_make_header $rev
diff --git a/regression/t-034.out b/regression/t-034.out
new file mode 100644
index 000..7bc9459
--- /dev/null
+++ b/regression/t-034.out
@@ -0,0 +1,567 @@
+% setup_git_repo
+% git tag base
+% create_commit a The sequence /. is forbidden.
+[master eebb76e] The sequence /. is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+ create mode 100644 a
+% create_commit a The sequence .lock/ is forbidden.
+[master 45e81b5] The sequence .lock/ is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a A/component/may/not/end/in/foo.lock
+[master bbf3f59] A/component/may/not/end/in/foo.lock
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Two consecutive dots (..) is forbidden.
+[master 1535e67] Two consecutive dots (..) is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Check/multiple/../dots/./foo..patch
+[master 48eb60c] Check/multiple/../dots/./foo..patch
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Space is forbidden.
+[master 10dea83] Space is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Tilde~is~forbidden.
+[master 70a83b7] Tilde~is~forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Caret^is^forbidden.
+[master ee6ef2c] Caret^is^forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Colon:is:forbidden.
+[master c077fe2] Colon:is:forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Delisforbidden.
+[master 589ee30] Delisforbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% git branch some-branch
+% git tag some-tag
+% create_commit a Ctrl
isforbidden.
+[master e63cdde] Ctrl
isforbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a CR
is
also
forbidden.
+[master 21ad093] CR
is
also
forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Question-mark?is?forbidden.
+[master be2fa9b] Question-mark?is?forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Asterisk*is*forbidden.
+[master af7b50f] Asterisk*is*forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a 

[GUILT 20/28] guilt graph: Handle patch names containing quotes.

[Per Cederqvist]

Quote quotes with a backslash in the guitl graph output.  Otherwise,
the dot file could contain syntax errors.

Added a test case.
---
 guilt-graph  |  2 ++
 regression/t-033.out | 22 ++
 regression/t-033.sh  |  9 +
 3 files changed, 33 insertions(+)

diff --git a/guilt-graph b/guilt-graph
index 575f03b..24ab83b 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -58,6 +58,8 @@ while [ $current != $base ]; do
 }`
[ -z $pname ]  pname=?
 
+   pname=`printf \%s\ $pname|sed 's/\/\/g'`
+
disp # checking rev $current
disp   \$current\ [label=\$pname\]
 
diff --git a/regression/t-033.out b/regression/t-033.out
index e638d7b..1c28ea9 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -63,3 +63,25 @@ digraph G {
ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
891bc14b5603474c9743fd04f3da888644413dc5 - 
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
 }
+% guilt new a-betterquicker'-patch.patch
+% git add file.txt
+% guilt refresh
+Patch a-betterquicker'-patch.patch refreshed
+% guilt pop
+Now at c.patch.
+% guilt push
+Applying patch..a-betterquicker'-patch.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev bc7df666a646739eaf559af23cab72f2bfd01f0e
+   bc7df666a646739eaf559af23cab72f2bfd01f0e 
[label=a-\betterquicker'-patch.patch]
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+   891bc14b5603474c9743fd04f3da888644413dc5 [label=c.patch]
+   bc7df666a646739eaf559af23cab72f2bfd01f0e - 
891bc14b5603474c9743fd04f3da888644413dc5; // ?
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+   c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+   ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+   891bc14b5603474c9743fd04f3da888644413dc5 - 
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index 57dce78..968292c 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -46,3 +46,12 @@ cmd git add file.txt
 cmd guilt refresh
 fixup_time_info c.patch
 cmd guilt graph
+
+# A patch name that contains funky characters, including unbalanced
+# quotes.
+cmd guilt new a-\betterquicker'-patch.patch
+cmd echo d  file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info a-\betterquicker'-patch.patch
+cmd guilt graph
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 06/28] Fix and simplify the do_get_patch function.

[Per Cederqvist]

When extracting the patch, we only want the actual patches.  We don't
want the --- delimiter.  Simplify the extraction by simply deleting
everything before the first diff  line.  (Use sed instead of awk to
simplify the code.)

Without this patch, guilt fold and guilt push sometimes fails if
guilt.diffstat is true.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/guilt b/guilt
index 8701481..c59cd0f 100755
--- a/guilt
+++ b/guilt
@@ -332,12 +332,7 @@ do_make_header()
 # usage: do_get_patch patchfile
 do_get_patch()
 {
-   awk '
-BEGIN{}
-/^(diff |---$|--- )/ {patch = 1}
-patch == 1 {print $0}
-END{}
-'  $1
+   sed -n '/^diff /,$p'  $1
 }
 
 # usage: do_get_header patchfile
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 24/28] disp no longer processes backslashes.

[Per Cederqvist]

Only one invocation of disp or _disp actually needed backslash
processing.  In quite a few instances, it was wrong to do backslash
processing, as the message contained data derived from the user.

Created the new function disp_e that should be used when backslash
processing is required, and changed disp and disp_ to use printf
code %s instead of %b.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/guilt b/guilt
index ca922aa..36cfd1e 100755
--- a/guilt
+++ b/guilt
@@ -36,15 +36,24 @@ usage()
exit 1
 }
 
-# echo -n is a bashism, use printf instead
+# Print arguments, but no trailing newline.
+# (echo -n is a bashism, use printf instead)
 _disp()
 {
-   printf %b $*
+   printf %s $*
 }
 
-# echo -e is a bashism, use printf instead
+# Print arguments.
+# (echo -E is a bashism, use printf instead)
 disp()
 {
+   printf %s\n $*
+}
+
+# Print arguments, processing backslash sequences.
+# (echo -e is a bashism, use printf instead)
+disp_e()
+{
printf %b\n $*
 }
 
@@ -117,7 +126,7 @@ else
 
disp 
disp Example:
-   disp \tguilt push
+   disp_e \tguilt push
 
# now, let's exit
exit 1
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 11/28] test suite: remove pointless redirection.

[Per Cederqvist]

The shouldfail function already redirects stderr to stdout, so there
is no need to do the same in t-028.sh and t-021.sh.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-021.sh | 2 +-
 regression/t-025.sh | 2 +-
 regression/t-028.sh | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/regression/t-021.sh b/regression/t-021.sh
index 6337d7b..614e870 100755
--- a/regression/t-021.sh
+++ b/regression/t-021.sh
@@ -61,7 +61,7 @@ for n in `_seq -2 $npatches`; do
if [ $n -gt 0 ]; then
cmd guilt pop -n $n
else
-   shouldfail guilt pop -n $n 21
+   shouldfail guilt pop -n $n
fi
 
cmd list_files
diff --git a/regression/t-025.sh b/regression/t-025.sh
index 3824608..985fed4 100755
--- a/regression/t-025.sh
+++ b/regression/t-025.sh
@@ -44,7 +44,7 @@ shouldfail guilt new white space
 cmd list_files
 
 for pname in prepend mode /abc ./blah ../blah abc/./blah abc/../blah abc/. 
abc/.. abc/ ; do
-   shouldfail guilt new $pname 21
+   shouldfail guilt new $pname
 
cmd list_files
 done
diff --git a/regression/t-028.sh b/regression/t-028.sh
index 8480100..88e9adb 100755
--- a/regression/t-028.sh
+++ b/regression/t-028.sh
@@ -29,6 +29,6 @@ guilt series | while read n; do
cmd guilt header $n
 done
 
-shouldfail guilt header non-existant 21
+shouldfail guilt header non-existant
 
 # FIXME: how do we check that -e works?
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 19/28] Check that guilt graph works when working on a branch with a comma.

[Per Cederqvist]

git branch names can contain commas.  Check that guilt graph works
even in that case.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-033.out | 62 
 regression/t-033.sh  | 37 +++
 2 files changed, 99 insertions(+)

diff --git a/regression/t-033.out b/regression/t-033.out
index 76613f9..e638d7b 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -1,3 +1,65 @@
 % setup_repo
 % guilt graph
 No patch applied.
+% git checkout -b a,graph master
+Switched to a new branch 'a,graph'
+% guilt init
+% guilt new a.patch
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..a.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev 95275d7c05c6a6176d3941374115b91272877f6c
+   95275d7c05c6a6176d3941374115b91272877f6c [label=a.patch]
+}
+% git add file.txt
+% guilt refresh
+Patch a.patch refreshed
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..a.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+   ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+}
+% guilt new b.patch
+% git add file2.txt
+% guilt refresh
+Patch b.patch refreshed
+% guilt pop
+Now at a.patch.
+% guilt push
+Applying patch..b.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+   c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+   ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+}
+% guilt new c.patch
+% git add file.txt
+% guilt refresh
+Patch c.patch refreshed
+% guilt pop
+Now at b.patch.
+% guilt push
+Applying patch..c.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+   891bc14b5603474c9743fd04f3da888644413dc5 [label=c.patch]
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+   c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+   ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+   891bc14b5603474c9743fd04f3da888644413dc5 - 
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index ae40577..57dce78 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -3,9 +3,46 @@
 # Test the graph code
 #
 
+function fixup_time_info
+{
+   cmd guilt pop
+   touch -a -m -t $TOUCH_DATE .git/patches/a,graph/$1
+   cmd guilt push
+}
+
 source $REG_DIR/scaffold
 
 cmd setup_repo
 
 shouldfail guilt graph
 
+cmd git checkout -b a,graph master
+
+cmd guilt init
+
+cmd guilt new a.patch
+
+fixup_time_info a.patch
+cmd guilt graph
+
+cmd echo a  file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info a.patch
+cmd guilt graph
+
+# An unrelated file. No deps.
+cmd guilt new b.patch
+cmd echo b  file2.txt
+cmd git add file2.txt
+cmd guilt refresh
+fixup_time_info b.patch
+cmd guilt graph
+
+# An change to an old file. Should add a dependency.
+cmd guilt new c.patch
+cmd echo c  file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info c.patch
+cmd guilt graph
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 18/28] guilt-graph: Handle commas in branch names.

[Per Cederqvist]

This fix relies on the fact that git branch names can not contain :.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-graph | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guilt-graph b/guilt-graph
index 00301d5..575f03b 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -52,7 +52,7 @@ safebranch=`echo $branch|sed 's%/%/%g'`
 while [ $current != $base ]; do
pname=`git show-ref | sed -n -e 
 /^$current refs\/patches\/$safebranch/ {
-   s,^$current refs/patches/$branch/,,
+   s:^$current refs/patches/$branch/::
p
q
 }`
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 22/28] The log.decorate setting should not influence patchbomb.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-patchbomb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guilt-patchbomb b/guilt-patchbomb
index 1231418..164b10c 100755
--- a/guilt-patchbomb
+++ b/guilt-patchbomb
@@ -47,7 +47,7 @@ if [ $? -ne 0 ]; then
 fi
 
 # display the list of commits to be sent as patches
-git log --pretty=oneline $r | cut -c 1-8,41- | $pager
+git log --no-decorate --pretty=oneline $r | cut -c 1-8,41- | $pager
 
 _disp Are these what you want to send? [Y/n] 
 read n
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 07/28] Added test cases for guilt fold.

[Per Cederqvist]

Test that we can combine any combination of patches with empty and
non-empty messages, both with and without guilt.diffstat.  (All
patches are empty.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-035.out | 659 +++
 regression/t-035.sh  |  88 +++
 2 files changed, 747 insertions(+)
 create mode 100644 regression/t-035.out
 create mode 100755 regression/t-035.sh

diff --git a/regression/t-035.out b/regression/t-035.out
new file mode 100644
index 000..04af146
--- /dev/null
+++ b/regression/t-035.out
@@ -0,0 +1,659 @@
+% setup_repo
+% git config guilt.diffstat true
+%% empty + empty (diffstat=true)
+% guilt new empty-1
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty-1
+Patch applied.
+% guilt new empty-2
+% guilt pop
+Now at empty-1.
+% guilt push
+Applying patch..empty-2
+Patch applied.
+% guilt pop
+Now at empty-1.
+% guilt fold empty-2
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty-1
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 4ea806e306f0228a8ef41f186035e7b04097f1f2  .git/patches/master/status
+f 7d261b8caad0f161c21daf5de65eeb521ff8c067  .git/patches/master/empty-1
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f d28d87b88c1e24d637e390dc3603cfa7c1715711  .git/patches/master/series
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-1~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-2~
+r bde3d337af70f36836ad606c800d194006f883b3  .git/refs/patches/master/empty-1
+% git log -p
+commit bde3d337af70f36836ad606c800d194006f883b3
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch empty-1
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
+% guilt pop
+All patches popped.
+% guilt delete -f empty-1
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-1~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-2~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/status
+%% empty + non-empty (diffstat=true)
+% guilt new empty
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty
+Patch applied.
+% guilt new -f -s -m A commit message. non-empty
+% guilt pop
+Now at empty.
+% guilt push
+Applying patch..non-empty
+Patch applied.
+% guilt pop
+Now at empty.
+% guilt fold non-empty
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 15aab0fd8b937eb3bb01841693f35dcb75da2faf  .git/patches/master/status
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd  .git/patches/master/empty~
+f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd  .git/patches/master/non-empty~
+f 683678040eef9334d6329e00d5b9babda3e65b57  .git/patches/master/empty
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f a26a22287b500a2a372e42c2bab03599bbe37cdf  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-1~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-2~
+r 4eedaa32894fc07af3298d8c1178052942a3ca6a  .git/refs/patches/master/empty
+% git log -p
+commit 4eedaa32894fc07af3298d8c1178052942a3ca6a
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+A commit message.
+
+Signed-off-by: Commiter Name commiter@email
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
+% 

[GUILT 12/28] guilt header: more robust header selection.

[Per Cederqvist]

If you run something like guilt header '.*' the command would crash,
because the grep comand that tries to ensure that the patch exist
would detect a match, but the later code expected the match to be
exact.

Fixed by comparing exact strings.

And as a creeping feature guilt header will now try to use the
supplied patch name as an unachored regexp if no exact match was
found.  If the regexp yields a unique match, it is used; if more than
one patch matches, the names of all patches are listed and the command
fails.  (Exercise left to the reader: generalized this so that guilt
push also accepts a unique regular expression.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-header | 33 ++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/guilt-header b/guilt-header
index 41e00cc..2e96406 100755
--- a/guilt-header
+++ b/guilt-header
@@ -45,10 +45,37 @@ esac
 [ -z $patch ]  die No patches applied.
 
 # check that patch exists in the series
-ret=`get_full_series | grep -e ^$patch\$ | wc -l`
-if [ $ret -eq 0 ]; then
-   die Patch $patch is not in the series
+full_series=`get_tmp_file series`
+get_full_series  $full_series
+found_patch=
+while read x
+do
+   if [ $x = $patch ]
+   then
+   found_patch=$patch
+   break
+   fi
+done  $full_series
+if [ -z $found_patch ]
+then
+   TMP_MATCHES=`get_tmp_file series`
+   grep $patch  $full_series  $TMP_MATCHES
+   nr=`wc -l  $TMP_MATCHES`
+   if [ $nr -gt 1 ]
+   then
+   echo $patch does not uniquely identify a patch. Did you mean 
any of these? 2
+   sed 's/^/  /' $TMP_MATCHES 2
+   rm -f $TMP_MATCHES
+   exit 1
+   elif [ $nr -eq 0 ]
+   then
+   rm -f $TMP_MATCHES
+   die Patch $patch is not in the series
+   fi
+   found_patch=`cat $TMP_MATCHES`
+   rm -f $TMP_MATCHES
 fi
+patch=$found_patch
 
 # FIXME: warn if we're editing an applied patch
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 08/28] Added more test cases for guilt new: empty patches.

[Per Cederqvist]

Test that empty patches are handled correctly, both with and without
the guilt.diffstat configuration option.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-020.out | 250 +++
 regression/t-020.sh  |  60 +
 2 files changed, 310 insertions(+)

diff --git a/regression/t-020.out b/regression/t-020.out
index af45734..7e07efa 100644
--- a/regression/t-020.out
+++ b/regression/t-020.out
@@ -1128,3 +1128,253 @@ f 9c18cc7abe6b87f18503714a80a677b4094eb457  
.git/patches/master/add
 f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
 f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
 f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/status
+% guilt new empty.patch
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty.patch
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f d15a1d2d34493f790c78ddacb8815b0b9536ee2b  .git/patches/master/series
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty.patch
+f e90b964f01cbef60bbe00c38c55d9ea86618a66a  .git/patches/master/status
+r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1  
.git/refs/patches/master/empty.patch
+% git log -p
+commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch empty.patch
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
+% git config guilt.diffstat true
+% guilt refresh
+Patch empty.patch refreshed
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty.patch
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 7d261b8caad0f161c21daf5de65eeb521ff8c067  .git/patches/master/empty.patch
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f d15a1d2d34493f790c78ddacb8815b0b9536ee2b  .git/patches/master/series
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty.patch~
+f e90b964f01cbef60bbe00c38c55d9ea86618a66a  .git/patches/master/status
+r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1  
.git/refs/patches/master/empty.patch
+% git log -p
+commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch empty.patch
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
+% git config guilt.diffstat false
+---
+
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty.patch
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 7d261b8caad0f161c21daf5de65eeb521ff8c067  .git/patches/master/empty.patch
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f d15a1d2d34493f790c78ddacb8815b0b9536ee2b  .git/patches/master/series
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty.patch~
+f e90b964f01cbef60bbe00c38c55d9ea86618a66a  .git/patches/master/status
+r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1  
.git/refs/patches/master/empty.patch
+% git log -p
+commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch empty.patch
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
+% git config guilt.diffstat true
+% guilt refresh
+Patch empty.patch refreshed
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty.patch

[GUILT 05/28] guilt new: Accept more than 4 arguments.

[Per Cederqvist]

The argument parser arbitrarily refused to accept more than 4
arguments.  That made it impossible to run guilt new -f -s -m msg
patch.  Removed the checks for the number of arguments from the
guilt new parser -- the other checks that are already there are
enough to catch all errors.

Give a better error message if -m isn't followed by a message
argument.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-new | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/guilt-new b/guilt-new
index bb68924..9528438 100755
--- a/guilt-new
+++ b/guilt-new
@@ -11,10 +11,6 @@ fi
 
 _main() {
 
-if [ $# -lt 1 ] || [ $# -gt 4 ]; then
-   usage
-fi
-
 while [ $# -gt 0 ] ; do
case $1 in
-f)
@@ -31,6 +27,9 @@ while [ $# -gt 0 ] ; do
fi
;;
-m)
+   if [ $# -eq 1 ]; then
+   usage
+   fi
msg=$2
shift
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 09/28] Test suite: properly check the exit status of commands.

[Per Cederqvist]

The cmd and shouldfail functions checked the exit status of the
replace_path function instead of the actual command that was running.
(The $? construct checks the exit status of the last command in a
pipeline, not the first command.)

Updated t-032.sh, which used shouldfail instead of cmd in one
place.  (The comment in the script makes it clear that the command is
expected to succeed.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/scaffold | 17 +++--
 regression/t-032.sh |  2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/regression/scaffold b/regression/scaffold
index 5c8b73e..e4d7487 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -51,18 +51,23 @@ function filter_dd
 function cmd
 {
echo % $@
-   $@ 21 | replace_path  return 0
-   return 1
+   (
+   exec 31
+   rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+   exit $rv
+   )
+   return $?
 }
 
 # usage: shouldfail cmd..
 function shouldfail
 {
echo % $@
-   (
-   $@ 21 || return 0
-   return 1
-   ) | replace_path
+   ! (
+   exec 31
+   rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+   exit $rv
+   )
return $?
 }
 
diff --git a/regression/t-032.sh b/regression/t-032.sh
index b1d5f19..bba401e 100755
--- a/regression/t-032.sh
+++ b/regression/t-032.sh
@@ -28,7 +28,7 @@ shouldfail guilt import -P foo3 foo
 cmd guilt import -P foo2 foo
 
 # ok
-shouldfail guilt import foo
+cmd guilt import foo
 
 # duplicate patch name (implicit)
 shouldfail guilt import foo
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 27/28] Minor testsuite fix.

[Per Cederqvist]

Fix remove_topic() in t-061.sh so that it doesn't print a git hash.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-061.out | 1 -
 regression/t-061.sh  | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/regression/t-061.out b/regression/t-061.out
index ef0f335..60ad56d 100644
--- a/regression/t-061.out
+++ b/regression/t-061.out
@@ -381,7 +381,6 @@ ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commit 
refs/patches/master/mode
 ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit
refs/patches/master/remove
 % guilt pop -a
 No patches applied.
-ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
 % git checkout guilt/master
 Switched to branch guilt/master
 % guilt pop -a
diff --git a/regression/t-061.sh b/regression/t-061.sh
index a9a4fea..b573885 100755
--- a/regression/t-061.sh
+++ b/regression/t-061.sh
@@ -15,7 +15,7 @@ old_style_branch() {
 
 remove_topic() {
cmd guilt pop -a
-   if git rev-parse --verify --quiet guilt/master
+   if git rev-parse --verify --quiet guilt/master /dev/null
then
cmd git checkout guilt/master
else
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 23/28] The log.decorate setting should not influence guilt rebase.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-rebase | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guilt-rebase b/guilt-rebase
index fd28e48..a1714a0 100755
--- a/guilt-rebase
+++ b/guilt-rebase
@@ -66,7 +66,7 @@ pop_all_patches
 git merge --no-commit $upstream  /dev/null 2 /dev/null
 
 disp 
-log=`git log -1 --pretty=oneline`
+log=`git log -1 --no-decorate --pretty=oneline`
 disp HEAD is now at `echo $log | cut -c 1-7`... `echo $log | cut -c 41-`
 
 #
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 28/28] Added guilt.reusebranch configuration option.

[Per Cederqvist]

From: Per Cederqvist ce...@lysator.liu.se

When the option is true (the default), Guilt does not create a new Git
branch when patches are applied.  This way, you can switch between
Guilt 0.35 and the current version of Guilt with no issues.

At a future time, maybe a year after Guilt with guilt.reusebranch
support is released, the default should be changed to false to take
advantage of the ability to use a separate Git branch when patches are
applied.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt|  29 +++-
 regression/scaffold  |   1 +
 regression/t-062.out | 441 +++
 regression/t-062.sh  | 140 
 4 files changed, 606 insertions(+), 5 deletions(-)
 create mode 100644 regression/t-062.out
 create mode 100755 regression/t-062.sh

diff --git a/guilt b/guilt
index 36cfd1e..c89d939 100755
--- a/guilt
+++ b/guilt
@@ -850,6 +850,9 @@ guilt_push_diff_context=1
 # default diffstat value: true or false
 DIFFSTAT_DEFAULT=false
 
+# default old_style_prefix value: true or false
+REUSE_BRANCH_DEFAULT=true
+
 # Prefix for guilt branches.
 GUILT_PREFIX=guilt/
 
@@ -861,6 +864,10 @@ GUILT_PREFIX=guilt/
 diffstat=`git config --bool guilt.diffstat`
 [ -z $diffstat ]  diffstat=$DIFFSTAT_DEFAULT
 
+# reuse Git branch?
+reuse_branch=`git config --bool guilt.reusebranch`
+[ -z $reuse_branch ]  reuse_branch=$REUSE_BRANCH_DEFAULT
+
 #
 # The following gets run every time this file is source'd
 #
@@ -925,13 +932,25 @@ else
die Unsupported operating system: $UNAME_S
 fi
 
-if [ $branch = $raw_git_branch ]  [ -n `get_top 2/dev/null` ]
+if [ -n `get_top 2/dev/null` ]
 then
-# This is for compat with old repositories that still have a
-# pushed patch without the new-style branch prefix.
-old_style_prefix=true
+   # If there is at least one pushed patch, we set
+   # old_style_prefix according to how it was pushed.  It is only
+   # possible to change the prefix style while no patches are
+   # applied.
+   if [ $branch = $raw_git_branch ]
+   then
+   old_style_prefix=true
+   else
+   old_style_prefix=false
+   fi
 else
-old_style_prefix=false
+   if $reuse_branch
+   then
+   old_style_prefix=true
+   else
+   old_style_prefix=false
+   fi
 fi
 
 _main $@
diff --git a/regression/scaffold b/regression/scaffold
index e4d7487..e4d2f35 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -93,6 +93,7 @@ function setup_git_repo
git config log.date default
git config log.decorate no
git config guilt.diffstat false
+   git config guilt.reusebranch false
 }
 
 function setup_guilt_repo
diff --git a/regression/t-062.out b/regression/t-062.out
new file mode 100644
index 000..727b436
--- /dev/null
+++ b/regression/t-062.out
@@ -0,0 +1,441 @@
+% setup_repo
+% git config guilt.reusebranch true
+% guilt push -a
+Applying patch..modify
+Patch applied.
+Applying patch..add
+Patch applied.
+Applying patch..remove
+Patch applied.
+Applying patch..mode
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 71596bf71b72c2717e1aee378aabefbfa19ab7c8  .git/patches/master/status
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+r 33633e7a1aa31972f125878baf7807be57b1672d  .git/refs/patches/master/modify
+r 37d588cc39848368810e88332bd03b083f2ce3ac  .git/refs/patches/master/add
+r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba  .git/refs/patches/master/mode
+r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9  .git/refs/patches/master/remove
+% git for-each-ref
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master
+37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/patches/master/mode
+33633e7a1aa31972f125878baf7807be57b1672d commit
refs/patches/master/modify
+ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit
refs/patches/master/remove
+% git for-each-ref
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master
+37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/patches/master/mode
+33633e7a1aa31972f125878baf7807be57b1672d commit
refs/patches/master/modify
+ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit
refs/patches/master/remove
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 

[GUILT 13/28] Check that guilt header '.*' fails.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-028.out | 7 +++
 regression/t-028.sh  | 4 
 2 files changed, 11 insertions(+)

diff --git a/regression/t-028.out b/regression/t-028.out
index 1564c09..ea72a3a 100644
--- a/regression/t-028.out
+++ b/regression/t-028.out
@@ -49,3 +49,10 @@ Signed-off-by: Commiter Name commiter@email
 
 % guilt header non-existant
 Patch non-existant is not in the series
+% guilt header .*
+.* does not uniquely identify a patch. Did you mean any of these?
+  modify
+  add
+  remove
+  mode
+  patch-with-some-desc
diff --git a/regression/t-028.sh b/regression/t-028.sh
index 88e9adb..2ce0378 100755
--- a/regression/t-028.sh
+++ b/regression/t-028.sh
@@ -31,4 +31,8 @@ done
 
 shouldfail guilt header non-existant
 
+# This is an evil variant of a non-existant patch.  However, this
+# patch name is a regexp that just happens to match an existing patch.
+shouldfail guilt header '.*'
+
 # FIXME: how do we check that -e works?
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 25/28] guilt push now fails when there are no more patches to push.

[Per Cederqvist]

This makes it easier to script operations on the entire queue, for
example run the test suite on each patch in the queue:

guilt pop -a;while guilt push; do make test||break; done

This brings guilt push in line with the push operation in Mercurial
Queues (hg qpush), which fails when there are no patches to apply.

Updated the test suite.

guilt push -a still does not fail.  (It successfully manages to
ensure that all patches are pushed, even if it did not have to do
anything to make it so.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-push   | 14 -
 regression/t-020.out | 89 
 regression/t-020.sh  | 13 +++-
 3 files changed, 108 insertions(+), 8 deletions(-)

diff --git a/guilt-push b/guilt-push
index 67687e7..350 100755
--- a/guilt-push
+++ b/guilt-push
@@ -55,6 +55,7 @@ fi
 
 patch=$1
 [ ! -z $all ]  patch=-a
+[ -z $patch ]  { patch=1; num=t; }
 
 if [ $patch = -a ]; then
# we are supposed to push all patches, get the last one out of
@@ -65,7 +66,7 @@ if [ $patch = -a ]; then
die There are no patches to push.
fi
 elif [ ! -z $num ]; then
-   # we are supposed to pop a set number of patches
+   # we are supposed to push a set number of patches
 
[ $patch -lt 0 ]  die Invalid number of patches to push.
 
@@ -78,11 +79,6 @@ elif [ ! -z $num ]; then
# clamp to minimum
[ $tidx -lt $eidx ]  eidx=$tidx
 
-elif [ -z $patch ]; then
-   # we are supposed to push only the next patch onto the stack
-
-   eidx=`wc -l  $applied`
-   eidx=`expr $eidx + 1`
 else
# we're supposed to push only up to a patch, make sure the patch is
# in the series
@@ -109,7 +105,11 @@ if [ $sidx -gt $eidx ]; then
else
disp File series fully applied, ends at patch `get_series | 
tail -n 1`
fi
-   exit 0
+   if [ -n $all ]; then
+   exit 0
+   else
+   exit 1
+   fi
 fi
 
 get_series | sed -n -e ${sidx},${eidx}p | while read p
diff --git a/regression/t-020.out b/regression/t-020.out
index 7e07efa..23cb9db 100644
--- a/regression/t-020.out
+++ b/regression/t-020.out
@@ -270,6 +270,95 @@ index 000..8baef1b
 +++ b/def
 @@ -0,0 +1 @@
 +abc
+% guilt push
+File series fully applied, ends at patch mode
+% guilt push -a
+File series fully applied, ends at patch mode
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 71596bf71b72c2717e1aee378aabefbfa19ab7c8  .git/patches/master/status
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+r 33633e7a1aa31972f125878baf7807be57b1672d  .git/refs/patches/master/modify
+r 37d588cc39848368810e88332bd03b083f2ce3ac  .git/refs/patches/master/add
+r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba  .git/refs/patches/master/mode
+r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9  .git/refs/patches/master/remove
+% git log -p
+commit ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch mode
+
+diff --git a/def b/def
+old mode 100644
+new mode 100755
+
+commit ffb7faa126a6d91bcdd44a494f76b96dd860b8b9
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch remove
+
+diff --git a/abd b/abd
+deleted file mode 100644
+index fd3896d..000
+--- a/abd
 /dev/null
+@@ -1 +0,0 @@
+-‰öuؽáZâñeÏÈE„£WÀV¼/›U?Ú|¢@6¤8'H¸1G_˜Í§*·ðRҙ¤
ªÂ~·
+\ No newline at end of file
+
+commit 37d588cc39848368810e88332bd03b083f2ce3ac
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch add
+
+diff --git a/abd b/abd
+new file mode 100644
+index 000..fd3896d
+--- /dev/null
 b/abd
+@@ -0,0 +1 @@
++‰öuؽáZâñeÏÈE„£WÀV¼/›U?Ú|¢@6¤8'H¸1G_˜Í§*·ðRҙ¤
ªÂ~·
+\ No newline at end of file
+
+commit 33633e7a1aa31972f125878baf7807be57b1672d
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch modify
+
+diff --git a/def b/def
+index 8baef1b..7d69c2f 100644
+--- a/def
 b/def
+@@ -1 +1,2 @@
+ abc
++asjhfksad
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
 % guilt pop --all
 All patches popped.
 % guilt push
diff --git a/regression/t-020.sh b/regression/t-020.sh
index 906aec6..0f9f85d 100755
--- a/regression/t-020.sh
+++ b/regression/t-020.sh
@@ -26,6 +26,17 @@ guilt series | while read n ; do
 done
 
 #

[GUILT 16/28] Fix backslash handling when creating names of imported patches.

[Per Cederqvist]

The 'echo %s' construct sometimes processes escape sequences.  (This
happens, for instance, under Ubuntu 14.04 when /bin/sh is actually
dash.)  We don't want that to happen when we are importing commits, so
use 'printf %s $s' instead.

(The -E option of bash that explicitly disables backslash expansion is
not portable; it is not supported by dash.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-import-commit  |  2 +-
 regression/t-034.out | 14 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/guilt-import-commit b/guilt-import-commit
index a4119d6..6eb2f4e 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -30,7 +30,7 @@ for rev in `git rev-list $rhash`; do
 
# Try to convert the first line of the commit message to a
# valid patch name.
-   fname=`echo $s | sed -e s//and/g -e s/[ :]/_/g -e s,[/\\],-,g \
+   fname=`printf %s $s | sed -e s//and/g -e s/[ :]/_/g -e 
s,[/\\],-,g \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
-e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
-e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
diff --git a/regression/t-034.out b/regression/t-034.out
index 7bc9459..bda4399 100644
--- a/regression/t-034.out
+++ b/regression/t-034.out
@@ -236,7 +236,7 @@ Date:   Mon Jan 1 00:00:00 2007 +
 About to begin conversion...
 Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541
 Converting 2a8b1889 as can-have-embedded-single-slashes
-Converting 0a46f8fa as backslash-isorbidden
+Converting 0a46f8fa as backslash-is-forbidden
 Converting aedb74fd as x
 Converting 30187ed0 as cannot@have@the@sequence@at-brace
 Converting 106e8e5a as cannot_end_in_
@@ -300,7 +300,7 @@ Applying patch..cannot@have@the@sequence@at-brace.patch
 Patch applied.
 Applying patch..x.patch
 Patch applied.
-Applying patch..backslash-isorbidden.patch
+Applying patch..backslash-is-forbidden.patch
 Patch applied.
 Applying patch..can-have-embedded-single-slashes.patch
 Patch applied.
@@ -311,7 +311,7 @@ Date:   Mon Jan 1 00:00:00 2007 +
 
 Can/have/embedded/single/slashes
 
-commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141 
(refs/patches/master/backslash-isorbidden.patch)
+commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141 
(refs/patches/master/backslash-is-forbidden.patch)
 Author: Author Name author@email
 Date:   Mon Jan 1 00:00:00 2007 +
 
@@ -518,8 +518,6 @@ d .git/patches/master
 d .git/refs/patches
 d .git/refs/patches/master
 f 06beca7069b9e576bd431f65d13862ed5d3e2a0f  
.git/patches/master/ctrlisforbidden.patch
-f 08267ec6783ea9d1adae55b275198f7594764ed0  .git/patches/master/series
-f 08267ec6783ea9d1adae55b275198f7594764ed0  .git/patches/master/status
 f 09b7e9be44ae5ec3a4bb30f5ee9d4ebc2c042f64  
.git/patches/master/two_consecutive_dots_(.)_is_forbidden.patch
 f 0b971c9a17aeca2319c93d700ffd98acc2a93451  
.git/patches/master/question-mark-is-forbidden.patch
 f 2b8392f63d61efc12add554555adae30883993cc  
.git/patches/master/cannot-end-in-slash-.patch
@@ -529,7 +527,7 @@ f 34e07c584032df137f19bdb66d93f316f00a5ac8  
.git/patches/master/tildeisforbidden
 f 49bab499826b63deb2bd704629d60c7268c57aee  
.git/patches/master/the_sequence_-._is_forbidden.patch
 f 5bcddb8ccb6e6e5e8a61e9e56cb2e0f70cbab2f5  
.git/patches/master/cannot@have@the@sequence@at-brace.patch
 f 637b982fe14a240de181ae63226b27e0c406b3dc  
.git/patches/master/asterisk-is-forbidden.patch
-f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3  
.git/patches/master/backslash-isorbidden.patch
+f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3  
.git/patches/master/backslash-is-forbidden.patch
 f 7b103c3c7ae298cd2334f6f49da48bae1424f77b  
.git/patches/master/crisalsoforbidden.patch
 f 9b810b8c63779c51d2e7f61ab59cd49835041563  .git/patches/master/x.patch
 f a22958d9ae9976fd7b2b5a9d0bcd44bf7ad9b08a  
.git/patches/master/caretisforbidden.patch
@@ -537,6 +535,8 @@ f ab325bf5a432937fc6f231d3e8a773a62d53952b  
.git/patches/master/multiple-slashes
 f cb9cffbd4465bddee266c20ccebd14eb687eaa89  
.git/patches/master/delisforbidden.patch
 f d0885a1a1fdee0fd1e4fedce3f7acd3100540bc4  
.git/patches/master/openbracketisforbidden.patch
 f d2903523fb66a346596eabbdd1bda4e52b266440  
.git/patches/master/check-multiple-.-dots-.-foo.patch
+f da90de1c84138194524994e0bc3bc4ca8189c999  .git/patches/master/series
+f da90de1c84138194524994e0bc3bc4ca8189c999  .git/patches/master/status
 f dfc11f76394059909671af036598c5fbe33001ba  
.git/patches/master/space_is_forbidden.patch
 f e47474c52d6c893f36d0457f885a6dd1267742bb  
.git/patches/master/colon_is_forbidden.patch
 f e7a5f8912592d9891e6159f5827c8b4f372cc406  
.git/patches/master/the_sequence_.lock-_is_forbidden.patch
@@ -548,7 +548,7 @@ r 1626a11d979a1e9e775c766484172212277153df  
.git/refs/patches/master/asterisk-is
 r 3a0d5ccef0359004fcaa9cee98fbd6a2c4432e74  
.git/refs/patches/master/tildeisforbidden.patch
 r 434e07cacdd8e7eb4723e67cb2d100b3a4121a3a  

[GUILT 17/28] guilt graph no longer loops when no patches are applied.

[Per Cederqvist]

Give an error message if no patches are applied.  Added a test case
that never terminates unless this fix is applied.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-graph  | 10 --
 regression/t-033.out |  3 +++
 regression/t-033.sh  | 11 +++
 3 files changed, 22 insertions(+), 2 deletions(-)
 create mode 100644 regression/t-033.out
 create mode 100755 regression/t-033.sh

diff --git a/guilt-graph b/guilt-graph
index b3469dc..00301d5 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -17,8 +17,14 @@ fi
 
 patchname=$1
 
-bottom=`git rev-parse refs/patches/$branch/$(head_n 1  $applied)`
-base=`git rev-parse $bottom^`
+bottompatch=$(head_n 1  $applied)
+if [ -z $bottompatch ]
+then
+   echo No patch applied. 2
+   exit 1
+fi
+
+base=`git rev-parse refs/patches/${branch}/${bottompatch}^`
 
 if [ -z $patchname ]; then
top=`git rev-parse HEAD`
diff --git a/regression/t-033.out b/regression/t-033.out
new file mode 100644
index 000..76613f9
--- /dev/null
+++ b/regression/t-033.out
@@ -0,0 +1,3 @@
+% setup_repo
+% guilt graph
+No patch applied.
diff --git a/regression/t-033.sh b/regression/t-033.sh
new file mode 100755
index 000..ae40577
--- /dev/null
+++ b/regression/t-033.sh
@@ -0,0 +1,11 @@
+#!/bin/bash
+#
+# Test the graph code
+#
+
+source $REG_DIR/scaffold
+
+cmd setup_repo
+
+shouldfail guilt graph
+
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 04/28] Allow guilt import-commit to run from a dir which contains spaces.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-import-commit | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/guilt-import-commit b/guilt-import-commit
index 20dcee2..9488ded 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -23,7 +23,7 @@ if ! must_commit_first; then
 fi
 
 disp About to begin conversion... 2
-disp Current head: `cat $GIT_DIR/refs/heads/\`git_branch\`` 2
+disp Current head: `cat \$GIT_DIR\/refs/heads/\`git_branch\`` 2
 
 for rev in `git rev-list $rhash`; do
s=`git log --pretty=oneline -1 $rev | cut -c 42-`
@@ -46,7 +46,7 @@ for rev in `git rev-list $rhash`; do
do_make_header $rev
echo 
git diff --binary $rev^..$rev
-   )  $GUILT_DIR/$branch/$fname
+   )  $GUILT_DIR/$branch/$fname
 
# FIXME: grab the GIT_AUTHOR_DATE from the commit object and set the
# timestamp on the patch
@@ -68,6 +68,6 @@ for rev in `git rev-list $rhash`; do
 done
 
 disp Done. 2
-disp Current head: `cat $GIT_DIR/refs/heads/\`git_branch\`` 2
+disp Current head: `cat \$GIT_DIR\/refs/heads/\`git_branch\`` 2
 
 }
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 01/28] The tests should not fail if guilt.diffstat is set.

[Per Cederqvist]

Explicitly set guilt.diffstat to its default value.  Without this, the
027 test (and possibly others) fail if guilt.diffstat is set to true
in ~/.gitconfig.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/scaffold | 1 +
 1 file changed, 1 insertion(+)

diff --git a/regression/scaffold b/regression/scaffold
index 546d8c6..5c8b73e 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -87,6 +87,7 @@ function setup_git_repo
# Explicitly set config that the tests rely on.
git config log.date default
git config log.decorate no
+   git config guilt.diffstat false
 }
 
 function setup_guilt_repo
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 10/28] Run test_failed if the exit status of a test script is bad.

[Per Cederqvist]

There were two problems with the old code:

 - Since set -e is in effect (that is set in scaffold) the run-test
   script exited immediately if a t-*.sh script failed.  This is not
   nice, as we want the error report that test_failed prints.

 - The code ran cd - between running the t-*.sh script and checking
   the exit status, so the exit status was lost.  (Actually, the exit
   status was saved in $ERR, but nothing ever looked at $ERR.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/run-tests | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/regression/run-tests b/regression/run-tests
index a10e796..d39f9ef 100755
--- a/regression/run-tests
+++ b/regression/run-tests
@@ -55,11 +55,16 @@ function run_test
 
# run the test
cd $REPODIR  /dev/null
-   $REG_DIR/t-$1.sh 21  $LOGFILE
-   ERR=$?
+   if $REG_DIR/t-$1.sh 21  $LOGFILE
+   then
+   ERR=false
+   else
+   ERR=true
+   fi
+
cd -  /dev/null
 
-   [ $? -ne 0 ]  test_failed
+   $ERR  test_failed
diff -u t-$1.out $LOGFILE || test_failed
 
echo done.
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT 14/28] Use git check-ref-format to validate patch names.

[Per Cederqvist]

The valid_patchname now lets git check-ref-format do its job instead
of trying (and failing) to implement the same rules.  See
git-check-ref-format(1) for a list of the rules.

Refer to the git-check-ref-format(1) man page in the error messages
produced when valid_patchname indicates that the name is bad.

Added testcases that breaks most of the rules in that man-page.

Git version 1.8.5 no longer allows the single character @ as a
branch name.  Guilt always rejects that name, for increased
compatibility.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt|  23 ++-
 guilt-fork   |   2 +-
 guilt-import |   2 +-
 guilt-new|   2 +-
 regression/t-025.out | 426 +--
 regression/t-025.sh  |  12 +-
 regression/t-032.out |   4 +-
 7 files changed, 448 insertions(+), 23 deletions(-)

diff --git a/guilt b/guilt
index c59cd0f..ca922aa 100755
--- a/guilt
+++ b/guilt
@@ -132,14 +132,21 @@ fi
 # usage: valid_patchname patchname
 valid_patchname()
 {
-   case $1 in
-   /*|./*|../*|*/./*|*/../*|*/.|*/..|*/|*\ *|*\*)
-   return 1;;
-   *:*)
-   return 1;;
-   *)
-   return 0;;
-   esac
+   if git check-ref-format --allow-onelevel $1
+   then
+   # Starting with Git version 1.8.5, a branch cannot be
+   # the single character @.  Make sure guilt rejects
+   # that name even if we are currently using an older
+   # version of Git.  This ensures that the test suite
+   # runs fine using any version of Git.
+   if [ $1 = @ ]
+   then
+   return 1
+   fi
+   return 0
+   else
+   return 1
+   fi
 }
 
 get_branch()
diff --git a/guilt-fork b/guilt-fork
index a85d391..6447e55 100755
--- a/guilt-fork
+++ b/guilt-fork
@@ -37,7 +37,7 @@ else
 fi
 
 if ! valid_patchname $newpatch; then
-   die The specified patch name contains invalid characters (:).
+   die The specified patch name is invalid according to 
git-check-ref-format(1).
 fi
 
 if [ -e $GUILT_DIR/$branch/$newpatch ]; then
diff --git a/guilt-import b/guilt-import
index 3e9b3bb..928e325 100755
--- a/guilt-import
+++ b/guilt-import
@@ -40,7 +40,7 @@ if [ -e $GUILT_DIR/$branch/$newname ]; then
 fi
 
 if ! valid_patchname $newname; then
-   die The specified patch name contains invalid characters (:).
+   die The specified patch name is invalid according to 
git-check-ref-format(1).
 fi
 
 # create any directories as needed
diff --git a/guilt-new b/guilt-new
index 9528438..9f7fa44 100755
--- a/guilt-new
+++ b/guilt-new
@@ -64,7 +64,7 @@ fi
 
 if ! valid_patchname $patch; then
disp Patchname is invalid. 2
-   die it cannot begin with '/', './' or '../', or contain /./, /../, or 
whitespace
+   die It must follow the rules in git-check-ref-format(1).
 fi
 
 # create any directories as needed
diff --git a/regression/t-025.out b/regression/t-025.out
index 7811ab1..01bc406 100644
--- a/regression/t-025.out
+++ b/regression/t-025.out
@@ -141,7 +141,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new white space
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -211,7 +211,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new /abc
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -235,7 +235,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new ./blah
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -259,7 +259,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new ../blah
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -283,7 +283,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new abc/./blah
 

Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB

[David Kastrup]

Junio C Hamano gits...@pobox.com writes:

 David Kastrup d...@gnu.org writes:

 Junio C Hamano gits...@pobox.com writes:

 Duy Nguyen pclo...@gmail.com writes:

 The only
 downside I see is large blobs will be packed  undeltified, which could
 increase pack size if you have lots of them.

 I think that is something that can be tweaked, unless the user tells
 us otherwise via command line override, when running the improved
 gc --aggressive ;-)

 deltaBaseCacheLimit is used for unpacking, not for packing.

 Hmm, doesn't packing need to read existing data?

Judging from the frequent out-of-memory conditions of git gc
--aggressive, packing is not restrained by deltaBaseCacheLimit.

-- 
David Kastrup
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB

[David Kastrup]

Jeff King p...@peff.net writes:

 On Wed, Mar 19, 2014 at 01:38:32PM +0100, David Kastrup wrote:

 The default of 16MiB causes serious thrashing for large delta chains
 combined with large files.

 Does it make much sense to bump this without also bumping
 MAX_DELTA_CACHE in sha1_file.c? In my measurements of linux.git, bumping
 the memory limit did not help much without also bumping the number of
 slots.

In the cases I checked, bumping MAX_DELTA_CACHE did not help much.
Bumping it once to 512 could be a slight improvement; larger values then
caused performance to regress.

 I guess that just bumping the memory limit would help with repos which
 have deltas on large-ish files (whereas the kernel just has a lot of
 deltas on a lot of little directories),

Well, those were the most pathological for git-blame so I was somewhat
focused on them.

 but I'd be curious how much.

http://repo.or.cz/r/wortliste.git

consists basically of adding lines to a single alphabetically sorted
file wortliste of currently size 15MB.

-- 
David Kastrup
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

On 3/20/2014 4:19 PM, Ilya Bobyr wrote:
 On 3/20/2014 11:38 AM, Junio C Hamano wrote:
 Ilya Bobyr ilya.bo...@gmail.com writes:

 [...]
 
 -opt_specflags* SP+ help LF
 +opt_specflags*arg_hint? SP+ help LF
   
 `opt_spec`::
 @@ -313,6 +313,12 @@ Each line of options has this format:
 * Use `!` to not make the corresponding negated long option
 available.
   +`arg_hint`::
 +`arg_hing`, if specified, is used as a name of the argument
 in the
 +help output, for options that take arguments. `arg_hint` is
 +terminated by the first whitespace. When output the name is
 shown in
 +angle braces.  Underscore symbols are replaced with spaces.
 The last part is troubling (and sounds not very sane).  Do we do
 such a munging anywhere else, or is it just here?  If the latter I'd
 prefer not to see such a hack.

 The following commands have spaces in argument names in the -h
 output for one or two arguments:
   * clone
s/clone/checkout/
   * commit
   * merge

 A number of commands use dashes to separate words in arguments names.

 git notes is the only command that uses an underscore in one
 argument name.

 At the moment space is used to separate option specification from the
 help line.  As argument name hint is part of the option specification
 it ends at the first space.

 It seems a bit unfair if sh based commands would not be able to use
 spaces while the build-in ones can.
 As underscores are not used in the UI (at least that was my impression
 so far), I thought that to be a good option.

 Do you think a different kind of escaping should be used? Backslashes?
 Or no spaces?

git merge also uses equals sign in one of the argument names.  That
would not be possible for sh based commands either.

As a lot of commands are using dashes instead of spaces, so not
supporting spaces is probably fine.

Another option I can think of is to use (or just allow) angle brackets
around argument names.  That would look similar to the actual output.
git shortlog has some punctuation in an argument name, which braces
would make a bit easier to read.
This is how an option description would look like then:

OPTION_SPEC=\
...
S,gpg-sign?key id GPG sign commit from commit
w?w[,i1[,i2]] shortlog option with a complicated argument name
...


If there is interest in this, I could code it up and post for discussion.

 [...]

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GUILT 20/28] guilt graph: Handle patch names containing quotes.

[Eric Sunshine]

On Fri, Mar 21, 2014 at 3:31 AM, Per Cederqvist ced...@opera.com wrote:
 Quote quotes with a backslash in the guitl graph output.  Otherwise,

s/guitl/guilt/

 the dot file could contain syntax errors.

 Added a test case.
 ---
  guilt-graph  |  2 ++
  regression/t-033.out | 22 ++
  regression/t-033.sh  |  9 +
  3 files changed, 33 insertions(+)

 diff --git a/guilt-graph b/guilt-graph
 index 575f03b..24ab83b 100755
 --- a/guilt-graph
 +++ b/guilt-graph
 @@ -58,6 +58,8 @@ while [ $current != $base ]; do
  }`
 [ -z $pname ]  pname=?

 +   pname=`printf \%s\ $pname|sed 's/\/\/g'`
 +
 disp # checking rev $current
 disp   \$current\ [label=\$pname\]

 diff --git a/regression/t-033.out b/regression/t-033.out
 index e638d7b..1c28ea9 100644
 --- a/regression/t-033.out
 +++ b/regression/t-033.out
 @@ -63,3 +63,25 @@ digraph G {
 ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
 891bc14b5603474c9743fd04f3da888644413dc5 - 
 ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
  }
 +% guilt new a-betterquicker'-patch.patch
 +% git add file.txt
 +% guilt refresh
 +Patch a-betterquicker'-patch.patch refreshed
 +% guilt pop
 +Now at c.patch.
 +% guilt push
 +Applying patch..a-betterquicker'-patch.patch
 +Patch applied.
 +% guilt graph
 +digraph G {
 +# checking rev bc7df666a646739eaf559af23cab72f2bfd01f0e
 +   bc7df666a646739eaf559af23cab72f2bfd01f0e 
 [label=a-\betterquicker'-patch.patch]
 +# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
 +   891bc14b5603474c9743fd04f3da888644413dc5 [label=c.patch]
 +   bc7df666a646739eaf559af23cab72f2bfd01f0e - 
 891bc14b5603474c9743fd04f3da888644413dc5; // ?
 +# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
 +   c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
 +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
 +   ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
 +   891bc14b5603474c9743fd04f3da888644413dc5 - 
 ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
 +}
 diff --git a/regression/t-033.sh b/regression/t-033.sh
 index 57dce78..968292c 100755
 --- a/regression/t-033.sh
 +++ b/regression/t-033.sh
 @@ -46,3 +46,12 @@ cmd git add file.txt
  cmd guilt refresh
  fixup_time_info c.patch
  cmd guilt graph
 +
 +# A patch name that contains funky characters, including unbalanced
 +# quotes.
 +cmd guilt new a-\betterquicker'-patch.patch
 +cmd echo d  file.txt
 +cmd git add file.txt
 +cmd guilt refresh
 +fixup_time_info a-\betterquicker'-patch.patch
 +cmd guilt graph
 --
 1.8.3.1

 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB

[Duy Nguyen]

On Fri, Mar 21, 2014 at 1:04 PM, David Kastrup d...@gnu.org wrote:
 Hmm, doesn't packing need to read existing data?

 Judging from the frequent out-of-memory conditions of git gc
 --aggressive, packing is not restrained by deltaBaseCacheLimit.

pack-objects memory usage is more controlled by pack.windowmemory,
which defaults to unlimited. Does it make sense to default to half
physical memory or something?
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB

[David Kastrup]

Junio C Hamano gits...@pobox.com writes:

 I know that the 512MiB default for the bitFileThreashold (aka
 forget about delta compression) came out of thin air.  It was just
 1GB is always too huge for anybody, so let's cut it in half and
 declare that value the initial version of a sane threashold,
 nothing more.

 So it might be that the problem is 512MiB is still too big, relative
 to the 16MiB of delta base cache, and the former may be what needs
 to be tweaked.

Well, the point with the 512MiB limit is basically that you can always
argue if you are working with 500MiB files, you will not be using just
128MiB of main memory anyway.  The 16MiB limit, in contrast, will get
utilized for basically every history, even one of small files, that's
deep enough.

 If a blob close to but below 512MiB is a problem for 16MiB delta base
 cache, it would still be too big to cause the same problem for 128MiB
 delta base cache---it would evict all the other objects and then end
 up not being able to fit in the limit itself, busting the limit
 immediately, no?

I think, but may be mistaken, that the delta base limit decides when to
flush whole blobs.  So a 500MiB file would still be encoded relative to
a single newer version anyway and take memory accordingly.

But I am not sure about that at all.

-- 
David Kastrup
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB

[David Kastrup]

Jeff King p...@peff.net writes:

 If you have before-and-after numbers for just this patch on some
 repository, that would be an interesting thing to put in the commit
 message.

It's a hen-and-egg problem regarding the benchmarks.  The most
impressive benchmarks arise with the git-blame performance work in
place, and the most impressive benchmarks for the git-blame performance
work are when this or something similar is in place.  Of course, when
there are two really deficient things slowing operations down, fixing
only one is going to be much less impressive.

So I decided to tackle the low-hanging fruit here first.  But it would
appear that this amounts in far too much work since it means I have to
search for and create some _other_ benchmarking scenario not hampered by
substandard code like the current git-blame is.

I have enough on my plate as it is, so even though it puts the _real_
git-blame work in a worse light, I should rather get that finished first
(nobody will argue to keep the useless threshing of it around).  Of
course, the person then creating the two-line change to
deltaBaseCacheLimit will be able to claim much larger performance
improvements in his commit message afterwards than what I can claim
regarding the git-blame work when going first, but that's life.

-- 
David Kastrup
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GSOC 2014]idea:Git Configuration API Improvement

[Matthieu Moy]

Yao Zhao zhaox...@umn.edu writes:

 Moy, thanks for explaining. You said API should be hided. Is that
 means I should indicate an arbitary feature in old version or new
 feature we added should be linked to a manipulation of inner
 structure? And I need to find the connection to make this abstraction?

Sorry, I do not understand what you mean.

The new code should be backward compatible with the old one, that is:
existing code using git_config() should continue working. There are a
lot of git_config() calls in the codebase, and a GSoC won't have time to
change them all into something new.

This does not mean we can't add new features, both on the file parsing
side (add the ability to unset a key) and on the user API side (allow
getting the value of a key more easily).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] status: disable translation when --porcelain is used

[Matthieu Moy]

Junio C Hamano gits...@pobox.com writes:

 diff --git a/wt-status.c b/wt-status.c
 index a452407..e55e5b9 100644
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -1509,19 +1509,23 @@ static void wt_shortstatus_print_tracking(struct 
 wt_status *s)
  return;
  }
  
 +const char *gone   = s-no_gettext ? gone   : _(gone);
 +const char *behind = s-no_gettext ? behind  : _(behind );
 +const char *ahead  = s-no_gettext ? ahead   : _(ahead );

 Having to repeat the same string constant twice (and a half for the
 variable name) each is an eyesore.  I wonder if we can do better,
 perhaps with:

 #define LABEL(string) (s-no_gettext ? (string) : _(string))

 and then

   color_fprintf(s-fp, header_color, LABEL(N_(gone)));

 or something along those lines?

I first thought about trying something clever with the preprocessor, but
since it's only for 3 strings, I went the KISS way. I tend to prefer my
version for simplicity, but no strong opinion here.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [GSoC] Draft of Proposal for GSoC

[Brian Bourn]

Parts of v2, once again, i'd love some more comments on what I've
rewritten


On Fri, Mar 21, 2014 at 1:42 AM, Jeff King p...@peff.net wrote:
 On Thu, Mar 20, 2014 at 02:15:29PM -0400, Brian Bourn wrote:

 Going through the annals of the listserve thus far I've found a few
 discussions which provide some insight towards this process as well as
 some experimental patches that never seem to have made it
 through[1][2][3][4]

 Reading the past work in this area is a good way to get familiar with
 it. It looks like most of the features discussed in the threads you link
 have been implemented. The one exception seems to be negative patterns.
 I think that would be a good feature to build on top of the unified
 implementation, once all three commands are using it.

 I would start by beginning a deprecation plan for git branch -l very
 similar to the one Junio presents in [5], moving -create-reflog to -g,

 That makes sense. I hadn't really considered -l as another point of
 inconsistency between the commands, but it definitely is.

 Following this I would begin the real work of the project which would
 involve moving the following flag operations into a standard library
 say 'list-options.h'

 --contains [6]
 --merged [7]
 --no-merged[8]
 --format
 This Library would build these options for later interpretation by 
 parse_options
 Can you sketch out what the API would look like for this unified
 library? What calls would the 3 programs need to make into it?


Something like this?

Sample api calls
Add_Opt_Group()
Parse_with_contains()
Parse_with_merged()
Parse_with_no_merged()
Parse_with_formatting()
(each of the 4 calls above may have internal calls within the library
in order to parse the option for each of the different function which
may call these functions)


 For the most part I haven't finalized my weekly schedule but a basic
 breakdown would be

 Can you go into more detail here? Remember that writing code is only one
 part of the project. You'll need to be submitting your work, getting
 review and feedback, and iterating on it.

 One problem that students have is queuing up a large amount of work to
 send to the list. Then they twiddle their thumbs waiting for review to
 come back (which takes a long time, because they just dumped a large
 body of work on the reviewers). If you want to make effective use of
 your time, it helps to try to break tasks down into smaller chunks, and
 think about the dependencies between the chunks. When one chunk is in
 review, you can be designing and coding on another.

This one I can absolutely understand, I tried to break this part down into very
managable parts and give myself a little time at the end of each coding period
to clean up each previous section.  this slop time also allows for me
to hopefully
add some of the extra features that have been thought of. I'm thinking something
like this makes it a little better,

Weekly Schedule

Start-Midterm
Week 1- Begin deprecation of -l in git branch/establish exactly how
long each stage of the deprecation should take.  Spend some time
reading *.c files even deeper while getting to know any current
patches occurring in any area near my work files.  Lastly, this week
will be spent going through the Mailing-list finding previous work
done in this area and any other experimental patches
Week 2- Move Opt_Group callbacks for the functions  into Library
Week 3-Make a Contains Function in the library which will work for all
three functions
Week 4-Add Merge function in library
Week 5-Add a No Merge function in library
Weeks 7-8 spend time polishing the library and cleaning up the patches
for final submission of library to the project

Deliverables for midterm- Library finished pending polish and
acceptance into the git repository

Midterm

Week 9- refactor all files to use the contains flag from the file.
Week 10- use Merge from library in all relevant files
Week 11-use no-merge from library in all relevant files
Week 11-12- implement the format flags in all relevant files (this
will be slightly harder as I think this might involve calling
for-each-ref in the code for tag and branch. Ultimately there is a
chance that part of the code for doing for-each-ref will end up in
this library as well), additionally add in the code for formatting the
relevant opt_Groups into the necessary files.
Week 13-14 Polish patches via mailing-list and clean up all the
refactoring of the files that has occurred.(optionally, add more
formatting changes such as negative patterns and numbering each output
into the library).

Deliverables for Final- working library hopefully added into the code,
and all of the relevant patches for using the library mostly polished
and, minimally, pending peer review for submission into the code base.

I do wonder if this plan might be a little on the conservative side,
if anything, I think
this could take a slightly shorter time than planned, but In that case
I can always
work on other additions to format.



 

Re: [PATCH] builtin/apply.c: use iswspace() to detect line-ending-like chars

[Michael Haggerty]

On 03/20/2014 08:39 PM, George Papanikolaou wrote:
 Removing the bloat of checking for both '\r' and '\n' with the prettier
 iswspace() function which checks for other characters as well. (read: \f \t 
 \v)
 ---
 
 This is one more try to clean up this fuzzy_matchlines() function as part of a
 microproject for GSOC. The rest more clarrified microprojects were taken.
 I'm obviously planning on applying.
 
 Thanks
 
 Signed-of-by: George 'papanikge' Papanikolaou g3orge@gmail.com
 
  builtin/apply.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/builtin/apply.c b/builtin/apply.c
 index b0d0986..912a53a 100644
 --- a/builtin/apply.c
 +++ b/builtin/apply.c
 @@ -295,9 +295,9 @@ static int fuzzy_matchlines(const char *s1, size_t n1,
   int result = 0;
  
   /* ignore line endings */
 - while ((*last1 == '\r') || (*last1 == '\n'))
 + while (iswspace(*last1))
   last1--;
 - while ((*last2 == '\r') || (*last2 == '\n'))
 + while (iswspace(*last2))
   last2--;
  
   /* skip leading whitespace */
 

In addition to Eric's comments...

What happens if the string consists *only* of whitespace?

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] remote-hg: do not fail on invalid bookmarks

[Max Horn]

Mercurial can have bookmarks pointing to nullid (the empty root
revision), while Git can not have references to it. When cloning or
fetching from a Mercurial repository that has such a bookmark, the
import failed because git-remote-hg was not be able to create the
corresponding reference.

Warn the user about the invalid reference, and do not advertise these
bookmarks as head refs, but otherwise continue the import. In
particular, we still keep track of the fact that the remote repository
has a bookmark of the given name, in case the user wants to modify that
bookmark.

Also add some test cases for this issue.

Reported-by: Antoine Pelisse apeli...@gmail.com
Signed-off-by: Max Horn m...@quendi.de
---
This is a different fix than in my previous attempts. I thought
a bit more about the issue, and determined that the previous fix,
while working, was not really correct: It is wrong to
treat nullid bookmarks as if they are non-existent; if e.g.
the user wants to modify the bookmark from git, we need to
into account that the remote already has a bookmark with that name.
Indeed, I extended the new test cases to cover this aspect.
With the previous fix, the new tests would fail upon pushing,
with the new one, they work.

 contrib/remote-helpers/git-remote-hg |  5 ++-
 contrib/remote-helpers/test-hg.sh| 67 
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index eb89ef6..36b5261 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -643,7 +643,10 @@ def do_list(parser):
 print ? refs/heads/branches/%s % gitref(branch)
 
 for bmark in bmarks:
-print ? refs/heads/%s % gitref(bmark)
+if  bmarks[bmark].hex() == '':
+warn(Ignoring invalid bookmark '%s', bmark)
+else:
+print ? refs/heads/%s % gitref(bmark)
 
 for tag, node in repo.tagslist():
 if tag == 'tip':
diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index a933b1e..f5d0d97 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -772,4 +772,71 @@ test_expect_success 'remote double failed push' '
)
 '
 
+test_expect_success 'clone remote with master null bookmark, then push to the 
bookmark' '
+   test_when_finished rm -rf gitrepo* hgrepo* 
+
+   (
+   hg init hgrepo 
+   cd hgrepo 
+   echo a a 
+   hg add a 
+   hg commit -m a 
+   hg bookmark -r null master
+   ) 
+
+   git clone hg::hgrepo gitrepo 
+   check gitrepo HEAD a 
+   cd gitrepo 
+   git checkout --quiet -b master 
+   echo b b 
+   git add b 
+   git commit -m b 
+   git push origin master
+'
+
+test_expect_success 'clone remote with default null bookmark, then push to the 
bookmark' '
+   test_when_finished rm -rf gitrepo* hgrepo* 
+
+   (
+   hg init hgrepo 
+   cd hgrepo 
+   echo a a 
+   hg add a 
+   hg commit -m a 
+   hg bookmark -r null -f default
+   ) 
+
+   git clone hg::hgrepo gitrepo 
+   check gitrepo HEAD a 
+   cd gitrepo 
+   git checkout --quiet -b default 
+   echo b b 
+   git add b 
+   git commit -m b 
+   git push origin default
+'
+
+test_expect_success 'clone remote with generic null bookmark, then push to the 
bookmark' '
+   test_when_finished rm -rf gitrepo* hgrepo* 
+
+   (
+   hg init hgrepo 
+   cd hgrepo 
+   echo a a 
+   hg add a 
+   hg commit -m a 
+   hg bookmark -r null bmark
+   ) 
+
+   git clone hg::hgrepo gitrepo 
+   check gitrepo HEAD a 
+   cd gitrepo 
+   git checkout --quiet -b bmark 
+   git remote -v 
+   echo b b 
+   git add b 
+   git commit -m b 
+   git push origin bmark
+'
+
 test_done
-- 
1.9.0.7.ga299b13


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] ls_colors.c: enable coloring on u+x files

[Duy Nguyen]

On Fri, Mar 21, 2014 at 12:41 AM, Junio C Hamano gits...@pobox.com wrote:
 Why even include cache.h for S_IXUSR?

 In the context of the patch I see S_ISGID mentioned and other S_*
 st_mode things are already in use in this function before this step,
 and presumably you are using them without problems, no?

My mistake. git-compat-util.h does include S_IXUSR. The original code
was S_IXUGO that's in neither files.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] Import $LS_COLORS parsing code from coreutils

[Duy Nguyen]

On Fri, Mar 21, 2014 at 2:09 AM, David Tran unsignedz...@gmail.com wrote:
 Nguyễn Thái Ngọc Duy pclouds at gmail.com writes:

 This could could help highlight files in ls-files or status output, or
 even diff --name-only (but that's questionable).

 This code is from coreutils.git commit
 7326d1f1a67edf21947ae98194f98c38b6e9e527 file src/ls.c. This is the
 last GPL-2 commit before coreutils turns to GPL-3.


 I don't know if this is something to consider but for my mac, I have another
 variable CLICOLOR which shows the colors if it is set. This is also true with
 FreeBSD[1] as well. I don't know if that should be checked if you're on those
 systems.

 I think it would be nice to have --color flag as well if you want to enable
 color output for just that one output.

My plan is stick to how git handles colors (e.g. --color and color.*
config variables). Is that enough or do you think git CLICOLOR should
override --color and color.*?


 [1]: https://unix.stackexchange.com/questions/2897/clicolor-and-ls-colors-in-
 bash

 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GSOC 2014]idea:Git Configuration API Improvement

[Matthieu Moy]

Mustafa Orkun Acar mustafaorkuna...@gmail.com writes:

 Hi, 
 I have completed my proposal about this project. But in one of the previous 
 emails; it says that the aim of the project is not storing configuration data 
 in the memory instead of making multiple git_config() calls. I
 also understand the project in this way. I need a clarification about it. 
 Thanks.

See my explanations at the bottom of

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

The goal _is_ to keep the configuration in memory, inside a single git
process. Not to maintain it in memory when the process dies (this would
require an additional daemon, which would be really overkill in our
case).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] doc: status, remove leftover statement about '#' prefix

[Dirk Wallenstein]

This hasn't been true since 2556b9962e7c0353d562b7bf70eed11d8f29d0b0

Signed-off-by: Dirk Wallenstein hals...@t-online.de
---
 Documentation/git-status.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index a4acaa0..def635f 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -97,7 +97,7 @@ configuration variable documented in linkgit:git-config[1].
 OUTPUT
 --
 The output from this command is designed to be used as a commit
-template comment, and all the output lines are prefixed with '#'.
+template comment.
 The default, long format, is designed to be human readable,
 verbose and descriptive.  Its contents and format are subject to change
 at any time.
-- 
1.9.0.1.g16216b6

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Want to start using Git on my windows 7 system.

[Izundu Kingsley]

Good Afternoon, am very curious about how powerful git can serve programmers 
and computer users. I will be glad if am helped with the git installation 
files, that is the files I will need to get git on my system and some hand-on 
materials for a quick get along. I will like to teach other of my friends about 
this as well. Thanks, I appreciate your pending assistance...Kingsley 
Sent from my BlackBerry wireless device from MTN

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit()

[Ashwin Jha]

 Original Message 
Subject:[PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit()
Date:   Fri, 21 Mar 2014 07:24:46 +0530
From:   Ashwin Jha ajha@gmail.com
To: git@vger.kernel.org
CC: Ashwin Jha ajha@gmail.com



modified fsck.c:fsck_commit(). Replaced memcmp() with starts_with() function.
starts_with() seems much more relevant than memcmp(). It uses one less argument
and its return value makes more sense.
skip_prefix() is not used as it uses strcmp() internally which seems 
unnecessarily
for current task. The current task can be easily done by providing offsets to 
the
buffer pointer (the way it is implemented currently).

Signed-off-by: Ashwin Jha ajha@gmail.com
---
 fsck.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fsck.c b/fsck.c
index 64bf279..82e1640 100644
--- a/fsck.c
+++ b/fsck.c
@@ -6,6 +6,7 @@
 #include commit.h
 #include tag.h
 #include fsck.h
+#include strbuf.h
 
 static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)

 {
@@ -290,12 +291,12 @@ static int fsck_commit(struct commit *commit, fsck_error 
error_func)
int parents = 0;
int err;
 
-	if (memcmp(buffer, tree , 5))

+   if (!starts_with(buffer, tree ))
return error_func(commit-object, FSCK_ERROR, invalid format - 
expected 'tree' line);
if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
return error_func(commit-object, FSCK_ERROR, invalid 'tree' line 
format - bad sha1);
buffer += 46;
-   while (!memcmp(buffer, parent , 7)) {
+   while (starts_with(buffer, parent )) {
if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n')
return error_func(commit-object, FSCK_ERROR, invalid 
'parent' line format - bad sha1);
buffer += 48;
@@ -322,15 +323,15 @@ static int fsck_commit(struct commit *commit, fsck_error 
error_func)
if (p || parents)
return error_func(commit-object, FSCK_ERROR, parent 
objects missing);
}
-   if (memcmp(buffer, author , 7))
+   if (!starts_with(buffer, author ))
return error_func(commit-object, FSCK_ERROR, invalid format - 
expected 'author' line);
buffer += 7;
err = fsck_ident(buffer, commit-object, error_func);
if (err)
return err;
-   if (memcmp(buffer, committer , strlen(committer )))
+   if (!starts_with(buffer, committer ))
return error_func(commit-object, FSCK_ERROR, invalid format - 
expected 'committer' line);
-   buffer += strlen(committer );
+   buffer += 10;
err = fsck_ident(buffer, commit-object, error_func);
if (err)
return err;
--
1.7.9.5



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] commit.c: use skip_prefix() instead of starts_with()

[Michael Haggerty]

On 03/04/2014 11:07 PM, Tanay Abhra wrote:
 In record_author_date()  parse_gpg_output(), the callers of
 starts_with() not just want to know if the string starts with the
 prefix, but also can benefit from knowing the string that follows
 the prefix.
 
 By using skip_prefix(), we can do both at the same time.
 
 Helped-by: Max Horn m...@quendi.de
 Helped-by: Junio C Hamano gits...@pobox.com
 Helped-by: Michael Haggerty mhag...@alum.mit.edu
 Signed-off-by: Tanay Abhra tanay...@gmail.com
 ---
 Patch V5 Minor revision of indentation
 Patch V4  Identation improved, removed useless comment. [1]
   Thanks to Junio C Hamano and Max Horn.
 [1] http://article.gmane.org/gmane.comp.version-control.git/243388
 
 Patch V3 Variable naming improved, removed assignments inside conditionals.
 Thanks to Junio C Hamano and Max Horn.
 
 Patch V2 Corrected email formatting ,reapplied the implementation according 
 to suggestions.
 Thanks to Michael Haggerty.
 
 This is in respect to GSoC microproject #10.
 
 In record_author_date(), extra and useless calls to strlen due to using 
 starts_with()
 were removed by using skip_prefix(). Extra variable ident_line was used as 
 buf is used in
 for loop update check.
 
 Other usages of starts_with() in the same file can be found with,
 
 $ grep -n starts_with commit.c
 
 1116:   else if (starts_with(line, gpg_sig_header) 
 1196:   if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
 
 The starts_with() in line 1116 was left as it is, as strlen values were pre 
 computed as
 global variables, and replacing may hamper the clarity.
 The starts_with() in line 1196 was replaced as it abstracts way the 
 skip_prefix part by
 directly using the function.
 Also skip_prefix() is inline when compared to starts_with().
 
 ---
  commit.c | 14 ++
  1 file changed, 6 insertions(+), 8 deletions(-)
 
 diff --git a/commit.c b/commit.c
 index 6bf4fe0..d37675c 100644
 --- a/commit.c
 +++ b/commit.c
 @@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);
  static void record_author_date(struct author_date_slab *author_date,
  struct commit *commit)
  {
 - const char *buf, *line_end;
 + const char *buf, *line_end, *ident_line;
   char *buffer = NULL;
   struct ident_split ident;
   char *date_end;
 @@ -566,14 +566,14 @@ static void record_author_date(struct author_date_slab 
 *author_date,
buf;
buf = line_end + 1) {
   line_end = strchrnul(buf, '\n');
 - if (!starts_with(buf, author )) {
 + ident_line = skip_prefix(buf, author );
 + if (!ident_line) {
   if (!line_end[0] || line_end[1] == '\n')
   return; /* end of header */
   continue;
   }
   if (split_ident_line(ident,
 -  buf + strlen(author ),
 -  line_end - (buf + strlen(author ))) ||
 +  ident_line, line_end - ident_line) ||
   !ident.date_begin || !ident.date_end)
   goto fail_exit; /* malformed author line */
   break;
 @@ -1193,10 +1193,8 @@ static void parse_gpg_output(struct signature_check 
 *sigc)
   for (i = 0; i  ARRAY_SIZE(sigcheck_gpg_status); i++) {
   const char *found, *next;
  
 - if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
 - /* At the very beginning of the buffer */
 - found = buf + strlen(sigcheck_gpg_status[i].check + 1);
 - } else {
 + found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
 + if(!found) {

Nit: There should be a space between if and the opening parenthesis.

   found = strstr(buf, sigcheck_gpg_status[i].check);
   if (!found)
   continue;
 

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] commit.c: use skip_prefix() instead of starts_with()

[Michael Haggerty]

On 03/21/2014 04:48 PM, Michael Haggerty wrote:
 On 03/04/2014 11:07 PM, Tanay Abhra wrote:
 [...]
 +found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
 +if(!found) {
 
 Nit: There should be a space between if and the opening parenthesis.

Oops, I see I am too late.  Junio must have fixed this when queuing the
patch.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Want to start using Git on my windows 7 system.

[Jim Garrison]

 -Original Message-
 Behalf Of Izundu Kingsley
 Sent: Friday, March 21, 2014 8:08 AM
 Subject: Want to start using Git on my windows 7 system.
 
 Good Afternoon, am very curious about how powerful git can serve
 programmers and computer users. I will be glad if am helped with the git
 installation files, that is the files I will need to get git on my system and 
 some
 hand-on materials for a quick get along. I will like to teach other of my 
 friends
 about this as well. Thanks, I appreciate your pending assistance...Kingsley
 Sent from my BlackBerry wireless device from MTN

Start here: http://git-scm.com/book

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

File extension conflict when working with git and latex

[Matthias Beyer]

Hi,

I struggled with a really nasty issue today (and yesterday):

I work on a semester project paper which is written in latex and
therefor version-controlled with git. We compile the document using
pdflatex, we automate the compiling with make.

pdflatex generates some temporary files, which are required if you
call it the second and third time (it is a top-down compiler, which
needs to be called several times). One of the file extensions is
.idx, which (kind of) conflicts with file extensions in .git/.

Unfortunetely, we wrote our `make clean` task recursively. I think you
can imagine what went wrong: The clean-task corrupted the repository,
as it removed .idx files from within .git/.

I lost work because of this ugly name collision.

My question now:

I know, I can fix this by fixing the clean task in my Makefile. But
maybe someone somewhere on this world doesn't know the git internals
as good as me (and, of course, my coworker). Is there _any chance
at all_ that this gets mentioned somewhere, so others don't fall into
this pit?

-- 
Mit freundlichen Grüßen,
Kind regards,
Matthias Beyer

Proudly sent with mutt.
Happily signed with gnupg.


pgpP45Ot9QiFb.pgp
Description: PGP signature

Re: File extension conflict when working with git and latex

[Simon Ruderich]

On Fri, Mar 21, 2014 at 05:13:24PM +0100, Matthias Beyer wrote:
 Unfortunetely, we wrote our `make clean` task recursively. I think you
 can imagine what went wrong: The clean-task corrupted the repository,
 as it removed .idx files from within .git/.

 I lost work because of this ugly name collision.

Hello Matthias,

You can recreate the .idx files by running

git index-pack .git/objects/pack/pack-hash.pack

for each pack file.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] Rewrite fsck.c:fsck_commit() replace memcmp() with starts_with()

[blacksimit]

From: Oguzhan Unlu cengoguzhanu...@gmail.com

My solution to make lines containing buffer += a_number; clearer to anyone is 
following; I defined a new int, magic_num, then assigned lengths of used 
strings to magic_num and then changed assignment lines through using magic_num 
so that where the number which is added to buffer is known although I 
eliminated 3rd parameter of memcmp() when using starts_with().   

Signed-off-by: Oguzhan Unlu cengoguzhanu...@gmail.com
---
 I didn't use skip_prefix() in this microproject and I plan to apply for GSOC 
2014. I expect your feedbacks!
 fsck.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/fsck.c b/fsck.c
index d43be98..4e5ca30 100644
--- a/fsck.c
+++ b/fsck.c
@@ -289,14 +289,17 @@ static int fsck_commit(struct commit *commit, fsck_error 
error_func)
struct commit_graft *graft;
int parents = 0;
int err;
-
+int magic_num;
+
+magic_num = strlen(tree ); /* magic_num is 5 */
if (!starts_with(buffer, tree ))
return error_func(commit-object, FSCK_ERROR, invalid format 
- expected 'tree' line);
-   if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
+   if (get_sha1_hex(buffer+magic_num, tree_sha1) || buffer[45] != '\n')
return error_func(commit-object, FSCK_ERROR, invalid 'tree' 
line format - bad sha1);
buffer += 46;
+magic_num = strlen(parent ); /* magic_num is 7 */
while (starts_with(buffer, parent )) {
-   if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n')
+   if (get_sha1_hex(buffer+magic_num, sha1) || buffer[47] != '\n')
return error_func(commit-object, FSCK_ERROR, invalid 
'parent' line format - bad sha1);
buffer += 48;
parents++;
@@ -322,15 +325,17 @@ static int fsck_commit(struct commit *commit, fsck_error 
error_func)
if (p || parents)
return error_func(commit-object, FSCK_ERROR, parent 
objects missing);
}
+magic_num = strlen(author ); /* magic_num is 7 */
if (!starts_with(buffer, author ))
return error_func(commit-object, FSCK_ERROR, invalid format 
- expected 'author' line);
-   buffer += 7;
+   buffer += magic_num;
err = fsck_ident(buffer, commit-object, error_func);
if (err)
return err;
+magic_num = strlen(committer); /* magic_num is 7 */
if (!starts_with(buffer, committer ))
return error_func(commit-object, FSCK_ERROR, invalid format 
- expected 'committer' line);
-   buffer += strlen(committer );
+   buffer += magic_num;
err = fsck_ident(buffer, commit-object, error_func);
if (err)
return err;
-- 
1.9.1.286.g5172cb3

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: File extension conflict when working with git and latex

[Matthias Beyer]

On 21-03-2014 17:36:28, Simon Ruderich wrote:
 On Fri, Mar 21, 2014 at 05:13:24PM +0100, Matthias Beyer wrote:
  Unfortunetely, we wrote our `make clean` task recursively. I think you
  can imagine what went wrong: The clean-task corrupted the repository,
  as it removed .idx files from within .git/.
 
  I lost work because of this ugly name collision.

 Hello Matthias,

 You can recreate the .idx files by running

 git index-pack .git/objects/pack/pack-hash.pack

 for each pack file.

Hi Simon,

I think so. I executed:

git fsck # reports N missing blobs, commits, trees and dangling stuff
git index-pack ...
git fsck # reports only dangling commits and blobs

I don't know if this means that the repository is fixed now?

-- 
Mit freundlichen Grüßen,
Kind regards,
Matthias Beyer

Proudly sent with mutt.
Happily signed with gnupg.


pgpxEdzzwXXTm.pgp
Description: PGP signature

Re: [PATCHv2] branch.c: simplify chain of if statements

[Junio C Hamano]

Dragos Foianu dragos.foi...@gmail.com writes:

 I'm not sure it's worth pursuing the table approach further, especially
 since a solution has already been accepted and merged into the codebase.

Yes.

I would further say that you already qualify as having finished a
microproject, if I were a part of the candidate selection panel.

The important thing is for potential candidates to learn the
process, not to have their change merged somewhere my tree, and you
and many others who did a microproject and tasted the process of
proposing a change, getting reviewed and learning what are expected
of their patch submissions have finished that part already.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] use starts_with() instead of !memcmp()

[Junio C Hamano]

Eric Sunshine sunsh...@sunshineco.com writes:

 On Tue, Mar 18, 2014 at 9:18 PM, Quint Guvernator
 quintus.pub...@gmail.com wrote:
 Another version, this time very in line with the review and commentary of
 Junio, Eric, and Michael.  This version boasts a revamped commit message and
 fewer but surer hunks changed.

 Explaining what changed in this version is indeed a courtesy to
 reviewers. Thanks.

So, is that a reviewed-by: Eric?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: File extension conflict when working with git and latex

[Simon Ruderich]

On Fri, Mar 21, 2014 at 05:46:51PM +0100, Matthias Beyer wrote:
 Hi Simon,

 I think so. I executed:

 git fsck # reports N missing blobs, commits, trees and dangling stuff
 git index-pack ...
 git fsck # reports only dangling commits and blobs

 I don't know if this means that the repository is fixed now?

Hello Matthias,

If nothing else was deleted, then yes, the repository should be
fine now.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] rev-parse --parseopt: option argument name hints

[Junio C Hamano]

Ilya Bobyr ilya.bo...@gmail.com writes:

 +   `arg_hing`, if specified, is used as a name of the argument in the
 +   help output, for options that take arguments. `arg_hint` is
 +   terminated by the first whitespace. When output the name is shown in
 +   angle braces.  Underscore symbols are replaced with spaces.
 The last part is troubling (and sounds not very sane).  Do we do
 such a munging anywhere else, or is it just here?  If the latter I'd
 prefer not to see such a hack.

 The following commands have spaces in argument names in the -h
 output for one or two arguments:
   * clone
   * commit
   * merge

 A number of commands use dashes to separate words in arguments names.

That was not what I asked.  I was asking if there is a precedent to
use you cannot have underscores in hint; they will be turned into
spaces quoting convention.  I do not think of any (we either do a
backslash-quote, c-quote inside dq-pair, or %20, depending on the
context).

Personally, because these hints are not even hints (they are more
like placeholders for value that makes it easier to refer to in the
description of an option [*1*]), I wouldn't shed tears if scripted
Porcelains cannot use a space in the argh.  In fact, it probably
makes the result harder to read and format more funnily if you had a
space in the argh string, be it in a subcommand implemented in C or
in a scripted Porcelain.

An optional argh is terminated by a whitespace is perfectly fine,
and by doing so we do not have to worry about having to introduce a
new quoting convention like you did, which is a big plus.


[Footnote]

*1* Perhaps like this:

--gpg-sign[=key-id]
Sign (with the key specified with key-id)

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

[Junio C Hamano]

Eric Sunshine sunsh...@sunshineco.com writes:

 Sorry, you're right about message[0] case not being a crasher (though
 the assert() still seems overkill).

Assert() often becomes no-op in production build.  I think this may
be an indication that table-driven may not be as good an approach
as many candidates thought.  The microproject suggestion asks them
to think _if_ that makes sense, and it is perfectly fine for them if
they answer no, it introduces more problems than it solves.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Mar 2014, #04; Thu, 20)

[kirr]

Junio,

On Thu, Mar 20, 2014 at 03:31:35PM -0700, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
  Quite a few topics are still outside 'pu' and I suspect some of the
  larger ones deserve deeper reviews to help moving them to 'next'.
  In principle, I'd prefer to keep any large topic that touch core
  part of the system cooking in 'next' for at least a full cycle, and
  the sooner they get merged to 'next', the better.  Help is greatly
  appreciated.
  ...
  * ks/tree-diff-nway (2014-03-04) 19 commits
   - combine-diff: speed it up, by using multiparent diff tree-walker directly
   - tree-diff: rework diff_tree() to generate diffs for multiparent cases as 
  well
   - Portable alloca for Git
   - tree-diff: reuse base str(buf) memory on sub-tree recursion
   - tree-diff: no need to call full diff_tree_sha1 from show_path()
   - tree-diff: rework diff_tree interface to be sha1 based
   - tree-diff: diff_tree() should now be static
   - tree-diff: remove special-case diff-emitting code for empty-tree cases
   - tree-diff: simplify tree_entry_pathcmp
   - tree-diff: show_path prototype is not needed anymore
   - tree-diff: rename compare_tree_entry - tree_entry_pathcmp
   - tree-diff: move all action-taking code out of compare_tree_entry()
   - tree-diff: don't assume compare_tree_entry() returns -1,0,1
   - tree-diff: consolidate code for emitting diffs and recursion in one place
   - tree-diff: show_tree() is not needed
   - tree-diff: no need to pass match to skip_uninteresting()
   - tree-diff: no need to manually verify that there is no mode change for a 
  path
   - combine-diff: move changed-paths scanning logic into its own function
   - combine-diff: move show_log_first logic/action out of paths scanning
 
   Instead of running N pair-wise diff-trees when inspecting a
   N-parent merge, find the set of paths that were touched by walking
   N+1 trees in parallel.  These set of paths can then be turned into
   N pair-wise diff-tree results to be processed through rename
   detections and such.  And N=2 case nicely degenerates to the usual
   2-way diff-tree, which is very nice.
 
 So I started re-reading this series, and decided that it might be
 easier to advance the topic piece-by-piece.  Will be merging up to
 consolidate code for emitting diffs to 'next', after tweaking that
 last commit a bit (see below).

Thanks, yes, I agree - merging it step-by-step is good approach as doing
it all at once requires more one-time effort, which is harder to do, and
otherwise the topic just stays without progress. So yes, let's do it
incrementally.


 Kirill Smelkov k...@mns.spb.ru writes:
 
  Currently both compare_tree_entry() and show_path() invoke opt diff
 
 s/show_path/show_entry/;

I agree, thanks for noticing.


  callbacks (opt-add_remove() and opt-change()), and also they both have
  code which decides whether to recurse into sub-tree, and whether to emit
  a tree as separate entry if DIFF_OPT_TREE_IN_RECURSIVE is set.
 
  I.e. we have code duplication and logic scattered on two places.
 
  Let's consolidate it...
  ...
  +/* convert path, t1/t2 - opt-diff_*() callbacks */
  +static void emit_diff(struct diff_options *opt, struct strbuf *path,
  + struct tree_desc *t1, struct tree_desc *t2)
  +{
  +   unsigned int mode1 = t1 ? t1-entry.mode : 0;
  +   unsigned int mode2 = t2 ? t2-entry.mode : 0;
  +
  +   if (mode1  mode2) {
  +   opt-change(opt, mode1, mode2, t1-entry.sha1, t2-entry.sha1,
  +   1, 1, path-buf, 0, 0);
 
 This is not too bad per-se, but it would have been even better if
 this part was done as:
 
   if (t1  t2) {
   opt-change(opt, t1-entry.mode1, t1-entry.mode2,
   t1-entry.sha1, t2-entry.sha1,
   1, 1, path-buf, 0, 0);
   }
 
 Then we do not have to rely on an extra convention, 'mode == 0'
 means the entry is missing, in addition to what we already have,
 i.e. t == NULL means the entry is missing.

I agree, but the reason it is done here so is to prepare for later changes in
tree-diff: rework diff_tree() to generate diffs for multiparent cases as
well, where modes will be right-available from `struct combine_diff_path` and
would also indicate absent entries:

-/* convert path, t1/t2 - opt-diff_*() callbacks */
-static void emit_diff(struct diff_options *opt, struct strbuf *path,
- struct tree_desc *t1, struct tree_desc *t2)
+/*
+ * convert path - opt-diff_*() callbacks
+ *
+ * emits diff to first parent only, and tells diff tree-walker that we are 
done
+ * with p and it can be freed.
+ */
+static int emit_diff_first_parent_only(struct diff_options *opt, struct 
combine_diff_path *p)
 {
-   unsigned int mode1 = t1 ? t1-entry.mode : 0;
-   unsigned int mode2 = t2 ? t2-entry.mode : 0;
-
-   if (mode1  mode2) {
-   opt-change(opt, mode1, mode2, 

Re: Configuring a third-party git hook

[Junio C Hamano]

Chris Angelico ros...@gmail.com writes:

 On Fri, Mar 21, 2014 at 2:43 PM, Jeff King p...@peff.net wrote:
 Thanks, the new text looks good to me. Please follow SubmittingPatches
 (notably, you need to sign-off your work, and please send patches inline
 rather than as attachments).

 Ah, didn't see that file.

It appears that we might need to be more explicit in that file,
though.


 From 6e1fc126ece37c6201d0c16b76c6c87781f7b02b Mon Sep 17 00:00:00 2001

Never paste the above line to your e-mail message.  It is only used
to separate individual messages/patches in the format-patch output.

 From: Chris Angelico ros...@gmail.com
 Date: Fri, 21 Mar 2014 10:45:08 +1100
 Subject: [PATCH] Explain that third-party tools may create 'git config'
  variables

You _may_ paste these in-body pseudo-header lines at the beginning of your
e-mail but (1) then these must be the first lines of your message,
not after doing random discussions at the beginning of the message
(you may separate that with scissors marker -- 8 --, though),
and (2) do so only they are used to correct what appears in the real
header lines in your e-mail message.

 * From:  is useful only when you are forwarding a patch written
   by somebody else; otherwise your authorship can be taken from the
   e-mail From:  header.

 * Date:  is the same way; Date : header in your e-mail is
   closer to the time wider world saw the change for the first time
   than when you made the commit, so it is usually not desired to
   see in-body pseudo-header.

 * Subject:  is used a lot more often than the above two,
   especially when you send a patch to an on-going discussion thread
   as a how about doing it this way? patch and do not want to
   change the e-mail Subject: (which may break the discussion
   thread).

Also I'd title the commit with the area it touches, i.e. starting it
with Explain blah is suboptimal.

Will queue with a minor tweak, with retitling the change and
rephrasing the ideally part, which invites people to say well it
may be so in the ideal world but the rule does not apply to me.

Thanks.

-- 8 --
From: Chris Angelico ros...@gmail.com
Date: Fri, 21 Mar 2014 15:07:08 +1100
Subject: [PATCH] config.txt: third-party tools may and do use their own 
variables

Signed-off-by: Chris Angelico ros...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/config.txt | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ab26963..a1ea605 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -131,8 +131,13 @@ Variables
 
 Note that this list is non-comprehensive and not necessarily complete.
 For command-specific variables, you will find a more detailed description
-in the appropriate manual page. You will find a description of non-core
-porcelain configuration variables in the respective porcelain documentation.
+in the appropriate manual page.
+
+Other git-related tools may and do use their own variables.  When
+inventing new variables for use in your own tool, make sure their
+names do not conflict with what are used by Git itself and other
+popular tools, and describe them in your documentation.
+
 
 advice.*::
These variables control various optional help messages designed to
-- 
1.9.1-443-g8f4a3d9

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] status: disable translation when --porcelain is used

[Junio C Hamano]

Matthieu Moy matthieu@grenoble-inp.fr writes:

 Junio C Hamano gits...@pobox.com writes:

 diff --git a/wt-status.c b/wt-status.c
 index a452407..e55e5b9 100644
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -1509,19 +1509,23 @@ static void wt_shortstatus_print_tracking(struct 
 wt_status *s)
 return;
 }
  
 +   const char *gone   = s-no_gettext ? gone   : _(gone);
 +   const char *behind = s-no_gettext ? behind  : _(behind );
 +   const char *ahead  = s-no_gettext ? ahead   : _(ahead );

 Having to repeat the same string constant twice (and a half for the
 variable name) each is an eyesore.  I wonder if we can do better,
 perhaps with:

 #define LABEL(string) (s-no_gettext ? (string) : _(string))

 and then

  color_fprintf(s-fp, header_color, LABEL(N_(gone)));

 or something along those lines?

 I first thought about trying something clever with the preprocessor, but
 since it's only for 3 strings, I went the KISS way. I tend to prefer my
 version for simplicity, but no strong opinion here.

Then I'll squash 61bf9709 (SQUASH??? fix decl-after-stmt and
simplify, 2014-03-20) in before merging the patch to 'next'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Git server memory requirements

[Cliff Brake]

I'm trying to get an idea how much memory is required for a git server
that is hosting linux kernel repos.

What we're seeing is that git uses around 1GB of RAM on the server
when a user does a clone of the Linux kernel source over ssh.  Does
this seem about right?  Is this amount fixed, or arbitrary (trade-off
memory for speed).  It seems subsequent concurrent clones use less
memory.

Is there any practical way to reduce the memory usage?  We're running
into occasional issues if there are multiple clones at once.

Is setting gc.auto=0 a good idea for large kernel repos?  The idea is
we can repack manually or in a cron on weekends rather than during
user operations.  However, manually running a git gc seems to use
about as much memory as a user clone.

It may be that our EC2 small instance (2.5GB) is not up to the task,
but would like to understand options (we can easily trade off some
speed for less memory if we can) before upgrading.

Thanks,
Cliff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [GSoC] Draft of Proposal for GSoC

[Junio C Hamano]

Brian Bourn ba.bo...@gmail.com writes:

 Something like this?

 Sample api calls
 Add_Opt_Group()
 Parse_with_contains()
 Parse_with_merged()
 Parse_with_no_merged()
 Parse_with_formatting()
 (each of the 4 calls above may have internal calls within the library
 in order to parse the option for each of the different function which
 may call these functions)

This list is a bit too sketchy to be called sample api calls, at
least to me.  Can you elaborate a bit more?

What do they do, what does the caller expect to see (do they get
something as return values?  do they expect some side effects?)?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Configuring a third-party git hook

[Chris Angelico]

On Sat, Mar 22, 2014 at 4:31 AM, Junio C Hamano gits...@pobox.com wrote:
 Chris Angelico ros...@gmail.com writes:

 On Fri, Mar 21, 2014 at 2:43 PM, Jeff King p...@peff.net wrote:
 Thanks, the new text looks good to me. Please follow SubmittingPatches
 (notably, you need to sign-off your work, and please send patches inline
 rather than as attachments).

 Ah, didn't see that file.

 It appears that we might need to be more explicit in that file,
 though.
 [chomp specifics]

Please do. I read through the file as a set of instructions, and would
have followed them if they'd been there. Fitting into a project like
that is what those sorts of guides are for.

 Also I'd title the commit with the area it touches, i.e. starting it
 with Explain blah is suboptimal.

Interestingly, this is exactly what my hook is for! It searches for
previous commits touching that file, looks for something separated off
by a colon, and pre-fills the commit message with that. (If there are
multiple options, they're all listed, commented out. Otherwise, it's
put in without a leading hash, so I just hit the End key - I use nano
for commit messages - and start typing.)

 Will queue with a minor tweak, with retitling the change and
 rephrasing the ideally part, which invites people to say well it
 may be so in the ideal world but the rule does not apply to me.

Awesome. I tried to keep it brief (and the ideally was from the
point of view of someone trying to configure someone else's tool), but
explicitly talking about creating new variables makes that even
clearer. Thanks.

ChrisA
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] Fix misuses of nor in comments

[Junio C Hamano]

Justin Lebar jle...@google.com writes:

 Thanks for the quick reply.

 When I send a new patch, should I fold these changes into the original
 commit, or should I send them as a separate commit?

 diff --git a/builtin/apply.c b/builtin/apply.c
 index b0d0986..6013e19 100644
 --- a/builtin/apply.c
 +++ b/builtin/apply.c
 @@ -4061,7 +4061,7 @@ static int write_out_one_reject(struct patch *patch)
 return error(_(cannot open %s: %s), namebuf, 
 strerror(errno));

 /* Normal git tools never deal with .rej, so do not pretend
 -* this is a git patch by saying --git nor give extended
 +* this is a git patch by saying --git or giving extended
  * headers.  While at it, maybe please kompare that wants
  * the trailing TAB and some garbage at the end of line ;-).
  */

 I don't think the change from give to giving here is grammatically 
 correct.

 Is it?  I might be misunderstanding the sentence, then.  I parse the
 new sentence as...

The new sentence should say what the original wanted to say, which I
think was:

- Do not pretend this is a git patch by saying --git
- Do not show extended headers.

I however think that extended headers is one attribute of a patch
being a git patch, so I would say that the break down of your new
version:

   Do not pretend this is a git patch by
   - saying --git, or
   - giving extended headers.

makes sense.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] Fix misuses of nor in comments

[Junio C Hamano]

Justin Lebar jle...@google.com writes:

 Thanks for the quick reply.

 When I send a new patch, should I fold these changes into the original
 commit, or should I send them as a separate commit?

While a patch is still in an early discussion stage, consider their
earlier incarnation rejected and send them afresh with [PATCH v2]
(or v3, v4,...) when rerolling.

When you do this kind of tree-wide clean-up, please make sure that
your patch applies cleanly to 'maint', 'master', 'next' and 'pu'
branches, to check if you are touching some area that are undergoing
other changes.  If you find conflicts, please remove overlapping
parts from your main patch, make the removed parts into separate
patches that can be applied on top once these other topics that are
in flight are ready to be merged.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] doc: status, remove leftover statement about '#' prefix

[Junio C Hamano]

Dirk Wallenstein hals...@t-online.de writes:

 This hasn't been true since 2556b9962e7c0353d562b7bf70eed11d8f29d0b0

 Signed-off-by: Dirk Wallenstein hals...@t-online.de
 ---

Good eyes.  Thanks.

  Documentation/git-status.txt | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
 index a4acaa0..def635f 100644
 --- a/Documentation/git-status.txt
 +++ b/Documentation/git-status.txt
 @@ -97,7 +97,7 @@ configuration variable documented in linkgit:git-config[1].
  OUTPUT
  --
  The output from this command is designed to be used as a commit
 -template comment, and all the output lines are prefixed with '#'.
 +template comment.
  The default, long format, is designed to be human readable,
  verbose and descriptive.  Its contents and format are subject to change
  at any time.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [GSoC] Draft of Proposal for GSoC

[Brian Bourn]

On Fri, Mar 21, 2014 at 1:45 PM, Junio C Hamano gits...@pobox.com wrote:
 Brian Bourn ba.bo...@gmail.com writes:

 Something like this?

 Sample api calls
 Add_Opt_Group()
 Parse_with_contains()
 Parse_with_merged()
 Parse_with_no_merged()
 Parse_with_formatting()
 (each of the 4 calls above may have internal calls within the library
 in order to parse the option for each of the different function which
 may call these functions)

 This list is a bit too sketchy to be called sample api calls, at
 least to me.  Can you elaborate a bit more?

 What do they do, what does the caller expect to see (do they get
 something as return values?  do they expect some side effects?)?

so something like this would be better I'm assuming?

Some basic sample API calls are found below, each of these would hold
code to complete parsing and/or formatting the flags.
Add_Opt_Group() - returns an OPT_CALLBACK with contains, merged,
no-merged, or formatting which can be used in a commands options list.

Execute_list()-the main call into the library and would pass into the
library all of the necessary flags and arguments for parsing the
request and executing it. This would accept the flags like
-contain, with arguments such as the commit or pattern that is being
searched for.

The next four commands would be called by execute_list() to execute
the original command with respect to the flags that are passed into
this library.
Parse_with_contains()
Parse_with_merged()
Parse_with_no_merged()
Parse_with_formatting()
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] use starts_with() instead of !memcmp()

[Eric Sunshine]

On Fri, Mar 21, 2014 at 12:53 PM, Junio C Hamano gits...@pobox.com wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:

 On Tue, Mar 18, 2014 at 9:18 PM, Quint Guvernator
 quintus.pub...@gmail.com wrote:
 Another version, this time very in line with the review and commentary of
 Junio, Eric, and Michael.  This version boasts a revamped commit message and
 fewer but surer hunks changed.

 Explaining what changed in this version is indeed a courtesy to
 reviewers. Thanks.

 So, is that a reviewed-by: Eric?

No, sorry. You and Peff were actively reviewing Quint's submissions,
so I merely scanned them quickly without making a careful examination.
I've been commenting upon so many GSoC submissions that it's hard to
remember which is which, and upon reading Another version, this time
very in line with the review and commentary Junio, Eric, and Michael,
I had to search the list archive to figure out why my name was listed.
Hence, the suggestion from me that providing a link to the previous
attempt is a welcome courtesy.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [GSoC] Draft of Proposal for GSoC

[Jeff King]

On Fri, Mar 21, 2014 at 02:03:41PM -0400, Brian Bourn wrote:

  What do they do, what does the caller expect to see (do they get
  something as return values?  do they expect some side effects?)?
 
 so something like this would be better I'm assuming?
 
 Some basic sample API calls are found below, each of these would hold
 code to complete parsing and/or formatting the flags.
 Add_Opt_Group() - returns an OPT_CALLBACK with contains, merged,
 no-merged, or formatting which can be used in a commands options list.
 
 Execute_list()-the main call into the library and would pass into the
 library all of the necessary flags and arguments for parsing the
 request and executing it. This would accept the flags like
 -contain, with arguments such as the commit or pattern that is being
 searched for.
 
 The next four commands would be called by execute_list() to execute
 the original command with respect to the flags that are passed into
 this library.
 Parse_with_contains()
 Parse_with_merged()
 Parse_with_no_merged()
 Parse_with_formatting()

Think about how the callers would use them. Will git-branch just call
Parse_with_contains? If so, where would that call go? What arguments
would it take, and what would it do?

I don't think those calls are enough. We probably need:

  1. Some structure to represent a list of refs and store its
 intermediate state.

  2. Some mechanism for telling that structure about the various
 filters, sorters, and formatters we want to use (and this needs to
 be hooked into the option-parsing somehow).

  3. Some mechanism for getting the listed refs out of that structure,
 formatting them, etc.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Configuring a third-party git hook

[Jeff King]

On Fri, Mar 21, 2014 at 10:31:59AM -0700, Junio C Hamano wrote:

 -- 8 --
 From: Chris Angelico ros...@gmail.com
 Date: Fri, 21 Mar 2014 15:07:08 +1100
 Subject: [PATCH] config.txt: third-party tools may and do use their own 
 variables
 [...]
 +Other git-related tools may and do use their own variables.  When
 +inventing new variables for use in your own tool, make sure their
 +names do not conflict with what are used by Git itself and other
 +popular tools, and describe them in your documentation.

I think this third line should be with what _is_ used to match the
verb and noun pluralness[1]. Or to keep better parallel structure with
the first clause, something like ...their names do not conflict with
those that are used by Git

-Peff

[1] Is there a word to mean the pluralness of a noun or verb (similar
to tense for a verb). Surely there is, but I could not think of
it. I wanted to say here that the pluralness of what and are
does not match (it seems like what is a mass noun, which usually
matches a singular verb).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: File extension conflict when working with git and latex

[Junio C Hamano]

Matthias Beyer m...@beyermatthias.de writes:

 I know, I can fix this by fixing the clean task in my Makefile. But
 maybe someone somewhere on this world doesn't know the git internals
 as good as me (and, of course, my coworker). Is there _any chance
 at all_ that this gets mentioned somewhere, so others don't fall into
 this pit?

Surely, we are here to please ;-)  All of us want to make sure
newbies do not shoot themselves in the foot.

But the problem is what exactly should be mentioned.  With a fresh
wound with your LaTeX project still in your mind, you may be tempted
to special case .idx, but other newbies may inflict the same kind
of hurt on themselves with different find patterns, e.g.

$ find . -name '[0-9a-f]*[0-9a-f]' -type f -print | xargs rm -f

when they know their project creates hexadecimal-numbered temoprary
files, or whatever other pattern that match the files they do not
care about, that also happens to match whatever is in $GIT_DIR.  The
only common caution that helps us to make sure others do not fall
into this pit is Files and directories in $GIT_DIR are used to
record your work; do not muck with them unless you know what you are
doing e.g. manually repairing a corrupt repository, but that is a
bit lame, isn't it?

It is tempting to suggest git clean '*.idx', but that is a good
fit in the Makefile only when you know everybody involved in the
project works in a checkout from Git, not from a tarball extract,
and does not apply to projects in general.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [GSoC] Draft of Proposal for GSoC

[Brian Bourn]

On Fri, Mar 21, 2014 at 2:07 PM, Jeff King p...@peff.net wrote:
 On Fri, Mar 21, 2014 at 02:03:41PM -0400, Brian Bourn wrote:

  What do they do, what does the caller expect to see (do they get
  something as return values?  do they expect some side effects?)?

 so something like this would be better I'm assuming?

 Some basic sample API calls are found below, each of these would hold
 code to complete parsing and/or formatting the flags.
 Add_Opt_Group() - returns an OPT_CALLBACK with contains, merged,
 no-merged, or formatting which can be used in a commands options list.

 Execute_list()-the main call into the library and would pass into the
 library all of the necessary flags and arguments for parsing the
 request and executing it. This would accept the flags like
 -contain, with arguments such as the commit or pattern that is being
 searched for.

 The next four commands would be called by execute_list() to execute
 the original command with respect to the flags that are passed into
 this library.
 Parse_with_contains()
 Parse_with_merged()
 Parse_with_no_merged()
 Parse_with_formatting()

 Think about how the callers would use them. Will git-branch just call
 Parse_with_contains? If so, where would that call go? What arguments
 would it take, and what would it do?

 I don't think those calls are enough. We probably need:

   1. Some structure to represent a list of refs and store its
  intermediate state.

   2. Some mechanism for telling that structure about the various
  filters, sorters, and formatters we want to use (and this needs to
  be hooked into the option-parsing somehow).

   3. Some mechanism for getting the listed refs out of that structure,
  formatting them, etc.

keeping some of my function calls to do the actual work I think I
settled on this

A possible API is given below, each of these would hold code to
complete parsing and/or formatting the flags.

There will be a struct in the library called refs_list() which when
initialized will iterate through all the refs in a repository and add
them to this list.

 there would be a function which would retrieve ref structs from that function.

Get_ref_from_list()- which would return a single ref from the list.

Add_Opt_Group() - returns an OPT_CALLBACK with contains, merged,
no-merged, or formatting which can be used in a commands options list.

Execute_list()-the main call into the library and would pass into the
library all of the necessary flags and arguments for parsing the
request and executing it. This would accept the flags like contain,
with arguments such as the commit or pattern that is being searched
for. This will then parse the refs_list using the four commands below
to make, sort, filter, and format an output list which will then be
printed or returned by this function.

Any Call into the API from an outside source would call one of the
previous two functions, all other commands in the API would be for
internal use only, in order to simplify the process of calling into
this library.

The next four commands would be called by execute_list() to further
format the refs_list with respect to the flags that are passed into
this library. These would also take the additional arguments from
execute_list() such as patterns to parse or which commit to filter
out. these calls would modify the refs_list for eventual printing.

Parse_list _with_contains()

Parse_list_with_merged()

Parse_list_with_no_merged()

Format_list()

of course this would still depend on deciding whether or not we want
to return to the original command to print or if printing can be
handled by the library itself.


 -Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Enable index-pack threading in msysgit.

[Karsten Blees]

Am 20.03.2014 02:25, schrieb Duy Nguyen:
 On Thu, Mar 20, 2014 at 4:35 AM, Stefan Zager sza...@chromium.org wrote:
 This adds a Windows implementation of pread.  Note that it is NOT
 safe to intersperse calls to read() and pread() on a file
 descriptor.  According to the ReadFile spec, using the 'overlapped'
 argument should not affect the implicit position pointer of the
 descriptor.  Experiments have shown that this is, in fact, a lie.

 To accomodate that fact, this change also incorporates:

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

 ... which gives each index-pack thread its own file descriptor.
 
 If the problem is mixing read() and pread() then perhaps it's enough to do
 
 output_fd = dup(output_fd);
 

Unfortunately not, dup() / DuplicateHandle() just opens another handle to the 
same file object (i.e. sharing the same file position).

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit()

[Eric Sunshine]

On Fri, Mar 21, 2014 at 5:18 AM, Ashwin Jha ajha@gmail.com wrote:
 On Fri, Mar 21, 2014 at 9:03 AM, Eric Sunshine sunsh...@sunshineco.com
 wrote:
 On Thu, Mar 20, 2014 at 9:54 PM, Ashwin Jha ajha@gmail.com wrote:
  Subject: [PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit()
  starts_with() seems much more relevant than memcmp(). It uses one less
  argument and its return value makes more sense.

 As a justification, uses one less argument falls flat, and really
 has nothing to do with the decision to make the change. The bit about
 the return value is a slightly better but is still weak.

 You might instead justify the change by pointing out that the name
 starts_with()
 does a better job of conveying the intention of the code, which is to
 check the string for a prefix, than does memcmp().

 Actually, from the line starts_with() seems much more relevant than
 memcmp() my intention was to say that starts_with() does a better job of
 conveying the intention of the code, which is to check the string for a
 prefix, than does memcmp() as mentioned by you.

Good to hear. When you resubmit (if you do), perhaps use that wording
or something similar to justify the change.

  skip_prefix() is not used as it uses strcmp() internally which seems
  unnecessarily
  for current task. The current task can be easily done by providing
  offsets to the
  buffer pointer (the way it is implemented currently).

 Not sure what this means. What is the current task, and what is
 implemented where currently?

 From current task, I meant to say the task of offsetting the buffer pointer
 to get the correct substring as in:
 get_sha1_hex(buffer+5, tree_sha1)

 Please forgive me for this. I should have written this in a better way.

Thanks for the clarification.

  Signed-off-by: Ashwin Jha ajha@gmail.com
  ---
   fsck.c |   11 ++-
   1 file changed, 6 insertions(+), 5 deletions(-)
 
  diff --git a/fsck.c b/fsck.c
  index 64bf279..82e1640 100644
  --- a/fsck.c
  +++ b/fsck.c
  @@ -6,6 +6,7 @@
   #include commit.h
   #include tag.h
   #include fsck.h
  +#include strbuf.h
 
   static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void
  *data)
   {
  @@ -290,12 +291,12 @@ static int fsck_commit(struct commit *commit,
  fsck_error error_func)
  int parents = 0;
  int err;
 
  -   if (memcmp(buffer, tree , 5))
  +   if (!starts_with(buffer, tree ))
  return error_func(commit-object, FSCK_ERROR, invalid
  format - expected 'tree' line);
  if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')

 One of the benefits of starts_with() and skip_prefix() is that they
 allow you to eliminate magic numbers, such as 5 in the memcmp()
 invocation. However, if you look a couple lines below, you see in the
 expression 'buffer+5' that the magic number is still present. In fact,
 the code becomes less clear with your change because the 5 in
 'buffer+5' is much more mysterious without the preceding
 memcmp(foo,bar,5). It is possible to eliminate this magic number,
 but starts_with() is not the answer.

 I considered this point while making the changes. But, I thought that since
 all that is required is a constant offset to the buffer pointer, using
 skip_prefix() will only add to the overhead of function calling.

  return error_func(commit-object, FSCK_ERROR, invalid
  'tree' line format - bad sha1);
  buffer += 46;

 And as you can see here (buffer +=46) will still be a problem even if I
 replace the buffer+5 code.
 I think a more better way would be to define these magic no. as macros.

 But, I guess you are right. The current changes do make it a bit unclear.

I understand your argument: since magic numbers remain elsewhere, then
little is gained by eliminating only a few of them via skip_prefix().
A counterargument might be that even that small gain can be a
maintenance bonus, since it reduces the number of potential places
where errors can be made when modifying the code. (But you are welcome
to counter that argument if you feel strongly about it.)

 To summarize, I can think of two ways:
 1. skip_prefix() can be used, in place of both starts_with() and memcmp().
 The return value of skip_prefix can
 be checked against NULL to determine whether correct format is used or
 not.
 Though, even this change will left some of the magic no (as shown
 above). ;-)
 2. Define macros for all the magic no. (and tags like tree, parent
 etc.). This way the code will be more clear
 and any future changes to these magic no. (or tag names) will be much
 easier to handle.

Perhaps provide an illustration to explain what you mean.

 In my opinion, 2 will be a better option. But, I can understand that I may
 have overlooked some potential flaws in this method.
 Please guide me to the correct approach. :-)

There isn't necessarily one correct approach. Judging from reviewer
responses to submissions by other GSoC hopefuls who tackled this

Re: [PATCH v2] Rewrite fsck.c:fsck_commit() replace memcmp() with starts_with()

[Junio C Hamano]

blacksimit cengoguzhanu...@gmail.com writes:

 From: Oguzhan Unlu cengoguzhanu...@gmail.com

 My solution to make lines containing buffer += a_number; clearer to anyone is 
 following; I defined a new int, magic_num, then assigned lengths of used 
 strings to magic_num and then changed assignment lines through using 
 magic_num so that where the number which is added to buffer is known although 
 I eliminated 3rd parameter of memcmp() when using starts_with().   
Eric told you in $gmane/244637:

 Wrap messages to 65-70 characters.

I do not think replacing 5 (or 7) with a variable makes anything
clearer; in fact, by forcing people to check what the last value
assigned to the variable every time they see +magic_num, the
resulting code is much harder to read, I would think.

Among the various submissions, I found this one one of the cleaner
solutions:

http://thread.gmane.org/gmane.comp.version-control.git/244019/focus=244020

and it has been queued as 2d820a61 (fsck.c:fsck_commit(): use
skip_prefix() to verify and skip constant, 2014-03-13) on 'pu'.

 Signed-off-by: Oguzhan Unlu cengoguzhanu...@gmail.com
 ---
  I didn't use skip_prefix() in this microproject and I plan to apply for GSOC 
 2014. I expect your feedbacks!
  fsck.c | 15 ++-
  1 file changed, 10 insertions(+), 5 deletions(-)

 diff --git a/fsck.c b/fsck.c
 index d43be98..4e5ca30 100644
 --- a/fsck.c
 +++ b/fsck.c
 @@ -289,14 +289,17 @@ static int fsck_commit(struct commit *commit, 
 fsck_error error_func)
   struct commit_graft *graft;
   int parents = 0;
   int err;
 -
 +int magic_num;
 +
 +magic_num = strlen(tree ); /* magic_num is 5 */
   if (!starts_with(buffer, tree ))
   return error_func(commit-object, FSCK_ERROR, invalid format 
 - expected 'tree' line);
 - if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
 + if (get_sha1_hex(buffer+magic_num, tree_sha1) || buffer[45] != '\n')
   return error_func(commit-object, FSCK_ERROR, invalid 'tree' 
 line format - bad sha1);
   buffer += 46;
 +magic_num = strlen(parent ); /* magic_num is 7 */
   while (starts_with(buffer, parent )) {
 - if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n')
 + if (get_sha1_hex(buffer+magic_num, sha1) || buffer[47] != '\n')
   return error_func(commit-object, FSCK_ERROR, invalid 
 'parent' line format - bad sha1);
   buffer += 48;
   parents++;
 @@ -322,15 +325,17 @@ static int fsck_commit(struct commit *commit, 
 fsck_error error_func)
   if (p || parents)
   return error_func(commit-object, FSCK_ERROR, parent 
 objects missing);
   }
 +magic_num = strlen(author ); /* magic_num is 7 */
   if (!starts_with(buffer, author ))
   return error_func(commit-object, FSCK_ERROR, invalid format 
 - expected 'author' line);
 - buffer += 7;
 + buffer += magic_num;
   err = fsck_ident(buffer, commit-object, error_func);
   if (err)
   return err;
 +magic_num = strlen(committer); /* magic_num is 7 */
   if (!starts_with(buffer, committer ))
   return error_func(commit-object, FSCK_ERROR, invalid format 
 - expected 'committer' line);
 - buffer += strlen(committer );
 + buffer += magic_num;
   err = fsck_ident(buffer, commit-object, error_func);
   if (err)
   return err;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Configuring a third-party git hook

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Fri, Mar 21, 2014 at 10:31:59AM -0700, Junio C Hamano wrote:

 -- 8 --
 From: Chris Angelico ros...@gmail.com
 Date: Fri, 21 Mar 2014 15:07:08 +1100
 Subject: [PATCH] config.txt: third-party tools may and do use their own 
 variables
 [...]
 +Other git-related tools may and do use their own variables.  When
 +inventing new variables for use in your own tool, make sure their
 +names do not conflict with what are used by Git itself and other
 +popular tools, and describe them in your documentation.

 I think this third line should be with what _is_ used to match the
 verb and noun pluralness[1]. Or to keep better parallel structure with
 the first clause, something like ...their names do not conflict with
 those that are used by Git

Thanks. I'll amend to do the those that are.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Enable index-pack threading in msysgit.

[Karsten Blees]

Am 21.03.2014 06:35, schrieb Stefan Zager:
 On Thu, Mar 20, 2014 at 10:21 PM, Duy Nguyen pclo...@gmail.com wrote:
 On Fri, Mar 21, 2014 at 08:51:18AM +0700, Duy Nguyen wrote:
 On Thu, Mar 20, 2014 at 11:08 PM, Stefan Zager sza...@chromium.org wrote:
 Duy, would you like to re-post your patch without the new pread 
 implementation?

 I will but let me try out the sliding window idea first. My quick
 tests on git.git show me we may only need 21k mmap instead of 177k
 pread. That hints some potential performance improvement.

 The patch at the bottom reuses (un)use_pack() instead of pread(). The
 results on linux-2.6 do not look any different. I guess we can drop
 the idea.

 It makes me wonder, though, what's wrong a simple patch like this to
 make pread in index-pack thread-safe? It does not look any different
 either from the performance point of view, perhaps because
 unpack_data() reads small deltas most of the time
 
 When you serialize disk access in this way, the effect on performance
 is really dependent on the behavior of the OS, as well as the locality
 of the read offsets.  Assuming -- fairly, I think -- that the reads
 will be pretty randomly distributed (i.e., no locality to speak of),
 then your best bet is get as many read operations in flight as
 possible, and let the disk scheduler optimize the seek time.
 

The read() implementation in MSVCRT.DLL is synchronized anyway, and I strongly 
suspect that this is also true for ReadFile() (at least for synchronous file 
handles, i.e. opened without FILE_FLAG_OVERLAPPED). So I guess separate file 
descriptors would help with parallel IO as well.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Configuring a third-party git hook

[Junio C Hamano]

Jeff King p...@peff.net writes:

 [1] Is there a word to mean the pluralness of a noun or verb (similar
 to tense for a verb).

I've seen plural vs singular often mentioned in the context of
subject and verb agreement.

en.wiktionary.org/wiki/concord talks about agreement in gender,
number, person, or case, so number may be the word you are
looking for.

http://en.wikipedia.org/wiki/Grammatical_number

 Surely there is, but I could not think of
 it. I wanted to say here that the pluralness of what and are
 does not match (it seems like what is a mass noun, which usually
 matches a singular verb).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] diff-no-index.c: rename read_directory()

[Andrei Dinu]

A subsequent patch will include dir.h in diff-no-index.c to 
access is_dot_or_dotdot(), however, dir.h declared a 
read_directory() which conflicts with a (different) 
read_directory() defined in diff-no-index.c. Rename
read_directory() from diff-no-index.c to avoid the conflict.

Signed-off-by: Andrei Dinu mandrei.d...@gmail.com

---

 I plan on applying to GSoc 2014

 I received your feedback and I resend the patches from the bug that I
 solved in the past

 diff-no-index.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 8e10bff..5e4a76c 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -16,7 +16,7 @@
 #include builtin.h
 #include string-list.h
 
-static int read_directory(const char *path, struct string_list *list)
+static int read_directory_path(const char *path, struct string_list *list)
 {
DIR *dir;
struct dirent *e;
@@ -107,9 +107,9 @@ static int queue_diff(struct diff_options *o,
int i1, i2, ret = 0;
size_t len1 = 0, len2 = 0;
 
-   if (name1  read_directory(name1, p1))
+   if (name1  read_directory_path(name1, p1))
return -1;
-   if (name2  read_directory(name2, p2)) {
+   if (name2  read_directory_path(name2, p2)) {
string_list_clear(p1, 0);
return -1;
}
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] diff-no-index.c: read_directory_path() use is_dot_or_dotdot()

[Andrei Dinu]

Use is_dot_or_dotdot() instead of manually checking against . or ...

Signed-off-by: Andrei Dinu mandrei.d...@gmail.com

---

 You can check the last version here: 

 http://thread.gmane.org/gmane.comp.version-control.git/244578

 I received the feedback and make the changes.

 I plan on applying to GSoC 2014

 diff-no-index.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 5e4a76c..2d1165f 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -15,6 +15,7 @@
 #include log-tree.h
 #include builtin.h
 #include string-list.h
+#include dir.h
 
 static int read_directory_path(const char *path, struct string_list *list)
 {
@@ -25,7 +26,7 @@ static int read_directory_path(const char *path, struct 
string_list *list)
return error(Could not open directory %s, path);
 
while ((e = readdir(dir)))
-   if (strcmp(., e-d_name)  strcmp(.., e-d_name))
+   if (!is_dot_or_dotdot(e-d_name))
string_list_insert(list, e-d_name);
 
closedir(dir);
-- 
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Rewrite fsck.c:fsck_commit() replace memcmp() with starts_with()

[Matthieu Moy]

blacksimit cengoguzhanu...@gmail.com writes:


 -
 +int magic_num;
 +
 +magic_num = strlen(tree ); /* magic_num is 5 */
   if (!starts_with(buffer, tree ))

Whitespace damage. It seems you have set your tab-width to something
other than 8, and indented with spaces. Please don't do either.

 +magic_num = strlen(committer); /* magic_num is 7 */

Typical example of a counter-productive comment. A good comment usually
explains _why_ the code is as it is, and not _what_ it is doing. C is a
much better lanuage than english to describe algorithm, so if you want
magic_num to become equal to 7, then write magic_num = 7 in code, not
in a comment.

Here, the reader has to spend time and energy to check the
correspondance between the code and the redundant comment ... and see
than they do not match!

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] diff-no-index.c: rename read_directory()

[Eric Sunshine]

On Fri, Mar 21, 2014 at 2:56 PM, Andrei Dinu mandrei.d...@gmail.com wrote:
 Subject: [PATCH 1/2] diff-no-index.c: rename read_directory()

It is helpful to reviewers if you indicate that this is a resubmission
by placing 'vN' inside [...], where N is the reroll number. For
instance, if this is your fourth version of the patch, you would say
[PATCH v4 1/2]. The -v option of git format-patch can help.

 A subsequent patch will include dir.h in diff-no-index.c to
 access is_dot_or_dotdot(), however, dir.h declared a

s/declared/declares/

 read_directory() which conflicts with a (different)
 read_directory() defined in diff-no-index.c. Rename
 read_directory() from diff-no-index.c to avoid the conflict.

 Signed-off-by: Andrei Dinu mandrei.d...@gmail.com

 ---

  I plan on applying to GSoc 2014

  I received your feedback and I resend the patches from the bug that I
  solved in the past

Other than the minor comment above about vN and the typo, both patches
look fine. Thanks.

No need to resend.

  diff-no-index.c |6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/diff-no-index.c b/diff-no-index.c
 index 8e10bff..5e4a76c 100644
 --- a/diff-no-index.c
 +++ b/diff-no-index.c
 @@ -16,7 +16,7 @@
  #include builtin.h
  #include string-list.h

 -static int read_directory(const char *path, struct string_list *list)
 +static int read_directory_path(const char *path, struct string_list *list)
  {
 DIR *dir;
 struct dirent *e;
 @@ -107,9 +107,9 @@ static int queue_diff(struct diff_options *o,
 int i1, i2, ret = 0;
 size_t len1 = 0, len2 = 0;

 -   if (name1  read_directory(name1, p1))
 +   if (name1  read_directory_path(name1, p1))
 return -1;
 -   if (name2  read_directory(name2, p2)) {
 +   if (name2  read_directory_path(name2, p2)) {
 string_list_clear(p1, 0);
 return -1;
 }
 --
 1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Rewrite fsck.c:fsck_commit() replace memcmp() with starts_with()

[Eric Sunshine]

In addition to the valuable review comments by Junio and Matthieu, see
a few more below...

On Fri, Mar 21, 2014 at 12:37 PM, blacksimit cengoguzhanu...@gmail.com wrote:
 From: Oguzhan Unlu cengoguzhanu...@gmail.com

 My solution to make lines containing buffer += a_number; clearer to anyone is 
 following; I defined a new int, magic_num, then assigned lengths of used 
 strings to magic_num and then changed assignment lines through using 
 magic_num so that where the number which is added to buffer is known although 
 I eliminated 3rd parameter of memcmp() when using starts_with().

The process you describe here is effectively what skip_prefix() does
except that you are doing the bookkeeping manually, whereas
skip_prefix() handles the details for you. This should be a good clue
that skip_prefix() is a better solution.

When writing a commit message, try to keep it impersonal; omit
statements like My solution and I defined.

Use imperative mood, which means that the commit message should tell
the code how to change itself. For instance replace X with Y or
retry operation X upon failure.

Let the code speak for itself. It's obvious that skip_prefix() takes
one less argument than memcmp(), so there is no need to talk about it
in the commit message.

Finally, explain why the change is a good thing.

 Signed-off-by: Oguzhan Unlu cengoguzhanu...@gmail.com
 ---
  I didn't use skip_prefix() in this microproject and I plan to apply for GSOC 
 2014. I expect your feedbacks!
  fsck.c | 15 ++-
  1 file changed, 10 insertions(+), 5 deletions(-)

 diff --git a/fsck.c b/fsck.c
 index d43be98..4e5ca30 100644
 --- a/fsck.c
 +++ b/fsck.c
 @@ -289,14 +289,17 @@ static int fsck_commit(struct commit *commit, 
 fsck_error error_func)
 struct commit_graft *graft;
 int parents = 0;
 int err;
 -
 +int magic_num;
 +
 +magic_num = strlen(tree ); /* magic_num is 5 */
 if (!starts_with(buffer, tree ))
 return error_func(commit-object, FSCK_ERROR, invalid 
 format - expected 'tree' line);
 -   if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
 +   if (get_sha1_hex(buffer+magic_num, tree_sha1) || buffer[45] != '\n')
 return error_func(commit-object, FSCK_ERROR, invalid 
 'tree' line format - bad sha1);
 buffer += 46;
 +magic_num = strlen(parent ); /* magic_num is 7 */
 while (starts_with(buffer, parent )) {
 -   if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n')
 +   if (get_sha1_hex(buffer+magic_num, sha1) || buffer[47] != 
 '\n')
 return error_func(commit-object, FSCK_ERROR, 
 invalid 'parent' line format - bad sha1);
 buffer += 48;
 parents++;
 @@ -322,15 +325,17 @@ static int fsck_commit(struct commit *commit, 
 fsck_error error_func)
 if (p || parents)
 return error_func(commit-object, FSCK_ERROR, 
 parent objects missing);
 }
 +magic_num = strlen(author ); /* magic_num is 7 */
 if (!starts_with(buffer, author ))
 return error_func(commit-object, FSCK_ERROR, invalid 
 format - expected 'author' line);
 -   buffer += 7;
 +   buffer += magic_num;
 err = fsck_ident(buffer, commit-object, error_func);
 if (err)
 return err;
 +magic_num = strlen(committer); /* magic_num is 7 */
 if (!starts_with(buffer, committer ))
 return error_func(commit-object, FSCK_ERROR, invalid 
 format - expected 'committer' line);
 -   buffer += strlen(committer );
 +   buffer += magic_num;
 err = fsck_ident(buffer, commit-object, error_func);
 if (err)
 return err;
 --
 1.9.1.286.g5172cb3

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit()

[Ashwin Jha]

On 03/22/2014 12:11 AM, Eric Sunshine wrote:

On Fri, Mar 21, 2014 at 5:18 AM, Ashwin Jha ajha@gmail.com wrote:

On Fri, Mar 21, 2014 at 9:03 AM, Eric Sunshine sunsh...@sunshineco.com
wrote:

On Thu, Mar 20, 2014 at 9:54 PM, Ashwin Jha ajha@gmail.com wrote:

Subject: [PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit()
starts_with() seems much more relevant than memcmp(). It uses one less
argument and its return value makes more sense.

As a justification, uses one less argument falls flat, and really
has nothing to do with the decision to make the change. The bit about
the return value is a slightly better but is still weak.

You might instead justify the change by pointing out that the name
starts_with()
does a better job of conveying the intention of the code, which is to
check the string for a prefix, than does memcmp().

Actually, from the line starts_with() seems much more relevant than
memcmp() my intention was to say that starts_with() does a better job of
conveying the intention of the code, which is to check the string for a
prefix, than does memcmp() as mentioned by you.

Good to hear. When you resubmit (if you do), perhaps use that wording
or something similar to justify the change.


skip_prefix() is not used as it uses strcmp() internally which seems
unnecessarily
for current task. The current task can be easily done by providing
offsets to the
buffer pointer (the way it is implemented currently).

Not sure what this means. What is the current task, and what is
implemented where currently?

 From current task, I meant to say the task of offsetting the buffer pointer
to get the correct substring as in:
get_sha1_hex(buffer+5, tree_sha1)

Please forgive me for this. I should have written this in a better way.

Thanks for the clarification.


Signed-off-by: Ashwin Jha ajha@gmail.com
---
  fsck.c |   11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fsck.c b/fsck.c
index 64bf279..82e1640 100644
--- a/fsck.c
+++ b/fsck.c
@@ -6,6 +6,7 @@
  #include commit.h
  #include tag.h
  #include fsck.h
+#include strbuf.h

  static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void
*data)
  {
@@ -290,12 +291,12 @@ static int fsck_commit(struct commit *commit,
fsck_error error_func)
 int parents = 0;
 int err;

-   if (memcmp(buffer, tree , 5))
+   if (!starts_with(buffer, tree ))
 return error_func(commit-object, FSCK_ERROR, invalid
format - expected 'tree' line);
 if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')

One of the benefits of starts_with() and skip_prefix() is that they
allow you to eliminate magic numbers, such as 5 in the memcmp()
invocation. However, if you look a couple lines below, you see in the
expression 'buffer+5' that the magic number is still present. In fact,
the code becomes less clear with your change because the 5 in
'buffer+5' is much more mysterious without the preceding
memcmp(foo,bar,5). It is possible to eliminate this magic number,
but starts_with() is not the answer.


I considered this point while making the changes. But, I thought that since
all that is required is a constant offset to the buffer pointer, using
skip_prefix() will only add to the overhead of function calling.

 return error_func(commit-object, FSCK_ERROR, invalid
'tree' line format - bad sha1);
 buffer += 46;

And as you can see here (buffer +=46) will still be a problem even if I
replace the buffer+5 code.
I think a more better way would be to define these magic no. as macros.

But, I guess you are right. The current changes do make it a bit unclear.

I understand your argument: since magic numbers remain elsewhere, then
little is gained by eliminating only a few of them via skip_prefix().
A counterargument might be that even that small gain can be a
maintenance bonus, since it reduces the number of potential places
where errors can be made when modifying the code. (But you are welcome
to counter that argument if you feel strongly about it.)


To summarize, I can think of two ways:
1. skip_prefix() can be used, in place of both starts_with() and memcmp().
The return value of skip_prefix can
 be checked against NULL to determine whether correct format is used or
not.
 Though, even this change will left some of the magic no (as shown
above). ;-)
2. Define macros for all the magic no. (and tags like tree, parent
etc.). This way the code will be more clear
 and any future changes to these magic no. (or tag names) will be much
easier to handle.

Perhaps provide an illustration to explain what you mean.
I think you want some explanation on point 2. What I have suggested here 
is that
all the keywords (like tree, parent) and magic no. (which are 
nothing but suitable

pointer offsets, used to fetch these keywords) be defined as macros.
This will serve two purposes:
1. The code will be more readable in the sense that each magic no. will 
have a meaningful name.

Re: [PATCH] Enable index-pack threading in msysgit.

[Karsten Blees]

Am 20.03.2014 22:56, schrieb Stefan Zager:
 On Thu, Mar 20, 2014 at 2:35 PM, Karsten Blees karsten.bl...@gmail.com 
 wrote:
 Am 20.03.2014 17:08, schrieb Stefan Zager:

 Going forward, there is still a lot of performance that gets left on
 the table when you rule out threaded file access.  There are not so
 many calls to read, mmap, and pread in the code; it should be possible
 to rationalize them and make them thread-safe -- at least, thread-safe
 for posix-compliant systems and msysgit, which covers the great
 majority of git users, I would hope.


 IMO a mostly XSI compliant pread (or even the git_pread() emulation) is 
 still better than forbidding the use of read() entirely. Switching from read 
 to pread everywhere requires that all callers have to keep track of the file 
 position, which means a _lot_ of code changes (read/xread/strbuf_read is 
 used in ~70 places throughout git). And how do you plan to deal with 
 platforms that don't have a thread-safe pread (HP, Cygwin)?

 Considering all that, Duy's solution of opening separate file descriptors 
 per thread seems to be the best pattern for future multi-threaded work.
 
 Does that mean you would endorse the (N threads) * (M pack files)
 approach to threading checkout and status?  That seems kind of
 crazy-town to me.  Not to mention that pack windows are not shared, so
 this approach to multi-threading can have the side-effect of blowing
 out memory consumption.  We have already had to dial back settings for
 pack.threads and core.deltaBaseCacheLimit, because threaded index-pack
 was causing OOM errors on 32-bit platforms.
 

Opening more file descriptors doesn't significantly increase the memory 
footprint, so it shouldn't matter whether the threads read data via shared or 
private descriptors.

git-status with core.preloadindex is already multithreaded (at least the first 
part), and AFAIK doesn't read pack files at all.

I'm still not convinced that multi-threaded git-checkout is a good idea. 
According to my tests this is actually slower than sequential checkout. You'd 
have to be very careful to only multi-thread the parts that don't do any IO, 
such as unpacking / undeltifying.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] Import $LS_COLORS parsing code from coreutils

[David Tran]

Duy Nguyen pclouds at gmail.com writes:

 On Fri, Mar 21, 2014 at 2:09 AM, David Tran unsignedzero at gmail.com
wrote:
  Nguyễn Thái Ngọc Duy pclouds at gmail.com writes:
 
  This could could help highlight files in ls-files or status output, or
  even diff --name-only (but that's questionable).
 
  This code is from coreutils.git commit
  7326d1f1a67edf21947ae98194f98c38b6e9e527 file src/ls.c. This is the
  last GPL-2 commit before coreutils turns to GPL-3.
 
 
  I don't know if this is something to consider but for my mac, I have another
  variable CLICOLOR which shows the colors if it is set. This is also true
with
  FreeBSD[1] as well. I don't know if that should be checked if you're on
those
  systems.
 
  I think it would be nice to have --color flag as well if you want to enable
  color output for just that one output.
 
 My plan is stick to how git handles colors (e.g. --color and color.*
 config variables). Is that enough or do you think git CLICOLOR should
 override --color and color.*?
 
I would say it is not an essential feature to have but something that might
be looked into once the color is implemented. If its not set, ignore it. If
it is set, check if it is truthy, is what I would do.




--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git server memory requirements

[Robin H. Johnson]

On Fri, Mar 21, 2014 at 01:38:01PM -0400,  Cliff Brake wrote:
 I'm trying to get an idea how much memory is required for a git server
 that is hosting linux kernel repos.
Speaking for Gentoo here, as we're working on our large repo migration,
and this was a concern originally. So far it's best mitigated by the
pack-bitmap patches in the next branch.

The initial clones become significantly faster and much less resource
intensive.

I'm hoping that the packed bitmap series lands soon in the master branch
and a formal release (a big +1 from me for it).

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Infrastructure Lead
E-Mail : robb...@gentoo.org
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/12] t0001: drop useless subshells

[Eric Sunshine]

On Thu, Mar 20, 2014 at 7:21 PM, Jeff King p...@peff.net wrote:
 Many tests use subshells, but don't actually change the
 shell environment. They were probably cargo-culted from
 earlier tests which did need subshells. Drop the useless
 ones.

 Signed-off-by: Jeff King p...@peff.net
 ---
 These ones should produce no behavior change at all; they're purely
 mechanical (foo  bar) to foo  bar (though of course I did them
 by hand, because you need to know that foo and bar do not affect the
 environment).

  t/t0001-init.sh | 61 
 +
  1 file changed, 22 insertions(+), 39 deletions(-)

 diff --git a/t/t0001-init.sh b/t/t0001-init.sh
 index 4560bba..55a68bc 100755
 --- a/t/t0001-init.sh
 +++ b/t/t0001-init.sh
 @@ -297,30 +286,24 @@ test_expect_success POSIXPERM 'init creates a new deep 
 directory (umask vs. shar

  test_expect_success 'init notices EEXIST (2)' '
 rm -fr newdir 
 -   (
 -   mkdir newdir 
 -   newdir/a
 -   test_must_fail git init newdir/a/b 
 -   test_path_is_file newdir/a
 -   )
 +   mkdir newdir 
 +   newdir/a

Broken -chain (though, not introduced by this patch).

 +   test_must_fail git init newdir/a/b 
 +   test_path_is_file newdir/a
  '

  test_expect_success POSIXPERM,SANITY 'init notices EPERM' '
 rm -fr newdir 
 -   (
 -   mkdir newdir 
 -   chmod -w newdir 
 -   test_must_fail git init newdir/a/b
 -   )
 +   mkdir newdir 
 +   chmod -w newdir 
 +   test_must_fail git init newdir/a/b
  '

  test_expect_success 'init creates a new bare directory with global --bare' '
 --
 1.9.0.560.g01ceb46
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GUILT 00/28] Teach guilt import-commit how create legal patch names, and more

[Jeff Sipek]

On Fri, Mar 21, 2014 at 08:31:38AM +0100, Per Cederqvist wrote:
 I recently found myself sitting on a train with a computer in front of
 me.  I tried to use guilt import-commit, which seemed to work, but
 when I tried to guilt push the commits I had just imported I got
 some errors.  It turned out that guilt import-commit had generated
 invalid patch names.
 
 I decided to fix the issue, and write a test case that demonstrated
 the problem.
 
 One thing led to another, and here I am, a few late nights at a hotel
 and a return trip on the train later, submitting a patch series in 28
 parts.  Sorry about the number of patches, but this is what happens
 when you uncover a bug while writing a test case for the bug you
 uncovered while writing a test case for your original problem.

No problem.  I prefer large number of patches instead of a big wad that's
much harder to follow.

 The patch series consists of:
...
  - Changed behavior: guilt push when there is nothing more to push
now uses exit status 1.  This makes it possible to write shell
loops such as while guilt push; do make test||break; done.  Also,
guilt pop when no patches are applied also uses exit status 1.
(This aligns guilt push and guilt pop with how hg qpush and
hg qpop has worked for several years.)

Sounds fine.

  - Changed behavior: by default, guilt no longer changes branch when
you push a patch.  You need to do git config guilt.reusebranch
false to re-enable that.  This patch sets the default value of
guilt.reusebranch to true; it should in my opinion change to false
a year or two after the next release.

Probably a fair thing to do.  I should really make a release soon :/

I'm sending this off before I go through all the patches so you know that
I've seen this and plan to comment/pull.  It'll probably take a bit to go
through all 28 :)

Thanks,

Jeff.

-- 
Ready; T=0.01/0.01 08:47:23
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] remote-hg: do not fail on invalid bookmarks

[Torsten Bögershausen]

On 2014-03-21 12.36, Max Horn wrote:
All tests passed :-), thanks from my side.
comments inline, some are debatable
 Mercurial can have bookmarks pointing to nullid (the empty root
 revision), while Git can not have references to it. When cloning or
 fetching from a Mercurial repository that has such a bookmark, the
 import failed because git-remote-hg was not be able to create the
 corresponding reference.

 Warn the user about the invalid reference, and do not advertise these
 bookmarks as head refs, but otherwise continue the import. In
 particular, we still keep track of the fact that the remote repository
 has a bookmark of the given name, in case the user wants to modify that
 bookmark.

 Also add some test cases for this issue.
s/some test cases/test cases/

 Reported-by: Antoine Pelisse apeli...@gmail.com
 Signed-off-by: Max Horn m...@quendi.de
 ---
 This is a different fix than in my previous attempts. I thought
 a bit more about the issue, and determined that the previous fix,
 while working, was not really correct: It is wrong to
 treat nullid bookmarks as if they are non-existent; if e.g.
 the user wants to modify the bookmark from git, we need to
 into account that the remote already has a bookmark with that name.
 Indeed, I extended the new test cases to cover this aspect.
 With the previous fix, the new tests would fail upon pushing,
 with the new one, they work.

  contrib/remote-helpers/git-remote-hg |  5 ++-
  contrib/remote-helpers/test-hg.sh| 67 
 
  2 files changed, 71 insertions(+), 1 deletion(-)

 diff --git a/contrib/remote-helpers/git-remote-hg 
 b/contrib/remote-helpers/git-remote-hg
 index eb89ef6..36b5261 100755
 --- a/contrib/remote-helpers/git-remote-hg
 +++ b/contrib/remote-helpers/git-remote-hg
 @@ -643,7 +643,10 @@ def do_list(parser):
  print ? refs/heads/branches/%s % gitref(branch)
  
  for bmark in bmarks:
 -print ? refs/heads/%s % gitref(bmark)
 +if  bmarks[bmark].hex() == 
 '':
 +warn(Ignoring invalid bookmark '%s', bmark)
 +else:
 +print ? refs/heads/%s % gitref(bmark)
  
  for tag, node in repo.tagslist():
  if tag == 'tip':
 diff --git a/contrib/remote-helpers/test-hg.sh 
 b/contrib/remote-helpers/test-hg.sh
 index a933b1e..f5d0d97 100755
 --- a/contrib/remote-helpers/test-hg.sh
 +++ b/contrib/remote-helpers/test-hg.sh
 @@ -772,4 +772,71 @@ test_expect_success 'remote double failed push' '
   )
  '
  
 +test_expect_success 'clone remote with master null bookmark, then push to 
 the bookmark' '
 + test_when_finished rm -rf gitrepo* hgrepo* 
 +
 + (
 + hg init hgrepo 
 + cd hgrepo 
Minor:
We can change the order here, to make the cd hgrepo the first line in the 
subshell:

+   hg init hgrepo 
+   (
+   cd hgrepo 


 + echo a a 
 + hg add a 
 + hg commit -m a 
 + hg bookmark -r null master
 + ) 
 +
 + git clone hg::hgrepo gitrepo 
 + check gitrepo HEAD a 
And here we do cd, and this should be done in a subshell
 + cd gitrepo 
 + git checkout --quiet -b master 
 + echo b b 
 + git add b 
 + git commit -m b 
 + git push origin master
 +'
 +
 +test_expect_success 'clone remote with default null bookmark, then push to 
 the bookmark' '
 + test_when_finished rm -rf gitrepo* hgrepo* 
 +
 + (
 + hg init hgrepo 
 + cd hgrepo 
(Same minor as above)
 + echo a a 
 + hg add a 
 + hg commit -m a 
 + hg bookmark -r null -f default
 + ) 
 +
 + git clone hg::hgrepo gitrepo 
 + check gitrepo HEAD a 
 + cd gitrepo 
 + git checkout --quiet -b default 
 + echo b b 
 + git add b 
 + git commit -m b 
 + git push origin default
 +'
 +
 +test_expect_success 'clone remote with generic null bookmark, then push to 
 the bookmark' '
 + test_when_finished rm -rf gitrepo* hgrepo* 
 +
 + (
 + hg init hgrepo 
 + cd hgrepo 
(Same as above)
 + echo a a 
 + hg add a 
 + hg commit -m a 
 + hg bookmark -r null bmark
 + ) 
 +
 + git clone hg::hgrepo gitrepo 
 + check gitrepo HEAD a 
 + cd gitrepo 
Sub-shell missing
 + git checkout --quiet -b bmark 
 + git remote -v 
 + echo b b 
 + git add b 
 + git commit -m b 
 + git push origin bmark
 +'
 +
  test_done

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Mar 2014, #04; Thu, 20)

[Junio C Hamano]

Torsten Bögershausen tbo...@web.de writes:

 On 03/20/2014 10:09 PM, Junio C Hamano wrote:
 * ap/remote-hg-skip-null-bookmarks (2014-03-19) 1 commit
   - remote-hg: do not fail on invalid bookmarks

   Will merge to 'next'.
 Hmm, am I the only one who has 11 failures in test-hg-hg-git.sh,
 like this:
 (Tested under Debian 7, commit 04570b241ddb53ad2f355977bae541bd7ff32af5)



 master cde5672] clear executable bit
  Author: A U Thor aut...@example.com
  1 file changed, 0 insertions(+), 0 deletions(-)
  mode change 100755 = 100644 alpha
 WARNING: Ignoring invalid bookmark 'master'
 To hg::../hgrepo-git
  ! [remote rejected] master - master
 error: failed to push some refs to 'hg::../hgrepo-git'
 not ok 1 - executable bit
 []
 # failed 11 among 11 test(s)
 1..11

This has again been replaced; I'll queue it as d06f17f8 (remote-hg:
do not fail on invalid bookmarks, 2014-03-21) on 'pu'.

Please holler if this still breaks for you.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/10] userdiff: cpp pattern simplification and test framework

[Johannes Sixt]

Here is a series that makes the hunk header pattern for C and C++ even
simpler than suggested by Peff in [1] to catch a lot more C++ functions
and two more C patterns.

As a preparatory work, the test cases are totally rewritten to make it
a lot simpler to drop in new tests. There was an earlier attempt to
change the infrastructure [2], and it is the reason for the widened Cc
list.

Two patches also extend the word regexp.

[1] http://article.gmane.org/gmane.comp.version-control.git/243408
[2] http://thread.gmane.org/gmane.comp.version-control.git/187269/focus=187497

Johannes Sixt (10):
  userdiff: support C++ -* and .* operators in the word regexp
  userdiff: support unsigned and long long suffixes of integer constants
  t4018: an infrastructure to test hunk headers
  t4018: convert perl pattern tests to the new infrastructure
  t4018: convert java pattern test to the new infrastructure
  t4018: convert custom pattern test to the new infrastructure
  t4018: reduce test files for pattern compilation tests
  t4018: test cases for the built-in cpp pattern
  t4018: test cases showing that the cpp pattern misses many anchor
points
  userdiff: have 'cpp' hunk header pattern catch more C++ anchor points

 t/t4018-diff-funcname.sh   | 230 ++---
 t/t4018/README |  18 +++
 t/t4018/cpp-c++-function   |   4 +
 t/t4018/cpp-class-constructor  |   4 +
 t/t4018/cpp-class-constructor-mem-init |   5 +
 t/t4018/cpp-class-definition   |   4 +
 t/t4018/cpp-class-definition-derived   |   5 +
 t/t4018/cpp-class-destructor   |   4 +
 t/t4018/cpp-function-returning-global-type |   4 +
 t/t4018/cpp-function-returning-nested  |   5 +
 t/t4018/cpp-function-returning-pointer |   4 +
 t/t4018/cpp-function-returning-reference   |   4 +
 t/t4018/cpp-gnu-style-function |   5 +
 t/t4018/cpp-namespace-definition   |   4 +
 t/t4018/cpp-operator-definition|   4 +
 t/t4018/cpp-skip-access-specifiers |   8 +
 t/t4018/cpp-skip-comment-block |   9 ++
 t/t4018/cpp-skip-labels|   8 +
 t/t4018/cpp-struct-definition  |   9 ++
 t/t4018/cpp-struct-single-line |   7 +
 t/t4018/cpp-template-function-definition   |   4 +
 t/t4018/cpp-union-definition   |   4 +
 t/t4018/cpp-void-c-function|   4 +
 t/t4018/custom1-pattern|  17 +++
 t/t4018/custom2-match-to-end-of-line   |   8 +
 t/t4018/custom3-alternation-in-pattern |  17 +++
 t/t4018/java-class-member-function |   8 +
 t/t4018/perl-skip-end-of-heredoc   |   8 +
 t/t4018/perl-skip-forward-decl |  10 ++
 t/t4018/perl-skip-sub-in-pod   |  18 +++
 t/t4018/perl-sub-definition|   4 +
 t/t4018/perl-sub-definition-kr-brace   |   4 +
 userdiff.c |  12 +-
 33 files changed, 303 insertions(+), 160 deletions(-)
 create mode 100644 t/t4018/README
 create mode 100644 t/t4018/cpp-c++-function
 create mode 100644 t/t4018/cpp-class-constructor
 create mode 100644 t/t4018/cpp-class-constructor-mem-init
 create mode 100644 t/t4018/cpp-class-definition
 create mode 100644 t/t4018/cpp-class-definition-derived
 create mode 100644 t/t4018/cpp-class-destructor
 create mode 100644 t/t4018/cpp-function-returning-global-type
 create mode 100644 t/t4018/cpp-function-returning-nested
 create mode 100644 t/t4018/cpp-function-returning-pointer
 create mode 100644 t/t4018/cpp-function-returning-reference
 create mode 100644 t/t4018/cpp-gnu-style-function
 create mode 100644 t/t4018/cpp-namespace-definition
 create mode 100644 t/t4018/cpp-operator-definition
 create mode 100644 t/t4018/cpp-skip-access-specifiers
 create mode 100644 t/t4018/cpp-skip-comment-block
 create mode 100644 t/t4018/cpp-skip-labels
 create mode 100644 t/t4018/cpp-struct-definition
 create mode 100644 t/t4018/cpp-struct-single-line
 create mode 100644 t/t4018/cpp-template-function-definition
 create mode 100644 t/t4018/cpp-union-definition
 create mode 100644 t/t4018/cpp-void-c-function
 create mode 100644 t/t4018/custom1-pattern
 create mode 100644 t/t4018/custom2-match-to-end-of-line
 create mode 100644 t/t4018/custom3-alternation-in-pattern
 create mode 100644 t/t4018/java-class-member-function
 create mode 100644 t/t4018/perl-skip-end-of-heredoc
 create mode 100644 t/t4018/perl-skip-forward-decl
 create mode 100644 t/t4018/perl-skip-sub-in-pod
 create mode 100644 t/t4018/perl-sub-definition
 create mode 100644 t/t4018/perl-sub-definition-kr-brace

-- 
1.8.5.2.244.g9fb3fb1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/10] userdiff: support unsigned and long long suffixes of integer constants

[Johannes Sixt]

Do not split constants such as 123U, 456ll, 789UL at the first U or
second L.

Signed-off-by: Johannes Sixt j...@kdbg.org
---
 userdiff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/userdiff.c b/userdiff.c
index 434535b..8830417 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -132,7 +132,7 @@
 ^((struct|class|enum)[^;]*)$,
 /* -- */
 [a-zA-Z_][a-zA-Z0-9_]*
-|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?
+|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*
 |[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*),
 PATTERNS(csharp,
 /* Keywords */
-- 
1.8.5.2.244.g9fb3fb1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/10] userdiff: support C++ -* and .* operators in the word regexp

[Johannes Sixt]

The character sequences -* and .* are valid C++ operators. Keep them
together in --word-diff mode.

Signed-off-by: Johannes Sixt j...@kdbg.org
---
 userdiff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/userdiff.c b/userdiff.c
index 10b61ec..434535b 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -133,7 +133,7 @@
 /* -- */
 [a-zA-Z_][a-zA-Z0-9_]*
 |[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?
-|[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-),
+|[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*),
 PATTERNS(csharp,
 /* Keywords */
 !^[ 
\t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n
-- 
1.8.5.2.244.g9fb3fb1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/10] t4018: convert java pattern test to the new infrastructure

[Johannes Sixt]

Signed-off-by: Johannes Sixt j...@kdbg.org
---
 t/t4018-diff-funcname.sh   | 4 
 t/t4018/java-class-member-function | 8 
 2 files changed, 8 insertions(+), 4 deletions(-)
 create mode 100644 t/t4018/java-class-member-function

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index c94a5f4..008325f 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -79,10 +79,6 @@ test_expect_success 'set up .gitattributes declaring drivers 
to test' '
EOF
 '
 
-test_expect_success 'preset java pattern' '
-   test_expect_funcname public static void main(
-'
-
 test_expect_success 'custom pattern' '
test_config diff.java.funcname !static
 !String
diff --git a/t/t4018/java-class-member-function 
b/t/t4018/java-class-member-function
new file mode 100644
index 000..298bc7a
--- /dev/null
+++ b/t/t4018/java-class-member-function
@@ -0,0 +1,8 @@
+public class Beer
+{
+   int special;
+   public static void main(String RIGHT[])
+   {
+   System.out.print(ChangeMe);
+   }
+}
-- 
1.8.5.2.244.g9fb3fb1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/10] t4018: an infrastructure to test hunk headers

[Johannes Sixt]

Add an infrastructure that simplifies adding new tests of the hunk
header regular expressions.

To add new tests, a file with the syntax to test can be dropped in the
directory t4018. The README file explains how a test file must contain;
the README itself tests the default behavior.

Signed-off-by: Johannes Sixt j...@kdbg.org
---
 t/t4018-diff-funcname.sh | 60 +++-
 t/t4018/README   | 18 +++
 2 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 t/t4018/README

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 38a092a..b467d9e 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -100,7 +100,25 @@ test_expect_funcname () {
grep ^@@.*@@ $1 diff
 }
 
-for p in ada bibtex cpp csharp fortran html java matlab objc pascal perl php 
python ruby tex
+diffpatterns=
+   ada
+   bibtex
+   cpp
+   csharp
+   fortran
+   html
+   java
+   matlab
+   objc
+   pascal
+   perl
+   php
+   python
+   ruby
+   tex
+
+
+for p in $diffpatterns
 do
test_expect_success builtin $p pattern compiles '
echo *.java diff=$p .gitattributes 
@@ -118,11 +136,6 @@ do
'
 done
 
-test_expect_success 'default behaviour' '
-   rm -f .gitattributes 
-   test_expect_funcname public class Beer\$
-'
-
 test_expect_success 'set up .gitattributes declaring drivers to test' '
cat .gitattributes -\EOF
*.java diff=java
@@ -182,4 +195,39 @@ test_expect_success 'alternation in pattern' '
test_expect_funcname public static void main(
 '
 
+test_expect_success 'setup hunk header tests' '
+   for i in $diffpatterns
+   do
+   echo $i-* diff=$i
+   done  .gitattributes 
+
+   # add all test files to the index
+   (
+   cd $TEST_DIRECTORY/t4018 
+   git --git-dir=$TRASH_DIRECTORY/.git add .
+   ) 
+
+   # place modified files in the worktree
+   for i in $(git ls-files)
+   do
+   sed -e s/ChangeMe/IWasChanged/ $TEST_DIRECTORY/t4018/$i 
$i || return 1
+   done
+'
+
+# check each individual file
+for i in $(git ls-files)
+do
+   if grep broken $i /dev/null 21
+   then
+   result=failure
+   else
+   result=success
+   fi
+   test_expect_$result hunk header: $i 
+   test_when_finished 'cat actual'   # for debugging only
+   git diff -U1 $i actual 
+   grep '@@ .* @@.*RIGHT' actual
+   
+done
+
 test_done
diff --git a/t/t4018/README b/t/t4018/README
new file mode 100644
index 000..283e01cc
--- /dev/null
+++ b/t/t4018/README
@@ -0,0 +1,18 @@
+How to write RIGHT test cases
+=
+
+Insert the word ChangeMe (exactly this form) at a distance of
+at least two lines from the line that must appear in the hunk header.
+
+The text that must appear in the hunk header must contain the word
+right, but in all upper-case, like in the title above.
+
+To mark a test case that highlights a malfunction, insert the word
+BROKEN in all lower-case somewhere in the file.
+
+This text is a bit twisted and out of order, but it is itself a
+test case for the default hunk header pattern. Know what you are doing
+if you change it.
+
+BTW, this tests that the head line goes to the hunk header, not the line
+of equal signs.
-- 
1.8.5.2.244.g9fb3fb1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html