Re: [PATCH v4] branch.c: change install_branch_config() to use skip_prefix()

[Eric Sunshine]

On Mon, Mar 3, 2014 at 1:36 AM, Guanglin Xu mzguang...@gmail.com wrote:
 to avoid a magic code of 11.

 Helped-by: Sun He sunheeh...@gmail.com
 Helped-by: Eric Sunshine sunsh...@sunshineco.com
 Helped-by: Jacopo Notarstefano jaco...@gmail.com

 Signed-off-by: Guanglin Xu mzguang...@gmail.com
 ---

 This is an implementation of the idea#2 of GSoC 2014 microproject.

  branch.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

 diff --git a/branch.c b/branch.c
 index 723a36b..4eec0ac 100644
 --- a/branch.c
 +++ b/branch.c
 @@ -49,8 +49,12 @@ static int should_setup_rebase(const char *origin)

  void install_branch_config(int flag, const char *local, const char *origin, 
 const char *remote)
  {
 -   const char *shortname = remote + 11;
 -   int remote_is_branch = starts_with(remote, refs/heads/);
 +   const char *shortname = skip_prefix(remote ,refs/heads/);
 +   int remote_is_branch;
 +   if (NULL == shortname)
 +   remote_is_branch = 0;
 +   else
 +   remote_is_branch = 1;

A bit verbose. Perhaps just:

int remote_is_branch = !!shortname;

which you will find to be idiomatic in this project.

 struct strbuf key = STRBUF_INIT;
 int rebasing = should_setup_rebase(origin);

 --
 1.9.0
--
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 00/11] Use ALLOC_GROW() instead of inline code

[Eric Sunshine]

On Mon, Mar 3, 2014 at 2:18 AM, Dmitry S. Dolzhenko
dmitrys.dolzhe...@yandex.ru wrote:
 Dmitry S. Dolzhenko (11):
   builtin/pack-objects.c: use ALLOC_GROW() in check_pbase_path()
   bundle.c: use ALLOC_GROW() in add_to_ref_list()
   cache-tree.c: use ALLOC_GROW() in find_subtree()
   commit.c: use ALLOC_GROW() in register_commit_graft()
   diff.c: use ALLOC_GROW()
   diffcore-rename.c: use ALLOC_GROW()
   patch-ids.c: use ALLOC_GROW() in add_commit()
   replace_object.c: use ALLOC_GROW() in register_replace_object()
   reflog-walk.c: use ALLOC_GROW()
   dir.c: use ALLOC_GROW() in create_simplify()
   attr.c: use ALLOC_GROW() in handle_attr_line()

  attr.c |  7 +--
  builtin/pack-objects.c |  9 +++--
  bundle.c   |  6 +-
  cache-tree.c   |  6 +-
  commit.c   |  8 ++--
  diff.c | 12 ++--
  diffcore-rename.c  | 12 ++--
  dir.c  |  5 +
  patch-ids.c|  5 +
  reflog-walk.c  | 12 ++--
  replace_object.c   |  8 ++--
  11 files changed, 18 insertions(+), 72 deletions(-)

 --
 1.8.5.3

 This version differs from previous only minor changes:
   - update commit messages
   - keep code lines within 80 columns

Place this commentary at the top of the cover letter since that's
where people look for it.

You want to ease the reviewer's job as much as possible, so it helps
to link to the previous submission, like this [1].

Likewise, you can help the reviewer by being more specific about how
you updated the commit messages (and perhaps by linking to the
relevant discussion points, like this [2][3]).

[1]: http://thread.gmane.org/gmane.comp.version-control.git/242857
[2]: http://article.gmane.org/gmane.comp.version-control.git/243004
[3]: http://article.gmane.org/gmane.comp.version-control.git/243049
--
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 12/27] *.sh: avoid hardcoding $GIT_DIR/hooks/...

[Eric Sunshine]

On Sat, Mar 1, 2014 at 7:12 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 If $GIT_COMMON_DIR is set, it should be $GIT_COMMON_DIR/hooks/, not
 $GIT_DIR/hooks/. Just let rev-parse --git-path handle it.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  git-am.sh  | 22 +++---
  git-rebase--interactive.sh |  6 +++---
  git-rebase--merge.sh   |  6 ++
  git-rebase.sh  |  4 ++--
  templates/hooks--applypatch-msg.sample |  4 ++--
  templates/hooks--pre-applypatch.sample |  4 ++--
  6 files changed, 22 insertions(+), 24 deletions(-)

 diff --git a/git-am.sh b/git-am.sh
 index bbea430..dfa0618 100755
 --- a/git-am.sh
 +++ b/git-am.sh
 @@ -803,10 +803,10 @@ To restore the original branch and stop patching run 
 \\$cmdline --abort\.
 continue
 fi

 -   if test -x $GIT_DIR/hooks/applypatch-msg
 +   hook=`git rev-parse --git-path hooks/applypatch-msg`

Did you want to use $(...) rather than `...`?

Same question for the remainder of the patch.

 +   if test -x $hook
 then
 -   $GIT_DIR/hooks/applypatch-msg $dotest/final-commit ||
 -   stop_here $this
 +   $hook $dotest/final-commit || stop_here $this
 fi

 if test -f $dotest/final-commit
 @@ -880,9 +880,10 @@ did you forget to use 'git add'?
 stop_here_user_resolve $this
 fi

 -   if test -x $GIT_DIR/hooks/pre-applypatch
 +   hook=`git rev-parse --git-path hooks/pre-applypatch`
 +   if test -x $hook
 then
 -   $GIT_DIR/hooks/pre-applypatch || stop_here $this
 +   $hook || stop_here $this
 fi

 tree=$(git write-tree) 
 @@ -908,18 +909,17 @@ did you forget to use 'git add'?
 echo $(cat $dotest/original-commit) $commit  
 $dotest/rewritten
 fi

 -   if test -x $GIT_DIR/hooks/post-applypatch
 -   then
 -   $GIT_DIR/hooks/post-applypatch
 -   fi
 +   hook=`git rev-parse --git-path hooks/post-applypatch`
 +   test -x $hook  $hook

 go_next
  done

  if test -s $dotest/rewritten; then
  git notes copy --for-rewrite=rebase  $dotest/rewritten
 -if test -x $GIT_DIR/hooks/post-rewrite; then
 -   $GIT_DIR/hooks/post-rewrite rebase  $dotest/rewritten
 +hook=`git rev-parse --git-path hooks/post-rewrite`
 +if test -x $hook; then
 +   $hook rebase  $dotest/rewritten
  fi
  fi

 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index 43c19e0..d741b04 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -632,9 +632,9 @@ do_next () {
 git notes copy --for-rewrite=rebase  $rewritten_list ||
 true # we don't care if this copying failed
 } 
 -   if test -x $GIT_DIR/hooks/post-rewrite 
 -   test -s $rewritten_list; then
 -   $GIT_DIR/hooks/post-rewrite rebase  $rewritten_list
 +   hook=`git rev-parse --git-path hooks/post-rewrite`
 +   if test -x $hook  test -s $rewritten_list; then
 +   $hook rebase  $rewritten_list
 true # we don't care if this hook failed
 fi 
 warn Successfully rebased and updated $head_name.
 diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
 index e7d96de..68f5d09 100644
 --- a/git-rebase--merge.sh
 +++ b/git-rebase--merge.sh
 @@ -93,10 +93,8 @@ finish_rb_merge () {
 if test -s $state_dir/rewritten
 then
 git notes copy --for-rewrite=rebase $state_dir/rewritten
 -   if test -x $GIT_DIR/hooks/post-rewrite
 -   then
 -   $GIT_DIR/hooks/post-rewrite rebase 
 $state_dir/rewritten
 -   fi
 +   hook=`git rev-parse --git-path hooks/post-rewrite`
 +   test -x $hook  $hook rebase $state_dir/rewritten
 fi
 say All done.
  }
 diff --git a/git-rebase.sh b/git-rebase.sh
 index 8a3efa2..1cf8dba 100755
 --- a/git-rebase.sh
 +++ b/git-rebase.sh
 @@ -195,9 +195,9 @@ run_specific_rebase () {

  run_pre_rebase_hook () {
 if test -z $ok_to_skip_pre_rebase 
 -  test -x $GIT_DIR/hooks/pre-rebase
 +  test -x `git rev-parse --git-path hooks/pre-rebase`
 then
 -   $GIT_DIR/hooks/pre-rebase ${1+$@} ||
 +   `git rev-parse --git-path hooks/pre-rebase` ${1+$@} ||
 die $(gettext The pre-rebase hook refused to rebase.)
 fi
  }
 diff --git a/templates/hooks--applypatch-msg.sample 
 b/templates/hooks--applypatch-msg.sample
 index 8b2a2fe..28b843b 100755
 --- a/templates/hooks--applypatch-msg.sample
 +++ b/templates/hooks--applypatch-msg.sample
 @@ -10,6 +10,6 @@
  # To enable this hook, rename this file to applypatch-msg.

  . git-sh-setup
 -test -x $GIT_DIR/hooks/commit-msg 
 -   exec $GIT_DIR/hooks/commit-msg ${1+$@}
 +commitmsg=`git 

Re: [PATCH] commit.c: Replace starts_with() with skip_prefix()

[Eric Sunshine]

Thanks for the submission. Some commentary below to expose you to the
review process on this project...

On Mon, Mar 3, 2014 at 2:47 AM, Karthik Nayak karthik@gmail.com wrote:
 Replace with skip_prefix(), which uses the inbuilt function
 strcmp() to compare.

Explaining the purpose of the change is indeed important, however,
this description misses the mark. See below.

 Other Places were this can be implemented:
 commit.c : line 1117
 commit.c : line 1197

Bonus points if you actually follow through with this.

 Signed-off-by: Karthik Nayak karthik@gmail.com
 ---
  commit.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/commit.c b/commit.c
 index 6bf4fe0..1e181cf 100644
 --- a/commit.c
 +++ b/commit.c
 @@ -552,6 +552,7 @@ static void record_author_date(struct author_date_slab 
 *author_date,
 char *buffer = NULL;
 struct ident_split ident;
 char *date_end;
 +   char *flag_sp; /* stores return value of skip_prefix() */

It's not clear why this variable is needed. It's only assigned (below)
but never referenced. Also, the name conveys little or no meaning. If
you need a comment to explain the purpose of the variable, that's
probably a good indication that it's poorly named.

 unsigned long date;

 if (!commit-buffer) {
 @@ -566,7 +567,7 @@ static void record_author_date(struct author_date_slab 
 *author_date,
  buf;
  buf = line_end + 1) {
 line_end = strchrnul(buf, '\n');
 -   if (!starts_with(buf, author )) {
 +   if ((flag_sp = skip_prefix(buf, author )) == NULL) {

Unfortunately, this change makes the code more difficult to read.
Let's review the GSoC microproject:

Rewrite commit.c:record_author_date() to use skip_prefix().
Are there other places in this file where skip_prefix() would
be **more readable** than starts_with()?

Note the part I **highlighted**. This is a good clue that use of
skip_prefix() can be used to simplify the code. Examine
record_author_date() more carefully and see if you can identify how to
do so.

 if (!line_end[0] || line_end[1] == '\n')
 return; /* end of header */
 continue;
 --
 1.9.0.138.g2de3478
--
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] branch.c: change install_branch_config() to use skip_prefix()

[Guanglin Xu]

Hi Eric,

Yes, you're right. !! is comfortably concise and also idiomatic in
Git sources.

Thanks,

Guanglin

2014-03-03 16:12 GMT+08:00 Eric Sunshine sunsh...@sunshineco.com:
 On Mon, Mar 3, 2014 at 1:36 AM, Guanglin Xu mzguang...@gmail.com wrote:
 to avoid a magic code of 11.

 Helped-by: Sun He sunheeh...@gmail.com
 Helped-by: Eric Sunshine sunsh...@sunshineco.com
 Helped-by: Jacopo Notarstefano jaco...@gmail.com

 Signed-off-by: Guanglin Xu mzguang...@gmail.com
 ---

 This is an implementation of the idea#2 of GSoC 2014 microproject.

  branch.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

 diff --git a/branch.c b/branch.c
 index 723a36b..4eec0ac 100644
 --- a/branch.c
 +++ b/branch.c
 @@ -49,8 +49,12 @@ static int should_setup_rebase(const char *origin)

  void install_branch_config(int flag, const char *local, const char *origin, 
 const char *remote)
  {
 -   const char *shortname = remote + 11;
 -   int remote_is_branch = starts_with(remote, refs/heads/);
 +   const char *shortname = skip_prefix(remote ,refs/heads/);
 +   int remote_is_branch;
 +   if (NULL == shortname)
 +   remote_is_branch = 0;
 +   else
 +   remote_is_branch = 1;

 A bit verbose. Perhaps just:

 int remote_is_branch = !!shortname;

 which you will find to be idiomatic in this project.

 struct strbuf key = STRBUF_INIT;
 int rebasing = should_setup_rebase(origin);

 --
 1.9.0
--
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] commit.c:record_author_date() use skip_prefix() instead of starts_with()

[Michael Haggerty]

The format of this email is wrong.  The non-commit-message notes should
come between the --- line (- note, there are three minus signs here)
and the patch itself.

On 03/01/2014 08:48 PM, Tanay Abhra wrote:
 Signed-off-by: Tanay Abhra tanay...@gmail.com
 ---
  commit.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/commit.c b/commit.c
 index 6bf4fe0..c954ecb 100644
 --- a/commit.c
 +++ b/commit.c
 @@ -566,7 +566,7 @@ static void record_author_date(struct author_date_slab 
 *author_date,
buf;
buf = line_end + 1) {
   line_end = strchrnul(buf, '\n');
 - if (!starts_with(buf, author )) {
 + if (!skip_prefix(buf, author )) {

If this is the only change, there is not much point, is there?  How does
this help?  Perhaps there is some way to take advantage of the
difference between starts_with() and skip_prefix() to simplify the rest
of the function?

   if (!line_end[0] || line_end[1] == '\n')
   return; /* end of header */
   continue;
 -- 
 1.7.9.5
 
 Hello,
 
 This is my patch for the GSoC microproject #10:
 
 Rewrite commit.c:record_author_date() to use skip_prefix(). 
 Are there other places in this file where skip_prefix() would be more 
 readable than starts_with()?
 
 Since skip_prefix() and starts_with() implement the same functionality with 
 different
 return values, they can be interchanged easily.
 
 Other usage of starts_with() in the same file can be found with
 
 $ grep -n starts_with commit.c
 
 1116: else if (starts_with(line, gpg_sig_header) 
 1196: if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {

The rhetorical question that was part of this microproject was meant to
inspire you to actually *FIX* the other spots, at least if the change
makes sense.

 I have a query,should I tackle a bug from the mailing lists or research about 
 the proposal 
 and present a rough draft?

My suggestion is that you follow up on this microproject until it is
perfect before worrying too much about the next step.

Michael

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

[PATCH v5] branch.c: change install_branch_config() to use skip_prefix()

[Guanglin Xu]

to avoid a magic code of 11.

Helped-by: Sun He sunheeh...@gmail.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Jacopo Notarstefano jaco...@gmail.com

Signed-off-by: Guanglin Xu mzguang...@gmail.com
---

This is an implementation of the idea#2 of GSoC 2014 microproject.

 branch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/branch.c b/branch.c
index 723a36b..fae7715 100644
--- a/branch.c
+++ b/branch.c
@@ -49,8 +49,8 @@ static int should_setup_rebase(const char *origin)
 
 void install_branch_config(int flag, const char *local, const char *origin, 
const char *remote)
 {
-   const char *shortname = remote + 11;
-   int remote_is_branch = starts_with(remote, refs/heads/);
+   const char *shortname = skip_prefix(remote ,refs/heads/);
+   int remote_is_branch = !!shortname;
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
 
-- 
1.9.0

--
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/3] rebase: accept -number as another way of saying HEAD~number

[Michael Haggerty]

On 03/02/2014 04:55 PM, Matthieu Moy wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:
 
 On Sun, Mar 2, 2014 at 3:53 AM, Eric Sunshine sunsh...@sunshineco.com 
 wrote:
 On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com 
 wrote:
 This is rev-list style, where people can do git rev-list -3 in
 addition to git rev-list HEAD~3. A lot of commands are driven by the
 revision machinery and also accept this form. This addition to rebase
 is just for convenience.

 I'm seeing some pretty strange results with this. If I use -1, -2, or
 -3 then it rebases the expected commits, however, -4 gives me 9
 commits, and -5 rebases 35 commits. Am I misunderstanding how this
 works?

 Nevermind. I wasn't paying attention to the fact that I was attempting
 to rebase merges.
 
 Your remark is actually interesting. Most (all?) Git commands taking
 -n as parameters act on n commits, regardless of merges.
 
 So, this commit creates an inconsistency between e.g. git log -3 (show
 last 3 commits) and git rebase -3 (rebase up to HEAD~3, but there may
 be more commits in case there are merges).
 
 I don't have a better proposal, but at least the inconsistancy should be
 documented (e.g. note that this is different from what other commands
 like 'git log' do when used with a -number option since ... in the
 manpage).

This might be a reason that -NUM is a bad idea.

Or perhaps -NUM should fail with an error message if any of the last
NUM commits are merges.  In that restricted scenario (which probably
accounts for 99% of rebases), -NUM is equivalent to HEAD~NUM.

Michael

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

[PATCH v3] finish_tmp_packfile():use strbuf for pathname construction

[Sun He]

The old version fixes a maximum length on the buffer, which could be a problem
if one is not certain of the length of get_object_directory().
Using strbuf can avoid the protential bug.

Helped-by: Michael Haggerty mhag...@alum.mit.edu
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sun He sunheeh...@gmail.com
---

PATCH v3 adds the reason why we should apply this patch.
 Thanks to Micheal Haggerty.
PATCH v3 transposes the space and comma before the third argument as 
Eric Sunshine suggested to meet the style of existing code.
 Thanks to Eric Sunshine.
PATCH v3 fixes the order of Helped-by and Signed-off-by.

PATCH v2 follows the suggestions of Eric Sunshine to use strbuf_setlen() 
instead of strbuf_remove(), etc.
 Thanks to Eric Sunshine.

This patch has assumed that you have already fix the bug of
tmpname in builtin/pack-objects.c:write_pack_file() warning()


 builtin/pack-objects.c | 15 ++-
 bulk-checkin.c |  8 +---
 pack-write.c   | 18 ++
 pack.h |  2 +-
 4 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..099d6ed 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -803,7 +803,7 @@ static void write_pack_file(void)
 
if (!pack_to_stdout) {
struct stat st;
-   char tmpname[PATH_MAX];
+   struct strbuf tmpname = STRBUF_INIT;
 
/*
 * Packs are runtime accessed in their mtime
@@ -826,23 +826,19 @@ static void write_pack_file(void)
tmpname, strerror(errno));
}
 
-   /* Enough space for -sha-1.pack? */
-   if (sizeof(tmpname) = strlen(base_name) + 50)
-   die(pack base name '%s' too long, base_name);
-   snprintf(tmpname, sizeof(tmpname), %s-, base_name);
+   strbuf_addf(tmpname, %s-, base_name);
 
if (write_bitmap_index) {
bitmap_writer_set_checksum(sha1);
bitmap_writer_build_type_index(written_list, 
nr_written);
}
 
-   finish_tmp_packfile(tmpname, pack_tmp_name,
+   finish_tmp_packfile(tmpname, pack_tmp_name,
written_list, nr_written,
pack_idx_opts, sha1);
 
if (write_bitmap_index) {
-   char *end_of_name_prefix = strrchr(tmpname, 0);
-   sprintf(end_of_name_prefix, %s.bitmap, 
sha1_to_hex(sha1));
+   strbuf_addf(tmpname, %s.bitmap, 
sha1_to_hex(sha1));
 
stop_progress(progress_state);
 
@@ -851,10 +847,11 @@ static void write_pack_file(void)
bitmap_writer_select_commits(indexed_commits, 
indexed_commits_nr, -1);
bitmap_writer_build(to_pack);
bitmap_writer_finish(written_list, nr_written,
-tmpname, 
write_bitmap_options);
+tmpname.buf, 
write_bitmap_options);
write_bitmap_index = 0;
}
 
+   strbuf_release(tmpname);
free(pack_tmp_name);
puts(sha1_to_hex(sha1));
}
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 118c625..98e651c 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -4,6 +4,7 @@
 #include bulk-checkin.h
 #include csum-file.h
 #include pack.h
+#include strbuf.h
 
 static int pack_compression_level = Z_DEFAULT_COMPRESSION;
 
@@ -23,7 +24,7 @@ static struct bulk_checkin_state {
 static void finish_bulk_checkin(struct bulk_checkin_state *state)
 {
unsigned char sha1[20];
-   char packname[PATH_MAX];
+   struct strbuf packname = STRBUF_INIT;
int i;
 
if (!state-f)
@@ -43,8 +44,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
close(fd);
}
 
-   sprintf(packname, %s/pack/pack-, get_object_directory());
-   finish_tmp_packfile(packname, state-pack_tmp_name,
+   strbuf_addf(packname, %s/pack/pack-, get_object_directory());
+   finish_tmp_packfile(packname, state-pack_tmp_name,
state-written, state-nr_written,
state-pack_idx_opts, sha1);
for (i = 0; i  state-nr_written; i++)
@@ -54,6 +55,7 @@ clear_exit:
free(state-written);
memset(state, 0, sizeof(*state));
 
+   strbuf_release(packname);
/* Make objects we just wrote available to 

Re: [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction

[He Sun]

2014-03-03 15:41 GMT+08:00 Eric Sunshine sunsh...@sunshineco.com:
 On Sat, Mar 1, 2014 at 9:29 PM, Sun He sunheeh...@gmail.com wrote:
 Signed-off-by: Sun He sunheeh...@gmail.com
 Helped-by: Eric Sunshine sunsh...@sunshineco.com
 Helped-by: Michael Haggerty mhag...@alum.mit.edu
 ---

 This patch has assumed that you have already fix the bug of
 tmpname in builtin/pack-objects.c:write_pack_file() warning()


  builtin/pack-objects.c | 15 ++-
  bulk-checkin.c |  8 +---
  pack-write.c   | 18 ++
  pack.h |  2 +-
  4 files changed, 22 insertions(+), 21 deletions(-)

 diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
 index c733379..099d6ed 100644
 --- a/builtin/pack-objects.c
 +++ b/builtin/pack-objects.c
 @@ -803,7 +803,7 @@ static void write_pack_file(void)

 if (!pack_to_stdout) {
 struct stat st;
 -   char tmpname[PATH_MAX];
 +   struct strbuf tmpname = STRBUF_INIT;

 /*
  * Packs are runtime accessed in their mtime
 @@ -826,23 +826,19 @@ static void write_pack_file(void)
 tmpname, strerror(errno));
 }

 -   /* Enough space for -sha-1.pack? */
 -   if (sizeof(tmpname) = strlen(base_name) + 50)
 -   die(pack base name '%s' too long, 
 base_name);
 -   snprintf(tmpname, sizeof(tmpname), %s-, base_name);
 +   strbuf_addf(tmpname, %s-, base_name);

 if (write_bitmap_index) {
 bitmap_writer_set_checksum(sha1);
 bitmap_writer_build_type_index(written_list, 
 nr_written);
 }

 -   finish_tmp_packfile(tmpname, pack_tmp_name,
 +   finish_tmp_packfile(tmpname, pack_tmp_name,
 written_list, nr_written,
 pack_idx_opts, sha1);

 if (write_bitmap_index) {
 -   char *end_of_name_prefix = strrchr(tmpname, 
 0);
 -   sprintf(end_of_name_prefix, %s.bitmap, 
 sha1_to_hex(sha1));
 +   strbuf_addf(tmpname, %s.bitmap 
 ,sha1_to_hex(sha1));

 Transpose the space and comma before the third argument.

 Other than this, the patch looks reasonable.


OK, got it.
I have already fixed it.
Thank you very much

 stop_progress(progress_state);

 @@ -851,10 +847,11 @@ static void write_pack_file(void)
 
 bitmap_writer_select_commits(indexed_commits, indexed_commits_nr, -1);
 bitmap_writer_build(to_pack);
 bitmap_writer_finish(written_list, 
 nr_written,
 -tmpname, 
 write_bitmap_options);
 +tmpname.buf, 
 write_bitmap_options);
 write_bitmap_index = 0;
 }

 +   strbuf_release(tmpname);
 free(pack_tmp_name);
 puts(sha1_to_hex(sha1));
 }
 diff --git a/bulk-checkin.c b/bulk-checkin.c
 index 118c625..98e651c 100644
 --- a/bulk-checkin.c
 +++ b/bulk-checkin.c
 @@ -4,6 +4,7 @@
  #include bulk-checkin.h
  #include csum-file.h
  #include pack.h
 +#include strbuf.h

  static int pack_compression_level = Z_DEFAULT_COMPRESSION;

 @@ -23,7 +24,7 @@ static struct bulk_checkin_state {
  static void finish_bulk_checkin(struct bulk_checkin_state *state)
  {
 unsigned char sha1[20];
 -   char packname[PATH_MAX];
 +   struct strbuf packname = STRBUF_INIT;
 int i;

 if (!state-f)
 @@ -43,8 +44,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
 *state)
 close(fd);
 }

 -   sprintf(packname, %s/pack/pack-, get_object_directory());
 -   finish_tmp_packfile(packname, state-pack_tmp_name,
 +   strbuf_addf(packname, %s/pack/pack-, get_object_directory());
 +   finish_tmp_packfile(packname, state-pack_tmp_name,
 state-written, state-nr_written,
 state-pack_idx_opts, sha1);
 for (i = 0; i  state-nr_written; i++)
 @@ -54,6 +55,7 @@ clear_exit:
 free(state-written);
 memset(state, 0, sizeof(*state));

 +   strbuf_release(packname);
 /* Make objects we just wrote available to ourselves */
 reprepare_packed_git();
  }
 diff --git a/pack-write.c b/pack-write.c
 index 9b8308b..9ccf804 100644
 --- a/pack-write.c
 +++ b/pack-write.c
 @@ -336,7 +336,7 @@ struct sha1file *create_tmp_packfile(char 
 

Re: [PATCH 2/3] rebase: accept -number as another way of saying HEAD~number

[Matthieu Moy]

Michael Haggerty mhag...@alum.mit.edu writes:

 Or perhaps -NUM should fail with an error message if any of the last
 NUM commits are merges.  In that restricted scenario (which probably
 accounts for 99% of rebases), -NUM is equivalent to HEAD~NUM.

Makes sense to me. So, -NUM would actually mean rebase the last NUM
commits (as well as being an alias for HEAD~NUM), but would fail when
it does not make sense (with an error message explaining the situation
and pointing the user to HEAD~N if this is what he wanted).

This would actually be a feature for me: I often want to rebase recent
enough history, and when my @{upstream} isn't well positionned, I
randomly type HEAD~N without remembering what N should be. When N is too
small, the rebase doesn't reach the interesting commit, and when N is
too big, it reaches a merge commit and I get a bunch of commits I'm not
allowed to edit in my todo-list. Then I have to abort the commit
manually. With -N failing on merge commits, the rebase would abort
itself automatically.

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

[PATCH v3] Replace memcpy with hashcpy when dealing hash copy globally

[Sun He]

Replacing memcpy with hashcpy is more directly and elegant.

Leave ppc/sha1.c alone, as it is an isolated component.
Pull cache.h(actually ../cache.h) in just for one memcpy
there is not proper.

Helped-by: Michael Haggerty mhag...@alum.mit.edu
Helped-by: Duy Nguyen pclo...@gmail.com
Signed-off-by: Sun He sunheeh...@gmail.com
---

PATCH v3 delete the one-space indentation on each line of commit message
as is suggested by Eric Sunshine.
 Thanks to Eric Sunshine.

PATCH v2 leave ppc/sha1.c alone.

The general rule is if cache.h or git-compat-util.h is included,
it is the first #include, and system includes will be always in
git-compat-tuil.h.
via Duy Nguyen

The change in PATCH v1 is not proper because I placed cache.h
in the end.
And adding it to the head is not a good way to achieve the goal,
as is said above ---.
 Thanks to Duy Nguyen.

Find the potential places with memcpy by the bash command:
 $ find . | xargs grep memcpy.*\(.*20.*\)

ppc/sha1.c doesn't include cache.h and it cannot use hashcpy().
So just leave memcpy(in ppc/sha1.c) alone.

 bundle.c| 2 +-
 grep.c  | 2 +-
 pack-bitmap-write.c | 2 +-
 reflog-walk.c   | 4 ++--
 refs.c  | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/bundle.c b/bundle.c
index e99065c..7809fbb 100644
--- a/bundle.c
+++ b/bundle.c
@@ -19,7 +19,7 @@ static void add_to_ref_list(const unsigned char *sha1, const 
char *name,
list-list = xrealloc(list-list,
list-alloc * sizeof(list-list[0]));
}
-   memcpy(list-list[list-nr].sha1, sha1, 20);
+   hashcpy(list-list[list-nr].sha1, sha1);
list-list[list-nr].name = xstrdup(name);
list-nr++;
 }
diff --git a/grep.c b/grep.c
index c668034..f5101f7 100644
--- a/grep.c
+++ b/grep.c
@@ -1650,7 +1650,7 @@ void grep_source_init(struct grep_source *gs, enum 
grep_source_type type,
break;
case GREP_SOURCE_SHA1:
gs-identifier = xmalloc(20);
-   memcpy(gs-identifier, identifier, 20);
+   hashcpy(gs-identifier, identifier);
break;
case GREP_SOURCE_BUF:
gs-identifier = NULL;
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 1218bef..5f1791a 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -530,7 +530,7 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
header.version = htons(default_version);
header.options = htons(flags | options);
header.entry_count = htonl(writer.selected_nr);
-   memcpy(header.checksum, writer.pack_checksum, 20);
+   hashcpy(header.checksum, writer.pack_checksum);
 
sha1write(f, header, sizeof(header));
dump_bitmap(f, writer.commits);
diff --git a/reflog-walk.c b/reflog-walk.c
index b2fbdb2..d490f7d 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -32,8 +32,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned 
char *nsha1,
sizeof(struct reflog_info));
}
item = array-items + array-nr;
-   memcpy(item-osha1, osha1, 20);
-   memcpy(item-nsha1, nsha1, 20);
+   hashcpy(item-osha1, osha1);
+   hashcpy(item-nsha1, nsha1);
item-email = xstrdup(email);
item-timestamp = timestamp;
item-tz = tz;
diff --git a/refs.c b/refs.c
index 89228e2..f90b7ea 100644
--- a/refs.c
+++ b/refs.c
@@ -1222,7 +1222,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache 
*refs,
if (ref == NULL)
return -1;
 
-   memcpy(sha1, ref-u.value.sha1, 20);
+   hashcpy(sha1, ref-u.value.sha1);
return 0;
 }
 
-- 
1.9.0.138.g2de3478.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: Branch Name Case Sensitivity

[Karsten Blees]

Am 01.03.2014 07:54, schrieb Torsten Bögershausen:
 On 2014-03-01 03.42, Lee Hopkins wrote:
 +
 +if(ignore_case)
 Only looking at ignore_case here closes the door for people
 who have a branch foo and Foo at the same time.
 (Which means that they are carefully running git pack-refs)
 How about something like this:
  +if (refs_ignore_case  0)
  +  refs_ignore_case = ignore_case;
  +if (refs_ignore_case)

I don't think this distinction is necessary, either you have a case-insensitive 
file system or you don't. The case that the .git directory is case-sensitive 
and the worktree directory isn't (or the other way around) is probably so 
exotic that we can ignore it.

 (And then we need the diff further down on top of this.)
 (And of course Documentation/config.txt)
 The main motivation is that you can set refs.ignorecase == true on
 e.g. Linux, to prevent to have branches Foo and foo at the same time,
 which gives problems when pulling into e.g. Windows/Mac OS

If you want to prevent problems with Windows/Mac OS, you should set 
core.ignorecase = true. I don't see why we need yet another config setting for 
refs (and logs?).

--
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/3] rebase: accept -number as another way of saying HEAD~number

[Duy Nguyen]

On Mon, Mar 3, 2014 at 4:37 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:

 Or perhaps -NUM should fail with an error message if any of the last
 NUM commits are merges.  In that restricted scenario (which probably
 accounts for 99% of rebases), -NUM is equivalent to HEAD~NUM.

 Makes sense to me. So, -NUM would actually mean rebase the last NUM
 commits (as well as being an alias for HEAD~NUM), but would fail when
 it does not make sense (with an error message explaining the situation
 and pointing the user to HEAD~N if this is what he wanted).

Agreed, but..

 This would actually be a feature for me: I often want to rebase recent
 enough history, and when my @{upstream} isn't well positionned, I
 randomly type HEAD~N without remembering what N should be. When N is too
 small, the rebase doesn't reach the interesting commit, and when N is
 too big, it reaches a merge commit and I get a bunch of commits I'm not
 allowed to edit in my todo-list. Then I have to abort the commit
 manually. With -N failing on merge commits, the rebase would abort
 itself automatically.

would git rebase -i --fork-point be what you need instead? It's a
new thing, but may be what we actually should use, not this -NUM..
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Michael Haggerty]

On 03/02/2014 10:09 AM, Eric Sunshine wrote:
 On Sun, Mar 2, 2014 at 4:04 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com 
 wrote:
 git rebase -e XYZ is basically the same as

 EDITOR=sed -i '1s/pick XYZ/edit XYZ/' $@ \
 git rebase -i XYZ^

 In English, it prepares the todo list for you to edit only commit XYZ
 to save your time. The time saving is only significant when you edit a
 lot of commits separately.

 Should this accept multiple -e arguments? Based upon the above
 justification, it sounds like it should, and I think that would be the
 intuitive expectation (at least for me).

 The current implementation, however, is broken with multiple -e arguments. 
 With:

 git rebase -i -e older -e newer

 'newer' is ignored entirely. However, with:

 git rebase -i -e newer -e older

 it errors out when rewriting the todo list:

 expected to find 'edit older' line but did not

 An implementation supporting multiple -e arguments would need to
 ensure that the todo list extends to the oldest rev specified by any
 -e argument.
 
 Of course, I'm misreading and abusing the intention of -e as if it is
 -e arg.

I think that your misreading is more consistent than the feature as
implemented.

git rebase -e OLDER

does not mean do 'git rebase -i OLDER' and oh, by the way, also set up
commit OLDER to be edited.  It means do 'git rebase -i OLDER^' ...
(note: OLDER^ and not OLDER).  So it is confusing to think as -e
as a modifier on an otherwise normal git rebase -i invocation.
Rather, it seems to me that -e and -i should be mutually exclusive
(and consider it an implementation detail that the former is implemented
using the latter).

And if that is our point of view, then is perfectly logical to allow it
to be specified multiple times.  OTOH there is no reason that v1 has to
allow multiple -e, as long as it properly rejects that usage.

Michael

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

Re: [PATCH 2/3] rebase: accept -number as another way of saying HEAD~number

[Matthieu Moy]

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

 On Mon, Mar 3, 2014 at 4:37 PM, Matthieu Moy

 This would actually be a feature for me: I often want to rebase recent
 enough history, and when my @{upstream} isn't well positionned, I
 randomly type HEAD~N without remembering what N should be. When N is too
 small, the rebase doesn't reach the interesting commit, and when N is
 too big, it reaches a merge commit and I get a bunch of commits I'm not
 allowed to edit in my todo-list. Then I have to abort the commit
 manually. With -N failing on merge commits, the rebase would abort
 itself automatically.

 would git rebase -i --fork-point be what you need instead?

I don't think so. My use case is when I did a manual merge, so
@{upstream} is helpless or even not positionned. There is for sure an
accurate command (remember the branch I just merged and put it on the
command-line), but my fingers prefer typing quick and dirty commands and
hope I won't get too many trial and error cycles ;-).

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

Re: [PATCH 2/3] rebase: accept -number as another way of saying HEAD~number

[Jeff King]

On Mon, Mar 03, 2014 at 10:37:11AM +0100, Matthieu Moy wrote:

 Michael Haggerty mhag...@alum.mit.edu writes:
 
  Or perhaps -NUM should fail with an error message if any of the last
  NUM commits are merges.  In that restricted scenario (which probably
  accounts for 99% of rebases), -NUM is equivalent to HEAD~NUM.
 
 Makes sense to me. So, -NUM would actually mean rebase the last NUM
 commits (as well as being an alias for HEAD~NUM), but would fail when
 it does not make sense (with an error message explaining the situation
 and pointing the user to HEAD~N if this is what he wanted).

Yeah, I agree with this. I think -NUM is only useful for linear
string-of-pearls history. With merges, it either means:

  - linearize history (a la git-log), go back NUM commits, and treat the
result as the upstream. This is useless, because it's difficult to
predict where you're going to end up. Using explicit ~ and ^
relative-commit operators to specify the upstream is more flexible
if you really want to do this (but I don't know why you would).

  - do not use an UNINTERESTING upstream as the cutoff, but instead
try to rebase exactly NUM commits. In the face of non-linear
history, I'm not even sure what this would mean, or how we would
implement it.

Neither of those is useful, so I don't think erroring out on them is a
bad thing. And by erroring out (rather than documenting some weird and
useless behavior), we don't have to worry about backwards compatibility
if we later _do_ come up with a useful behavior (the best I can think of
is that --first-parent -3 might have a use, but I think that should
already be covered, as the results of --first-parent are by-definition
linear).

 This would actually be a feature for me: I often want to rebase recent
 enough history, and when my @{upstream} isn't well positionned, I
 randomly type HEAD~N without remembering what N should be. When N is too
 small, the rebase doesn't reach the interesting commit, and when N is
 too big, it reaches a merge commit and I get a bunch of commits I'm not
 allowed to edit in my todo-list. Then I have to abort the commit
 manually. With -N failing on merge commits, the rebase would abort
 itself automatically.

I do the same thing.

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

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Duy Nguyen]

On Mon, Mar 3, 2014 at 5:10 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 03/02/2014 10:09 AM, Eric Sunshine wrote:
 On Sun, Mar 2, 2014 at 4:04 AM, Eric Sunshine sunsh...@sunshineco.com 
 wrote:
 On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com 
 wrote:
 git rebase -e XYZ is basically the same as

 EDITOR=sed -i '1s/pick XYZ/edit XYZ/' $@ \
 git rebase -i XYZ^

 In English, it prepares the todo list for you to edit only commit XYZ
 to save your time. The time saving is only significant when you edit a
 lot of commits separately.

 Should this accept multiple -e arguments? Based upon the above
 justification, it sounds like it should, and I think that would be the
 intuitive expectation (at least for me).

 The current implementation, however, is broken with multiple -e arguments. 
 With:

 git rebase -i -e older -e newer

 'newer' is ignored entirely. However, with:

 git rebase -i -e newer -e older

 it errors out when rewriting the todo list:

 expected to find 'edit older' line but did not

 An implementation supporting multiple -e arguments would need to
 ensure that the todo list extends to the oldest rev specified by any
 -e argument.

 Of course, I'm misreading and abusing the intention of -e as if it is
 -e arg.

 I think that your misreading is more consistent than the feature as
 implemented.

 git rebase -e OLDER

 does not mean do 'git rebase -i OLDER' and oh, by the way, also set up
 commit OLDER to be edited.  It means do 'git rebase -i OLDER^' ...
 (note: OLDER^ and not OLDER).  So it is confusing to think as -e
 as a modifier on an otherwise normal git rebase -i invocation.
 Rather, it seems to me that -e and -i should be mutually exclusive
 (and consider it an implementation detail that the former is implemented
 using the latter).

 And if that is our point of view, then is perfectly logical to allow it
 to be specified multiple times.

Logically, yes. Practically, no. If you have to put multiple -e and
some hashes in one line, wouldn't editing to-do list in your favorite
editor be faster?

 OTOH there is no reason that v1 has to
 allow multiple -e, as long as it properly rejects that usage.
-- 
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 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so

[Ilya Bobyr]

We used to show (missing ) next to tests skipped because they are
specified in GIT_SKIP_TESTS.  Use (matched by GIT_SKIP_TESTS) instead.
---
 t/test-lib.sh |   13 -
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..89a405b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -446,25 +446,28 @@ test_finish_ () {
 
 test_skip () {
to_skip=
+   skipped_reason=
if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
then
to_skip=t
+   skipped_reason=matched GIT_SKIP_TESTS
fi
if test -z $to_skip  test -n $test_prereq 
   ! test_have_prereq $test_prereq
then
to_skip=t
-   fi
-   case $to_skip in
-   t)
+
of_prereq=
if test $missing_prereq != $test_prereq
then
of_prereq= of $test_prereq
fi
-
+   skipped_reason=missing $missing_prereq${of_prereq}
+   fi
+   case $to_skip in
+   t)
say_color skip 3 skipping test: $@
-   say_color skip ok $test_count # skip $1 (missing 
$missing_prereq${of_prereq})
+   say_color skip ok $test_count # skip $1 ($skipped_reason)
: true
;;
*)
-- 
1.7.9

--
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] test-lib: GIT_TEST_ONLY to run only specific tests

[Ilya Bobyr]

This is a counterpart to GIT_SKIP_TESTS.  Mostly useful when debugging.
---
 t/README  |   15 +++
 t/test-lib.sh |8 
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/t/README b/t/README
index caeeb9d..f939987 100644
--- a/t/README
+++ b/t/README
@@ -187,6 +187,21 @@ and either can match the t[0-9]{4} part to skip the whole
 test, or t[0-9]{4} followed by .$number to say which
 particular test to skip.
 
+Sometimes the opposite is desired - ability to execute only one or
+several tests.  Mostly while debugging tests.  For that you can say
+
+$ GIT_TEST_ONLY=t9200.8 sh ./t9200-git-cvsexport-commit.sh
+
+or, similrary to GIT_SKIP_TESTS
+
+$ GIT_TEST_ONLY='t[0-4]??? t91?? t9200.8' make
+
+In additiona to matching against test suite number.test number
+GIT_TEST_ONLY is matched against just the test numbes.  This comes
+handy when you are running only one test:
+
+$ GIT_TEST_ONLY='[0-8]' sh ./t9200-git-cvsexport-commit.sh
+
 Note that some tests in the existing test suite rely on previous
 test item, so you cannot arbitrarily disable one and expect the
 remainder of test to check what the test originally was intended
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 89a405b..12bf436 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -464,6 +464,14 @@ test_skip () {
fi
skipped_reason=missing $missing_prereq${of_prereq}
fi
+   if test -z $to_skip  test -n $GIT_TEST_ONLY 
+   ! match_pattern_list $this_test.$test_count $GIT_TEST_ONLY 
+   ! match_pattern_list $test_count $GIT_TEST_ONLY
+   then
+   to_skip=t
+   skipped_reason=not in GIT_TEST_ONLY
+   fi
+
case $to_skip in
t)
say_color skip 3 skipping test: $@
-- 
1.7.9

--
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] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

Built-in commands can specify names for option arguments, that are shown
when usage text is generated for the command.  sh based commands should
be able to do the same.

Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace.  Underscores are replaced with
whitespace.  It is unlikely that an underscore would be useful in the
hint text.

Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
---
 Documentation/git-rev-parse.txt |   11 +--
 builtin/rev-parse.c |   17 -
 t/t1502-rev-parse-parseopt.sh   |   20 
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..4cb6e02 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
 
 'git rev-parse --parseopt' input format is fully text based. It has two parts,
 separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(could be more than one) are used for the usage.
 The lines after the separator describe the options.
 
 Each line of options has this format:
 
 
-opt_specflags* SP+ help LF
+opt_specflags*argh? SP+ help LF
 
 
 `opt_spec`::
@@ -313,6 +313,12 @@ Each line of options has this format:
 
* Use `!` to not make the corresponding negated long option available.
 
+`argh`::
+   `argh`, if specified, is used as a name of the argument, if the
+   option takes an argument. `argh` is terminated by the first
+   whitespace. Angle braces are added automatically.  Underscore symbols
+   are replaced with spaces.
+
 The remainder of the line, after stripping the spaces, is used
 as the help associated to the option.
 
@@ -333,6 +339,7 @@ h,helpshow the help
 
 foo   some nifty option --foo
 bar=  some cool option --bar with an argument
+baz=arg   another cool option --baz with an argument named arg
 
   An option group Header
 C?option C with an optional argument
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index aaeb611..83a769e 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
usage[unb++] = strbuf_detach(sb, NULL);
}
 
-   /* parse: (short|short,long|long)[=?]? SP+ help */
+   /* parse: (short|short,long|long)[*=?!]*arghint? SP+ help */
while (strbuf_getline(sb, stdin, '\n') != EOF) {
const char *s;
+   const char *argh;
struct option *o;
 
if (!sb.len)
@@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
o-value = parsed;
o-flags = PARSE_OPT_NOARG;
o-callback = parseopt_dump;
+
+   /* Possible argument name hint */
+   argh = s;
+   while (s  sb.buf  strchr(*=?!, s[-1]) == NULL)
+   --s;
+   if (s != sb.buf  s != argh) {
+   char *a;
+   o-argh = a = xmemdupz(s, argh - s);
+   while (a = strchr(a, '_'))
+   *a = ' ';
+   }
+   if (s == sb.buf)
+   s = argh;
+
while (s  sb.buf  strchr(*=?!, s[-1])) {
switch (*--s) {
case '=':
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 83b1300..bf0db05 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -18,6 +18,17 @@ An option group Header
 -C[...]   option C with an optional argument
 -d, --data[=...]  short and long option with an optional argument
 
+Argument hints
+-b arg  short option required argument
+--bar2 arg  long option required argument
+-e, --fuz with spaces
+  short and long option required argument
+-s[some]short option optional argument
+--long[=data]   long option optional argument
+-g, --fluf[=path]   short and long option optional argument
+--longest a very long argument hint
+  a very long argument hint
+
 Extras
 --extra1  line above used to cause a segfault but no longer 
does
 
@@ -39,6 +50,15 @@ b,baz a short and long option
 C?option C with an optional argument
 d,data?   short and long option with an optional argument
 
+ Argument hints
+b=arg short option required argument
+bar2=arg  long option required argument
+e,fuz=with_spaces  short and long option required argument
+s?someshort option optional argument
+long?data long option optional argument
+g,fluf?path short and long option optional argument

Re: [PATCH 2/3] rebase: accept -number as another way of saying HEAD~number

[David Kastrup]

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

 On Mon, Mar 3, 2014 at 4:37 PM, Matthieu Moy
 matthieu@grenoble-inp.fr wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:

 Or perhaps -NUM should fail with an error message if any of the last
 NUM commits are merges.  In that restricted scenario (which probably
 accounts for 99% of rebases), -NUM is equivalent to HEAD~NUM.

 Makes sense to me. So, -NUM would actually mean rebase the last NUM
 commits (as well as being an alias for HEAD~NUM), but would fail when
 it does not make sense (with an error message explaining the situation
 and pointing the user to HEAD~N if this is what he wanted).

 Agreed, but..

 This would actually be a feature for me: I often want to rebase recent
 enough history, and when my @{upstream} isn't well positionned, I
 randomly type HEAD~N without remembering what N should be. When N is too
 small, the rebase doesn't reach the interesting commit, and when N is
 too big, it reaches a merge commit and I get a bunch of commits I'm not
 allowed to edit in my todo-list. Then I have to abort the commit
 manually. With -N failing on merge commits, the rebase would abort
 itself automatically.

 would git rebase -i --fork-point be what you need instead? It's a
 new thing, but may be what we actually should use, not this -NUM..

-0 might be a good mnemonic for --fork-point, though.

Of course, when using --preserve-merges explicitly it would appear that
-NUM should not error out.

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

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[David Kastrup]

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

 Logically, yes. Practically, no. If you have to put multiple -e and
 some hashes in one line, wouldn't editing to-do list in your favorite
 editor be faster?

An editor is the last resort when the card puncher is broken.

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

My advice for GSoC applicants

[Michael Haggerty]

Hi,

Based on my experience so far as a first-time Google Summer of Code
mentor, I just wrote a blog article containing some hopefully useful
advice for students applying to the program.  Please note that this is
my personal opinion only and doesn't necessarily reflect the views of
the Git/libgit2 projects as a whole.

My secret tip for GSoC success

http://softwareswirl.blogspot.com/2014/03/my-secret-tip-for-gsoc-success.html

Michael

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

submodules: reuse .git/modules/... for multiple checkouts of same URL

[Stefan.Liebl]

Hi,

I have a git superproject with 3 submodules. The submodules are cloned from the 
same URL but use different branches. Git clones the repo three times and I have 
three entries in .git/modules. Is it possible to reuse the first clone for the 
next submodule clones?

Stefan
--
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: submodules: reuse .git/modules/... for multiple checkouts of same URL

[Andrew Keller]

On Mar 3, 2014, at 7:24 AM, stefan.li...@partner.bmw.de wrote:

 I have a git superproject with 3 submodules. The submodules are cloned from 
 the same URL but use different branches. Git clones the repo three times and 
 I have three entries in .git/modules. Is it possible to reuse the first clone 
 for the next submodule clones?

Sort of - but I don't think you'll like the side effects.

If each of the submodules are checked out on a different branch, then HEAD is 
going to be different in each repository in .git/modules.  So, each of the 
repositories in .git/modules are unique.

You could share the objects database (see --shared or --reference) which would 
dramatically reduce the size of each repo on its own, but that comes with a 
side effect: because there is no communication between any of the repositories 
involved in sharing objects, git-gc will happily delete an object that may be 
unneeded locally, but another repository will suddenly think it's corrupt.

You could also customize the refspecs in each repository.  For example, if one 
of the submodules has only 'origin/contrib' checked out, then you could change 
the refspec to '+refs/heads/contrib:refs/remotes/origin/contrib', which will 
cause only the objects pertaining to that branch will be downloaded.  This has 
the most benefit when the commit graph is orphaned in some way.  However, this 
approach requires manual labor every time you initialize a submodule.

 - Andrew Keller

--
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] Use ALLOC_GROW() instead of inline code

[He Sun]

2014-02-28 4:45 GMT+08:00 Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru:
 Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@gmail.com
 ---
  attr.c |  7 +--
  builtin/pack-objects.c |  7 +--
  bundle.c   |  6 +-
  cache-tree.c   |  6 +-
  commit.c   |  8 ++--
  diff.c | 12 ++--
  diffcore-rename.c  | 12 ++--
  dir.c  |  5 +
  patch-ids.c|  5 +
  read-cache.c   |  9 ++---
  reflog-walk.c  | 13 +++--
  replace_object.c   |  8 ++--
  12 files changed, 19 insertions(+), 79 deletions(-)


I find another two files can be fixed this way.
- builtin/mktree.c
- sha1_file.c

I find it by searching the key word alloc_nr throughout the source code.
May be there are other places of this kind of code and you can find it
by other ways.

 diff --git a/attr.c b/attr.c
 index 8d13d70..734222d 100644
 --- a/attr.c
 +++ b/attr.c
 @@ -338,12 +338,7 @@ static void handle_attr_line(struct attr_stack *res,
 a = parse_attr_line(line, src, lineno, macro_ok);
 if (!a)
 return;
 -   if (res-alloc = res-num_matches) {
 -   res-alloc = alloc_nr(res-num_matches);
 -   res-attrs = xrealloc(res-attrs,
 - sizeof(struct match_attr *) *
 - res-alloc);
 -   }
 +   ALLOC_GROW(res-attrs, res-num_matches + 1, res-alloc);
 res-attrs[res-num_matches++] = a;
  }

 diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
 index 541667f..92cbce8 100644
 --- a/builtin/pack-objects.c
 +++ b/builtin/pack-objects.c
 @@ -1156,12 +1156,7 @@ static int check_pbase_path(unsigned hash)
 if (0 = pos)
 return 1;
 pos = -pos - 1;
 -   if (done_pbase_paths_alloc = done_pbase_paths_num) {
 -   done_pbase_paths_alloc = alloc_nr(done_pbase_paths_alloc);
 -   done_pbase_paths = xrealloc(done_pbase_paths,
 -   done_pbase_paths_alloc *
 -   sizeof(unsigned));
 -   }
 +   ALLOC_GROW(done_pbase_paths, done_pbase_paths_num + 1, 
 done_pbase_paths_alloc);
 done_pbase_paths_num++;
 if (pos  done_pbase_paths_num)
 memmove(done_pbase_paths + pos + 1,
 diff --git a/bundle.c b/bundle.c
 index e99065c..1388a3e 100644
 --- a/bundle.c
 +++ b/bundle.c
 @@ -14,11 +14,7 @@ static const char bundle_signature[] = # v2 git bundle\n;
  static void add_to_ref_list(const unsigned char *sha1, const char *name,
 struct ref_list *list)
  {
 -   if (list-nr + 1 = list-alloc) {
 -   list-alloc = alloc_nr(list-nr + 1);
 -   list-list = xrealloc(list-list,
 -   list-alloc * sizeof(list-list[0]));
 -   }
 +   ALLOC_GROW(list-list, list-nr + 1, list-alloc);
 memcpy(list-list[list-nr].sha1, sha1, 20);
 list-list[list-nr].name = xstrdup(name);
 list-nr++;
 diff --git a/cache-tree.c b/cache-tree.c
 index 0bbec43..30149d1 100644
 --- a/cache-tree.c
 +++ b/cache-tree.c
 @@ -75,11 +75,7 @@ static struct cache_tree_sub *find_subtree(struct 
 cache_tree *it,
 return NULL;

 pos = -pos-1;
 -   if (it-subtree_alloc = it-subtree_nr) {
 -   it-subtree_alloc = alloc_nr(it-subtree_alloc);
 -   it-down = xrealloc(it-down, it-subtree_alloc *
 -   sizeof(*it-down));
 -   }
 +   ALLOC_GROW(it-down, it-subtree_nr + 1, it-subtree_alloc);
 it-subtree_nr++;

 down = xmalloc(sizeof(*down) + pathlen + 1);
 diff --git a/commit.c b/commit.c
 index 6bf4fe0..e004314 100644
 --- a/commit.c
 +++ b/commit.c
 @@ -147,12 +147,8 @@ int register_commit_graft(struct commit_graft *graft, 
 int ignore_dups)
 return 1;
 }
 pos = -pos - 1;
 -   if (commit_graft_alloc = ++commit_graft_nr) {
 -   commit_graft_alloc = alloc_nr(commit_graft_alloc);
 -   commit_graft = xrealloc(commit_graft,
 -   sizeof(*commit_graft) *
 -   commit_graft_alloc);
 -   }
 +   ALLOC_GROW(commit_graft, commit_graft_nr + 1, commit_graft_alloc);
 +   commit_graft_nr++;
 if (pos  commit_graft_nr)
 memmove(commit_graft + pos + 1,
 commit_graft + pos,
 diff --git a/diff.c b/diff.c
 index 8e4a6a9..f5f0fd1 100644
 --- a/diff.c
 +++ b/diff.c
 @@ -1361,11 +1361,7 @@ static struct diffstat_file *diffstat_add(struct 
 diffstat_t *diffstat,
  {
 struct diffstat_file *x;
 x = xcalloc(sizeof (*x), 1);
 -   if (diffstat-nr == diffstat-alloc) {
 -   diffstat-alloc = alloc_nr(diffstat-alloc);
 -   diffstat-files = 

Re: Branch Name Case Sensitivity

[Lee Hopkins]

 I don't think this distinction is necessary, either you have a 
 case-insensitive file system or you don't. The case
 that the .git directory is case-sensitive and the worktree directory isn't 
 (or the other way around) is
 probably so exotic that we can ignore it.

I think Torsten's use case was for someone who is carefully curating
their loose and packed-refs, e.g. gc.packrefs = false. This could be
for backwards compatibility (existing ambiguous refs whose names
cannot be changed for some reason) or simply because they want to.

 If you want to prevent problems with Windows/Mac OS, you should set 
 core.ignorecase = true. I don't see why we need
 yet another config setting for refs (and logs?).

Since refs.ignorecase falls back to core.ignorecase, you could just
set core.ignorecase = true and feel safe when sharing with Windows/Mac
OS. I think having the distinction just makes Git more flexible, OTOH
I can see how having both refs.ignorecase and core.ignorecase could be
confusing and possibly redundant.
--
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] contrib/subtree - unset prefix before proceeding

[Gilles Filippini]

Hi,

Any chance to have this patch considered?
Thanks in advance,

_g.

Gilles Filippini a écrit , Le 01/03/2014 17:33:
 This is to prevent unwanted prefix when such an environment variable
 exists. The case occurs for example during the Debian package build
 where the git-subtree test suite is called with 'prefix=/usr', which
 makes test 21 fail:
 not ok 21 - Check that prefix argument is required for split
 
 Signed-off-by: Gilles Filippini gilles.filipp...@free.fr
 ---
  contrib/subtree/git-subtree.sh | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
 index dc59a91..db925ca 100755
 --- a/contrib/subtree/git-subtree.sh
 +++ b/contrib/subtree/git-subtree.sh
 @@ -46,6 +46,7 @@ ignore_joins=
  annotate=
  squash=
  message=
 +prefix=
  
  debug()
  {
 

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

[PATCH v2 2/2] i18n: assure command not corrupted by _() process

[Sandy Carter]

Separate message from command examples to avoid translation issues
such as a dash being omitted in a translation.

Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com
---
 builtin/branch.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index b4d7716..b304da8 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -1022,11 +1022,13 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
 */
if (argc == 1  track == BRANCH_TRACK_OVERRIDE 
!branch_existed  remote_tracking) {
-   fprintf(stderr, _(\nIf you wanted to make '%s' track 
'%s', do this:\n\n), head, branch-name);
-   fprintf(stderr, _(git branch -d %s\n), 
branch-name);
-   fprintf(stderr, _(git branch --set-upstream-to 
%s\n), branch-name);
+   fprintf(stderr, \n);
+   fprintf(stderr, _(If you wanted to make '%s' track 
'%s', do this:), head, branch-name);
+   fprintf(stderr, \n\n);
+   fprintf(stderr, git branch -d %s\n, branch-name);
+   fprintf(stderr, git branch --set-upstream-to 
%s\n, branch-name);
+   fprintf(stderr, \n);
}
-
} else
usage_with_options(builtin_branch_usage, options);
 
-- 
1.9.0

--
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 : Add --no-separate-git-dir option to add and update command.

[Henri GEIST]

This new option prevent git submodule add|update to clone the missing
submodules with the --separate-git-dir option.
Then the submodule will be regular repository and their gitdir will not
be placed in the superproject gitdir/modules directory.

Signed-off-by: Henri GEIST geist.he...@laposte.net
---
 Documentation/git-submodule.txt |   18 --
 git-submodule.sh|   22 --
 t/t7400-submodule-basic.sh  |   12 
 3 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 21cb59a..303a475 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -10,13 +10,14 @@ SYNOPSIS
 
 [verse]
 'git submodule' [--quiet] add [-b branch] [-f|--force] [--name name]
- [--reference repository] [--depth depth] [--] repository 
[path]
+ [--reference repository] [--depth depth] 
[--no-separate-git-dir]
+ [--] repository [path]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
 'git submodule' [--quiet] init [--] [path...]
 'git submodule' [--quiet] deinit [-f|--force] [--] path...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
  [-f|--force] [--rebase|--merge|--checkout] [--reference 
repository]
- [--depth depth] [--recursive] [--] [path...]
+ [--depth depth] [--recursive] [--no-separate-git-dir] [--] 
[path...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n]
  [commit] [--] [path...]
 'git submodule' [--quiet] foreach [--recursive] command
@@ -107,6 +108,10 @@ is the superproject and submodule repositories will be kept
 together in the same relative location, and only the
 superproject's URL needs to be provided: git-submodule will correctly
 locate the submodule using the relative URL in .gitmodules.
++
+If `--no-separate-git-dir` is specified, missing submodules will be cloned
+has normal git repository without the option `--separate-git-dir` pointing
+to the modules directory of the superproject gitdir.
 
 status::
Show the status of the submodules. This will print the SHA-1 of the
@@ -185,6 +190,10 @@ If the submodule is not yet initialized, and you just want 
to use the
 setting as stored in .gitmodules, you can automatically initialize the
 submodule with the `--init` option.
 +
+If `--no-separate-git-dir` is specified, missing submodules will be cloned
+has normal git repository without the option `--separate-git-dir` pointing
+to the modules directory of the superproject gitdir.
++
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 +
@@ -363,6 +372,11 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` 
options carefully.
clone with a history truncated to the specified number of revisions.
See linkgit:git-clone[1]
 
+--no-separate-git-dir::
+   This option is valid for add and update commands. Specify that missing
+   submodules should be clonned as self contain repository without a
+   separate gitdir placed in the modules directory of the superproject
+   gitdir.
 
 path...::
Paths to submodule(s). When specified this will restrict the command
diff --git a/git-submodule.sh b/git-submodule.sh
index a33f68d..36eaf31 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -5,11 +5,11 @@
 # Copyright (c) 2007 Lars Hjemli
 
 dashless=$(basename $0 | sed -e 's/-/ /')
-USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference 
repository] [--] repository [path]
+USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference 
repository] [--no-separate-git-dir] [--] repository [path]
or: $dashless [--quiet] status [--cached] [--recursive] [--] [path...]
or: $dashless [--quiet] init [--] [path...]
or: $dashless [--quiet] deinit [-f|--force] [--] path...
-   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] 
[-f|--force] [--rebase] [--reference repository] [--merge] [--recursive] [--] 
[path...]
+   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] 
[-f|--force] [--rebase] [--reference repository] [--merge] [--recursive] 
[--no-separate-git-dir] [--] [path...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit n] 
[commit] [--] [path...]
or: $dashless [--quiet] foreach [--recursive] command
or: $dashless [--quiet] sync [--recursive] [--] [path...]
@@ -36,6 +36,7 @@ update=
 prefix=
 custom_name=
 depth=
+noseparategitdir=
 
 # The function takes at most 2 arguments. The first argument is the
 # URL that navigates to the submodule origin repo. When relative, this URL
@@ -270,6 +271,17 @@ module_clone()
quiet=-q
fi
 
+
+   if test -n $noseparategitdir
+   then
+   (
+   clear_local_git_env
+  

[PATCH v2 1/2] i18n: proposed command missing leading dash

[Sandy Carter]

Add missing leading dash to proposed commands in french output when
using the command:
git branch --set-upstream remotename/branchname
and when upstream is gone

Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com
---
 po/fr.po | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/po/fr.po b/po/fr.po
index e10263f..0b9d59e 100644
--- a/po/fr.po
+++ b/po/fr.po
@@ -1075,7 +1075,7 @@ msgstr Votre branche est basée sur '%s', mais la branche 
amont a disparu.\n
 
 #: remote.c:1875
 msgid   (use \git branch --unset-upstream\ to fixup)\n
-msgstr   (utilisez \git branch -unset-upstream\ pour corriger)\n
+msgstr   (utilisez \git branch --unset-upstream\ pour corriger)\n
 
 #: remote.c:1878
 #, c-format
@@ -3266,7 +3266,7 @@ msgstr git branch -d %s\n
 #: builtin/branch.c:1027
 #, c-format
 msgid git branch --set-upstream-to %s\n
-msgstr git branch -set-upstream-to %s\n
+msgstr git branch --set-upstream-to %s\n
 
 #: builtin/bundle.c:47
 #, c-format
-- 
1.9.0

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

Re: [PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so

[Philip Oakley]

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

We used to show (missing ) next to tests skipped because they are
specified in GIT_SKIP_TESTS.  Use (matched by GIT_SKIP_TESTS) 
instead.


The message below forgets the by. Otherwise looks sensible.


---
t/test-lib.sh |   13 -
1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..89a405b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -446,25 +446,28 @@ test_finish_ () {

test_skip () {
 to_skip=
+ skipped_reason=
 if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
 then
 to_skip=t
+ skipped_reason=matched GIT_SKIP_TESTS


s/matched GIT_SKIP_TESTS/matched by GIT_SKIP_TESTS/


 fi
 if test -z $to_skip  test -n $test_prereq 
! test_have_prereq $test_prereq
 then
 to_skip=t
- fi
- case $to_skip in
- t)
+
 of_prereq=
 if test $missing_prereq != $test_prereq
 then
 of_prereq= of $test_prereq
 fi
-
+ skipped_reason=missing $missing_prereq${of_prereq}
+ fi
+ case $to_skip in
+ t)
 say_color skip 3 skipping test: $@
- say_color skip ok $test_count # skip $1 (missing 
$missing_prereq${of_prereq})

+ say_color skip ok $test_count # skip $1 ($skipped_reason)
 : true
 ;;
 *)
--
1.7.9


--
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] commit.c: Replace starts_with() with skip_prefix()

[karthik nayak]

Hello Eric,
Thanks for Pointing out everything, i had a thorough look and fixed a
couple of things.
Here is an Updated Patch.
- Removed unnecessary code and variables.
- Replaced all instances of starts_with() with skip_prefix()

Replace starts_with() with skip_prefix() for better reading purposes.
Also to replace buf + strlen(author ) by skip_prefix(), which is
saved in a new const char variable buf_skipprefix.

Signed-off-by: Karthik Nayak karthik@gmail.com
---
 commit.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/commit.c b/commit.c
index 6bf4fe0..e5dc2e2 100644
--- a/commit.c
+++ b/commit.c
@@ -552,6 +552,7 @@ static void record_author_date(struct
author_date_slab *author_date,
  char *buffer = NULL;
  struct ident_split ident;
  char *date_end;
+ const char *buf_skipprefix;
  unsigned long date;

  if (!commit-buffer) {
@@ -562,18 +563,20 @@ static void record_author_date(struct
author_date_slab *author_date,
  return;
  }

+ buf_skipprefix = skip_prefix(buf, author );
+
  for (buf = commit-buffer ? commit-buffer : buffer;
  buf;
  buf = line_end + 1) {
  line_end = strchrnul(buf, '\n');
- if (!starts_with(buf, author )) {
+ if (!buf_skipprefix) {
  if (!line_end[0] || line_end[1] == '\n')
  return; /* end of header */
  continue;
  }
  if (split_ident_line(ident,
- buf + strlen(author ),
- line_end - (buf + strlen(author ))) ||
+ buf_skipprefix,
+ line_end - buf_skipprefix) ||
 !ident.date_begin || !ident.date_end)
  goto fail_exit; /* malformed author line */
  break;
@@ -1113,7 +1116,7 @@ int parse_signed_commit(const unsigned char *sha1,
  next = next ? next + 1 : tail;
  if (in_signature  line[0] == ' ')
  sig = line + 1;
- else if (starts_with(line, gpg_sig_header) 
+ else if (skip_prefix(line, gpg_sig_header) 
  line[gpg_sig_header_len] == ' ')
  sig = line + gpg_sig_header_len + 1;
  if (sig) {
@@ -1193,7 +1196,7 @@ static void parse_gpg_output(struct signature_check *sigc)
  for (i = 0; i  ARRAY_SIZE(sigcheck_gpg_status); i++) {
  const char *found, *next;

- if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
+ if (skip_prefix(buf, sigcheck_gpg_status[i].check + 1)) {
  /* At the very beginning of the buffer */
  found = buf + strlen(sigcheck_gpg_status[i].check + 1);
  } else {
-- 
1.9.0.138.g2de3478
--
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] test-lib: GIT_TEST_ONLY to run only specific tests

[Philip Oakley]

Minor nits.

From: Ilya Bobyr ilya.bo...@gmail.com
This is a counterpart to GIT_SKIP_TESTS.  Mostly useful when 
debugging.

---
t/README  |   15 +++
t/test-lib.sh |8 
2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/t/README b/t/README
index caeeb9d..f939987 100644
--- a/t/README
+++ b/t/README
@@ -187,6 +187,21 @@ and either can match the t[0-9]{4} part to skip 
the whole

test, or t[0-9]{4} followed by .$number to say which
particular test to skip.

+Sometimes the opposite is desired - ability to execute only one or
+several tests.  Mostly while debugging tests.  For that you can say
+
+$ GIT_TEST_ONLY=t9200.8 sh ./t9200-git-cvsexport-commit.sh
+
+or, similrary to GIT_SKIP_TESTS
+
+$ GIT_TEST_ONLY='t[0-4]??? t91?? t9200.8' make
+
+In additiona to matching against test suite number.test number
+GIT_TEST_ONLY is matched against just the test .  This comes

s/numbes/numbers/


+handy when you are running only one test:

s/handy/in handy/


+
+$ GIT_TEST_ONLY='[0-8]' sh ./t9200-git-cvsexport-commit.sh
+
Note that some tests in the existing test suite rely on previous
test item, so you cannot arbitrarily disable one and expect the
remainder of test to check what the test originally was intended
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 89a405b..12bf436 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -464,6 +464,14 @@ test_skip () {
 fi
 skipped_reason=missing $missing_prereq${of_prereq}
 fi
+ if test -z $to_skip  test -n $GIT_TEST_ONLY 
+ ! match_pattern_list $this_test.$test_count $GIT_TEST_ONLY 
+ ! match_pattern_list $test_count $GIT_TEST_ONLY
+ then
+ to_skip=t
+ skipped_reason=not in GIT_TEST_ONLY
+ fi
+
 case $to_skip in
 t)
 say_color skip 3 skipping test: $@
--
Philip 


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

[PATCH V2] commit.c: Use skip_prefix() instead of starts_with()

[Tanay Abhra]

In record_author_date()  parse_gpg_output() ,using skip_prefix() instead of
starts_with() is more elegant and abstracts away the details.

Helped-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Patch V2 Corrected email formatting ,reapplied the implementation according to 
suggestions.
Thanks to Michael Haggerty.

This is in respect to GSoC microproject #10.

In record_author_date(), extra and useless calls to strlen due to using 
starts_with()
were removed by using skip_prefix(). Extra variable skip was used as buf is 
used in 
for loop update check.

Other usages of starts_with() in the same file can be found with,

$ grep -n starts_with commit.c

1116:   else if (starts_with(line, gpg_sig_header) 
1196:   if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {

The starts_with() in line 1116 was left as it is, as strlen values were pre 
generated as 
global variables.
The starts_with() in line 1196 was replaced as it abstracts way the skip_prefix 
part by
directly using the function.
Also skip_prefix() is inline when compared to starts_with().

 commit.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/commit.c b/commit.c
index 6bf4fe0..668c703 100644
--- a/commit.c
+++ b/commit.c
@@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);
 static void record_author_date(struct author_date_slab *author_date,
   struct commit *commit)
 {
-   const char *buf, *line_end;
+   const char *buf, *line_end, *skip;
char *buffer = NULL;
struct ident_split ident;
char *date_end;
@@ -566,14 +566,15 @@ static void record_author_date(struct author_date_slab 
*author_date,
 buf;
 buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
-   if (!starts_with(buf, author )) {
+   if (!(skip = skip_prefix(buf, author ))) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
+   buf = skip;
if (split_ident_line(ident,
-buf + strlen(author ),
-line_end - (buf + strlen(author ))) ||
+buf,
+line_end - buf) ||
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed author line */
break;
@@ -1193,9 +1194,9 @@ static void parse_gpg_output(struct signature_check *sigc)
for (i = 0; i  ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
 
-   if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
+   if (found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1)) 
{
/* At the very beginning of the buffer */
-   found = buf + strlen(sigcheck_gpg_status[i].check + 1);
+   ;
} else {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
-- 
1.9.0

--
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] cache_tree_find(): remove redundant check in while condition

[Michael Haggerty]

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
A trivial little cleanup.

 cache-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cache-tree.c b/cache-tree.c
index 0bbec43..7f63c23 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -565,7 +565,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree 
*it, const char *pat
return NULL;
it = sub-cache_tree;
if (slash)
-   while (*slash  *slash == '/')
+   while (*slash == '/')
slash++;
if (!slash || !*slash)
return it; /* prefix ended with slashes */
-- 
1.9.0

--
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] cache_tree_find(): remove redundant check in while condition

[David Kastrup]

Michael Haggerty mhag...@alum.mit.edu writes:

 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 ---
 A trivial little cleanup.

  cache-tree.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/cache-tree.c b/cache-tree.c
 index 0bbec43..7f63c23 100644
 --- a/cache-tree.c
 +++ b/cache-tree.c
 @@ -565,7 +565,7 @@ static struct cache_tree *cache_tree_find(struct 
 cache_tree *it, const char *pat
   return NULL;
   it = sub-cache_tree;
   if (slash)
 - while (*slash  *slash == '/')
 + while (*slash == '/')
   slash++;
   if (!slash || !*slash)
   return it; /* prefix ended with slashes */

That seems dragging around a NULL slash a lot.  How about not checking
for it multiple times?

if (!slash)
return it;
while (*slash == '/')
slash++;
if (!*slash)
return it; /* prefix ended with slashes */

As a bonus, the comment appears to be actually correct.  Attempting to
verify its correctness by seeing whether a non-NULL slash is guaranteed
to really end with slashes, we find the following code above for
defining slash:

slash = strchr(path, '/');
if (!slash)
slash = path + strlen(path);

So it is literally impossible for slash to ever be NULL and all the
checking is nonsensical.  In addition, prefix ended with slashes does
not seem overly convincing when this code path is reached whether or not
there is a slash at all (I am not sure about it: it depends on the
preceding find_subtree to some degree).

So perhaps all of that should just be

while (*slash == '/')
slash++;
if (!*slash)
return it;

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

Re: My advice for GSoC applicants

[Philip Oakley]

From: Michael Haggerty mhag...@alum.mit.edu
Cc: Dmitry Dolzhenko dmitrys.dolzhe...@yandex.ru; Sun He
sunheeh...@gmail.com; Brian Gesiak modoca...@gmail.com; Tanay
Abhra tanay...@gmail.com; Kyriakos Georgiou
kyriakos.a.georg...@gmail.com; Siddharth Goel
siddharth98...@gmail.com; Guanglin Xu mzguang...@gmail.com;
Karthik Nayak karthik@gmail.com; Alberto Corona
albco...@gmail.com; Jacopo Notarstefano
jacopo.notarstef...@gmail.com


Hi,

Based on my experience so far as a first-time Google Summer of Code
mentor, I just wrote a blog article containing some hopefully useful
advice for students applying to the program.  Please note that this is
my personal opinion only and doesn't necessarily reflect the views of
the Git/libgit2 projects as a whole.

   My secret tip for GSoC success

http://softwareswirl.blogspot.com/2014/03/my-secret-tip-for-gsoc-success.html

Michael

--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--


In particular I liked :  If the documentation is unclear, it is OK to
ask for a clarification, but then _fix the documentation_ so that your
mentor never has to answer the same question again.

So the rhetorical question(s) for students would be :-

- was the Git documentation useful - did you see the README?, did it
lead, easily, to the useful places [1]? How could the wording/layout be
improved for the first time reader?

- which points of clarification were most useful and are they in the
documentation? Where should they be included?

- which points needed repeating often, and why? Where was the
disconnect?

- what would a patch look like...

Philip Oakley

[1] README; INSTALL; Documentation/SubmittingPatches;
Documentation/CodingGuidelines;

--
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] cache_tree_find(): remove redundant check in while condition

[Michael Haggerty]

On 03/03/2014 05:32 PM, David Kastrup wrote:
 [...]
 So perhaps all of that should just be
 
   while (*slash == '/')
   slash++;
   if (!*slash)
   return it;
 

I just fixed a little thing that caught my eye.  You OWNED it.  You are
absolutely right.

Will you prepare a patch or would you like me to do it?

Michael

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

Re: My advice for GSoC applicants

[Rick Umali]

On Mon, Mar 3, 2014 at 5:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
 My secret tip for GSoC success

 http://softwareswirl.blogspot.com/2014/03/my-secret-tip-for-gsoc-success.html

I enjoyed reading that BLOG post. I daresay some of the points you
raise are pertinent for any new contributor to any open-source code
base.
--
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] cache_tree_find(): remove redundant check in while condition

[David Kastrup]

Michael Haggerty mhag...@alum.mit.edu writes:

 On 03/03/2014 05:32 PM, David Kastrup wrote:
 [...]
 So perhaps all of that should just be
 
  while (*slash == '/')
  slash++;
  if (!*slash)
  return it;
 

 I just fixed a little thing that caught my eye.  You OWNED it.  You are
 absolutely right.

 Will you prepare a patch or would you like me to do it?

Feel free.  I really should be finishing some other patches instead...

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

Re: [PATCH] submodule : Add --no-separate-git-dir option to add and update command.

[Jens Lehmann]

Am 03.03.2014 14:47, schrieb Henri GEIST:
 This new option prevent git submodule add|update to clone the missing
 submodules with the --separate-git-dir option.
 Then the submodule will be regular repository and their gitdir will not
 be placed in the superproject gitdir/modules directory.

And what is your motivation for this? After all submodules containing
a .git directory are second class citizens (because they can never be
safely removed by regular git commands).

 Signed-off-by: Henri GEIST geist.he...@laposte.net
 ---
  Documentation/git-submodule.txt |   18 --
  git-submodule.sh|   22 --
  t/t7400-submodule-basic.sh  |   12 
  3 files changed, 48 insertions(+), 4 deletions(-)
 
 diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
 index 21cb59a..303a475 100644
 --- a/Documentation/git-submodule.txt
 +++ b/Documentation/git-submodule.txt
 @@ -10,13 +10,14 @@ SYNOPSIS
  
  [verse]
  'git submodule' [--quiet] add [-b branch] [-f|--force] [--name name]
 -   [--reference repository] [--depth depth] [--] repository 
 [path]
 +   [--reference repository] [--depth depth] 
 [--no-separate-git-dir]
 +   [--] repository [path]
  'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
  'git submodule' [--quiet] init [--] [path...]
  'git submodule' [--quiet] deinit [-f|--force] [--] path...
  'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
 [-f|--force] [--rebase|--merge|--checkout] [--reference 
 repository]
 -   [--depth depth] [--recursive] [--] [path...]
 +   [--depth depth] [--recursive] [--no-separate-git-dir] [--] 
 [path...]
  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) 
 n]
 [commit] [--] [path...]
  'git submodule' [--quiet] foreach [--recursive] command
 @@ -107,6 +108,10 @@ is the superproject and submodule repositories will be 
 kept
  together in the same relative location, and only the
  superproject's URL needs to be provided: git-submodule will correctly
  locate the submodule using the relative URL in .gitmodules.
 ++
 +If `--no-separate-git-dir` is specified, missing submodules will be cloned
 +has normal git repository without the option `--separate-git-dir` pointing
 +to the modules directory of the superproject gitdir.
  
  status::
   Show the status of the submodules. This will print the SHA-1 of the
 @@ -185,6 +190,10 @@ If the submodule is not yet initialized, and you just 
 want to use the
  setting as stored in .gitmodules, you can automatically initialize the
  submodule with the `--init` option.
  +
 +If `--no-separate-git-dir` is specified, missing submodules will be cloned
 +has normal git repository without the option `--separate-git-dir` pointing
 +to the modules directory of the superproject gitdir.
 ++
  If `--recursive` is specified, this command will recurse into the
  registered submodules, and update any nested submodules within.
  +
 @@ -363,6 +372,11 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` 
 options carefully.
   clone with a history truncated to the specified number of revisions.
   See linkgit:git-clone[1]
  
 +--no-separate-git-dir::
 + This option is valid for add and update commands. Specify that missing
 + submodules should be clonned as self contain repository without a
 + separate gitdir placed in the modules directory of the superproject
 + gitdir.
  
  path...::
   Paths to submodule(s). When specified this will restrict the command
 diff --git a/git-submodule.sh b/git-submodule.sh
 index a33f68d..36eaf31 100755
 --- a/git-submodule.sh
 +++ b/git-submodule.sh
 @@ -5,11 +5,11 @@
  # Copyright (c) 2007 Lars Hjemli
  
  dashless=$(basename $0 | sed -e 's/-/ /')
 -USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference 
 repository] [--] repository [path]
 +USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference 
 repository] [--no-separate-git-dir] [--] repository [path]
 or: $dashless [--quiet] status [--cached] [--recursive] [--] [path...]
 or: $dashless [--quiet] init [--] [path...]
 or: $dashless [--quiet] deinit [-f|--force] [--] path...
 -   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] 
 [-f|--force] [--rebase] [--reference repository] [--merge] [--recursive] 
 [--] [path...]
 +   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] 
 [-f|--force] [--rebase] [--reference repository] [--merge] [--recursive] 
 [--no-separate-git-dir] [--] [path...]
 or: $dashless [--quiet] summary [--cached|--files] [--summary-limit n] 
 [commit] [--] [path...]
 or: $dashless [--quiet] foreach [--recursive] command
 or: $dashless [--quiet] sync [--recursive] [--] [path...]
 @@ -36,6 +36,7 @@ update=
  prefix=
  custom_name=
  depth=
 +noseparategitdir=
  
  # The function takes at most 2 arguments. 

Re: [PATCH v3 01/25] path.c: make get_pathname() return strbuf instead of static buffer

[Junio C Hamano]

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

 On Thu, Feb 20, 2014 at 6:26 AM, Junio C Hamano gits...@pobox.com wrote:
 Is there a reason not to do just an equivalent of

 #define git_pathdup mkpathdup

 and be done with it?  Am I missing something?

 They have a subtle difference: mkpathdup() calls cleanup_path() while
 git_pathdup() does not. Without auditing all call sites, we can't be
 sure this difference is insignificant. So I keep both functions
 separate for now.

Thanks; that is a very good explanation why #define'ing two to be
the same would not be a workable solution to the ownership issue I
raised.

  char *git_pathdup(const char *fmt, ...)
  {
 - char path[PATH_MAX], *ret;
 + struct strbuf *path = get_pathname();
   va_list args;
   va_start(args, fmt);
 - ret = vsnpath(path, sizeof(path), fmt, args);
 + vsnpath(path, fmt, args);
   va_end(args);
 - return xstrdup(ret);
 + return strbuf_detach(path, NULL);
  }

This feels somewhat wrong.

This function is for callers who are willing to take ownership of
the path buffer and promise to free the returned buffer when they
are done, because you are returning strbuf_detach()'ed piece of
memory, giving the ownership away.

The whole point of using get_pathname() is to allow callers not to
care about allocation issues on the paths they scribble on during
their short-and-simple codepaths that do not have too many uses of
similar temporary path buffers.  Why borrow from that round-robin
pool (which may now cause some codepaths to overflow the number of
such active temporary path buffers---have they been all audited)?
--
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: Branch Name Case Sensitivity

[Junio C Hamano]

Lee Hopkins leer...@gmail.com writes:

 I went ahead and took a stab at a solution. My solution is more
 aggressive than a warning, I actually prevent the creation of
 ambiguous refs. My changes are also in refs.c, which may not be
 appropriate, but it seemed like the natural place.

 I have never contributed to Git (in fact this is my first dive into
 the source) and my C is a bit rusty, so bear with me, this is just a
 suggestion:

 ---
  refs.c |   31 ---
  1 files changed, 24 insertions(+), 7 deletions(-)

Starting something like this from forbidding is likely to turn out
to be a very bad idea that can break existing repositories.

A new configuration

refs.caseInsensitive = {warn|error|allow}

that defaults to warn and the user can choose to set to error to
forbid, would be more palatable, I would say.

If the variable is not in 'core.' namespace, you should implement
this check at the Porcelain level, allowing lower-level tools like
update-ref as an escape hatch that let users bypass the restriction
to be used to correct breakages; it would mean an unconditional if
!stricmp(), it is an error in refs.c will not work well.

I think it might be OK to have

core.allowCaseInsentitiveRefs = {yes|no|warn}

which defaults to 'warn' (and 'yes' corresponds to 'allow', 'no'
corresponds to 'error', in the previous suggestion), instead. If we
wanted to prevent even lower-level tools like update-ref from
bypassing the check, that is.

--
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 07/25] reflog: avoid constructing .lock path with git_path

[Junio C Hamano]

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

 On Wed, Feb 26, 2014 at 5:44 AM, Junio C Hamano gits...@pobox.com wrote:
 Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 git_path() soon understands the path given to it. Some paths abc may
 become def while other ghi may become ijk. We don't want
 git_path() to interfere with .lock path construction. Concatenate
 .lock after the path has been resolved by git_path() so if abc
 becomes def, we'll have def.lock, not ijk.

 Hmph.  I am not sure if the above is readable (or if I am reading it
 correctly).

 Definitely not now that I have had my break from the series and reread it.

 If abc becomes def, it would take deliberate work to make
 abc.lock into ijk, and it would be natural to expect that
 abc.lock would become def.lock without any fancy trick, no?

 A better explanation may be, while many paths are not converted by
 git_path() (abc - abc), some of them will be based on the given
 path (def - ghi). Giving path def.lock to git_path() may confuse
 it and make it believe def.lock should not be converted because the
 signature is def.lock not def. But we want the lock file to have
 the same base name with the locked file (e.g. ghi.lock, not
 def.lock). So it's best to append .lock after git_path() has done
 its conversion.

 The alternative is teach git_path about .lock, but I don't really
 want to put more logic down there.

I think your attempt to sound generic by using abc, def,
etc. backfired and made the description only obscure.  It would have
been immediately understandable if it were more like this:

Among pathnames in $GIT_DIR, e.g. index or packed-refs,
we want to automatically and silently map some of them to
the $GIT_DIR of the repository we are borrowing from via
$GIT_COMMON_DIR mechanism.  When we formulate the pathname
for its lockfile, we want it to be in the same location as
its final destination.  index is not shared and needs to
remain in the borrowing repository, while packed-refs is
shared and needs to go to the borrowed repository.

git_path() could be taught about the .lock suffix and map
index.lock and packed-refs.lock the same way their
basenames are mapped, but instead the caller can help by
asking where the basename (e.g. index) is mapped to
git_path() and then appending .lock after the mapping is
done.

Thanks for an explanation.  With that understanding, the patch makes
sense.


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

Re: [PATCH] repack: add `repack.honorpackkeep` config var

[Junio C Hamano]

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

 On Fri, Feb 28, 2014 at 10:45:39AM -0800, Junio C Hamano wrote:

  Exactly. The two features (bitmaps and .keep) are not compatible with
  each other, so you have to prioritize one. If you are using static .keep
  files, you might want them to continue being respected at the expense of
  using bitmaps for that repo. So I think you want a separate option from
  --write-bitmap-index to allow the appropriate flexibility.
 
 What is the appropriate flexibility, though?  If the user wants to
 use bitmap, we would need to drop .keep, no?

 Or the flip side: if the user wants to use .keep, we should drop
 bitmaps. My point is that we do not know which way the user wants to
 go, so we should not tie the options together.

Hmph.  I think the short of your later explanation is global config
may tell us to use bitmap, in which case we would need a way to
defeat that and have existing .keep honored, and it makes it easier
to do so if these two are kept separate, because you do not want to
run around and selectively disable bitmaps in these repositories.
We can instead do so with repack.packKeptObjects in the global
configuration. and I tend to agree with the reasoning.

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

Re: [PATCH] repack: add `repack.honorpackkeep` config var

[Jeff King]

On Mon, Mar 03, 2014 at 10:13:47AM -0800, Junio C Hamano wrote:

  Or the flip side: if the user wants to use .keep, we should drop
  bitmaps. My point is that we do not know which way the user wants to
  go, so we should not tie the options together.
 
 Hmph.  I think the short of your later explanation is global config
 may tell us to use bitmap, in which case we would need a way to
 defeat that and have existing .keep honored, and it makes it easier
 to do so if these two are kept separate, because you do not want to
 run around and selectively disable bitmaps in these repositories.
 We can instead do so with repack.packKeptObjects in the global
 configuration. and I tend to agree with the reasoning.

Yes. Do you need a re-roll from me? I think the last version I sent +
the squash to tie the default to bitmap-writing makes the most sense.

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

Re: [PATCH v2 00/11] Use ALLOC_GROW() instead of inline code

[Junio C Hamano]

Michael Haggerty mhag...@alum.mit.edu writes:

 On 02/28/2014 10:29 AM, Dmitry S. Dolzhenko wrote:
 Thank you for your remarks. In this patch I tried to take them into account.
 
 Dmitry S. Dolzhenko (11):
   builtin/pack-objects.c: change check_pbase_path() to use ALLOC_GROW()
   bundle.c: change add_to_ref_list() to use ALLOC_GROW()
   cache-tree.c: change find_subtree() to use ALLOC_GROW()
   commit.c: change register_commit_graft() to use ALLOC_GROW()
   diff.c: use ALLOC_GROW() instead of inline code
   diffcore-rename.c: use ALLOC_GROW() instead of inline code
   patch-ids.c: change add_commit() to use ALLOC_GROW()
   replace_object.c: change register_replace_object() to use ALLOC_GROW()
   reflog-walk.c: use ALLOC_GROW() instead of inline code
   dir.c: change create_simplify() to use ALLOC_GROW()
   attr.c: change handle_attr_line() to use ALLOC_GROW()
 
  attr.c |  7 +--
  builtin/pack-objects.c |  7 +--
  bundle.c   |  6 +-
  cache-tree.c   |  6 +-
  commit.c   |  8 ++--
  diff.c | 12 ++--
  diffcore-rename.c  | 12 ++--
  dir.c  |  5 +
  patch-ids.c|  5 +
  reflog-walk.c  | 13 +++--
  replace_object.c   |  8 ++--
  11 files changed, 17 insertions(+), 72 deletions(-)

 Everything looks fine to me.  Assuming the test suite ran 100%,

 Acked-by: Michael Haggerty mhag...@alum.mit.edu

Looked good (modulo titles, which I think we already discussed),
and queued on 'pu'.

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

Re: [PATCH v2 01/11] builtin/pack-objects.c: change check_pbase_path() to use ALLOC_GROW()

[Junio C Hamano]

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

 I realize that I just bikeshedded on subject lines for half a page, and
 part of me wants to go kill myself in shame. But I feel like I see the
 technique misapplied often enough that maybe some guidance is merited.

Thanks.  What I queued read like these:

$ git shortlog ..dd/use-alloc-grow

Dmitry S. Dolzhenko (11):
  builtin/pack-objects.c: use ALLOC_GROW() in check_pbase_path()
  bundle.c: use ALLOC_GROW() in add_to_ref_list()
  cache-tree.c: use ALLOC_GROW() in find_subtree()
  commit.c: use ALLOC_GROW() in register_commit_graft()
  diff.c: use ALLOC_GROW() instead of inline code
  diffcore-rename.c: use ALLOC_GROW() instead of inline code
  patch-ids.c: use ALLOC_GROW() in add_commit()
  replace_object.c: use ALLOC_GROW() in register_replace_object()
  reflog-walk.c: use ALLOC_GROW() instead of inline code
  dir.c: use ALLOC_GROW() in create_simplify()
  attr.c: use ALLOC_GROW() in handle_attr_line()

but I tend to agree with you that we can just stop at use ALLOC_GROW
after the filename.
--
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] implemented strbuf_write_or_die()

[Junio C Hamano]

Faiz Kothari faiz.of...@gmail.com writes:

 Signed-off-by: Faiz Kothari faiz.of...@gmail.com
 ---
 Implemented write_or_die.c:strbuf_write_or_die() and used in relevant places
 to substitute write_or_die(). I spotted other places where strbuf can be used
 in place of buf[MAX_PATH] but that would require a change in prototype of a 
 lot of functions and functions calling them and so on
 I'll look for more places where strbuf can be used safely.

 Thanks.

  builtin/cat-file.c |2 +-
  builtin/notes.c|4 ++--
  builtin/receive-pack.c |2 +-
  builtin/send-pack.c|2 +-
  builtin/stripspace.c   |2 +-
  builtin/tag.c  |2 +-
  bundle.c   |2 +-
  cache.h|1 +
  credential-store.c |2 +-
  fetch-pack.c   |2 +-
  http-backend.c |2 +-
  remote-curl.c  |8 +---
  write_or_die.c |9 +
  13 files changed, 26 insertions(+), 14 deletions(-)

It does not reduce the code, it does not make the resulting code
read any easier.

What is the benefit of this change?
--
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] implemented strbuf_write_or_die()

[Junio C Hamano]

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

 On Sat, Mar 1, 2014 at 7:51 AM, He Sun sunheeh...@gmail.com wrote:
 2014-03-01 19:21 GMT+08:00 Faiz Kothari faiz.of...@gmail.com:
 diff --git a/remote-curl.c b/remote-curl.c
 index 10cb011..dee8716 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -634,7 +634,7 @@ static int rpc_service(struct rpc_state *rpc, struct 
 discovery *heads)
 if (start_command(client))
 exit(1);
 if (preamble)
 -   write_or_die(client.in, preamble-buf, preamble-len);
 +   strbuf_write_or_die(client.in, preamble);
 if (heads)
 write_or_die(client.in, heads-buf, heads-len);

 This should be changed. May be you can use Ctrl-F to search write_or_die().
 Or if you are using vim, use / and n to find all.

 It's not obvious from the patch fragment, but 'heads' is not a strbuf,
 so Faiz correctly left this invocation alone.

That is a very good sign why this change is merely a code-churn and
not an improvement, isn't it?  We know (and any strbuf user should
know) that -buf and -len are the ways to learn the pointer and the
length the strbuf holds.  Why anybody thinks it is benefitial to
introduce another function that is _only_ for writing out strbuf and
cannot be used to write out a plain buffer is simply beyond me.

--
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: An idea for git bisect and a GSoC enquiry

[Junio C Hamano]

Jacopo Notarstefano jacopo.notarstef...@gmail.com writes:

 Here my proposal differs in that I have no way of knowing which label
 is good and which label is bad: I blindly accept two distinct labels
 and bisect with those. I gave an example of this behaviour above.

I think you fundamentally cannot use two labels that are merely
distinct and bisect correctly.  You need to know which ones
(i.e. good) are to be excluded and the other (i.e. bad) are to be
included when computing the remaining to be tested set of commits.
--
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

Rewriting git history during git-svn conversion

[Robert Dailey]

Hi,

I am converting my team's SVN server to GIT but they aren't ready to
transition yet. People are still working out of SVN, so I need to keep
the git-svn clone around to do 'git svn fetch' to continue to keep it
synchronized with SVN.

Once everyone is ready to switch, and after converting all of the
tag-branches to real GIT tags, I plan to push all branches  tags to
the new central git repository.

However, before pushing to the central GIT repo, I want to remove some
giant directories from the repository history. Specifically some third
party library directories. I have found a way to do this here:
http://stackoverflow.com/questions/10067848/remove-folder-and-its-contents-from-git-githubs-history

Is it safe to do this while still using git svn fetch? Will it
properly continue to convert SVN commits on top of my rewritten
history? If not, what changes can I make after I run the commands
linked by the URL above so that git svn continues to work normally?

Note that I plan to delete the third party libraries from SVN side and
then pull down the changes to git through git-svn, then at that point
clean it up (hopefully the git filter-branch command ignores the
commit that deleted the files and only hunts down the commits where
they were added or modified).

Any guidance on this would be much appreciated.
--
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] commit.c:record_author_date() use skip_prefix() instead of starts_with()

[Junio C Hamano]

Michael Haggerty mhag...@alum.mit.edu writes:

 -if (!starts_with(buf, author )) {
 +if (!skip_prefix(buf, author )) {

 If this is the only change, there is not much point, is there?  How does
 this help?  Perhaps there is some way to take advantage of the
 difference between starts_with() and skip_prefix() to simplify the rest
 of the function?

I admit I lost track, but wasn't there a discussion to use
starts_with/ends_with when appropriate (namely, the caller is
absolutely not interested in what the remainder of the string is
after skipping the prefix), moving away from skip_prefix()?  Isn't
this change going in the wrong direction?
--
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] write_pack_file: use correct variable in diagnostic

[Junio C Hamano]

Sun He sunheeh...@gmail.com writes:

 'pack_tmp_name' is the subject of the utime() check, so report it in the
 warning, not the uninitialized 'tmpname'

 Signed-off-by: Sun He sunheeh...@gmail.com
 ---

  Changing the subject and adding valid information as tutored by 
  Eric Sunshine.
  Thanks to him.

  builtin/pack-objects.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
 index c733379..4922ce5 100644
 --- a/builtin/pack-objects.c
 +++ b/builtin/pack-objects.c
 @@ -823,7 +823,7 @@ static void write_pack_file(void)
   utb.modtime = --last_mtime;
   if (utime(pack_tmp_name, utb)  0)
   warning(failed utime() on %s: %s,
 - tmpname, strerror(errno));
 + pack_tmp_name, strerror(errno));
   }
  
   /* Enough space for -sha-1.pack? */

Very nicely done.  Thanks.

And big Thanks to Eric guiding this patch through.

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

Fwd: [PATCH] implemented strbuf_write_or_die()

[Faiz Kothari]

On Tue, Mar 4, 2014 at 12:01 AM, Junio C Hamano gits...@pobox.com wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:

 On Sat, Mar 1, 2014 at 7:51 AM, He Sun sunheeh...@gmail.com wrote:
 2014-03-01 19:21 GMT+08:00 Faiz Kothari faiz.of...@gmail.com:
 diff --git a/remote-curl.c b/remote-curl.c
 index 10cb011..dee8716 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -634,7 +634,7 @@ static int rpc_service(struct rpc_state *rpc, struct 
 discovery *heads)
 if (start_command(client))
 exit(1);
 if (preamble)
 -   write_or_die(client.in, preamble-buf, preamble-len);
 +   strbuf_write_or_die(client.in, preamble);
 if (heads)
 write_or_die(client.in, heads-buf, heads-len);

 This should be changed. May be you can use Ctrl-F to search write_or_die().
 Or if you are using vim, use / and n to find all.

 It's not obvious from the patch fragment, but 'heads' is not a strbuf,
 so Faiz correctly left this invocation alone.

 That is a very good sign why this change is merely a code-churn and
 not an improvement, isn't it?  We know (and any strbuf user should
 know) that -buf and -len are the ways to learn the pointer and the
 length the strbuf holds.  Why anybody thinks it is benefitial to
 introduce another function that is _only_ for writing out strbuf and
 cannot be used to write out a plain buffer is simply beyond me.

Hi,
Thanks for the feedback. Yes, I do realize, its kind of a code churn.
I didn't realize it until I looked at the sign you pointed out.
But it was a good exercise to go through the code as this is one of
the GSoC microprojects.
Sorry, it didn't turn out to be a beneficial one. My bad.

Thanks a lot again for the suggestions and feedback.

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

Re: [PATCH v2] branch.c: change install_branch_config() to use skip_prefix()

[Junio C Hamano]

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

 diff --git a/branch.c b/branch.c
 index 723a36b..ca4e824 100644
 --- a/branch.c
 +++ b/branch.c
 @@ -50,7 +50,7 @@ static int should_setup_rebase(const char *origin)
  void install_branch_config(int flag, const char *local, const char *origin, 
 const char *remote)
  {
 const char *shortname = remote + 11;
 -   int remote_is_branch = starts_with(remote, refs/heads/);
 +   int remote_is_branch = (NULL != skip_prefix(remote ,refs/heads/));

 This actually makes the code more difficult to read and understand.
 There's a more fundamental reason to use skip_prefix() here. See if
 you can figure it out. (Hint: shortname)

I've already queued 0630aa49 (branch: use skip_prefix() in
install_branch_config(), 2014-02-28) on this topic, by the way.

--
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] git-config: document interactive.singlekey requires Term::Readkey

[Junio C Hamano]

Simon Ruderich si...@ruderich.org writes:

 Most distributions don't require Term::Readkey as dependency,
 leaving the user to wonder why the setting doesn't work.

 Signed-off-by: Simon Ruderich si...@ruderich.org

Thanks, but is it true that interactive.singlekey requries
Term::ReadKey?

The relevant part of git-add--interactive reads like so:

if ($repo-config_bool(interactive.singlekey)) {
eval {
require Term::ReadKey;
Term::ReadKey-import;
$use_readkey = 1;
};
eval {
require Term::Cap;
my $termcap = Term::Cap-Tgetent;
foreach (values %$termcap) {
$term_escapes{$_} = 1 if /^\e/;
}
$use_termcap = 1;
};
}

The implementation of prompt_single_character sub wants to use
ReadKey, but can still let the user interact with the program by
falling back to a cooked input when it is not available, so perhaps
a better fix might be something like this:

if (!$use_readkey) {
print STDERR missing Term::ReadKey, disabling 
interactive.singlekey\n;
}

inside the above if() that prepares $use_readkey?

You also misspelled the package name it seems ;-)
--
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] skip_prefix: rewrite so that prefix is scanned once

[Junio C Hamano]

Siddharth Goel siddharth98...@gmail.com writes:

 Helped-by: Eric Sunshine sunsh...@sunshineco.com
 Signed-off-by: Siddharth Goel siddharth98...@gmail.com
 ---
 Added a space after colon in the subject as compared to previous 
 patch [PATCH v2].

 [PATCH v2]: http://thread.gmane.org/gmane.comp.version-control.git/243150

Whenever you see Change, Rewrite, etc. in the subject of a patch
that touches existing code, think twice.  The subject line is a
scarce real estate to be wasted on a noiseword that carries no real
information, and we already know a patch that touches existing code
changes or rewrites things.

Subject: [PATCH v3] skip_prefix: scan prefix only once

perhaps?

  git-compat-util.h | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

 diff --git a/git-compat-util.h b/git-compat-util.h
 index 614a5e9..550dce3 100644
 --- a/git-compat-util.h
 +++ b/git-compat-util.h
 @@ -357,8 +357,11 @@ extern int suffixcmp(const char *str, const char 
 *suffix);
  
  static inline const char *skip_prefix(const char *str, const char *prefix)
  {
 - size_t len = strlen(prefix);
 - return strncmp(str, prefix, len) ? NULL : str + len;
 + while (*prefix != '\0'  *str == *prefix) {
 + str++;
 + prefix++;
 + }
 + return (*prefix == '\0' ? str : NULL);

Unlike another patch I saw the other day on the same topic, this
checks *prefix twice for the last round, even though I think this
one is probably slightly easier to read.  I dunno.

--
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] submodule : Add --no-separate-git-dir option to add and update command.

[Junio C Hamano]

[CC'ing the submodule area experts.]

Henri GEIST geist.he...@laposte.net writes:

 This new option prevent git submodule add|update to clone the missing
 submodules with the --separate-git-dir option.
 Then the submodule will be regular repository and their gitdir will not
 be placed in the superproject gitdir/modules directory.

 Signed-off-by: Henri GEIST geist.he...@laposte.net
 ---

Thanks.

The above describes what the new option does, but does not explain
why the new option is a good idea in the first place.

Given that we used to directly clone into the superproject's working
tree like this patch does, realized that it was a very bad idea and
are trying to move to the direction of keeping it in modules/
subdirectory of the superproject's .git directory, there needs to be
a very good explanation to justify why this going backwards is
sometimes a desirable thing.


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

Re: [PATCH V2] commit.c: Use skip_prefix() instead of starts_with()

[Junio C Hamano]

Tanay Abhra tanay...@gmail.com writes:

 In record_author_date()  parse_gpg_output() ,using skip_prefix() instead of
 starts_with() is more elegant and abstracts away the details.

Avoid subjective judgement like more elegant when justifying your
change; you are not your own judge.

The caller of starts_with() actually can use the string that follows
the expected prefix and that is the reason why using skip_prefix()
in these places is a good idea.  There is no need to be subjective
to justify that change.

I do not think there is any more abstracting away of the details in
this change.  The updated uses a different and more suitable
abstraction than the original.

 diff --git a/commit.c b/commit.c
 index 6bf4fe0..668c703 100644
 --- a/commit.c
 +++ b/commit.c
 @@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);
  static void record_author_date(struct author_date_slab *author_date,
  struct commit *commit)
  {
 - const char *buf, *line_end;
 + const char *buf, *line_end, *skip;
   char *buffer = NULL;
   struct ident_split ident;
   char *date_end;
 @@ -566,14 +566,15 @@ static void record_author_date(struct author_date_slab 
 *author_date,
buf;
buf = line_end + 1) {
   line_end = strchrnul(buf, '\n');
 - if (!starts_with(buf, author )) {
 + if (!(skip = skip_prefix(buf, author ))) {

We tend to avoid assignments in conditionals.

   if (!line_end[0] || line_end[1] == '\n')
   return; /* end of header */
   continue;
   }
 + buf = skip;
   if (split_ident_line(ident,
 -  buf + strlen(author ),
 -  line_end - (buf + strlen(author ))) ||
 +  buf,
 +  line_end - buf) ||
   !ident.date_begin || !ident.date_end)
   goto fail_exit; /* malformed author line */
   break;

If you give a sensible name to what 'buf + strlen(author )' is,
then the result becomes a lot more readable compared to the
original, and I think that is what this change is about.

And skip is not a good name for that.  'but + strlen(author )'
is what split_ident_line() expects its input to be split; let's
tentatively call it ident_line and see what the call looks like:

split_ident_line(ident, ident_line, line_end - ident_line))

And that is what we want to see here.  It is a bit more clear than
the original that we are splitting the ident information on the line,
ident_line (you could call it ident_begin) points at the beginning
and line_end points at the end of that ident information.

Use of skip_prefix(), which I am sure you took the name of the new
variable skip from, is merely an implementation detail of finding
where the ident begins.  A good rule of thumb to remember is to name
things after what they are, not how you obtain them, how they are
used or what they are used for/as.

 @@ -1193,9 +1194,9 @@ static void parse_gpg_output(struct signature_check 
 *sigc)
   for (i = 0; i  ARRAY_SIZE(sigcheck_gpg_status); i++) {
   const char *found, *next;
  
 - if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
 + if (found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1)) 
 {
   /* At the very beginning of the buffer */
 - found = buf + strlen(sigcheck_gpg_status[i].check + 1);
 + ;
   } else {
   found = strstr(buf, sigcheck_gpg_status[i].check);
   if (!found)

This hunk looks good.  It can be a separate patch but they are both
minor changes so it is OK to have it in a single patch.
--
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] implemented strbuf_write_or_die()

[Eric Sunshine]

On Mon, Mar 3, 2014 at 1:31 PM, Junio C Hamano gits...@pobox.com wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:

 On Sat, Mar 1, 2014 at 7:51 AM, He Sun sunheeh...@gmail.com wrote:
 2014-03-01 19:21 GMT+08:00 Faiz Kothari faiz.of...@gmail.com:
 diff --git a/remote-curl.c b/remote-curl.c
 index 10cb011..dee8716 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -634,7 +634,7 @@ static int rpc_service(struct rpc_state *rpc, struct 
 discovery *heads)
 if (start_command(client))
 exit(1);
 if (preamble)
 -   write_or_die(client.in, preamble-buf, preamble-len);
 +   strbuf_write_or_die(client.in, preamble);
 if (heads)
 write_or_die(client.in, heads-buf, heads-len);

 This should be changed. May be you can use Ctrl-F to search write_or_die().
 Or if you are using vim, use / and n to find all.

 It's not obvious from the patch fragment, but 'heads' is not a strbuf,
 so Faiz correctly left this invocation alone.

 That is a very good sign why this change is merely a code-churn and
 not an improvement, isn't it?  We know (and any strbuf user should
 know) that -buf and -len are the ways to learn the pointer and the
 length the strbuf holds.  Why anybody thinks it is benefitial to
 introduce another function that is _only_ for writing out strbuf and
 cannot be used to write out a plain buffer is simply beyond me.

As a potential GSoC student and newcomer to the project, Faiz would
not have known that this would be considered unwanted churn when he
chose the task from the GSoC microproject page [1]. Perhaps it would
be a good idea to retire this item from the list?

On the other hand, it did expose Faiz to the iterative code review
process on this project and gave him a taste of what would be expected
of him as a GSoC student, so the microproject achieved that important
goal, and thus wasn't an utter failure.

[1]: https://github.com/git/git.github.io/blob/master/SoC-2014-Microprojects.md
--
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] repack: add `repack.honorpackkeep` config var

[Junio C Hamano]

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

 On Mon, Mar 03, 2014 at 10:13:47AM -0800, Junio C Hamano wrote:

  Or the flip side: if the user wants to use .keep, we should drop
  bitmaps. My point is that we do not know which way the user wants to
  go, so we should not tie the options together.
 
 Hmph.  I think the short of your later explanation is global config
 may tell us to use bitmap, in which case we would need a way to
 defeat that and have existing .keep honored, and it makes it easier
 to do so if these two are kept separate, because you do not want to
 run around and selectively disable bitmaps in these repositories.
 We can instead do so with repack.packKeptObjects in the global
 configuration. and I tend to agree with the reasoning.

 Yes. Do you need a re-roll from me? I think the last version I sent +
 the squash to tie the default to bitmap-writing makes the most sense.

I have 9e20b390 (repack: add `repack.packKeptObjects` config var,
2014-02-26); I do not recall I've squashed anything into it, though.

Do you mean this one?

Here's the interdiff for doing the fallback:

---
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3a3d84f..a8ddc7f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2139,7 +2139,9 @@ repack.usedeltabaseoffset::
 repack.packKeptObjects::
If set to true, makes `git repack` act as if
`--pack-kept-objects` was passed. See linkgit:git-repack[1] for
-   details. Defaults to false.
+   details. Defaults to `false` normally, but `true` if a bitmap
+   index is being written (either via `--write-bitmap-index` or
+   `pack.writeBitmaps`).
 
 rerere.autoupdate::
When set to true, `git-rerere` updates the index with the
diff --git a/builtin/repack.c b/builtin/repack.c
index 49947b2..6b0b62d 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -9,7 +9,7 @@
 #include argv-array.h
 
 static int delta_base_offset = 1;
-static int pack_kept_objects;
+static int pack_kept_objects = -1;
 static char *packdir, *packtmp;
 
 static const char *const git_repack_usage[] = {
@@ -190,6 +190,9 @@ int cmd_repack(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, builtin_repack_options,
git_repack_usage, 0);
 
+   if (pack_kept_objects  0)
+   pack_kept_objects = write_bitmap;
+
packdir = mkpathdup(%s/pack, get_object_directory());
packtmp = mkpathdup(%s/.tmp-%d-pack, packdir, (int)getpid());
 
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index f8431a8..b1eed5c 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -21,7 +21,7 @@ test_expect_success 'objects in packs marked .keep are not 
repacked' '
objsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |
sed -e s/^\([0-9a-f]\{40\}\).*/\1/) 
mv pack-* .git/objects/pack/ 
-   git repack -A -d -l 
+   git repack --no-pack-kept-objects -A -d -l 
git prune-packed 
for p in .git/objects/pack/*.idx; do
idx=$(basename $p)
@@ -35,9 +35,9 @@ test_expect_success 'objects in packs marked .keep are not 
repacked' '
test -z $found_duplicate_object
 '
 
-test_expect_success '--pack-kept-objects duplicates objects' '
+test_expect_success 'writing bitmaps duplicates .keep objects' '
# build on $objsha1, $packsha1, and .keep state from previous
-   git repack -Adl --pack-kept-objects 
+   git repack -Adl 
test_when_finished found_duplicate_object= 
for p in .git/objects/pack/*.idx; do
idx=$(basename $p)
--
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] implemented strbuf_write_or_die()

[David Kastrup]

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

 As a potential GSoC student and newcomer to the project, Faiz would
 not have known that this would be considered unwanted churn when he
 chose the task from the GSoC microproject page [1]. Perhaps it would
 be a good idea to retire this item from the list?

 On the other hand, it did expose Faiz to the iterative code review
 process on this project and gave him a taste of what would be expected
 of him as a GSoC student, so the microproject achieved that important
 goal, and thus wasn't an utter failure.

And the microproject has the fabulous property that we can use it over
and over again to have a newcomer try committing patches: the previously
reported problem that we were running out of microprojects will not
occur when every patch is eventually going to be rejected.

Joking aside, this is a motivational disaster.  It should be retired.

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

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

[Eric Sunshine]

On Mon, Mar 3, 2014 at 1:40 PM, Junio C Hamano gits...@pobox.com wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:

 -if (!starts_with(buf, author )) {
 +if (!skip_prefix(buf, author )) {

 If this is the only change, there is not much point, is there?  How does
 this help?  Perhaps there is some way to take advantage of the
 difference between starts_with() and skip_prefix() to simplify the rest
 of the function?

 I admit I lost track, but wasn't there a discussion to use
 starts_with/ends_with when appropriate (namely, the caller is
 absolutely not interested in what the remainder of the string is
 after skipping the prefix), moving away from skip_prefix()?  Isn't
 this change going in the wrong direction?

Yes, it would be going in the wrong direction if this was all there
was to it, but the particular GSoC microproject [1] which inspired
this (incomplete) submission expects that the potential student will
dig deeper and discover how skip_prefix() can be used to achieve
greater simplification in record_author_date() and in other places in
the same file.

[1]: https://github.com/git/git.github.io/blob/master/SoC-2014-Microprojects.md
--
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] branch: use skip_prefix

[Junio C Hamano]

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

 On Fri, Feb 28, 2014 at 12:04:19PM +0900, Brian Gesiak wrote:

 From: modocache modoca...@gmail.com

 Both your emailed patches have this, which is due to your author name
 not matching your sending identity. You probably want to set user.name,
 or if you already have (which it looks like you might have from your
 Signed-off-by), use git commit --amend --reset-author to update the
 author information.

 The install_branch_config function reimplemented the skip_prefix
 function inline. Use skip_prefix function instead for brevity.
 
 Signed-off-by: Brian Gesiak modoca...@gmail.com
 Reported-by: Michael Haggerty mhag...@alum.mit.edu

 It's a minor thing, but usually these footer lines try to follow a
 chronological order. So the report would come before the signoff (and a
 further signoff from the maintainer would go after yours).

 diff --git a/branch.c b/branch.c
 index 723a36b..e163f3c 100644
 --- a/branch.c
 +++ b/branch.c
 [...]

 The patch itself looks OK to me.

 -Peff

Thanks.  Queued and pushed out on 'pu' with fixups 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] repack: add `repack.honorpackkeep` config var

[Jeff King]

On Mon, Mar 03, 2014 at 11:51:06AM -0800, Junio C Hamano wrote:

  Yes. Do you need a re-roll from me? I think the last version I sent +
  the squash to tie the default to bitmap-writing makes the most sense.
 
 I have 9e20b390 (repack: add `repack.packKeptObjects` config var,
 2014-02-26); I do not recall I've squashed anything into it, though.
 
 Do you mean this one?
 
 Here's the interdiff for doing the fallback:
 [...]

Yes. Though I just noticed that the commit message needs updating if
that is squashed in. Here is the whole patch, with that update.

And I am dropping Vicent as the author, since I think there are now
literally zero lines of his left. ;)

-- 8 --
Subject: [PATCH] repack: add `repack.packKeptObjects` config var

The git-repack command always passes `--honor-pack-keep`
to pack-objects. This has traditionally been a good thing,
as we do not want to duplicate those objects in a new pack,
and we are not going to delete the old pack.

However, when bitmaps are in use, it is important for a full
repack to include all reachable objects, even if they may be
duplicated in a .keep pack. Otherwise, we cannot generate
the bitmaps, as the on-disk format requires the set of
objects in the pack to be fully closed.

Even if the repository does not generally have .keep files,
a simultaneous push could cause a race condition in which a
.keep file exists at the moment of a repack. The repack may
try to include those objects in one of two situations:

  1. The pushed .keep pack contains objects that were
 already in the repository (e.g., blobs due to a revert of
 an old commit).

  2. Receive-pack updates the refs, making the objects
 reachable, but before it removes the .keep file, the
 repack runs.

In either case, we may prefer to duplicate some objects in
the new, full pack, and let the next repack (after the .keep
file is cleaned up) take care of removing them.

This patch introduces both a command-line and config option
to disable the `--honor-pack-keep` option.  By default, it
is triggered when pack.writeBitmaps (or `--write-bitmap-index`
is turned on), but specifying it explicitly can override the
behavior (e.g., in cases where you prefer .keep files to
bitmaps, but only when they are present).

Note that this option just disables the pack-objects
behavior. We still leave packs with a .keep in place, as we
do not necessarily know that we have duplicated all of their
objects.

Signed-off-by: Jeff King p...@peff.net
---
 Documentation/config.txt |  7 +++
 Documentation/git-repack.txt |  8 
 builtin/repack.c | 13 -
 t/t7700-repack.sh| 18 +-
 4 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index becbade..a8ddc7f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2136,6 +2136,13 @@ repack.usedeltabaseoffset::
false and repack. Access from old Git versions over the
native protocol are unaffected by this option.
 
+repack.packKeptObjects::
+   If set to true, makes `git repack` act as if
+   `--pack-kept-objects` was passed. See linkgit:git-repack[1] for
+   details. Defaults to `false` normally, but `true` if a bitmap
+   index is being written (either via `--write-bitmap-index` or
+   `pack.writeBitmaps`).
+
 rerere.autoupdate::
When set to true, `git-rerere` updates the index with the
resulting contents after it cleanly resolves conflicts using
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 002cfd5..4786a78 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -117,6 +117,14 @@ other objects in that pack they already have locally.
must be able to refer to all reachable objects. This option
overrides the setting of `pack.writebitmaps`.
 
+--pack-kept-objects::
+   Include objects in `.keep` files when repacking.  Note that we
+   still do not delete `.keep` packs after `pack-objects` finishes.
+   This means that we may duplicate objects, but this makes the
+   option safe to use when there are concurrent pushes or fetches.
+   This option is generally only useful if you are writing bitmaps
+   with `-b` or `pack.writebitmaps`, as it ensures that the
+   bitmapped packfile has the necessary objects.
 
 Configuration
 -
diff --git a/builtin/repack.c b/builtin/repack.c
index 49f5857..6b0b62d 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -9,6 +9,7 @@
 #include argv-array.h
 
 static int delta_base_offset = 1;
+static int pack_kept_objects = -1;
 static char *packdir, *packtmp;
 
 static const char *const git_repack_usage[] = {
@@ -22,6 +23,10 @@ static int repack_config(const char *var, const char *value, 
void *cb)
delta_base_offset = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, 

Re: RFC GSoC idea: new git config features

[Junio C Hamano]

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

 Most callbacks would convert to a query system in a pretty
 straightforward way, but some that have side effects might be tricky.
 Converting them all may be too large for a GSoC project, but I think you
 could do it gradually:

   1. Convert the parser to read into an in-memory representation, but
  leave git_config() as a wrapper which iterates over it.

   2. Add query functions like config_string_get() above.

   3. Convert callbacks to query functions one by one.

   4. Eventually drop git_config().

 A GSoC project could take us partway through (3).

I actually discarded the read from these config files to preparsed
structure to memory, later to be consumed by repeated calls to the
git_config() callback functions, making the only difference from the
current scheme that the preparsed structure will be reset when there
is the new 'reset to the original' definition as obvious and
uninteresting.

This is one of these times that I find myself blessed with capable
others that can go beyond, building on top of such an idea that I
may have discarded without thinking it through, around me ;-)

Yes, the new abstraction like config_type_get() that can live
alongside the existing git_config() feeds callback chain
everything and gradually replace the latter, would be a good way
forward.  Given that we read configuration multiple times anyway for
different purposes, even without the new abstraction, the end result
might perform better if we read the files once and reused in later
calls to git_config().

Thanks.

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

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Eric Sunshine]

On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 git rebase -e XYZ is basically the same as

 EDITOR=sed -i '1s/pick XYZ/edit XYZ/' $@ \
 git rebase -i XYZ^

 In English, it prepares the todo list for you to edit only commit XYZ
 to save your time. The time saving is only significant when you edit a
 lot of commits separately.

Is it correct to single out only edit for special treatment? If
allowing edit on the command-line, then shouldn't command-line
reword also be supported? I, for one, often need to reword a commit
message (or two or three); far more frequently than I need to edit a
commit.

(This is a genuine question about perceived favoritism of edit, as
opposed to a request to further bloat the interface.)
--
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] submodule : Add --no-separate-git-dir option to add and update command.

[Henri GEIST]

Le lundi 03 mars 2014 à 17:45 +, Jens Lehmann a écrit :
 Am 03.03.2014 14:47, schrieb Henri GEIST:
  This new option prevent git submodule add|update to clone the missing
  submodules with the --separate-git-dir option.
  Then the submodule will be regular repository and their gitdir will not
  be placed in the superproject gitdir/modules directory.
 
 And what is your motivation for this? After all submodules containing
 a .git directory are second class citizens (because they can never be
 safely removed by regular git commands).


I recognize most people will prefer to have the .git directory separate.
And I do not intend to make this option the default.

My reasons are:

  - As it is not clearly stated in the doc that the gitdir is separate.
The first time I have copied one module to an USB key I had a big
surprise.

  - This will not change anything for people not using it.

  - I use an other patch which I plane to send later which enable multiple
level of superproject to add a gitlink to the same submodule.
And in this case the superproject containing the separate gitdir will be
arbitrary and depend on the processing order of the
'git submodule update --recursive' command.

  - I have written this for myself and have using it since 2012 and send it in
the hope it could be useful for someone else even if it is only a few
people. But if its not the case no problem I will keep using it for myself.


  Signed-off-by: Henri GEIST geist.he...@laposte.net
  ---
   Documentation/git-submodule.txt |   18 --
   git-submodule.sh|   22 --
   t/t7400-submodule-basic.sh  |   12 
   3 files changed, 48 insertions(+), 4 deletions(-)
  
  diff --git a/Documentation/git-submodule.txt 
  b/Documentation/git-submodule.txt
  index 21cb59a..303a475 100644
  --- a/Documentation/git-submodule.txt
  +++ b/Documentation/git-submodule.txt
  @@ -10,13 +10,14 @@ SYNOPSIS
   
   [verse]
   'git submodule' [--quiet] add [-b branch] [-f|--force] [--name name]
  - [--reference repository] [--depth depth] [--] repository 
  [path]
  + [--reference repository] [--depth depth] 
  [--no-separate-git-dir]
  + [--] repository [path]
   'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
   'git submodule' [--quiet] init [--] [path...]
   'git submodule' [--quiet] deinit [-f|--force] [--] path...
   'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
[-f|--force] [--rebase|--merge|--checkout] [--reference 
  repository]
  - [--depth depth] [--recursive] [--] [path...]
  + [--depth depth] [--recursive] [--no-separate-git-dir] [--] 
  [path...]
   'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) 
  n]
[commit] [--] [path...]
   'git submodule' [--quiet] foreach [--recursive] command
  @@ -107,6 +108,10 @@ is the superproject and submodule repositories will be 
  kept
   together in the same relative location, and only the
   superproject's URL needs to be provided: git-submodule will correctly
   locate the submodule using the relative URL in .gitmodules.
  ++
  +If `--no-separate-git-dir` is specified, missing submodules will be cloned
  +has normal git repository without the option `--separate-git-dir` pointing
  +to the modules directory of the superproject gitdir.
   
   status::
  Show the status of the submodules. This will print the SHA-1 of the
  @@ -185,6 +190,10 @@ If the submodule is not yet initialized, and you just 
  want to use the
   setting as stored in .gitmodules, you can automatically initialize the
   submodule with the `--init` option.
   +
  +If `--no-separate-git-dir` is specified, missing submodules will be cloned
  +has normal git repository without the option `--separate-git-dir` pointing
  +to the modules directory of the superproject gitdir.
  ++
   If `--recursive` is specified, this command will recurse into the
   registered submodules, and update any nested submodules within.
   +
  @@ -363,6 +372,11 @@ for linkgit:git-clone[1]'s `--reference` and 
  `--shared` options carefully.
  clone with a history truncated to the specified number of revisions.
  See linkgit:git-clone[1]
   
  +--no-separate-git-dir::
  +   This option is valid for add and update commands. Specify that missing
  +   submodules should be clonned as self contain repository without a
  +   separate gitdir placed in the modules directory of the superproject
  +   gitdir.
   
   path...::
  Paths to submodule(s). When specified this will restrict the command
  diff --git a/git-submodule.sh b/git-submodule.sh
  index a33f68d..36eaf31 100755
  --- a/git-submodule.sh
  +++ b/git-submodule.sh
  @@ -5,11 +5,11 @@
   # Copyright (c) 2007 Lars Hjemli
   
   dashless=$(basename $0 | sed -e 's/-/ /')
  -USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] 
  [--reference repository] [--] 

Re: [PATCH] implemented strbuf_write_or_die()

[Junio C Hamano]

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

 On Mon, Mar 3, 2014 at 1:31 PM, Junio C Hamano gits...@pobox.com wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:

 On Sat, Mar 1, 2014 at 7:51 AM, He Sun sunheeh...@gmail.com wrote:
 2014-03-01 19:21 GMT+08:00 Faiz Kothari faiz.of...@gmail.com:
 diff --git a/remote-curl.c b/remote-curl.c
 index 10cb011..dee8716 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -634,7 +634,7 @@ static int rpc_service(struct rpc_state *rpc, struct 
 discovery *heads)
 if (start_command(client))
 exit(1);
 if (preamble)
 -   write_or_die(client.in, preamble-buf, preamble-len);
 +   strbuf_write_or_die(client.in, preamble);
 if (heads)
 write_or_die(client.in, heads-buf, heads-len);

 This should be changed. May be you can use Ctrl-F to search write_or_die().
 Or if you are using vim, use / and n to find all.

 It's not obvious from the patch fragment, but 'heads' is not a strbuf,
 so Faiz correctly left this invocation alone.

 That is a very good sign why this change is merely a code-churn and
 not an improvement, isn't it?  We know (and any strbuf user should
 know) that -buf and -len are the ways to learn the pointer and the
 length the strbuf holds.  Why anybody thinks it is benefitial to
 introduce another function that is _only_ for writing out strbuf and
 cannot be used to write out a plain buffer is simply beyond me.

 As a potential GSoC student and newcomer to the project, Faiz would
 not have known that this would be considered unwanted churn when he
 chose the task from the GSoC microproject page [1]. Perhaps it would
 be a good idea to retire this item from the list?

I don't think I saw this on the microproject suggestion page when I
last looked at it, and assumed that this was on the student's own
initiative.

 On the other hand, it did expose Faiz to the iterative code review
 process on this project and gave him a taste of what would be expected
 of him as a GSoC student, so the microproject achieved that important
 goal, and thus wasn't an utter failure.

 [1]: 
 https://github.com/git/git.github.io/blob/master/SoC-2014-Microprojects.md

Surely.

I would have to say that this is not a good sample exercise to
suggest to new students and I'd encourage dropping it from the list.
You could argue that it is an effective way to cull people with bad
design taste to mix suggestions to make the codebase worse and see
who picks them, but I do not think it is very fair ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] git-config: document interactive.singlekey requires Term::ReadKey

[Simon Ruderich]

Most distributions don't require Term::ReadKey as dependency, leaving
the user to wonder why the setting doesn't work.

Signed-off-by: Simon Ruderich si...@ruderich.org
---

On Mon, Mar 03, 2014 at 10:58:58AM -0800, Junio C Hamano wrote:
 Thanks, but is it true that interactive.singlekey requries
 Term::ReadKey?

Yes, it requires it. The code also works fine without
Term::ReadKey, but the feature singlekey requires this module.
I assumed a user enabling this option would also want to use the
feature, therefore requires is fine IMHO.

 The implementation of prompt_single_character sub wants to use
 ReadKey, but can still let the user interact with the program by
 falling back to a cooked input when it is not available, so perhaps
 a better fix might be something like this:

 if (!$use_readkey) {
   print STDERR missing Term::ReadKey, disabling 
 interactive.singlekey\n;
 }

 inside the above if() that prepares $use_readkey?

Good idea. Implemented in an additional patch.

I think the documentation should also be updated (this patch) to
make it clear to a reader of the man page, that an additional
module is required, without having him to try to use the option.

 You also misspelled the package name it seems ;-)

Oops, sorry. Fixed in this reroll.

Regards
Simon

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

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f4d793..406a582 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1633,7 +1633,7 @@ interactive.singlekey::
linkgit:git-add[1], linkgit:git-checkout[1], linkgit:git-commit[1],
linkgit:git-reset[1], and linkgit:git-stash[1]. Note that this
setting is silently ignored if portable keystroke input
-   is not available.
+   is not available; requires the Perl module Term::ReadKey.
 
 log.abbrevCommit::
If true, makes linkgit:git-log[1], linkgit:git-show[1], and
-- 
1.9.0.11.g9a08b42

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

[PATCH] git-add--interactive: warn if module for interactive.singlekey is missing

[Simon Ruderich]

Suggested-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Simon Ruderich si...@ruderich.org
---
 git-add--interactive.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 24bb1ab..d3bca12 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -58,6 +58,9 @@ if ($repo-config_bool(interactive.singlekey)) {
Term::ReadKey-import;
$use_readkey = 1;
};
+   if (!$use_readkey) {
+   print STDERR missing Term::ReadKey, disabling 
interactive.singlekey\n;
+   }
eval {
require Term::Cap;
my $termcap = Term::Cap-Tgetent;
-- 
1.9.0.11.g9a08b42

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

Help needed: Tests failed While replacing char array with strbuf in bulk-checkin.c

[saikrishna.sripada]

Hi ,
I am trying do complete the microproject 4, inorder to apply to GSOC.
I have made the below changes:

https://gist.github.com/anhsirksai/9334565

Post my changes compilation is succes in the source directory.
But when I ran the tests[make in t/ directory] my tests are failing 
saying



 free(): invalid pointer: 0x3630376532353636 ***
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x2b5f3b540b96]
/home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x4fb829]
/home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x47d425]
/home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x4064ad]
/home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x405a04]
/home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x404cbd]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x2b5f3b4e376d]
/home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x405109]


Can some one please help me with the memory allacation and 
strbuf_release()


Thanks,
--sai krishna

--
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] implemented strbuf_write_or_die()

[Eric Sunshine]

On Mon, Mar 3, 2014 at 3:35 PM, Junio C Hamano gits...@pobox.com wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:
 On Mon, Mar 3, 2014 at 1:31 PM, Junio C Hamano gits...@pobox.com wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:
 It's not obvious from the patch fragment, but 'heads' is not a strbuf,
 so Faiz correctly left this invocation alone.

 That is a very good sign why this change is merely a code-churn and
 not an improvement, isn't it?  We know (and any strbuf user should
 know) that -buf and -len are the ways to learn the pointer and the
 length the strbuf holds.  Why anybody thinks it is benefitial to
 introduce another function that is _only_ for writing out strbuf and
 cannot be used to write out a plain buffer is simply beyond me.

 As a potential GSoC student and newcomer to the project, Faiz would
 not have known that this would be considered unwanted churn when he
 chose the task from the GSoC microproject page [1]. Perhaps it would
 be a good idea to retire this item from the list?

 I don't think I saw this on the microproject suggestion page when I
 last looked at it, and assumed that this was on the student's own
 initiative.

I also had not seen it earlier on the microprojects page and had the
same reaction until I re-checked the page and found that it had been
added [1].

The microprojects page already instructs students to indicate that a
submission is for GSoC [2] (and many have followed the advice), but
perhaps we can avoid this sort of misunderstanding in the future by
making it more explicit: for instance, tell them to add [GSoC] to the
Subject:.

[1]: 
https://github.com/git/git.github.io/commit/f314120a2b5e831459673c612a3630ad953d9954
[2]: 
https://github.com/git/git.github.io/blame/master/SoC-2014-Microprojects.md#L83

 On the other hand, it did expose Faiz to the iterative code review
 process on this project and gave him a taste of what would be expected
 of him as a GSoC student, so the microproject achieved that important
 goal, and thus wasn't an utter failure.

 [1]: 
 https://github.com/git/git.github.io/blob/master/SoC-2014-Microprojects.md

 Surely.

 I would have to say that this is not a good sample exercise to
 suggest to new students and I'd encourage dropping it from the list.
 You could argue that it is an effective way to cull people with bad
 design taste to mix suggestions to make the codebase worse and see
 who picks them, but I do not think it is very fair ;-)

Agreed. The item should be dropped from the list.
--
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/3] rebase: accept -number as another way of saying HEAD~number

[Junio C Hamano]

Michael Haggerty mhag...@alum.mit.edu writes:

 This might be a reason that -NUM is a bad idea.

 Or perhaps -NUM should fail with an error message if any of the last
 NUM commits are merges.  In that restricted scenario (which probably
 accounts for 99% of rebases), -NUM is equivalent to HEAD~NUM.

That sounds like one possible way out; the opposite would be to
enable mode that preserges merges when we see any.

If rebase had a --first-parent mode that simply replays only the
commits on the first parent chain, merging the same second and later
parents without looking at their history when dealing with merge
commits, and always used that mode unless --flatten-history was
given, the world might have been a better place.  We cannot go there
in a single step, but we could plan such a backward incompatible
migration if we wanted to.
--
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/3] rebase: accept -number as another way of saying HEAD~number

[Junio C Hamano]

Matthieu Moy matthieu@grenoble-inp.fr writes:

 Michael Haggerty mhag...@alum.mit.edu writes:

 Or perhaps -NUM should fail with an error message if any of the last
 NUM commits are merges.  In that restricted scenario (which probably
 accounts for 99% of rebases), -NUM is equivalent to HEAD~NUM.

 Makes sense to me. So, -NUM would actually mean rebase the last NUM
 commits (as well as being an alias for HEAD~NUM), but would fail when
 it does not make sense (with an error message explaining the situation
 and pointing the user to HEAD~N if this is what he wanted).

 This would actually be a feature for me: I often want to rebase recent
 enough history, and when my @{upstream} isn't well positionned,...

Could you elaborate on this a bit?  What does isn't well
positioned mean?  Do you mean the upstream has advanced but there
is no reason for my topic to build on that---I'd rather want to make
sure I can view 'diff @{1} HEAD' and understand what my changes
before the rebase was?  That is, what you really want is

git rebase -i --onto $(git merge-base @{upstream} HEAD) @{upstream}

but that is too long to type?

If it is very common (and I suspect it is), we may want to support
such a short-hand---the above does not make any sense without '-i',
but I would say with '-i' you do not want to reBASE on an updated
base most of the time.  git rebase -i @{upstream}...HEAD or
something?
--
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: Help needed: Tests failed While replacing char array with strbuf in bulk-checkin.c

[Eric Sunshine]

On Mon, Mar 3, 2014 at 4:11 PM, saikrishna.sripada
saikrishna.srip...@students.iiit.ac.in wrote:
 I am trying do complete the microproject 4, inorder to apply to GSOC.
 I have made the below changes:

 https://gist.github.com/anhsirksai/9334565

 Post my changes compilation is succes in the source directory.
 But when I ran the tests[make in t/ directory] my tests are failing saying

 
  free(): invalid pointer: 0x3630376532353636 ***
 === Backtrace: =
 /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x2b5f3b540b96]
 /home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x4fb829]
 /home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x47d425]
 /home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x4064ad]
 /home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x405a04]
 /home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x404cbd]
 /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x2b5f3b4e376d]
 /home/saikrishna/Desktop/libcloud-0.14.1/sai/git/git[0x405109]
 

 Can some one please help me with the memory allacation and strbuf_release()

Read the microproject text carefully and _fully_. It provides the clue
you need to understand the problem.

Rewrite bulk-checkin.c:finish_bulk_checkin() to use a strbuf for
handling packname, and explain why this is useful. Also check if
the first argument of pack-write.c:finish_tmp_packfile() can be
made const.

If, after making a closer examination of the mentioned functions, the
problem still eludes you, ask here again.
--
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 00/14] Use ALLOC_GROW() instead of inline code

[Dmitry S. Dolzhenko]

This version differs from previous [1] the following changes:
  - added three new commits with similar changes in builtin/mktree.c,
cache-tree.c and sha1_file.c.
  - updated commit messages: use ALLOC_GROW() in function_name() instead of
change function_name() to use ALLOC_GROW()
  - updated [PATCH v2 01/11] [2] to keep code lines within 80 columns in 
builtin/pack-objects.c

Duy Nguyen, Michael Haggerty, Junio C Hamano, Eric Sunshine, and He Sun, 
thanks you very much for your remarks and advices

[1] http://thread.gmane.org/gmane.comp.version-control.git/242919
[2] http://thread.gmane.org/gmane.comp.version-control.git/242920

Dmitry S. Dolzhenko (14):
  builtin/pack-objects.c: use ALLOC_GROW() in check_pbase_path()
  bundle.c: use ALLOC_GROW() in add_to_ref_list()
  cache-tree.c: use ALLOC_GROW() in find_subtree()
  commit.c: use ALLOC_GROW() in register_commit_graft()
  diff.c: use ALLOC_GROW()
  diffcore-rename.c: use ALLOC_GROW()
  patch-ids.c: use ALLOC_GROW() in add_commit()
  replace_object.c: use ALLOC_GROW() in register_replace_object()
  reflog-walk.c: use ALLOC_GROW()
  dir.c: use ALLOC_GROW() in create_simplify()
  attr.c: use ALLOC_GROW() in handle_attr_line()
  builtin/mktree.c: use ALLOC_GROW() in append_to_tree()
  read-cache.c: use ALLOC_GROW() in add_index_entry()
  sha1_file.c: use ALLOC_GROW() in pretend_sha1_file()

 attr.c |  7 +--
 builtin/mktree.c   |  5 +
 builtin/pack-objects.c |  9 +++--
 bundle.c   |  6 +-
 cache-tree.c   |  6 +-
 commit.c   |  8 ++--
 diff.c | 12 ++--
 diffcore-rename.c  | 12 ++--
 dir.c  |  5 +
 patch-ids.c|  5 +
 read-cache.c   |  6 +-
 reflog-walk.c  | 12 ++--
 replace_object.c   |  8 ++--
 sha1_file.c|  7 +--
 14 files changed, 21 insertions(+), 87 deletions(-)

-- 
1.8.3.2

--
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 08/14] replace_object.c: use ALLOC_GROW() in register_replace_object()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 replace_object.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/replace_object.c b/replace_object.c
index cdcaf8c..843deef 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -36,12 +36,8 @@ static int register_replace_object(struct replace_object 
*replace,
return 1;
}
pos = -pos - 1;
-   if (replace_object_alloc = ++replace_object_nr) {
-   replace_object_alloc = alloc_nr(replace_object_alloc);
-   replace_object = xrealloc(replace_object,
- sizeof(*replace_object) *
- replace_object_alloc);
-   }
+   ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);
+   replace_object_nr++;
if (pos  replace_object_nr)
memmove(replace_object + pos + 1,
replace_object + pos,
-- 
1.8.3.2

--
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 10/14] dir.c: use ALLOC_GROW() in create_simplify()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 dir.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/dir.c b/dir.c
index 98bb50f..4ae38e4 100644
--- a/dir.c
+++ b/dir.c
@@ -1341,10 +1341,7 @@ static struct path_simplify *create_simplify(const char 
**pathspec)
 
for (nr = 0 ; ; nr++) {
const char *match;
-   if (nr = alloc) {
-   alloc = alloc_nr(alloc);
-   simplify = xrealloc(simplify, alloc * 
sizeof(*simplify));
-   }
+   ALLOC_GROW(simplify, nr + 1, alloc);
match = *pathspec++;
if (!match)
break;
-- 
1.8.3.2

--
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 03/14] cache-tree.c: use ALLOC_GROW() in find_subtree()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 cache-tree.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 0bbec43..30149d1 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -75,11 +75,7 @@ static struct cache_tree_sub *find_subtree(struct cache_tree 
*it,
return NULL;
 
pos = -pos-1;
-   if (it-subtree_alloc = it-subtree_nr) {
-   it-subtree_alloc = alloc_nr(it-subtree_alloc);
-   it-down = xrealloc(it-down, it-subtree_alloc *
-   sizeof(*it-down));
-   }
+   ALLOC_GROW(it-down, it-subtree_nr + 1, it-subtree_alloc);
it-subtree_nr++;
 
down = xmalloc(sizeof(*down) + pathlen + 1);
-- 
1.8.3.2

--
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 11/14] attr.c: use ALLOC_GROW() in handle_attr_line()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 attr.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/attr.c b/attr.c
index 8d13d70..734222d 100644
--- a/attr.c
+++ b/attr.c
@@ -338,12 +338,7 @@ static void handle_attr_line(struct attr_stack *res,
a = parse_attr_line(line, src, lineno, macro_ok);
if (!a)
return;
-   if (res-alloc = res-num_matches) {
-   res-alloc = alloc_nr(res-num_matches);
-   res-attrs = xrealloc(res-attrs,
- sizeof(struct match_attr *) *
- res-alloc);
-   }
+   ALLOC_GROW(res-attrs, res-num_matches + 1, res-alloc);
res-attrs[res-num_matches++] = a;
 }
 
-- 
1.8.3.2

--
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 06/14] diffcore-rename.c: use ALLOC_GROW()

[Dmitry S. Dolzhenko]

Use ALLOC_GROW() instead inline code in
locate_rename_dst() and register_rename_src()

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 diffcore-rename.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/diffcore-rename.c b/diffcore-rename.c
index 9b4f068..fbf3272 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -38,11 +38,7 @@ static struct diff_rename_dst *locate_rename_dst(struct 
diff_filespec *two,
if (!insert_ok)
return NULL;
/* insert to make it at first */
-   if (rename_dst_alloc = rename_dst_nr) {
-   rename_dst_alloc = alloc_nr(rename_dst_alloc);
-   rename_dst = xrealloc(rename_dst,
- rename_dst_alloc * sizeof(*rename_dst));
-   }
+   ALLOC_GROW(rename_dst, rename_dst_nr + 1, rename_dst_alloc);
rename_dst_nr++;
if (first  rename_dst_nr)
memmove(rename_dst + first + 1, rename_dst + first,
@@ -82,11 +78,7 @@ static struct diff_rename_src *register_rename_src(struct 
diff_filepair *p)
}
 
/* insert to make it at first */
-   if (rename_src_alloc = rename_src_nr) {
-   rename_src_alloc = alloc_nr(rename_src_alloc);
-   rename_src = xrealloc(rename_src,
- rename_src_alloc * sizeof(*rename_src));
-   }
+   ALLOC_GROW(rename_src, rename_src_nr + 1, rename_src_alloc);
rename_src_nr++;
if (first  rename_src_nr)
memmove(rename_src + first + 1, rename_src + first,
-- 
1.8.3.2

--
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 05/14] diff.c: use ALLOC_GROW()

[Dmitry S. Dolzhenko]

Use ALLOC_GROW() instead inline code in
diffstat_add() and diff_q()

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 diff.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/diff.c b/diff.c
index e800666..aebdfda 100644
--- a/diff.c
+++ b/diff.c
@@ -1361,11 +1361,7 @@ static struct diffstat_file *diffstat_add(struct 
diffstat_t *diffstat,
 {
struct diffstat_file *x;
x = xcalloc(sizeof (*x), 1);
-   if (diffstat-nr == diffstat-alloc) {
-   diffstat-alloc = alloc_nr(diffstat-alloc);
-   diffstat-files = xrealloc(diffstat-files,
-   diffstat-alloc * sizeof(x));
-   }
+   ALLOC_GROW(diffstat-files, diffstat-nr + 1, diffstat-alloc);
diffstat-files[diffstat-nr++] = x;
if (name_b) {
x-from_name = xstrdup(name_a);
@@ -3965,11 +3961,7 @@ struct diff_queue_struct diff_queued_diff;
 
 void diff_q(struct diff_queue_struct *queue, struct diff_filepair *dp)
 {
-   if (queue-alloc = queue-nr) {
-   queue-alloc = alloc_nr(queue-alloc);
-   queue-queue = xrealloc(queue-queue,
-   sizeof(dp) * queue-alloc);
-   }
+   ALLOC_GROW(queue-queue, queue-nr + 1, queue-alloc);
queue-queue[queue-nr++] = dp;
 }
 
-- 
1.8.3.2

--
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 02/14] bundle.c: use ALLOC_GROW() in add_to_ref_list()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 bundle.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/bundle.c b/bundle.c
index e99065c..1388a3e 100644
--- a/bundle.c
+++ b/bundle.c
@@ -14,11 +14,7 @@ static const char bundle_signature[] = # v2 git bundle\n;
 static void add_to_ref_list(const unsigned char *sha1, const char *name,
struct ref_list *list)
 {
-   if (list-nr + 1 = list-alloc) {
-   list-alloc = alloc_nr(list-nr + 1);
-   list-list = xrealloc(list-list,
-   list-alloc * sizeof(list-list[0]));
-   }
+   ALLOC_GROW(list-list, list-nr + 1, list-alloc);
memcpy(list-list[list-nr].sha1, sha1, 20);
list-list[list-nr].name = xstrdup(name);
list-nr++;
-- 
1.8.3.2

--
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 01/14] builtin/pack-objects.c: use ALLOC_GROW() in check_pbase_path()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 builtin/pack-objects.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..0ffad6f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1213,12 +1213,9 @@ static int check_pbase_path(unsigned hash)
if (0 = pos)
return 1;
pos = -pos - 1;
-   if (done_pbase_paths_alloc = done_pbase_paths_num) {
-   done_pbase_paths_alloc = alloc_nr(done_pbase_paths_alloc);
-   done_pbase_paths = xrealloc(done_pbase_paths,
-   done_pbase_paths_alloc *
-   sizeof(unsigned));
-   }
+   ALLOC_GROW(done_pbase_paths,
+  done_pbase_paths_num + 1,
+  done_pbase_paths_alloc);
done_pbase_paths_num++;
if (pos  done_pbase_paths_num)
memmove(done_pbase_paths + pos + 1,
-- 
1.8.3.2

--
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 14/14] sha1_file.c: use ALLOC_GROW() in pretend_sha1_file()

[Dmitry S. Dolzhenko]

Helped-by: He Sun sunheeh...@gmail.com
Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 sha1_file.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 019628a..3cb17b8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2624,12 +2624,7 @@ int pretend_sha1_file(void *buf, unsigned long len, enum 
object_type type,
hash_sha1_file(buf, len, typename(type), sha1);
if (has_sha1_file(sha1) || find_cached_object(sha1))
return 0;
-   if (cached_object_alloc = cached_object_nr) {
-   cached_object_alloc = alloc_nr(cached_object_alloc);
-   cached_objects = xrealloc(cached_objects,
- sizeof(*cached_objects) *
- cached_object_alloc);
-   }
+   ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
co = cached_objects[cached_object_nr++];
co-size = len;
co-type = type;
-- 
1.8.3.2

--
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 04/14] commit.c: use ALLOC_GROW() in register_commit_graft()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 commit.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/commit.c b/commit.c
index 6bf4fe0..e004314 100644
--- a/commit.c
+++ b/commit.c
@@ -147,12 +147,8 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
return 1;
}
pos = -pos - 1;
-   if (commit_graft_alloc = ++commit_graft_nr) {
-   commit_graft_alloc = alloc_nr(commit_graft_alloc);
-   commit_graft = xrealloc(commit_graft,
-   sizeof(*commit_graft) *
-   commit_graft_alloc);
-   }
+   ALLOC_GROW(commit_graft, commit_graft_nr + 1, commit_graft_alloc);
+   commit_graft_nr++;
if (pos  commit_graft_nr)
memmove(commit_graft + pos + 1,
commit_graft + pos,
-- 
1.8.3.2

--
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 13/14] read-cache.c: use ALLOC_GROW() in add_index_entry()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 read-cache.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index fb440b4..cbdf954 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -990,11 +990,7 @@ int add_index_entry(struct index_state *istate, struct 
cache_entry *ce, int opti
}
 
/* Make sure the array is big enough .. */
-   if (istate-cache_nr == istate-cache_alloc) {
-   istate-cache_alloc = alloc_nr(istate-cache_alloc);
-   istate-cache = xrealloc(istate-cache,
-   istate-cache_alloc * 
sizeof(*istate-cache));
-   }
+   ALLOC_GROW(istate-cache, istate-cache_nr + 1, istate-cache_alloc);
 
/* Add it in.. */
istate-cache_nr++;
-- 
1.8.3.2

--
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 07/14] patch-ids.c: use ALLOC_GROW() in add_commit()

[Dmitry S. Dolzhenko]

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 patch-ids.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/patch-ids.c b/patch-ids.c
index bc8a28f..bf81b92 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -83,10 +83,7 @@ static struct patch_id *add_commit(struct commit *commit,
ent = bucket-bucket[bucket-nr++];
hashcpy(ent-patch_id, sha1);
 
-   if (ids-alloc = ids-nr) {
-   ids-alloc = alloc_nr(ids-nr);
-   ids-table = xrealloc(ids-table, sizeof(ent) * ids-alloc);
-   }
+   ALLOC_GROW(ids-table, ids-nr + 1, ids-alloc);
if (pos  ids-nr)
memmove(ids-table + pos + 1, ids-table + pos,
sizeof(ent) * (ids-nr - pos));
-- 
1.8.3.2

--
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/3] rebase: accept -number as another way of saying HEAD~number

[Matthieu Moy]

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

 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Michael Haggerty mhag...@alum.mit.edu writes:

 Or perhaps -NUM should fail with an error message if any of the last
 NUM commits are merges.  In that restricted scenario (which probably
 accounts for 99% of rebases), -NUM is equivalent to HEAD~NUM.

 Makes sense to me. So, -NUM would actually mean rebase the last NUM
 commits (as well as being an alias for HEAD~NUM), but would fail when
 it does not make sense (with an error message explaining the situation
 and pointing the user to HEAD~N if this is what he wanted).

 This would actually be a feature for me: I often want to rebase recent
 enough history, and when my @{upstream} isn't well positionned,...

 Could you elaborate on this a bit?  What does isn't well
 positioned mean?

The most common case is when @{upstream} is not positionned at all,
because I'm on a temporary branch on which I did not configure it. The
other case is when I did a manual merge of a branch other than upstream.

As I said in another message, there would probably be cleaner solutions,
but the trial and error one does not work that bad.

-- 
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: My advice for GSoC applicants

[Junio C Hamano]

Michael Haggerty mhag...@alum.mit.edu writes:

 Based on my experience so far as a first-time Google Summer of Code
 mentor, I just wrote a blog article containing some hopefully useful
 advice for students applying to the program.  Please note that this is
 my personal opinion only and doesn't necessarily reflect the views of
 the Git/libgit2 projects as a whole.

 My secret tip for GSoC success

 http://softwareswirl.blogspot.com/2014/03/my-secret-tip-for-gsoc-success.html

Thanks for writing this.

Also thanks for the MicroProject approach to introduce potential
students and the community.

Multiple students seem to be hitting the same microprojects (aka we
are running out of micros), which might be a bit unfortunate.  I
think the original plan might have been that for a student candidate
to pass, his-or-her patch must hit my tree and queued somewhere, but
with these duplicates I do not think it is fair to disqualify those
who interacted with reviewers well but solved an already solved
micro.

Even with the duplicates I think we are learning how well each
student respond to reviews (better ones even seem to pick up lessons
from reviews on others' threads that tackle micros different from
their own) and what his-or-her general cognitive ability is.
--
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: [GSoC14][RFC] Proposal Draft: Refactor tempfile handling

[Jeff King]

On Sun, Mar 02, 2014 at 06:04:39AM +0900, Brian Gesiak wrote:

 My name is Brian Gesiak. I'm a research student at the University of
 Tokyo, and I'm hoping to participate in this year's Google Summer of
 Code by contributing to Git. I'm a longtime user, first-time
 contributor--some of you may have noticed my microproject
 patches.[1][2]

Yes, we did notice them. Thanks, and welcome. :)

 The ideas page points out that while lock files are closed and
 unlinked[3] when the program exits[4], object pack files implement
 their own brand of temp file creation and deletion. This
 implementation doesn't share the same guarantees as lock files--it is
 possible that the program terminates before the temp file is
 unlinked.[5]
 
 Lock file references are stored in a linked list. When the program
 exits, this list is traversed and each file is closed and unlinked. It
 seems to me that this mechanism is appropriate for temp files in
 general, not just lock files. Thus, my proposal would be to extract
 this logic into a separate module--tempfile.h, perhaps. Lock and
 object files would share the tempfile implementation.
 
 That is, both object and lock temp files would be stored in a linked
 list, and all of these would be deleted at program exit.

Yes, I think this is definitely the right way to go. We should be able
to unify the tempfile handling for all of git. Once the logic is
extracted into a nice API, there are several other places that can use
it, too:

  - the external diff code creates tempfiles and uses its own cleanup
routines

  - the shallow_XX tempfiles (these are not cleaned right now,
though I sent a patch recently for them to do their own cleanup)

Those are just off the top of my head. There may be other spots, too.

It is worth thinking in your proposal about some of the things that the
API will want to handle. What are the mismatches in how lockfiles and
object files are handled? E.g., how do we finalize them into place?
How should the API be designed to minimize race conditions (e.g., if we
get a signal delivered while we are committing or cleaning up a file)?

 Please let me know if this seems like it would make for an interesting
 proposal, or if perhaps there is something I am overlooking. Any
 feedback at all would be appreciated. Thank you!

You definitely have a grasp of what the project is aiming for, and which
areas need to be touched.

-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 v4 09/14] reflog-walk.c: use ALLOC_GROW()

[Dmitry S. Dolzhenko]

Use ALLOC_GROW() instead inline code in
add_commit_info() and read_one_reflog()

Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 reflog-walk.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index b2fbdb2..2899729 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -26,11 +26,7 @@ static int read_one_reflog(unsigned char *osha1, unsigned 
char *nsha1,
struct complete_reflogs *array = cb_data;
struct reflog_info *item;
 
-   if (array-nr = array-alloc) {
-   array-alloc = alloc_nr(array-nr + 1);
-   array-items = xrealloc(array-items, array-alloc *
-   sizeof(struct reflog_info));
-   }
+   ALLOC_GROW(array-items, array-nr + 1, array-alloc);
item = array-items + array-nr;
memcpy(item-osha1, osha1, 20);
memcpy(item-nsha1, nsha1, 20);
@@ -114,11 +110,7 @@ static void add_commit_info(struct commit *commit, void 
*util,
struct commit_info_lifo *lifo)
 {
struct commit_info *info;
-   if (lifo-nr = lifo-alloc) {
-   lifo-alloc = alloc_nr(lifo-nr + 1);
-   lifo-items = xrealloc(lifo-items,
-   lifo-alloc * sizeof(struct commit_info));
-   }
+   ALLOC_GROW(lifo-items, lifo-nr + 1, lifo-alloc);
info = lifo-items + lifo-nr;
info-commit = commit;
info-util = util;
-- 
1.8.3.2

--
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 12/14] builtin/mktree.c: use ALLOC_GROW() in append_to_tree()

[Dmitry S. Dolzhenko]

Helped-by: He Sun sunheeh...@gmail.com
Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
---
 builtin/mktree.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/builtin/mktree.c b/builtin/mktree.c
index f92ba40..a964d6b 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -23,10 +23,7 @@ static void append_to_tree(unsigned mode, unsigned char 
*sha1, char *path)
if (strchr(path, '/'))
die(path %s contains slash, path);
 
-   if (alloc = used) {
-   alloc = alloc_nr(used);
-   entries = xrealloc(entries, sizeof(*entries) * alloc);
-   }
+   ALLOC_GROW(entries, used + 1, alloc);
ent = entries[used++] = xmalloc(sizeof(**entries) + len + 1);
ent-mode = mode;
ent-len = len;
-- 
1.8.3.2

--
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] skip_prefix: rewrite so that prefix is scanned once

[Junio C Hamano]

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

 Siddharth Goel siddharth98...@gmail.com writes:

 Helped-by: Eric Sunshine sunsh...@sunshineco.com
 Signed-off-by: Siddharth Goel siddharth98...@gmail.com
 ---
 Added a space after colon in the subject as compared to previous 
 patch [PATCH v2].

 [PATCH v2]: http://thread.gmane.org/gmane.comp.version-control.git/243150

 Whenever you see Change, Rewrite, etc. in the subject of a patch
 that touches existing code, think twice.  The subject line is a
 scarce real estate to be wasted on a noiseword that carries no real
 information, and we already know a patch that touches existing code
 changes or rewrites things.

 Subject: [PATCH v3] skip_prefix: scan prefix only once

 perhaps?

  git-compat-util.h | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

 diff --git a/git-compat-util.h b/git-compat-util.h
 index 614a5e9..550dce3 100644
 --- a/git-compat-util.h
 +++ b/git-compat-util.h
 @@ -357,8 +357,11 @@ extern int suffixcmp(const char *str, const char 
 *suffix);
  
  static inline const char *skip_prefix(const char *str, const char *prefix)
  {
 -size_t len = strlen(prefix);
 -return strncmp(str, prefix, len) ? NULL : str + len;
 +while (*prefix != '\0'  *str == *prefix) {
 +str++;
 +prefix++;
 +}
 +return (*prefix == '\0' ? str : NULL);

 Unlike another patch I saw the other day on the same topic, this
 checks *prefix twice for the last round, even though I think this
 one is probably slightly easier to read.  I dunno.

That is, something like this instead.  After looking at it again, I
do not think it is less readable than the above.

 git-compat-util.h | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index cbd86c3..68ffaef 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -357,8 +357,14 @@ extern int suffixcmp(const char *str, const char *suffix);
 
 static inline const char *skip_prefix(const char *str, const char *prefix)
 {
-   size_t len = strlen(prefix);
-   return strncmp(str, prefix, len) ? NULL : str + len;
+   while (1) {
+   if (!*prefix)
+   return str;
+   if (*str != *prefix)
+   return NULL;
+   prefix++;
+   str++;
+   }
 }
 
 #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
--
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 05/14] diff.c: use ALLOC_GROW()

[Junio C Hamano]

Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru writes:

 Use ALLOC_GROW() instead inline code in
 diffstat_add() and diff_q()

...instead of open coding it in... may read better.


 Signed-off-by: Dmitry S. Dolzhenko dmitrys.dolzhe...@yandex.ru
 ---
  diff.c | 12 ++--
  1 file changed, 2 insertions(+), 10 deletions(-)

 diff --git a/diff.c b/diff.c
 index e800666..aebdfda 100644
 --- a/diff.c
 +++ b/diff.c
 @@ -1361,11 +1361,7 @@ static struct diffstat_file *diffstat_add(struct 
 diffstat_t *diffstat,
  {
   struct diffstat_file *x;
   x = xcalloc(sizeof (*x), 1);
 - if (diffstat-nr == diffstat-alloc) {
 - diffstat-alloc = alloc_nr(diffstat-alloc);
 - diffstat-files = xrealloc(diffstat-files,
 - diffstat-alloc * sizeof(x));
 - }
 + ALLOC_GROW(diffstat-files, diffstat-nr + 1, diffstat-alloc);
   diffstat-files[diffstat-nr++] = x;
   if (name_b) {
   x-from_name = xstrdup(name_a);
 @@ -3965,11 +3961,7 @@ struct diff_queue_struct diff_queued_diff;
  
  void diff_q(struct diff_queue_struct *queue, struct diff_filepair *dp)
  {
 - if (queue-alloc = queue-nr) {
 - queue-alloc = alloc_nr(queue-alloc);
 - queue-queue = xrealloc(queue-queue,
 - sizeof(dp) * queue-alloc);
 - }
 + ALLOC_GROW(queue-queue, queue-nr + 1, queue-alloc);
   queue-queue[queue-nr++] = dp;
  }
--
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: [BUG] Halt during fetch on MacOS

[Conley Owens]

On Sun, Mar 2, 2014 at 6:36 AM, Max Horn m...@quendi.de wrote:

 On 01.03.2014, at 00:26, Conley Owens c...@android.com wrote:

 $ git --version  # This is just the git from MacPorts
 git version 1.8.5.5
 $ sw_vers
 ProductName:Mac OS X
 ProductVersion: 10.8.5
 BuildVersion:   12F45

 I cannot reproduce this, neither with 1.8.5.5 nor with 1.9.0. I am also 
 running Mac OS X 10.8.5.

 Note: I tried this both with you original test.sh, and also with a version 
 were I replaced the  by , as per Jeff's suggestion. It (as predicted) 
 didn't make any difference).

 If this is a race condition, it might be easier to trigger it on slower 
 hardware. I am running this on a 2012 MBP with 2.3 Ghz i7 and an SSD. What is 
 your test machine?

Mac mini i7 with 1TB fusion drive and 16GB ram.

It also seemed to trigger more when grabbing larger repositories.
Replaceexternal/tinyxml2 with docs/source.android.com and you
might be more likely to trigger the issue.  You can also try
increasing the number of processes to spawn from 100 to 150.



 Cheers,
 Max


 test.sh
 
 #!/bin/bash
 rungit() {
mkdir $1
GIT_DIR=$1 git init --bare
echo '[remote aosp]'  $1/config
echo 'url =
 https://android.googlesource.com/platform/external/tinyxml2' 
 $1/config
GIT_DIR=$1 git fetch aosp +refs/heads/master:refs/remotes/aosp/master
rm -rf $1
 }

 for i in $(seq 1 100)
 do
rungit testdir$i 
 done
 
 $ ./test.sh  # Warning! This script fetches ~40MB of data

 When everything cools, you can see that there are some fetches hanging
 (typically).
 $ ps | grep 'git fetch'
 ...
 63310 ttys0040:00.01 git fetch aosp
 +refs/heads/master:refs/remotes/aosp/master
 63314 ttys0040:00.01 git fetch aosp
 +refs/heads/master:refs/remotes/aosp/master
 63319 ttys0040:00.01 git fetch aosp
 +refs/heads/master:refs/remotes/aosp/master
 63407 ttys0040:00.00 git fetch aosp
 +refs/heads/master:refs/remotes/aosp/master
 63414 ttys0040:00.00 git fetch aosp
 +refs/heads/master:refs/remotes/aosp/master
 63420 ttys0040:00.00 git fetch aosp
 +refs/heads/master:refs/remotes/aosp/master
 ...

 You can look at the parent process of each and see that one half
 spawned the other half, or you can look at the environment variables
 for each to see that there are two processes operating in the same
 directory for each directory where there's an issue.
 $ echo $(for pid in $(ps | grep 'git fetch' | grep -o '^[0-9]*'); do
 ps -p $pid -wwwE | grep 'GIT_DIR=[^ ]*' -o; done) | sort
 GIT_DIR=testdir14
 GIT_DIR=testdir14
 GIT_DIR=testdir32
 GIT_DIR=testdir32
 GIT_DIR=testdir47
 GIT_DIR=testdir47

 I've searched through the mailing list, but this doesn't seem to be a
 known issue.  I've only seen this occur on macs (and with a good deal
 of regularity).  It doesn't occur on my Ubuntu box.

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