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

[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 "1">foo && git add foo && git commit -m "1"
cd ..
git clone repo_a repo_b
cd repo_a
echo "2">foo && 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 
An:  Thomas Ackermann 
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 :
> > 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'"

[Bryan Turner]

On 6 January 2014 01:24, Thomas Ackermann  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 "1">foo && git add foo && git commit -m "1"
> cd ..
> git clone repo_a repo_b
> cd repo_a
> echo "2">foo && 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 
> An:  Thomas Ackermann 
> 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 :
> > > 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'"

[Jiang Xin]

2014/1/6 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 "1">foo && git add foo && git commit -m "1"
> cd ..
> git clone repo_a repo_b
> cd repo_a
> echo "2">foo && 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'"

[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

[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

[Michael Haggerty]

Signed-off-by: Michael Haggerty 
---
 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

[Michael Haggerty]

This makes it more obvious that values of "st" don't persist across
loop iterations.

Signed-off-by: Michael Haggerty 
---
 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

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

[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 
---
 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 05/17] safe_create_leading_directories(): split on first of multiple slashes

[Michael Haggerty]

If the input path has multiple slashes between path components (e.g.,
"foo//bar"), then the old code was breaking the path at the last
slash, not the first one.  So in the above example, the second slash
was overwritten with NUL, resulting in the parent directory being
sought as "foo/".

When stat() is called on "foo/", it fails with ENOTDIR if "foo" exists
but is not a directory.  This caused the wrong path to be taken in the
subsequent logic.

So instead, split path components at the first intercomponent slash
rather than the last one.

Signed-off-by: Michael Haggerty 
---
 sha1_file.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 54202e8..f3190c6 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -115,9 +115,10 @@ int safe_create_leading_directories(char *path)
 
if (!slash)
break;
-   while (*(slash + 1) == '/')
-   slash++;
+
next_component = slash + 1;
+   while (*next_component == '/')
+   next_component++;
if (!*next_component)
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 10/17] lock_ref_sha1_basic(): on SCLD_VANISHED, retry

[Michael Haggerty]

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 
---
 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 */
+   default:
last_errno = errno;
error("unable to create directory for %s", ref_file);
goto 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 08/17] cmd_init_db(): when creating directories, handle errors conservatively

[Michael Haggerty]

safe_create_leading_directories_const() returns a non-zero value on
error.  The old code at this calling site recognized a couple of
particular error values, and treated all other return values as
success.  Instead, be more conservative: recognize the errors we are
interested in, but treat any other nonzero values as failures.  This
is more robust in case somebody adds another possible return value
without telling us.

Signed-off-by: Michael Haggerty 
---
 builtin/init-db.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 6f69593..c7c76bb 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -515,13 +515,14 @@ int cmd_init_db(int argc, const char **argv, const char 
*prefix)
saved = shared_repository;
shared_repository = 0;
switch 
(safe_create_leading_directories_const(argv[0])) {
+   case SCLD_OK:
+   case SCLD_PERMS:
+   break;
case SCLD_EXISTS:
errno = EEXIST;
/* fallthru */
-   case SCLD_FAILED:
-   die_errno(_("cannot mkdir %s"), 
argv[0]);
-   break;
default:
+   die_errno(_("cannot mkdir %s"), 
argv[0]);
break;
}
shared_repository = saved;
-- 
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

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

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

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

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

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

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

[PATCH v2 06/17] safe_create_leading_directories(): always restore slash at end of loop

[Michael Haggerty]

Always restore the slash that we scribbled over at the end of the
loop, rather than also fixing it up at each premature exit from the
loop.  This makes it harder to forget to do the cleanup as new paths
are added to the code.

Signed-off-by: Michael Haggerty 
---
 sha1_file.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index f3190c6..4513cbb 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -108,8 +108,9 @@ int mkdir_in_gitdir(const char *path)
 int safe_create_leading_directories(char *path)
 {
char *next_component = path + offset_1st_component(path);
+   int ret = 0;
 
-   while (next_component) {
+   while (!ret && next_component) {
struct stat st;
char *slash = strchr(next_component, '/');
 
@@ -125,25 +126,20 @@ int safe_create_leading_directories(char *path)
*slash = '\0';
if (!stat(path, &st)) {
/* path exists */
-   if (!S_ISDIR(st.st_mode)) {
-   *slash = '/';
-   return -3;
-   }
+   if (!S_ISDIR(st.st_mode))
+   ret = -3;
} else if (mkdir(path, 0777)) {
if (errno == EEXIST &&
-   !stat(path, &st) && S_ISDIR(st.st_mode)) {
+   !stat(path, &st) && S_ISDIR(st.st_mode))
; /* somebody created it since we checked */
-   } else {
-   *slash = '/';
-   return -1;
-   }
+   else
+   ret = -1;
} else if (adjust_shared_perm(path)) {
-   *slash = '/';
-   return -2;
+   ret = -2;
}
*slash = '/';
}
-   return 0;
+   return ret;
 }
 
 int safe_create_leading_directories_const(const char *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 07/17] safe_create_leading_directories(): introduce enum for return values

[Michael Haggerty]

Instead of returning magic integer values (which a couple of callers
go to the trouble of distinguishing), return values from an enum.  Add
a docstring.

Signed-off-by: Michael Haggerty 
---
 builtin/init-db.c |  4 ++--
 cache.h   | 17 +++--
 merge-recursive.c |  2 +-
 sha1_file.c   | 16 
 4 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index b3f03cf..6f69593 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -515,10 +515,10 @@ int cmd_init_db(int argc, const char **argv, const char 
*prefix)
saved = shared_repository;
shared_repository = 0;
switch 
(safe_create_leading_directories_const(argv[0])) {
-   case -3:
+   case SCLD_EXISTS:
errno = EEXIST;
/* fallthru */
-   case -1:
+   case SCLD_FAILED:
die_errno(_("cannot mkdir %s"), 
argv[0]);
break;
default:
diff --git a/cache.h b/cache.h
index ce377e1..c6a4157 100644
--- a/cache.h
+++ b/cache.h
@@ -736,8 +736,21 @@ enum sharedrepo {
 };
 int git_config_perm(const char *var, const char *value);
 int adjust_shared_perm(const char *path);
-int safe_create_leading_directories(char *path);
-int safe_create_leading_directories_const(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.
+ */
+enum scld_error {
+   SCLD_OK = 0,
+   SCLD_FAILED = -1,
+   SCLD_PERMS = -2,
+   SCLD_EXISTS = -3
+};
+enum scld_error safe_create_leading_directories(char *path);
+enum scld_error safe_create_leading_directories_const(const char *path);
+
 int mkdir_in_gitdir(const char *path);
 extern void home_config_paths(char **global, char **xdg, char *file);
 extern char *expand_user_path(const char *path);
diff --git a/merge-recursive.c b/merge-recursive.c
index a18bd15..8400a8e 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -693,7 +693,7 @@ static int make_room_for_path(struct merge_options *o, 
const char *path)
/* Make sure leading directories are created */
status = safe_create_leading_directories_const(path);
if (status) {
-   if (status == -3) {
+   if (status == SCLD_EXISTS) {
/* something else exists */
error(msg, path, _(": perhaps a D/F conflict?"));
return -1;
diff --git a/sha1_file.c b/sha1_file.c
index 4513cbb..5594f11 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -105,12 +105,12 @@ int mkdir_in_gitdir(const char *path)
return adjust_shared_perm(path);
 }
 
-int safe_create_leading_directories(char *path)
+enum scld_error safe_create_leading_directories(char *path)
 {
char *next_component = path + offset_1st_component(path);
-   int ret = 0;
+   enum scld_error ret = SCLD_OK;
 
-   while (!ret && next_component) {
+   while (ret == SCLD_OK && next_component) {
struct stat st;
char *slash = strchr(next_component, '/');
 
@@ -127,26 +127,26 @@ int safe_create_leading_directories(char *path)
if (!stat(path, &st)) {
/* path exists */
if (!S_ISDIR(st.st_mode))
-   ret = -3;
+   ret = SCLD_EXISTS;
} else if (mkdir(path, 0777)) {
if (errno == EEXIST &&
!stat(path, &st) && S_ISDIR(st.st_mode))
; /* somebody created it since we checked */
else
-   ret = -1;
+   ret = SCLD_FAILED;
} else if (adjust_shared_perm(path)) {
-   ret = -2;
+   ret = SCLD_PERMS;
}
*slash = '/';
}
return ret;
 }
 
-int safe_create_leading_directories_const(const char *path)
+enum scld_error safe_create_leading_directories_const(const char *path)
 {
/* path points to cache entries, so xstrdup before messing with it */
char *buf = xstrdup(path);
-   int result = safe_create_leading_directories(buf);
+   enum scld_error result = safe_create_leading_directories(buf);
free(buf);
return result;
 }
-- 
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 03/17] safe_create_leading_directories(): add explicit "slash" pointer

[Michael Haggerty]

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 
---
 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;
 }
-- 
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 14/17] rename_ref(): extract function rename_tmp_log()

[Michael Haggerty]

It's about to become a bit more complex.

Signed-off-by: Michael Haggerty 
---
 refs.c | 52 ++--
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/refs.c b/refs.c
index 8a3d3ea..5bc01a7 100644
--- a/refs.c
+++ b/refs.c
@@ -2528,6 +2528,35 @@ int delete_ref(const char *refname, const unsigned char 
*sha1, int delopt)
  */
 #define TMP_RENAMED_LOG  "logs/refs/.tmp-renamed-log"
 
+static int rename_tmp_log(const char *newrefname)
+{
+   if (safe_create_leading_directories(git_path("logs/%s", newrefname))) {
+   error("unable to create directory for %s", newrefname);
+   return -1;
+   }
+
+ retry:
+   if (rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", newrefname))) 
{
+   if (errno==EISDIR || errno==ENOTDIR) {
+   /*
+* rename(a, b) when b is an existing
+* directory ought to result in ISDIR, but
+* Solaris 5.8 gives ENOTDIR.  Sheesh.
+*/
+   if (remove_empty_directories(git_path("logs/%s", 
newrefname))) {
+   error("Directory not empty: logs/%s", 
newrefname);
+   return -1;
+   }
+   goto retry;
+   } else {
+   error("unable to move logfile "TMP_RENAMED_LOG" to 
logs/%s: %s",
+   newrefname, strerror(errno));
+   return -1;
+   }
+   }
+   return 0;
+}
+
 int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
 {
unsigned char sha1[20], orig_sha1[20];
@@ -2575,30 +2604,9 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
}
}
 
-   if (log && safe_create_leading_directories(git_path("logs/%s", 
newrefname))) {
-   error("unable to create directory for %s", newrefname);
+   if (log && rename_tmp_log(newrefname))
goto rollback;
-   }
 
- retry:
-   if (log && rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", 
newrefname))) {
-   if (errno==EISDIR || errno==ENOTDIR) {
-   /*
-* rename(a, b) when b is an existing
-* directory ought to result in ISDIR, but
-* Solaris 5.8 gives ENOTDIR.  Sheesh.
-*/
-   if (remove_empty_directories(git_path("logs/%s", 
newrefname))) {
-   error("Directory not empty: logs/%s", 
newrefname);
-   goto rollback;
-   }
-   goto retry;
-   } else {
-   error("unable to move logfile "TMP_RENAMED_LOG" to 
logs/%s: %s",
-   newrefname, strerror(errno));
-   goto rollback;
-   }
-   }
logmoved = log;
 
lock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL);
-- 
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 15/17] rename_tmp_log(): handle a possible mkdir/rmdir race

[Michael Haggerty]

If a directory vanishes while renaming the temporary reflog file,
retry (up to 3 times).  This could happen if another process deletes
the directory created by safe_create_leading_directories() just before
we rename the file into the directory.

As far as I can tell, this race could not occur internal to git.  The
only time that a directory under $GIT_DIR/logs is deleted is if room
has to be made for a log file for a reference with the same name;
for example, in the following sequence:

git branch foo/bar# Creates file .git/logs/refs/heads/foo/bar
git branch -d foo/bar # Deletes file but leaves .git/logs/refs/heads/foo/
git branch foo# Deletes .git/logs/refs/heads/foo/

But the only reason the last command deletes the directory is because
it wants to create a file with the same name.  So if another process
(e.g.,

git branch foo/baz

) wants to create that directory, one of the two is doomed to failure
anyway because of a D/F conflict.

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

diff --git a/refs.c b/refs.c
index 5bc01a7..8de636e 100644
--- a/refs.c
+++ b/refs.c
@@ -2530,12 +2530,14 @@ int delete_ref(const char *refname, const unsigned char 
*sha1, int delopt)
 
 static int rename_tmp_log(const char *newrefname)
 {
+   int attempts = 3;
+
+ retry:
if (safe_create_leading_directories(git_path("logs/%s", newrefname))) {
error("unable to create directory for %s", newrefname);
return -1;
}
 
- retry:
if (rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", newrefname))) 
{
if (errno==EISDIR || errno==ENOTDIR) {
/*
@@ -2548,6 +2550,13 @@ static int rename_tmp_log(const char *newrefname)
return -1;
}
goto retry;
+   } else if (errno == ENOENT && --attempts > 0) {
+   /*
+* Maybe another process just deleted one of
+* the directories in the path to newrefname.
+* Try again from the beginning.
+*/
+   goto retry;
} else {
error("unable to move logfile "TMP_RENAMED_LOG" to 
logs/%s: %s",
newrefname, strerror(errno));
-- 
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: Re: [PATCH 2/2] Introduce git submodule attached update

[Heiko Voigt]

On Sun, Jan 05, 2014 at 10:46:11PM +0100, Francesco Pretto wrote:
> 2014/1/5 Heiko Voigt :
> > 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..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..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..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..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

[Heiko Voigt]

On Mon, Jan 06, 2014 at 12:22:23AM +0100, Francesco Pretto wrote:
> 2014/1/5 Heiko Voigt :
> > 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..attached "true"
> $ git submodule update
> 
> # Detach submodule
> $ git config submodule..attached "false"
> $ git submodule update
> 
> # Unset property in both ".gitmodules" and ".git/config" means -> do nothing
> $ git config --unset submodule..attached
> $ git submodule update
> 
> Also my "submodule..attached" property at the moment behaves
> like "submodule..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

[Heiko Voigt]

On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote:
> 2014/1/5 W. Trevor King :
> > 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..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..branch if submodule..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..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..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

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

[Jeff King]

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.

-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

[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.
  Com

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

[Junio C Hamano]

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

[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

[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..branch is set, it *always* creates a new local
> > > branch of that name pointing to the exact sha1.  If
> > > submodule..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..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..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..branch gives us now but possibly enabled by some
option (i.e.: submodule..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..update (defaulting to checkout) is checkout,
> > >   create a detached HEAD.
> > > * Otherwise, create a new branch submodule..branch
> > >   (defaulting to master).
> > 
> > Why not trigger the attached state with the submodule..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/
> > 
> > assuming that submodule update learns some more support for this.
> 
> Isn't that already what 'git update --remote ' already
> does?

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

git checkout origin/

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

git submodule update

will:

1. if no local branch with that name exists:

   checkout the remote/

2. If a local branch with that name exists:

   checkout the local branch and possibly advance it according to its

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

[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

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

[Junio C Hamano]

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

[Junio C Hamano]

Heiko Voigt  writes:

> On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote:
>> 2014/1/5 W. Trevor King :
>> > 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..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: Re: [RFC v2] submodule: Respect requested branch on all clones

[Heiko Voigt]

On Sun, Jan 05, 2014 at 05:12:56PM -0800, W. Trevor King wrote:
> On Sun, Jan 05, 2014 at 04:33:14PM -0800, W. Trevor King wrote:
> > The only people who would need *automatic* rebase recovery would be
> > superproject devs update-cloning the subproject.  That's a small
> > enough cross-section that I don't think it deserves the ambiguity of
> > gitlink-to-reference.  In that case, all you really need is a way to
> > force a recovery gitlink (i.e. add a 'commit' object to the tree by
> > hand).
> 
> Actually, you recovering by hand is a lot easier.  Setup a
> rebased-away gitlink target:
> 
>   mkdir subproject &&
>   (
> cd subproject &&
> git init
> echo 'Subproject' > README &&
> git add README &&
> git commit -m 'Subproject v1' &&
> echo 'Changes' >> README &&
> git commit -am 'Subproject v2'
>   ) &&
>   mkdir superproject &&
>   (
> cd superproject &&
> git init
> git submodule add ../subproject &&
> git commit -m 'Superproject v1'
>   ) &&
>   (
> cd subproject &&
> git reset --hard HEAD^ &&
> git reflog expire --expire=now --all &&
> git gc --aggressive --prune=now
>   )
> 
> Then a recursive clone of the superproject dies:
> 
>   $ git clone --recursive superproject super2
>   Cloning into 'super2'...
>   done.
>   Submodule 'subproject' (/tmp/x/subproject) registered for path 'subproject'
>   Cloning into 'subproject'...
>   done.
>   fatal: reference is not a tree: f589144d16282d1a80d17a9032c6f1d332e38dd0
>   Unable to checkout 'f589144d16282d1a80d17a9032c6f1d332e38dd0' in submodule 
> path 'subproject'
> 
> But you still have the submodule checkout (up until the $sha1 setup):
> 
>   $ cd super2
>   $ git diff
>   diff --git a/subproject b/subproject
>   index f589144..82d4553 16
>   --- a/subproject
>   +++ b/subproject
>   @@ -1 +1 @@
>   -Subproject commit f589144d16282d1a80d17a9032c6f1d332e38dd0
>   +Subproject commit 82d4553fe437ae014f22bbc87a082c6d19e5d9f9-dirty
> 
> And you can automatically update to match the upstream remote:
> 
>   $ git submodule update --remote --force
>   Submodule path 'subproject': checked out 
> '82d4553fe437ae014f22bbc87a082c6d19e5d9f9'
>   $ git diff
>   diff --git a/subproject b/subproject
>   index f589144..82d4553 16
>   --- a/subproject
>   +++ b/subproject
>   @@ -1 +1 @@
>   -Subproject commit f589144d16282d1a80d17a9032c6f1d332e38dd0
>   +Subproject commit 82d4553fe437ae014f22bbc87a082c6d19e5d9f9
> 
> When explicitly updating to the superproject or subproject's
> (--remote) new tip is so easy, I don't see a need for floating the
> gitlinks themselves.

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

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

git submodule attach [--pull]  

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.

workflow. What do you think?

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

[Junio C Hamano]

"W. Trevor King"  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'"

[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

[Junio C Hamano]

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

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

[Junio C Hamano]

Thomas Ackermann  writes:

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

By the way, this is nothing new in 1.8.5; we didn't bother saying
up-to-date before, so you may not have noticed, but its silence was
already telling you that your branch was up-to-date with respect to
what you are building on top of.

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

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

[Ramkumar Ramachandra]

Hi,

This is inspired by merge.defaultToUpstream. Disclaimer: I have an
"fpm" alias for doing this (separate from "fp" for when I need to
generate patches against some other branch), which I'd like to get rid
of.

Thanks.

Ramkumar Ramachandra (2):
  completion: complete format.coverLetter
  format-patch: introduce format.defaultTo

 Documentation/config.txt   |  4 
 builtin/log.c  |  7 +++
 contrib/completion/git-completion.bash |  2 ++
 t/t4014-format-patch.sh| 10 ++
 4 files changed, 23 insertions(+)

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

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

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

[Junio C Hamano]

Thomas Ackermann  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 
> ---
>  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
> -___
> +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

[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..update (defaulting to checkout) is checkout,
> > > >   create a detached HEAD.
> > > > * Otherwise, create a new branch submodule..branch
> > > >   (defaulting to master).
> > > 
> > > Why not trigger the attached state with the
> > > submodule..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/
> > > 
> > > assuming that submodule update learns some more support for this.
> > 
> > Isn't that already what 'git update --remote ' already
> > does?
> 
> Does it? As far as I understood (not using the branch option yet) it
> only does
> 
>   git checkout origin/
> 
> 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..update.

> What I was thinking is that when submodule..branch is set a
> 
>   git submodule update
> 
> will:
> 
> 1. if no local branch with that name exists:
> 
>checkout the remote/
> 
> 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..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/

With my patch this happens with the initial clone-update (as of v2,
only when submodule..branch is set.  In a hypothetical v3, only
when submodule..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..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/ 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 

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

[Ramkumar Ramachandra]

Signed-off-by: Ramkumar Ramachandra 
---
 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

[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

[W. Trevor King]

On Mon, Jan 06, 2014 at 08:56:22AM -0800, Junio C Hamano wrote:
> Heiko Voigt  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

[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

[Junio C Hamano]

Junio C Hamano  writes:

> "W. Trevor King"  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: Re: [PATCH 2/2] Introduce git submodule attached update

[Francesco Pretto]

Dear Heiko, my replies below. I also take a couple excerpts from other
emails, as I prefer to not flame on different threads :) .

2014/1/6 Heiko Voigt :
> On Sun, Jan 05, 2014 at 10:46:11PM +0100, Francesco Pretto wrote:
>> It means he doesn't need to control other developers commit to be
>> checked out so he sets "submodule..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..ignore is set to "all" in the master branch of the
> superproject? From your other email referring to svn:externals I figure
> that.
>

Correct, but this works also if the branch of the superproject is a
different branch than "master". I think you are right in a point, see
the next reply.

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

Ok, you are right: setting "submodule..ignore" to "all" by
default with a switch "--attached" in "git submodule add" is too much.
My point is just make sure users will checkout an attached HEAD.

>> "submodule..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..branch?
>

"submodule..attacched" set to true implies "--remote". sha1 of
the latest commit is taken from "origin/$branch". In this way you get
the latest commit of that branch, and you do a 'merge', 'rebase',
'checkout' or '!command' according to the configured ''. This
mechanism is already in the patch at the current state.

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

The developer does it voluntarily, at his responsibility, because he
may decide to partecipate more actively to the development of the
submodule and still want to use a simple "git submodule update" to
updates his submodules, overriding its configuration as it can be done
for other properties like, for example, "branch". This ability is of
course already possible reattaching the HEAD manually but you loose
the convenient ability to use "git submodule update".

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

I'm really sorry, I thought this was already clear from the first
patch iteration. I will go more in depth:

Say we have our actual projects "project1" and "project2". Say we have
a project "common". This "common" project is *not* independent: it
exists only to serve "project1" and "project2" and it's tested only in
the scope of "project1" and "project2". Also "common" is very actively
developed and follows the same lifecyle of "project1" and "project2"
on separate branches. I think that it's important that you get this
point: developers of "common" don't cl

Re: [PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command

[Francesco Pretto]

Ok, applying the suggested modifications and resending shortly.

Thank you,
Francesco

2014/1/6 Junio C Hamano :
> Junio C Hamano  writes:
>
>> "W. Trevor King"  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

[Junio C Hamano]

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

[Junio C Hamano]

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

[Junio C Hamano]

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

[Junio C Hamano]

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

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

[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 ""] 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

[Junio C Hamano]

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

[Junio C Hamano]

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

[Ramkumar Ramachandra]

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

It does accept a , 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

[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

[Francesco Pretto]

According to "Documentation/gitmodules.txt", 'checkout' is a valid
'submodule..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 
---
 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

[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 ""] 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
 as a configuration variable value, but I think it's useful.
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

[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 
Signed-off-by: Jens Lehmann 
---

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..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^ 2>actual &&
+   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

[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

[Jeff King]

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

> Jeff King  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] Introduce git submodule attached update

[David Engster]

Francesco Pretto writes:
> 2014/1/6 Heiko Voigt :
>> 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?
>>
>
> I'm really sorry, I thought this was already clear from the first
> patch iteration. I will go more in depth:

While I have some trouble understanding all the details of Francesco's
description, I find the idea of "attaching submodules to branches" very
useful.  I think I could well use that to simplify my merging grunt work
for GNU Emacs (which, in case you're wondering, is probably switching to
git as its VCS). I don't mean to hijack this thread, and I guess my use
case is a bit different than what Francesco has in mind; still, I think
it is similar enough that my use case could help in talking about the
details of his patch; if not, please feel free to ignore it.

GNU Emacs ships with some pretty large packages, namely Gnus, Org and
CEDET, which are also available as "stand-alone" versions for manual
installation, and their development happens in separate upstream
repositories. Since I'm a CEDET developer, I'll use it as an example in
the following.

First off, it is important to note that merges are always
bi-directional: not only is new CEDET code pulled into Emacs, but Emacs
developers also change things which have to be merged back upstream. So
far, merging between the two repositories was done manually by me, which
is error-prone (and boring). I think that by pulling in CEDET directly
as a submodule, this merging could be made easier. Most importantly, my
hope is that more people than me could do it. :-)

Here's how I would like this to work; first the CEDET -> Emacs part,
which is rather straight-forward:

- The CEDET repository has two branches: 'master' and 'stable'.

- The Emacs repository imports CEDET's 'stable' branch as a submodule.

- CEDET's main development happens in 'master', and the CEDET developers
  are responsible for merging stable code to 'stable'. They will then
  make a new commit for the submodule in Emacs accordingly.

The Emacs -> CEDET part is more hairy. Most of the time, the fixes
happening in the Emacs repository for CEDET are very small and/or
trivial and can usually be considered "always stable": fixes for
spelling, compiler warnings, or small refactorings like renames,
etc. This kind of "merging back to CEDET upstream" should hence be as
easy as possible for Emacs developers:

- When an Emacs developer changes something in the CEDET submodule, the
  changes they commit should by default automatically land in CEDET's
  'stable' branch. That means that when they enter the submodule, they
  should be in the branch 'stable' instead of being detached, and a push
  should update the 'stable' branch in CEDET accordingly. The submodule
  must then be committed as well.

- It is then up to the CEDET developers to merge these changes into the
  'master' branch of the CEDET repo.

I know that the "correct" workflow would be to always use feature
branches, but it'd be nice if that could be avoided if one so chooses.

A little picture in the hope that it makes things clearer:

 +---+
 |  master   | <--
+---++---+| Merges to/from master
| CEDET | | done only by CEDET developers
+---+ | 
 +---+|
 |  stable   | <--  <
 +---+   |
 |
 |
 | Any Emacs developer
 | can push and commit
 | submodule
+++--+   |
| Emacs  | -- | lisp/cedet submodule | <-
+++--+

AFAICS the main problem with this approach is that one always has to
think of committing the new SHA1 of the submodule. If I understand
Francesco correctly, he wants to eliminate the need for that by simply
always taking the head of the attached branch. I also think that would
be a nice feature, since in the above drawing, the lisp/cedet submodule
should always follow the 'stable' branch in CEDET upstream. However, as
Heiko notes, the history must be preserved to be able to go back to
earlier revisions, so there must be some kind of commit for the
submodule when 'stable' changes; maybe that could be automated somehow?

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

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

[Junio C Hamano]

Ramkumar Ramachandra  writes:

> Junio C Hamano wrote:
>>  - why is a single branch name sufficient?
>
> It does accept a , 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

[Junio C Hamano]

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

[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

[John Szakmeister]

On Mon, Jan 6, 2014 at 3:18 PM, Jeff King  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

[Junio C Hamano]

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

[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

[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

[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

[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

[John Szakmeister]

On Mon, Jan 6, 2014 at 3:42 PM, Jonathan Nieder  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:
   

> 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

[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

[Junio C Hamano]

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

[Junio C Hamano]

"W. Trevor King"  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

[Junio C Hamano]

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

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

[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

[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

[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 

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

[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

[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

[Junio C Hamano]

Ramkumar Ramachandra  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)

[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  -- " 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_

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

[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.pushdefault"set 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-forkedfrom<>5*:~/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

[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

[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

[Francesco Pretto]

2014/1/6 Heiko Voigt :
>
> 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..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..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..attached" property that says two things:
- at the first clone on "git submodule update" stay attached to
"submodule..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]  
>
> 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..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)

[Francesco Pretto]

2014/1/6 Junio C Hamano :
>
>  - 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

[Junio C Hamano]

Jeff King  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)

[Junio C Hamano]

Francesco Pretto  writes:

> 2014/1/6 Junio C Hamano :
>>
>>  - 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

[Francesco Pretto]

2014/1/7 Francesco Pretto :
> To not break the existing behavior what it's really needed here, IMO,
> is a "submodule..attached" property that says two things:
> - at the first clone on "git submodule update" stay attached to
> "submodule..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..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

[Junio C Hamano]

Jens Lehmann  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 
> Signed-off-by: Jens Lehmann 
> ---
>
> 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..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^ 2>actual &&
> + 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)

[Francesco Pretto]

2014/1/7 Junio C Hamano :
> 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

[Junio C Hamano]

Francesco Pretto  writes:

> According to "Documentation/gitmodules.txt", 'checkout' is a valid
> 'submodule..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