Aw: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'
Hi Jiang, this happens with all of my repo clones (I am using V1.8.5.2 on Windows and on Linux). Steps to reproduce: mkdir repo_a cd repo_a git init . echo 1foo git add foo git commit -m 1 cd .. git clone repo_a repo_b cd repo_a echo 2foo git add foo git commit -m 2 cd ../repo_b git status git checkout -b branch git checkout master 'git status' and 'git checkout master' in repo_b are now reporting Your branch is up-to-date with 'origin/master' which is obviously wrong. --- Thomas - Original Nachricht Von: Jiang Xin worldhello@gmail.com An: Thomas Ackermann th.ac...@arcor.de Datum: 06.01.2014 06:31 Betreff: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master' 2014/1/5 Thomas Ackermann th.ac...@arcor.de: Since f223459 status: always show tracking branch even no change 'git status' (and 'git checkout master' always says Your branch is up-to-date with 'origin/master' even if 'origin/master' is way ahead from local 'master'. Hi, Thomas Can you provide your operations so that I can reproduce this issue? In the commit you mentioned above, there was a new test case named 'checkout (up-to-date with upstream)' added in 't6040'. I also add two test-cases locally in order to reproduce the issue you report, and run them in arbitrary orders, but they all look fine: ok 4 - checkout (behind upstream) ok 5 - checkout (ahead upstream) ok 6 - checkout (diverged from upstream) ok 7 - checkout with local tracked branch ok 8 - checkout (upstream is gone) ok 9 - checkout (up-to-date with upstream) ok 10 - checkout (upstream is gone) ok 11 - checkout with local tracked branch ok 12 - checkout (diverged from upstream) ok 13 - checkout (ahead upstream) ok 14 - checkout (behind upstream) ok 15 - checkout (diverged from upstream) ok 16 - checkout (upstream is gone) ok 17 - checkout (ahead upstream) ok 18 - checkout with local tracked branch ok 19 - checkout (behind upstream) The two additional test cases I used locally are: checkout_test1() { test_expect_success 'checkout (behind upstream)' ' ( cd test git checkout b3 ) actual test_i18ngrep is behind .* by 1 commit, and can be fast-forwarded actual ' } checkout_test_2() { test_expect_success 'checkout (ahead upstream)' ' ( cd test git checkout b4 ) actual test_i18ngrep is ahead of .* by 2 commits actual ' } -- Jiang Xin --- Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'
On 6 January 2014 01:24, Thomas Ackermann th.ac...@arcor.de wrote: Hi Jiang, this happens with all of my repo clones (I am using V1.8.5.2 on Windows and on Linux). Steps to reproduce: mkdir repo_a cd repo_a git init . echo 1foo git add foo git commit -m 1 cd .. git clone repo_a repo_b cd repo_a echo 2foo git add foo git commit -m 2 cd ../repo_b git status git checkout -b branch git checkout master 'git status' and 'git checkout master' in repo_b are now reporting Your branch is up-to-date with 'origin/master' which is obviously wrong. Unfortunately that's not true. In repo_b your ref for origin/master has not moved. It has remotely (meaning refs/heads/master in repo_a has moved), but git status is not hitting the remote to find out; it only looks at the local state. To see what I mean, run git fetch in repo_b. Once you do that, you'll see that git status correctly reports you're behind. --- Thomas - Original Nachricht Von: Jiang Xin worldhello@gmail.com An: Thomas Ackermann th.ac...@arcor.de Datum: 06.01.2014 06:31 Betreff: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master' 2014/1/5 Thomas Ackermann th.ac...@arcor.de: Since f223459 status: always show tracking branch even no change 'git status' (and 'git checkout master' always says Your branch is up-to-date with 'origin/master' even if 'origin/master' is way ahead from local 'master'. Hi, Thomas Can you provide your operations so that I can reproduce this issue? In the commit you mentioned above, there was a new test case named 'checkout (up-to-date with upstream)' added in 't6040'. I also add two test-cases locally in order to reproduce the issue you report, and run them in arbitrary orders, but they all look fine: ok 4 - checkout (behind upstream) ok 5 - checkout (ahead upstream) ok 6 - checkout (diverged from upstream) ok 7 - checkout with local tracked branch ok 8 - checkout (upstream is gone) ok 9 - checkout (up-to-date with upstream) ok 10 - checkout (upstream is gone) ok 11 - checkout with local tracked branch ok 12 - checkout (diverged from upstream) ok 13 - checkout (ahead upstream) ok 14 - checkout (behind upstream) ok 15 - checkout (diverged from upstream) ok 16 - checkout (upstream is gone) ok 17 - checkout (ahead upstream) ok 18 - checkout with local tracked branch ok 19 - checkout (behind upstream) The two additional test cases I used locally are: checkout_test1() { test_expect_success 'checkout (behind upstream)' ' ( cd test git checkout b3 ) actual test_i18ngrep is behind .* by 1 commit, and can be fast-forwarded actual ' } checkout_test_2() { test_expect_success 'checkout (ahead upstream)' ' ( cd test git checkout b4 ) actual test_i18ngrep is ahead of .* by 2 commits actual ' } -- Jiang Xin --- Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'
2014/1/6 Thomas Ackermann th.ac...@arcor.de: Hi Jiang, this happens with all of my repo clones (I am using V1.8.5.2 on Windows and on Linux). Steps to reproduce: mkdir repo_a cd repo_a git init . echo 1foo git add foo git commit -m 1 cd .. git clone repo_a repo_b cd repo_a echo 2foo git add foo git commit -m 2 cd ../repo_b git status git checkout -b branch Oops. Do git fetch then git checkout master You will get what you want. git checkout master 'git status' and 'git checkout master' in repo_b are now reporting Your branch is up-to-date with 'origin/master' which is obviously wrong. -- Jiang Xin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Aw: Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'
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
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
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- sha1_file.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index daacc0c..c9245a6 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -125,8 +125,7 @@ int safe_create_leading_directories(char *path) *pos = '/'; return -3; } - } - else if (mkdir(path, 0777)) { + } else if (mkdir(path, 0777)) { if (errno == EEXIST !stat(path, st) S_ISDIR(st.st_mode)) { ; /* somebody created it since we checked */ @@ -134,8 +133,7 @@ int safe_create_leading_directories(char *path) *pos = '/'; return -1; } - } - else if (adjust_shared_perm(path)) { + } else if (adjust_shared_perm(path)) { *pos = '/'; return -2; } -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/17] safe_create_leading_directories(): reduce scope of local variable
This makes it more obvious that values of st don't persist across loop iterations. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- sha1_file.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index c9245a6..cc9957e 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -108,9 +108,10 @@ int mkdir_in_gitdir(const char *path) int safe_create_leading_directories(char *path) { char *pos = path + offset_1st_component(path); - struct stat st; while (pos) { + struct stat st; + pos = strchr(pos, '/'); if (!pos) break; -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/17] safe_create_leading_directories(): add new error value SCLD_VANISHED
Add a new possible error result that can be returned by safe_create_leading_directories() and safe_create_leading_directories_const(): SCLD_VANISHED. This value indicates that a file or directory on the path existed at one point (either it already existed or the function created it), but then it disappeared. This probably indicates that another process deleted the directory while we were working. If SCLD_VANISHED is returned, the caller might want to retry the function call, as there is a chance that a new attempt will succeed. Why doesn't safe_create_leading_directories() do the retrying internally? Because an empty directory isn't really ever safe until it holds a file. So even if safe_create_leading_directories() were absolutely sure that the directory existed before it returned, there would be no guarantee that the directory still existed when the caller tried to write something in it. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- cache.h | 10 +- sha1_file.c | 11 +++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/cache.h b/cache.h index c6a4157..f34c0a7 100644 --- a/cache.h +++ b/cache.h @@ -741,12 +741,20 @@ int adjust_shared_perm(const char *path); * Create the directory containing the named path, using care to be * somewhat safe against races. Return one of the scld_error values * to indicate success/failure. + * + * SCLD_VANISHED indicates that one of the ancestor directories of the + * path existed at one point during the function call and then + * suddenly vanished, probably because another process pruned the + * directory while we were working. To be robust against this kind of + * race, callers might want to try invoking the function again when it + * returns SCLD_VANISHED. */ enum scld_error { SCLD_OK = 0, SCLD_FAILED = -1, SCLD_PERMS = -2, - SCLD_EXISTS = -3 + SCLD_EXISTS = -3, + SCLD_VANISHED = -4 }; enum scld_error safe_create_leading_directories(char *path); enum scld_error safe_create_leading_directories_const(const char *path); diff --git a/sha1_file.c b/sha1_file.c index 5594f11..69a4e95 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -132,6 +132,17 @@ enum scld_error safe_create_leading_directories(char *path) if (errno == EEXIST !stat(path, st) S_ISDIR(st.st_mode)) ; /* somebody created it since we checked */ + else if (errno == ENOENT) + /* +* Either mkdir() failed because +* somebody just pruned the containing +* directory, or stat() failed because +* the file that was in our way was +* just removed. Either way, inform +* the caller that it might be worth +* trying again: +*/ + ret = SCLD_VANISHED; else ret = SCLD_FAILED; } else if (adjust_shared_perm(path)) { -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/17] rename_tmp_log(): limit the number of remote_empty_directories() attempts
This doesn't seem to be a likely error, but we've got the counter anyway, so we might as well use it for an added bit of safety. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 8de636e..490525a 100644 --- a/refs.c +++ b/refs.c @@ -2530,7 +2530,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) static int rename_tmp_log(const char *newrefname) { - int attempts = 3; + int attempts = 4; retry: if (safe_create_leading_directories(git_path(logs/%s, newrefname))) { @@ -2539,7 +2539,7 @@ static int rename_tmp_log(const char *newrefname) } if (rename(git_path(TMP_RENAMED_LOG), git_path(logs/%s, newrefname))) { - if (errno==EISDIR || errno==ENOTDIR) { + if ((errno==EISDIR || errno==ENOTDIR) --attempts 0) { /* * rename(a, b) when b is an existing * directory ought to result in ISDIR, but -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/17] remove_dir_recurse(): handle disappearing files and directories
If a file or directory that we are trying to remove disappears (e.g., because another process has pruned it), do not consider it an error. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- dir.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/dir.c b/dir.c index 11e1520..716b613 100644 --- a/dir.c +++ b/dir.c @@ -1476,7 +1476,9 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) flag = ~REMOVE_DIR_KEEP_TOPLEVEL; dir = opendir(path-buf); if (!dir) { - if (errno == EACCES !keep_toplevel) + if (errno == ENOENT) + return keep_toplevel ? -1 : 0; + else if (errno == EACCES !keep_toplevel) /* * An empty dir could be removable even if it * is unreadable: @@ -1496,13 +1498,21 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) strbuf_setlen(path, len); strbuf_addstr(path, e-d_name); - if (lstat(path-buf, st)) - ; /* fall thru */ - else if (S_ISDIR(st.st_mode)) { + if (lstat(path-buf, st)) { + if (errno == ENOENT) + /* +* file disappeared, which is what we +* wanted anyway +*/ + continue; + /* fall thru */ + } else if (S_ISDIR(st.st_mode)) { if (!remove_dir_recurse(path, flag, kept_down)) continue; /* happy */ - } else if (!only_empty !unlink(path-buf)) + } else if (!only_empty + (!unlink(path-buf) || errno == ENOENT)) { continue; /* happy, too */ + } /* path too long, stat fails, or non-directory still exists */ ret = -1; @@ -1512,7 +1522,7 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) strbuf_setlen(path, original_len); if (!ret !keep_toplevel !kept_down) - ret = rmdir(path-buf); + ret = (!rmdir(path-buf) || errno == ENOENT) ? 0 : -1; else if (kept_up) /* * report the uplevel that it is not an error that we -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/17] remove_dir_recurse(): tighten condition for removing unreadable dir
If opendir() fails on the top-level directory, it makes sense to try to delete it anyway--but only if the failure was due to EACCES. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- dir.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dir.c b/dir.c index 23b6de4..11e1520 100644 --- a/dir.c +++ b/dir.c @@ -1476,8 +1476,11 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) flag = ~REMOVE_DIR_KEEP_TOPLEVEL; dir = opendir(path-buf); if (!dir) { - /* an empty dir could be removed even if it is unreadble */ - if (!keep_toplevel) + if (errno == EACCES !keep_toplevel) + /* +* An empty dir could be removable even if it +* is unreadable: +*/ return rmdir(path-buf); else return -1; -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/17] lock_ref_sha1_basic(): if locking fails with ENOENT, retry
If hold_lock_file_for_update() fails with errno==ENOENT, it might be because somebody else (for example, a pack-refs process) has just deleted one of the lockfile's ancestor directories. So if this condition is detected, try again (up to 3 times). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 6eb8a02..8a3d3ea 100644 --- a/refs.c +++ b/refs.c @@ -2081,7 +2081,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, lock-lk = xcalloc(1, sizeof(struct lock_file)); - lflags = LOCK_DIE_ON_ERROR; + lflags = 0; if (flags REF_NODEREF) { refname = orig_refname; lflags |= LOCK_NODEREF; @@ -2109,6 +2109,17 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, } lock-lock_fd = hold_lock_file_for_update(lock-lk, ref_file, lflags); + if (lock-lock_fd 0) { + if (errno == ENOENT --attempts 0) + /* +* Maybe somebody just deleted one of the +* directories leading to ref_file. Try +* again: +*/ + goto retry; + else + unable_to_lock_index_die(ref_file, errno); + } return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock; error_return: -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/17] rename_tmp_log(): on SCLD_VANISHED, retry
If safe_create_leading_directories() fails because a file along the path unexpectedly vanished, try again from the beginning. Try at most 3 times. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 490525a..810f802 100644 --- a/refs.c +++ b/refs.c @@ -2533,7 +2533,14 @@ static int rename_tmp_log(const char *newrefname) int attempts = 4; retry: - if (safe_create_leading_directories(git_path(logs/%s, newrefname))) { + switch (safe_create_leading_directories(git_path(logs/%s, newrefname))) { + case SCLD_OK: + break; /* success */ + case SCLD_VANISHED: + if (--attempts 0) + goto retry; + /* fall through */ + default: error(unable to create directory for %s, newrefname); return -1; } -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/17] safe_create_leading_directories(): rename local variable
Rename pos to next_component, because now it always points at the next component of the path name that has to be processed. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- sha1_file.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 197766d..54202e8 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -107,18 +107,18 @@ int mkdir_in_gitdir(const char *path) int safe_create_leading_directories(char *path) { - char *pos = path + offset_1st_component(path); + char *next_component = path + offset_1st_component(path); - while (pos) { + while (next_component) { struct stat st; - char *slash = strchr(pos, '/'); + char *slash = strchr(next_component, '/'); if (!slash) break; while (*(slash + 1) == '/') slash++; - pos = slash + 1; - if (!*pos) + next_component = slash + 1; + if (!*next_component) break; *slash = '\0'; -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: do not fail on invalid bookmarks
On 2013-12-29 12.30, Antoine Pelisse wrote: Mercurial can have bookmarks pointing to nullid (the empty root revision), while Git can not have references to it. When cloning or fetching from a Mercurial repository that has such a bookmark, the import will fail because git-remote-hg will not be able to create the corresponding reference. Warn the user about the invalid reference, and continue the import, instead of stopping right away. Signed-off-by: Antoine Pelisse apeli...@gmail.com --- contrib/remote-helpers/git-remote-hg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index eb89ef6..12d850e 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -625,6 +625,9 @@ def list_head(repo, cur): def do_list(parser): repo = parser.repo for bmark, node in bookmarks.listbookmarks(repo).iteritems(): +if node == '': +warn(Ignoring invalid bookmark '%s', bmark) +continue bmarks[bmark] = repo[node] cur = repo.dirstate.branch() (Side note: ap/remote-hg-skip-null-bookmarks) When I run the test-suite like this: ~/projects/git/git.pu/contrib/remote-helpers$ debug=t verbose=t make test-hg-hg-git.sh All 11 test cases fail on my systems (Debian Wheezy and Mac OS X): [snip] WARNING: Ignoring invalid bookmark 'master' To hg::../hgrepo-git ! [remote rejected] master - master error: failed to push some refs to 'hg::../hgrepo-git' not ok 1 - executable bit # [snip] -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH 2/2] Introduce git submodule attached update
On Sun, Jan 05, 2014 at 10:46:11PM +0100, Francesco Pretto wrote: 2014/1/5 Heiko Voigt hvo...@hvoigt.net: The following questions directly pop into my mind: - What means the maintainer does not track the submodules sha1? Does that mean the superproject always refers to submodule commits using branches? It means he doesn't need to control other developers commit to be checked out so he sets submodule.name.ignore to all. In this way he and the developers can work actively in their submodule copy. So practically speaking: You mean that the value of submodule.name.ignore is set to all in the master branch of the superproject? From your other email referring to svn:externals I figure that. - What happens if you want to go back to an earlier revision? Lets say a tagged release? How is ensured that you get the correct revision in the submodules? submodule.name.branch is one setting that is not copied in .git/config by git submodule init. git submodule update will use the setting in .gitmodules if not overridden voluntarily by the developer in .git/config. The maintainer can change that setting in .gitmodules and commit the change. Modifies will be propagated by the next git pull git submodule update of the developer in the superproject. I do not understand how does that ensure you get the correct submodule revision when checking out a tagged release? To get a precise revision the superproject needs to track a sha1 of a submodule commit. I do not see how that has anything to do with submodule.name.branch? - In which situations does the developer or maintainer switch between your attached/detached mode? The developer/maintainer does so optionally and voluntarily and it effects only its private working tree. This does not answer my question. I would like to find out the reason why one would do the switch. - What is the repository branch which is given to the developer by the maintainer used for? Who creates this branch and who merges into it? The branch of course must exist prior submodule adding. In this use-case it does not really matter who creates it and who merges into it. Everyone with the right to merge into it has to work in the submodule seamlessly, as it was working on separate clone of the same repository used as the submodule. o Here is the same. I am searching for a description like: If the developer works on a feature that needs a submodule change he: - creates a submodule branch - configures that submodule branch in the superproject: git config -f .gitmodules submodule.common.branch dev/some-feature git commit -am TEMP: track submodule common on branch - and pushes out his superproject branch The submodule branch is then posted for review and continued to work on. Once everyone involved is happy with the submodule change the branch in there gets merged to master. Now the branch in the superproject is modified to drop the change in .gitmodules and the sha1 reference in the superproject is updated to the current master of the superproject. The superproject branch is posted for review. ... Could you describe something like this for your workflow? A complete change lifecycle when a developer works, as you call it, actively in a submodule? - What are these subsequent merge or rebase update operations? Do you mean everyone has submodule.name.update configured to merge or rebase? subsequent merge or rebase update operations are just the ones after the initial clone/checkout, nothing particular. To clarify you are talking about issuing git merge or git rebase commands in the superproject? Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH 2/2] Introduce git submodule attached update
On Mon, Jan 06, 2014 at 12:22:23AM +0100, Francesco Pretto wrote: 2014/1/5 Heiko Voigt hvo...@hvoigt.net: Could you please extend the description of your use-case so we can understand your goal better? Maybe I found better words to explain you my goal: the current git submodule use-case threats the submodule as a project independent dependency. My use case threats the submodule as part of the superproject repository. It could be easier to say that in this way submodules would behave very similarly to svn:externals, something that is actually missing in git. My goal is obtain this without altering git behavior for the existing use case. I am not so sure. svn:externals was IMO a hack in SVN to bind projects together. It does not record the revision and so has nothing to do with version control. If you simply want to always checkout the development tip of some project you could do something like this: git submodule foreach 'git fetch git checkout origin/master' The demand for this 'missing feature' which we call the 'floating submodules' model has been around for some time but until now we could convince people that its not a feature but you are actually loosing history information. The workflow could always be changed to allow recording revisions. Which is why you use git in the first place right? If you discard revisions for submodules tracking down regression bugs can become a big problem or completely impossible. Try using git bisect on such a history. - In which situations does the developer or maintainer switch between your attached/detached mode? As I told you in the other answer this is voluntary done by the developer, as he prefers. Could you tell me a typical reason? I came to the conclusion that the --attach|--detach switches for the update command are not that useful and can be removed. It's still possible to obtain the switch between detached/attached very easily in this way: # Attach submodule $ git config submodule.name.attached true $ git submodule update # Detach submodule $ git config submodule.name.attached false $ git submodule update # Unset property in both .gitmodules and .git/config means - do nothing $ git config --unset submodule.name.attached $ git submodule update Also my submodule.name.attached property at the moment behaves like submodule.name.update: it is copied in .git/config by git submodule init. This is probably a mistake: the overridden value should be stored in .git/config only at the developer will, so the maintainer has still a chance to modify it in .gitmodules and propagate the behavior. I would send an updated patch but at this point I prefer to wait for a full review. Lets first discuss and figure out what is the real missing feature here and what should be implemented before working further on the code. Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote: 2014/1/5 W. Trevor King wk...@tremily.us: On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote: Also it could break some users that rely on the current behavior. The current code always has a detached HEAD after an initial-clone update, regardless of submodule.name.update, which doesn't match those docs either. I perfectly agree with you that the documentation is a bit contradictory with regard to update command and detached HEAD. That's why it's so hard to add a feature and keep the same spirit of those that coded submodules at first. Also, I think that submodules didn't get much feedback with regards to these pitfalls because many people try to setup them, they see that update detaches the HEAD and they think hmmm, maybe submodules are not what I was looking for. I am not so sure about that. Why should detached HEAD make you think like that? For us at $dayjob we have a pre-commit hook that denies you to commit on a detached HEAD and asks you to create a branch first. You then work on that branch and send it out for review. If the reviewer is happy he merges it into a stable branch (master most times) of the submodule. Only revisions that are on a stable branch in a submodule are allowed to be linked in a superprojects branch that should be merged. Before the submodule's branch gets merged we usually track the development branches sha1 of the submodule in the superproject. For cleanup in the submodule I currently use fixup! commits most times so the referenced sha1 is not lost. In the very end when everyone is happy with the submodule change I rebase, change the referenced sha1 in the superproject and send the final branch out for review another time. Adding a check to only checkout submodule.name.branch if submodule.name.update was 'rebase', 'merge', or 'none' would be easy, but I don't think that makes much sense. I can't see any reason for folks who specify submodule.name.branch to prefer a detached HEAD over a local branch matching the remote branch's name. I think the reason is that it still matches the original use case of submodules devs: - the maintainer decides the specific commit developers should have; Nope. We usually do not have a maintainer. We use a review based workflow. Everyone is allowed to review. If you develop you need to send you changes to a reviewer first who then merges when he is ok with it. - developers checkout that commit and don't pull (you can't do git pull in a detached HEAD); Exactly. We consider pull evil ;-) Seriously: To update we only do fast forward merges of local stable branches. Only reviewers or maintainers are allowed to merge and push into stable branches. Direct commits to stable branches are forbidden. To review we have a shortcut to update the stable branch in git gui for which the code can be found on my github[1]. - they optionally get the upstream commit from the specified submodule.name.branch with --remote. They are still in a detached HEAD and can't do git pull. Yes, why would you do a git pull in a submodule? Don't you want to do something like git checkout -t -b dev/my-topic origin/master to start your development? Maybe who coded submodules at first was thinking that the best way to contribute to a project is to checkout that repository, and not work in the submodule. As said, this works well when the submodule repository is a full project, and not a bunch of shared code. Why not work in the submodule? See explanation above. Cheers Heiko [1] https://github.com/hvoigt/git/commits/hv/gui-improvements -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] Making use of bitmaps for thin objects
On Sun, Dec 22, 2013 at 09:55:23PM +, Ben Maurer wrote: One issue with this approach is that it seems git-pack-index doesn't perform as well with thin packs. git-index-pack uses a multi-threaded approach to resolving the deltas. However, the multithreading only works on deltas that are exclusively in the pack. After the multi-threaded phase, it incrementally brings in objects from external packs, but in single threaded manner. Many objects in the pack have some dependency on an external object, therefore, defeating the multithreading. Yes. It will also just plain perform worse, because it will have to copy over more external objects. This is somewhat made up for getting an actual smaller pack size, but I suspect the completed thin-pack ends up larger than what the server would otherwise send. Because the server is blindly reusing on-disk deltas (which is good, because it takes load off of the server), it misses good delta opportunities between objects in the sent pack (which are likely almost as small, but would not require fixing on the other end). Single-threading the extra work we have to do just exacerbates the problem, of course. Still, I think it will be a net win for end-to-end wall clock time of the operation. You are saving CPU time on the server end, and you're saving network bandwidth with a smaller pack. In my tests on torvalds/linux, doing a fetch across a local machine (so basically discounting network improvements), the times look like (this is end-to-end, counting both server and client CPU time): [vanilla] real0m3.850s user0m7.504s sys 0m0.380s [patched] real0m2.785s user0m2.472s sys 0m0.180s So it was a win both for wall-clock and CPU. What's the use case for a pack file with a SHA1 reference living inside the pack file (why not just use an offset?) Would it make sense to assume that all such files are external and bring them in in the first phase. Once upon a time, ref-delta was the only format supported by packfiles. Later, delta-base-offset was invented, and the client and server negotiate the use of the feature before the packfile is generated (and even when we reuse objects, pack-objects will rewrite the header on the fly to use ref-delta if necessary). These days, pretty much everybody supports delta-base-offset, so I don't think there is any reason index-pack should see ref-delta for a non-thin object. We could probably teach index-pack an --assume-refs-are-thin option to optimize for this case, and have fetch-pack/receive-pack pass it whenever they know that delta-base-offset was negotiated. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] Making use of bitmaps for thin objects
On Sun, Dec 22, 2013 at 11:47:34AM -0800, Ben Maurer wrote: Jeff King's bitmap branch appears to give a very substantial speedup. After applying this branch, the counting objects phase is basically free. However, I found that the compression phase still takes a non-trivial amount of time. Sorry for the slow reply; I've been on vacation. First off, I'm excited that you're looking into using bitmaps. We've been using them for a while at GitHub, but more testing is definitely appreciated. :) When you build your bitmaps, do you set the pack.writeBitmapHashCache option? We found that it makes a significant difference during the compression phase, as otherwise git attempts deltas between random files based on size. Here are some numbers for a simulated fetch from torvalds/linux, representing about 7 weeks of history. Running: from=2d3c627502f2a9b0a7de06a5a2df2365542a72c9 to=f0a679afefc0b6288310f88606b4bb1f243f1aa9 run() { (echo $to echo ^$from) | git pack-objects --stdout --all-progress --revs /dev/null } echo == no hash cache git repack -adb 2/dev/null time run echo == with hash cache git -c pack.writebitmaphashcache=1 repack -adb 2/dev/null time run produces: == no hash cache Counting objects: 20661, done. Delta compression using up to 8 threads. Compressing objects: 100% (7700/7700), done. Writing objects: 100% (20661/20661), 23.23 MiB | 11.13 MiB/s, done. Total 20661 (delta 13884), reused 16638 (delta 12940) real0m3.626s user0m10.760s sys 0m0.060s == with hash cache Counting objects: 20661, done. Delta compression using up to 8 threads. Compressing objects: 100% (7700/7700), done. Writing objects: 100% (20661/20661), 22.64 MiB | 10.82 MiB/s, done. Total 20661 (delta 14038), reused 16638 (delta 12940) real0m3.072s user0m6.168s sys 0m0.100s So our resulting pack shrinks a little because we find better deltas, but note that we save a fair bit of CPU time (the wall clock time ends up not all that different, because the single-threaded writing phase represents a big chunk of that). It looks like most of the time spent compressing objects was in cases where the object was already compressed in the packfile, but the delta was based on an object that the client already had. For some reason, --thin wasn't enabling reuse of these deltas. I'm not too surprised. The long-time strategy for a fetch has been to walk down the haves and wants to their merge base. That boundary commit is marked as a preferred base, meaning we won't send it, but it's a good base for other objects, since we know the client has it. Technically _all_ of the history reachable from that merge base could be marked as a preferred base, but we don't do so for efficiency reasons: 1. It's expensive to walk the full object graph for a small fetch, and 2. You would clog the delta-search algorithm if you had a very large number of preferred-base objects. With bitmaps, though, the history walk is free (we just check each object against our have bitmap), so (1) is a non-issue. For (2), we probably don't want to stick each object into the preferred-base list, but we do want to reuse on-disk deltas we have, if we know the other side has the base. I don't know if you went through the same line of thinking, but that matches your proposed solution. :) This is a hacky, poorly styled attempt to figure how how much better performance could be. With the bitmap branch, git should know what objects the client has already and can easily test if an existing delta can be reused. I don't know the branch well enough to code this, so as a hack, I just assumed the client has any delta base that is in the pack file (for our repo, this is always true, because we have a linear history) Even without a linear history, it mostly works. If you are fetching all of the branches from the other side, then you will end up with all of the objects that the remote has. Which means that either you already have the base, or the remote is about to send it to you. It will break down, though, whenever the other side has something you're not fetching. For that you really need to do the have bitmap check. This greatly reduces the time: $ { echo HEAD echo ^$have; } | time ../git-bitmap/install/bin/git pack-objects --use-bitmap-index --revs --stdout --thin /dev/null Counting objects: 220909, done. Compressing objects: 100% (14203/14203), done. Total 220909 (delta 194050), reused 220909 (delta 199885) 3.57user 1.28system 0:04.59elapsed 105%CPU (0avgtext+0avgdata 2007296maxresident)k 0inputs+0outputs (0major+416243minor)pagefaults 0swaps You might try with --all-progress (or pipe to wc -c), as this should be reducing the output size, too. Here's my same torvalds/linux test, run with the patch I'm including below: Counting objects: 20661, done. Delta compression using up to 8 threads. Compressing objects: 100% (3677/3677), done. Writing
Re: Bug report: stash in upstream caused remote fetch to fail
Jeff King p...@peff.net writes: On Fri, Jan 03, 2014 at 04:12:51PM -0500, Matt Burke wrote: + git init -q + git fetch -q -fu ../../../other '+refs/*:refs/*' fatal: bad object 9b985fbe6a2b783c16756077a8be261c94b6c197 error: ../../../other did not send all necessary objects I was going to ask you to send your repository, but I can easily reproduce here. I guess people don't run into it because it's uncommon to fetch the whole refs/ namespace from a non-bare repo (and bare repos do not tend to have stashes). Here's a minimal reproduction recipe: git init repo cd repo echo content foo git add . git commit -m foo echo more foo git stash git init --bare sub cd sub git fetch .. 'refs/*:refs/*' It looks like we are not feeding refs/stash properly to pack-objects. I'll try to take a closer look later today. I looked at this in the past and I vaguely recall that we reject it in the for-each-ref loop with check-ref-format saying eh, that is a single-level name. At that point I stopped digging, thinking it was a feature ;-) based on your exact observation about stash vs bare/non-bare. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend
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
On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote: On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote: On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote: If submodule.name.branch is set, it *always* creates a new local branch of that name pointing to the exact sha1. If submodule.name.branch is not set, we still create a detached-HEAD checkout of the exact sha1. Thanks for this clarification. Since the usual usage with --remote is with a remote-tracking branch, I confused this here. I am not sure whether blindly creating a local branch from the recorded sha1 is the right thing to do. In what situations would that be helpful? In any situation where your going to develop the submodule locally, you're going to want a branch to develop in. Starting local-submodule developers off on a branch seems useful, even if we can only use submodule.name.branch to guess at their preferred local branch name. Sometimes (often?) the guess will be right. However, A detached HEAD will never be right for local development, so being right sometimes is still an improvement ;). Starting developers at a local submodule branch makes sense. But lets think further. What happens after the initial update? Most times the submodule will already be initialized and cloned. Then developers will still get a detached HEAD even with your local branch feature. If there are no changes on it should we advance the local branch somehow on update? If it does not exist anymore should we recreate it? At $dayjob we usually use feature branches for our work. So if someone wants to work in a submodule you simply create a branch at the current sha1 which you then send out for review. I'm all for named feature branches for development, and in this case submodule.name.branch is likely to be the wrong choice. However, it's still safer to develop in that branch and then rename the branch to match your feature than it would be to develop your fix with a detached HEAD. If your developers have enough discipline to always checkout their feature branch before starting development, my patch won't affect them. However, I know a number of folks who go into fight-or-flight mode when they have a detached HEAD :p. I agree having an initial branch makes it less likely to loose committed changes. Thats good. Also starting on some local branch name and then renaming the branch sounds quite practical. Then we could recreate the default local branch on update (like described above). The reason why one would set a branch option here is to share the superproject branch with colleagues. He can make sure they can always fetch and checkout the submodule even though the branch there is still under cleanup and thus will be rebased often. The commit referenced by sha1 would not be available to a developer fetching after a rebase. Yeah, floating gitlinks are something else. I'd be happy to have that functionality (gitlinks pointing to references) should be built into gitlinks themselves, not added as an additional layer in the submodule script. This gitlinked sha1 rebased out of existence scenario is the first I've heard where I think gitlinked references would be useful. Yeah I have been thinking about this for quite a while now, but have not yet found the time to really think it through and come up with a good solution that does not put you in danger of unprecise revisions. The only solution I can think of is a similar approach as submodule.name.branch gives us now but possibly enabled by some option (i.e.: submodule.name.remote = true). This way you always get the current tip of development but still see the differences (which you can choose to commit) in git status. Thinking through this more, perhaps the logic should be: * If submodule.name.update (defaulting to checkout) is checkout, create a detached HEAD. * Otherwise, create a new branch submodule.name.branch (defaulting to master). Why not trigger the attached state with the submodule.name.branch configuration option? If there is a local branch available use that, if not the tracking branch (as it is currently). Then a developer can start working on the branch with: cd submodule; git checkout -t origin/branchname assuming that submodule update learns some more support for this. Isn't that already what 'git update --remote submodule' already does? Does it? As far as I understood (not using the branch option yet) it only does git checkout origin/branchname so there is no local branch created that tracks the remote branch (-t). What I was thinking is that when submodule.name.branch is set a git submodule update will: 1. if no local branch with that name exists: checkout the remote/branch 2. If a local branch with that name exists: checkout the local branch and possibly advance it according to its setting. Thinking further:
Re: Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'
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
On Mon, Jan 06, 2014 at 03:18:05PM +0100, Heiko Voigt wrote: If you simply want to always checkout the development tip of some project you could do something like this: git submodule foreach 'git fetch git checkout origin/master' Or (respecting submodule.name.branch): $ git submodule update --remote You can even: $ git submodule update --remote --recursive whenever you get an itch to upgrade everything everything in one sweeping, hard-to-debug move ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH] [RFC] Making use of bitmaps for thin objects
Jeff King p...@peff.net writes: We could probably teach index-pack an --assume-refs-are-thin option to optimize for this case, and have fetch-pack/receive-pack pass it whenever they know that delta-base-offset was negotiated. I thought the existing negotiation merely means I understand offset encoded bases, so you are allowed to use that encoding, not I will not accept encoding other than the offset format, so you must use that encoding for everything. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
Heiko Voigt hvo...@hvoigt.net writes: On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote: 2014/1/5 W. Trevor King wk...@tremily.us: On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote: Also it could break some users that rely on the current behavior. The current code always has a detached HEAD after an initial-clone update, regardless of submodule.name.update, which doesn't match those docs either. I perfectly agree with you that the documentation is a bit contradictory with regard to update command and detached HEAD. That's why it's so hard to add a feature and keep the same spirit of those that coded submodules at first. Also, I think that submodules didn't get much feedback with regards to these pitfalls because many people try to setup them, they see that update detaches the HEAD and they think hmmm, maybe submodules are not what I was looking for. I am not so sure about that. Why should detached HEAD make you think like that? For us at $dayjob we have a pre-commit hook that denies you to commit on a detached HEAD and asks you to create a branch first. Perception is irrational ;-) We long-timers think detached is a perfect starting point for both users of submodule who merely want to use the specified commit and developers who want to work on the submodule to match the need of the superproject. The former do not have to do anything, and the latter will have to chdir to the submodule working tree and create a branch (or update the branch with rebase or pull on top of the specified commit) as the first thing before doing anything. Not everybody is a long-timer, but the saving grace is that nobody stays a newcomer. BUT. - developers checkout that commit and don't pull (you can't do git pull in a detached HEAD); Exactly. We consider pull evil ;-) Seriously: To update we only do fast forward merges of local stable branches. ... Yes, why would you do a git pull in a submodule? Don't you want to do something like git checkout -t -b dev/my-topic origin/master to start your development? As long as origin/master contains the commit specified by the superproject, that would work, but it may be a good thing to use a mode of submodule.*.update that does not have to drop the user into a detached state in the first place. I somehow thought that was what rebase (or merge) was about, that is, starting from the state where a branch is checked out in the submodule working tree, an update in the superproject would cause that branch checked out in the submodule brought up to date with respect to the commit specified in the superproject tree. If that is not how it is supposed to work, please correct me---and we may have to add another mode that does so (or even make rebase/merge do so as a bugfix). And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command
W. Trevor King wk...@tremily.us writes: On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote: +case $update_module in +'') +;; # Unset update mode +checkout | rebase | merge | none) +;; # Known update modes +!*) +;; # Custom update command +*) +update_module= +echo 2 warning: invalid update mode for submodule '$name' +;; +esac I'd prefer `die …` to `echo 2 …`. It's hard to know if mapping the user's preferred (unknown) update mechanism to 'checkout' is serious or not. This commit also makes me think that --rebase, --merge, and --checkout should be replaced with a single --update={rebase|merge|checkout|!…} option, but that's probably food for another commit (and a long finger-breaking deprecation period). All of the above points sound sensible to me. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Aw: Re: Re: Re: [Bug report] 'git status' always says Your branch is up-to-date with 'origin/master'
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
Jiang Xin worldhello@gmail.com writes: Please pull l10n update for maint branch. It can be merged to master without conflict. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] format-patch: introduce format.defaultTo
A very common workflow for preparing patches involves working off a topic branch and generating patches against 'master' to send off to the maintainer. However, a plain $ git format-patch -o outgoing is a no-op on a topic branch, and the user has to remember to specify 'master' explicitly everytime. Save the user the extra keystrokes by introducing format.defaultTo which can contain the name of a branch against which to base patches. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Documentation/config.txt | 4 builtin/log.c | 7 +++ contrib/completion/git-completion.bash | 1 + t/t4014-format-patch.sh| 10 ++ 4 files changed, 22 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index a405806..b90abd1 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1135,6 +1135,10 @@ format.coverLetter:: format-patch is invoked, but in addition can be set to auto, to generate a cover-letter only when there's more than one patch. +format.defaultTo:: + The name of a branch against which to generate patches by + default. You'd usually want this to be 'master'. + filter.driver.clean:: The command which is used to convert the content of a worktree file to a blob upon checkin. See linkgit:gitattributes[5] for diff --git a/builtin/log.c b/builtin/log.c index b97373d..ebc419e 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -674,6 +674,7 @@ static int thread; static int do_signoff; static const char *signature = git_version_string; static int config_cover_letter; +static const char *config_defaultto; enum { COVER_UNSET, @@ -750,6 +751,8 @@ static int git_format_config(const char *var, const char *value, void *cb) config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF; return 0; } + if (!strcmp(var, format.defaultto)) + return git_config_string(config_defaultto, var, value); return git_log_config(var, value, cb); } @@ -1324,6 +1327,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) die (_(--subject-prefix and -k are mutually exclusive.)); rev.preserve_subject = keep_subject; + if (argc 2 config_defaultto) { + argv[1] = config_defaultto; + argc++; + } argc = setup_revisions(argc, argv, rev, s_r_opt); if (argc 1) die (_(unrecognized argument: %s), argv[1]); diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 39b81f7..75699d4 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1992,6 +1992,7 @@ _git_config () format.attach format.cc format.coverLetter + format.defaultTo format.headers format.numbered format.pretty diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 73194b2..46c0337 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1370,4 +1370,14 @@ test_expect_success 'cover letter auto user override' ' test_line_count = 2 list ' +test_expect_success 'defaultTo side' ' + mkdir -p tmp + test_when_finished rm -rf tmp; + git config --unset format.defaultTo + + git config format.defaultTo side + git format-patch -o tmp list + test_line_count = 3 list +' + test_done -- 1.8.5.2.229.g4448466.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Improve user-manual html and pdf formatting
Thomas Ackermann th.ac...@arcor.de writes: Use asciidoc style 'article' instead of 'book' and change asciidoc title level. This removes blank first page and superfluous Part I page (there is no Part II) in pdf output. Also pdf size is decreased by this from 77 to 67 pages. In html output this removes unnecessary sub-tocs and chapter numbering. Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/Makefile| 2 +- Documentation/user-manual.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 91a12c7..36c58fc 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -324,7 +324,7 @@ manpage-base-url.xsl: manpage-base-url.xsl.in user-manual.xml: user-manual.txt user-manual.conf $(QUIET_ASCIIDOC)$(RM) $@+ $@ \ - $(ASCIIDOC) $(ASCIIDOC_EXTRA) -b docbook -d book -o $@+ $ \ + $(ASCIIDOC) $(ASCIIDOC_EXTRA) -b docbook -d article -o $@+ $ \ mv $@+ $@ technical/api-index.txt: technical/api-index-skel.txt \ diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index cbb01a1..130c2f4 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -1,5 +1,5 @@ -Git User Manual -___ +#65279;Git User Manual +=== Will queue after dropping the extra. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
On Mon, Jan 06, 2014 at 04:47:39PM +0100, Heiko Voigt wrote: On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote: On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote: On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote: Thinking through this more, perhaps the logic should be: * If submodule.name.update (defaulting to checkout) is checkout, create a detached HEAD. * Otherwise, create a new branch submodule.name.branch (defaulting to master). Why not trigger the attached state with the submodule.name.branch configuration option? If there is a local branch available use that, if not the tracking branch (as it is currently). Then a developer can start working on the branch with: cd submodule; git checkout -t origin/branchname assuming that submodule update learns some more support for this. Isn't that already what 'git update --remote submodule' already does? Does it? As far as I understood (not using the branch option yet) it only does git checkout origin/branchname so there is no local branch created that tracks the remote branch (-t). That's right. Anyone who wants to do local development in a submodule should probably not be using checkout updates, hence my proposal above to base local-branch creation on submodule.name.update. What I was thinking is that when submodule.name.branch is set a git submodule update will: 1. if no local branch with that name exists: checkout the remote/branch 2. If a local branch with that name exists: checkout the local branch and possibly advance it according to its setting. This sounds too complicated to me ;). Thinking further: Maybe submodule.name.update = pull could denote that a user wants to have a branch ready for work in a submodule. This sounds like my quoted realization above. We both even preface the idea with thinking ;). However, I think merge, rebase, !command, and all other non-checkout update schemes are already signals that the developer is interested in local developent (and therefore wants a branch), without the need to add an aditional 'pull' (and then how to distinguish between rebase/merge?). submodule update will then 1. if no local branch with that name exists: - automatically create the branch based on the referenced sha1 - set up that its tracking remote/branch With my patch this happens with the initial clone-update (as of v2, only when submodule.name.branch is set. In a hypothetical v3, only when submodule.name.update is not checkout). I'm not sure we want to do this if the user switches to non-checkout updates after the initial cloning update though. They may actually have work in that detached HEAD that we'd be clobbering. - issue a git pull in the submodule I think that updating using the gitlinked sha1 (a local update) and updating using the upstream origin/$branch (a --remote update) should always be two distinct events. Combining them in a single operation just complicates the situation. 2. if a local branch with that name exists: - issue a git pull in the submodule That's what we already have with submodule.name.update as 'merge'. The merged object is either the gitlinked sha1 (a local update) or a re-fetched upstream branch tip (a --remote update). We are on a local branch at this point, but not neccessarily pointing at the gitlinked sha1. The reset here ensures that the new local branch does indeed point at the gitlinked sha1. But isn't this a fresh clone? Why should the branch point at anything else? We don't pass $sha1 to module_clone(). Before my patch, we don't even pass $branch to module_clone(). That means that module_clone() will only checkout the gitlinked sha1 when the upstream HEAD (or $branch with my patch) happens to point to the gitlinked sha1. For example, if Alice adds Charie's repo as a submodule (gitlinking his current master d2dbd39), then Charlie pushes a new commit d0de817 to his master, and then Bob clones Alice's superproject. Post-clone, Charlie's submodule will have checked out Charlie's new d0de817, and we need update's additional: git reset --hard -q d2dbd39 to rewind to Alice's gitlinked sha1. Ah yeah, sorry I was confusing this with the checkout of remote/branch here again. Since I have done that twice already maybe we should be careful about not confusing users with this as well... After wrapping my head around the fact that you want to simply create a local branch on the referenced sha1 (and hopefully remembering it) I still would like to think a little more about it and let it settle a bit. The way I keep this straight is: 1. Submodules are links to Git commits (that's how they're stored in the index). 2. There are two places to look if you want to update the linked commit: a. The superproject's tree (a local updates). b.
[PATCH 1/2] completion: complete format.coverLetter
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- contrib/completion/git-completion.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 51c2dd4..39b81f7 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1991,6 +1991,7 @@ _git_config () fetch.unpackLimit format.attach format.cc + format.coverLetter format.headers format.numbered format.pretty -- 1.8.5.2.229.g4448466.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Minor convinience feature: format.defaultTo
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
On Mon, Jan 06, 2014 at 08:56:22AM -0800, Junio C Hamano wrote: Heiko Voigt hvo...@hvoigt.net writes: Yes, why would you do a git pull in a submodule? Don't you want to do something like git checkout -t -b dev/my-topic origin/master to start your development? As long as origin/master contains the commit specified by the superproject, that would work, but it may be a good thing to use a mode of submodule.*.update that does not have to drop the user into a detached state in the first place. I somehow thought that was what rebase (or merge) was about, that is, starting from the state where a branch is checked out in the submodule working tree, an update in the superproject would cause that branch checked out in the submodule brought up to date with respect to the commit specified in the superproject tree. That is my understanding as well. In fact, I don't think the detached-HEAD-vs-branch distinction is important here, you can still rebase your detached HEAD onto the superproject's referenced commit (or origin/$branch with --remote). This will also work for merge, and should work for well-crafted !commands. And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? I think so, but we currently don't have a never detached route for folks that are cloning submodules via update (instead of via 'submodule add'). Currently, new clone-updates will always leave you with a detached HEAD (unless you have branch-creation in your update !command). My patch aims to close this detached-HEAD gap, for folks we expect will be doing local development, by creating an initial branch at clone-update time. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[BUG] rebase --onto doesn't abort properly
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 gits...@pobox.com writes: W. Trevor King wk...@tremily.us writes: On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote: + case $update_module in + '') + ;; # Unset update mode + checkout | rebase | merge | none) + ;; # Known update modes + !*) + ;; # Custom update command + *) + update_module= + echo 2 warning: invalid update mode for submodule '$name' + ;; + esac I'd prefer `die …` to `echo 2 …`. It's hard to know if mapping the user's preferred (unknown) update mechanism to 'checkout' is serious or not. This commit also makes me think that --rebase, --merge, and --checkout should be replaced with a single --update={rebase|merge|checkout|!…} option, but that's probably food for another commit (and a long finger-breaking deprecation period). All of the above points sound sensible to me. I'll tentatively queue this on 'pu' (with the suggested die update), with some rewording of the log message. The patch needs to be signed-off, though. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command
Ok, applying the suggested modifications and resending shortly. Thank you, Francesco 2014/1/6 Junio C Hamano gits...@pobox.com: Junio C Hamano gits...@pobox.com writes: W. Trevor King wk...@tremily.us writes: On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote: + case $update_module in + '') + ;; # Unset update mode + checkout | rebase | merge | none) + ;; # Known update modes + !*) + ;; # Custom update command + *) + update_module= + echo 2 warning: invalid update mode for submodule '$name' + ;; + esac I'd prefer `die …` to `echo 2 …`. It's hard to know if mapping the user's preferred (unknown) update mechanism to 'checkout' is serious or not. This commit also makes me think that --rebase, --merge, and --checkout should be replaced with a single --update={rebase|merge|checkout|!…} option, but that's probably food for another commit (and a long finger-breaking deprecation period). All of the above points sound sensible to me. I'll tentatively queue this on 'pu' (with the suggested die update), with some rewording of the log message. The patch needs to be signed-off, though. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/17] lock_ref_sha1_basic(): on SCLD_VANISHED, retry
Michael Haggerty mhag...@alum.mit.edu writes: If safe_create_leading_directories() fails because a file along the path unexpectedly vanished, try again (up to 3 times). This can occur if another process is deleting directories at the same time as we are trying to make them. For example, git pack-refs --all tries to delete the loose refs and any empty directories that are left behind. If a pack-refs process is running, then it might delete a directory that we need to put a new loose reference in. If safe_create_leading_directories() thinks this might have happened, then take its advice and try again (maximum three attempts). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 3926136..6eb8a02 100644 --- a/refs.c +++ b/refs.c @@ -2039,6 +2039,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, int type, lflags; int mustexist = (old_sha1 !is_null_sha1(old_sha1)); int missing = 0; + int attempts = 3; lock = xcalloc(1, sizeof(struct ref_lock)); lock-lock_fd = -1; @@ -2093,7 +2094,15 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, if ((flags REF_NODEREF) (type REF_ISSYMREF)) lock-force_write = 1; - if (safe_create_leading_directories(ref_file)) { + retry: + switch (safe_create_leading_directories(ref_file)) { + case SCLD_OK: + break; /* success */ + case SCLD_VANISHED: + if (--attempts 0) + goto retry; + /* fall through */ Hmph. Having no backoff/sleep at all might be OK here as long as the other side that removes does not retry (and I do not think the other side would, even though I haven't read through the series to the end yet ;-)). This may be just a style thing, but I find that the variable name attempts that starts out as 3 quite misleading, as its value is not the number of attempts made but the remaining number of attempts allowed. Starting it from 0 and then if (attempts++ MAX_ATTEMPTS) goto retry; would be one way to clarify it. Renaming it to remaining_attempts would be another. + default: last_errno = errno; error(unable to create directory for %s, ref_file); goto error_return; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 13/17] remove_dir_recurse(): handle disappearing files and directories
Michael Haggerty mhag...@alum.mit.edu writes: If a file or directory that we are trying to remove disappears (e.g., because another process has pruned it), do not consider it an error. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- dir.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/dir.c b/dir.c index 11e1520..716b613 100644 --- a/dir.c +++ b/dir.c @@ -1476,7 +1476,9 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) flag = ~REMOVE_DIR_KEEP_TOPLEVEL; dir = opendir(path-buf); if (!dir) { - if (errno == EACCES !keep_toplevel) + if (errno == ENOENT) + return keep_toplevel ? -1 : 0; If we were told that foo/bar must go, but do not bother removing foo/, is it an error if foo itself did not exist? I think this step does not change the behaviour from the original (we used to say oh, we were told to keep_toplevel, and the top-level cannot be read for whatever reason, so it is an error), but because this series is giving us a finer grained error diagnosis, we may want to think about it further, perhaps as a follow-up. I also wonder why the keep-toplevel logic is in this recurse part of the callchain. If everything in foo/bar/ can be removed but foo/bar/ is unreadable, it is OK, when opendir(foo/bar) failed with EACCESS, to attempt to rmdir(foo/bar) whether we were told not to attempt removing foo/, no? + else if (errno == EACCES !keep_toplevel) That is, I am wondering why this part just checks !keep_toplevel, not (!keep_toplevel || has_dir_separator(path-buf)) or something. /* * An empty dir could be removable even if it * is unreadable: @@ -1496,13 +1498,21 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) strbuf_setlen(path, len); strbuf_addstr(path, e-d_name); - if (lstat(path-buf, st)) - ; /* fall thru */ - else if (S_ISDIR(st.st_mode)) { + if (lstat(path-buf, st)) { + if (errno == ENOENT) + /* + * file disappeared, which is what we + * wanted anyway + */ + continue; + /* fall thru */ + } else if (S_ISDIR(st.st_mode)) { if (!remove_dir_recurse(path, flag, kept_down)) continue; /* happy */ - } else if (!only_empty !unlink(path-buf)) + } else if (!only_empty +(!unlink(path-buf) || errno == ENOENT)) { continue; /* happy, too */ + } /* path too long, stat fails, or non-directory still exists */ ret = -1; @@ -1512,7 +1522,7 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) strbuf_setlen(path, original_len); if (!ret !keep_toplevel !kept_down) - ret = rmdir(path-buf); + ret = (!rmdir(path-buf) || errno == ENOENT) ? 0 : -1; else if (kept_up) /* * report the uplevel that it is not an error that we -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 17/17] rename_tmp_log(): on SCLD_VANISHED, retry
Michael Haggerty mhag...@alum.mit.edu writes: If safe_create_leading_directories() fails because a file along the path unexpectedly vanished, try again from the beginning. Try at most 3 times. As the previous step bumped it from 3 to 4 without explanation, the above no longer reflects reality ;-) The series mostly looked sane from a cursory read. Will re-queue. Thanks. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 490525a..810f802 100644 --- a/refs.c +++ b/refs.c @@ -2533,7 +2533,14 @@ static int rename_tmp_log(const char *newrefname) int attempts = 4; retry: - if (safe_create_leading_directories(git_path(logs/%s, newrefname))) { + switch (safe_create_leading_directories(git_path(logs/%s, newrefname))) { + case SCLD_OK: + break; /* success */ + case SCLD_VANISHED: + if (--attempts 0) + goto retry; + /* fall through */ + default: error(unable to create directory for %s, newrefname); return -1; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/17] safe_create_leading_directories(): add explicit slash pointer
Michael Haggerty mhag...@alum.mit.edu writes: Keep track of the position of the slash character independently of pos, thereby making the purpose of each variable clearer and working towards other upcoming changes. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- This step has an interaction with $gmane/239878 where Windows folks want it to pay attention to is_dir_sep()---over there, a backslash could separate directory path components. AFAIK, the function was meant to be used only on paths we internally generate, and the paths we internally generate all are slash separated, so it could be argued that feeding a path, whose path components are separated by backslashes, that we obtained from the end user without converting it to the internal form in some codepaths (e.g. $there in git clone $url $there) are bugs we acquired over time that need to be fixed, but it is easy enough to use is_dir_sep() here to work it around, and doing so will not negatively affect 1. UNIX-only projects by forbidding use of a byte with backslash in it as a path component character (yes, I am imagining using Shift-JIS that can use a backslash as the second byte of two-byte character in the pathname on UNIX); and 2. UNIX-and-Windows mixed projects, as you cannot sanely use such a pathname with backslash as part of a path component if its tree needs to be checked out on Windows. sha1_file.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index cc9957e..197766d 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -111,19 +111,21 @@ int safe_create_leading_directories(char *path) while (pos) { struct stat st; + char *slash = strchr(pos, '/'); - pos = strchr(pos, '/'); - if (!pos) + if (!slash) break; - while (*++pos == '/') - ; + while (*(slash + 1) == '/') + slash++; + pos = slash + 1; if (!*pos) break; - *--pos = '\0'; + + *slash = '\0'; if (!stat(path, st)) { /* path exists */ if (!S_ISDIR(st.st_mode)) { - *pos = '/'; + *slash = '/'; return -3; } } else if (mkdir(path, 0777)) { @@ -131,14 +133,14 @@ int safe_create_leading_directories(char *path) !stat(path, st) S_ISDIR(st.st_mode)) { ; /* somebody created it since we checked */ } else { - *pos = '/'; + *slash = '/'; return -1; } } else if (adjust_shared_perm(path)) { - *pos = '/'; + *slash = '/'; return -2; } - *pos++ = '/'; + *slash = '/'; } return 0; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Hi, Ramkumar Ramachandra wrote: a plain $ git format-patch -o outgoing is a no-op on a topic branch, and the user has to remember to specify 'master' explicitly everytime. Save the user the extra keystrokes by introducing format.defaultTo Not excited. Two reasons: 1. Most config settings are in noun form: e.g., [remote] pushDefault = foo. That makes their names easy to guess and makes them easy to talk about: I set the default remote for pushing by changing the remote.pushdefault setting. '[url foo] insteadOf' is an exception to that and a bit of an aberration. This new '[format] defaultTo' repeats the same end-with-a-preposition mistake, while I think it would be better to learn from it. 2. Wouldn't a more natural default be @{u}..HEAD instead of relying on the user to do the make-work of keeping a local branch that tracks master up to date? Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Ramkumar Ramachandra artag...@gmail.com writes: A very common workflow for preparing patches involves working off a topic branch and generating patches against 'master' to send off to the maintainer. However, a plain $ git format-patch -o outgoing is a no-op on a topic branch,... Two points. - why is a single branch name sufficient? - is it a better option to simply default to @{u}, if one exists, instead of failing? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] rebase --onto doesn't abort properly
Ramkumar Ramachandra artag...@gmail.com writes: Hi, On the latest git, I noticed that a rebase --onto doesn't abort properly. Steps to reproduce: # on some topic branch $ git rebase --onto master @~10 ^C # quickly! $ git rebase --abort # HEAD is still detached I do not think --abort was designed to abort an uncontrolled stop like ^C in the first place. To allow that kind of recovery, you need to teach rebase to first record the state you would want to go back to before doing anything, but then what happens if the ^C comes when you are still in the middle of doing so? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Junio C Hamano wrote: - why is a single branch name sufficient? It does accept a revision, so any form is allowed; but why would anyone want that in a format.defaultTo? I'm not sure we want to impose an artificial restriction on the configuration variable though. - is it a better option to simply default to @{u}, if one exists, instead of failing? I'm not sure @{u} is a good default. Personally, my workflow involves publishing my fork before sending out patches; mainly so that I can compare with @{u} when I do re-spins. People can put @{u} in format.defaultTo if it suits their workflow though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] rebase --onto doesn't abort properly
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
According to Documentation/gitmodules.txt, 'checkout' is a valid 'submodule.name.update' command. Also git-submodule.sh refers to it and processes it correctly. Reflecting commit 'ac1fbb' to support this syntax and also validate property values during 'update' command, issuing an error if the value found is unknown. Signed-off-by: Francesco Pretto cez...@gmail.com --- git-submodule.sh | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 2677f2e..4a30087 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -622,7 +622,7 @@ cmd_init() test -z $(git config submodule.$name.update) then case $upd in - rebase | merge | none) + checkout | rebase | merge | none) ;; # known modes of updating *) echo 2 warning: unknown update mode '$upd' suggested for submodule '$name' @@ -805,6 +805,17 @@ cmd_update() update_module=$update else update_module=$(git config submodule.$name.update) + case $update_module in + '') + ;; # Unset update mode + checkout | rebase | merge | none) + ;; # Known update modes + !*) + ;; # Custom update command + *) + die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') + ;; + esac fi displaypath=$(relative_path $prefix$sm_path) -- 1.8.5.2.229.g4448466.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Jonathan Nieder wrote: 1. Most config settings are in noun form: e.g., [remote] pushDefault = foo. That makes their names easy to guess and makes them easy to talk about: I set the default remote for pushing by changing the remote.pushdefault setting. '[url foo] insteadOf' is an exception to that and a bit of an aberration. This new '[format] defaultTo' repeats the same end-with-a-preposition mistake, while I think it would be better to learn from it. I agree that it's somewhat unconventional to allow people to put a revision as a configuration variable value, but I think it's useful. url.url.insteadOf is incredibly useful, for instance. 2. Wouldn't a more natural default be @{u}..HEAD instead of relying on the user to do the make-work of keeping a local branch that tracks master up to date? Like I said in my message to Junio, @{u} is not necessarily the best default for all workflows, although you can fill that into format.defaultTo. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mv: better document side effects when moving a submodule
The Submodules section of the git mv documentation mentions what will happen when a submodule with a gitfile gets moved with newer git. But it doesn't talk about what happens when the user changes between commits before and after the move, which does not update the work tree like using the mv command did the first time. Explain what happens and what the user has to do manually to fix that. Also document this in a new test. Reported-by: George Papanikolaou g3orge@gmail.com Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Am 09.12.2013 18:49, schrieb Jens Lehmann: Am 09.12.2013 11:59, schrieb George Papanikolaou: Also after mv you need to run 'submodule update' and I think this should be documented somewhere. You're right, this should be mentioned in the mv man page. I'll prepare a patch for that. Does this new paragraph make it clearer? Documentation/git-mv.txt | 10 ++ t/t7001-mv.sh| 21 + 2 files changed, 31 insertions(+) diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt index b1f7988..c9e8568 100644 --- a/Documentation/git-mv.txt +++ b/Documentation/git-mv.txt @@ -52,6 +52,16 @@ core.worktree setting to make the submodule work in the new location. It also will attempt to update the submodule.name.path setting in the linkgit:gitmodules[5] file and stage that file (unless -n is used). +Please note that each time a superproject update moves a populated +submodule (e.g. when switching between commits before and after the +move) a stale submodule checkout will remain in the old location +and an empty directory will appear in the new location. To populate +the submodule again in the new location the user will have to run +git submodule update afterwards. Removing the old directory is +only safe when it uses a gitfile, as otherwise the history of the +submodule will be deleted too. Both steps will be obsolete when +recursive submodule update has been implemented. + GIT --- Part of the linkgit:git[1] suite diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 3bfdfed..e3c8c2c 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -442,4 +442,25 @@ test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' ' git diff-files --quiet -- sub .gitmodules ' +test_expect_success 'checking out a commit before submodule moved needs manual updates' ' + git mv sub sub2 + git commit -m moved sub to sub2 + git checkout -q HEAD^ 2actual + echo warning: unable to rmdir sub2: Directory not empty expected + test_i18ncmp expected actual + git status -s sub2 actual + echo ?? sub2/ expected + test_cmp expected actual + ! test -f sub/.git + test -f sub2/.git + git submodule update + test -f sub/.git + rm -rf sub2 + git diff-index --exit-code HEAD + git update-index --refresh + git diff-files --quiet -- sub .gitmodules + git status -s sub2 actual + ! test -s actual +' + test_done -- 1.8.5.2.230.g9325930 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report: stash in upstream caused remote fetch to fail
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
On Mon, Jan 06, 2014 at 08:37:49AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: We could probably teach index-pack an --assume-refs-are-thin option to optimize for this case, and have fetch-pack/receive-pack pass it whenever they know that delta-base-offset was negotiated. I thought the existing negotiation merely means I understand offset encoded bases, so you are allowed to use that encoding, not I will not accept encoding other than the offset format, so you must use that encoding for everything. You are right about what it means. But this is an optimization, not a correctness thing. So if we assume that senders who are allowed to send offsets will generally do so, it might be a reasonable optimization to guess that ref-delta objects will need thin completion. If we are wrong, the worst case is that we add an extra local object to the end of the pack. So as long as we are right most of the time, it may still be a win. Of course, it may also be possible to simply multi-thread the thin-completion portion of index-pack. That would be even better, though I am not sure how it would work. The resolution of an object in one thread can always become the input for another thread. But maybe we could have each thread come up with a proposed set of objects to add to the pack, and then drop duplicates or something. I haven't looked closely. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: - why is a single branch name sufficient? It does accept a revision, so any form is allowed; but why would anyone want that in a format.defaultTo? I'm not sure we want to impose an artificial restriction on the configuration variable though. I meant a single branch as opposed to depending on what branch you are sending out, you may have to use a different upstream starting point, and a single format.defaultTo that does not read what your HEAD currently points at may not be enough. Unless you set @{u} to this new configuration, in which case the choice becomes dynamic depending on the current branch, but - if that is the only sane choice based on the current branch, why not use that as the default without having to set the configuration? - Or if that is still insufficient, don't we need branch.*.forkedFrom that is different from branch.*.merge, so that different branches you want to show format-patch output can have different reference points? After all, format-patch to send things out to upstream is like asking the other side to do a rebase you would do in your repository, so whatever git rebase that were too lazy to specify what the fork point was when applying may be a reasonable type-saver default. Yes, sometimes people need to rebase onto somewhere they did not fork from, but that is why they can give explicit $upstream and $onto to the command---I do not think it is any different for format-patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report: stash in upstream caused remote fetch to fail
Jeff King p...@peff.net writes: On Mon, Jan 06, 2014 at 08:16:31AM -0800, Junio C Hamano wrote: I was going to ask you to send your repository, but I can easily reproduce here. I guess people don't run into it because it's uncommon to fetch the whole refs/ namespace from a non-bare repo (and bare repos do not tend to have stashes). Here's a minimal reproduction recipe: git init repo cd repo echo content foo git add . git commit -m foo echo more foo git stash git init --bare sub cd sub git fetch .. 'refs/*:refs/*' It looks like we are not feeding refs/stash properly to pack-objects. I'll try to take a closer look later today. I looked at this in the past and I vaguely recall that we reject it in the for-each-ref loop with check-ref-format saying eh, that is a single-level name. At that point I stopped digging, thinking it was a feature ;-) based on your exact observation about stash vs bare/non-bare. I am fine with rejecting it with a warning, but we should not then complain that the other side did not send us the object, since we should not be asking for it at all. I also do not see us complaining about the funny ref anywhere. So there is definitely _a_ bug here. :) Oh, no question about that. I was just pointing somebody who already has volunteered to take a look in a direction I recall was where the issue was ;-) Thanks. I think somebody else mentioned recently that we do not handle malformed refs consistently. I think it was: http://article.gmane.org/gmane.comp.version-control.git/239381 which might or might not be related. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
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
On Mon, Jan 6, 2014 at 3:18 PM, Jeff King p...@peff.net wrote: On Mon, Jan 06, 2014 at 12:06:51PM -0800, Junio C Hamano wrote: Unless you set @{u} to this new configuration, in which case the choice becomes dynamic depending on the current branch, but - if that is the only sane choice based on the current branch, why not use that as the default without having to set the configuration? - Or if that is still insufficient, don't we need branch.*.forkedFrom that is different from branch.*.merge, so that different branches you want to show format-patch output can have different reference points? Yeah, I had similar thoughts. I personally use branch.*.merge as forkedFrom, and it seems like we are going that way anyway with things like git rebase and git merge defaulting to upstream. But then there is git push -u and push.default = upstream, which treats the upstream config as something else entirely. Just for more reference, I rarely use branch.*.merge as forkedFrom. I typically want to use master as my target, but like Ram, I publish my changes elsewhere for safe keeping. I think in a typical, feature branch-based workflow @{u} would be nearly useless. -John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Jeff King p...@peff.net writes: On Mon, Jan 06, 2014 at 12:06:51PM -0800, Junio C Hamano wrote: Unless you set @{u} to this new configuration, in which case the choice becomes dynamic depending on the current branch, but - if that is the only sane choice based on the current branch, why not use that as the default without having to set the configuration? - Or if that is still insufficient, don't we need branch.*.forkedFrom that is different from branch.*.merge, so that different branches you want to show format-patch output can have different reference points? Yeah, I had similar thoughts. I personally use branch.*.merge as forkedFrom, and it seems like we are going that way anyway with things like git rebase and git merge defaulting to upstream. But then there is git push -u and push.default = upstream, which treats the upstream config as something else entirely. So it seems like there is already some confusion, and either way we go, thisis making it worse to some degree (I do not blame Ram, but rather he has stumbled into a hidden sand pit that we have been building for the past few years... :). I wonder if it is too late to try to clarify this dual usage. It kind of seems like the push config is this is the place I publish to. Which, in many workflows, just so happens to be the exact same as the place you forked from. Could we introduce a new branch.*.pushupstream variable that falls back to branch.*.merge? Or is that just throwing more fuel on the fire (more sand in the pit in my analogy, I guess). I admit I haven't thought it through yet, though. And even if it does work, it may throw a slight monkey wrench in the proposed push.default transition. Yeah, when I say upstream, I never mean it as where I publish. Your upstream is where you get others' work from. For a push to somewhere for safekeeping or other people to look at triangular workflow, it does not make any sense to treat that I publish there place as an upstream (hence having branch.*.remote pointing at that publishing point). Once you stop doing that, and instead using branch.*.remote = origin, and branch.*.merge = master, where 'origin' is not your publishing point, @{u} will again start making sense, I think. And I thought that is what setting remote.pushdefault to the publishing point repository was about. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] Making use of bitmaps for thin objects
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
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
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
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
On Mon, Jan 6, 2014 at 3:42 PM, Jonathan Nieder jrnie...@gmail.com wrote: John Szakmeister wrote: I think in a typical, feature branch-based workflow @{u} would be nearly useless. I thought the idea of @{u} was that it represents which ref one typically wants to compare the current branch to. It is used by 'git branch -v' to show how far ahead or behind a branch is and used by 'git pull --rebase' to forward-port a branch, for example. So a topic branch with @{u} pointing to 'master' or 'origin/master' seems pretty normal and hopefully the shortcuts it allows can make life more convenient. Is there an outline of this git workflow in the documentation somewhere? Do you save your work in a forked repo anywhere? If so, how do you typically save your work. I typically have my @{u} pointing to where I save my work. Perhaps I'm missing something important here, but I don't feel like the current command set and typical workflow (at least those in tutorials) leads you in that direction. Here is one example: https://www.atlassian.com/git/workflows#!workflow-feature-branch It is *not* primarily about where the branch gets pushed. After all, in both the 'matching' and the 'simple' mode, git push does not push the current branch to its upstream @{u} unless @{u} happens to have the same name. Then where does it get pushed? Do you always specify where to save your work? FWIW, I think the idea of treating @{u} as the eventual recipient of your changes is good, but then it seems like Git is lacking the publish my changes to this other branch concept. Am I missing something? If there is something other than @{u} to represent this latter concept, I think `git push` should default to that instead. But, at least with my current knowledge, that doesn't exist--without explicitly saying so--or treating @{u} as that branch. If there's a better way to do this, I'd love to hear it! Thanks! -John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] [RFC] Making use of bitmaps for thin objects
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
Jeff King p...@peff.net writes: On Mon, Jan 06, 2014 at 12:38:30PM -0800, Junio C Hamano wrote: I wonder if it is too late to try to clarify this dual usage. It kind of seems like the push config is this is the place I publish to. Which, in many workflows, just so happens to be the exact same as the place you forked from. Could we introduce a new branch.*.pushupstream variable that falls back to branch.*.merge? Or is that just throwing more fuel on the fire (more sand in the pit in my analogy, I guess). I admit I haven't thought it through yet, though. And even if it does work, it may throw a slight monkey wrench in the proposed push.default transition. Yeah, when I say upstream, I never mean it as where I publish. Your upstream is where you get others' work from. That's my thinking, as well, but it means the upstream push.default is nonsensical. I've thought that all along, but it seems like other people find it useful. I guess because they are in a non-triangular, non-feature-branch setup (I suppose you could think of a central-repo feature-branch workflow as a special form of triangular setup, where the remote is bi-directional, but the branch names are triangular). If we want to declare push -u and push.default=upstream as tools for certain simple bi-directional workflows, that makes sense. But I suspect it may cause extra confusion when people make the jump to using a triangular workflow. I do not think there is no want to declare involved. If I correctly recall how push -u came about, it was merely a way to appease those who complained that their new branch created by running checkout -b branch origin/branch has already set up the branch.*.remote and branch.*.merge configurations nicely for them and allow them to immediately go ahead and start using the centralized I merge from their 'branch' and push to that, but when they create a new branch on their own and want to make the branch of the same name at the origin to be the upstream, they have to futz with the configuration. The declaration was made long time ago when we started using @{upstream}, and there is no new extra confusion. For a push to somewhere for safekeeping or other people to look at triangular workflow, it does not make any sense to treat that I publish there place as an upstream (hence having branch.*.remote pointing at that publishing point). You _might_ treat it the same way we treat the upstream, in some special cases. For example, when you say git status, it is useful to see how your topic and the upstream have progressed (i.e., do I need to pull from upstream?). But you may _also_ want to know how your local branch differs from its pushed counterpart (i.e., do I have unsaved commits here that I want to push up?). Correct; I am not saying where do I publish is never relevant. It is just it is not something useful for format-patch to use as the default fork-point. So having two config options might help with that. Of course, your push upstream (or whatever you want to call it) does not logically have one value. You may push to several places, and would want to compare to each. Yes. But most likely, if you always push a single branch to multiple places, it won't be like you push it to only one of the places today and another one tomorrow, leaving everybody out of sync. Unless there is a site that is temporarily down, in which case that one may stay behind, the normal state would be that all of them point at the same commit (that is how I publish to multiple places anyway). Once you stop doing that, and instead using branch.*.remote = origin, and branch.*.merge = master, where 'origin' is not your publishing point, @{u} will again start making sense, I think. And I thought that is what setting remote.pushdefault to the publishing point repository was about. If that were sufficient, then we would just need push.default = current, and not upstream (nor simple). I lobbied for that during the discussion, but people seemed to think that upstream was better/more useful. Maybe it was just because remote.pushdefault did not exist then. Yeah, I think in the 2.0 world with pushdefault (i.e. triangular), the default 'simple' turns into 'current'. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
W. Trevor King wk...@tremily.us writes: And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? I think so, but we currently don't have a never detached route for folks that are cloning submodules via update (instead of via 'submodule add'). Currently, new clone-updates will always leave you with a detached HEAD (unless you have branch-creation in your update !command). My patch aims to close this detached-HEAD gap, for folks we expect will be doing local development, by creating an initial branch at clone-update time. I am not a submodule expert so I may be missing some other gaps, but what your change does sounds sensible to me. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
John Szakmeister j...@szakmeister.net writes: Am I missing something? If there is something other than @{u} to represent this latter concept, I think `git push` should default to that instead. But, at least with my current knowledge, that doesn't exist--without explicitly saying so--or treating @{u} as that branch. If there's a better way to do this, I'd love to hear it! I see Ram who worked on landing the remote.pushdefault feature is CC'ed; this work was started in early April 2013 and your config and workflow may not have been adjusted to it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: do not fail on invalid bookmarks
Thanks for noticing, I can reproduce at work, I will try to come-up with an improved version soon, Cheers, Antoine On Mon, Jan 6, 2014 at 2:52 PM, Torsten Bögershausen tbo...@web.de wrote: On 2013-12-29 12.30, Antoine Pelisse wrote: Mercurial can have bookmarks pointing to nullid (the empty root revision), while Git can not have references to it. When cloning or fetching from a Mercurial repository that has such a bookmark, the import will fail because git-remote-hg will not be able to create the corresponding reference. Warn the user about the invalid reference, and continue the import, instead of stopping right away. Signed-off-by: Antoine Pelisse apeli...@gmail.com --- contrib/remote-helpers/git-remote-hg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index eb89ef6..12d850e 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -625,6 +625,9 @@ def list_head(repo, cur): def do_list(parser): repo = parser.repo for bmark, node in bookmarks.listbookmarks(repo).iteritems(): +if node == '': +warn(Ignoring invalid bookmark '%s', bmark) +continue bmarks[bmark] = repo[node] cur = repo.dirstate.branch() (Side note: ap/remote-hg-skip-null-bookmarks) When I run the test-suite like this: ~/projects/git/git.pu/contrib/remote-helpers$ debug=t verbose=t make test-hg-hg-git.sh All 11 test cases fail on my systems (Debian Wheezy and Mac OS X): [snip] WARNING: Ignoring invalid bookmark 'master' To hg::../hgrepo-git ! [remote rejected] master - master error: failed to push some refs to 'hg::../hgrepo-git' not ok 1 - executable bit # [snip] -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] Making use of bitmaps for thin objects
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 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
Howdy, Is there any policy on making static initializers thread-safe? I'm working on an experimental patch to introduce threading, but I'm running into a few non-thread-safe bits like this, in convert.c: static const char *conv_attr_name[] = { crlf, ident, filter, eol, text, }; #define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name) static void convert_attrs(struct conv_attrs *ca, const char *path) { int i; static struct git_attr_check ccheck[NUM_CONV_ATTRS]; if (!ccheck[0].attr) { for (i = 0; i NUM_CONV_ATTRS; i++) ccheck[i].attr = git_attr(conv_attr_name[i]); user_convert_tail = user_convert; git_config(read_convert_config, NULL); } } The easy fix would be to stick the initialization bit into an 'extern int init_convert_config();' function, and invoke it prior to the threaded part of my code. That would be no worse than the current state of affairs, which is to say that adding threading is rife with hidden perils. A more comprehensive fix might be: #include pthread.h static pthread_mutex_t convert_config_mutex = PTHREAD_MUTEX_INITIALIZER; static void convert_attrs(struct conv_attrs *ca, const char *path) { int i; static struct git_attr_check ccheck[NUM_CONV_ATTRS]; pthread_mutex_lock(convert_config_mutex); if (!ccheck[0].attr) { for (i = 0; i NUM_CONV_ATTRS; i++) ccheck[i].attr = git_attr(conv_attr_name[i]); user_convert_tail = user_convert; git_config(read_convert_config, NULL); } pthread_mutex_unlock(convert_config_mutex); } Unfortunately, I don't think mingw/msys supports PTHREAD_MUTEX_INITIALIZER. A possible workaround would be: static pthread_mutex_t convert_config_mutex; static void init_convert_config_mutex() __attribute__((constructor)); static void init_convert_config_mutex() { pthread_mutex_init(convert_config_mutex); } But then, I'm not whether mingw/msys supports __attribute__(constructor). Can anyone give me some guidance before I go to much further into the weeds (and I'm neck-deep as it is)? Thanks, Stefan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
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
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
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: I meant a single branch as opposed to depending on what branch you are sending out, you may have to use a different upstream starting point, and a single format.defaultTo that does not read what your HEAD currently points at may not be enough. Unless you set @{u} to this new configuration, in which case the choice becomes dynamic depending on the current branch, but - if that is the only sane choice based on the current branch, why not use that as the default without having to set the configuration? - Or if that is still insufficient, don't we need branch.*.forkedFrom that is different from branch.*.merge, so that different branches you want to show format-patch output can have different reference points? Ah, I was going for an equivalent of merge.defaultToUpstream, but I guess branch.*.forkedFrom is a good way to go. As I said in the different subthread, I am not convinced that you would need the complexity of branch.*.forkedFrom. If you set your upstream to the true upstream (not your publishing point), and have remote.pushdefault set to 'publish', you can expect git push to do the right thing, and then always say git show-branch publish/topic topic to see where your last published branch is relative to what you have, no? Mapping topic@{publish} to refs/remotes/publish/topic (or when you have 'topic' checked out, mapping @{publish} to it) is a trivial but is a separate usefulness, I would guess. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Jan 2014, #01; Mon, 6)
Welcome to the first issue of What's cooking report for the new year. Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * fc/remote-helper-fixes (2013-12-26) 5 commits (merged to 'next' on 2013-12-26 at ce5f872) + remote-hg: test 'shared_path' in a moved clone (merged to 'next' on 2013-12-17 at aa4dc07) + remote-hg: add tests for special filenames + remote-hg: fix 'shared path' path + remote-helpers: add extra safety checks + remote-hg: avoid buggy strftime() * jc/push-refmap (2013-12-04) 3 commits (merged to 'next' on 2013-12-12 at 71e358f) + push: also use upstream mapping when pushing a single ref + push: use remote.$name.push as a refmap + builtin/push.c: use strbuf instead of manual allocation Make git push origin master update the same ref that would be updated by our 'master' when git push origin (no refspecs) is run while the 'master' branch is checked out, which makes git push more symmetric to git fetch and more usable for the triangular workflow. * jk/cat-file-regression-fix (2013-12-12) 2 commits (merged to 'next' on 2013-12-13 at 3713e01) + cat-file: handle --batch format with missing type/size + cat-file: pass expand_data to print_object_or_die git cat-file --batch=, an admittedly useless command, did not behave very well. * jk/name-pack-after-byte-representation (2013-12-16) 3 commits (merged to 'next' on 2013-12-17 at 0bc385c) + pack-objects doc: treat output filename as opaque (merged to 'next' on 2013-12-09 at 247b2d0) + pack-objects: name pack files after trailer hash + sha1write: make buffer const-correct (this branch is tangled with jk/pack-bitmap.) Two packfiles that contain the same set of objects have traditionally been named identically, but that made repacking a repository that is already fully packed without any cruft with a different packing parameter cumbersome. Update the convention to name the packfile after the bytestream representation of the data, not after the set of objects in it. * jk/pull-rebase-using-fork-point (2013-12-10) 2 commits (merged to 'next' on 2013-12-13 at 1862c12) + rebase: use reflog to find common base with upstream + pull: use merge-base --fork-point when appropriate * jk/rev-parse-double-dashes (2013-12-09) 2 commits (merged to 'next' on 2013-12-13 at d26bac7) + rev-parse: be more careful with munging arguments + rev-parse: correctly diagnose revision errors before -- git rev-parse revs -- paths did not implement the usual disambiguation rules the commands in the git log family used in the same way. * js/gnome-keyring (2013-12-16) 1 commit (merged to 'next' on 2013-12-17 at 422fd61) + contrib/git-credential-gnome-keyring.c: small stylistic cleanups Style fix. * tg/diff-no-index-refactor (2013-12-16) 4 commits (merged to 'next' on 2013-12-17 at 009d8d8) + diff: avoid some nesting + diff: add test for --no-index executed outside repo (merged to 'next' on 2013-12-13 at 523f7c4) + diff: don't read index when --no-index is given + diff: move no-index detection to builtin/diff.c git diff ../else/where/A ../else/where/B when ../else/where is clearly outside the repository, and git diff --no-index A B, do not have to look at the index at all, but we used to read the index unconditionally. * zk/difftool-counts (2013-12-16) 2 commits (merged to 'next' on 2013-12-16 at 0e0d235) + diff.c: fix some recent whitespace style violations (merged to 'next' on 2013-12-12 at ba35694) + difftool: display the number of files in the diff queue in the prompt Show the total number of paths and the number of paths shown so far when git difftool prompts to launch an external diff tool, which would give users some sense of progress. -- [New Topics] * ta/format-user-manual-as-an-article (2014-01-06) 1 commit (merged to 'next' on 2014-01-06 at 37858f6) + user-manual: improve html and pdf formatting Update the way the user-manual is formatted via AsciiDoc to save trees. Will merge to 'master'. * bm/merge-base-octopus-dedup (2013-12-30) 2 commits (merged to 'next' on 2014-01-06 at 355d62b) + merge-base --octopus: reduce the result from get_octopus_merge_bases() + merge-base: separate --independent codepath into its own helper git merge-base --octopus used to leave cleaning up suboptimal result to the caller, but now it does the clean-up itself. Will merge to 'master'. * jk/test-framework-updates (2014-01-02) 3 commits (merged to 'next' on 2014-01-06 at f81f373) + t: drop known breakage test + t: simplify HARNESS_ACTIVE hack + t: set TEST_OUTPUT_DIRECTORY for
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Junio C Hamano wrote:. As I said in the different subthread, I am not convinced that you would need the complexity of branch.*.forkedFrom. If you set your upstream to the true upstream (not your publishing point), and have remote.pushdefaultset to 'publish', you can expect git push to do the right thing, and then always say git show-branch publish/topic topic I think it's highly sub-optimal to have a local-branch @{u} for several reasons; the prompt is almost useless in this case, and it will always show your forked-branch ahead of 'master' (assuming that 'master' doesn't update itself in the duration of your development). While doing respins, the prompt doesn't aid you in any way. Besides, on several occasions, I found myself working on the same forked-branch from two different machines; then the publish-point isn't necessarily always a publish-point: it's just another upstream for the branch. The point of a special branch.*.forkedFrom is that you can always show the master..@ information in the prompt ignoring divergences; after all, a divergence simply means that you need to rebase onto the latest 'master' ('master' is never going to get a non-ff update anyway). So, instead of crippling the functionality around the publish-point, let's build minimal required functionality around branch.*.forkedFrom. I'd love a prompt like: branch-forkedfrom5*:~/src/git$ Clearly, branch-forkedfrom has diverged from my publish-point (I'm probably doing a respin), and has 5 commits (it's 5 commits ahead of 'master' ignoring divergences). to see where your last published branch is relative to what you have, no? Mapping topic@{publish} to refs/remotes/publish/topic (or when you have 'topic' checked out, mapping @{publish} to it) is a trivial but is a separate usefulness, I would guess. I think a @{publish} is needed for when branch.*.remote is different from remote.pushDefault. Like I said earlier, it's probably too much information to give to the user: divergences with respect to two remotes. So, we'll hold it off until someone finds a usecase for it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
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
On Mon, Jan 06, 2014 at 12:17:21PM -0800, Junio C Hamano wrote: I am fine with rejecting it with a warning, but we should not then complain that the other side did not send us the object, since we should not be asking for it at all. I also do not see us complaining about the funny ref anywhere. So there is definitely _a_ bug here. :) Oh, no question about that. I was just pointing somebody who already has volunteered to take a look in a direction I recall was where the issue was ;-) Oh, crud, did I volunteer? :) So I found the problem, but I'm really not sure what to make of it. We do check the refname format when evaluating the refspecs, in: do_fetch get_ref_map get_fetch_map check_refname_format Before calling it, we check that it starts with refs/, and then pass the _whole_ refname into check_refname_format. So the latter sees refs/stash. And that's acceptable, as it's not a single-level ref. Thus we do not get the funny ref message. The code looks like this: if (!starts_with((*rmp)-peer_ref-name, refs/) || check_refname_format((*rmp)-peer_ref-name, 0)) { /* print funny ref and ignore */ Then we ask fetch_refs_via_pack to get the actual objects for us. And it checks our refs again, with this call chain: do_fetch fetch_refs transport_fetch_refs fetch_refs_via_pack fetch_pack do_fetch_pack everything_local filter_refs check_refname_format Here, the code looks like this: if (!memcmp(ref-name, refs/, 5) check_refname_format(ref-name + 5, 0)) ; /* trash */ At first I thought we are doing the same check (is it syntactically valid, and is it in refs/), but we're not. We are actually checking the format _only_ of stuff in refs/, and ignoring the rest. Which really makes no sense to me. If it were memcmp(...) || check_refname_format(), then it would be roughly the same check. But it would still be wrong, because note that we pass only the bits under refs/ to check_refname_format. So it sees only stash, and then complains that it is single-level. So the symptom we are seeing is because we are filtering with two different rulesets in different places. But I'm really not sure how to resolve it. The one in filter_refs seems nonsensical to me. Checking _only_ things under refs/ doesn't make sense. And even if that was sensible, feeding half a ref to check_refname_format does not work. In addition to the single-level check, it has other rules that want to see the whole ref (e.g., the ref @ is not valid, but refs/@ is OK; it cannot distinguish them without seeing the prefix). So I can see two options: 1. Make the check in filter_refs look like the one in get_fetch_map. That at least makes them the same, which alleviates the symptom. But we still are running two checks, and if they ever get out of sync, it causes problems. 2. Abolish the check in filter_refs. I think this makes the most sense from the perspective of fetch, because we will already have cleaned up the ref list. But we might need to leave the filtering in place for people who call fetch-pack as a bare plumbing command. It's really not clear to me what the check in filter_refs was trying to do. It dates all the way back to 1baaae5 (Make maximal use of the remote refs, 2005-10-28), but there is not much explanation. I haven't dug into the list around that time to see if there's any discussion. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/6 Heiko Voigt hvo...@hvoigt.net: I agree. If we were to support this more easily we could add a configuration option so you can omit the --remote (i.e.: submodule.name.remote=true, as I also suggested in the other email). That way the developer checking out a branch in flight does not even need to know whether (and which) submodules sha1s are still in flight and temporarily set this configuration in the branches .gitmodules file. submodule.name.remote can be useful but can be added later to aid the current use case. To not break the existing behavior what it's really needed here, IMO, is a submodule.name.attached property that says two things: - at the first clone on git submodule update stay attached to submodule.name.branch; - implies --remote, as it's the only thing that makes sense when the submodules are attached. My patch at the current unreleased state does exactly this. Maybe that could actually be the attach operation Francesco is suggesting: git submodule attach [--pull] submodule path branchname will attach the specified submodule to a branch. That means it changes the .gitmodule file accordingly and stages it. With the --pull switch one can specify whether a local branch tracking the remote branch should be automatically created. Names and the command format are just a suggestion here. That way we can support the fork superproject needing submodule changes and send submodule changes upstream first. My patch didn't do this, as the maintainer can do these things quite easily[1] (maintainer is cooler with respect to other devs :) ), but I think it could be good to also have this feature. The feature I think that are still needed and you don't mention are: - an --attached switch for the add command when the maintainer create the submodule the first time (DONE in patch); - a easy way to attach|detach the submodule locally by developer. This should: * fix the head state (DONE in patch); * fix the local .git/config submodule.name.attached property accordingly (DONE in patch, unreleased). I do the latest in the update command but it seems bad to touch .git/config in the update command... Maybe we should have a git submodule head command that does all these things: --attach (for the maintainer), --attach|--detach (for the developer). [1] $ ( cd submodule git branch newbranch git push -u origin HEAD) $ git config -f .gitmodules submodule.newbranch.branch newbranch $ git config -f .gitmodules submodule.newbranch.attached true $ git add . git commit -m Forked superproject git push -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)
2014/1/6 Junio C Hamano gits...@pobox.com: - git-submodule.sh: support 'checkout' as a valid update mode Need to pick up a rerolled one. I resent it, can you see it? Thank you, Francesco -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report: stash in upstream caused remote fetch to fail
Jeff King p...@peff.net writes: Then we ask fetch_refs_via_pack to get the actual objects for us. And it checks our refs again, with this call chain: do_fetch fetch_refs transport_fetch_refs fetch_refs_via_pack fetch_pack do_fetch_pack everything_local filter_refs check_refname_format Here, the code looks like this: if (!memcmp(ref-name, refs/, 5) check_refname_format(ref-name + 5, 0)) ; /* trash */ I think this should feed ref-name, not ref-name + 5; an obvious alternative would be to use REFNAME_ALLOW_ONELEVEL; you are also right that is probably a misspelt ||; this is about protecting ourselves from creating garbage in our ref namespace, so accepting anything outside refs/ makes little sense. It's really not clear to me what the check in filter_refs was trying to do. It dates all the way back to 1baaae5 (Make maximal use of the remote refs, 2005-10-28), but there is not much explanation. I haven't dug into the list around that time to see if there's any discussion. I think the funny refs the log message talks about is about filtering we know we can reach these objects via our alternates, but these are not refs we actually have. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)
Francesco Pretto ceztk...@gmail.com writes: 2014/1/6 Junio C Hamano gits...@pobox.com: - git-submodule.sh: support 'checkout' as a valid update mode Need to pick up a rerolled one. I resent it, can you see it? I know. I saw it and that is why I left the note to self. The thing is, it takes a non trivial amount of time to run through a single day's integration cycle, and any reroll that comes later in a day once the cycle started may be too late for that day. Otherwise I have to discard the the result of earlier merges and tests and start over from scratch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 Francesco Pretto cez...@gmail.com: To not break the existing behavior what it's really needed here, IMO, is a submodule.name.attached property that says two things: - at the first clone on git submodule update stay attached to submodule.name.branch; - implies --remote, as it's the only thing that makes sense when the submodules are attached. Unless you decide to go with the proposed approach of Trevor, where submodule.name.branch set means attached (if it's not changed: this thread is quite hard to follow...). To this end, Junio could sync with more long-timers (Heiko?) submodule users/devs to understand if this breaks too much or not. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mv: better document side effects when moving a submodule
Jens Lehmann jens.lehm...@web.de writes: The Submodules section of the git mv documentation mentions what will happen when a submodule with a gitfile gets moved with newer git. But it doesn't talk about what happens when the user changes between commits before and after the move, which does not update the work tree like using the mv command did the first time. Explain what happens and what the user has to do manually to fix that. Also document this in a new test. Reported-by: George Papanikolaou g3orge@gmail.com Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Am 09.12.2013 18:49, schrieb Jens Lehmann: Am 09.12.2013 11:59, schrieb George Papanikolaou: Also after mv you need to run 'submodule update' and I think this should be documented somewhere. You're right, this should be mentioned in the mv man page. I'll prepare a patch for that. Does this new paragraph make it clearer? Don't we have bugs section that we can use to list the known limitations like this? Documentation/git-mv.txt | 10 ++ t/t7001-mv.sh| 21 + It also may make sense to express the test as this is what we would like to see happen eventually in the form of test_expect_failure; it is not a big deal though. 2 files changed, 31 insertions(+) diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt index b1f7988..c9e8568 100644 --- a/Documentation/git-mv.txt +++ b/Documentation/git-mv.txt @@ -52,6 +52,16 @@ core.worktree setting to make the submodule work in the new location. It also will attempt to update the submodule.name.path setting in the linkgit:gitmodules[5] file and stage that file (unless -n is used). +Please note that each time a superproject update moves a populated +submodule (e.g. when switching between commits before and after the +move) a stale submodule checkout will remain in the old location +and an empty directory will appear in the new location. To populate +the submodule again in the new location the user will have to run +git submodule update afterwards. Removing the old directory is +only safe when it uses a gitfile, as otherwise the history of the +submodule will be deleted too. Both steps will be obsolete when +recursive submodule update has been implemented. + GIT --- Part of the linkgit:git[1] suite diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 3bfdfed..e3c8c2c 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -442,4 +442,25 @@ test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' ' git diff-files --quiet -- sub .gitmodules ' +test_expect_success 'checking out a commit before submodule moved needs manual updates' ' + git mv sub sub2 + git commit -m moved sub to sub2 + git checkout -q HEAD^ 2actual + echo warning: unable to rmdir sub2: Directory not empty expected + test_i18ncmp expected actual + git status -s sub2 actual + echo ?? sub2/ expected + test_cmp expected actual + ! test -f sub/.git + test -f sub2/.git + git submodule update + test -f sub/.git + rm -rf sub2 + git diff-index --exit-code HEAD + git update-index --refresh + git diff-files --quiet -- sub .gitmodules + git status -s sub2 actual + ! test -s actual +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)
2014/1/7 Junio C Hamano gits...@pobox.com: The thing is, it takes a non trivial amount of time to run through a single day's integration cycle, and any reroll that comes later in a day once the cycle started may be too late for that day. Otherwise I have to discard the the result of earlier merges and tests and start over from scratch. Got it, thank you. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-submodule.sh: Support 'checkout' as a valid update command
Francesco Pretto cez...@gmail.com writes: According to Documentation/gitmodules.txt, 'checkout' is a valid 'submodule.name.update' command. As you can see in the surrounding text, we call the value of submodule.*.update a mode, not a command. Also git-submodule.sh refers to it and processes it correctly. This present tense puzzles me. If it already refers to checkout and handles it correctly is there anything that needs to be done? Or did you mean it should refer to and process it but it doesn't, so make it so? Reflecting commit 'ac1fbb' to support this syntax and also validate property values during 'update' command, issuing an error if the value found is unknown. Sorry, but -ECANNOTPARSE. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-submodule.sh: Support 'checkout' as a valid update command
2014/1/7 Junio C Hamano gits...@pobox.com: Francesco Pretto cez...@gmail.com writes: According to Documentation/gitmodules.txt, 'checkout' is a valid 'submodule.name.update' command. As you can see in the surrounding text, we call the value of submodule.*.update a mode, not a command. Ok. Also git-submodule.sh refers to it and processes it correctly. This present tense puzzles me. If it already refers to checkout and handles it correctly is there anything that needs to be done? Or did you mean it should refer to and process it but it doesn't, so make it so? Like you said, it already refers to checkout and handles it correctly. I think the use of the simple present tense here is correct: it's a fact. Feel free to advice another wording if you prefer. Reflecting commit 'ac1fbb' to support this syntax and also validate property values during 'update' command, issuing an error if the value found is unknown. Sorry, but -ECANNOTPARSE. Not sure what's wrong here, can you explain why it's failing? I'm using git-format-patch/git-send-email with default settings. Also, if you can edit and keep the sign-off (I'm not familiar with the mailing-list maintainer workflow, sorry), feel free to do it. Thanks -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Fwd: static initializers
On Mon, Jan 6, 2014 at 11:05 PM, Stefan Zager sza...@google.com wrote: Forwarding to msysgit for any guidance about win equivalents for PTHREAD_MUTEX_INITIALIZER or __attribute__((constructor)). As you've probably already guessed, PTHREAD_MUTEX_INITIALIZER isn't supported in our pthreads-emulator. I did send out a patch adding support for it a while ago, but it hasn't been heavily tested. __attribute__((constructor)) doesn't work on MSVC, which we also build with. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
On Mon, Jan 6, 2014 at 5:54 PM, Ramkumar Ramachandra artag...@gmail.com wrote: John Szakmeister wrote: Then where does it get pushed? Do you always specify where to save your work? FWIW, I think the idea of treating @{u} as the eventual recipient of your changes is good, but then it seems like Git is lacking the publish my changes to this other branch concept. Am I missing something? If there is something other than @{u} to represent this latter concept, I think `git push` should default to that instead. But, at least with my current knowledge, that doesn't exist--without explicitly saying so--or treating @{u} as that branch. If there's a better way to do this, I'd love to hear it! That's why we invented remote.pushdefault and branch.*.pushremote. When you say $ git push it automatically goes to the right remote instead of going to the place you fetched from. You can read up on how push.default interacts with this setting too, although I always recommend push.default = current. This was the piece that I was missing. I remember when remote.pushdefault was added, but questioned its usefulness because it just changes the remote that a branch is pushed to, not necessarily the name. I somehow missed the 'current' option for 'push.default'... which is surprising since I seem to spend an incredible amount of time looking at the git-config man page. I guess it's not a good idea to set 'push.default' to 'upstream' in this triangle workflow then, otherwise the branch name being pushed to will be 'branch.*.merge'. Is that correct? I just want to make sure I understand what's happening here. Given this new found knowledge, I'm not sure it makes sense for `git status` to show me the status against the upstream vs. the publish location. The latter makes a little more sense to me, though I see the usefulness of either one. Thanks for taking the time to explain this. I'm going to have to spend some more time with this configuration and see what the sticking points are. -John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/6 Junio C Hamano gits...@pobox.com: As long as origin/master contains the commit specified by the superproject, that would work, but it may be a good thing to use a mode of submodule.*.update that does not have to drop the user into a detached state in the first place. I somehow thought that was what rebase (or merge) was about, that is, starting from the state where a branch is checked out in the submodule working tree, an update in the superproject would cause that branch checked out in the submodule brought up to date with respect to the commit specified in the superproject tree. If that is not how it is supposed to work, please correct me---and we may have to add another mode that does so (or even make rebase/merge do so as a bugfix). I think the mode you are referring to is actually my submodule.name.attached property. As I said to Heiko, the submodule.name.attached property says two things: - at the first clone on git submodule update stay attached to submodule.name.branch; - implies --remote, as it's the only thing that makes sense when the submodules are attached. And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? I think the reattach|detach option are still needed (it is debatable if we should have something like git submodule head command or we can keep them in git submodule update) because the user may want to do so and doing it requires things should be really atomic: * fix the head state; * set the local .git/config submodule.name.attached property. The --attach switch also can add a great bonus: it can reintegrate orphaned commits when reattaching and having a update mode with merge or rebase. This is already in my patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-submodule.sh: Support 'checkout' as a valid update command
2014/1/7 Junio C Hamano gits...@pobox.com: Sorry, but -ECANNOTPARSE. A bird told me what -ECANNOTPARSE means. Tell me if this comment sounds better: According to Documentation/gitmodules.txt, 'checkout' is a valid 'submodule.name.update' mode. Also git-submodule.sh already refers to it and handles it correctly. Fix cmd_init() to also accept 'checkout' as valid update mode and add a similar validation in cmd_update(), issuing an error if the value read is unknown. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pager: set LV=-c alongside LESS=FRSX
On systems with lv configured as the preferred pager (i.e., DEFAULT_PAGER=lv at build time, or PAGER=lv exported in the environment) git commands that use color show control codes instead of color in the pager: $ git diff ^[[1mdiff --git a/.mailfilter b/.mailfilter^[[m ^[[1mindex aa4f0b2..17e113e 100644^[[m ^[[1m--- a/.mailfilter^[[m ^[[1m+++ b/.mailfilter^[[m ^[[36m@@ -1,11 +1,58 @@^[[m less avoids this problem because git uses the LESS environment variable to pass the -R option ('output ANSI color escapes in raw form') by default. Use the LV environment variable to pass 'lv' the -c option ('allow ANSI escape sequences for text decoration / color') to fix it for lv, too. Noticed when the default value for color.ui flipped to 'auto' in v1.8.4-rc0~36^2~1 (2013-06-10). Reported-by: Olaf Meeuwissen olaf.meeuwis...@avasys.jp Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Olaf Meeuwissen wrote[1]: Yes, it's called LV and documented in the lv(1) manual page. Simply search for 'env' ;-) Ah, thanks. How about this patch? [1] http://bugs.debian.org/730527 Documentation/config.txt | 4 git-sh-setup.sh | 3 ++- pager.c | 11 +-- perl/Git/SVN/Log.pm | 1 + t/t7006-pager.sh | 12 5 files changed, 28 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index a405806..ed59853 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -567,6 +567,10 @@ be passed to the shell by Git, which will translate the final command to `LESS=FRSX less -+S`. The environment tells the command to set the `S` option to chop long lines but the command line resets it to the default to fold long lines. ++ +Likewise, when the `LV` environment variable is unset, Git sets it +to `-c`. You can override this setting by exporting `LV` with +another value or setting `core.pager` to `lv +c`. core.whitespace:: A comma separated list of common whitespace problems to diff --git a/git-sh-setup.sh b/git-sh-setup.sh index 190a539..fffa3c7 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -159,7 +159,8 @@ git_pager() { GIT_PAGER=cat fi : ${LESS=-FRSX} - export LESS + : ${LV=-c} + export LESS LV eval $GIT_PAGER '$@' } diff --git a/pager.c b/pager.c index 345b0bc..0cc75a8 100644 --- a/pager.c +++ b/pager.c @@ -80,8 +80,15 @@ void setup_pager(void) pager_process.use_shell = 1; pager_process.argv = pager_argv; pager_process.in = -1; - if (!getenv(LESS)) { - static const char *env[] = { LESS=FRSX, NULL }; + if (!getenv(LESS) || !getenv(LV)) { + static const char *env[3]; + int i = 0; + + if (!getenv(LESS)) + env[i++] = LESS=FRSX; + if (!getenv(LV)) + env[i++] = LV=-c; + env[i] = NULL; pager_process.env = env; } if (start_command(pager_process)) diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm index 3f8350a..34f2869 100644 --- a/perl/Git/SVN/Log.pm +++ b/perl/Git/SVN/Log.pm @@ -117,6 +117,7 @@ sub run_pager { } open STDIN, '', $rfd or fatal Can't redirect stdin: $!; $ENV{LESS} ||= 'FRSX'; + $ENV{LV} ||= '-c'; exec $pager or fatal Can't run pager: $! ($pager); } diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index ff25908..7fe3367 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -37,6 +37,18 @@ test_expect_failure TTY 'pager runs from subdir' ' test_cmp expected actual ' +test_expect_success TTY 'LESS and LV envvars are set for pagination' ' + ( + sane_unset LESS LV + PAGER=env pager-env.out + export PAGER + + test_terminal git log + ) + grep ^LESS= pager-env.out + grep ^LV= pager-env.out +' + test_expect_success TTY 'some commands do not use a pager' ' rm -f paginated.out test_terminal git rev-list HEAD -- 1.8.5.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report: stash in upstream caused remote fetch to fail
Junio C Hamano gits...@pobox.com writes: It's really not clear to me what the check in filter_refs was trying to do. It dates all the way back to 1baaae5 (Make maximal use of the remote refs, 2005-10-28), but there is not much explanation. I haven't dug into the list around that time to see if there's any discussion. I think the funny refs the log message talks about is about filtering we know we can reach these objects via our alternates, but these are not refs we actually have. Actually, I think the above recollection of mine was completely bogus. The is there because we do allow things like HEAD (they are the funny ones) as well as things inside refs/, and the latter is the only thing we had a check-ref-format to dictate the format when the code was written. I do not mind tightening things a bit (e.g. outside refs/, only allow HEAD and nothing else). A good first step might be to enforce allow-onelevel outside refs/ (so that we can allow HEAD) and for those inside refs/ check without allow-onelevel but without skipping the prefix. It is a separate story if it makes much sense to allow fetching refs/stash or ignoring when running git clone. Operationally, I think it makes more sense to ignore refs/stash, not because it is a one-level name, but because what a stash is. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] sha1_name: don't resolve refs when core.warnambiguousrefs is false
This change ensures get_sha1_basic() doesn't try to resolve full hashes as refs when ambiguous ref warnings are disabled. This provides a substantial performance improvement when passing many hashes to a command (like git rev-list --stdin) when core.warnambiguousrefs is false. The check incurs 6 stat()s for every hash supplied, which can be costly over NFS. --- sha1_name.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index e9c2999..10bd007 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -451,9 +451,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) int at, reflog_len, nth_prior = 0; if (len == 40 !get_sha1_hex(str, sha1)) { - if (warn_on_object_refname_ambiguity) { + if (warn_ambiguous_refs warn_on_object_refname_ambiguity) { refs_found = dwim_ref(str, len, tmp_sha1, real_ref); - if (refs_found 0 warn_ambiguous_refs) { + if (refs_found 0) { warning(warn_msg, len, str); if (advice_object_name_warning) fprintf(stderr, %s\n, _(object_name_msg)); -- 1.8.3.4 (Apple Git-47) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sha1_name: don't resolve refs when core.warnambiguousrefs is false
On Mon, Jan 6, 2014 at 7:32 PM, Brodie Rao bro...@sf.io wrote: This change ensures get_sha1_basic() doesn't try to resolve full hashes as refs when ambiguous ref warnings are disabled. This provides a substantial performance improvement when passing many hashes to a command (like git rev-list --stdin) when core.warnambiguousrefs is false. The check incurs 6 stat()s for every hash supplied, which can be costly over NFS. Forgot to add: Signed-off-by: Brodie Rao bro...@sf.io --- sha1_name.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index e9c2999..10bd007 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -451,9 +451,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) int at, reflog_len, nth_prior = 0; if (len == 40 !get_sha1_hex(str, sha1)) { - if (warn_on_object_refname_ambiguity) { + if (warn_ambiguous_refs warn_on_object_refname_ambiguity) { refs_found = dwim_ref(str, len, tmp_sha1, real_ref); - if (refs_found 0 warn_ambiguous_refs) { + if (refs_found 0) { warning(warn_msg, len, str); if (advice_object_name_warning) fprintf(stderr, %s\n, _(object_name_msg)); -- 1.8.3.4 (Apple Git-47) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html