Aw: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'

2014-01-06 Thread Thomas Ackermann
 
Hi Jiang,

this happens with all of my repo clones (I am using V1.8.5.2
on Windows and on Linux). Steps to reproduce:

mkdir repo_a  cd repo_a  git init .
echo 1foo  git add foo  git commit -m 1
cd ..
git clone repo_a repo_b
cd repo_a
echo 2foo  git add foo  git commit -m 2
cd ../repo_b
git status
git checkout -b branch
git checkout master

'git status' and 'git checkout master' in repo_b are now 
reporting Your branch is up-to-date with 'origin/master'
which is obviously wrong.

---
Thomas

- Original Nachricht 
Von: Jiang Xin worldhello@gmail.com
An:  Thomas Ackermann th.ac...@arcor.de
Datum:   06.01.2014 06:31
Betreff: Re: [Bug report] 'git status' always says Your branch is up-to-date
 with 'origin/master'

 2014/1/5 Thomas Ackermann th.ac...@arcor.de:
  Since f223459 status: always show tracking branch even no change
  'git status' (and 'git checkout master' always says
  Your branch is up-to-date with 'origin/master'
  even if 'origin/master' is way ahead from local 'master'.
 
 Hi, Thomas
 
 Can you provide your operations so that I can reproduce this issue?
 
 In the commit you mentioned above, there was a new test case named
 'checkout (up-to-date with upstream)' added in 't6040'. I also add two
 test-cases locally in order to reproduce the issue you report, and run
 them in arbitrary orders, but they all look fine:
 
 ok 4 - checkout (behind upstream)
 ok 5 - checkout (ahead upstream)
 ok 6 - checkout (diverged from upstream)
 ok 7 - checkout with local tracked branch
 ok 8 - checkout (upstream is gone)
 ok 9 - checkout (up-to-date with upstream)
 ok 10 - checkout (upstream is gone)
 ok 11 - checkout with local tracked branch
 ok 12 - checkout (diverged from upstream)
 ok 13 - checkout (ahead upstream)
 ok 14 - checkout (behind upstream)
 ok 15 - checkout (diverged from upstream)
 ok 16 - checkout (upstream is gone)
 ok 17 - checkout (ahead upstream)
 ok 18 - checkout with local tracked branch
 ok 19 - checkout (behind upstream)
 
 
 The two additional test cases I used locally are:
 
 checkout_test1() {
 test_expect_success 'checkout (behind upstream)' '
 (
 cd test  git checkout b3
 ) actual 
 test_i18ngrep is behind .* by 1 commit, and can be
 fast-forwarded actual
 '
 }
 
 checkout_test_2() {
 test_expect_success 'checkout (ahead upstream)' '
 (
 cd test  git checkout b4
 ) actual 
 test_i18ngrep is ahead of .* by 2 commits actual
 '
 }
 
 -- 
 Jiang Xin
 

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


Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'

2014-01-06 Thread Bryan Turner
On 6 January 2014 01:24, Thomas Ackermann th.ac...@arcor.de wrote:


 Hi Jiang,

 this happens with all of my repo clones (I am using V1.8.5.2
 on Windows and on Linux). Steps to reproduce:

 mkdir repo_a  cd repo_a  git init .
 echo 1foo  git add foo  git commit -m 1
 cd ..
 git clone repo_a repo_b
 cd repo_a
 echo 2foo  git add foo  git commit -m 2
 cd ../repo_b
 git status
 git checkout -b branch
 git checkout master

 'git status' and 'git checkout master' in repo_b are now
 reporting Your branch is up-to-date with 'origin/master'
 which is obviously wrong.


Unfortunately that's not true. In repo_b your ref for origin/master
has not moved. It has remotely (meaning refs/heads/master in repo_a
has moved), but git status is not hitting the remote to find out; it
only looks at the local state. To see what I mean, run git fetch in
repo_b. Once you do that, you'll see that git status correctly reports
you're behind.


 ---
 Thomas

 - Original Nachricht 
 Von: Jiang Xin worldhello@gmail.com
 An:  Thomas Ackermann th.ac...@arcor.de
 Datum:   06.01.2014 06:31
 Betreff: Re: [Bug report] 'git status' always says Your branch is up-to-date
  with 'origin/master'

  2014/1/5 Thomas Ackermann th.ac...@arcor.de:
   Since f223459 status: always show tracking branch even no change
   'git status' (and 'git checkout master' always says
   Your branch is up-to-date with 'origin/master'
   even if 'origin/master' is way ahead from local 'master'.
 
  Hi, Thomas
 
  Can you provide your operations so that I can reproduce this issue?
 
  In the commit you mentioned above, there was a new test case named
  'checkout (up-to-date with upstream)' added in 't6040'. I also add two
  test-cases locally in order to reproduce the issue you report, and run
  them in arbitrary orders, but they all look fine:
 
  ok 4 - checkout (behind upstream)
  ok 5 - checkout (ahead upstream)
  ok 6 - checkout (diverged from upstream)
  ok 7 - checkout with local tracked branch
  ok 8 - checkout (upstream is gone)
  ok 9 - checkout (up-to-date with upstream)
  ok 10 - checkout (upstream is gone)
  ok 11 - checkout with local tracked branch
  ok 12 - checkout (diverged from upstream)
  ok 13 - checkout (ahead upstream)
  ok 14 - checkout (behind upstream)
  ok 15 - checkout (diverged from upstream)
  ok 16 - checkout (upstream is gone)
  ok 17 - checkout (ahead upstream)
  ok 18 - checkout with local tracked branch
  ok 19 - checkout (behind upstream)
 
 
  The two additional test cases I used locally are:
 
  checkout_test1() {
  test_expect_success 'checkout (behind upstream)' '
  (
  cd test  git checkout b3
  ) actual 
  test_i18ngrep is behind .* by 1 commit, and can be
  fast-forwarded actual
  '
  }
 
  checkout_test_2() {
  test_expect_success 'checkout (ahead upstream)' '
  (
  cd test  git checkout b4
  ) actual 
  test_i18ngrep is ahead of .* by 2 commits actual
  '
  }
 
  --
  Jiang Xin
 

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


Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'

2014-01-06 Thread Jiang Xin
2014/1/6 Thomas Ackermann th.ac...@arcor.de:

 Hi Jiang,

 this happens with all of my repo clones (I am using V1.8.5.2
 on Windows and on Linux). Steps to reproduce:

 mkdir repo_a  cd repo_a  git init .
 echo 1foo  git add foo  git commit -m 1
 cd ..
 git clone repo_a repo_b
 cd repo_a
 echo 2foo  git add foo  git commit -m 2
 cd ../repo_b
 git status
 git checkout -b branch

Oops. Do

git fetch

then

git checkout master

You will get what you want.

 git checkout master

 'git status' and 'git checkout master' in repo_b are now
 reporting Your branch is up-to-date with 'origin/master'
 which is obviously wrong.


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


Aw: Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'

2014-01-06 Thread Thomas Ackermann
 
 
 Unfortunately that's not true. In repo_b your ref for origin/master
 has not moved. It has remotely (meaning refs/heads/master in repo_a
 has moved), but git status is not hitting the remote to find out; it
 only looks at the local state. To see what I mean, run git fetch in
 repo_b. Once you do that, you'll see that git status correctly reports
 you're behind.
 

OK; my expectation was, that the remote is checked for this ...
I see that this feature is useful for all non-trivial use cases
where you have several branches beside master for which the
remotes will be updated by git fetch.
But for the simple use case where you only have a master
branch I consider it not really helpful and - at least for me -
misleading.

---
Thomas
--
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 00/17] Fix some mkdir/rmdir races

2014-01-06 Thread Michael Haggerty
This is v2 of changes [1] to remove some mkdir/rmdir races around
safe_create_leading_directories() and a couple of its callers.  Thanks
to Jonathan Nieder for his thorough review of v1 and to Ramsay Jones
for pointing out a typo.  I think I have addressed all of their
suggestions.

This patch series has a bigger scope than v1.  The main differences is
that I took Jonathan's (implicit?) suggestion to move the retrying to
a level higher, where files are actually created in the directories
and therefore the directories are definitely no longer subject to
pruning.

Differences from v1:

* Improve some commit messages

* Break up some changes to safe_create_leading_directories() into
  smaller steps.

* Fix a problem in safe_create_leading_directories() when handling
  paths with multiple slashes (e.g., foo//bar).  (I noticed this
  pre-existing problem while making the other changes.)

* Change how retrying is done:

  * safe_create_leading_directories() doesn't retry internally;
instead, its return value is turned into an enum with a new value,
SCLD_VANISHED, that indicates that a directory along the path
vanished while it was working.  This return value is an indication
that its caller might want to try calling the function again.

  * Change rename_ref() to retry if either
safe_create_leading_directories() returns SCLD_VANISHED or if its
own call to rename() returns ENOENT.

* Fix a similar mkdir/rmdir race in lock_ref_sha1_basic().  This one
  is actually more interesting than the one in rename_ref() because it
  can be triggered internal to git.

* Fix a race in remove_dir_recurse(): if somebody else deletes a file
  or directory that the function wanted to delete anyway, don't treat
  it as an error.

The main git-internal race that I know to be fixed by these changes is
when pack-refs is trying to delete empty directories at the same time
as another process is trying to create a new reference.  It can be
reproduced by this script:

BRANCHES=foo/bar/xyzzy foo/bar/plugh
HEADS=h1 h2

rm -rf test-repo
$GIT init test-repo
cd test-repo
$GIT config user.name 'Lou User'
$GIT config user.email 'lu...@example.com'

for h in $HEADS
do
$GIT commit --allow-empty -m $h
$GIT branch $h
done

(
while true
do
for b in $BRANCHES
do
for h in $HEADS
do
$GIT update-ref refs/heads/$b $h
done
done
done
) 
pid1=$!

(
while true
do
$GIT pack-refs --all
done
) 
pid2=$!

The script has to fail if update-ref and pack-refs try to lock the
same reference at the same time, because we don't handle lock
contention:

fatal: Unable to create 
'/home/mhagger/tmp/test-repo/.git/refs/heads/foo/bar/plugh.lock': File exists.

If no other git process is currently running, this probably means a
git process crashed in this repository earlier. Make sure no other git
process is running and remove the file manually to continue.

Also, if update-ref changes the value of a reference while pack-refs
is running, then pack-refs emits a warning and leaves the new value of
the loose reference in place:

error: Ref refs/heads/foo/bar/xyzzy is at 
003a9cedc3ec79b8f589b158ffe91177f0a611b3 but expected 
34bf1e34d27a639732a01fef0a791e58c2c0882f

But it used to have other unnecessary failures, too, related to
mkdir/rmdir races between the two programs:

error: unable to create directory for .git/refs/heads/foo/bar/plugh

and

fatal: Unable to create 
'/home/mhagger/tmp/test-repo/.git/refs/heads/foo/bar/plugh.lock': No such file 
or directory

This patch series eliminates the last two types of failures.  Here are
tallies of failures from running the above script for 60 seconds
before and after this change.  Before:

Total updates: 46900
Unable to create '*.lock': File exists: 1992
* is at * but expected *: 940
unable to create directory: 187
Unable to create '*.lock': No such file or directory: 197

After:

Total updates: 47892
Unable to create '*.lock': File exists: 2105
* is at * but expected *: 835
unable to create directory: 0
Unable to create '*.lock': No such file or directory: 0

Clearly, more work is still needed in this area.  For example, the *
is at * but expected * errors are not really errors at all and should
not be reported as such.  And we might want to consider adding a short
delay and then a retry when acquiring locks.  If I run the same test
script with the following patch, then most or all of the Unable to
create '*.lock': File exists errors go away too.

==
diff --git a/refs.c b/refs.c
index 810f802..b3ff1f5 100644
--- a/refs.c
+++ b/refs.c
@@ -2117,7 +2117,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 * again:
   

[PATCH v2 01/17] safe_create_leading_directories(): fix format of if chaining

2014-01-06 Thread Michael Haggerty
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 sha1_file.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index daacc0c..c9245a6 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -125,8 +125,7 @@ int safe_create_leading_directories(char *path)
*pos = '/';
return -3;
}
-   }
-   else if (mkdir(path, 0777)) {
+   } else if (mkdir(path, 0777)) {
if (errno == EEXIST 
!stat(path, st)  S_ISDIR(st.st_mode)) {
; /* somebody created it since we checked */
@@ -134,8 +133,7 @@ int safe_create_leading_directories(char *path)
*pos = '/';
return -1;
}
-   }
-   else if (adjust_shared_perm(path)) {
+   } else if (adjust_shared_perm(path)) {
*pos = '/';
return -2;
}
-- 
1.8.5.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 v2 02/17] safe_create_leading_directories(): reduce scope of local variable

2014-01-06 Thread Michael Haggerty
This makes it more obvious that values of st don't persist across
loop iterations.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 sha1_file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index c9245a6..cc9957e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -108,9 +108,10 @@ int mkdir_in_gitdir(const char *path)
 int safe_create_leading_directories(char *path)
 {
char *pos = path + offset_1st_component(path);
-   struct stat st;
 
while (pos) {
+   struct stat st;
+
pos = strchr(pos, '/');
if (!pos)
break;
-- 
1.8.5.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 v2 09/17] safe_create_leading_directories(): add new error value SCLD_VANISHED

2014-01-06 Thread Michael Haggerty
Add a new possible error result that can be returned by
safe_create_leading_directories() and
safe_create_leading_directories_const(): SCLD_VANISHED.  This value
indicates that a file or directory on the path existed at one point
(either it already existed or the function created it), but then it
disappeared.  This probably indicates that another process deleted the
directory while we were working.  If SCLD_VANISHED is returned, the
caller might want to retry the function call, as there is a chance
that a new attempt will succeed.

Why doesn't safe_create_leading_directories() do the retrying
internally?  Because an empty directory isn't really ever safe until
it holds a file.  So even if safe_create_leading_directories() were
absolutely sure that the directory existed before it returned, there
would be no guarantee that the directory still existed when the caller
tried to write something in it.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 cache.h | 10 +-
 sha1_file.c | 11 +++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index c6a4157..f34c0a7 100644
--- a/cache.h
+++ b/cache.h
@@ -741,12 +741,20 @@ int adjust_shared_perm(const char *path);
  * Create the directory containing the named path, using care to be
  * somewhat safe against races.  Return one of the scld_error values
  * to indicate success/failure.
+ *
+ * SCLD_VANISHED indicates that one of the ancestor directories of the
+ * path existed at one point during the function call and then
+ * suddenly vanished, probably because another process pruned the
+ * directory while we were working.  To be robust against this kind of
+ * race, callers might want to try invoking the function again when it
+ * returns SCLD_VANISHED.
  */
 enum scld_error {
SCLD_OK = 0,
SCLD_FAILED = -1,
SCLD_PERMS = -2,
-   SCLD_EXISTS = -3
+   SCLD_EXISTS = -3,
+   SCLD_VANISHED = -4
 };
 enum scld_error safe_create_leading_directories(char *path);
 enum scld_error safe_create_leading_directories_const(const char *path);
diff --git a/sha1_file.c b/sha1_file.c
index 5594f11..69a4e95 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -132,6 +132,17 @@ enum scld_error safe_create_leading_directories(char *path)
if (errno == EEXIST 
!stat(path, st)  S_ISDIR(st.st_mode))
; /* somebody created it since we checked */
+   else if (errno == ENOENT)
+   /*
+* Either mkdir() failed because
+* somebody just pruned the containing
+* directory, or stat() failed because
+* the file that was in our way was
+* just removed.  Either way, inform
+* the caller that it might be worth
+* trying again:
+*/
+   ret = SCLD_VANISHED;
else
ret = SCLD_FAILED;
} else if (adjust_shared_perm(path)) {
-- 
1.8.5.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 v2 16/17] rename_tmp_log(): limit the number of remote_empty_directories() attempts

2014-01-06 Thread Michael Haggerty
This doesn't seem to be a likely error, but we've got the counter
anyway, so we might as well use it for an added bit of safety.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 refs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 8de636e..490525a 100644
--- a/refs.c
+++ b/refs.c
@@ -2530,7 +2530,7 @@ int delete_ref(const char *refname, const unsigned char 
*sha1, int delopt)
 
 static int rename_tmp_log(const char *newrefname)
 {
-   int attempts = 3;
+   int attempts = 4;
 
  retry:
if (safe_create_leading_directories(git_path(logs/%s, newrefname))) {
@@ -2539,7 +2539,7 @@ static int rename_tmp_log(const char *newrefname)
}
 
if (rename(git_path(TMP_RENAMED_LOG), git_path(logs/%s, newrefname))) 
{
-   if (errno==EISDIR || errno==ENOTDIR) {
+   if ((errno==EISDIR || errno==ENOTDIR)  --attempts  0) {
/*
 * rename(a, b) when b is an existing
 * directory ought to result in ISDIR, but
-- 
1.8.5.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 v2 13/17] remove_dir_recurse(): handle disappearing files and directories

2014-01-06 Thread Michael Haggerty
If a file or directory that we are trying to remove disappears (e.g.,
because another process has pruned it), do not consider it an error.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 dir.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/dir.c b/dir.c
index 11e1520..716b613 100644
--- a/dir.c
+++ b/dir.c
@@ -1476,7 +1476,9 @@ static int remove_dir_recurse(struct strbuf *path, int 
flag, int *kept_up)
flag = ~REMOVE_DIR_KEEP_TOPLEVEL;
dir = opendir(path-buf);
if (!dir) {
-   if (errno == EACCES  !keep_toplevel)
+   if (errno == ENOENT)
+   return keep_toplevel ? -1 : 0;
+   else if (errno == EACCES  !keep_toplevel)
/*
 * An empty dir could be removable even if it
 * is unreadable:
@@ -1496,13 +1498,21 @@ static int remove_dir_recurse(struct strbuf *path, int 
flag, int *kept_up)
 
strbuf_setlen(path, len);
strbuf_addstr(path, e-d_name);
-   if (lstat(path-buf, st))
-   ; /* fall thru */
-   else if (S_ISDIR(st.st_mode)) {
+   if (lstat(path-buf, st)) {
+   if (errno == ENOENT)
+   /*
+* file disappeared, which is what we
+* wanted anyway
+*/
+   continue;
+   /* fall thru */
+   } else if (S_ISDIR(st.st_mode)) {
if (!remove_dir_recurse(path, flag, kept_down))
continue; /* happy */
-   } else if (!only_empty  !unlink(path-buf))
+   } else if (!only_empty 
+  (!unlink(path-buf) || errno == ENOENT)) {
continue; /* happy, too */
+   }
 
/* path too long, stat fails, or non-directory still exists */
ret = -1;
@@ -1512,7 +1522,7 @@ static int remove_dir_recurse(struct strbuf *path, int 
flag, int *kept_up)
 
strbuf_setlen(path, original_len);
if (!ret  !keep_toplevel  !kept_down)
-   ret = rmdir(path-buf);
+   ret = (!rmdir(path-buf) || errno == ENOENT) ? 0 : -1;
else if (kept_up)
/*
 * report the uplevel that it is not an error that we
-- 
1.8.5.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 v2 12/17] remove_dir_recurse(): tighten condition for removing unreadable dir

2014-01-06 Thread Michael Haggerty
If opendir() fails on the top-level directory, it makes sense to try
to delete it anyway--but only if the failure was due to EACCES.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 dir.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index 23b6de4..11e1520 100644
--- a/dir.c
+++ b/dir.c
@@ -1476,8 +1476,11 @@ static int remove_dir_recurse(struct strbuf *path, int 
flag, int *kept_up)
flag = ~REMOVE_DIR_KEEP_TOPLEVEL;
dir = opendir(path-buf);
if (!dir) {
-   /* an empty dir could be removed even if it is unreadble */
-   if (!keep_toplevel)
+   if (errno == EACCES  !keep_toplevel)
+   /*
+* An empty dir could be removable even if it
+* is unreadable:
+*/
return rmdir(path-buf);
else
return -1;
-- 
1.8.5.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 v2 11/17] lock_ref_sha1_basic(): if locking fails with ENOENT, retry

2014-01-06 Thread Michael Haggerty
If hold_lock_file_for_update() fails with errno==ENOENT, it might be
because somebody else (for example, a pack-refs process) has just
deleted one of the lockfile's ancestor directories.  So if this
condition is detected, try again (up to 3 times).

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 refs.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 6eb8a02..8a3d3ea 100644
--- a/refs.c
+++ b/refs.c
@@ -2081,7 +2081,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 
lock-lk = xcalloc(1, sizeof(struct lock_file));
 
-   lflags = LOCK_DIE_ON_ERROR;
+   lflags = 0;
if (flags  REF_NODEREF) {
refname = orig_refname;
lflags |= LOCK_NODEREF;
@@ -2109,6 +2109,17 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
}
 
lock-lock_fd = hold_lock_file_for_update(lock-lk, ref_file, lflags);
+   if (lock-lock_fd  0) {
+   if (errno == ENOENT  --attempts  0)
+   /*
+* Maybe somebody just deleted one of the
+* directories leading to ref_file.  Try
+* again:
+*/
+   goto retry;
+   else
+   unable_to_lock_index_die(ref_file, errno);
+   }
return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
 
  error_return:
-- 
1.8.5.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 v2 17/17] rename_tmp_log(): on SCLD_VANISHED, retry

2014-01-06 Thread Michael Haggerty
If safe_create_leading_directories() fails because a file along the
path unexpectedly vanished, try again from the beginning.  Try at most
3 times.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 refs.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 490525a..810f802 100644
--- a/refs.c
+++ b/refs.c
@@ -2533,7 +2533,14 @@ static int rename_tmp_log(const char *newrefname)
int attempts = 4;
 
  retry:
-   if (safe_create_leading_directories(git_path(logs/%s, newrefname))) {
+   switch (safe_create_leading_directories(git_path(logs/%s, 
newrefname))) {
+   case SCLD_OK:
+   break; /* success */
+   case SCLD_VANISHED:
+   if (--attempts  0)
+   goto retry;
+   /* fall through */
+   default:
error(unable to create directory for %s, newrefname);
return -1;
}
-- 
1.8.5.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 v2 04/17] safe_create_leading_directories(): rename local variable

2014-01-06 Thread Michael Haggerty
Rename pos to next_component, because now it always points at the
next component of the path name that has to be processed.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 sha1_file.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 197766d..54202e8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -107,18 +107,18 @@ int mkdir_in_gitdir(const char *path)
 
 int safe_create_leading_directories(char *path)
 {
-   char *pos = path + offset_1st_component(path);
+   char *next_component = path + offset_1st_component(path);
 
-   while (pos) {
+   while (next_component) {
struct stat st;
-   char *slash = strchr(pos, '/');
+   char *slash = strchr(next_component, '/');
 
if (!slash)
break;
while (*(slash + 1) == '/')
slash++;
-   pos = slash + 1;
-   if (!*pos)
+   next_component = slash + 1;
+   if (!*next_component)
break;
 
*slash = '\0';
-- 
1.8.5.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] remote-hg: do not fail on invalid bookmarks

2014-01-06 Thread Torsten Bögershausen
On 2013-12-29 12.30, Antoine Pelisse wrote:
 Mercurial can have bookmarks pointing to nullid (the empty root
 revision), while Git can not have references to it.
 When cloning or fetching from a Mercurial repository that has such a
 bookmark, the import will fail because git-remote-hg will not be able to
 create the corresponding reference.
 
 Warn the user about the invalid reference, and continue the import,
 instead of stopping right away.
 
 Signed-off-by: Antoine Pelisse apeli...@gmail.com
 ---
  contrib/remote-helpers/git-remote-hg | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/contrib/remote-helpers/git-remote-hg 
 b/contrib/remote-helpers/git-remote-hg
 index eb89ef6..12d850e 100755
 --- a/contrib/remote-helpers/git-remote-hg
 +++ b/contrib/remote-helpers/git-remote-hg
 @@ -625,6 +625,9 @@ def list_head(repo, cur):
  def do_list(parser):
  repo = parser.repo
  for bmark, node in bookmarks.listbookmarks(repo).iteritems():
 +if node == '':
 +warn(Ignoring invalid bookmark '%s', bmark)
 +continue
  bmarks[bmark] = repo[node]
  
  cur = repo.dirstate.branch()
 
(Side note: ap/remote-hg-skip-null-bookmarks)

When I run the test-suite like this:
~/projects/git/git.pu/contrib/remote-helpers$ debug=t verbose=t make 
test-hg-hg-git.sh

All 11 test cases fail on my systems (Debian Wheezy and Mac OS X):
[snip]
WARNING: Ignoring invalid bookmark 'master'
To hg::../hgrepo-git
 ! [remote rejected] master - master
error: failed to push some refs to 'hg::../hgrepo-git'
not ok 1 - executable bit
#   
[snip]


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


Re: Re: [PATCH 2/2] Introduce git submodule attached update

2014-01-06 Thread Heiko Voigt
On Sun, Jan 05, 2014 at 10:46:11PM +0100, Francesco Pretto wrote:
 2014/1/5 Heiko Voigt hvo...@hvoigt.net:
  The following questions directly pop into my mind:
 
   - What means the maintainer does not track the submodules sha1? Does
 that mean the superproject always refers to submodule commits using
 branches?
 
 It means he doesn't need to control other developers commit to be
 checked out so he sets submodule.name.ignore to all. In this way
 he and the developers can work actively in their submodule copy.

So practically speaking: You mean that the value of
submodule.name.ignore is set to all in the master branch of the
superproject? From your other email referring to svn:externals I figure
that.

   - What happens if you want to go back to an earlier revision? Lets say
 a tagged release? How is ensured that you get the correct revision in
 the submodules?
 
 submodule.name.branch is one setting that is not copied in
 .git/config by git submodule init. git submodule update will use
 the setting in .gitmodules if not overridden voluntarily by the
 developer in .git/config. The maintainer can change that setting in
 .gitmodules and commit the change. Modifies will be propagated by
 the next git pull  git submodule update of the developer in the
 superproject.

I do not understand how does that ensure you get the correct submodule
revision when checking out a tagged release? To get a precise revision
the superproject needs to track a sha1 of a submodule commit. I do not
see how that has anything to do with submodule.name.branch?

   - In which situations does the developer or maintainer switch between
 your attached/detached mode?
 
 The developer/maintainer does so optionally and voluntarily and it
 effects only its private working tree.

This does not answer my question. I would like to find out the reason
why one would do the switch.

   - What is the repository branch which is given to the developer by
 the maintainer used for? Who creates this branch and who merges into
 it?
 
 The branch of course must exist prior submodule adding. In this
 use-case it does not really matter who creates it and who merges into
 it. Everyone with the right to merge into it has to work in the
 submodule seamlessly, as it was working on separate clone of the same
 repository used as the submodule.
o
Here is the same. I am searching for a description like:

If the developer works on a feature that needs a submodule change he:
  - creates a submodule branch
  - configures that submodule branch in the superproject:
git config -f .gitmodules submodule.common.branch dev/some-feature
git commit -am TEMP: track submodule common on branch
 - and pushes out his superproject branch

The submodule branch is then posted for review and continued to work on.

Once everyone involved is happy with the submodule change the branch in
there gets merged to master.

Now the branch in the superproject is modified to drop the change in
.gitmodules and the sha1 reference in the superproject is updated to the
current master of the superproject.

The superproject branch is posted for review.

...

Could you describe something like this for your workflow? A complete
change lifecycle when a developer works, as you call it, actively in a
submodule?

   - What are these subsequent merge or rebase update operations? Do
 you mean everyone has submodule.name.update configured to merge or
 rebase?
 
 
 subsequent merge or rebase update operations are just the ones
 after the initial clone/checkout, nothing particular.

To clarify you are talking about issuing git merge or git rebase
commands in the superproject?

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


Re: Re: [PATCH 2/2] Introduce git submodule attached update

2014-01-06 Thread Heiko Voigt
On Mon, Jan 06, 2014 at 12:22:23AM +0100, Francesco Pretto wrote:
 2014/1/5 Heiko Voigt hvo...@hvoigt.net:
  Could you please extend the description of your use-case so we can
  understand your goal better?
 
 
 Maybe I found better words to explain you my goal: the current git
 submodule use-case threats the submodule as a project independent
 dependency. My use case threats the submodule as part of the
 superproject repository. It could be easier to say that in this way
 submodules would behave very similarly to svn:externals, something
 that is actually missing in git. My goal is obtain this without
 altering git behavior for the existing use case.

I am not so sure. svn:externals was IMO a hack in SVN to bind projects
together. It does not record the revision and so has nothing to do
with version control. If you simply want to always checkout the
development tip of some project you could do something like this:

git submodule foreach 'git fetch  git checkout origin/master'

The demand for this 'missing feature' which we call the 'floating
submodules' model has been around for some time but until now we could
convince people that its not a feature but you are actually loosing
history information.

The workflow could always be changed to allow recording revisions. Which
is why you use git in the first place right? If you discard revisions
for submodules tracking down regression bugs can become a big problem or
completely impossible. Try using git bisect on such a history.

   - In which situations does the developer or maintainer switch between
 your attached/detached mode?
 
 As I told you in the other answer this is voluntary done by the
 developer, as he prefers.

Could you tell me a typical reason?


 I came to the conclusion that the
 --attach|--detach switches for the update command are not that
 useful and can be removed. It's still possible to obtain the switch
 between detached/attached very easily in this way:
 
 # Attach submodule
 $ git config submodule.name.attached true
 $ git submodule update
 
 # Detach submodule
 $ git config submodule.name.attached false
 $ git submodule update
 
 # Unset property in both .gitmodules and .git/config means - do nothing
 $ git config --unset submodule.name.attached
 $ git submodule update
 
 Also my submodule.name.attached property at the moment behaves
 like submodule.name.update: it is copied in .git/config by git
 submodule init. This is probably a mistake: the overridden value
 should be stored in .git/config only at the developer will, so the
 maintainer has still a chance to modify it in .gitmodules and
 propagate the behavior.
 
 I would send an updated patch but at this point I prefer to wait for a
 full review.

Lets first discuss and figure out what is the real missing feature here
and what should be implemented before working further on the code.

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


Re: Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-06 Thread Heiko Voigt
On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote:
 2014/1/5 W. Trevor King wk...@tremily.us:
  On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote:
  Also it could break some users that rely on the current behavior.
 
  The current code always has a detached HEAD after an initial-clone
  update, regardless of submodule.name.update, which doesn't match
  those docs either.
 
 I perfectly agree with you that the documentation is a bit
 contradictory with regard to update command and detached HEAD.
 That's why it's so hard to add a feature and keep the same spirit of
 those that coded submodules at first. Also, I think that submodules
 didn't get much feedback with regards to these pitfalls because many
 people try to setup them, they see that update detaches the HEAD and
 they think hmmm, maybe submodules are not what I was looking for.

I am not so sure about that. Why should detached HEAD make you think
like that? For us at $dayjob we have a pre-commit hook that denies you
to commit on a detached HEAD and asks you to create a branch first.

You then work on that branch and send it out for review. If the reviewer
is happy he merges it into a stable branch (master most times) of the
submodule. Only revisions that are on a stable branch in a submodule are
allowed to be linked in a superprojects branch that should be merged.

Before the submodule's branch gets merged we usually track the
development branches sha1 of the submodule in the superproject. For
cleanup in the submodule I currently use fixup! commits most times so
the referenced sha1 is not lost. In the very end when everyone is happy
with the submodule change I rebase, change the referenced sha1 in the
superproject and send the final branch out for review another time.

  Adding a check to only checkout
  submodule.name.branch if submodule.name.update was 'rebase',
  'merge', or 'none' would be easy, but I don't think that makes much
  sense.  I can't see any reason for folks who specify
  submodule.name.branch to prefer a detached HEAD over a local branch
  matching the remote branch's name.
 
 I think the reason is that it still matches the original use case of
 submodules devs:
 - the maintainer decides the specific commit developers should have;

Nope. We usually do not have a maintainer. We use a review based
workflow. Everyone is allowed to review. If you develop you need to send
you changes to a reviewer first who then merges when he is ok with it.

 - developers checkout that commit and don't pull (you can't do git
 pull in a detached HEAD);

Exactly. We consider pull evil ;-) Seriously: To update we only do fast
forward merges of local stable branches. Only reviewers or maintainers
are allowed to merge and push into stable branches. Direct commits to
stable branches are forbidden.

To review we have a shortcut to update the stable branch in git gui
for which the code can be found on my github[1].

 - they optionally get the upstream commit from the specified
 submodule.name.branch with --remote. They are still in a
 detached HEAD and can't do git pull.

Yes, why would you do a git pull in a submodule? Don't you want to do
something like

git checkout -t -b dev/my-topic origin/master

to start your development?

 Maybe who coded submodules at first was thinking that the best way to
 contribute to a project is to checkout that repository, and not work
 in the submodule. As said, this works well when the submodule
 repository is a full project, and not a bunch of shared code.

Why not work in the submodule? See explanation above.

Cheers Heiko


[1] https://github.com/hvoigt/git/commits/hv/gui-improvements
--
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] [RFC] Making use of bitmaps for thin objects

2014-01-06 Thread Jeff King
On Sun, Dec 22, 2013 at 09:55:23PM +, Ben Maurer wrote:

 One issue with this approach is that it seems git-pack-index doesn't
 perform as well with thin packs. git-index-pack uses a multi-threaded
 approach to resolving the deltas. However, the multithreading only
 works on deltas that are exclusively in the pack. After the
 multi-threaded phase, it incrementally brings in objects from external
 packs, but in single threaded manner. Many objects in the pack have
 some dependency on an external object, therefore, defeating the
 multithreading.

Yes. It will also just plain perform worse, because it will have to
copy over more external objects. This is somewhat made up for getting an
actual smaller pack size, but I suspect the completed thin-pack ends up
larger than what the server would otherwise send. Because the server is
blindly reusing on-disk deltas (which is good, because it takes load off
of the server), it misses good delta opportunities between objects in
the sent pack (which are likely almost as small, but would not require
fixing on the other end).

Single-threading the extra work we have to do just exacerbates the
problem, of course.

Still, I think it will be a net win for end-to-end wall clock time of
the operation. You are saving CPU time on the server end, and you're
saving network bandwidth with a smaller pack.

In my tests on torvalds/linux, doing a fetch across a local machine (so
basically discounting network improvements), the times look like (this
is end-to-end, counting both server and client CPU time):

  [vanilla]
  real0m3.850s
  user0m7.504s
  sys 0m0.380s

  [patched]
  real0m2.785s
  user0m2.472s
  sys 0m0.180s

So it was a win both for wall-clock and CPU.

 What's the use case for a pack file with a SHA1 reference living
 inside the pack file (why not just use an offset?) Would it make sense
 to assume that all such files are external and bring them in in the
 first phase.

Once upon a time, ref-delta was the only format supported by packfiles. Later,
delta-base-offset was invented, and the client and server negotiate the
use of the feature before the packfile is generated (and even when we
reuse objects, pack-objects will rewrite the header on the fly to use
ref-delta if necessary).

These days, pretty much everybody supports delta-base-offset, so I don't
think there is any reason index-pack should see ref-delta for a non-thin
object. We could probably teach index-pack an --assume-refs-are-thin
option to optimize for this case, and have fetch-pack/receive-pack pass
it whenever they know that delta-base-offset was negotiated.

-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] [RFC] Making use of bitmaps for thin objects

2014-01-06 Thread Jeff King
On Sun, Dec 22, 2013 at 11:47:34AM -0800, Ben Maurer wrote:

 Jeff King's bitmap branch appears to give a very substantial speedup.
 After applying this branch, the counting objects phase is basically
 free. However, I found that the compression phase still takes a
 non-trivial amount of time.

Sorry for the slow reply; I've been on vacation.

First off, I'm excited that you're looking into using bitmaps. We've
been using them for a while at GitHub, but more testing is definitely
appreciated. :)

When you build your bitmaps, do you set the pack.writeBitmapHashCache
option? We found that it makes a significant difference during the
compression phase, as otherwise git attempts deltas between random files
based on size. Here are some numbers for a simulated fetch from
torvalds/linux, representing about 7 weeks of history. Running:

  from=2d3c627502f2a9b0a7de06a5a2df2365542a72c9
  to=f0a679afefc0b6288310f88606b4bb1f243f1aa9
  run() {
(echo $to  echo ^$from) |
git pack-objects --stdout --all-progress --revs /dev/null
  }

  echo == no hash cache
  git repack -adb 2/dev/null
  time run

  echo == with hash cache
  git -c pack.writebitmaphashcache=1 repack -adb 2/dev/null
  time run

produces:

  == no hash cache
  Counting objects: 20661, done.
  Delta compression using up to 8 threads.
  Compressing objects: 100% (7700/7700), done.
  Writing objects: 100% (20661/20661), 23.23 MiB | 11.13 MiB/s, done.
  Total 20661 (delta 13884), reused 16638 (delta 12940)

  real0m3.626s
  user0m10.760s
  sys 0m0.060s

  == with hash cache
  Counting objects: 20661, done.
  Delta compression using up to 8 threads.
  Compressing objects: 100% (7700/7700), done.
  Writing objects: 100% (20661/20661), 22.64 MiB | 10.82 MiB/s, done.
  Total 20661 (delta 14038), reused 16638 (delta 12940)

  real0m3.072s
  user0m6.168s
  sys 0m0.100s

So our resulting pack shrinks a little because we find better deltas,
but note that we save a fair bit of CPU time (the wall clock time ends
up not all that different, because the single-threaded writing phase
represents a big chunk of that).

 It looks like most of the time spent compressing objects was in cases
 where the object was already compressed in the packfile, but the delta
 was based on an object that the client already had. For some reason,
 --thin wasn't enabling reuse of these deltas.

I'm not too surprised. The long-time strategy for a fetch has been to
walk down the haves and wants to their merge base. That boundary
commit is marked as a preferred base, meaning we won't send it, but
it's a good base for other objects, since we know the client has it.

Technically _all_ of the history reachable from that merge base could be
marked as a preferred base, but we don't do so for efficiency reasons:

  1. It's expensive to walk the full object graph for a small fetch, and

  2. You would clog the delta-search algorithm if you had a very large
 number of preferred-base objects.

With bitmaps, though, the history walk is free (we just check each
object against our have bitmap), so (1) is a non-issue. For (2), we
probably don't want to stick each object into the preferred-base list,
but we do want to reuse on-disk deltas we have, if we know the other
side has the base.

I don't know if you went through the same line of thinking, but that
matches your proposed solution. :)

 This is a hacky, poorly styled attempt to figure how how much better
 performance could be. With the bitmap branch, git should know what
 objects the client has already and can easily test if an existing delta
 can be reused. I don't know the branch well enough to code this, so
 as a hack, I just assumed the client has any delta base that is in
 the pack file (for our repo, this is always true, because we have a
 linear history)

Even without a linear history, it mostly works. If you are fetching all
of the branches from the other side, then you will end up with all of
the objects that the remote has. Which means that either you already
have the base, or the remote is about to send it to you.

It will break down, though, whenever the other side has something you're
not fetching. For that you really need to do the have bitmap check.

 This greatly reduces the time:
 
 $ { echo HEAD  echo ^$have; } | time ../git-bitmap/install/bin/git 
 pack-objects --use-bitmap-index --revs --stdout --thin  /dev/null
 Counting objects: 220909, done.
 Compressing objects: 100% (14203/14203), done.
 Total 220909 (delta 194050), reused 220909 (delta 199885)
 3.57user 1.28system 0:04.59elapsed 105%CPU (0avgtext+0avgdata 
 2007296maxresident)k
 0inputs+0outputs (0major+416243minor)pagefaults 0swaps

You might try with --all-progress (or pipe to wc -c), as this should
be reducing the output size, too.

Here's my same torvalds/linux test, run with the patch I'm including
below:

  Counting objects: 20661, done.
  Delta compression using up to 8 threads.
  Compressing objects: 100% (3677/3677), done.
  Writing 

Re: Bug report: stash in upstream caused remote fetch to fail

2014-01-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Fri, Jan 03, 2014 at 04:12:51PM -0500, Matt Burke wrote:

 + git init -q
 + git fetch -q -fu ../../../other '+refs/*:refs/*'
 fatal: bad object 9b985fbe6a2b783c16756077a8be261c94b6c197
 error: ../../../other did not send all necessary objects

 I was going to ask you to send your repository, but I can easily
 reproduce here. I guess people don't run into it because it's uncommon
 to fetch the whole refs/ namespace from a non-bare repo (and bare repos
 do not tend to have stashes). Here's a minimal reproduction recipe:

   git init repo 
   cd repo 
   echo content foo 
   git add . 
   git commit -m foo 
   echo more foo 
   git stash 
   git init --bare sub 
   cd sub 
   git fetch .. 'refs/*:refs/*'

 It looks like we are not feeding refs/stash properly to pack-objects.
 I'll try to take a closer look later today.

I looked at this in the past and I vaguely recall that we reject it
in the for-each-ref loop with check-ref-format saying eh, that is a
single-level name.

At that point I stopped digging, thinking it was a feature ;-)
based on your exact observation about stash vs bare/non-bare.

--
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-svn: workaround for a bug in svn serf backend

2014-01-06 Thread Andreas Stricker
Hi Roman

   git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 
 +)
 Thanks!

Well thanks to you for finding and fixing this bug that really annoyed
me just before Christmas again. Your bug analysis proved my observation
that even a fresh checkout (as I suggested in my last message) didn't
fix this issue.

And thanks to the reviewers too. Awesome!

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


Re: Re: Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-06 Thread Heiko Voigt
On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote:
 On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote:
  On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote:
   If submodule.name.branch is set, it *always* creates a new local
   branch of that name pointing to the exact sha1.  If
   submodule.name.branch is not set, we still create a
   detached-HEAD checkout of the exact sha1.
  
  Thanks for this clarification. Since the usual usage with --remote
  is with a remote-tracking branch, I confused this here. I am not
  sure whether blindly creating a local branch from the recorded sha1
  is the right thing to do. In what situations would that be helpful?
 
 In any situation where your going to develop the submodule locally,
 you're going to want a branch to develop in.  Starting local-submodule
 developers off on a branch seems useful, even if we can only use
 submodule.name.branch to guess at their preferred local branch name.
 Sometimes (often?) the guess will be right.  However, A detached HEAD
 will never be right for local development, so being right sometimes is
 still an improvement ;).

Starting developers at a local submodule branch makes sense. But lets
think further. What happens after the initial update? Most times the
submodule will already be initialized and cloned. Then developers will
still get a detached HEAD even with your local branch feature.

If there are no changes on it should we advance the local branch
somehow on update? If it does not exist anymore should we recreate it?

  At $dayjob we usually use feature branches for our work. So if
  someone wants to work in a submodule you simply create a branch at
  the current sha1 which you then send out for review.
 
 I'm all for named feature branches for development, and in this case
 submodule.name.branch is likely to be the wrong choice.  However,
 it's still safer to develop in that branch and then rename the branch
 to match your feature than it would be to develop your fix with a
 detached HEAD.  If your developers have enough discipline to always
 checkout their feature branch before starting development, my patch
 won't affect them.  However, I know a number of folks who go into
 fight-or-flight mode when they have a detached HEAD :p.

I agree having an initial branch makes it less likely to loose committed
changes. Thats good. Also starting on some local branch name and then
renaming the branch sounds quite practical. Then we could recreate the
default local branch on update (like described above).

  The reason why one would set a branch option here is to share the
  superproject branch with colleagues. He can make sure they can
  always fetch and checkout the submodule even though the branch there
  is still under cleanup and thus will be rebased often. The commit
  referenced by sha1 would not be available to a developer fetching
  after a rebase.
 
 Yeah, floating gitlinks are something else.  I'd be happy to have that
 functionality (gitlinks pointing to references) should be built into
 gitlinks themselves, not added as an additional layer in the submodule
 script.  This gitlinked sha1 rebased out of existence scenario is
 the first I've heard where I think gitlinked references would be
 useful.

Yeah I have been thinking about this for quite a while now, but have not
yet found the time to really think it through and come up with a good
solution that does not put you in danger of unprecise revisions. The
only solution I can think of is a similar approach as
submodule.name.branch gives us now but possibly enabled by some
option (i.e.: submodule.name.remote = true).
This way you always get the current tip of development but still see the
differences (which you can choose to commit) in git status.

   Thinking through this more, perhaps the logic should be:
   
   * If submodule.name.update (defaulting to checkout) is checkout,
 create a detached HEAD.
   * Otherwise, create a new branch submodule.name.branch
 (defaulting to master).
  
  Why not trigger the attached state with the submodule.name.branch
  configuration option? If there is a local branch available use that,
  if not the tracking branch (as it is currently). Then a developer
  can start working on the branch with:
  
  cd submodule; git checkout -t origin/branchname
  
  assuming that submodule update learns some more support for this.
 
 Isn't that already what 'git update --remote submodule' already
 does?

Does it? As far as I understood (not using the branch option yet) it
only does

git checkout origin/branchname

so there is no local branch created that tracks the remote branch (-t).
What I was thinking is that when submodule.name.branch is set a

git submodule update

will:

1. if no local branch with that name exists:

   checkout the remote/branch

2. If a local branch with that name exists:

   checkout the local branch and possibly advance it according to its
   setting.

Thinking further: 

Re: Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'

2014-01-06 Thread Jonathan Nieder
Hi,

Thomas Ackermann wrote:

In repo_b your ref for origin/master
 has not moved. It has remotely (meaning refs/heads/master in repo_a
 has moved), but git status is not hitting the remote to find out; it
 only looks at the local state.
[...]
 But for the simple use case where you only have a master
 branch I consider it not really helpful and - at least for me -
 misleading.

I see what you mean, and you're not the only one.

Git follows a rule of never contact another machine unless explicitly
asked to using a command such as 'git pull' or 'git fetch'.  To
support this, it makes a distinction between (1) the remote-tracking
ref origin/master and (2) the actual branch master in the remote
repository.  The former is what is updated by 'git fetch', and the
latter is something git does not know about without talking to the
remote server.

What documentation did you use when first starting to learn git?
Perhaps it can be fixed to emphasize the distinction between (1) and
(2) earlier.

Thanks,
Jonathan
--
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] Introduce git submodule attached update

2014-01-06 Thread W. Trevor King
On Mon, Jan 06, 2014 at 03:18:05PM +0100, Heiko Voigt wrote:
 If you simply want to always checkout the development tip of some
 project you could do something like this:
 
   git submodule foreach 'git fetch  git checkout origin/master'

Or (respecting submodule.name.branch):

  $ git submodule update --remote

You can even:

  $ git submodule update --remote --recursive

whenever you get an itch to upgrade everything everything in one
sweeping, hard-to-debug move ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: [PATCH] [RFC] Making use of bitmaps for thin objects

2014-01-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 We could probably teach index-pack an --assume-refs-are-thin
 option to optimize for this case, and have fetch-pack/receive-pack pass
 it whenever they know that delta-base-offset was negotiated.

I thought the existing negotiation merely means I understand offset
encoded bases, so you are allowed to use that encoding, not I will
not accept encoding other than the offset format, so you must use
that encoding for everything.

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


Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-06 Thread Junio C Hamano
Heiko Voigt hvo...@hvoigt.net writes:

 On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote:
 2014/1/5 W. Trevor King wk...@tremily.us:
  On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote:
  Also it could break some users that rely on the current behavior.
 
  The current code always has a detached HEAD after an initial-clone
  update, regardless of submodule.name.update, which doesn't match
  those docs either.
 
 I perfectly agree with you that the documentation is a bit
 contradictory with regard to update command and detached HEAD.
 That's why it's so hard to add a feature and keep the same spirit of
 those that coded submodules at first. Also, I think that submodules
 didn't get much feedback with regards to these pitfalls because many
 people try to setup them, they see that update detaches the HEAD and
 they think hmmm, maybe submodules are not what I was looking for.

 I am not so sure about that. Why should detached HEAD make you think
 like that? For us at $dayjob we have a pre-commit hook that denies you
 to commit on a detached HEAD and asks you to create a branch first.

Perception is irrational ;-)

We long-timers think detached is a perfect starting point for both
users of submodule who merely want to use the specified commit and
developers who want to work on the submodule to match the need of
the superproject.  The former do not have to do anything, and the
latter will have to chdir to the submodule working tree and create a
branch (or update the branch with rebase or pull on top of the
specified commit) as the first thing before doing anything.

Not everybody is a long-timer, but the saving grace is that nobody
stays a newcomer.

BUT.

 - developers checkout that commit and don't pull (you can't do git
 pull in a detached HEAD);

 Exactly. We consider pull evil ;-) Seriously: To update we only do fast
 forward merges of local stable branches. 
 ...
 Yes, why would you do a git pull in a submodule? Don't you want to do
 something like

   git checkout -t -b dev/my-topic origin/master

 to start your development?

As long as origin/master contains the commit specified by the
superproject, that would work, but it may be a good thing to use a
mode of submodule.*.update that does not have to drop the user into
a detached state in the first place.  I somehow thought that was
what rebase (or merge) was about, that is, starting from the state
where a branch is checked out in the submodule working tree, an
update in the superproject would cause that branch checked out in
the submodule brought up to date with respect to the commit
specified in the superproject tree.  If that is not how it is
supposed to work, please correct me---and we may have to add another
mode that does so (or even make rebase/merge do so as a bugfix).

And wouldn't it make it unnecessary to have a new re-attach option
if such a mode that never have to detach is used?
--
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] git-submodule.sh: Support 'checkout' as a valid update command

2014-01-06 Thread Junio C Hamano
W. Trevor King wk...@tremily.us writes:

 On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote:
 +case $update_module in
 +'')
 +;; # Unset update mode
 +checkout | rebase | merge | none)
 +;; # Known update modes
 +!*)
 +;; # Custom update command
 +*)
 +update_module=
 +echo 2 warning: invalid update mode for 
 submodule '$name'
 +;;
 +esac

 I'd prefer `die …` to `echo 2 …`.  It's hard to know if mapping
 the user's preferred (unknown) update mechanism to 'checkout' is
 serious or not.

 This commit also makes me think that --rebase, --merge, and --checkout
 should be replaced with a single --update={rebase|merge|checkout|!…}
 option, but that's probably food for another commit (and a long
 finger-breaking deprecation period).

All of the above points sound sensible to 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


Aw: Re: Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'

2014-01-06 Thread Thomas Ackermann
 
  But for the simple use case where you only have a master
  branch I consider it not really helpful and - at least for me -
  misleading.
 
 I see what you mean, and you're not the only one.
 
 Git follows a rule of never contact another machine unless explicitly
 asked to using a command such as 'git pull' or 'git fetch'.  To
 support this, it makes a distinction between (1) the remote-tracking
 ref origin/master and (2) the actual branch master in the remote
 repository.  The former is what is updated by 'git fetch', and the
 latter is something git does not know about without talking to the
 remote server.
 
 What documentation did you use when first starting to learn git?
 Perhaps it can be fixed to emphasize the distinction between (1) and
 (2) earlier.

I think it's not the problem of the documentation but of myself
not having it read thorough enough ;-)

(This new feature in V1.8.5 of course is not documented in any of the books
up to now but in the future could be used to explain the above mentioned
rule.)

Thanks to you, Bryan and Jiang for your help!

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


Re: [GIT PULL] l10n update for maint branch

2014-01-06 Thread Junio C Hamano
Jiang Xin worldhello@gmail.com writes:

 Please pull l10n update for maint branch. It can be merged to master
 without conflict.

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


[PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
A very common workflow for preparing patches involves working off a
topic branch and generating patches against 'master' to send off to the
maintainer. However, a plain

  $ git format-patch -o outgoing

is a no-op on a topic branch, and the user has to remember to specify
'master' explicitly everytime. Save the user the extra keystrokes by
introducing format.defaultTo which can contain the name of a branch
against which to base patches.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Documentation/config.txt   |  4 
 builtin/log.c  |  7 +++
 contrib/completion/git-completion.bash |  1 +
 t/t4014-format-patch.sh| 10 ++
 4 files changed, 22 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a405806..b90abd1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1135,6 +1135,10 @@ format.coverLetter::
format-patch is invoked, but in addition can be set to auto, to
generate a cover-letter only when there's more than one patch.
 
+format.defaultTo::
+   The name of a branch against which to generate patches by
+   default. You'd usually want this to be 'master'.
+
 filter.driver.clean::
The command which is used to convert the content of a worktree
file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/builtin/log.c b/builtin/log.c
index b97373d..ebc419e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -674,6 +674,7 @@ static int thread;
 static int do_signoff;
 static const char *signature = git_version_string;
 static int config_cover_letter;
+static const char *config_defaultto;
 
 enum {
COVER_UNSET,
@@ -750,6 +751,8 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON : 
COVER_OFF;
return 0;
}
+   if (!strcmp(var, format.defaultto))
+   return git_config_string(config_defaultto, var, value);
 
return git_log_config(var, value, cb);
 }
@@ -1324,6 +1327,10 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
die (_(--subject-prefix and -k are mutually exclusive.));
rev.preserve_subject = keep_subject;
 
+   if (argc  2  config_defaultto) {
+   argv[1] = config_defaultto;
+   argc++;
+   }
argc = setup_revisions(argc, argv, rev, s_r_opt);
if (argc  1)
die (_(unrecognized argument: %s), argv[1]);
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 39b81f7..75699d4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1992,6 +1992,7 @@ _git_config ()
format.attach
format.cc
format.coverLetter
+   format.defaultTo
format.headers
format.numbered
format.pretty
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 73194b2..46c0337 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1370,4 +1370,14 @@ test_expect_success 'cover letter auto user override' '
test_line_count = 2 list
 '
 
+test_expect_success 'defaultTo side' '
+   mkdir -p tmp 
+   test_when_finished rm -rf tmp;
+   git config --unset format.defaultTo 
+
+   git config format.defaultTo side 
+   git format-patch -o tmp list 
+   test_line_count = 3 list
+'
+
 test_done
-- 
1.8.5.2.229.g4448466.dirty

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


Re: [PATCH] Improve user-manual html and pdf formatting

2014-01-06 Thread Junio C Hamano
Thomas Ackermann th.ac...@arcor.de writes:

 Use asciidoc style 'article' instead of 'book' and change asciidoc title 
 level.
 This removes blank first page and superfluous Part I page (there is no 
 Part II)
 in pdf output. Also pdf size is decreased by this from 77 to 67 pages.
 In html output this removes unnecessary sub-tocs and chapter numbering.

 Signed-off-by: Thomas Ackermann th.ac...@arcor.de
 ---
  Documentation/Makefile| 2 +-
  Documentation/user-manual.txt | 4 ++--
  2 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/Documentation/Makefile b/Documentation/Makefile
 index 91a12c7..36c58fc 100644
 --- a/Documentation/Makefile
 +++ b/Documentation/Makefile
 @@ -324,7 +324,7 @@ manpage-base-url.xsl: manpage-base-url.xsl.in
  
  user-manual.xml: user-manual.txt user-manual.conf
   $(QUIET_ASCIIDOC)$(RM) $@+ $@  \
 - $(ASCIIDOC) $(ASCIIDOC_EXTRA) -b docbook -d book -o $@+ $  \
 + $(ASCIIDOC) $(ASCIIDOC_EXTRA) -b docbook -d article -o $@+ $  \
   mv $@+ $@
  
  technical/api-index.txt: technical/api-index-skel.txt \
 diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
 index cbb01a1..130c2f4 100644
 --- a/Documentation/user-manual.txt
 +++ b/Documentation/user-manual.txt
 @@ -1,5 +1,5 @@
 -Git User Manual
 -___
 +#65279;Git User Manual
 +===

Will queue after dropping the extra.

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: [RFC v2] submodule: Respect requested branch on all clones

2014-01-06 Thread W. Trevor King
On Mon, Jan 06, 2014 at 04:47:39PM +0100, Heiko Voigt wrote:
 On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote:
  On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote:
   On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote:
Thinking through this more, perhaps the logic should be:

* If submodule.name.update (defaulting to checkout) is checkout,
  create a detached HEAD.
* Otherwise, create a new branch submodule.name.branch
  (defaulting to master).
   
   Why not trigger the attached state with the
   submodule.name.branch configuration option? If there is a
   local branch available use that, if not the tracking branch (as
   it is currently). Then a developer can start working on the
   branch with:
   
 cd submodule; git checkout -t origin/branchname
   
   assuming that submodule update learns some more support for this.
  
  Isn't that already what 'git update --remote submodule' already
  does?
 
 Does it? As far as I understood (not using the branch option yet) it
 only does
 
   git checkout origin/branchname
 
 so there is no local branch created that tracks the remote branch (-t).

That's right.  Anyone who wants to do local development in a submodule
should probably not be using checkout updates, hence my proposal above
to base local-branch creation on submodule.name.update.

 What I was thinking is that when submodule.name.branch is set a
 
   git submodule update
 
 will:
 
 1. if no local branch with that name exists:
 
checkout the remote/branch
 
 2. If a local branch with that name exists:
 
checkout the local branch and possibly advance it according to
its setting.

This sounds too complicated to me ;).

 Thinking further: Maybe submodule.name.update = pull could denote
 that a user wants to have a branch ready for work in a submodule.

This sounds like my quoted realization above.  We both even preface
the idea with thinking ;).  However, I think merge, rebase,
!command, and all other non-checkout update schemes are already
signals that the developer is interested in local developent (and
therefore wants a branch), without the need to add an aditional 'pull'
(and then how to distinguish between rebase/merge?).

 submodule update will then
 
 1. if no local branch with that name exists:
 
- automatically create the branch based on the referenced sha1
- set up that its tracking remote/branch

With my patch this happens with the initial clone-update (as of v2,
only when submodule.name.branch is set.  In a hypothetical v3, only
when submodule.name.update is not checkout).  I'm not sure we want
to do this if the user switches to non-checkout updates after the
initial cloning update though.  They may actually have work in that
detached HEAD that we'd be clobbering.

- issue a git pull in the submodule

I think that updating using the gitlinked sha1 (a local update) and
updating using the upstream origin/$branch (a --remote update) should
always be two distinct events.  Combining them in a single operation
just complicates the situation.

 2. if a local branch with that name exists:
 
- issue a git pull in the submodule

That's what we already have with submodule.name.update as 'merge'.
The merged object is either the gitlinked sha1 (a local update) or a
re-fetched upstream branch tip (a --remote update).

We are on a local branch at this point, but not neccessarily
pointing at the gitlinked sha1.  The reset here ensures that
the new local branch does indeed point at the gitlinked sha1.
   
   But isn't this a fresh clone? Why should the branch point at
   anything else?
  
  We don't pass $sha1 to module_clone().  Before my patch, we don't
  even pass $branch to module_clone().  That means that
  module_clone() will only checkout the gitlinked sha1 when the
  upstream HEAD (or $branch with my patch) happens to point to the
  gitlinked sha1.  For example, if Alice adds Charie's repo as a
  submodule (gitlinking his current master d2dbd39), then Charlie
  pushes a new commit d0de817 to his master, and then Bob clones
  Alice's superproject.  Post-clone, Charlie's submodule will have
  checked out Charlie's new d0de817, and we need update's
  additional:
  
git reset --hard -q d2dbd39
  
  to rewind to Alice's gitlinked sha1.
 
 Ah yeah, sorry I was confusing this with the checkout of
 remote/branch here again. Since I have done that twice already
 maybe we should be careful about not confusing users with this as
 well...

 After wrapping my head around the fact that you want to simply create a
 local branch on the referenced sha1 (and hopefully remembering it) I
 still would like to think a little more about it and let it settle a
 bit.

The way I keep this straight is:

1. Submodules are links to Git commits (that's how they're stored in
   the index).
2. There are two places to look if you want to update the linked
   commit:
   a. The superproject's tree (a local updates).
   b. 

[PATCH 1/2] completion: complete format.coverLetter

2014-01-06 Thread Ramkumar Ramachandra
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..39b81f7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1991,6 +1991,7 @@ _git_config ()
fetch.unpackLimit
format.attach
format.cc
+   format.coverLetter
format.headers
format.numbered
format.pretty
-- 
1.8.5.2.229.g4448466.dirty

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


Re: [PATCH 0/2] Minor convinience feature: format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
 Ramkumar Ramachandra (2):
   completion: complete format.coverLetter
   format-patch: introduce format.defaultTo

Any thoughts on checkout.defaultTo? I have a com alias to checkout 'master'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-06 Thread W. Trevor King
On Mon, Jan 06, 2014 at 08:56:22AM -0800, Junio C Hamano wrote:
 Heiko Voigt hvo...@hvoigt.net writes:
  Yes, why would you do a git pull in a submodule? Don't you want to do
  something like
 
  git checkout -t -b dev/my-topic origin/master
 
  to start your development?
 
 As long as origin/master contains the commit specified by the
 superproject, that would work, but it may be a good thing to use a
 mode of submodule.*.update that does not have to drop the user into
 a detached state in the first place.  I somehow thought that was
 what rebase (or merge) was about, that is, starting from the state
 where a branch is checked out in the submodule working tree, an
 update in the superproject would cause that branch checked out in
 the submodule brought up to date with respect to the commit
 specified in the superproject tree.

That is my understanding as well.  In fact, I don't think the
detached-HEAD-vs-branch distinction is important here, you can still
rebase your detached HEAD onto the superproject's referenced commit
(or origin/$branch with --remote).  This will also work for merge, and
should work for well-crafted !commands.

 And wouldn't it make it unnecessary to have a new re-attach option
 if such a mode that never have to detach is used?

I think so, but we currently don't have a never detached route for
folks that are cloning submodules via update (instead of via
'submodule add').  Currently, new clone-updates will always leave you
with a detached HEAD (unless you have branch-creation in your update
!command).  My patch aims to close this detached-HEAD gap, for folks
we expect will be doing local development, by creating an initial
branch at clone-update time.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


[BUG] rebase --onto doesn't abort properly

2014-01-06 Thread Ramkumar Ramachandra
Hi,

On the latest git, I noticed that a rebase --onto doesn't abort
properly. Steps to reproduce:

  # on some topic branch
  $ git rebase --onto master @~10
  ^C # quickly!
  $ git rebase --abort
  # HEAD is still detached

I tried going back a few revisions, and the bug seems to be very old;
I'm surprised I didn't notice it earlier.

Are others able to reproduce this?

Thanks.

Ram
--
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] git-submodule.sh: Support 'checkout' as a valid update command

2014-01-06 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 W. Trevor King wk...@tremily.us writes:

 On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote:
 +   case $update_module in
 +   '')
 +   ;; # Unset update mode
 +   checkout | rebase | merge | none)
 +   ;; # Known update modes
 +   !*)
 +   ;; # Custom update command
 +   *)
 +   update_module=
 +   echo 2 warning: invalid update mode for 
 submodule '$name'
 +   ;;
 +   esac

 I'd prefer `die …` to `echo 2 …`.  It's hard to know if mapping
 the user's preferred (unknown) update mechanism to 'checkout' is
 serious or not.

 This commit also makes me think that --rebase, --merge, and --checkout
 should be replaced with a single --update={rebase|merge|checkout|!…}
 option, but that's probably food for another commit (and a long
 finger-breaking deprecation period).

 All of the above points sound sensible to me.

I'll tentatively queue this on 'pu' (with the suggested die
update), with some rewording of the log message.  The patch needs to
be signed-off, though.

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 1/2] git-submodule.sh: Support 'checkout' as a valid update command

2014-01-06 Thread Francesco Pretto
Ok, applying the suggested modifications and resending shortly.

Thank you,
Francesco

2014/1/6 Junio C Hamano gits...@pobox.com:
 Junio C Hamano gits...@pobox.com writes:

 W. Trevor King wk...@tremily.us writes:

 On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote:
 +   case $update_module in
 +   '')
 +   ;; # Unset update mode
 +   checkout | rebase | merge | none)
 +   ;; # Known update modes
 +   !*)
 +   ;; # Custom update command
 +   *)
 +   update_module=
 +   echo 2 warning: invalid update mode for 
 submodule '$name'
 +   ;;
 +   esac

 I'd prefer `die …` to `echo 2 …`.  It's hard to know if mapping
 the user's preferred (unknown) update mechanism to 'checkout' is
 serious or not.

 This commit also makes me think that --rebase, --merge, and --checkout
 should be replaced with a single --update={rebase|merge|checkout|!…}
 option, but that's probably food for another commit (and a long
 finger-breaking deprecation period).

 All of the above points sound sensible to me.

 I'll tentatively queue this on 'pu' (with the suggested die
 update), with some rewording of the log message.  The patch needs to
 be signed-off, though.

 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 10/17] lock_ref_sha1_basic(): on SCLD_VANISHED, retry

2014-01-06 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 If safe_create_leading_directories() fails because a file along the
 path unexpectedly vanished, try again (up to 3 times).

 This can occur if another process is deleting directories at the same
 time as we are trying to make them.  For example, git pack-refs
 --all tries to delete the loose refs and any empty directories that
 are left behind.  If a pack-refs process is running, then it might
 delete a directory that we need to put a new loose reference in.

 If safe_create_leading_directories() thinks this might have happened,
 then take its advice and try again (maximum three attempts).

 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 ---
  refs.c | 11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)

 diff --git a/refs.c b/refs.c
 index 3926136..6eb8a02 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -2039,6 +2039,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
 *refname,
   int type, lflags;
   int mustexist = (old_sha1  !is_null_sha1(old_sha1));
   int missing = 0;
 + int attempts = 3;
  
   lock = xcalloc(1, sizeof(struct ref_lock));
   lock-lock_fd = -1;
 @@ -2093,7 +2094,15 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
 *refname,
   if ((flags  REF_NODEREF)  (type  REF_ISSYMREF))
   lock-force_write = 1;
  
 - if (safe_create_leading_directories(ref_file)) {
 + retry:
 + switch (safe_create_leading_directories(ref_file)) {
 + case SCLD_OK:
 + break; /* success */
 + case SCLD_VANISHED:
 + if (--attempts  0)
 + goto retry;
 + /* fall through */

Hmph.

Having no backoff/sleep at all might be OK here as long as the other
side that removes does not retry (and I do not think the other side
would, even though I haven't read through the series to the end yet
;-)).

This may be just a style thing, but I find that the variable name
attempts that starts out as 3 quite misleading, as its value is
not the number of attempts made but the remaining number of
attempts allowed.  Starting it from 0 and then

if (attempts++  MAX_ATTEMPTS)
goto retry;

would be one way to clarify it.  Renaming it to remaining_attempts
would be another.

 + default:
   last_errno = errno;
   error(unable to create directory for %s, ref_file);
   goto error_return;
--
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 13/17] remove_dir_recurse(): handle disappearing files and directories

2014-01-06 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 If a file or directory that we are trying to remove disappears (e.g.,
 because another process has pruned it), do not consider it an error.

 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 ---
  dir.c | 22 --
  1 file changed, 16 insertions(+), 6 deletions(-)

 diff --git a/dir.c b/dir.c
 index 11e1520..716b613 100644
 --- a/dir.c
 +++ b/dir.c
 @@ -1476,7 +1476,9 @@ static int remove_dir_recurse(struct strbuf *path, int 
 flag, int *kept_up)
   flag = ~REMOVE_DIR_KEEP_TOPLEVEL;
   dir = opendir(path-buf);
   if (!dir) {
 - if (errno == EACCES  !keep_toplevel)
 + if (errno == ENOENT)
 + return keep_toplevel ? -1 : 0;

If we were told that foo/bar must go, but do not bother removing
foo/, is it an error if foo itself did not exist?  I think this
step does not change the behaviour from the original (we used to say
oh, we were told to keep_toplevel, and the top-level cannot be read
for whatever reason, so it is an error), but because this series is
giving us a finer grained error diagnosis, we may want to think
about it further, perhaps as a follow-up.

I also wonder why the keep-toplevel logic is in this recurse part
of the callchain. If everything in foo/bar/ can be removed but
foo/bar/ is unreadable, it is OK, when opendir(foo/bar) failed
with EACCESS, to attempt to rmdir(foo/bar) whether we were told
not to attempt removing foo/, no?

 + else if (errno == EACCES  !keep_toplevel)

That is, I am wondering why this part just checks !keep_toplevel,
not

(!keep_toplevel || has_dir_separator(path-buf))

or something.

   /*
* An empty dir could be removable even if it
* is unreadable:
 @@ -1496,13 +1498,21 @@ static int remove_dir_recurse(struct strbuf *path, 
 int flag, int *kept_up)
  
   strbuf_setlen(path, len);
   strbuf_addstr(path, e-d_name);
 - if (lstat(path-buf, st))
 - ; /* fall thru */
 - else if (S_ISDIR(st.st_mode)) {
 + if (lstat(path-buf, st)) {
 + if (errno == ENOENT)
 + /*
 +  * file disappeared, which is what we
 +  * wanted anyway
 +  */
 + continue;
 + /* fall thru */
 + } else if (S_ISDIR(st.st_mode)) {
   if (!remove_dir_recurse(path, flag, kept_down))
   continue; /* happy */
 - } else if (!only_empty  !unlink(path-buf))
 + } else if (!only_empty 
 +(!unlink(path-buf) || errno == ENOENT)) {
   continue; /* happy, too */
 + }
  
   /* path too long, stat fails, or non-directory still exists */
   ret = -1;
 @@ -1512,7 +1522,7 @@ static int remove_dir_recurse(struct strbuf *path, int 
 flag, int *kept_up)
  
   strbuf_setlen(path, original_len);
   if (!ret  !keep_toplevel  !kept_down)
 - ret = rmdir(path-buf);
 + ret = (!rmdir(path-buf) || errno == ENOENT) ? 0 : -1;
   else if (kept_up)
   /*
* report the uplevel that it is not an error that we
--
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 17/17] rename_tmp_log(): on SCLD_VANISHED, retry

2014-01-06 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 If safe_create_leading_directories() fails because a file along the
 path unexpectedly vanished, try again from the beginning.  Try at most
 3 times.

As the previous step bumped it from 3 to 4 without explanation, the
above no longer reflects reality ;-)

The series mostly looked sane from a cursory read.

Will re-queue.  Thanks.



 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 ---
  refs.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

 diff --git a/refs.c b/refs.c
 index 490525a..810f802 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -2533,7 +2533,14 @@ static int rename_tmp_log(const char *newrefname)
   int attempts = 4;
  
   retry:
 - if (safe_create_leading_directories(git_path(logs/%s, newrefname))) {
 + switch (safe_create_leading_directories(git_path(logs/%s, 
 newrefname))) {
 + case SCLD_OK:
 + break; /* success */
 + case SCLD_VANISHED:
 + if (--attempts  0)
 + goto retry;
 + /* fall through */
 + default:
   error(unable to create directory for %s, newrefname);
   return -1;
   }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 03/17] safe_create_leading_directories(): add explicit slash pointer

2014-01-06 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 Keep track of the position of the slash character independently of
 pos, thereby making the purpose of each variable clearer and
 working towards other upcoming changes.

 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 ---

This step has an interaction with $gmane/239878 where Windows folks
want it to pay attention to is_dir_sep()---over there, a backslash
could separate directory path components.

AFAIK, the function was meant to be used only on paths we internally
generate, and the paths we internally generate all are slash
separated, so it could be argued that feeding a path, whose path
components are separated by backslashes, that we obtained from the
end user without converting it to the internal form in some
codepaths (e.g. $there in git clone $url $there) are bugs we
acquired over time that need to be fixed, but it is easy enough to
use is_dir_sep() here to work it around, and doing so will
not negatively affect

 1. UNIX-only projects by forbidding use of a byte with backslash in
it as a path component character (yes, I am imagining using
Shift-JIS that can use a backslash as the second byte of
two-byte character in the pathname on UNIX); and

 2. UNIX-and-Windows mixed projects, as you cannot sanely use such a
pathname with backslash as part of a path component if its tree
needs to be checked out on Windows.


  sha1_file.c | 20 +++-
  1 file changed, 11 insertions(+), 9 deletions(-)

 diff --git a/sha1_file.c b/sha1_file.c
 index cc9957e..197766d 100644
 --- a/sha1_file.c
 +++ b/sha1_file.c
 @@ -111,19 +111,21 @@ int safe_create_leading_directories(char *path)
  
   while (pos) {
   struct stat st;
 + char *slash = strchr(pos, '/');
  
 - pos = strchr(pos, '/');
 - if (!pos)
 + if (!slash)
   break;
 - while (*++pos == '/')
 - ;
 + while (*(slash + 1) == '/')
 + slash++;
 + pos = slash + 1;
   if (!*pos)
   break;
 - *--pos = '\0';
 +
 + *slash = '\0';
   if (!stat(path, st)) {
   /* path exists */
   if (!S_ISDIR(st.st_mode)) {
 - *pos = '/';
 + *slash = '/';
   return -3;
   }
   } else if (mkdir(path, 0777)) {
 @@ -131,14 +133,14 @@ int safe_create_leading_directories(char *path)
   !stat(path, st)  S_ISDIR(st.st_mode)) {
   ; /* somebody created it since we checked */
   } else {
 - *pos = '/';
 + *slash = '/';
   return -1;
   }
   } else if (adjust_shared_perm(path)) {
 - *pos = '/';
 + *slash = '/';
   return -2;
   }
 - *pos++ = '/';
 + *slash = '/';
   }
   return 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/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Jonathan Nieder
Hi,

Ramkumar Ramachandra wrote:

  a plain

   $ git format-patch -o outgoing

 is a no-op on a topic branch, and the user has to remember to specify
 'master' explicitly everytime. Save the user the extra keystrokes by
 introducing format.defaultTo

Not excited.  Two reasons:

 1. Most config settings are in noun form: e.g.,
[remote] pushDefault = foo.  That makes their names easy to guess
and makes them easy to talk about: I set the default remote for
pushing by changing the remote.pushdefault setting.

'[url foo] insteadOf' is an exception to that and a bit of an
aberration.

This new '[format] defaultTo' repeats the same end-with-a-preposition
mistake, while I think it would be better to learn from it.

 2. Wouldn't a more natural default be @{u}..HEAD instead of relying on
the user to do the make-work of keeping a local branch that tracks
master up to date?

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


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 A very common workflow for preparing patches involves working off a
 topic branch and generating patches against 'master' to send off to the
 maintainer. However, a plain

   $ git format-patch -o outgoing

 is a no-op on a topic branch,...

Two points.

 - why is a single branch name sufficient?

 - is it a better option to simply default to @{u}, if one exists,
   instead of failing?

--
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] rebase --onto doesn't abort properly

2014-01-06 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Hi,

 On the latest git, I noticed that a rebase --onto doesn't abort
 properly. Steps to reproduce:

   # on some topic branch
   $ git rebase --onto master @~10
   ^C # quickly!
   $ git rebase --abort
   # HEAD is still detached

I do not think --abort was designed to abort an uncontrolled stop
like ^C in the first place.  To allow that kind of recovery, you
need to teach rebase to first record the state you would want to
go back to before doing anything, but then what happens if the ^C
comes when you are still in the middle of doing so?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
  - why is a single branch name sufficient?

It does accept a revision, so any form is allowed; but why would
anyone want that in a format.defaultTo? I'm not sure we want to impose
an artificial restriction on the configuration variable though.

  - is it a better option to simply default to @{u}, if one exists,
instead of failing?

I'm not sure @{u} is a good default. Personally, my workflow involves
publishing my fork before sending out patches; mainly so that I can
compare with @{u} when I do re-spins. People can put @{u} in
format.defaultTo if it suits their workflow though.
--
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] rebase --onto doesn't abort properly

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 I do not think --abort was designed to abort an uncontrolled stop
 like ^C in the first place.

Why not? All it requires is a reset --hard to
.git/rebase-apply/head-name, as usual, no?

 To allow that kind of recovery, you
 need to teach rebase to first record the state you would want to
 go back to before doing anything, but then what happens if the ^C
 comes when you are still in the middle of doing so?

I'm a bit puzzled: doesn't rebase write_basic_state() as soon as it
starts? It's true that we can't recover from a ^C before that, but I
would expect to be able to recover after a ^C somewhere in the middle.
--
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-submodule.sh: Support 'checkout' as a valid update command

2014-01-06 Thread Francesco Pretto
According to Documentation/gitmodules.txt, 'checkout' is a valid
'submodule.name.update' command. Also git-submodule.sh refers to
it and processes it correctly. Reflecting commit 'ac1fbb' to support
this syntax and also validate property values during 'update' command,
issuing an error if the value found is unknown.

Signed-off-by: Francesco Pretto cez...@gmail.com
---
 git-submodule.sh | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2677f2e..4a30087 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -622,7 +622,7 @@ cmd_init()
   test -z $(git config submodule.$name.update)
then
case $upd in
-   rebase | merge | none)
+   checkout | rebase | merge | none)
;; # known modes of updating
*)
echo 2 warning: unknown update mode '$upd' 
suggested for submodule '$name'
@@ -805,6 +805,17 @@ cmd_update()
update_module=$update
else
update_module=$(git config submodule.$name.update)
+   case $update_module in
+   '')
+   ;; # Unset update mode
+   checkout | rebase | merge | none)
+   ;; # Known update modes
+   !*)
+   ;; # Custom update command
+   *)
+   die $(eval_gettext Invalid update mode 
'$update_module' for submodule '$name')
+   ;;
+   esac
fi
 
displaypath=$(relative_path $prefix$sm_path)
-- 
1.8.5.2.229.g4448466.dirty

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


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Jonathan Nieder wrote:
  1. Most config settings are in noun form: e.g.,
 [remote] pushDefault = foo.  That makes their names easy to guess
 and makes them easy to talk about: I set the default remote for
 pushing by changing the remote.pushdefault setting.

 '[url foo] insteadOf' is an exception to that and a bit of an
 aberration.

 This new '[format] defaultTo' repeats the same end-with-a-preposition
 mistake, while I think it would be better to learn from it.

I agree that it's somewhat unconventional to allow people to put a
revision as a configuration variable value, but I think it's useful.
url.url.insteadOf is incredibly useful, for instance.

  2. Wouldn't a more natural default be @{u}..HEAD instead of relying on
 the user to do the make-work of keeping a local branch that tracks
 master up to date?

Like I said in my message to Junio, @{u} is not necessarily the best
default for all workflows, although you can fill that into
format.defaultTo.
--
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] mv: better document side effects when moving a submodule

2014-01-06 Thread Jens Lehmann
The Submodules section of the git mv documentation mentions what will
happen when a submodule with a gitfile gets moved with newer git. But it
doesn't talk about what happens when the user changes between commits
before and after the move, which does not update the work tree like using
the mv command did the first time.

Explain what happens and what the user has to do manually to fix that.
Also document this in a new test.

Reported-by: George Papanikolaou g3orge@gmail.com
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---

Am 09.12.2013 18:49, schrieb Jens Lehmann:
 Am 09.12.2013 11:59, schrieb George Papanikolaou:
 Also after mv you need to run 'submodule update' and I think this should be
 documented somewhere.
 
 You're right, this should be mentioned in the mv man page. I'll
 prepare a patch for that.

Does this new paragraph make it clearer?


 Documentation/git-mv.txt | 10 ++
 t/t7001-mv.sh| 21 +
 2 files changed, 31 insertions(+)

diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt
index b1f7988..c9e8568 100644
--- a/Documentation/git-mv.txt
+++ b/Documentation/git-mv.txt
@@ -52,6 +52,16 @@ core.worktree setting to make the submodule work in the new 
location.
 It also will attempt to update the submodule.name.path setting in
 the linkgit:gitmodules[5] file and stage that file (unless -n is used).

+Please note that each time a superproject update moves a populated
+submodule (e.g. when switching between commits before and after the
+move) a stale submodule checkout will remain in the old location
+and an empty directory will appear in the new location. To populate
+the submodule again in the new location the user will have to run
+git submodule update afterwards. Removing the old directory is
+only safe when it uses a gitfile, as otherwise the history of the
+submodule will be deleted too. Both steps will be obsolete when
+recursive submodule update has been implemented.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 3bfdfed..e3c8c2c 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -442,4 +442,25 @@ test_expect_success 'mv --dry-run does not touch the 
submodule or .gitmodules' '
git diff-files --quiet -- sub .gitmodules
 '

+test_expect_success 'checking out a commit before submodule moved needs manual 
updates' '
+   git mv sub sub2 
+   git commit -m moved sub to sub2 
+   git checkout -q HEAD^ 2actual 
+   echo warning: unable to rmdir sub2: Directory not empty expected 
+   test_i18ncmp expected actual 
+   git status -s sub2 actual 
+   echo ?? sub2/ expected 
+   test_cmp expected actual 
+   ! test -f sub/.git 
+   test -f sub2/.git 
+   git submodule update 
+   test -f sub/.git 
+   rm -rf sub2 
+   git diff-index --exit-code HEAD 
+   git update-index --refresh 
+   git diff-files --quiet -- sub .gitmodules 
+   git status -s sub2 actual 
+   ! test -s actual
+'
+
 test_done
-- 
1.8.5.2.230.g9325930


--
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 report: stash in upstream caused remote fetch to fail

2014-01-06 Thread Jeff King
On Mon, Jan 06, 2014 at 08:16:31AM -0800, Junio C Hamano wrote:

  I was going to ask you to send your repository, but I can easily
  reproduce here. I guess people don't run into it because it's uncommon
  to fetch the whole refs/ namespace from a non-bare repo (and bare repos
  do not tend to have stashes). Here's a minimal reproduction recipe:
 
git init repo 
cd repo 
echo content foo 
git add . 
git commit -m foo 
echo more foo 
git stash 
git init --bare sub 
cd sub 
git fetch .. 'refs/*:refs/*'
 
  It looks like we are not feeding refs/stash properly to pack-objects.
  I'll try to take a closer look later today.
 
 I looked at this in the past and I vaguely recall that we reject it
 in the for-each-ref loop with check-ref-format saying eh, that is a
 single-level name.
 
 At that point I stopped digging, thinking it was a feature ;-)
 based on your exact observation about stash vs bare/non-bare.

I am fine with rejecting it with a warning, but we should not then
complain that the other side did not send us the object, since we should
not be asking for it at all. I also do not see us complaining about the
funny ref anywhere.  So there is definitely _a_ bug here. :)

I think somebody else mentioned recently that we do not handle malformed
refs consistently. I think it was:

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

which might or might not be related.

-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] [RFC] Making use of bitmaps for thin objects

2014-01-06 Thread Jeff King
On Mon, Jan 06, 2014 at 08:37:49AM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  We could probably teach index-pack an --assume-refs-are-thin
  option to optimize for this case, and have fetch-pack/receive-pack pass
  it whenever they know that delta-base-offset was negotiated.
 
 I thought the existing negotiation merely means I understand offset
 encoded bases, so you are allowed to use that encoding, not I will
 not accept encoding other than the offset format, so you must use
 that encoding for everything.

You are right about what it means. But this is an optimization, not a
correctness thing. So if we assume that senders who are allowed to send
offsets will generally do so, it might be a reasonable optimization to
guess that ref-delta objects will need thin completion. If we are wrong,
the worst case is that we add an extra local object to the end of the
pack. So as long as we are right most of the time, it may still be a
win.

Of course, it may also be possible to simply multi-thread the
thin-completion portion of index-pack. That would be even better, though
I am not sure how it would work. The resolution of an object in one
thread can always become the input for another thread. But maybe we
could have each thread come up with a proposed set of objects to add to
the pack, and then drop duplicates or something. I haven't looked
closely.

-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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
  - why is a single branch name sufficient?

 It does accept a revision, so any form is allowed; but why would
 anyone want that in a format.defaultTo? I'm not sure we want to impose
 an artificial restriction on the configuration variable though.

I meant a single branch as opposed to depending on what branch
you are sending out, you may have to use a different upstream
starting point, and a single format.defaultTo that does not read
what your HEAD currently points at may not be enough.

Unless you set @{u} to this new configuration, in which case the
choice becomes dynamic depending on the current branch, but

 - if that is the only sane choice based on the current branch, why
   not use that as the default without having to set the
   configuration?

 - Or if that is still insufficient, don't we need branch.*.forkedFrom
   that is different from branch.*.merge, so that different branches
   you want to show format-patch output can have different
   reference points?

After all, format-patch to send things out to upstream is like
asking the other side to do a rebase you would do in your
repository, so whatever git rebase that were too lazy to specify
what the fork point was when applying may be a reasonable type-saver
default.  Yes, sometimes people need to rebase onto somewhere they
did not fork from, but that is why they can give explicit $upstream
and $onto to the command---I do not think it is any different for
format-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: Bug report: stash in upstream caused remote fetch to fail

2014-01-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Mon, Jan 06, 2014 at 08:16:31AM -0800, Junio C Hamano wrote:

  I was going to ask you to send your repository, but I can easily
  reproduce here. I guess people don't run into it because it's uncommon
  to fetch the whole refs/ namespace from a non-bare repo (and bare repos
  do not tend to have stashes). Here's a minimal reproduction recipe:
 
git init repo 
cd repo 
echo content foo 
git add . 
git commit -m foo 
echo more foo 
git stash 
git init --bare sub 
cd sub 
git fetch .. 'refs/*:refs/*'
 
  It looks like we are not feeding refs/stash properly to pack-objects.
  I'll try to take a closer look later today.
 
 I looked at this in the past and I vaguely recall that we reject it
 in the for-each-ref loop with check-ref-format saying eh, that is a
 single-level name.
 
 At that point I stopped digging, thinking it was a feature ;-)
 based on your exact observation about stash vs bare/non-bare.

 I am fine with rejecting it with a warning, but we should not then
 complain that the other side did not send us the object, since we should
 not be asking for it at all. I also do not see us complaining about the
 funny ref anywhere.  So there is definitely _a_ bug here. :)

Oh, no question about that.  I was just pointing somebody who
already has volunteered to take a look in a direction I recall was
where the issue was ;-)

Thanks.


 I think somebody else mentioned recently that we do not handle malformed
 refs consistently. I think it was:

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

 which might or might not be related.

 -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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Jeff King
On Mon, Jan 06, 2014 at 12:06:51PM -0800, Junio C Hamano wrote:

 Unless you set @{u} to this new configuration, in which case the
 choice becomes dynamic depending on the current branch, but
 
  - if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?
 
  - Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?

Yeah, I had similar thoughts. I personally use branch.*.merge as
forkedFrom, and it seems like we are going that way anyway with things
like git rebase and git merge defaulting to upstream. But then there
is git push -u and push.default = upstream, which treats the
upstream config as something else entirely.

So it seems like there is already some confusion, and either way we go,
thisis making it worse to some degree (I do not blame Ram, but rather he
has stumbled into a hidden sand pit that we have been building for the
past few years... :).

I wonder if it is too late to try to clarify this dual usage. It kind of
seems like the push config is this is the place I publish to. Which,
in many workflows, just so happens to be the exact same as the place you
forked from. Could we introduce a new branch.*.pushupstream variable
that falls back to branch.*.merge? Or is that just throwing more fuel on
the fire (more sand in the pit in my analogy, I guess).

I admit I haven't thought it through yet, though. And even if it does
work, it may throw a slight monkey wrench in the proposed push.default
transition.

-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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread John Szakmeister
On Mon, Jan 6, 2014 at 3:18 PM, Jeff King p...@peff.net wrote:
 On Mon, Jan 06, 2014 at 12:06:51PM -0800, Junio C Hamano wrote:

 Unless you set @{u} to this new configuration, in which case the
 choice becomes dynamic depending on the current branch, but

  - if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?

  - Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?

 Yeah, I had similar thoughts. I personally use branch.*.merge as
 forkedFrom, and it seems like we are going that way anyway with things
 like git rebase and git merge defaulting to upstream. But then there
 is git push -u and push.default = upstream, which treats the
 upstream config as something else entirely.

Just for more reference, I rarely use branch.*.merge as
forkedFrom.  I typically want to use master as my target, but like
Ram, I publish my changes elsewhere for safe keeping.  I think in a
typical, feature branch-based workflow @{u} would be nearly useless.

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


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Mon, Jan 06, 2014 at 12:06:51PM -0800, Junio C Hamano wrote:

 Unless you set @{u} to this new configuration, in which case the
 choice becomes dynamic depending on the current branch, but
 
  - if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?
 
  - Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?

 Yeah, I had similar thoughts. I personally use branch.*.merge as
 forkedFrom, and it seems like we are going that way anyway with things
 like git rebase and git merge defaulting to upstream. But then there
 is git push -u and push.default = upstream, which treats the
 upstream config as something else entirely.

 So it seems like there is already some confusion, and either way we go,
 thisis making it worse to some degree (I do not blame Ram, but rather he
 has stumbled into a hidden sand pit that we have been building for the
 past few years... :).

 I wonder if it is too late to try to clarify this dual usage. It kind of
 seems like the push config is this is the place I publish to. Which,
 in many workflows, just so happens to be the exact same as the place you
 forked from. Could we introduce a new branch.*.pushupstream variable
 that falls back to branch.*.merge? Or is that just throwing more fuel on
 the fire (more sand in the pit in my analogy, I guess).

 I admit I haven't thought it through yet, though. And even if it does
 work, it may throw a slight monkey wrench in the proposed push.default
 transition.

Yeah, when I say upstream, I never mean it as where I publish.
Your upstream is where you get others' work from.

For a push to somewhere for safekeeping or other people to look at
triangular workflow, it does not make any sense to treat that I
publish there place as an upstream (hence having branch.*.remote
pointing at that publishing point).  Once you stop doing that, and
instead using branch.*.remote = origin, and branch.*.merge = master,
where 'origin' is not your publishing point, @{u} will again start
making sense, I think.

And I thought that is what setting remote.pushdefault to the
publishing point repository was about.


--
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] [RFC] Making use of bitmaps for thin objects

2014-01-06 Thread Jeff King
On Mon, Jan 06, 2014 at 09:57:23AM -0500, Jeff King wrote:

 diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
 index c733379..0cff874 100644
 --- a/builtin/pack-objects.c
 +++ b/builtin/pack-objects.c
 @@ -1402,6 +1402,19 @@ static void check_object(struct object_entry *entry)
   base_entry-delta_child = entry;
   unuse_pack(w_curs);
   return;
 + } else if(base_ref  bitmap_have(base_ref)) {
 + entry-type = entry-in_pack_type;
 + entry-delta_size = entry-size;
 + /*
 +  * XXX we'll leak this, but it's probably OK
 +  * since we'll exit immediately after the packing
 +  * is done
 +  */
 + entry-delta = xcalloc(1, sizeof(*entry-delta));
 + hashcpy(entry-delta-idx.sha1, base_ref);
 + entry-delta-preferred_base = 1;
 + unuse_pack(w_curs);
 + return;
   }

Just reading over this again, the conditional here should obviously be
checking thin (which needs to become a global, as in your patch).

-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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Jonathan Nieder
John Szakmeister wrote:

I think in a
 typical, feature branch-based workflow @{u} would be nearly useless.

I thought the idea of @{u} was that it represents which ref one
typically wants to compare the current branch to.  It is used by
'git branch -v' to show how far ahead or behind a branch is and
used by 'git pull --rebase' to forward-port a branch, for example.

So a topic branch with @{u} pointing to 'master' or 'origin/master'
seems pretty normal and hopefully the shortcuts it allows can make
life more convenient.

It is *not* primarily about where the branch gets pushed.  After all,
in both the 'matching' and the 'simple' mode, git push does not push
the current branch to its upstream @{u} unless @{u} happens to have
the same name.

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


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Jeff King
On Mon, Jan 06, 2014 at 03:29:57PM -0500, John Szakmeister wrote:

  Yeah, I had similar thoughts. I personally use branch.*.merge as
  forkedFrom, and it seems like we are going that way anyway with things
  like git rebase and git merge defaulting to upstream. But then there
  is git push -u and push.default = upstream, which treats the
  upstream config as something else entirely.
 
 Just for more reference, I rarely use branch.*.merge as
 forkedFrom.  I typically want to use master as my target, but like
 Ram, I publish my changes elsewhere for safe keeping.  I think in a
 typical, feature branch-based workflow @{u} would be nearly useless.

In my feature-branch development for git.git, my upstream is almost
always origin/master[1]. However, sometimes feature branches have
dependencies[2] on each other. Representing that via the upstream
field makes sense, since that is what you forked from, and what you
would want git rebase to start from.

-Peff

[1] I do not even have a local master branch for git.git work, as it
would just be a pain to keep up to date. I am either working
directly on a topic branch, or I am integrating in my own personal
branch.

[2] You should try to minimize dependencies between feature branches, of
course, but sometimes they simply form a logical progression of
features.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Jeff King
On Mon, Jan 06, 2014 at 12:38:30PM -0800, Junio C Hamano wrote:

  I wonder if it is too late to try to clarify this dual usage. It kind of
  seems like the push config is this is the place I publish to. Which,
  in many workflows, just so happens to be the exact same as the place you
  forked from. Could we introduce a new branch.*.pushupstream variable
  that falls back to branch.*.merge? Or is that just throwing more fuel on
  the fire (more sand in the pit in my analogy, I guess).
 
  I admit I haven't thought it through yet, though. And even if it does
  work, it may throw a slight monkey wrench in the proposed push.default
  transition.
 
 Yeah, when I say upstream, I never mean it as where I publish.
 Your upstream is where you get others' work from.

That's my thinking, as well, but it means the upstream push.default is
nonsensical. I've thought that all along, but it seems like other people
find it useful. I guess because they are in a non-triangular,
non-feature-branch setup (I suppose you could think of a central-repo
feature-branch workflow as a special form of triangular setup, where
the remote is bi-directional, but the branch names are triangular).

If we want to declare push -u and push.default=upstream as tools for
certain simple bi-directional workflows, that makes sense. But I suspect
it may cause extra confusion when people make the jump to using a
triangular workflow.

 For a push to somewhere for safekeeping or other people to look at
 triangular workflow, it does not make any sense to treat that I
 publish there place as an upstream (hence having branch.*.remote
 pointing at that publishing point).

You _might_ treat it the same way we treat the upstream, in some special
cases. For example, when you say git status, it is useful to see how
your topic and the upstream have progressed (i.e., do I need to pull
from upstream?). But you may _also_ want to know how your local branch
differs from its pushed counterpart (i.e., do I have unsaved commits
here that I want to push up?).

So having two config options might help with that. Of course, your push
upstream (or whatever you want to call it) does not logically have one
value. You may push to several places, and would want to compare to
each.

 Once you stop doing that, and
 instead using branch.*.remote = origin, and branch.*.merge = master,
 where 'origin' is not your publishing point, @{u} will again start
 making sense, I think.
 
 And I thought that is what setting remote.pushdefault to the
 publishing point repository was about.

If that were sufficient, then we would just need push.default =
current, and not upstream (nor simple). I lobbied for that during
the discussion, but people seemed to think that upstream was
better/more useful. Maybe it was just because remote.pushdefault did not
exist then.

-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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread John Szakmeister
On Mon, Jan 6, 2014 at 3:42 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 John Szakmeister wrote:

I think in a
 typical, feature branch-based workflow @{u} would be nearly useless.

 I thought the idea of @{u} was that it represents which ref one
 typically wants to compare the current branch to.  It is used by
 'git branch -v' to show how far ahead or behind a branch is and
 used by 'git pull --rebase' to forward-port a branch, for example.

 So a topic branch with @{u} pointing to 'master' or 'origin/master'
 seems pretty normal and hopefully the shortcuts it allows can make
 life more convenient.

Is there an outline of this git workflow in the documentation
somewhere?  Do you save your work in a forked repo anywhere?  If so,
how do you typically save your work.  I typically have my @{u}
pointing to where I save my work.  Perhaps I'm missing something
important here, but I don't feel like the current command set and
typical workflow (at least those in tutorials) leads you in that
direction.

Here is one example:
   https://www.atlassian.com/git/workflows#!workflow-feature-branch

 It is *not* primarily about where the branch gets pushed.  After all,
 in both the 'matching' and the 'simple' mode, git push does not push
 the current branch to its upstream @{u} unless @{u} happens to have
 the same name.

Then where does it get pushed?  Do you always specify where to save your work?

FWIW, I think the idea of treating @{u} as the eventual recipient of
your changes is good, but then it seems like Git is lacking the
publish my changes to this other branch concept.

Am I missing something?  If there is something other than @{u} to
represent this latter concept, I think `git push` should default to
that instead.  But, at least with my current knowledge, that doesn't
exist--without explicitly saying so--or treating @{u} as that branch.
If there's a better way to do this, I'd love to hear it!

Thanks!

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


RE: [PATCH] [RFC] Making use of bitmaps for thin objects

2014-01-06 Thread Ben Maurer
 Sorry for the slow reply; I've been on vacation.

No worries.

 When you build your bitmaps, do you set the pack.writeBitmapHashCache
 option? We found that it makes a significant difference during the
 compression phase, as otherwise git attempts deltas between random files
 based on size. Here are some numbers for a simulated fetch from
 torvalds/linux, representing about 7 weeks of history. Running:

Yeah, I enabled this option. I don't have timings for generating the bitmap 
index without this option unfortunately (this is a pretty large repo, doing the 
full repack that I needed to do to generate the first version took 15+ minutes)

 Having been confused by it myself before, I think the magic keyword is
 preferred base. Once you know that, the code makes more sense. :)

Ah, nice! That was pretty confusing :-)

 Let me know how this patch does for you. My testing has been fairly
 limited so far.

This patch looks like a much cleaner version :-). Works well for me, but my 
test setup may not be great since I didn't get any errors from completely 
ignoring the haves bitmap :-). --
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Mon, Jan 06, 2014 at 12:38:30PM -0800, Junio C Hamano wrote:

  I wonder if it is too late to try to clarify this dual usage. It kind of
  seems like the push config is this is the place I publish to. Which,
  in many workflows, just so happens to be the exact same as the place you
  forked from. Could we introduce a new branch.*.pushupstream variable
  that falls back to branch.*.merge? Or is that just throwing more fuel on
  the fire (more sand in the pit in my analogy, I guess).
 
  I admit I haven't thought it through yet, though. And even if it does
  work, it may throw a slight monkey wrench in the proposed push.default
  transition.
 
 Yeah, when I say upstream, I never mean it as where I publish.
 Your upstream is where you get others' work from.

 That's my thinking, as well, but it means the upstream push.default is
 nonsensical. I've thought that all along, but it seems like other people
 find it useful. I guess because they are in a non-triangular,
 non-feature-branch setup (I suppose you could think of a central-repo
 feature-branch workflow as a special form of triangular setup, where
 the remote is bi-directional, but the branch names are triangular).

 If we want to declare push -u and push.default=upstream as
 tools for certain simple bi-directional workflows, that makes
 sense.  But I suspect it may cause extra confusion when people
 make the jump to using a triangular workflow.

I do not think there is no want to declare involved.  If I
correctly recall how push -u came about, it was merely a way to
appease those who complained that their new branch created by
running checkout -b branch origin/branch has already set up the
branch.*.remote and branch.*.merge configurations nicely for them
and allow them to immediately go ahead and start using the
centralized I merge from their 'branch' and push to that, but when
they create a new branch on their own and want to make the branch of
the same name at the origin to be the upstream, they have to futz
with the configuration.  The declaration was made long time ago when
we started using @{upstream}, and there is no new extra confusion.

 For a push to somewhere for safekeeping or other people to look at
 triangular workflow, it does not make any sense to treat that I
 publish there place as an upstream (hence having branch.*.remote
 pointing at that publishing point).

 You _might_ treat it the same way we treat the upstream, in some special
 cases. For example, when you say git status, it is useful to see how
 your topic and the upstream have progressed (i.e., do I need to pull
 from upstream?). But you may _also_ want to know how your local branch
 differs from its pushed counterpart (i.e., do I have unsaved commits
 here that I want to push up?).

Correct; I am not saying where do I publish is never relevant.  It
is just it is not something useful for format-patch to use as the
default fork-point.

 So having two config options might help with that. Of course, your push
 upstream (or whatever you want to call it) does not logically have one
 value. You may push to several places, and would want to compare to
 each.

Yes.  But most likely, if you always push a single branch to
multiple places, it won't be like you push it to only one of the
places today and another one tomorrow, leaving everybody out of
sync.  Unless there is a site that is temporarily down, in which
case that one may stay behind, the normal state would be that all of
them point at the same commit (that is how I publish to multiple
places anyway).

 Once you stop doing that, and
 instead using branch.*.remote = origin, and branch.*.merge = master,
 where 'origin' is not your publishing point, @{u} will again start
 making sense, I think.
 
 And I thought that is what setting remote.pushdefault to the
 publishing point repository was about.

 If that were sufficient, then we would just need push.default =
 current, and not upstream (nor simple). I lobbied for that during
 the discussion, but people seemed to think that upstream was
 better/more useful. Maybe it was just because remote.pushdefault did not
 exist then.

Yeah, I think in the 2.0 world with pushdefault (i.e. triangular),
the default 'simple' turns into 'current'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-06 Thread Junio C Hamano
W. Trevor King wk...@tremily.us writes:

 And wouldn't it make it unnecessary to have a new re-attach option
 if such a mode that never have to detach is used?

 I think so, but we currently don't have a never detached route for
 folks that are cloning submodules via update (instead of via
 'submodule add').  Currently, new clone-updates will always leave you
 with a detached HEAD (unless you have branch-creation in your update
 !command).  My patch aims to close this detached-HEAD gap, for folks
 we expect will be doing local development, by creating an initial
 branch at clone-update time.

I am not a submodule expert so I may be missing some other gaps, but
what your change does sounds sensible to 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: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Junio C Hamano
John Szakmeister j...@szakmeister.net writes:

 Am I missing something?  If there is something other than @{u} to
 represent this latter concept, I think `git push` should default to
 that instead.  But, at least with my current knowledge, that doesn't
 exist--without explicitly saying so--or treating @{u} as that branch.
 If there's a better way to do this, I'd love to hear it!

I see Ram who worked on landing the remote.pushdefault feature is
CC'ed; this work was started in early April 2013 and your config and
workflow may not have been adjusted to it.
--
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] remote-hg: do not fail on invalid bookmarks

2014-01-06 Thread Antoine Pelisse
Thanks for noticing,
I can reproduce at work, I will try to come-up with an improved version soon,

Cheers,
Antoine

On Mon, Jan 6, 2014 at 2:52 PM, Torsten Bögershausen tbo...@web.de wrote:
 On 2013-12-29 12.30, Antoine Pelisse wrote:
 Mercurial can have bookmarks pointing to nullid (the empty root
 revision), while Git can not have references to it.
 When cloning or fetching from a Mercurial repository that has such a
 bookmark, the import will fail because git-remote-hg will not be able to
 create the corresponding reference.

 Warn the user about the invalid reference, and continue the import,
 instead of stopping right away.

 Signed-off-by: Antoine Pelisse apeli...@gmail.com
 ---
  contrib/remote-helpers/git-remote-hg | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/contrib/remote-helpers/git-remote-hg 
 b/contrib/remote-helpers/git-remote-hg
 index eb89ef6..12d850e 100755
 --- a/contrib/remote-helpers/git-remote-hg
 +++ b/contrib/remote-helpers/git-remote-hg
 @@ -625,6 +625,9 @@ def list_head(repo, cur):
  def do_list(parser):
  repo = parser.repo
  for bmark, node in bookmarks.listbookmarks(repo).iteritems():
 +if node == '':
 +warn(Ignoring invalid bookmark '%s', bmark)
 +continue
  bmarks[bmark] = repo[node]

  cur = repo.dirstate.branch()

 (Side note: ap/remote-hg-skip-null-bookmarks)

 When I run the test-suite like this:
 ~/projects/git/git.pu/contrib/remote-helpers$ debug=t verbose=t make 
 test-hg-hg-git.sh

 All 11 test cases fail on my systems (Debian Wheezy and Mac OS X):
 [snip]
 WARNING: Ignoring invalid bookmark 'master'
 To hg::../hgrepo-git
  ! [remote rejected] master - master
 error: failed to push some refs to 'hg::../hgrepo-git'
 not ok 1 - executable bit
 #
 [snip]


--
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] [RFC] Making use of bitmaps for thin objects

2014-01-06 Thread Jeff King
On Mon, Jan 06, 2014 at 09:15:04PM +, Ben Maurer wrote:

  Let me know how this patch does for you. My testing has been fairly
  limited so far.
 
 This patch looks like a much cleaner version :-). Works well for me,
 but my test setup may not be great since I didn't get any errors from
 completely ignoring the haves bitmap :-).

Great. Out of curiosity, can you show the before/after? The timings
should be similar to what your patch produced, but I'm really curious to
see how the pack size changes.

-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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 I meant a single branch as opposed to depending on what branch
 you are sending out, you may have to use a different upstream
 starting point, and a single format.defaultTo that does not read
 what your HEAD currently points at may not be enough.

 Unless you set @{u} to this new configuration, in which case the
 choice becomes dynamic depending on the current branch, but

  - if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?

  - Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?

Ah, I was going for an equivalent of merge.defaultToUpstream, but I
guess branch.*.forkedFrom is a good way to go.
--
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


static initializers

2014-01-06 Thread Stefan Zager
Howdy,

Is there any policy on making static initializers thread-safe?  I'm
working on an experimental patch to introduce threading, but I'm
running into a few non-thread-safe bits like this, in convert.c:

static const char *conv_attr_name[] = {
crlf, ident, filter, eol, text,
};
#define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name)

static void convert_attrs(struct conv_attrs *ca, const char *path)
{
int i;
static struct git_attr_check ccheck[NUM_CONV_ATTRS];

if (!ccheck[0].attr) {
for (i = 0; i  NUM_CONV_ATTRS; i++)
ccheck[i].attr = git_attr(conv_attr_name[i]);
user_convert_tail = user_convert;
git_config(read_convert_config, NULL);
}
}



The easy fix would be to stick the initialization bit into an 'extern
int init_convert_config();' function, and invoke it prior to the
threaded part of my code.  That would be no worse than the current
state of affairs, which is to say that adding threading is rife with
hidden perils.

A more comprehensive fix might be:

#include pthread.h

static pthread_mutex_t convert_config_mutex = PTHREAD_MUTEX_INITIALIZER;

static void convert_attrs(struct conv_attrs *ca, const char *path)
{
int i;
static struct git_attr_check ccheck[NUM_CONV_ATTRS];

pthread_mutex_lock(convert_config_mutex);
if (!ccheck[0].attr) {
for (i = 0; i  NUM_CONV_ATTRS; i++)
ccheck[i].attr = git_attr(conv_attr_name[i]);
user_convert_tail = user_convert;
git_config(read_convert_config, NULL);
}
pthread_mutex_unlock(convert_config_mutex);
}


Unfortunately, I don't think mingw/msys supports
PTHREAD_MUTEX_INITIALIZER.  A possible workaround would be:

static pthread_mutex_t convert_config_mutex;

static void init_convert_config_mutex() __attribute__((constructor));
static void init_convert_config_mutex()
{
pthread_mutex_init(convert_config_mutex);
}


But then, I'm not whether mingw/msys supports __attribute__(constructor).


Can anyone give me some guidance before I go to much further into the
weeds (and I'm neck-deep as it is)?

Thanks,

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: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Jeff King wrote:
 Yeah, I had similar thoughts. I personally use branch.*.merge as
 forkedFrom, and it seems like we are going that way anyway with things
 like git rebase and git merge defaulting to upstream.

My issue with that is that I no idea where my branch is with respect
to my forked upstream; I find that extremely useful when doing
re-spins.

 But then there
 is git push -u and push.default = upstream, which treats the
 upstream config as something else entirely.

push.default = upstream is a bit of a disaster, in my opinion. I've
advocated push.default = current on multiple occasions, and wrote the
initial remote.pushDefault series with that configuration in mind.

 I wonder if it is too late to try to clarify this dual usage. It kind of
 seems like the push config is this is the place I publish to. Which,
 in many workflows, just so happens to be the exact same as the place you
 forked from. Could we introduce a new branch.*.pushupstream variable
 that falls back to branch.*.merge? Or is that just throwing more fuel on
 the fire (more sand in the pit in my analogy, I guess).

We already have a branch.*.pushremote, and I don't see the value of
branch.*.pushbranch (what you're referring to as pushupstream, I
assume) except for Gerrit users. Frankly, I don't use full triangular
workflows myself mainly because my prompt is compromised: when I have
a branch.*.remote different from branch.*.pushremote, I'd like to see
where my branch is with respect to @{u} and @{publish} (not yet
invented); that's probably too much information to digest anyway, so I
use central workflow (pointing to my fork) for each of my branches,
except master (which points to Junio's repo).

 I admit I haven't thought it through yet, though. And even if it does
 work, it may throw a slight monkey wrench in the proposed push.default
 transition.

We're transitioning to push.default = simple which is even simpler than current.
--
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] [RFC] Making use of bitmaps for thin objects

2014-01-06 Thread Ben Maurer
It looks like for my repo the size win wasn't as big (~10%). Is it possible 
that with the kernel test you got extremely lucky and there was some huge 
binary blob that thin packing turned into a tiny delta?

The repo I'm testing with here isn't a typical codebase -- it is used to store 
configuration information and it has very different update patterns than most 
codebases.

When you get a chance, it'd be handy if you could push an updated version of 
your change out to your public github repo. I'd like to see if folks here are 
interested in testing this more, and it'd be good to make sure we're testing 
the diff that is targeted for upstream.

Bitmap index, without thin packing:

Counting objects: 158825, done.
Delta compression using up to 32 threads.
Compressing objects: 100% (18113/18113), done.
Writing objects: 100% (158825/158825), 89.87 MiB | 11.23 MiB/s, done.
Total 158825 (delta 139493), reused 153076 (delta 135730)
real 15.60
user 34.38
sys 2.99


Bitmap index, with thin packing:

Counting objects: 158825, done.
Delta compression using up to 32 threads.
Compressing objects: 100% (12364/12364), done.
Writing objects: 100% (158825/158825), 81.35 MiB | 0 bytes/s, done.
Total 158825 (delta 135730), reused 158825 (delta 141479)
real 2.70
user 2.28
sys 0.65



From: Jeff King [p...@peff.net]
Sent: Monday, January 06, 2014 1:57 PM
To: Ben Maurer
Cc: git@vger.kernel.org
Subject: Re: [PATCH] [RFC] Making use of bitmaps for thin objects

On Mon, Jan 06, 2014 at 09:15:04PM +, Ben Maurer wrote:

  Let me know how this patch does for you. My testing has been fairly
  limited so far.

 This patch looks like a much cleaner version :-). Works well for me,
 but my test setup may not be great since I didn't get any errors from
 completely ignoring the haves bitmap :-).

Great. Out of curiosity, can you show the before/after? The timings
should be similar to what your patch produced, but I'm really curious to
see how the pack size changes.

-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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 I meant a single branch as opposed to depending on what branch
 you are sending out, you may have to use a different upstream
 starting point, and a single format.defaultTo that does not read
 what your HEAD currently points at may not be enough.

 Unless you set @{u} to this new configuration, in which case the
 choice becomes dynamic depending on the current branch, but

  - if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?

  - Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?

 Ah, I was going for an equivalent of merge.defaultToUpstream, but I
 guess branch.*.forkedFrom is a good way to go.

As I said in the different subthread, I am not convinced that you
would need the complexity of branch.*.forkedFrom.  If you set your
upstream to the true upstream (not your publishing point), and
have remote.pushdefault set to 'publish', you can expect

git push

to do the right thing, and then always say

git show-branch publish/topic topic

to see where your last published branch is relative to what you
have, no?  Mapping topic@{publish} to refs/remotes/publish/topic
(or when you have 'topic' checked out, mapping @{publish} to it)
is a trivial but is a separate usefulness, I would guess.




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


What's cooking in git.git (Jan 2014, #01; Mon, 6)

2014-01-06 Thread Junio C Hamano
Welcome to the first issue of What's cooking report for the new
year.

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to master]

* fc/remote-helper-fixes (2013-12-26) 5 commits
  (merged to 'next' on 2013-12-26 at ce5f872)
 + remote-hg: test 'shared_path' in a moved clone
  (merged to 'next' on 2013-12-17 at aa4dc07)
 + remote-hg: add tests for special filenames
 + remote-hg: fix 'shared path' path
 + remote-helpers: add extra safety checks
 + remote-hg: avoid buggy strftime()


* jc/push-refmap (2013-12-04) 3 commits
  (merged to 'next' on 2013-12-12 at 71e358f)
 + push: also use upstream mapping when pushing a single ref
 + push: use remote.$name.push as a refmap
 + builtin/push.c: use strbuf instead of manual allocation

 Make git push origin master update the same ref that would be
 updated by our 'master' when git push origin (no refspecs) is run
 while the 'master' branch is checked out, which makes git push
 more symmetric to git fetch and more usable for the triangular
 workflow.


* jk/cat-file-regression-fix (2013-12-12) 2 commits
  (merged to 'next' on 2013-12-13 at 3713e01)
 + cat-file: handle --batch format with missing type/size
 + cat-file: pass expand_data to print_object_or_die

 git cat-file --batch=, an admittedly useless command, did not
 behave very well.


* jk/name-pack-after-byte-representation (2013-12-16) 3 commits
  (merged to 'next' on 2013-12-17 at 0bc385c)
 + pack-objects doc: treat output filename as opaque
  (merged to 'next' on 2013-12-09 at 247b2d0)
 + pack-objects: name pack files after trailer hash
 + sha1write: make buffer const-correct
 (this branch is tangled with jk/pack-bitmap.)

 Two packfiles that contain the same set of objects have
 traditionally been named identically, but that made repacking a
 repository that is already fully packed without any cruft with a
 different packing parameter cumbersome. Update the convention to
 name the packfile after the bytestream representation of the data,
 not after the set of objects in it.


* jk/pull-rebase-using-fork-point (2013-12-10) 2 commits
  (merged to 'next' on 2013-12-13 at 1862c12)
 + rebase: use reflog to find common base with upstream
 + pull: use merge-base --fork-point when appropriate


* jk/rev-parse-double-dashes (2013-12-09) 2 commits
  (merged to 'next' on 2013-12-13 at d26bac7)
 + rev-parse: be more careful with munging arguments
 + rev-parse: correctly diagnose revision errors before --

 git rev-parse revs -- paths did not implement the usual
 disambiguation rules the commands in the git log family used in
 the same way.


* js/gnome-keyring (2013-12-16) 1 commit
  (merged to 'next' on 2013-12-17 at 422fd61)
 + contrib/git-credential-gnome-keyring.c: small stylistic cleanups

 Style fix.


* tg/diff-no-index-refactor (2013-12-16) 4 commits
  (merged to 'next' on 2013-12-17 at 009d8d8)
 + diff: avoid some nesting
 + diff: add test for --no-index executed outside repo
  (merged to 'next' on 2013-12-13 at 523f7c4)
 + diff: don't read index when --no-index is given
 + diff: move no-index detection to builtin/diff.c

 git diff ../else/where/A ../else/where/B when ../else/where is
 clearly outside the repository, and git diff --no-index A B, do
 not have to look at the index at all, but we used to read the index
 unconditionally.


* zk/difftool-counts (2013-12-16) 2 commits
  (merged to 'next' on 2013-12-16 at 0e0d235)
 + diff.c: fix some recent whitespace style violations
  (merged to 'next' on 2013-12-12 at ba35694)
 + difftool: display the number of files in the diff queue in the prompt

 Show the total number of paths and the number of paths shown so far
 when git difftool prompts to launch an external diff tool, which
 would give users some sense of progress.

--
[New Topics]

* ta/format-user-manual-as-an-article (2014-01-06) 1 commit
  (merged to 'next' on 2014-01-06 at 37858f6)
 + user-manual: improve html and pdf formatting

 Update the way the user-manual is formatted via AsciiDoc to save
 trees.

 Will merge to 'master'.


* bm/merge-base-octopus-dedup (2013-12-30) 2 commits
  (merged to 'next' on 2014-01-06 at 355d62b)
 + merge-base --octopus: reduce the result from get_octopus_merge_bases()
 + merge-base: separate --independent codepath into its own helper

 git merge-base --octopus used to leave cleaning up suboptimal
 result to the caller, but now it does the clean-up itself.

 Will merge to 'master'.


* jk/test-framework-updates (2014-01-02) 3 commits
  (merged to 'next' on 2014-01-06 at f81f373)
 + t: drop known breakage test
 + t: simplify HARNESS_ACTIVE hack
 + t: set TEST_OUTPUT_DIRECTORY for 

Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:.
 As I said in the different subthread, I am not convinced that you
 would need the complexity of branch.*.forkedFrom.  If you set your
 upstream to the true upstream (not your publishing point), and
 have remote.pushdefaultset to 'publish', you can expect

 git push

 to do the right thing, and then always say

 git show-branch publish/topic topic

I think it's highly sub-optimal to have a local-branch @{u} for
several reasons; the prompt is almost useless in this case, and it
will always show your forked-branch ahead of 'master' (assuming that
'master' doesn't update itself in the duration of your development).
While doing respins, the prompt doesn't aid you in any way. Besides,
on several occasions, I found myself working on the same forked-branch
from two different machines; then the publish-point isn't necessarily
always a publish-point: it's just another upstream for the branch.
The point of a special branch.*.forkedFrom is that you can always show
the master..@ information in the prompt ignoring divergences; after
all, a divergence simply means that you need to rebase onto the latest
'master' ('master' is never going to get a non-ff update anyway).

So, instead of crippling the functionality around the publish-point,
let's build minimal required functionality around branch.*.forkedFrom.
I'd love a prompt like:

  branch-forkedfrom5*:~/src/git$

Clearly, branch-forkedfrom has diverged from my publish-point (I'm
probably doing a respin), and has 5 commits (it's 5 commits ahead of
'master' ignoring divergences).

 to see where your last published branch is relative to what you
 have, no?  Mapping topic@{publish} to refs/remotes/publish/topic
 (or when you have 'topic' checked out, mapping @{publish} to it)
 is a trivial but is a separate usefulness, I would guess.

I think a @{publish} is needed for when branch.*.remote is different
from remote.pushDefault. Like I said earlier, it's probably too much
information to give to the user: divergences with respect to two
remotes. So, we'll hold it off until someone finds a usecase for it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
John Szakmeister wrote:
 Then where does it get pushed?  Do you always specify where to save your work?

 FWIW, I think the idea of treating @{u} as the eventual recipient of
 your changes is good, but then it seems like Git is lacking the
 publish my changes to this other branch concept.

 Am I missing something?  If there is something other than @{u} to
 represent this latter concept, I think `git push` should default to
 that instead.  But, at least with my current knowledge, that doesn't
 exist--without explicitly saying so--or treating @{u} as that branch.
 If there's a better way to do this, I'd love to hear it!

That's why we invented remote.pushdefault and branch.*.pushremote. When you say

  $ git push

it automatically goes to the right remote instead of going to the
place you fetched from. You can read up on how push.default interacts
with this setting too, although I always recommend push.default =
current.
--
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 report: stash in upstream caused remote fetch to fail

2014-01-06 Thread Jeff King
On Mon, Jan 06, 2014 at 12:17:21PM -0800, Junio C Hamano wrote:

  I am fine with rejecting it with a warning, but we should not then
  complain that the other side did not send us the object, since we should
  not be asking for it at all. I also do not see us complaining about the
  funny ref anywhere.  So there is definitely _a_ bug here. :)
 
 Oh, no question about that.  I was just pointing somebody who
 already has volunteered to take a look in a direction I recall was
 where the issue was ;-)

Oh, crud, did I volunteer? :)

So I found the problem, but I'm really not sure what to make of it. We
do check the refname format when evaluating the refspecs, in:

do_fetch
  get_ref_map
get_fetch_map
  check_refname_format

Before calling it, we check that it starts with refs/, and then pass
the _whole_ refname into check_refname_format. So the latter sees
refs/stash. And that's acceptable, as it's not a single-level ref.
Thus we do not get the funny ref message.

The code looks like this:

  if (!starts_with((*rmp)-peer_ref-name, refs/) ||
  check_refname_format((*rmp)-peer_ref-name, 0)) {
/* print funny ref and ignore */

Then we ask fetch_refs_via_pack to get the actual objects for us. And
it checks our refs again, with this call chain:

  do_fetch
fetch_refs
  transport_fetch_refs
fetch_refs_via_pack
  fetch_pack
do_fetch_pack
  everything_local
filter_refs
  check_refname_format

Here, the code looks like this:

  if (!memcmp(ref-name, refs/, 5) 
  check_refname_format(ref-name + 5, 0))
; /* trash */

At first I thought we are doing the same check (is it syntactically
valid, and is it in refs/), but we're not. We are actually checking
the format _only_ of stuff in refs/, and ignoring the rest. Which
really makes no sense to me.

If it were memcmp(...) || check_refname_format(), then it would be
roughly the same check. But it would still be wrong, because note that
we pass only the bits under refs/ to check_refname_format. So it sees
only stash, and then complains that it is single-level.

So the symptom we are seeing is because we are filtering with two
different rulesets in different places. But I'm really not sure how to
resolve it. The one in filter_refs seems nonsensical to me.

Checking _only_ things under refs/ doesn't make sense. And even if that
was sensible, feeding half a ref to check_refname_format does not work.
In addition to the single-level check, it has other rules that want
to see the whole ref (e.g., the ref @ is not valid, but refs/@ is
OK; it cannot distinguish them without seeing the prefix).

So I can see two options:

  1. Make the check in filter_refs look like the one in get_fetch_map.
 That at least makes them the same, which alleviates the symptom.
 But we still are running two checks, and if they ever get out of
 sync, it causes problems.

  2. Abolish the check in filter_refs. I think this makes the most sense
 from the perspective of fetch, because we will already have cleaned up
 the ref list. But we might need to leave the filtering in place for
 people who call fetch-pack as a bare plumbing command.

It's really not clear to me what the check in filter_refs was trying to
do. It dates all the way back to 1baaae5 (Make maximal use of the remote
refs, 2005-10-28), but there is not much explanation. I haven't dug into
the list around that time to see if there's any discussion.

-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: Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-06 Thread Francesco Pretto
2014/1/6 Heiko Voigt hvo...@hvoigt.net:

 I agree. If we were to support this more easily we could add a
 configuration option so you can omit the --remote (i.e.:
 submodule.name.remote=true, as I also suggested in the other email).

 That way the developer checking out a branch in flight does not even
 need to know whether (and which) submodules sha1s are still in flight
 and temporarily set this configuration in the branches .gitmodules file.


submodule.name.remote can be useful but can be added later to aid
the current use case.

To not break the existing behavior what it's really needed here, IMO,
is a submodule.name.attached property that says two things:
- at the first clone on git submodule update stay attached to
submodule.name.branch;
- implies --remote, as it's the only thing that makes sense when the
submodules are attached.

My patch at the current unreleased state does exactly this.

 Maybe that could actually be the attach operation Francesco is
 suggesting:

 git submodule attach [--pull] submodule path branchname

 will attach the specified submodule to a branch. That means it changes
 the .gitmodule file accordingly and stages it. With the --pull switch
 one can specify whether a local branch tracking the remote branch should
 be automatically created. Names and the command format are just a
 suggestion here.

 That way we can support the

 fork superproject needing submodule changes and send submodule
 changes upstream first.


My patch didn't do this, as the maintainer can do these things quite
easily[1] (maintainer is cooler with respect to other devs :) ), but
I think it could be good to also have this feature.

The feature I think that are still needed and you don't mention are:
- an --attached switch for the add command when the maintainer
create the submodule the first time (DONE in patch);
- a easy way to attach|detach the submodule locally by developer. This should:
* fix the head state (DONE in patch);
* fix the local .git/config submodule.name.attached property
accordingly (DONE in patch, unreleased).

I do the latest in the update command but it seems bad to touch
.git/config in the update command...

Maybe we should have a git submodule head command that does all
these things: --attach (for the maintainer), --attach|--detach (for
the developer).

[1]
$ ( cd submodule  git branch newbranch  git push -u origin HEAD)
$ git config -f .gitmodules submodule.newbranch.branch newbranch
$ git config -f .gitmodules submodule.newbranch.attached true
$ git add .  git commit -m Forked superproject  git push
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)

2014-01-06 Thread Francesco Pretto
2014/1/6 Junio C Hamano gits...@pobox.com:

  - git-submodule.sh: support 'checkout' as a valid update mode

  Need to pick up a rerolled one.


I resent it, can you see it?

Thank you,
Francesco
--
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 report: stash in upstream caused remote fetch to fail

2014-01-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Then we ask fetch_refs_via_pack to get the actual objects for us. And
 it checks our refs again, with this call chain:

   do_fetch
 fetch_refs
   transport_fetch_refs
 fetch_refs_via_pack
   fetch_pack
 do_fetch_pack
   everything_local
 filter_refs
   check_refname_format

 Here, the code looks like this:

   if (!memcmp(ref-name, refs/, 5) 
   check_refname_format(ref-name + 5, 0))
 ; /* trash */

I think this should feed ref-name, not ref-name + 5; an obvious
alternative would be to use REFNAME_ALLOW_ONELEVEL; you are also
right that  is probably a misspelt ||; this is about protecting
ourselves from creating garbage in our ref namespace, so accepting
anything outside refs/ makes little sense.

 It's really not clear to me what the check in filter_refs was trying to
 do. It dates all the way back to 1baaae5 (Make maximal use of the remote
 refs, 2005-10-28), but there is not much explanation. I haven't dug into
 the list around that time to see if there's any discussion.

I think the funny refs the log message talks about is about
filtering we know we can reach these objects via our alternates,
but these are not refs we actually have.


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


Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)

2014-01-06 Thread Junio C Hamano
Francesco Pretto ceztk...@gmail.com writes:

 2014/1/6 Junio C Hamano gits...@pobox.com:

  - git-submodule.sh: support 'checkout' as a valid update mode

  Need to pick up a rerolled one.


 I resent it, can you see it?

I know. I saw it and that is why I left the note to self.

The thing is, it takes a non trivial amount of time to run through a
single day's integration cycle, and any reroll that comes later in a
day once the cycle started may be too late for that day.  Otherwise
I have to discard the the result of earlier merges and tests and
start over from scratch.


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


Re: Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-06 Thread Francesco Pretto
2014/1/7 Francesco Pretto cez...@gmail.com:
 To not break the existing behavior what it's really needed here, IMO,
 is a submodule.name.attached property that says two things:
 - at the first clone on git submodule update stay attached to
 submodule.name.branch;
 - implies --remote, as it's the only thing that makes sense when the
 submodules are attached.


Unless you decide to go with the proposed approach of Trevor, where
submodule.name.branch set means attached (if it's not changed:
this thread is quite hard to follow...). To this end, Junio could sync
with more long-timers (Heiko?) submodule users/devs to understand if
this breaks too much or not.
--
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] mv: better document side effects when moving a submodule

2014-01-06 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 The Submodules section of the git mv documentation mentions what will
 happen when a submodule with a gitfile gets moved with newer git. But it
 doesn't talk about what happens when the user changes between commits
 before and after the move, which does not update the work tree like using
 the mv command did the first time.

 Explain what happens and what the user has to do manually to fix that.
 Also document this in a new test.

 Reported-by: George Papanikolaou g3orge@gmail.com
 Signed-off-by: Jens Lehmann jens.lehm...@web.de
 ---

 Am 09.12.2013 18:49, schrieb Jens Lehmann:
 Am 09.12.2013 11:59, schrieb George Papanikolaou:
 Also after mv you need to run 'submodule update' and I think this should be
 documented somewhere.
 
 You're right, this should be mentioned in the mv man page. I'll
 prepare a patch for that.

 Does this new paragraph make it clearer?

Don't we have bugs section that we can use to list the known
limitations like this?

  Documentation/git-mv.txt | 10 ++
  t/t7001-mv.sh| 21 +

It also may make sense to express the test as this is what we would
like to see happen eventually in the form of test_expect_failure;
it is not a big deal though.

  2 files changed, 31 insertions(+)

 diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt
 index b1f7988..c9e8568 100644
 --- a/Documentation/git-mv.txt
 +++ b/Documentation/git-mv.txt
 @@ -52,6 +52,16 @@ core.worktree setting to make the submodule work in the 
 new location.
  It also will attempt to update the submodule.name.path setting in
  the linkgit:gitmodules[5] file and stage that file (unless -n is used).

 +Please note that each time a superproject update moves a populated
 +submodule (e.g. when switching between commits before and after the
 +move) a stale submodule checkout will remain in the old location
 +and an empty directory will appear in the new location. To populate
 +the submodule again in the new location the user will have to run
 +git submodule update afterwards. Removing the old directory is
 +only safe when it uses a gitfile, as otherwise the history of the
 +submodule will be deleted too. Both steps will be obsolete when
 +recursive submodule update has been implemented.
 +
  GIT
  ---
  Part of the linkgit:git[1] suite
 diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
 index 3bfdfed..e3c8c2c 100755
 --- a/t/t7001-mv.sh
 +++ b/t/t7001-mv.sh
 @@ -442,4 +442,25 @@ test_expect_success 'mv --dry-run does not touch the 
 submodule or .gitmodules' '
   git diff-files --quiet -- sub .gitmodules
  '

 +test_expect_success 'checking out a commit before submodule moved needs 
 manual updates' '
 + git mv sub sub2 
 + git commit -m moved sub to sub2 
 + git checkout -q HEAD^ 2actual 
 + echo warning: unable to rmdir sub2: Directory not empty expected 
 + test_i18ncmp expected actual 
 + git status -s sub2 actual 
 + echo ?? sub2/ expected 
 + test_cmp expected actual 
 + ! test -f sub/.git 
 + test -f sub2/.git 
 + git submodule update 
 + test -f sub/.git 
 + rm -rf sub2 
 + git diff-index --exit-code HEAD 
 + git update-index --refresh 
 + git diff-files --quiet -- sub .gitmodules 
 + git status -s sub2 actual 
 + ! test -s actual
 +'
 +
  test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)

2014-01-06 Thread Francesco Pretto
2014/1/7 Junio C Hamano gits...@pobox.com:
 The thing is, it takes a non trivial amount of time to run through a
 single day's integration cycle, and any reroll that comes later in a
 day once the cycle started may be too late for that day.  Otherwise
 I have to discard the the result of earlier merges and tests and
 start over from scratch.


Got it, thank you.
--
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-submodule.sh: Support 'checkout' as a valid update command

2014-01-06 Thread Junio C Hamano
Francesco Pretto cez...@gmail.com writes:

 According to Documentation/gitmodules.txt, 'checkout' is a valid
 'submodule.name.update' command.

As you can see in the surrounding text, we call the value of
submodule.*.update a mode, not a command.

 Also git-submodule.sh refers to
 it and processes it correctly.

This present tense puzzles me.  If it already refers to checkout and
handles it correctly is there anything that needs to be done?  Or
did you mean it should refer to and process it but it doesn't, so
make it so?

 Reflecting commit 'ac1fbb' to support
 this syntax and also validate property values during 'update' command,
 issuing an error if the value found is unknown.

Sorry, but -ECANNOTPARSE.
--
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-submodule.sh: Support 'checkout' as a valid update command

2014-01-06 Thread Francesco Pretto
2014/1/7 Junio C Hamano gits...@pobox.com:
 Francesco Pretto cez...@gmail.com writes:

 According to Documentation/gitmodules.txt, 'checkout' is a valid
 'submodule.name.update' command.

 As you can see in the surrounding text, we call the value of
 submodule.*.update a mode, not a command.


Ok.

 Also git-submodule.sh refers to
 it and processes it correctly.

 This present tense puzzles me.  If it already refers to checkout and
 handles it correctly is there anything that needs to be done?  Or
 did you mean it should refer to and process it but it doesn't, so
 make it so?


Like you said, it already refers to checkout and handles it
correctly. I think the use of the simple present tense here is
correct: it's a fact. Feel free to advice another wording if you
prefer.

 Reflecting commit 'ac1fbb' to support
 this syntax and also validate property values during 'update' command,
 issuing an error if the value found is unknown.

 Sorry, but -ECANNOTPARSE.

Not sure what's wrong here, can you explain why it's failing? I'm
using git-format-patch/git-send-email with default settings. Also, if
you can edit and keep the sign-off (I'm not familiar with the
mailing-list maintainer workflow, sorry), feel free to do it.

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: [msysGit] Fwd: static initializers

2014-01-06 Thread Erik Faye-Lund
On Mon, Jan 6, 2014 at 11:05 PM, Stefan Zager sza...@google.com wrote:
 Forwarding to msysgit for any guidance about win equivalents for
 PTHREAD_MUTEX_INITIALIZER or __attribute__((constructor)).

As you've probably already guessed, PTHREAD_MUTEX_INITIALIZER isn't
supported in our pthreads-emulator. I did send out a patch adding
support for it a while ago, but it hasn't been heavily tested.

__attribute__((constructor)) doesn't work on MSVC, which we also build with.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread John Szakmeister
On Mon, Jan 6, 2014 at 5:54 PM, Ramkumar Ramachandra artag...@gmail.com wrote:
 John Szakmeister wrote:
 Then where does it get pushed?  Do you always specify where to save your 
 work?

 FWIW, I think the idea of treating @{u} as the eventual recipient of
 your changes is good, but then it seems like Git is lacking the
 publish my changes to this other branch concept.

 Am I missing something?  If there is something other than @{u} to
 represent this latter concept, I think `git push` should default to
 that instead.  But, at least with my current knowledge, that doesn't
 exist--without explicitly saying so--or treating @{u} as that branch.
 If there's a better way to do this, I'd love to hear it!

 That's why we invented remote.pushdefault and branch.*.pushremote. When you 
 say

   $ git push

 it automatically goes to the right remote instead of going to the
 place you fetched from. You can read up on how push.default interacts
 with this setting too, although I always recommend push.default =
 current.

This was the piece that I was missing.  I remember when
remote.pushdefault was added, but questioned its usefulness because it
just changes the remote that a branch is pushed to, not necessarily
the name.  I somehow missed the 'current' option for 'push.default'...
which is surprising since I seem to spend an incredible amount of time
looking at the git-config man page.  I guess it's not a good idea to
set 'push.default' to 'upstream' in this triangle workflow then,
otherwise the branch name being pushed to will be 'branch.*.merge'.
Is that correct?  I just want to make sure I understand what's
happening here.

Given this new found knowledge, I'm not sure it makes sense for `git
status` to show me the status against the upstream vs. the publish
location.  The latter makes a little more sense to me, though I see
the usefulness of either one.

Thanks for taking the time to explain this.  I'm going to have to
spend some more time with this configuration and see what the sticking
points are.

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


Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-06 Thread Francesco Pretto
2014/1/6 Junio C Hamano gits...@pobox.com:

 As long as origin/master contains the commit specified by the
 superproject, that would work, but it may be a good thing to use a
 mode of submodule.*.update that does not have to drop the user into
 a detached state in the first place.  I somehow thought that was
 what rebase (or merge) was about, that is, starting from the state
 where a branch is checked out in the submodule working tree, an
 update in the superproject would cause that branch checked out in
 the submodule brought up to date with respect to the commit
 specified in the superproject tree.  If that is not how it is
 supposed to work, please correct me---and we may have to add another
 mode that does so (or even make rebase/merge do so as a bugfix).


I think the mode you are referring to is actually my
submodule.name.attached property. As I said to Heiko, the
submodule.name.attached property says two things:
- at the first clone on git submodule update stay attached to
submodule.name.branch;
- implies --remote, as it's the only thing that makes sense when the
submodules are attached.

 And wouldn't it make it unnecessary to have a new re-attach option
 if such a mode that never have to detach is used?

I think the reattach|detach option are still needed (it is debatable
if we should have something like git submodule head command or we
can keep them in git submodule update) because the user may want to
do so and doing it requires things should be really atomic:
* fix the head state;
* set the local .git/config submodule.name.attached property.

The --attach switch also can add a great bonus: it can reintegrate
orphaned commits when reattaching and having a update mode with
merge or rebase. This is already in my 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] git-submodule.sh: Support 'checkout' as a valid update command

2014-01-06 Thread Francesco Pretto
2014/1/7 Junio C Hamano gits...@pobox.com:
 Sorry, but -ECANNOTPARSE.


A bird told me what -ECANNOTPARSE means. Tell me if this comment sounds better:

According to Documentation/gitmodules.txt, 'checkout' is a valid
'submodule.name.update' mode. Also git-submodule.sh already refers
to it and handles it correctly. Fix cmd_init() to also accept 'checkout' as
valid update mode and add a similar validation in cmd_update(), issuing
an error if the value read is unknown.
--
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] pager: set LV=-c alongside LESS=FRSX

2014-01-06 Thread Jonathan Nieder
On systems with lv configured as the preferred pager (i.e.,
DEFAULT_PAGER=lv at build time, or PAGER=lv exported in the
environment) git commands that use color show control codes instead of
color in the pager:

$ git diff
^[[1mdiff --git a/.mailfilter b/.mailfilter^[[m
^[[1mindex aa4f0b2..17e113e 100644^[[m
^[[1m--- a/.mailfilter^[[m
^[[1m+++ b/.mailfilter^[[m
^[[36m@@ -1,11 +1,58 @@^[[m

less avoids this problem because git uses the LESS environment
variable to pass the -R option ('output ANSI color escapes in raw
form') by default.  Use the LV environment variable to pass 'lv' the
-c option ('allow ANSI escape sequences for text decoration / color')
to fix it for lv, too.

Noticed when the default value for color.ui flipped to 'auto' in
v1.8.4-rc0~36^2~1 (2013-06-10).

Reported-by: Olaf Meeuwissen olaf.meeuwis...@avasys.jp
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Olaf Meeuwissen wrote[1]:

 Yes, it's called LV and documented in the lv(1) manual page.  Simply
 search for 'env' ;-)

Ah, thanks.  How about this patch?

[1] http://bugs.debian.org/730527

 Documentation/config.txt |  4 
 git-sh-setup.sh  |  3 ++-
 pager.c  | 11 +--
 perl/Git/SVN/Log.pm  |  1 +
 t/t7006-pager.sh | 12 
 5 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a405806..ed59853 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -567,6 +567,10 @@ be passed to the shell by Git, which will translate the 
final
 command to `LESS=FRSX less -+S`. The environment tells the command
 to set the `S` option to chop long lines but the command line
 resets it to the default to fold long lines.
++
+Likewise, when the `LV` environment variable is unset, Git sets it
+to `-c`.  You can override this setting by exporting `LV` with
+another value or setting `core.pager` to `lv +c`.
 
 core.whitespace::
A comma separated list of common whitespace problems to
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 190a539..fffa3c7 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -159,7 +159,8 @@ git_pager() {
GIT_PAGER=cat
fi
: ${LESS=-FRSX}
-   export LESS
+   : ${LV=-c}
+   export LESS LV
 
eval $GIT_PAGER '$@'
 }
diff --git a/pager.c b/pager.c
index 345b0bc..0cc75a8 100644
--- a/pager.c
+++ b/pager.c
@@ -80,8 +80,15 @@ void setup_pager(void)
pager_process.use_shell = 1;
pager_process.argv = pager_argv;
pager_process.in = -1;
-   if (!getenv(LESS)) {
-   static const char *env[] = { LESS=FRSX, NULL };
+   if (!getenv(LESS) || !getenv(LV)) {
+   static const char *env[3];
+   int i = 0;
+
+   if (!getenv(LESS))
+   env[i++] = LESS=FRSX;
+   if (!getenv(LV))
+   env[i++] = LV=-c;
+   env[i] = NULL;
pager_process.env = env;
}
if (start_command(pager_process))
diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm
index 3f8350a..34f2869 100644
--- a/perl/Git/SVN/Log.pm
+++ b/perl/Git/SVN/Log.pm
@@ -117,6 +117,7 @@ sub run_pager {
}
open STDIN, '', $rfd or fatal Can't redirect stdin: $!;
$ENV{LESS} ||= 'FRSX';
+   $ENV{LV} ||= '-c';
exec $pager or fatal Can't run pager: $! ($pager);
 }
 
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index ff25908..7fe3367 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -37,6 +37,18 @@ test_expect_failure TTY 'pager runs from subdir' '
test_cmp expected actual
 '
 
+test_expect_success TTY 'LESS and LV envvars are set for pagination' '
+   (
+   sane_unset LESS LV 
+   PAGER=env pager-env.out 
+   export PAGER 
+
+   test_terminal git log
+   ) 
+   grep ^LESS= pager-env.out 
+   grep ^LV= pager-env.out
+'
+
 test_expect_success TTY 'some commands do not use a pager' '
rm -f paginated.out 
test_terminal git rev-list HEAD 
-- 
1.8.5.1

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


Re: Bug report: stash in upstream caused remote fetch to fail

2014-01-06 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 It's really not clear to me what the check in filter_refs was trying to
 do. It dates all the way back to 1baaae5 (Make maximal use of the remote
 refs, 2005-10-28), but there is not much explanation. I haven't dug into
 the list around that time to see if there's any discussion.

 I think the funny refs the log message talks about is about
 filtering we know we can reach these objects via our alternates,
 but these are not refs we actually have.

Actually, I think the above recollection of mine was completely
bogus.  The  is there because we do allow things like HEAD (they
are the funny ones) as well as things inside refs/, and the latter
is the only thing we had a check-ref-format to dictate the format
when the code was written.

I do not mind tightening things a bit (e.g. outside refs/, only
allow HEAD and nothing else).  A good first step might be to enforce
allow-onelevel outside refs/ (so that we can allow HEAD) and for
those inside refs/ check without allow-onelevel but without skipping
the prefix.

It is a separate story if it makes much sense to allow fetching
refs/stash or ignoring when running git clone.  Operationally, I
think it makes more sense to ignore refs/stash, not because it is a
one-level name, but because what a stash 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


[PATCH] sha1_name: don't resolve refs when core.warnambiguousrefs is false

2014-01-06 Thread Brodie Rao
This change ensures get_sha1_basic() doesn't try to resolve full hashes
as refs when ambiguous ref warnings are disabled.

This provides a substantial performance improvement when passing many
hashes to a command (like git rev-list --stdin) when
core.warnambiguousrefs is false. The check incurs 6 stat()s for every
hash supplied, which can be costly over NFS.
---
 sha1_name.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index e9c2999..10bd007 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -451,9 +451,9 @@ static int get_sha1_basic(const char *str, int len, 
unsigned char *sha1)
int at, reflog_len, nth_prior = 0;
 
if (len == 40  !get_sha1_hex(str, sha1)) {
-   if (warn_on_object_refname_ambiguity) {
+   if (warn_ambiguous_refs  warn_on_object_refname_ambiguity) {
refs_found = dwim_ref(str, len, tmp_sha1, real_ref);
-   if (refs_found  0  warn_ambiguous_refs) {
+   if (refs_found  0) {
warning(warn_msg, len, str);
if (advice_object_name_warning)
fprintf(stderr, %s\n, 
_(object_name_msg));
-- 
1.8.3.4 (Apple Git-47)

--
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] sha1_name: don't resolve refs when core.warnambiguousrefs is false

2014-01-06 Thread Brodie Rao
On Mon, Jan 6, 2014 at 7:32 PM, Brodie Rao bro...@sf.io wrote:
 This change ensures get_sha1_basic() doesn't try to resolve full hashes
 as refs when ambiguous ref warnings are disabled.

 This provides a substantial performance improvement when passing many
 hashes to a command (like git rev-list --stdin) when
 core.warnambiguousrefs is false. The check incurs 6 stat()s for every
 hash supplied, which can be costly over NFS.

Forgot to add:

Signed-off-by: Brodie Rao bro...@sf.io

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

 diff --git a/sha1_name.c b/sha1_name.c
 index e9c2999..10bd007 100644
 --- a/sha1_name.c
 +++ b/sha1_name.c
 @@ -451,9 +451,9 @@ static int get_sha1_basic(const char *str, int len, 
 unsigned char *sha1)
 int at, reflog_len, nth_prior = 0;

 if (len == 40  !get_sha1_hex(str, sha1)) {
 -   if (warn_on_object_refname_ambiguity) {
 +   if (warn_ambiguous_refs  warn_on_object_refname_ambiguity) {
 refs_found = dwim_ref(str, len, tmp_sha1, real_ref);
 -   if (refs_found  0  warn_ambiguous_refs) {
 +   if (refs_found  0) {
 warning(warn_msg, len, str);
 if (advice_object_name_warning)
 fprintf(stderr, %s\n, 
 _(object_name_msg));
 --
 1.8.3.4 (Apple Git-47)

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