Another slash at index size

2013-02-21 Thread Nguyễn Thái Ngọc Duy
I noticed that even with v4, we still duplicate a lot of info in the
remaining fields. ce_uid and ce_guid for instance are unlikely to
change ever between entries. So I attempt to store offsets between the
previous entry instead. The result looks good. This is webkit index:

 25M index-v2
 14M index-v4
7.7M index-v5
4.5M index-v5.gz

gzip beats me naturally, we still have a lot of spare bits and we
don't use dictionaries. But the code is simpler and should run faster
than gzip.

Performance is improved too:

$ time GIT_INDEX_FILE=index-v2 ./git ls-files |head -n1 /dev/null

real0m0.437s
user0m0.385s
sys 0m0.048s
$ time GIT_INDEX_FILE=index-v4 ./git ls-files |head -n1 /dev/null

real0m0.319s
user0m0.277s
sys 0m0.040s
$ time GIT_INDEX_FILE=index-v5 ./git ls-files |head -n1 /dev/null

real0m0.250s
user0m0.213s
sys 0m0.036s

Some details on the new format:

 - in general varint is used to store numbers, unless we know the
   numbers are really big.
 - flags is the first field on disk, it has extra bits to let git know
   what to do with the rest of the fields
 - Many fields like ctime, mtime, uid, gid, dev, ino are stored as
   offsets
 - ce_mode's special values 100644 and 100755 are stored in flags. So
   unless you use gitlinks or something else, ce_mode will not be
   stored.
 - ce_namelen is no longer stored on disk
 - pathname is compressed just like in v4

---
 cache.h  |   2 +-
 read-cache.c | 273 +--
 2 files changed, 269 insertions(+), 6 deletions(-)

diff --git a/cache.h b/cache.h
index e493563..6ab53e7 100644
--- a/cache.h
+++ b/cache.h
@@ -106,7 +106,7 @@ struct cache_header {
 };
 
 #define INDEX_FORMAT_LB 2
-#define INDEX_FORMAT_UB 4
+#define INDEX_FORMAT_UB 5
 
 /*
  * The cache_time is just the low 32 bits of the
diff --git a/read-cache.c b/read-cache.c
index 827ae55..147ace1 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1260,7 +1260,7 @@ static int verify_hdr(struct cache_header *hdr, unsigned 
long size)
if (hdr-hdr_signature != htonl(CACHE_SIGNATURE))
return error(bad signature);
hdr_version = ntohl(hdr-hdr_version);
-   if (hdr_version  2 || 4  hdr_version)
+   if (hdr_version  2 || 5  hdr_version)
return error(bad index version %d, hdr_version);
git_SHA1_Init(c);
git_SHA1_Update(c, hdr, size - 20);
@@ -1407,6 +1407,137 @@ static struct cache_entry *create_from_disk(struct 
ondisk_cache_entry *ondisk,
return ce;
 }
 
+/*
+ * Often used flags come first to keep flags in common case small so
+ * that encode_varint() produces fewer bytes
+ */
+#define CE5_OFS_MTIME  (1  0)
+#define CE5_MODEMASK   (3  1)
+#define CE5_MODE_6440
+#define CE5_MODE_755   (1  1)
+#define CE5_MODE_FULL  (2  1)
+#define CE5_FULL_INO   (1  3)
+#define CE5_STAGESHIFT  4
+#define CE5_STAGEMASK  (3  CE5_STAGESHIFT)
+#define CE5_ITA(1  6)
+#define CE5_VALID  (1  7)
+#define CE5_SWT(1  8)
+#define CE5_FULL_UID   (1  9)
+#define CE5_FULL_GID   (1  10)
+#define CE5_FULL_TIME  (1  11)
+#define CE5_FULL_DEV   (1  12)
+
+static uintmax_t decode_offset(const unsigned char **bufp, uintmax_t base)
+{
+   uintmax_t offset = decode_varint(bufp);
+   if (offset  1) /* negative */
+   return base - (offset  1);
+   else
+   return base + (offset  1);
+
+}
+
+static struct cache_entry *create_from_disk_v5(const unsigned char *data,
+  unsigned long *consumed,
+  struct strbuf *previous_name,
+  const struct cache_entry 
*previous_ce)
+{
+   const unsigned char *orig = data;
+   struct cache_entry ce_tmp;
+   struct cache_entry *ce;
+   unsigned int flags;
+
+   flags = decode_varint(data);
+   ce_tmp.ce_flags = ((flags  CE5_STAGEMASK)  CE5_STAGESHIFT)  
CE_STAGESHIFT;
+   if (flags  CE5_ITA)
+   ce_tmp.ce_flags |= CE_INTENT_TO_ADD;
+   if (flags  CE5_VALID)
+   ce_tmp.ce_flags |= CE_VALID;
+   if (flags  CE5_SWT)
+   ce_tmp.ce_flags |= CE_SKIP_WORKTREE;
+
+   if (flags  CE5_FULL_TIME) { /* full format, 16 bytes */
+   ce_tmp.ce_ctime.sec = ntoh_l(*(uint32_t*)data);
+   data += sizeof(uint32_t);
+   ce_tmp.ce_ctime.nsec = ntoh_l(*(uint32_t*)data);
+   data += sizeof(uint32_t);
+
+   ce_tmp.ce_mtime.sec = ntoh_l(*(uint32_t*)data);
+   data += sizeof(uint32_t);
+   ce_tmp.ce_mtime.nsec = ntoh_l(*(uint32_t*)data);
+   data += sizeof(uint32_t);
+   } else {
+   /* offset from previous ce */
+   ce_tmp.ce_ctime.sec = decode_offset(data, 

running git from non-standard location on Mac

2013-02-21 Thread James French
Hi,

I wonder if someone could help me. I installed git on a Mac and then I copied 
the install somewhere else (which I do want to do, trust me).  I'm now having 
trouble with git svn. I'm getting Can't locate Git/SVN.pm in @INC...

I've added the bin folder to PATH. What else do I need to do? Do I need to use 
-exec-path=/MyPathToGit/libexec/git-core? How do I change the content of @INC?

Apologies if this is a dumb question, I'm not much of a unix man.

Cheers,
James

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


RFD: concatening textconv filters

2013-02-21 Thread Michael J Gruber
During my day-to-day UGFWIINIT I noticed that we don't do textconv
iteratively. E.g.: I have a file

SuperSecretButDumbFormat.pdf.gpg

and textconv filters with attributes set for *.gpg and *.pdf (using
gpg resp. pdftotext). For Git, the file has only the gpg
attribute, of course. In this case, I would have wanted to pass the gpg
output through pdftotext.

Now, I can set up an extra filter gpgtopdftotext for *.pdf.gpg (hoping
I get the ordering in .gitattributes right), of course, but wondering
whether we could and should support concatenating filters by either

- making it easy to request it (say by setting
filter.gpgtopdftotext.textconvpipe to a list of textconv filter names
which are to be applied in sequence)

or

- doing it automatically (remove the pattern which triggered the filter,
and apply attributes again to the resulting pathspec)

Maybe it's just not worth the effort. Or a nice GSoC project ;)

Michael
--
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: running git from non-standard location on Mac

2013-02-21 Thread Konstantin Khomoutov
On Thu, 21 Feb 2013 09:48:36 +
James French james.fre...@naturalmotion.com wrote:

 I wonder if someone could help me. I installed git on a Mac and then
 I copied the install somewhere else (which I do want to do, trust
 me).  I'm now having trouble with git svn. I'm getting Can't locate
 Git/SVN.pm in @INC...
 
 I've added the bin folder to PATH. What else do I need to do? Do I
 need to use -exec-path=/MyPathToGit/libexec/git-core? How do I change
 the content of @INC?
 
 Apologies if this is a dumb question, I'm not much of a unix man.

`git svn` is implemented in Perl (which is supposedly bundled with your
Git package, but I'm not sure), and SVN.pm is a Perl module (a
library written in Perl, .pm stands for Perl Module).

@INC is an internal variable used by Perl to locate its modules.
Its contents is partially inferred from the Perl's installation
location and partially from the environment.

This [1] should help you get started with affecting @INC.

1. http://stackoverflow.com/a/2526809/720999
--
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: running git from non-standard location on Mac

2013-02-21 Thread Matthieu Moy
Konstantin Khomoutov flatw...@users.sourceforge.net writes:

 This [1] should help you get started with affecting @INC.

In the particular case of Git, the Makefile hardcodes the path to the
Git library. The script git-svn in Git's exec-path should start with:

use lib (split(/:/, $ENV{GITPERLLIB} || /some/hardcoded/path/to/perl/5.10.1));

Setting the $GITPERLLIB environment variable or editing the script to
let the hardcoded path point to the place where the Git.pm file is
should do it.


But I still have to wonder why you didn't build Git with the right paths
in the first place.

-- 
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: running git from non-standard location on Mac

2013-02-21 Thread James French


-Original Message-
From: Konstantin Khomoutov [mailto:flatw...@users.sourceforge.net] 
Sent: 21 February 2013 10:35
To: James French
Cc: git@vger.kernel.org
Subject: Re: running git from non-standard location on Mac

On Thu, 21 Feb 2013 09:48:36 +
James French james.fre...@naturalmotion.com wrote:

 I wonder if someone could help me. I installed git on a Mac and then I 
 copied the install somewhere else (which I do want to do, trust me).  
 I'm now having trouble with git svn. I'm getting Can't locate 
 Git/SVN.pm in @INC...
 
 I've added the bin folder to PATH. What else do I need to do? Do I 
 need to use -exec-path=/MyPathToGit/libexec/git-core? How do I change 
 the content of @INC?
 
 Apologies if this is a dumb question, I'm not much of a unix man.

`git svn` is implemented in Perl (which is supposedly bundled with your Git 
package, but I'm not sure), and SVN.pm is a Perl module (a library written in 
Perl, .pm stands for Perl Module).

@INC is an internal variable used by Perl to locate its modules.
Its contents is partially inferred from the Perl's installation location and 
partially from the environment.

This [1] should help you get started with affecting @INC.

1. http://stackoverflow.com/a/2526809/720999



Thanks for the help guys. I got it working using --exec-path and PERL5LIB 
environment variable. But Matthieu is right, I should build it from source to 
do it properly.
--
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 branch HEAD dumps core when on detached head (NULL pointer dereference)

2013-02-21 Thread Duy Nguyen
On Thu, Feb 21, 2013 at 7:27 PM, Per Cederqvist ced...@opera.com wrote:
 Running git branch HEAD may be a stupid thing to do. It actually
 was a mistake on my part. Still, I don't think git should dereference
 a NULL pointer.

We should not. Can you make a patch to fix it (with test cases)? You
may want to fix the two preceding blocks, if (new_upstream) and if
(unset_upstream), as well. They don't check for NULL branch either. I
think we can say something like detached HEAD is not valid for this
operation before exit.


 Tested using git 1.8.1.4 adn 1.8.1.1.

 Repeat by:

 mkdir branchcrash || exit 1
 cd branchcrash
 git init
 touch a; git add a; git commit -mAdded a.
 touch b; git add b; git commit -mAdded b.
 git checkout HEAD^
 git branch HEAD

 The last command dumps core.  gdb session:

 gdb /usr/local/bin/git core
 GNU gdb (Ubuntu/Linaro 7.4-2012.04-0ubuntu2.1) 7.4-2012.04
 Copyright (C) 2012 Free Software Foundation, Inc.
 License GPLv3+: GNU GPL version 3 or later
 http://gnu.org/licenses/gpl.html
 This is free software: you are free to change and redistribute it.
 There is NO WARRANTY, to the extent permitted by law.  Type show copying
 and show warranty for details.
 This GDB was configured as x86_64-linux-gnu.
 For bug reporting instructions, please see:
 http://bugs.launchpad.net/gdb-linaro/...
 Reading symbols from /usr/local/bin/git...done.
 [New LWP 7174]

 warning: Can't read pathname for load map: Input/output error.
 [Thread debugging using libthread_db enabled]
 Using host libthread_db library /lib/x86_64-linux-gnu/libthread_db.so.1.
 Core was generated by `git branch HEAD'.
 Program terminated with signal 11, Segmentation fault.
 #0  cmd_branch (argc=1, argv=0x7fffe6e2a0f0, prefix=optimized out)
 at builtin/branch.c:919
 919 strbuf_addf(buf, refs/remotes/%s, branch-name);
 (gdb) bt
 #0  cmd_branch (argc=1, argv=0x7fffe6e2a0f0, prefix=optimized out)
 at builtin/branch.c:919
 #1  0x004046ac in run_builtin (argv=0x7fffe6e2a0f0, argc=2,
 p=optimized out) at git.c:273
 #2  handle_internal_command (argc=2, argv=0x7fffe6e2a0f0) at git.c:434
 #3  0x00404df3 in run_argv (argv=0x7fffe6e29f90,
 argcp=0x7fffe6e29f9c)
 at git.c:480
 #4  main (argc=2, argv=0x7fffe6e2a0f0) at git.c:555
 (gdb) p branch
 $1 = (struct branch *) 0x0
 (gdb) quit

 /ceder
 --
 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


git branch --contains x y creates a branch instead of checking containment

2013-02-21 Thread Per Cederqvist

The git branch --list --contains x y command lists
all branches that contains commit x and matches the
pattern y. Reading the git-branch(1) manual page gives
the impression that --list is redundant, and that
you can instead write

   git branch --contains x y

That command does something completely different,
though. The --contains x part is silently ignored,
so it creates a branch named y pointing at HEAD.

Tested in git 1.8.1.1 and 1.8.1.4.

In my opinion, there are two ways to fix this:

 - change the git branch implementation so
   that --contains implies --list.

 - change the manual page synopsis so that
   it is clear that --contains can only be
   used if --list is also used.  Also add an
   error check to the git branch implementation
   so that using --contains without specifying
   --list produces a fatal error message.

Personally I would prefer the first solution.

Repeat by running these commands:

# Set up a repo with two commits.
# Branch a points to the oldest one, and
# b and master to the newest one.
mkdir contains || exit 1
cd contains
git init
touch a; git add a; git commit -mAdded a.
git branch a
touch b; git add b; git commit -mAdded b.
git branch b

git branch --list --contains a
# Prints a, b and master, as expected.

git branch --contains a
# Prints a, b and master, as expected.
# In this case, the --list option can be removed.

git branch --list --contains a b
# Prints b, as expected.  b is a pattern.

git branch --list --contains a c
# Prints nothing, as expected, as the c pattern doesn't match any of
# the existing branches.

git for-each-ref
# Prints three lines: refs/heads/a, refs/heads/b and
# refs/heads/master, as expected.

git branch --contains a c
# Prints nothing, as expected, but...

git for-each-ref
# Prints four lines!  Apparently, the command above created
# refs/heads/c.

/ceder

P.S. What I really wanted to do was git merge-base
--is-ancestor a b, but I keep forgetting its name.
--
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 branch HEAD dumps core when on detached head (NULL pointer dereference)

2013-02-21 Thread Per Cederqvist

On 02/21/13 13:50, Duy Nguyen wrote:

On Thu, Feb 21, 2013 at 7:27 PM, Per Cederqvist ced...@opera.com wrote:

Running git branch HEAD may be a stupid thing to do. It actually
was a mistake on my part. Still, I don't think git should dereference
a NULL pointer.


We should not. Can you make a patch to fix it (with test cases)? You
may want to fix the two preceding blocks, if (new_upstream) and if
(unset_upstream), as well. They don't check for NULL branch either. I
think we can say something like detached HEAD is not valid for this
operation before exit.


Sorry, but isolating the issue reporting it here is about as much time
as I can spend on this issue. Learning the coding standard of Git and
how to write test cases is not something I'm prepared to do, at least
not at the moment. I hope there is a place for users and reporters of
bugs in the Git community.

/ceder



Tested using git 1.8.1.4 adn 1.8.1.1.

Repeat by:

 mkdir branchcrash || exit 1
 cd branchcrash
 git init
 touch a; git add a; git commit -mAdded a.
 touch b; git add b; git commit -mAdded b.
 git checkout HEAD^
 git branch HEAD

The last command dumps core.  gdb session:

gdb /usr/local/bin/git core
GNU gdb (Ubuntu/Linaro 7.4-2012.04-0ubuntu2.1) 7.4-2012.04
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type show copying
and show warranty for details.
This GDB was configured as x86_64-linux-gnu.
For bug reporting instructions, please see:
http://bugs.launchpad.net/gdb-linaro/...
Reading symbols from /usr/local/bin/git...done.
[New LWP 7174]

warning: Can't read pathname for load map: Input/output error.
[Thread debugging using libthread_db enabled]
Using host libthread_db library /lib/x86_64-linux-gnu/libthread_db.so.1.
Core was generated by `git branch HEAD'.
Program terminated with signal 11, Segmentation fault.
#0  cmd_branch (argc=1, argv=0x7fffe6e2a0f0, prefix=optimized out)
 at builtin/branch.c:919
919 strbuf_addf(buf, refs/remotes/%s, branch-name);
(gdb) bt
#0  cmd_branch (argc=1, argv=0x7fffe6e2a0f0, prefix=optimized out)
 at builtin/branch.c:919
#1  0x004046ac in run_builtin (argv=0x7fffe6e2a0f0, argc=2,
 p=optimized out) at git.c:273
#2  handle_internal_command (argc=2, argv=0x7fffe6e2a0f0) at git.c:434
#3  0x00404df3 in run_argv (argv=0x7fffe6e29f90,
argcp=0x7fffe6e29f9c)
 at git.c:480
#4  main (argc=2, argv=0x7fffe6e2a0f0) at git.c:555
(gdb) p branch
$1 = (struct branch *) 0x0
(gdb) quit

 /ceder
--
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: git branch HEAD dumps core when on detached head (NULL pointer dereference)

2013-02-21 Thread Duy Nguyen
On Thu, Feb 21, 2013 at 8:24 PM, Per Cederqvist ced...@opera.com wrote:
 Sorry, but isolating the issue reporting it here is about as much time
 as I can spend on this issue. Learning the coding standard of Git and
 how to write test cases is not something I'm prepared to do, at least
 not at the moment. I hope there is a place for users and reporters of
 bugs in the Git community.

Sure. No problem. I just thought you might want to finish it off. I'll
look into it.
-- 
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


[PATCH] branch: segfault fixes and validation

2013-02-21 Thread Nguyễn Thái Ngọc Duy
branch_get() can return NULL (so far on detached HEAD only) but some
code paths in builtin/branch.c cannot deal with that and cause
segfaults. Fix it.

While at there, make sure to bail out when the user gives 2 or more
arguments, but only the first one is processed.

Reported-by: Per Cederqvist ced...@opera.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/branch.c  | 20 
 t/t3200-branch.sh | 21 +
 2 files changed, 41 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..c1d688e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -889,6 +889,13 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
} else if (new_upstream) {
struct branch *branch = branch_get(argv[0]);
 
+   if (argc  1)
+   die(_(too many branches to set new upstream));
+
+   if (!branch)
+   die(_(could not figure out the branch name from '%s'),
+   argc == 1 ? argv[0] : HEAD);
+
if (!ref_exists(branch-refname))
die(_(branch '%s' does not exist), branch-name);
 
@@ -901,6 +908,13 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
struct branch *branch = branch_get(argv[0]);
struct strbuf buf = STRBUF_INIT;
 
+   if (argc  1)
+   die(_(too many branches to unset upstream));
+
+   if (!branch)
+   die(_(could not figure out the branch name from '%s'),
+   argc == 1 ? argv[0] : HEAD);
+
if (!branch_has_merge_config(branch)) {
die(_(Branch '%s' has no upstream information), 
branch-name);
}
@@ -916,6 +930,12 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
int branch_existed = 0, remote_tracking = 0;
struct strbuf buf = STRBUF_INIT;
 
+   if (!strcmp(argv[0], HEAD))
+   die(_(it does not make sense to create 'HEAD' 
manually));
+
+   if (!branch)
+   die(_(could not figure out the branch name from 
'%s'), argv[0]);
+
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make 
sense with a branch name));
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index f3e0e4a..12f1e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -42,6 +42,10 @@ test_expect_success \
 'git branch a/b/c should create a branch' \
 'git branch a/b/c  test_path_is_file .git/refs/heads/a/b/c'
 
+test_expect_success \
+'git branch HEAD should fail' \
+'test_must_fail git branch HEAD'
+
 cat expect EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL 1117150200 +
branch: Created from master
 EOF
@@ -388,6 +392,14 @@ test_expect_success \
 'git tag foobar 
  test_must_fail git branch --track my11 foobar'
 
+test_expect_success '--set-upstream-to fails on multiple branches' \
+'test_must_fail git branch --set-upstream-to master a b c'
+
+test_expect_success '--set-upstream-to fails on detached HEAD' \
+'git checkout HEAD^{} 
+ test_must_fail git branch --set-upstream-to master 
+ git checkout -'
+
 test_expect_success 'use --set-upstream-to modify HEAD' \
 'test_config branch.master.remote foo 
  test_config branch.master.merge foo 
@@ -417,6 +429,15 @@ test_expect_success 'test --unset-upstream on HEAD' \
  test_must_fail git branch --unset-upstream
 '
 
+test_expect_success '--unset-upstream should fail on multiple branches' \
+'test_must_fail git branch --unset-upstream a b c'
+
+test_expect_success '--unset-upstream should fail on detached HEAD' \
+'git checkout HEAD^{} 
+ test_must_fail git branch --unset-upstream 
+ git checkout -
+'
+
 test_expect_success 'test --unset-upstream on a particular branch' \
 'git branch my15
  git branch --set-upstream-to master my14 
-- 
1.8.1.2.536.gf441e6d

--
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] archive: let remote clients get reachable commits

2013-02-21 Thread Sergey Segeev
Some time we need to get valid commit without a ref but with proper
tree-ish, now we can't do that.

This patch allow upload-archive's to use reachability checking
rather than checking that is a ref. This means a remote client can
fetch a tip of any valid sha1 or tree-ish.
---
 archive.c   | 24 +---
 t/t5000-tar-tree.sh |  2 ++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/archive.c b/archive.c
index 93e00bb..0a48985 100644
--- a/archive.c
+++ b/archive.c
@@ -5,6 +5,7 @@
 #include archive.h
 #include parse-options.h
 #include unpack-trees.h
+#include refs.h
 
 static char const * const archive_usage[] = {
N_(git archive [options] tree-ish [path...]),
@@ -241,6 +242,13 @@ static void parse_pathspec_arg(const char **pathspec,
}
 }
 
+static int check_reachable(const char *refname, const unsigned char *sha1,
+   int flag, void *cb_data)
+{
+
+   return in_merge_bases(cb_data, lookup_commit_reference_gently(sha1, 1));
+}
+
 static void parse_treeish_arg(const char **argv,
struct archiver_args *ar_args, const char *prefix,
int remote)
@@ -252,22 +260,16 @@ static void parse_treeish_arg(const char **argv,
const struct commit *commit;
unsigned char sha1[20];
 
-   /* Remotes are only allowed to fetch actual refs */
-   if (remote) {
-   char *ref = NULL;
-   const char *colon = strchr(name, ':');
-   int refnamelen = colon ? colon - name : strlen(name);
-
-   if (!dwim_ref(name, refnamelen, sha1, ref))
-   die(no such ref: %.*s, refnamelen, name);
-   free(ref);
-   }
-
if (get_sha1(name, sha1))
die(Not a valid object name);
 
commit = lookup_commit_reference_gently(sha1, 1);
if (commit) {
+
+   /* Remotes are only allowed to fetch actual objects */
+   if (remote  !for_each_ref(check_reachable, (void *)commit))
+   die(Not a valid object name);
+
commit_sha1 = commit-object.sha1;
archive_time = commit-date;
} else {
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index e7c240f..fc35406 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -194,6 +194,8 @@ test_expect_success 'clients cannot access unreachable 
commits' '
sha1=`git rev-parse HEAD` 
git reset --hard HEAD^ 
git archive $sha1 remote.tar 
+   git archive --remote=. $sha1 remote.tar 
+   git tag -d unreachable 
test_must_fail git archive --remote=. $sha1 remote.tar
 '
 
-- 
1.8.1.4

--
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: Google Summer of Code 2013 (GSoC13)

2013-02-21 Thread Carlos Martín Nieto
Michael Schubert s...@schu.io writes:

 On 02/18/2013 06:42 PM, Jeff King wrote:
 
 I will do it again, if people feel strongly about Git being a part of
 it. However, I have gotten a little soured on the GSoC experience. Not
 because of anything Google has done; it's a good idea, and I think they
 do a fine of administering the program. But I have noticed that the work
 that comes out of GSoC the last few years has quite often not been
 merged, or not made a big impact in the codebase, and nor have the
 participants necessarily stuck around.
 
 And I do not want to blame the students here (some of whom are on the cc
 list :) ). They are certainly under no obligation to stick around after
 GSoC ends, and I know they have many demands on their time. But I am
 also thinking about what Git wants to get out of GSoC (and to my mind,
 the most important thing is contributors).

 Speaking of libgit2:

 Git provided the libgit2 project with a slot each of the last three GSOC.
 The contributions made by the former students (Disclaimer: one of them
 speaking) have been quite important for libgit2 and all three students
 are still involved. Each project was an important push towards building
 a new, feature complete Git library.

Right, speaking of libgit2. GSoC has been very successful (as Michael,
I'm also somewhat biased) for libgit2. This happens outside of the git
ML so it probably hasn't gotten as much visibility here.

I believe it's partly because there were still larger parts where most
of the work was technical and the goal was quite clear, as git had
already set the standard and expectations and the decisions had to be
mostly about how to implement it in a way that makes sense for a
library, rather than it living inside of git, which is not always easy,
but you can experiment with different uses of it.

It's also possible that part of the success was the fact that we were
already acquainted with the release often and early policy, as we'd
been involved with FLOSS for a while already.

The current gaping hole in libgit2 is the lack of merge support, which
is the last hurdle to a stable 1.0 release. There is already some work
by Edward Thomson that needs to be reviewed and merged. I'm not sure
that there's enough for a whole summer there, but you could throw in the
review and merge of another missing feature, which is making the
reference storage generic, as it currently only supports the
git-compatible file-based one. There's other nice-to-have things like
thin-pack support that you could use to fill up a summer, though I'm not
sure that goes with the spirit of the programme.

Something else that needs love is Git for Windows. I believe both git
and libgit2 would benefit a lot from a project to take some parts of git
that are implemented in a scripting language and port them to use
libgit2. As Git for Windows needs to ship a ton of dependencies anyway,
using a pre-1.0 library wouldn't be an issue and it can be used to
experiment with an eventual porting of git to be one user of libgit2
rather than a completely different implementation. The more immediate
benefit for Git for Windows would be less reliance on languages that are
awkward to use on Windows and need their own environment. Mentoring from
the libgit2 probably wouldn't be much of an issue to organise, though
I'm not sure if the GfW team would have time for the part that involves
its peculiarities.

So there's a couple of projects that could be done with some realistic
chance of being merged upstream, as they'd be technical, as long as we
do tell the student to send small units of work to be reviewed often.

Cheers,
   cmn
--
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: Google Summer of Code 2013 (GSoC13)

2013-02-21 Thread Thomas Rast
Shawn Pearce spea...@spearce.org writes:

 On Mon, Feb 18, 2013 at 9:42 AM, Jeff King p...@peff.net wrote:
 On Mon, Feb 18, 2013 at 06:23:01PM +0100, Thomas Rast wrote:

 * We need an org admin.  AFAIK this was done by Peff and Shawn in
   tandem last year.  Would you do it again?

 I will do it again, if people feel strongly about Git being a part of
 it. However, I have gotten a little soured on the GSoC experience. Not
 because of anything Google has done; it's a good idea, and I think they
 do a fine of administering the program. But I have noticed that the work
 that comes out of GSoC the last few years has quite often not been
 merged, or not made a big impact in the codebase, and nor have the
 participants necessarily stuck around.

 This.

 I actually think Git should take a year off from GSoC and not
 participate. Consequently I will not be volunteering as backup org
 admin.

Fair enough.  But I think if that's the decision (and modulo libgit2
praise, it seems to be pretty much the consensus?), we should probably
have some Idea why we are doing it?

You wrote:

 Git has been involved since 2007. In all of that time we have had very
 few student projects merge successfully into their upstream project
 (e.g. git.git, JGit or libgit2) before the end of GSoC. Even fewer
 students have stuck around and remained active contributors. When I
 look at the amount of effort we contributors put into GSoC, I think we
 are misusing our limited time and resources. The intention of the GSoC
 program is to grow new open source developers, and increase our
 community of contributors. Somehow I think Git is falling well short
 of its potential here. This is especially true if you compare Git's
 GSoC program to some other equally long-running GSoC programs.

If that's the outset (and it's certainly true for a lot of the
projects), aren't the options (not limited to just one):

* We have some discussion about why we fail, what to do better, etc. and
  hopefully also manage to clean up some old projects and get them
  included.  That way we can learn something from it.

* We try to look at how more successful communities are doing it
  (e.g. there were some posts about how KDE bumped their student
  retention rate).

* We try to mentor some projects that aren't GSoC sponsered.  That way
  we can hope to gain mentoring experience.

I'm not very optimistic about any of these, as:

- There weren't any in-depth discussions post-GSoC to analyze what went
  wrong.

- Contributor time is so limited that we're usually short on reviews.
  Adding mentoring for the sake of trying it to duties isn't very
  promising.

Thus I'm a bit afraid that after a year off, we won't have learned
anything new.  To the contrary, some previous mentors/students will
inevitably have disappeared into the mists of time, and with them their
experience.  Unless we do something about it, next year we'll be in an
_even worse_ position than this year.

I'm mildly pessimistic about doing something over the list, but
perhaps we can have an extended discussion at git-merge provided enough
of you show up there?  I can try to prepare some material.


(Maybe we should all make a bunch of clones of ourselves.  We can put
one copy each into a room so they can figure out GSoC, and have another
group doing our favorite git hacking while we're at it.)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
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 branch --contains x y creates a branch instead of checking containment

2013-02-21 Thread Jeff King
On Thu, Feb 21, 2013 at 02:00:27PM +0100, Per Cederqvist wrote:

 That command does something completely different,
 though. The --contains x part is silently ignored,
 so it creates a branch named y pointing at HEAD.
 
 Tested in git 1.8.1.1 and 1.8.1.4.
 
 In my opinion, there are two ways to fix this:
 
  - change the git branch implementation so
that --contains implies --list.

I think that is the best option, too. In fact, I even wrote a patch. :)

It's d040350 (branch: let branch filters imply --list, 2013-01-31), and
it's already in v1.8.2-rc0.

-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: git branch --contains x y creates a branch instead of checking containment

2013-02-21 Thread Per Cederqvist

On 02/21/13 16:58, Jeff King wrote:

On Thu, Feb 21, 2013 at 02:00:27PM +0100, Per Cederqvist wrote:


That command does something completely different,
though. The --contains x part is silently ignored,
so it creates a branch named y pointing at HEAD.

Tested in git 1.8.1.1 and 1.8.1.4.

In my opinion, there are two ways to fix this:

  - change the git branch implementation so
that --contains implies --list.


I think that is the best option, too. In fact, I even wrote a patch. :)

It's d040350 (branch: let branch filters imply --list, 2013-01-31), and
it's already in v1.8.2-rc0.

-Peff


Great! Thanks for the quick fix of my bug report. Negative response
time... not bad. Not bad at all. :-)

/ceder

--
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: RFD: concatening textconv filters

2013-02-21 Thread Junio C Hamano
Michael J Gruber g...@drmicha.warpmail.net writes:

 ... but wondering
 whether we could and should support concatenating filters by either

 - making it easy to request it (say by setting
 filter.gpgtopdftotext.textconvpipe to a list of textconv filter names
 which are to be applied in sequence)

 or

 - doing it automatically (remove the pattern which triggered the filter,
 and apply attributes again to the resulting pathspec)

I think what you are getting at is to start from something like this:

= .gitattributes =
*.gpg   diff=gpg
*.pdf   diff=pdf

and have git cat-file --textconv frotz.pdf.gpg (and other textconv
users) notice that:

 (1) The path matches *.gpg pattern, and calls for
 diff.gpg.textconv conversion.  This already happens in the
 current system.

 (2) After stripping the *.gpg pattern (i.e. look at the part of
 the path that matched the wildcard part * in the attribute
 selector), notice that the remainder, frotz.pdf, could match
 the *.pdf pattern.  The output from the previous filter could
 be treated as if it were a blob that is stored in that path.

A few issues that need to be addressed while designing this feature
that come to my mind at random are:

 * This seems to call for a new concept, but what exactly is that
   concept?  Your RFD sounds as if you desire a cascadable
   textconv, but it may be of a somewhat larger scope, virtual
   blob at a virtual path, which the last sentence in (2) above
   seems to suggest.

 * What is this new concept an attribute to?  If we express this as
   the textconv conversion result of any path with attribute
   diff=gpg can be treated as the contents of a virtual blob, then
   we are making it an attribute of the gpg type, i.e.

= .git/config =
[diff gpg]
textconv = gpg -v
textconvProducesVirtualBlob = yes

   To me, that seems sufficient for this particular application at
   the first glance, but are there other attributes that may want to
   produce such virtual blob for further processing?  Is limiting
   this to textconv too restrictive?  I do not know.

 * What is the rule to come up with the virtual path to base the
   attribute look-up on for the virtual blob contents?  In the
   above example, the pattern was a simple *.gpg, and we used a
   naïve what did the asterisk match?, but imagine a case where
   you have some documents that you want to do gpg -v and some you
   don't.  You express this by having the former class of files
   named with conv- prefix, or some convention that is convenient
   for you.

   Your .gitattributes may say something like:

= .gitattributes =
conv-*.gpg  diff=gpg

   When deciding what attributes to use to further process the
   result of conversion (i.e. virtual blob contents) for
   conv-frotz.pdf.gpg, what virtual path should we use?  Should we
   use conv-frotz.pdf, or just frotz.pdf?

   The difference does not matter--either would work is not a
   satisfactory answer, once you consider that you may want to have
   two or more classes of pdf files that you may want to treat
   differently, just like you did for gpg encrypted files in this
   example setting.  It seems to suggest that we want to use
   conv-frotz.pdf as the virtual path, but how would we derive that
   from the pattern conv-*.gpg and path conv-frotz.pdf.gpg?  It
   appears to me that you would need a way to say between the two
   literal parts in the pattern, conv- part needs to be kept but
   .gpg part needs to be stripped when forming the result.


--
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] branch: segfault fixes and validation

2013-02-21 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 branch_get() can return NULL (so far on detached HEAD only)...

Do you anticipate any other cases where the API call should validly
return NULL?  I offhand do not, ...

 but some
 code paths in builtin/branch.c cannot deal with that and cause
 segfaults. Fix it.

 While at there, make sure to bail out when the user gives 2 or more
 arguments, but only the first one is processed.

 Reported-by: Per Cederqvist ced...@opera.com
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  builtin/branch.c  | 20 
  t/t3200-branch.sh | 21 +
  2 files changed, 41 insertions(+)

 diff --git a/builtin/branch.c b/builtin/branch.c
 index 6371bf9..c1d688e 100644
 --- a/builtin/branch.c
 +++ b/builtin/branch.c
 @@ -889,6 +889,13 @@ int cmd_branch(int argc, const char **argv, const char 
 *prefix)
   } else if (new_upstream) {
   struct branch *branch = branch_get(argv[0]);
  
 + if (argc  1)
 + die(_(too many branches to set new upstream));
 +
 + if (!branch)
 + die(_(could not figure out the branch name from '%s'),
 + argc == 1 ? argv[0] : HEAD);

... and find this could not figure out very unfriendly to the
user.  Is it a bug in the implementation, silly Git failing to find
what branch the user meant?  What recourse does the user have at
this point?

Or is it a user error to ask Git to operate on the branch pointed at
by HEAD, when HEAD does not refer to any branch?  If that is the
case, then the message should say that there is no current branch to
operate on because the user is on a detached HEAD.  That would point
the user in the right direction to correct the user error, no?

Of course, argv[0] may not be HEAD and in that case you may have to
say no such branch %s % argv[0] or something.  The point is that
could not figure out feels a bit too lazy.

 @@ -901,6 +908,13 @@ int cmd_branch(int argc, const char **argv, const char 
 *prefix)
   struct branch *branch = branch_get(argv[0]);
   struct strbuf buf = STRBUF_INIT;
  
 + if (argc  1)
 + die(_(too many branches to unset upstream));
 +
 + if (!branch)
 + die(_(could not figure out the branch name from '%s'),
 + argc == 1 ? argv[0] : HEAD);

Likewise.

 @@ -916,6 +930,12 @@ int cmd_branch(int argc, const char **argv, const char 
 *prefix)
   int branch_existed = 0, remote_tracking = 0;
   struct strbuf buf = STRBUF_INIT;
  
 + if (!strcmp(argv[0], HEAD))
 + die(_(it does not make sense to create 'HEAD' 
 manually));
 +
 + if (!branch)
 + die(_(could not figure out the branch name from 
 '%s'), argv[0]);

Likewise.

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: git branch --contains x y creates a branch instead of checking containment

2013-02-21 Thread Junio C Hamano
Per Cederqvist ced...@opera.com writes:

 On 02/21/13 16:58, Jeff King wrote:
 On Thu, Feb 21, 2013 at 02:00:27PM +0100, Per Cederqvist wrote:

 That command does something completely different,
 though. The --contains x part is silently ignored,
 so it creates a branch named y pointing at HEAD.

 Tested in git 1.8.1.1 and 1.8.1.4.

 In my opinion, there are two ways to fix this:

   - change the git branch implementation so
 that --contains implies --list.

 I think that is the best option, too. In fact, I even wrote a patch. :)

 It's d040350 (branch: let branch filters imply --list, 2013-01-31), and
 it's already in v1.8.2-rc0.

 -Peff

 Great! Thanks for the quick fix of my bug report. Negative response
 time... not bad. Not bad at all. :-)

Yeah, Jeff has a time machine ;-)
--
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


Fw:เบื้องหลัง 16 ศพบาเจาะ

2013-02-21 Thread december24
เมื่อกลางดึกวันที่ 13 ก.พ. 56 คนร้ายกว่า 50 คน บุกโจมตีฐานทหารนาวิกโยธินที่
บ้านยือลอ ต.บาเระเหนือ อ.บาเจาะ จ.นราธิวาส มีผู้เสียชีวิต 16 คน เป็นข่าวที่น่า
สะเทือนใจมาก  แม้ว่าผู้ที่เสียชีวิตจะเป็นคนร้าย  
ถึงยังไงพวกเขาเหล่านั้นล้วนแต่เป็นคน
ไทยด้วยกัน อะไรเป็นเหตุที่ทำให้เขาเหล่านี้ต้องจับอาวุธขึ้นมาฆ่ากัน 
ความต้องการแยกตัว
เป็นเอกราช? ความเชื่อทางศาสนา? หรือไม่ได้รับความยุติธรรมจากชีวิตความเป็นอยู่?  
ซึ่ง
เป็นสาเหตุที่ถูกนำมาใช้อ้างกันบ่อยมาก  
แต่หลังจากนี้ครอบครัวของผู้เสียชีวิตเขาจะต้อง
เผชิญกับชีวิตที่ลำบากกว่าเดิมหลายเท่า 
เพราะว่าเดิมทีกลุ่มคนเหล่านี้ต้องหลบๆซ่อนๆไม่
สามารถทำมาหากินได้ ลูกเมียก็ต้องพลอยลำบากไปด้วย ภรรยาของผู้เสียชีวิตคนหนึ่งเล่า
ให้นักข่าวฟังว่า สามีเคยกลับมาบ้านก่อนคืนเกิดเหตุ 
ได้บอกกับลูกว่าให้ขยันเรียนตั้งใจ
ศึกษาให้สูง ส่วนตัวเขานั้นได้ถลำตัวเข้าร่วมขบวนการแล้วออกมาไม่ได้เพราะต้องถูกฆ่า
ตายเหมือนกัน  ส่วนข่าวที่รู้ล่วงหน้าได้มาอย่างไร?  
มีสมาชิกจำนวนหนึ่งที่เข้าร่วมกับกลุ่ม
ผู้ก่อความไม่สงบ รู้ว่ากลุ่มที่ตนศรัทธาไปพัวพันกับยาเสพติด  พวกเขาจึงกลับใจมาให้
ความร่วมมือกับเจ้าหน้าที่
 “หลังจากฮาเซ็มเสียชีวิต  เกรงว่าอาจทำให้ยาเสพติดระบาด เพราะไม่มีใครคอย
ตักเตือนและไม่มีคนที่สามารถควบคุมเยาวชนได้  ฮาเซ็มชอบอ่านคัมภีร์อัลกุรอานเป็น
ประจำ จึงมีคำภีร์อัลกุรอานติดตัวตลอด”นั่นคือเสียงสะท้อนของนางปรัชญา เบ็นเจะมูดอ 
ภรรยานายฮาเซ็ม 
9 ต.ค. 50   เจ้าหน้าที่ได้อายัดเงินกว่า 30  ล้านบาท  จากการจับกุมนายมะยากี 
ยาโกะ นักค้ายาเสพติดในพื้นที่ อ.สุไหงโก-ลก จ.นราธิวาส 
ที่ซุกซ่อนเงินในท่อพีวีซีทั้งหมด 
9 ท่อ บอกได้ไหมว่าเงินจากการค้ายาเสพติดจำนวนมากขนาดนี้ไม่เกี่ยวข้องกับการก่อ
ความไม่สงบในจังหวัดชายแดนใต้  หรือแกล้งทำเงียบ ๆ ไว้


RE: QNX support

2013-02-21 Thread Kraai, Matt
Junio C Hamano writes:
 David Ondřich david.ondr...@aveco.com writes:
  I've read [1] recently, there's been some QNX port being
  initiated. Does that involve also old versions of QNX 4?

No, I haven't been working on QNX 4 support.  I've been targeting QNX 6.3.2, 
with a little testing on QNX 6.5.0.  I doubt what I've done would work on QNX 4 
since it's so different from QNX 6.
 
  Since we are using QNX both internally and for our customers we
  started porting Git on QNX ourselves some time ago and we do have
  some experiences. Basically, it's possible to get Git up and
  running but there are some limitations, and some hacks have to be
  applied.
 
  If some additional info wanted, please contact me.

Now that Git is building and usable, the next logical step is to investigate 
and fix the test suite failures.  If you have any information about these, that 
could be helpful.

-- 
Matt
--
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 v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline

2013-02-21 Thread Junio C Hamano
Brandon Casey draf...@gmail.com writes:

 Teach append_signoff to detect whether a blank line exists at the position
 that the signed-off-by line will be added, and refrain from adding an
 additional one if one already exists.  Or, add an additional line if one
 is needed to make sure the new footer is separated from the message body
 by a blank line.

 Signed-off-by: Brandon Casey bca...@nvidia.com
 ---


 A slight tweak.  And I promise, no more are coming.

When I do

$ git commit -s

it should start my editor with this in the buffer:



Signed-off-by: Junio C Hamano gits...@pobox.com


and the cursor blinking at the beginning of the file.  Annoyingly
this step breaks it by removing the leading blank line.
--
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: Credentials and the Secrets API...

2013-02-21 Thread John Szakmeister
On Wed, Feb 20, 2013 at 12:01 PM, Ted Zlatanov t...@lifelogs.com wrote:
 On Sat, 9 Feb 2013 05:58:47 -0500 John Szakmeister j...@szakmeister.net 
 wrote:
[snip]

 JS Yes, I think it has.  Several other applications appear to be using
 JS it, including some things considered core in Fedora--which is a good
 JS sign.

 Wonderful.  Do you still have interest in working on this credential?

I do, but I'm a bit short on time right now.  So if you or someone
else wants to pick up and run with it, please feel free.  If I get
some cycles over the next couple of months, I'll give it a go though.

-John
--
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: Re* [PATCH 2/2] check-ignore.c: fix segfault with '.' argument from repo root

2013-02-21 Thread Junio C Hamano
Adam Spiers g...@adamspiers.org writes:

 On Tue, Feb 19, 2013 at 06:53:07PM -0800, Junio C Hamano wrote:
 Adam Spiers g...@adamspiers.org writes:
 
  OK, thanks for the information.  IMHO it would be nice if 'git
  format-patch' and 'git am' supported this style of inline patch
  inclusion, but maybe there are good reasons to discourage it?
 
 git am --scissors is a way to process such e-mail where the patch
 submitter continues discussion in the top part of a message,
 concludes the message with:
 
  A patch to do so is attached.
  -- 8 --
 
 and then tells the MUA to read in an output from format-patch into
 the e-mail buffer.

 Ah, nice!  I didn't know about that.

  You still need to strip out unneeded headers
 like the From , From:  and Date:  lines when you add the
 scissors anyway, and this is applicable only for a single-patch
 series, so the feature does not fit well as a format-patch option.

 Rather than requiring the user to manually strip out unneeded headers,
 wouldn't it be friendlier and less error-prone to add a new --inline
 option to format-patch which omitted them in the first place?  It
 should be easy to make it bail with an error when multiple revisions
 are requested.

Perhaps.
--
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] format-patch: rename no_inline field

2013-02-21 Thread Junio C Hamano
The name of the fields invites a misunderstanding that setting it to
false, saying No, I will not to tell you not to inline, make the
patch inlined in the body of the message, but that is not what it
does.  The result is still a MIME attachment as long as
mime_boundary is set.  This field only controls if the content
disposition of a MIME attachment is set to attachment or inline.

Rename it to clarify what it is used for.  Besides, a toggle whose
name is no_frotz is asking for a double-negation.  Calling it
disposition-attachment allows us to naturally read setting the
field to true as Yes, I want to set content-disposition to
'attachment'.

Signed-off-by: Junio C Hamano gits...@pobox.com
---

 * This is a general clean-up patch that does not add any feature
   nor fixes any bug.

 builtin/log.c | 6 +++---
 log-tree.c| 2 +-
 revision.h| 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..30265d8 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -983,7 +983,7 @@ static int attach_callback(const struct option *opt, const 
char *arg, int unset)
rev-mime_boundary = arg;
else
rev-mime_boundary = git_version_string;
-   rev-no_inline = unset ? 0 : 1;
+   rev-disposition_attachment = unset ? 0 : 1;
return 0;
 }
 
@@ -996,7 +996,7 @@ static int inline_callback(const struct option *opt, const 
char *arg, int unset)
rev-mime_boundary = arg;
else
rev-mime_boundary = git_version_string;
-   rev-no_inline = 0;
+   rev-disposition_attachment = 0;
return 0;
 }
 
@@ -1172,7 +1172,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
 
if (default_attach) {
rev.mime_boundary = default_attach;
-   rev.no_inline = 1;
+   rev.disposition_attachment = 1;
}
 
/*
diff --git a/log-tree.c b/log-tree.c
index 5dc45c4..34ec20d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -408,7 +408,7 @@ void log_write_email_headers(struct rev_info *opt, struct 
commit *commit,
  filename=\%s\\n\n,
 mime_boundary_leader, opt-mime_boundary,
 filename.buf,
-opt-no_inline ? attachment : inline,
+opt-disposition_attachment ? attachment : inline,
 filename.buf);
opt-diffopt.stat_sep = buffer;
strbuf_release(filename);
diff --git a/revision.h b/revision.h
index 5da09ee..90813dd 100644
--- a/revision.h
+++ b/revision.h
@@ -142,7 +142,7 @@ struct rev_info {
const char  *extra_headers;
const char  *log_reencode;
const char  *subject_prefix;
-   int no_inline;
+   int disposition_attachment;
int show_log_size;
struct string_list *mailmap;
 
-- 
1.8.2.rc0.129.gcce6fe7

--
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] git-filter-branch.txt: clarify ident variables usage

2013-02-21 Thread Tadeusz Andrzej Kadłubowski
There is a rare edge case of git-filter-branch: a filter that unsets
identity variables from the environment. Link to git-commit-tree
clarifies how Git would fall back in this situation.

Signed-off-by: Tadeusz Andrzej Kadłubowski y...@hell.org.pl
---
 Documentation/git-filter-branch.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index dfd12c9..e50ee2f 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -65,9 +65,9 @@ Prior to that, the $GIT_COMMIT environment variable will be 
set to contain
 the id of the commit being rewritten.  Also, GIT_AUTHOR_NAME,
 GIT_AUTHOR_EMAIL, GIT_AUTHOR_DATE, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
 and GIT_COMMITTER_DATE are set according to the current commit.  The values
-of these variables after the filters have run, are used for the new commit.
-If any evaluation of command returns a non-zero exit status, the whole
-operation will be aborted.
+of these variables after the filters have run, are used for the new commit
+(see linkgit:git-commit-tree[1] for details).  If any evaluation of command
+returns a non-zero exit status, the whole operation will be aborted.
  A 'map' function is available that takes an original sha1 id argument
 and outputs a rewritten sha1 id if the commit has been already
-- 
1.7.11.7

--
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] Documentation: filter-branch env-filter example

2013-02-21 Thread Tadeusz Andrzej Kadłubowski
filter-branch --env-filter example that shows how to change the email
address in all commits before publishing a project.

Signed-off-by: Tadeusz Andrzej Kadłubowski y...@hell.org.pl
---
 Documentation/git-filter-branch.txt | 21 +
 1 file changed, 21 insertions(+)

diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index e50ee2f..660bd32 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -329,6 +329,27 @@ git filter-branch --msg-filter '
 ' HEAD~10..HEAD
 
 +The `--env-filter` option can be used to modify committer and/or author
+identity.  For example, if you found out that your commits have the wrong
+identity due to a misconfigured user.email, you can make a correction,
+before publishing the project, like this:
+
+
+
+git filter-branch --env-filter '
+   if test $GIT_AUTHOR_EMAIL = root@localhost
+   then
+   GIT_AUTHOR_EMAIL=j...@example.com
+   export GIT_AUTHOR_EMAIL
+   fi
+   if test $GIT_COMMITTER_EMAIL = root@localhost
+   then
+   GIT_COMMITTER_EMAIL=j...@example.com
+   export GIT_COMMITTER_EMAIL
+   fi
+' -- --all
+
+
 To restrict rewriting to only part of the history, specify a revision
 range in addition to the new branch name.  The new branch name will
 point to the top-most revision that a 'git rev-list' of this range
-- 
1.7.11.7

--
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 v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline

2013-02-21 Thread Brandon Casey
On Thu, Feb 21, 2013 at 12:26 PM, Brandon Casey draf...@gmail.com wrote:

 But, this does not fix the same problem for 'cherry-pick --edit -s'
 when used to cherry-pick a commit without a sob.

Correction: when used to cherry-pick a commit with an empty commit message.
--
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 v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline

2013-02-21 Thread Brandon Casey
On Thu, Feb 21, 2013 at 10:51 AM, Junio C Hamano gits...@pobox.com wrote:
 Brandon Casey draf...@gmail.com writes:

 Teach append_signoff to detect whether a blank line exists at the position
 that the signed-off-by line will be added, and refrain from adding an
 additional one if one already exists.  Or, add an additional line if one
 is needed to make sure the new footer is separated from the message body
 by a blank line.

 Signed-off-by: Brandon Casey bca...@nvidia.com
 ---


 A slight tweak.  And I promise, no more are coming.

 When I do

 $ git commit -s

 it should start my editor with this in the buffer:

 

 Signed-off-by: Junio C Hamano gits...@pobox.com
 

 and the cursor blinking at the beginning of the file.  Annoyingly
 this step breaks it by removing the leading blank line.

Yes.  The fix described by John Keeping restores the above behavior
for 'commit -s'.  Or the fix I described which inserts two preceding
newlines so it looks like this:

   


   Signed-off-by: Junio C Hamano gits...@pobox.com
   

So then the cursor would be placed on the first line and a space would
separate it from the sob which is arguably a better indication to the
user that a blank line should separate the commit message body from
the sob.

But, this does not fix the same problem for 'cherry-pick --edit -s'
when used to cherry-pick a commit without a sob.  The cherry-pick part
of it would add the extra preceding newlines, but then cherry-pick
passes the buffer to 'git commit' via .git/MERGE_MSG which then
cleans the buffer, removing the empty lines, in prepare_to_commit()
before allowing the editor to operate on it.

Using 'cherry-pick --edit -s' to cherry-pick a commit with an empty
commit message is going to be a pretty rare corner case.  It would be
nice to have the same behavior for it that we decide to have for
'commit -s', but it's probably not worth going through contortions to
make it happen.

-Brandon
--
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] format-patch: --inline-single

2013-02-21 Thread Junio C Hamano
Some people may find it convenient to append a simple patch at the
bottom of a discussion e-mail separated by a scissors mark, ready
to be applied with git am -c.  Introduce --inline-single option
to format-patch to do so.  A typical usage example might be to start
'F'ollow-up to a discussion, write your message, conclude with a
patch to do so may look like this., and 

\C-u M-! git format-patch --inline-single -1 HEAD ENTER

if you are an Emacs user.  Users of other MUA's may want to consult
their manuals to find equivalent command to append output from an
external command to the message being composed.

It does not make any sense to use this mode when formatting multiple
patches, or to combine this with options such as --attach, --inline,
and --cover-letter, so some of such uses are forbidden.  There may
be more insane combination the check in this patch may not even
bother to reject.  Caveat emptor.

Signed-off-by: Junio C Hamano gits...@pobox.com
---

 * I did this as a lunch-time hack, but I'll leave it to interested
   readers as an exercise to find corner case bugs, e.g. some
   insane combinations of options may not be diagnosed as usage
   errors, and to update the tests and documentation.

   Personally, git format-patch --stdout -1 HEAD with manual
   editing is more flexible, so I am not interested in spending
   cycles to polish this further myself.

   The preliminary patch 1/2 I sent earlier is worth doing, though.

 builtin/log.c | 32 
 commit.h  |  1 +
 log-tree.c|  7 ++-
 pretty.c  | 27 ++-
 revision.h|  1 +
 5 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 30265d8..5ad0837 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1000,6 +1000,19 @@ static int inline_callback(const struct option *opt, 
const char *arg, int unset)
return 0;
 }
 
+static int inline_single_callback(const struct option *opt, const char *arg, 
int unset)
+{
+   struct rev_info *rev = (struct rev_info *)opt-value;
+   rev-mime_boundary = NULL;
+   rev-inline_single = 1;
+
+   /* defeat configured format.attach, format.thread, etc. */
+   free(default_attach);
+   default_attach = NULL;
+   thread = 0;
+   return 0;
+}
+
 static int header_callback(const struct option *opt, const char *arg, int 
unset)
 {
if (unset) {
@@ -1149,6 +1162,10 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
PARSE_OPT_OPTARG, thread_callback },
OPT_STRING(0, signature, signature, N_(signature),
N_(add a signature)),
+   { OPTION_CALLBACK, 0, inline-single, rev, NULL,
+ N_(single patch appendable to the end of an e-mail body),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG,
+ inline_single_callback },
OPT_BOOLEAN(0, quiet, quiet,
N_(don't print the patch filenames)),
OPT_END()
@@ -1185,6 +1202,17 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
 PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 PARSE_OPT_KEEP_DASHDASH);
 
+   /* Set defaults and check incompatible options */
+   if (rev.inline_single) {
+   use_stdout = 1;
+   if (cover_letter)
+   die(_(inline-single and cover-letter are 
incompatible.));
+   if (thread)
+   die(_(inline-single and thread are incompatible.));
+   if (output_directory)
+   die(_(inline-single and output-directory are 
incompatible.));
+   }
+
if (0  reroll_count) {
struct strbuf sprefix = STRBUF_INIT;
strbuf_addf(sprefix, %s v%d,
@@ -1373,6 +1401,10 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
list[nr - 1] = commit;
}
total = nr;
+
+   if (rev.inline_single  total != 1)
+   die(_(inline-single is only for a single commit));
+
if (!keep_subject  auto_number  total  1)
numbered = 1;
if (numbered)
diff --git a/commit.h b/commit.h
index 4138bb4..f3d9959 100644
--- a/commit.h
+++ b/commit.h
@@ -85,6 +85,7 @@ struct pretty_print_context {
int preserve_subject;
enum date_mode date_mode;
unsigned date_mode_explicit:1;
+   unsigned inline_single:1;
int need_8bit_cte;
char *notes_message;
struct reflog_walk_info *reflog_info;
diff --git a/log-tree.c b/log-tree.c
index 34ec20d..15c9749 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -358,7 +358,11 @@ void log_write_email_headers(struct rev_info *opt, struct 
commit *commit,
subject = Subject: ;
}
 
-   printf(From %s Mon Sep 17 00:00:00 2001\n, name);
+   if 

Re: [PATCH 1/2] git-filter-branch.txt: clarify ident variables usage

2013-02-21 Thread Junio C Hamano
Tadeusz Andrzej Kadłubowski  y...@hell.org.pl writes:

 There is a rare edge case of git-filter-branch: a filter that unsets
 identity variables from the environment. Link to git-commit-tree
 clarifies how Git would fall back in this situation.

I find it unclear in the updated text _why_ the reader may want to
refer to that other documentation.

 Signed-off-by: Tadeusz Andrzej Kadłubowski y...@hell.org.pl
 ---
  Documentation/git-filter-branch.txt | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/Documentation/git-filter-branch.txt 
 b/Documentation/git-filter-branch.txt
 index dfd12c9..e50ee2f 100644
 --- a/Documentation/git-filter-branch.txt
 +++ b/Documentation/git-filter-branch.txt
 @@ -65,9 +65,9 @@ Prior to that, the $GIT_COMMIT environment variable will be 
 set to contain
  the id of the commit being rewritten.  Also, GIT_AUTHOR_NAME,
  GIT_AUTHOR_EMAIL, GIT_AUTHOR_DATE, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
  and GIT_COMMITTER_DATE are set according to the current commit.  The values
 -of these variables after the filters have run, are used for the new commit.
 -If any evaluation of command returns a non-zero exit status, the whole
 -operation will be aborted.
 +of these variables after the filters have run, are used for the new commit
 +(see linkgit:git-commit-tree[1] for details).  If any evaluation of command
 +returns a non-zero exit status, the whole operation will be aborted.

Here is my attempt to clarify it a bit.

Also, ...variables... are taken from the current commit
and exported to the environment, in order to affect the
author and committer identities of the replacement commit
created by linkgit:git-commit-tree[1] after the filters have
run.

A user who contemplates to unset them should be able to guess the
consequences, even though the above text does not single out such an
insane misuse to put an undue stress on it.

   A 'map' function is available that takes an original sha1 id argument
  and outputs a rewritten sha1 id if the commit has been 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 2/2] Documentation: filter-branch env-filter example

2013-02-21 Thread Junio C Hamano
Tadeusz Andrzej Kadłubowski  y...@hell.org.pl writes:

 filter-branch --env-filter example that shows how to change the email
 address in all commits before publishing a project.

 Signed-off-by: Tadeusz Andrzej Kadłubowski y...@hell.org.pl
 ---

Assuming that the result formats well both as html and manpage, the
added example looks good to me.  I somehow suspect that there is a
patch-mangling going on, though.

  Documentation/git-filter-branch.txt | 21 +
  1 file changed, 21 insertions(+)

 diff --git a/Documentation/git-filter-branch.txt 
 b/Documentation/git-filter-branch.txt
 index e50ee2f..660bd32 100644
 --- a/Documentation/git-filter-branch.txt
 +++ b/Documentation/git-filter-branch.txt
 @@ -329,6 +329,27 @@ git filter-branch --msg-filter '
  ' HEAD~10..HEAD
  
  +The `--env-filter` option can be used to modify committer and/or author
 +identity.  For example, if you found out that your commits have the wrong
 +identity due to a misconfigured user.email, you can make a correction,
 +before publishing the project, like this:
 +
 +
 +
 +git filter-branch --env-filter '
 + if test $GIT_AUTHOR_EMAIL = root@localhost
 + then
 + GIT_AUTHOR_EMAIL=j...@example.com
 + export GIT_AUTHOR_EMAIL
 + fi
 + if test $GIT_COMMITTER_EMAIL = root@localhost
 + then
 + GIT_COMMITTER_EMAIL=j...@example.com
 + export GIT_COMMITTER_EMAIL
 + fi
 +' -- --all
 +
 +
  To restrict rewriting to only part of the history, specify a revision
  range in addition to the new branch name.  The new branch name will
  point to the top-most revision that a 'git rev-list' of this range
--
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 v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline

2013-02-21 Thread Junio C Hamano
Brandon Casey draf...@gmail.com writes:

 Yes.  The fix described by John Keeping restores the above behavior
 for 'commit -s'.  Or the fix I described which inserts two preceding
 newlines so it looks like this:




Signed-off-by: Junio C Hamano gits...@pobox.com


 So then the cursor would be placed on the first line and a space would
 separate it from the sob which is arguably a better indication to the
 user that a blank line should separate the commit message body from
 the sob.

That sounds like an improvement to me.

 But, this does not fix the same problem for 'cherry-pick --edit -s'
 when used to cherry-pick a commit without a sob. ...
 Using 'cherry-pick --edit -s' to cherry-pick a commit with an empty
 commit message is going to be a pretty rare corner case

We actively discourage an empty commit message by requiring users to
say commit --allow-empty-message.  I think it is in line with the
philosophy for a Porcelain command git cherry-pick -s to punish
users by making them work harder to use a commit with an empty
message ;-).


--
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] Fix in Git.pm cat_blob crashes on large files

2013-02-21 Thread Joshua Clayton
Greetings.
This is my first patch here. Hopefully I get the stylistic  political
details right... :)
Patch applies against maint and master

(If I understand the mechanics, in theory a negative offset should work,
 if the values lined up just right, but would be very wrong, overwriting the
lower contents of the file)

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Affects git svn clone/fetch
Original code loaded entire file contents into a variable
before writing to disk. If the offset within the variable passed
2 GiB, it becrame negative, resulting in a crash.
On a 32 bit system, or a system with low memory it may crash before
reaching 2 GiB due to memory exhaustion.
Fix writes in smaller 64K increments. Tested to work on git svn fetch

Signed-off-by: Joshua Clayton stillcompil...@gmail.com
---
 perl/Git.pm |   19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..e55840f 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -942,6 +942,8 @@ sub cat_blob {
my $size = $1;

my $blob;
+   my $blobSize = 0;
+   my $flushSize = 1024*64;
my $bytesRead = 0;

while (1) {
@@ -949,13 +951,21 @@ sub cat_blob {
last unless $bytesLeft;

my $bytesToRead = $bytesLeft  1024 ? $bytesLeft : 1024;
-   my $read = read($in, $blob, $bytesToRead, $bytesRead);
+   my $read = read($in, $blob, $bytesToRead, $blobSize);
unless (defined($read)) {
$self-_close_cat_blob();
throw Error::Simple(in pipe went bad);
}
-
$bytesRead += $read;
+   $blobSize += $read;
+   if (($blobSize = $flushSize) || ($bytesLeft = 1024)) {
+   unless (print $fh $blob) {
+   $self-_close_cat_blob();
+   throw Error::Simple(couldn't write to passed 
in filehandle);
+   }
+   $blob = ;
+   $blobSize = 0;
+   }
}

# Skip past the trailing newline.
@@ -970,11 +980,6 @@ sub cat_blob {
throw Error::Simple(didn't find newline after blob);
}

-   unless (print $fh $blob) {
-   $self-_close_cat_blob();
-   throw Error::Simple(couldn't write to passed in filehandle);
-   }
-
return $size;
 }

-- 
1.7.10.4
--
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] Fix in Git.pm cat_blob crashes on large files

2013-02-21 Thread Jeff King
On Thu, Feb 21, 2013 at 02:13:32PM -0800, Joshua Clayton wrote:

 Greetings.
 This is my first patch here. Hopefully I get the stylistic  political
 details right... :)
 Patch applies against maint and master

I have some comments. :)

The body of your email should contain the commit message (i.e., whatever
people reading git log a year from now would see). Cover letter bits
like this should go after the ---. That way git am knows which part
is which.

 Developer's Certificate of Origin 1.1

You don't need to include the DCO. Your Signed-off-by is an indication
that you agree to it.

 Affects git svn clone/fetch
 Original code loaded entire file contents into a variable
 before writing to disk. If the offset within the variable passed
 2 GiB, it becrame negative, resulting in a crash.

Interesting. I didn't think perl had signed wrap-around issues like
this, as its numeric variables are not strictly integers. But I don't
have a 32-bit machine to test on (and numbers larger than 2G obviously
work on 64-bit machines). At any rate, though:

 On a 32 bit system, or a system with low memory it may crash before
 reaching 2 GiB due to memory exhaustion.

Yeah, it is stupid to read the whole thing into memory if we are just
going to dump it to another filehandle.

 @@ -949,13 +951,21 @@ sub cat_blob {
   last unless $bytesLeft;
 
   my $bytesToRead = $bytesLeft  1024 ? $bytesLeft : 1024;
 - my $read = read($in, $blob, $bytesToRead, $bytesRead);
 + my $read = read($in, $blob, $bytesToRead, $blobSize);
   unless (defined($read)) {
   $self-_close_cat_blob();
   throw Error::Simple(in pipe went bad);
   }

Hmph. The existing code already reads in 1024-byte chunks. For no
reason, as far as I can tell, since we are just loading the blob buffer
incrementally into memory, only to then flush it all out at once.

Why do you read at the $blobSize offset? If we are just reading in
chunks, we be able to just keep writing to the start of our small
buffer, as we flush each chunk out before trying to read more.

IOW, shouldn't the final code look like this:

  my $bytesLeft = $size;
  while ($bytesLeft  0) {
  my $buf;
  my $bytesToRead = $bytesLeft  1024 ? $bytesLeft : 1024;
  my $read = read($in, $buf, $bytesToRead);
  unless (defined($read)) {
  $self-_close_cat_blob();
  throw Error::Simple(unable to read cat-blob pipe);
  }
  unless (print $fh $buf) {
  $self-_close_cat_blob();
  throw Error::Simple(unable to write blob output);
  }

  $bytesLeft -= $read;
  }

By having the read and flush size be the same, it's much simpler.

Your change (and my proposed code) do mean that an error during the read
operation will result in a truncated output file, rather than an empty
one. I think that is OK, though. That can happen anyway in the original
due to a failure in the print step. Any caller who wants to be careful
that they leave only a full file in place must either:

  1. Check the return value of cat_blob and verify that the result has
 $size bytes, and otherwise delete it.

  2. Write to a temporary file, then once success has been returned from
 cat_blob, rename the result into place.

Neither of which is affected by this change.

-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: [RFC] Provide a mechanism to turn off symlink resolution in ceiling paths

2013-02-21 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 Unfortunately I am swamped with other work right now so I don't have
 time to test the code and might not be able to respond promptly to
 feedback.

A note like the above is a good way to give a cue to others so that
we can work together to pick up, tie the loose ends and move us
closer to the goal, and is very much appreciated.

I think the patch makes sense; I expanded on the part that has
Anders's report in the log message and added a trivial test.

Testing and eyeballing by others would help very much.  We'd
obviously need our sign-off as well ;-)

Thanks.

-- 8 --
From: Michael Haggerty mhag...@alum.mit.edu
Date: Wed, 20 Feb 2013 10:09:24 +0100
Subject: [PATCH] Provide a mechanism to turn off symlink resolution in ceiling 
paths

Commit 1b77d83cab 'setup_git_directory_gently_1(): resolve symlinks
in ceiling paths' changed the setup code to resolve symlinks in the
entries in GIT_CEILING_DIRECTORIES.  Because those entries are
compared textually to the symlink-resolved current directory, an
entry in GIT_CEILING_DIRECTORIES that contained a symlink would have
no effect.  It was known that this could cause performance problems
if the symlink resolution *itself* touched slow filesystems, but it
was thought that such use cases would be unlikely.  The intention of
the earlier change was to deal with a case when the user has this:

GIT_CEILING_DIRECTORIES=/home/gitster

but in reality, /home/gitster is a symbolic link to somewhere else,
e.g. /net/machine/home4/gitster. A textual comparison between the
specified value /home/gitster and the location getcwd(3) returns
would not help us, but readlink(/home/gitster) would still be
fast.

After this change was released, Anders Kaseorg ande...@mit.edu
reported:

 [...] my computer has been acting so slow when I’m not connected to
 the network.  I put various network filesystem paths in
 $GIT_CEILING_DIRECTORIES, such as
 /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents
 /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and
 /afs/athena.mit.edu/user/a/n which all live in different AFS
 volumes).  Now when I’m not connected to the network, every
 invocation of Git, including the __git_ps1 in my shell prompt, waits
 for AFS to timeout.

To allow users to work this around, give them a mechanism to turn
off symlink resolution in GIT_CEILING_DIRECTORIES entries.  All the
entries that follow an empty entry will not be checked for symbolic
links and used literally in comparison.  E.g. with these:

GIT_CEILING_DIRECTORIES=:/foo/bar:/xyzzy or
GIT_CEILING_DIRECTORIES=/foo/bar::/xyzzy

we will not readlink(/xyzzy), and with the former, we will not
readlink(/foo/bar), either.
---
 Documentation/git.txt   | 19 +--
 setup.c | 32 ++--
 t/t1504-ceiling-dirs.sh | 17 +
 3 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 6710cb0..5c03616 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -653,12 +653,19 @@ git so take care if using Cogito etc.
The '--namespace' command-line option also sets this value.
 
 'GIT_CEILING_DIRECTORIES'::
-   This should be a colon-separated list of absolute paths.
-   If set, it is a list of directories that git should not chdir
-   up into while looking for a repository directory.
-   It will not exclude the current working directory or
-   a GIT_DIR set on the command line or in the environment.
-   (Useful for excluding slow-loading network directories.)
+   This should be a colon-separated list of absolute paths.  If
+   set, it is a list of directories that git should not chdir up
+   into while looking for a repository directory (useful for
+   excluding slow-loading network directories).  It will not
+   exclude the current working directory or a GIT_DIR set on the
+   command line or in the environment.  Normally, Git has to read
+   the entries in this list are read to resolve any symlinks that
+   might be present in order to compare them with the current
+   directory.  However, if even this access is slow, you
+   can add an empty entry to the list to tell Git that the
+   subsequent entries are not symlinks and needn't be resolved;
+   e.g.,
+   'GIT_CEILING_DIRECTORIES=/maybe/symlink::/very/slow/non/symlink'.
 
 'GIT_DISCOVERY_ACROSS_FILESYSTEM'::
When run in a directory that does not have .git repository
diff --git a/setup.c b/setup.c
index f108c4b..1b12017 100644
--- a/setup.c
+++ b/setup.c
@@ -624,22 +624,32 @@ static dev_t get_device_or_die(const char *path, const 
char *prefix, int prefix_
 /*
  * A string_list_each_func_t function that canonicalizes an entry
  * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or
- * discards it if unusable.
+ * discards it if unusable.  The presence of an empty 

Re: [PATCH 2/2] format-patch: --inline-single

2013-02-21 Thread Jeff King
On Thu, Feb 21, 2013 at 12:26:22PM -0800, Junio C Hamano wrote:

 Some people may find it convenient to append a simple patch at the
 bottom of a discussion e-mail separated by a scissors mark, ready
 to be applied with git am -c.  Introduce --inline-single option
 to format-patch to do so.  A typical usage example might be to start
 'F'ollow-up to a discussion, write your message, conclude with a
 patch to do so may look like this., and
 
 \C-u M-! git format-patch --inline-single -1 HEAD ENTER
 
 if you are an Emacs user.  Users of other MUA's may want to consult
 their manuals to find equivalent command to append output from an
 external command to the message being composed.

Interesting. I usually just do this by hand, but this could save a few
keystrokes in my workflow.

 +static int is_current_user(const struct pretty_print_context *pp,
 +const char *email, size_t emaillen,
 +const char *name, size_t namelen)
 +{
 + const char *me = git_committer_info(0);
 + const char *myname, *mymail;
 + size_t mynamelen, mymaillen;
 + struct ident_split ident;
 +
 + if (split_ident_line(ident, me, strlen(me)))
 + return 0; /* play safe, as we do not know */
 + mymail = ident.mail_begin;
 + mymaillen = ident.mail_end - ident.mail_begin;
 + myname = ident.name_begin;
 + mynamelen = ident.name_end - ident.name_begin;
 + if (pp-mailmap)
 + map_user(pp-mailmap, mymail, mymaillen, myname, mynamelen);
 + return (mymaillen == emaillen 
 + mynamelen == namelen 
 + !memcmp(mymail, email, emaillen) 
 + !memcmp(myname, name, namelen));
 +}

Nice, I'm glad you handled this case properly. I've wondered if we
should have an option to do a similar test when writing out the real
message format. I.e., to put the extra From line in the body of the
message when !is_current_user(). Traditionally we have just said that
is the responsibility of the MUA you use, and let send-email handle it.
But it means people who do not use send-email have to reimplement the
feature themselves.

 @@ -421,6 +443,9 @@ void pp_user_info(const struct pretty_print_context *pp,
   if (pp-mailmap)
   map_user(pp-mailmap, mailbuf, maillen, namebuf, namelen);
  
 + if (pp-inline_single  is_current_user(pp, mailbuf, maillen, namebuf, 
 namelen))
 + return;
 +
   strbuf_init(mail, 0);
   strbuf_init(name, 0);

This makes sense to suppress the user line when it is not necessary. But
we should probably always be suppressing the Date line, as it is almost
always useless.

I also wonder if we should suppress the subject-prefix in such a case,
as it is not adding anything (it is not the subject of the email, so it
does not need to grab attention there, and it will not make it into the
final commit). On the other hand, having tried it, the Subject: looks
a little lonely without it. Perhaps the [PATCH] is still necessary to
grab attention after the scissors line. I dunno.

Patch for both below if you want to pick up either suggestion.

diff --git a/log-tree.c b/log-tree.c
index 15c9749..8994354 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -348,7 +348,8 @@ void log_write_email_headers(struct rev_info *opt, struct 
commit *commit,
 digits_in_number(opt-total),
 opt-nr, opt-total);
subject = buffer;
-   } else if (opt-total == 0  opt-subject_prefix  
*opt-subject_prefix) {
+   } else if (opt-total == 0  !opt-inline_single 
+  opt-subject_prefix  *opt-subject_prefix) {
static char buffer[256];
snprintf(buffer, sizeof(buffer),
 Subject: [%s] ,
diff --git a/pretty.c b/pretty.c
index 363b3d9..1a7352c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -490,7 +490,8 @@ void pp_user_info(const struct pretty_print_context *pp,
strbuf_addf(sb, Date:   %s\n, show_date(time, tz, 
pp-date_mode));
break;
case CMIT_FMT_EMAIL:
-   strbuf_addf(sb, Date: %s\n, show_date(time, tz, 
DATE_RFC2822));
+   if (!pp-inline_single)
+   strbuf_addf(sb, Date: %s\n, show_date(time, tz, 
DATE_RFC2822));
break;
case CMIT_FMT_FULLER:
strbuf_addf(sb, %sDate: %s\n, what, show_date(time, tz, 
pp-date_mode));
--
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] Fix in Git.pm cat_blob crashes on large files

2013-02-21 Thread Joshua Clayton
On Thu, Feb 21, 2013 at 2:43 PM, Jeff King p...@peff.net wrote:
 On Thu, Feb 21, 2013 at 02:13:32PM -0800, Joshua Clayton wrote:

 Greetings.
 This is my first patch here. Hopefully I get the stylistic  political
 details right... :)
 Patch applies against maint and master

 I have some comments. :)

 The body of your email should contain the commit message (i.e., whatever
 people reading git log a year from now would see). Cover letter bits
 like this should go after the ---. That way git am knows which part
 is which.

 Developer's Certificate of Origin 1.1

 You don't need to include the DCO. Your Signed-off-by is an indication
 that you agree to it.

 Affects git svn clone/fetch
 Original code loaded entire file contents into a variable
 before writing to disk. If the offset within the variable passed
 2 GiB, it becrame negative, resulting in a crash.

 Interesting. I didn't think perl had signed wrap-around issues like
 this, as its numeric variables are not strictly integers. But I don't
 have a 32-bit machine to test on (and numbers larger than 2G obviously
 work on 64-bit machines). At any rate, though:

 On a 32 bit system, or a system with low memory it may crash before
 reaching 2 GiB due to memory exhaustion.

 Yeah, it is stupid to read the whole thing into memory if we are just
 going to dump it to another filehandle.

 @@ -949,13 +951,21 @@ sub cat_blob {
   last unless $bytesLeft;

   my $bytesToRead = $bytesLeft  1024 ? $bytesLeft : 1024;
 - my $read = read($in, $blob, $bytesToRead, $bytesRead);
 + my $read = read($in, $blob, $bytesToRead, $blobSize);
   unless (defined($read)) {
   $self-_close_cat_blob();
   throw Error::Simple(in pipe went bad);
   }

 Hmph. The existing code already reads in 1024-byte chunks. For no
 reason, as far as I can tell, since we are just loading the blob buffer
 incrementally into memory, only to then flush it all out at once.

 Why do you read at the $blobSize offset? If we are just reading in
 chunks, we be able to just keep writing to the start of our small
 buffer, as we flush each chunk out before trying to read more.

 IOW, shouldn't the final code look like this:

   my $bytesLeft = $size;
   while ($bytesLeft  0) {
   my $buf;
   my $bytesToRead = $bytesLeft  1024 ? $bytesLeft : 1024;
   my $read = read($in, $buf, $bytesToRead);
   unless (defined($read)) {
   $self-_close_cat_blob();
   throw Error::Simple(unable to read cat-blob pipe);
   }
   unless (print $fh $buf) {
   $self-_close_cat_blob();
   throw Error::Simple(unable to write blob output);
   }

   $bytesLeft -= $read;
   }

 By having the read and flush size be the same, it's much simpler.

My original bugfix did just read 1024, and write 1024. That works fine
and, yes, is simpler.
I changed it to be more similar to the original code in case there
were performance reasons for doing it that way.
That was the only reason I could think of for the design, and adding
the $flushSize variable means that
some motivated person could easily optimize it.

So far I have been too lazy to profile the two versions
I guess I'll try a trivial git svn init; git svn fetch and check back in.
Its running now.


 Your change (and my proposed code) do mean that an error during the read
 operation will result in a truncated output file, rather than an empty
 one. I think that is OK, though. That can happen anyway in the original
 due to a failure in the print step. Any caller who wants to be careful
 that they leave only a full file in place must either:

   1. Check the return value of cat_blob and verify that the result has
  $size bytes, and otherwise delete it.

   2. Write to a temporary file, then once success has been returned from
  cat_blob, rename the result into place.

 Neither of which is affected by this change.

 -Peff

In git svn fetch (which is how I discovered it) the file being passed
to cat_blob is a temporary file, which is checksummed before putting
it into place.
--
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] Fix in Git.pm cat_blob crashes on large files

2013-02-21 Thread Jeff King
On Thu, Feb 21, 2013 at 03:18:40PM -0800, Joshua Clayton wrote:

  By having the read and flush size be the same, it's much simpler.
 
 My original bugfix did just read 1024, and write 1024. That works fine
 and, yes, is simpler.
 I changed it to be more similar to the original code in case there
 were performance reasons for doing it that way.
 That was the only reason I could think of for the design, and adding
 the $flushSize variable means that
 some motivated person could easily optimize it.
 
 So far I have been too lazy to profile the two versions
 I guess I'll try a trivial git svn init; git svn fetch and check back in.
 Its running now.

I doubt it will make much of a difference; we are already writing to a
perl filehandle, so it will be buffered there (which I assume is 4K, but
I haven't checked). And your version retains the 1024-byte read. I do
think 1024 is quite low for this sort of descriptor-to-descriptor
copying. I'd be tempted to just bump that 1024 to 64K.

 In git svn fetch (which is how I discovered it) the file being passed
 to cat_blob is a temporary file, which is checksummed before putting
 it into place.

Great. There may be other callers outside of our tree, of course, but I
think it's pretty clear that the responsibility is on the caller to make
sure the function succeeded. We are changing what gets put on the output
stream for various error conditions, but ultimately that is an
implementation detail that the caller should not be depending on.

-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: git svn problem, probably a regression

2013-02-21 Thread RIceball LEE
Joseph Crowell joseph.w.crowell at gmail.com writes:

 
   Use of uninitialized value $u in substitution (s///) at
 /usr/local/Cellar/git/1.8.0/lib/Git/SVN.pm
  line 106.
   Use of uninitialized value $u in concatenation (.) or string at
  /usr/local/Cellar/git/1.8.0/lib/Git/SVN.pm line 106.
   refs/remotes/svn/asset-manager-redesign: 'svn+ssh://IP address' not 
   found
 in ''
 
  
  If you have time and can reproduce the bug at will, it would be very
  helpful to use git-bisect to pinpoint the exact commit that causes the
  breakage.
  
  -Peff
  
 
 I just encountered the exact same issue while converting an svn repo to git on
 Windows through Git Bash. Same error.
 
 


me too:

git svn fetch
Found possible branch point: 
https://amanda.svn.sourceforge.net/svnroot/amanda/amanda/branches/amanda-260 = 
https://amanda.svn.sourceforge.net/svnroot/amanda/amanda/tags/amanda260p1, 1022
Use of uninitialized value $u in substitution (s///) at 
/usr/local/Cellar/git/1.8.1.3/lib/Git/SVN.pm 
line 106.
Use of uninitialized value $u in concatenation (.) or string at 
/usr/local/Cellar/git/1.8.1.3/lib/Git/SVN.pm line 106.
refs/remotes/svn/amanda-260: 
'https://amanda.svn.sourceforge.net/svnroot/amanda' 
not found 
in ''

--
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 4/4] t7800: defaults is no longer a builtin tool name

2013-02-21 Thread David Aguilar
On Wed, Feb 20, 2013 at 9:00 PM, Junio C Hamano gits...@pobox.com wrote:
 David Aguilar dav...@gmail.com writes:

 diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
 index fb00273..21fbba9 100755
 --- a/t/t7800-difftool.sh
 +++ b/t/t7800-difftool.sh
 @@ -60,9 +60,9 @@ test_expect_success PERL 'custom commands' '
  '

  test_expect_success PERL 'custom tool commands override built-ins' '
 - test_config difftool.defaults.cmd cat \\$REMOTE\ 
 + test_config difftool.vimdiff cat \\$REMOTE\ 
   echo master expect 
 - git difftool --tool defaults --no-prompt branch actual 
 + git difftool --tool vimdiff --no-prompt branch actual 
   test_cmp expect actual
  '

 Eek.

 $ sh t7800-difftool.sh -i
 ok 1 - setup
 ok 2 - custom commands
 not ok 3 - custom tool commands override built-ins
 #
 #   test_config difftool.vimdiff cat \\$REMOTE\ 
 #   echo master expect 
 #   git difftool --tool vimdiff --no-prompt branch actual 
 #   test_cmp expect actual
 #

 Running the same test with -v seems to get stuck with this
 forever:

 expecting success:
 test_config difftool.vimdiff cat \\$REMOTE\ 
 echo master expect 
 git difftool --tool vimdiff --no-prompt branch actual 
 test_cmp expect actual

 Vim: Warning: Output is not to a terminal
 Vim: Warning: Input is not from a terminal


Oh boy.  I botched the rebase.  I have a replacement.  I thought I
re-ran the tests!  I'll resend it.
-- 
David
--
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 v4 4/4] t7800: defaults is no longer a builtin tool name

2013-02-21 Thread David Aguilar
073678b8e6324a155fa99f40eee0637941a70a34 reworked the
mergetools/ directory so that every file corresponds to a
difftool-supported tool.  When this happened the defaults
file went away as it was no longer needed by mergetool--lib.

t7800 tests that configured commands can override builtins,
but this test was not adjusted when the defaults file was
removed because the test continued to pass.

Adjust the test to use the everlasting vimdiff tool name
instead of defaults so that it correctly tests against a tool
that is known by mergetool--lib.

Signed-off-by: David Aguilar dav...@gmail.com
---
Replacement patch to fix my botched rebase.

 t/t7800-difftool.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index fb00273..3aab6e1 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -60,9 +60,9 @@ test_expect_success PERL 'custom commands' '
 '
 
 test_expect_success PERL 'custom tool commands override built-ins' '
-   test_config difftool.defaults.cmd cat \\$REMOTE\ 
+   test_config difftool.vimdiff.cmd cat \\$REMOTE\ 
echo master expect 
-   git difftool --tool defaults --no-prompt branch actual 
+   git difftool --tool vimdiff --no-prompt branch actual 
test_cmp expect actual
 '
 
-- 
1.8.2.rc0.20.gf548dd7

--
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 2/2] format-patch: --inline-single

2013-02-21 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 @@ -421,6 +443,9 @@ void pp_user_info(const struct pretty_print_context *pp,
  if (pp-mailmap)
  map_user(pp-mailmap, mailbuf, maillen, namebuf, namelen);
  
 +if (pp-inline_single  is_current_user(pp, mailbuf, maillen, namebuf, 
 namelen))
 +return;
 +
  strbuf_init(mail, 0);
  strbuf_init(name, 0);

 This makes sense to suppress the user line when it is not necessary. But
 we should probably always be suppressing the Date line, as it is almost
 always useless.

When I (figuratively) am sending my patch in a discussion, saying
You could do it this way, on the other hand, I agree that the date
is uninteresting.

I however think I would prefer to keep the Date: line when I am
relaying somebody else's work during a discussion.  It is more like
Yeah, Peff already did that with this commit; here it is for
reference. The fact that I have _your_ patch makes it more done,
than the case I send out my own patch.

Besides, removing an extra line in the MUA editor is far easier than
having to type what the tool helpfully omitted, guided by an it
is almost always useless that is not backed by the user preference.
I'd rather err on the side of giving extra than omitting too much.

 I also wonder if we should suppress the subject-prefix in such a case,
 as it is not adding anything (it is not the subject of the email, so it
 does not need to grab attention there, and it will not make it into the
 final commit).

If the user does not want to waste too much space in the message,
not passing the --subject-prefix=foo from the command line, or
editing it out in the editor buffer if for some reason the user ran
the command with the option, are both easy things to do.  I do not
think extra lines to excise subject prefix is not worth it, and who
knows what the user's preferences are.

But there is something more important.

We should make sure that we disable MIMEy stuff (i.e. MIME-Version,
C-T-E: 8bit/quoted-printable, Content-type, etc.) when producing the
output to be appended to the body, which should be just a straight
8-bit text.  I do not think the posted patch tries to do anything to
that effect.
--
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 2/2] format-patch: --inline-single

2013-02-21 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

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

 @@ -421,6 +443,9 @@ void pp_user_info(const struct pretty_print_context *pp,
 if (pp-mailmap)
 map_user(pp-mailmap, mailbuf, maillen, namebuf, namelen);
  
 +   if (pp-inline_single  is_current_user(pp, mailbuf, maillen, namebuf, 
 namelen))
 +   return;
 +
 strbuf_init(mail, 0);
 strbuf_init(name, 0);

 This makes sense to suppress the user line when it is not necessary. But
 we should probably always be suppressing the Date line, as it is almost
 always useless.

 When I (figuratively) am sending my patch in a discussion, saying
 You could do it this way, on the other hand, I agree that the date
 is uninteresting.

Just in case somebody is wondering, please s/, on the other hand//;
above.  I swapped the paragraphs after I wrote them X-.
--
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 v4 4/4] t7800: defaults is no longer a builtin tool name

2013-02-21 Thread Junio C Hamano
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


-B option of git log

2013-02-21 Thread Eckhard Maass
Hello,

As far as I understand the documentation, -B of git-log should help
correct rename detection. But it does not seem to work for me.

Let me get a setup:

$ git init .
Initialized empty Git repository in /tmp/t2/.git/
$ echo 'Lorem ipsum doler sed. Lorem ipsum doler sed. Lorem ipsum doler
sed. Lorem ipsum doler sed.'  a
$ git add a
$ git commit -m 'Init.'
[master (root-commit) b78205c] Init.
 1 file changed, 1 insertion(+)
 create mode 100644 a
$ mv a b
$ echo 'new'  a
$ git add -A .
$ git commit -m '2nd.'
[master a30ca49] 2nd.
 2 files changed, 2 insertions(+), 1 deletion(-)
 create mode 100644 b

Now, I get the following:

$ git log --oneline -B20%/80% -M20% --name-status
a30ca49 2nd.
M   a
A   b
b78205c Init.
A   a

But I would expect that git-log shows me a rename from a to b and a new a.

What is my misunderstanding? (And I tried it also with files with a
couple of lines.)

SEcki
--
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: -B option of git log

2013-02-21 Thread Jeff King
On Fri, Feb 22, 2013 at 01:44:00AM +0100, Eckhard Maass wrote:

 Let me get a setup:
 
 $ git init .
 Initialized empty Git repository in /tmp/t2/.git/
 $ echo 'Lorem ipsum doler sed. Lorem ipsum doler sed. Lorem ipsum doler
 sed. Lorem ipsum doler sed.'  a
 $ git add a
 $ git commit -m 'Init.'
 [master (root-commit) b78205c] Init.
  1 file changed, 1 insertion(+)
  create mode 100644 a
 $ mv a b
 $ echo 'new'  a
 $ git add -A .
 $ git commit -m '2nd.'
 [master a30ca49] 2nd.
  2 files changed, 2 insertions(+), 1 deletion(-)
  create mode 100644 b
 
 Now, I get the following:
 
 $ git log --oneline -B20%/80% -M20% --name-status
 a30ca49 2nd.
 M   a
 A   b
 b78205c Init.
 A   a
 
 But I would expect that git-log shows me a rename from a to b and a new a.

I think the problem is that your test file is so tiny that it falls
afoul of git's MINIMUM_BREAK_SIZE heuristic of 400 bytes (which prevents
false positives on tiny files). Try replacing your Lorem ipsum echo
with something like seq 1 150, and I think you will find the result
you are expecting.

-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


[PATCH] submodule update: when using recursion, show full path

2013-02-21 Thread Will Entriken
From d3fe2c76e6fa53e4cfa6f81600685c21bdadd4e3 Mon Sep 17 00:00:00 2001
From: William Entriken github@phor.net
Date: Thu, 21 Feb 2013 23:10:07 -0500
Subject: [PATCH] submodule update: when using recursion, show full path

Previously when using update with recursion, only the path for the
inner-most module was printed. Now the path is printed from GIT_DIR.
This now matches the behavior of submodule foreach
---

First patch. Several tests failed, but they were failing before I
started. This is against maint, I would consider this a (low priority)
bug.

How does it look? Please let me know next steps.

Signed-off-by: William Entriken github@phor.net

 git-submodule.sh | 31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2365149..f2c53c9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -588,7 +588,7 @@ cmd_update()
  die_if_unmatched $mode
  if test $stage = U
  then
- echo 2 Skipping unmerged submodule $sm_path
+ echo 2 Skipping unmerged submodule $prefix$sm_path
  continue
  fi
  name=$(module_name $sm_path) || exit
@@ -602,7 +602,7 @@ cmd_update()

  if test $update_module = none
  then
- echo Skipping submodule '$sm_path'
+ echo Skipping submodule '$prefix$sm_path'
  continue
  fi

@@ -611,7 +611,7 @@ cmd_update()
  # Only mention uninitialized submodules when its
  # path have been specified
  test $# != 0 
- say $(eval_gettext Submodule path '\$sm_path' not initialized
+ say $(eval_gettext Submodule path '\$prefix\$sm_path' not initialized
 Maybe you want to use 'update --init'?)
  continue
  fi
@@ -624,7 +624,7 @@ Maybe you want to use 'update --init'?)
  else
  subsha1=$(clear_local_git_env; cd $sm_path 
  git rev-parse --verify HEAD) ||
- die $(eval_gettext Unable to find current revision in submodule
path '\$sm_path')
+ die $(eval_gettext Unable to find current revision in submodule
path '\$prefix\$sm_path')
  fi

  if test $subsha1 != $sha1 -o -n $force
@@ -643,7 +643,7 @@ Maybe you want to use 'update --init'?)
  (clear_local_git_env; cd $sm_path 
  ( (rev=$(git rev-list -n 1 $sha1 --not --all 2/dev/null) 
  test -z $rev) || git-fetch)) ||
- die $(eval_gettext Unable to fetch in submodule path '\$sm_path')
+ die $(eval_gettext Unable to fetch in submodule path '\$prefix\$sm_path')
  fi

  # Is this something we just cloned?
@@ -657,20 +657,20 @@ Maybe you want to use 'update --init'?)
  case $update_module in
  rebase)
  command=git rebase
- die_msg=$(eval_gettext Unable to rebase '\$sha1' in submodule path
'\$sm_path')
- say_msg=$(eval_gettext Submodule path '\$sm_path': rebased into '\$sha1')
+ die_msg=$(eval_gettext Unable to rebase '\$sha1' in submodule path
'\$prefix\$sm_path')
+ say_msg=$(eval_gettext Submodule path '\$prefix\$sm_path': rebased
into '\$sha1')
  must_die_on_failure=yes
  ;;
  merge)
  command=git merge
- die_msg=$(eval_gettext Unable to merge '\$sha1' in submodule path
'\$sm_path')
- say_msg=$(eval_gettext Submodule path '\$sm_path': merged in '\$sha1')
+ die_msg=$(eval_gettext Unable to merge '\$sha1' in submodule path
'\$prefix\$sm_path')
+ say_msg=$(eval_gettext Submodule path '\$prefix\$sm_path': merged
in '\$sha1')
  must_die_on_failure=yes
  ;;
  *)
  command=git checkout $subforce -q
- die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule
path '\$sm_path')
- say_msg=$(eval_gettext Submodule path '\$sm_path': checked out '\$sha1')
+ die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule
path '\$prefix\$sm_path')
+ say_msg=$(eval_gettext Submodule path '\$prefix\$sm_path': checked
out '\$sha1')
  ;;
  esac

@@ -688,11 +688,16 @@ Maybe you want to use 'update --init'?)

  if test -n $recursive
  then
- (clear_local_git_env; cd $sm_path  eval cmd_update $orig_flags)
+ (
+ prefix=$prefix$sm_path/
+ clear_local_git_env
+ cd $sm_path 
+ eval cmd_update $orig_flags
+ )
  res=$?
  if test $res -gt 0
  then
- die_msg=$(eval_gettext Failed to recurse into submodule path '\$sm_path')
+ die_msg=$(eval_gettext Failed to recurse into submodule path
'\$prefix\$sm_path')
  if test $res -eq 1
  then
  err=${err};$die_msg
--
1.7.11.3
--
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] add: allow users to silence Git 2.0 warnings about add -u

2013-02-21 Thread David Aguilar
When git add -u is invoked from a subdirectory it prints a
loud warning message about an upcoming Git 2.0 behavior change.
Some users do not care to be warned.  Accomodate them.

The add.silence-pathless-warnings configuration variable can
now be used to silence this warning.

Signed-off-by: David Aguilar dav...@gmail.com
---
I found the warning a informative but also a little annoying.
I can imagine others might as well.

I would also like to change the warning message to mention what the Git 2.0
behavior will be (which it does not mention), but I realize that the string
has already been translated.  That can be a follow-on patch if this is seen as
a worthwhile change, but might not be worth the trouble since it's a problem
which will go away in 2.0.

 Documentation/config.txt |  7 +++
 builtin/add.c|  8 +++-
 t/t2200-add-update.sh| 11 +++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3bb53da..b6ed859 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -648,6 +648,13 @@ core.abbrev::
for abbreviated object names to stay unique for sufficiently long
time.
 
+add.silence-pathless-warnings::
+   Tells 'git add' to silence warnings when 'git add -u' is used in
+   a subdirectory without specifying a path.  Git 2.0 updates the
+   whole tree.  Git 1.x updates the current directory only, and warns
+   about the upcoming change unless this variable is set to true.
+   False by default, and ignored by Git 2.0.
+
 add.ignore-errors::
 add.ignoreErrors::
Tells 'git add' to continue adding files when some files cannot be
diff --git a/builtin/add.c b/builtin/add.c
index 0dd014e..01b9cac 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -272,6 +272,7 @@ N_(The following paths are ignored by one of your 
.gitignore files:\n);
 
 static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
 static int ignore_add_errors, addremove, intent_to_add, ignore_missing = 0;
+static int silence_pathless_warnings;
 
 static struct option builtin_add_options[] = {
OPT__DRY_RUN(show_only, N_(dry run)),
@@ -296,6 +297,8 @@ static int add_config(const char *var, const char *value, 
void *cb)
!strcmp(var, add.ignore-errors)) {
ignore_add_errors = git_config_bool(var, value);
return 0;
+   } else if (!strcmp(var, add.silence-pathless-warnings)) {
+   silence_pathless_warnings = git_config_bool(var, value);
}
return git_default_config(var, value, cb);
 }
@@ -321,7 +324,8 @@ static int add_files(struct dir_struct *dir, int flags)
return exit_status;
 }
 
-static void warn_pathless_add(const char *option_name, const char *short_name) 
{
+static void warn_pathless_add(const char *option_name, const char *short_name)
+{
/*
 * To be consistent with git add -p and most Git
 * commands, we should default to being tree-wide, but
@@ -332,6 +336,8 @@ static void warn_pathless_add(const char *option_name, 
const char *short_name) {
 * turned into a die(...), and eventually we may
 * reallow the command with a new behavior.
 */
+   if (silence_pathless_warnings)
+   return;
warning(_(The behavior of 'git add %s (or %s)' with no path argument 
from a\n
  subdirectory of the tree will change in Git 2.0 and should 
not be used anymore.\n
  To add content for the whole tree, run:\n
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 4cdebda..779dbe7 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -171,4 +171,15 @@ test_expect_success 'add -u non-existent should fail' '
! (git ls-files | grep non-existent)
 '
 
+test_expect_success 'add.silence-pathless-warnings configuration variable' '
+   : expect 
+   test_config add.silence-pathless-warnings true 
+   (
+   cd dir1 
+   echo more sub2 
+   git add -u
+   ) actual 21 
+   test_cmp expect actual
+'
+
 test_done
-- 
1.8.2.rc0.22.gb3600c3.dirty

--
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] add: allow users to silence Git 2.0 warnings about add -u

2013-02-21 Thread Junio C Hamano
David Aguilar dav...@gmail.com writes:

 When git add -u is invoked from a subdirectory it prints a
 loud warning message about an upcoming Git 2.0 behavior change.
 Some users do not care to be warned.  Accomodate them.

I do not think this is what we discussed to do.

It was very much deliberate to make the way to squelch the warning
not a set once and *forget*, aka configuration variable, but a
simple-to-type extra command line argument i.e. git add -u ., that
you will *always* type to train your fingers to explicitly say what
you mean, so that the default switch will not matter to existing
users.

--
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] add: allow users to silence Git 2.0 warnings about add -u

2013-02-21 Thread David Aguilar
On Thu, Feb 21, 2013 at 10:23 PM, Junio C Hamano gits...@pobox.com wrote:
 David Aguilar dav...@gmail.com writes:

 When git add -u is invoked from a subdirectory it prints a
 loud warning message about an upcoming Git 2.0 behavior change.
 Some users do not care to be warned.  Accomodate them.

 I do not think this is what we discussed to do.

 It was very much deliberate to make the way to squelch the warning
 not a set once and *forget*, aka configuration variable, but a
 simple-to-type extra command line argument i.e. git add -u ., that
 you will *always* type to train your fingers to explicitly say what
 you mean, so that the default switch will not matter to existing
 users.

In my use case:

- The user is always in a subdir; the behavior change
is immaterial to them.

- The user does not care about these details,
and is not harmed by using the short and sweet command.

Please enlighten me.
Are we really getting rid of it and replacing it with :/?
That syntax looks like a meh face.. just sayin'

I was actually surprised when add -u didn't do the whole tree
and am happy that 2.0 will make it do the right thing...

(and perhaps I am deluded, and am not aware of what 2.0 will
do when not given pathspecs.. is it really going to die()?
that's so mean! ;)

Sorry if I am missing most of the context.
I was reading this in builtin/add.c:

/*
 * To be consistent with git add -p and most Git
 * commands, we should default to being tree-wide, but
 * this is not the original behavior and can't be
 * changed until users trained themselves not to type
 * git add -u or git add -A. For now, we warn and
 * keep the old behavior. Later, this warning can be
 * turned into a die(...), and eventually we may
 * reallow the command with a new behavior.
 */

...and I was being too optimistic about, and eventually.

I misread that and thought it meant that (eventually) 2.0
would default to the full tree and fix the consistency.

I didn't think that meant 2.0 would die and git add -u
will not be a valid syntax anymore.

Why punish these users?  They are going to have :/ face.

Unlike push.default, whose warning can be silenced with configuration,
git 1.x does not have a way to silence this warning without retraining
existing users.

In other words I will have to answer an email about it one day, and I'm lazy ;-)


Another example...

$ git grep 'stuff' :/

would it be too much to teach it to do:

$ git grep -u 'stuff'

(some users are really simple...)

but in 2.0 that -u would be a no-op because grep will be full tree, no?

Would having that as an option and configuration be a way to allow 1.x
users to transition themselves to a 2.0 world?

I need to read the old discussions.
Can someone tell me the magic google search syntax they use to dig them up?


Would a better way be a method to make git add -u behave like 2.0 today?

I'm thinking of Python's from __future__ import better_behavior as a analog.

If full-tree is a better default then that should be the default.
Surely that's better than die(), no?

Apologies in advance as I have not read the discussions (yet).
-- 
David
--
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