Re: What's cooking in git.git (Jul 2014, #04; Tue, 22)
From: Junio C Hamano gits...@pobox.com -- [Stalled] * po/everyday-doc (2014-01-27) 1 commit - Make 'git help everyday' work This may make the said command to emit something, but the source is not meant to be formatted into a manual pages to begin with, and also its contents are a bit stale. It may be a good first step in the right direction, but needs more work to at least get the mark-up right before public consumption. Will hold. It's not clear to me which bits of mark-up are 'wrong' and must be reworked, and which markup styles are just unusal and could wait until better eyes can review the detailed content. At the moment I'm guessing as to what changes to do, and falling down various Asciidoc rabbit holes. I'd judged my initial changes as 'adequate' rather than 'good' because of that issue. Philip -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: confused about remote branch management
On 23/07/14 14:49, Ross Boylan wrote: My local master branch is the result of a merge of upstream master and some local changes. I want to merge in more recent upstream work. git pull doesn't seem to have updated origin/master, and git checkout origin/master also doesn't seem to work. Here's some info that may be relevant. ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git remote -v originhttps://github.com/emacs-ess/ESS.git (fetch) originhttps://github.com/emacs-ess/ESS.git (push) personal https://github.com/RossBoylan/ESS.git (fetch) personal https://github.com/RossBoylan/ESS.git (push) # I think I originally cloned from what is now the personal remote # and switched names later so origin refers to upstream. ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git branch -v * master 8fa569c [ahead 340] merge from origin # 340 ahead of personal is plausible, but ahead from origin seems odd ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git --version git version 1.7.10.4 ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git branch -a * master remotes/origin/S+eldoc remotes/origin/gretl remotes/origin/linewise_callbacks remotes/origin/litprog remotes/origin/master remotes/origin/transmissions remotes/personal/HEAD - personal/master remotes/personal/S+eldoc remotes/personal/gretl remotes/personal/linewise_callbacks remotes/personal/litprog remotes/personal/master remotes/personal/transmissions ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git status # On branch master # Your branch is ahead of 'personal/master' by 340 commits. # nothing to commit (working directory clean) ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git checkout origin/master Note: checking out 'origin/master'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name HEAD is now at a33a2f9... Merge branch 'trunk' ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git checkout master Previous HEAD position was a33a2f9... Merge branch 'trunk' Switched to branch 'master' ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git pull origin master # [messages] Not committing merge; use 'git commit' to complete the merge. I think this is the relevant message. By doing a git pull you are asking to merge the branch 'master' from the remote 'origin' into the current branch (which happens to also be called 'master'). What I'm guessing is in # [messages] is something about a merge conflict that needs resolving before the merge can be completed. There are various ways to resolve the conflict but probably the easiest would be git mergetool git commit I personally use have merge.tool set to kdiff3 but there are plenty of other options. Another option is to abort this attempt and try again (*warning* here be dragons). git checkout master git reset --hard HEAD git pull Note: git uses some heuristics to determine what to merge when you don't specify what to pull. This should be origin/master unless branch.master.remote and branch.master.merge are set to something weird. This probably won't do away with the need to resolve your merge conflicts but at least you'll be starting from a clean slate. ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git status # On branch master # Changes to be committed: # [long list] ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git show origin/master commit a33a2f9e06185a225af7d72ea3896cfd260e455e Merge: 136742f 6b22a88 Author: Vitalie Spinu spinu...@gmail.com Date: Mon Jan 20 00:43:30 2014 -0800 Merge branch 'trunk' # this was the head of origin/master BEFORE I did the pull. # I think this means it has not been updated. ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git commit -m merge in upstream, probably fe7d609..8fa569c [master 59f6841] merge in upstream, probably fe7d609..8fa569c ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git show origin/master -v # no change -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe 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 v7 23/31] checkout: clean up half-prepared directories in --to mode
On Mon, Jul 21, 2014 at 6:55 AM, Eric Sunshine sunsh...@sunshineco.com wrote: @@ -848,13 +878,21 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, strbuf_addf(sb_repo, %d, counter); } name = strrchr(sb_repo.buf, '/') + 1; + + junk_pid = getpid(); + atexit(remove_junk); + sigchain_push_common(remove_junk_on_signal); + if (mkdir(sb_repo.buf, 0777)) die_errno(_(could not create directory of '%s'), sb_repo.buf); + junk_git_dir = sb_repo.buf; I've managed to convince myself that, although junk_git_dir becomes a dangling pointer by the end of prepare_linked_checkout(), it should never afterward be accessed. Perhaps it would make sense to make this easier to follow by clearing junk_git_dir when is_junk is cleared? You're right. And the dangling junk_git_dir can be accessed if ret below is non-zero. Will strdup() both junk_*_dir and free them in if (!ret) block. Thanks for catching. @@ -879,7 +917,14 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, memset(cp, 0, sizeof(cp)); cp.git_cmd = 1; cp.argv = opts-saved_argv; - return run_command(cp); + ret = run_command(cp); + if (!ret) + is_junk = 0; Here: perhaps also set is_junk_dir to NULL since it otherwise is about to become a dangling pointer. + strbuf_release(sb); + strbuf_release(sb_repo); + strbuf_release(sb_git); + return ret; -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] config.c: change the function signature of `git_config_string()`
Junio C Hamano gits...@pobox.com writes: *1* We have safe_create_leading_directories_const() that works around this for input parameter around its _const less counterpart, which is ugly but livable solution. I think it would actually be a reasonable solution to avoid casting here and there on the caller side. Another option would be to _return_ a non-const char * instead of outputing it as a by-address parameter. We'd lose the consiseness of return git_config_get_string(...) (but in cases where the return value is ignored, we do not really care) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] nd/multiple-work-trees follow-ups
The series has entered 'next' so I can't replace patches any more. Besides the brown paper bag fixes, checkout now rejects if a branch is already checked out elsewhere. Nguyễn Thái Ngọc Duy (5): gitrepository-layout.txt: s/ignored/ignored if/ prune --repos: fix uninitialized access checkout --to: no auto-detach if the ref is already checked out checkout --to: fix dangling pointers in remove_junk() environment.c: fix incorrect git_graft_file initialization Documentation/config.txt | 3 ++ Documentation/gitrepository-layout.txt | 6 +-- advice.c | 2 + advice.h | 1 + builtin/checkout.c | 88 -- builtin/prune.c| 16 +++ environment.c | 2 +- t/t0060-path-utils.sh | 1 + t/t2025-checkout-to.sh | 40 t/t2026-prune-linked-checkouts.sh | 2 +- 10 files changed, 102 insertions(+), 59 deletions(-) -- 1.9.1.346.ga2b5940 -- To unsubscribe from this list: send the line 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/5] prune --repos: fix uninitialized access
There's a code path in prune_repo_dir() that does not initialize 'st' buffer, which is checked by the caller, prune_repos_dir(). Instead of leaking some prune logic out to prune_repos_dir(), move 'st' into prune_repo_dir(). Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/prune.c | 16 ++-- t/t2026-prune-linked-checkouts.sh | 2 +- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/builtin/prune.c b/builtin/prune.c index 28b7adf..e72c391 100644 --- a/builtin/prune.c +++ b/builtin/prune.c @@ -112,8 +112,9 @@ static void prune_object_dir(const char *path) } } -static int prune_repo_dir(const char *id, struct stat *st, struct strbuf *reason) +static int prune_repo_dir(const char *id, struct strbuf *reason) { + struct stat st; char *path; int fd, len; @@ -123,26 +124,23 @@ static int prune_repo_dir(const char *id, struct stat *st, struct strbuf *reason } if (file_exists(git_path(repos/%s/locked, id))) return 0; - if (stat(git_path(repos/%s/gitdir, id), st)) { - st-st_mtime = expire; + if (stat(git_path(repos/%s/gitdir, id), st)) { strbuf_addf(reason, _(Removing repos/%s: gitdir file does not exist), id); return 1; } fd = open(git_path(repos/%s/gitdir, id), O_RDONLY); if (fd 0) { - st-st_mtime = expire; strbuf_addf(reason, _(Removing repos/%s: unable to read gitdir file (%s)), id, strerror(errno)); return 1; } - len = st-st_size; + len = st.st_size; path = xmalloc(len + 1); read_in_full(fd, path, len); close(fd); while (len (path[len - 1] == '\n' || path[len - 1] == '\r')) len--; if (!len) { - st-st_mtime = expire; strbuf_addf(reason, _(Removing repos/%s: invalid gitdir file), id); free(path); return 1; @@ -162,7 +160,7 @@ static int prune_repo_dir(const char *id, struct stat *st, struct strbuf *reason return 1; } free(path); - return 0; + return st.st_mtime = expire; } static void prune_repos_dir(void) @@ -172,15 +170,13 @@ static void prune_repos_dir(void) DIR *dir = opendir(git_path(repos)); struct dirent *d; int ret; - struct stat st; if (!dir) return; while ((d = readdir(dir)) != NULL) { if (!strcmp(d-d_name, .) || !strcmp(d-d_name, ..)) continue; strbuf_reset(reason); - if (!prune_repo_dir(d-d_name, st, reason) || - st.st_mtime expire) + if (!prune_repo_dir(d-d_name, reason)) continue; if (show_only || verbose) printf(%s\n, reason.buf); diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh index 4ccfa4e..79d84cb 100755 --- a/t/t2026-prune-linked-checkouts.sh +++ b/t/t2026-prune-linked-checkouts.sh @@ -77,7 +77,7 @@ test_expect_success 'not prune recent checkouts' ' mkdir zz mkdir -p .git/repos/jlm echo $TRASH_DIRECTORY/zz .git/repos/jlm/gitdir - git prune --repos --verbose + git prune --repos --verbose --expire=2.days.ago test -d .git/repos/jlm ' -- 1.9.1.346.ga2b5940 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] gitrepository-layout.txt: s/ignored/ignored if/
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/gitrepository-layout.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt index bed4f1a..6bd82af 100644 --- a/Documentation/gitrepository-layout.txt +++ b/Documentation/gitrepository-layout.txt @@ -139,7 +139,7 @@ is often called 'detached HEAD.' See linkgit:git-checkout[1] for details. config:: - Repository specific configuration file. This file is ignored + Repository specific configuration file. This file is ignored if $GIT_COMMON_DIR is set and $GIT_COMMON_DIR/config will be used instead. @@ -220,7 +220,7 @@ remotes:: logs:: Records of changes made to refs are stored in this directory. See linkgit:git-update-ref[1] - for more information. This directory is ignored + for more information. This directory is ignored if $GIT_COMMON_DIR is set and $GIT_COMMON_DIR/logs will be used instead. @@ -252,7 +252,7 @@ modules:: repos:: Contains worktree specific information of linked checkouts. Each subdirectory contains the worktree-related - part of a linked checkout. This directory is ignored + part of a linked checkout. This directory is ignored if $GIT_COMMON_DIR is set and $GIT_COMMON_DIR/repos will be used instead. -- 1.9.1.346.ga2b5940 -- To unsubscribe from this list: send the line 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 3/5] checkout --to: no auto-detach if the ref is already checked out
Give the user a choice in this case. If they want to detach, they can go with '--detach --to ...', or they could switch branch of the checkout that's holding the ref in question. Or they could just create a new branch with '-b xxx --to yyy' Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/config.txt | 3 ++ advice.c | 2 ++ advice.h | 1 + builtin/checkout.c | 73 t/t2025-checkout-to.sh | 16 +-- 5 files changed, 56 insertions(+), 39 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 57999fa..4a41202 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -201,6 +201,9 @@ advice.*:: rmHints:: In case of failure in the output of linkgit:git-rm[1], show directions on how to proceed from the current state. + checkoutTo:: + In case of failure in the output of git checkout --to, + show directions on how to proceed from the current state. -- core.fileMode:: diff --git a/advice.c b/advice.c index 9b42033..b1fb524 100644 --- a/advice.c +++ b/advice.c @@ -15,6 +15,7 @@ int advice_detached_head = 1; int advice_set_upstream_failure = 1; int advice_object_name_warning = 1; int advice_rm_hints = 1; +int advice_checkout_to = 1; static struct { const char *name; @@ -35,6 +36,7 @@ static struct { { setupstreamfailure, advice_set_upstream_failure }, { objectnamewarning, advice_object_name_warning }, { rmhints, advice_rm_hints }, + { checkoutto, advice_checkout_to }, /* make this an alias for backward compatibility */ { pushnonfastforward, advice_push_update_rejected } diff --git a/advice.h b/advice.h index 5ecc6c1..a288219 100644 --- a/advice.h +++ b/advice.h @@ -18,6 +18,7 @@ extern int advice_detached_head; extern int advice_set_upstream_failure; extern int advice_object_name_warning; extern int advice_rm_hints; +extern int advice_checkout_to; int git_default_advice_config(const char *var, const char *value); __attribute__((format (printf, 1, 2))) diff --git a/builtin/checkout.c b/builtin/checkout.c index c83f476..d35245a 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1006,31 +1006,52 @@ static const char *unique_tracking_name(const char *name, unsigned char *sha1) return NULL; } -static int check_linked_checkout(struct branch_info *new, - const char *name, const char *path) +static void check_linked_checkout(struct branch_info *new, const char *id) { struct strbuf sb = STRBUF_INIT; + struct strbuf path = STRBUF_INIT; + struct strbuf gitdir = STRBUF_INIT; const char *start, *end; - if (strbuf_read_file(sb, path, 0) 0 || - !skip_prefix(sb.buf, ref:, start)) { - strbuf_release(sb); - return 0; - } + if (id) + strbuf_addf(path, %s/repos/%s/HEAD, get_git_common_dir(), id); + else + strbuf_addf(path, %s/HEAD, get_git_common_dir()); + + if (strbuf_read_file(sb, path.buf, 0) = 0 || + !skip_prefix(sb.buf, ref:, start)) + goto done; while (isspace(*start)) start++; end = start; while (*end !isspace(*end)) end++; - if (!strncmp(start, new-path, end - start) - new-path[end - start] == '\0') { - strbuf_release(sb); - new-path = NULL; /* detach */ - new-checkout = xstrdup(name); /* reason */ - return 1; - } + if (strncmp(start, new-path, end - start) || + new-path[end - start] != '\0') + goto done; + if (id) { + strbuf_reset(path); + strbuf_addf(path, %s/repos/%s/gitdir, + get_git_common_dir(), id); + if (strbuf_read_file(gitdir, path.buf, 0) = 0) + goto done; + while (gitdir.len (gitdir.buf[gitdir.len - 1] == '\n' || + gitdir.buf[gitdir.len - 1] == '\r')) + gitdir.buf[--gitdir.len] = '\0'; + } else + strbuf_addstr(gitdir, get_git_common_dir()); + if (advice_checkout_to) + die(_(%s is already checked out at %s.\n + Either use --detach or -b together with --to + or switch branch in the the other checkout.), + new-path, gitdir.buf); + else + die(_(%s is already checked out at %s.), + new-path, gitdir.buf); +done: + strbuf_release(path); strbuf_release(sb); - return 0; + strbuf_release(gitdir); } static void check_linked_checkouts(struct branch_info *new) @@ -1045,27 +1066,17 @@ static void check_linked_checkouts(struct
[PATCH 5/5] environment.c: fix incorrect git_graft_file initialization
info/grafts should be part of the common repository when accessed from a linked checkout (iow $GIT_COMMON_DIR/info/grafts, not $GIT_DIR/info/grafts). git_path(info/grafts) returns correctly, even without this fix, because it detects that $GIT_GRAFT_FILE is not set, so it goes with the common rule: anything except sparse-checkout in 'info' belongs to common repo. But get_graft_file() would return a wrong value and that one is used for setting grafts up. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- environment.c | 2 +- t/t0060-path-utils.sh | 1 + t/t2025-checkout-to.sh | 18 ++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/environment.c b/environment.c index 50ed40a..d5b0788 100644 --- a/environment.c +++ b/environment.c @@ -157,7 +157,7 @@ static void setup_git_env(void) objects, git_db_env); git_index_file = git_path_from_env(INDEX_ENVIRONMENT, git_dir, index, git_index_env); - git_graft_file = git_path_from_env(GRAFT_ENVIRONMENT, git_dir, + git_graft_file = git_path_from_env(GRAFT_ENVIRONMENT, git_common_dir, info/grafts, git_graft_env); if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT)) check_replace_refs = 0; diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index da82aab..93605f4 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -269,6 +269,7 @@ test_git_path GIT_COMMON_DIR=bar logs/HEAD .git/logs/HEAD test_git_path GIT_COMMON_DIR=bar objects bar/objects test_git_path GIT_COMMON_DIR=bar objects/bar bar/objects/bar test_git_path GIT_COMMON_DIR=bar info/exclude bar/info/exclude +test_git_path GIT_COMMON_DIR=bar info/grafts bar/info/grafts test_git_path GIT_COMMON_DIR=bar info/sparse-checkout .git/info/sparse-checkout test_git_path GIT_COMMON_DIR=bar remotes/bar bar/remotes/bar test_git_path GIT_COMMON_DIR=bar branches/bar bar/branches/bar diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh index 8a00310..508993f 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-checkout-to.sh @@ -81,4 +81,22 @@ test_expect_success 'checkout from a bare repo without --to' ' ) ' +test_expect_success 'checkout with grafts' ' + test_when_finished rm .git/info/grafts + test_commit abc + SHA1=`git rev-parse HEAD` + test_commit def + test_commit xyz + echo `git rev-parse HEAD` $SHA1 .git/info/grafts + cat expected -\EOF + xyz + abc + EOF + git log --format=%s -2 actual + test_cmp expected actual + git checkout --detach --to grafted master + git --git-dir=grafted/.git log --format=%s -2 actual + test_cmp expected actual +' + test_done -- 1.9.1.346.ga2b5940 -- To unsubscribe from this list: send the line 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 4/5] checkout --to: fix dangling pointers in remove_junk()
junk_git_dir is set to sb_repo.buf. By the end of prepare_linked_checkout(), sb_repo is freed and so junk_git_dir points to nowhere. If the second checkout command fails, is_junk remains non-zero, remove_junk() will be called and try to clean junk_git_dir, which could be anything now (if it does not crash the program). The new test may pass even without this patch. But it does fail under valgrind (without this patch) with Invalid read of size 8 at the right line. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/checkout.c | 15 ++- t/t2025-checkout-to.sh | 6 ++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index d35245a..e62c084 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -825,8 +825,8 @@ static int switch_branches(const struct checkout_opts *opts, return ret || writeout_error; } -static const char *junk_work_tree; -static const char *junk_git_dir; +static char *junk_work_tree; +static char *junk_git_dir; static int is_junk; static pid_t junk_pid; @@ -895,7 +895,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, if (mkdir(sb_repo.buf, 0777)) die_errno(_(could not create directory of '%s'), sb_repo.buf); - junk_git_dir = sb_repo.buf; + junk_git_dir = xstrdup(sb_repo.buf); is_junk = 1; /* @@ -909,7 +909,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, if (safe_create_leading_directories_const(sb_git.buf)) die_errno(_(could not create leading directories of '%s'), sb_git.buf); - junk_work_tree = path; + junk_work_tree = xstrdup(path); strbuf_reset(sb); strbuf_addf(sb, %s/gitdir, sb_repo.buf); @@ -939,8 +939,13 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, cp.git_cmd = 1; cp.argv = opts-saved_argv; ret = run_command(cp); - if (!ret) + if (!ret) { is_junk = 0; + free(junk_work_tree); + free(junk_git_dir); + junk_work_tree = NULL; + junk_git_dir = NULL; + } strbuf_reset(sb); strbuf_addf(sb, %s/locked, sb_repo.buf); unlink_or_warn(sb.buf); diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh index c6601a4..8a00310 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-checkout-to.sh @@ -12,6 +12,12 @@ test_expect_success 'checkout --to not updating paths' ' test_must_fail git checkout --to -- init.t ' +test_expect_success 'checkout --to refuses to checkout locked branch' ' + test_must_fail git checkout --to zere master + ! test -d zere + ! test -d .git/repos/zere +' + test_expect_success 'checkout --to a new worktree' ' git rev-parse HEAD expect git checkout --detach --to here master -- 1.9.1.346.ga2b5940 -- To unsubscribe from this list: send the line unsubscribe 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 in get_pwd_cwd() in Windows?
On Wed, Jul 23, 2014 at 2:35 AM, René Scharfe l@web.de wrote: Am 21.07.2014 16:13, schrieb Duy Nguyen: This function tests if $PWD is the same as getcwd() using st_dev and st_ino. But on Windows these fields are always zero (mingw.c:do_lstat). If cwd is moved away, I think falling back to $PWD is wrong. I don't understand the use of $PWD in the first place. 1b9a946 (Use nonrelative paths instead of absolute paths for cloned repositories - 2008-06-05) does not explain much. The commit message reads: Particularly for the alternates file, if one will be created, we want a path that doesn't depend on the current directory, but we want to retain any symlinks in the path as given and any in the user's view of the current directory when the path was given. The intent of the patch seems to be to prefer $PWD if it points to the same directory as the one returned by getcwd() in order to preserve the user's view. That's why it introduces make_nonrelative_path() (now called absolute_path()), in contrast to make_absolute_path() (now called real_path()). I imagine it's useful e.g. if your home is accessed through a symlink: /home/foo - /some/boring/mountpoint Then real_path(bar) would give you /some/boring/mountpoint/bar, while absolute_path(bar) returned /home/foo/bar. Not resolving symlinks keeps the path friendly in this case. And it keeps working even after the user's home is migrated to /a/bigger/partition and /home/foo is updated accordingly. If it's saved back, then yes it's useful. And I think that's the case in clone.c. I was tempted to remove this code (because it only works if you stand at worktree's root dir anyway, else cwd is moved) but I guess we can just disable this code on Windows only instead. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] Make locked paths absolute when current directory is changed
On Tue, Jul 22, 2014 at 12:04 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: On Mon, Jul 21, 2014 at 8:27 PM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: +void make_locked_paths_absolute(void) +{ + struct lock_file *lk; + for (lk = lock_file_list; lk != NULL; lk = lk-next) { + if (lk-filename !is_absolute_path(lk-filename)) { + char *to_free = lk-filename; + lk-filename = xstrdup(absolute_path(lk-filename)); + free(to_free); + } + } +} I just have to ask, why are we putting relative pathnames in this list to begin with? Why not use an absolute path when taking the lock in all cases? (calling absolute_path() and using the result to take the lock, storing it in the lock_file list, should not be in the critical path, right? Not that I have measured it, of course! :) Conservative :) I'm still scared from 044bbbc (Make git_dir a path relative to work_tree in setup_work_tree() - 2008-06-19). But yeah looking through grep hold_ I think none of the locks is in critical path. absolute_path() can die() if cwd is longer than PATH_MAX (and doing this reduces the chances of that happening). But René is adding strbuf_getcwd() that can remove that PATH_MAX. So I guess we should be fine with putting absolute_path() in hold_lock_file_...* OK, we should center these efforts around the strbuf_getcwd() topic, basing the other topic on realpath() and this one on it then? OK. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] .mailmap: Combine my emails
Google mail has had the extension @googlemail.com for a long time in Germany as @gmail.de was already taken by a competitor. Nowadays the original gmail company isn't there anymore(?), hence Googlemail also introduced @gmail.com in Germany, which I switched to. This changed mail address of mine first appeared in 398dd4bd039680b (2014-07-10, .mailmap: map different names with the same email address together) ironically. Signed-off-by: Stefan Beller stefanbel...@gmail.com --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index 2edbeb5..8aefb5a 100644 --- a/.mailmap +++ b/.mailmap @@ -204,6 +204,7 @@ Seth Falcon s...@userprimary.net sfal...@fhcrc.org Shawn O. Pearce spea...@spearce.org Simon Hausmann hausm...@kde.org si...@lst.de Simon Hausmann hausm...@kde.org shaus...@trolltech.com +Stefan Beller stefanbel...@gmail.com stefanbel...@googlemail.com Stefan Naewe stefan.na...@gmail.com stefan.na...@atlas-elektronik.com Stefan Naewe stefan.na...@gmail.com stefan.na...@googlemail.com Stefan Sperling s...@elego.de s...@stsp.name -- 2.0.2.608.g398dd4b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] General Manpage: Switch homepage for stats
According to http://meta.ohloh.net/2014/07/black-duck-open-hub/ the site name of ohloh changed to openhub. Change the man page accordingly. Signed-off-by: Stefan Beller stefanbel...@gmail.com --- Documentation/git.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index d0ddfcb..2dbc63c 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -1044,7 +1044,7 @@ Authors --- Git was started by Linus Torvalds, and is currently maintained by Junio C Hamano. Numerous contributions have come from the Git mailing list -git@vger.kernel.org. http://www.ohloh.net/p/git/contributors/summary +git@vger.kernel.org. http://www.openhub.net/p/git/contributors/summary gives you a more complete list of contributors. If you have a clone of git.git itself, the -- 2.0.2.608.g398dd4b -- To unsubscribe from this list: send the line unsubscribe 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 in get_pwd_cwd() in Windows?
Am 23.07.2014 13:53, schrieb Duy Nguyen: On Wed, Jul 23, 2014 at 2:35 AM, René Scharfe l@web.de wrote: Am 21.07.2014 16:13, schrieb Duy Nguyen: This function tests if $PWD is the same as getcwd() using st_dev and st_ino. But on Windows these fields are always zero (mingw.c:do_lstat). If cwd is moved away, I think falling back to $PWD is wrong. I don't understand the use of $PWD in the first place. 1b9a946 (Use nonrelative paths instead of absolute paths for cloned repositories - 2008-06-05) does not explain much. The commit message reads: Particularly for the alternates file, if one will be created, we want a path that doesn't depend on the current directory, but we want to retain any symlinks in the path as given and any in the user's view of the current directory when the path was given. The intent of the patch seems to be to prefer $PWD if it points to the same directory as the one returned by getcwd() in order to preserve the user's view. That's why it introduces make_nonrelative_path() (now called absolute_path()), in contrast to make_absolute_path() (now called real_path()). I imagine it's useful e.g. if your home is accessed through a symlink: /home/foo - /some/boring/mountpoint Then real_path(bar) would give you /some/boring/mountpoint/bar, while absolute_path(bar) returned /home/foo/bar. Not resolving symlinks keeps the path friendly in this case. And it keeps working even after the user's home is migrated to /a/bigger/partition and /home/foo is updated accordingly. If it's saved back, then yes it's useful. And I think that's the case in clone.c. I was tempted to remove this code (because it only works if you stand at worktree's root dir anyway, else cwd is moved) but I guess we can just disable this code on Windows only instead. It is disabled on Windows as of 7d092adc get_pwd_cwd(): Do not trust st_dev/st_ino blindly -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: confused about remote branch management
On Jul 23, 2014 5:11 AM, Ross Boylan r...@biostat.ucsf.edu wrote: My local master branch is the result of a merge of upstream master and some local changes. I want to merge in more recent upstream work. git pull doesn't seem to have updated origin/master, and git checkout origin/master also doesn't seem to work. git pull with two parameters in older versions will not update remote tracking branches. That's because the last parameter expects a refspec with a source and destination and you only specify a source. Doing a git fetch will update them. ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git --version git version 1.7.10.4 Version 1.8.4 changes this behavior and will update the remote tracking branches. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
rebase flattens history when it shouldn't?
Hello, $ git --version git version 1.9.3 Please consider the following history: --C-- / \ / M topic,HEAD / / A---B master shouldn't $ git rebase master be a no-op here? According to my reading of the rebase manual page, it should be a no-op, as 'topic' is a descendant of the 'master'. Instead, git rebase master flattens the history to: C topic,HEAD / A---B master I'd expect --force-rebase to be required for this to happen: -f, --force-rebase Force the rebase even if the current branch is a descendant of the commit you are rebasing onto. Normally non-interactive rebase will exit with the message Current branch is up to date in such a situation. Incompatible with the --interactive option. Also notice that: $ git rebase --preserve-merges --verbose master does perform the rebasing work, even though it does not change the history in the end. Here is use-case where it came from and where it gave me real surprise: I have pull.rebase=true in configuration. Being on a remote tracking branch, I've successfully pulled from the origin and had no any local changes on this branch. Then I've successfully merged another branch to the current one but didn't push the changes back upstream. A few hours later I returned to the work and issued git pull that instead of doing nothing (as it would be should pull.rebase be either false or preserve) created a surprising mess. Do you think it's worth fixing? Here are reproduction commands for the example history: git init t cd t echo A a echo B b git add a b git commit -m A -a git checkout -b x echo A a git commit -m C -a git checkout master echo B b git commit -m B -a git checkout -b topic git merge -m M x git branch -d x git rebase master -- Sergey. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Unify subcommand structure; introduce double dashes for all subcommands?
In the user survey 2012 question 23 (In your opinion, which areas in Git need improvement?), the most crucial point identified was the user interface. I wonder if there are any more recent surveys, showing if this has changed. Now when we want to improve the user interface, we're likely talking about the porcelain commands only, as that's what most users perceive as the commandline user interface. A git command is generally setup as: git command [subcommand] [options] ... The subcommands vary wildly by the nature of the command. However all subcommands could at least follow one style. The commands bundle, notes, stash and submodule have subcommands without two leading dashes (i.e. git stash list) as opposed to all other commands (i.e. git tag --list). So my proposal is to unify the structure of the subcommands to either have always leading dashes or never. This would need a longterm thinking of course (e.g. introduce new options with(out) dashes, but support existing commands until git 3.0 or such, then drop them.) Was there a discussion about this topic already? I'd like to read on these discussions, if any. I could think about the following points being interesting * user interface (what is more appealing to a user?) * ease of transition (Is it really worth it? How long does it take to pay off?) * ease of implementation (Could we reuse the option parser already in place for the double-dashed subcommands, i.e. have less LoC) * error-proneness of the transition 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 3/5] checkout --to: no auto-detach if the ref is already checked out
Nguyễn Thái Ngọc Duy venit, vidit, dixit 23.07.2014 13:43: Give the user a choice in this case. If they want to detach, they can go with '--detach --to ...', or they could switch branch of the checkout that's holding the ref in question. Or they could just create a new branch with '-b xxx --to yyy' Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/config.txt | 3 ++ advice.c | 2 ++ advice.h | 1 + builtin/checkout.c | 73 t/t2025-checkout-to.sh | 16 +-- 5 files changed, 56 insertions(+), 39 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 57999fa..4a41202 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -201,6 +201,9 @@ advice.*:: rmHints:: In case of failure in the output of linkgit:git-rm[1], show directions on how to proceed from the current state. + checkoutTo:: + In case of failure in the output of git checkout --to, + show directions on how to proceed from the current state. -- core.fileMode:: diff --git a/advice.c b/advice.c index 9b42033..b1fb524 100644 --- a/advice.c +++ b/advice.c @@ -15,6 +15,7 @@ int advice_detached_head = 1; int advice_set_upstream_failure = 1; int advice_object_name_warning = 1; int advice_rm_hints = 1; +int advice_checkout_to = 1; static struct { const char *name; @@ -35,6 +36,7 @@ static struct { { setupstreamfailure, advice_set_upstream_failure }, { objectnamewarning, advice_object_name_warning }, { rmhints, advice_rm_hints }, + { checkoutto, advice_checkout_to }, /* make this an alias for backward compatibility */ { pushnonfastforward, advice_push_update_rejected } diff --git a/advice.h b/advice.h index 5ecc6c1..a288219 100644 --- a/advice.h +++ b/advice.h @@ -18,6 +18,7 @@ extern int advice_detached_head; extern int advice_set_upstream_failure; extern int advice_object_name_warning; extern int advice_rm_hints; +extern int advice_checkout_to; int git_default_advice_config(const char *var, const char *value); __attribute__((format (printf, 1, 2))) diff --git a/builtin/checkout.c b/builtin/checkout.c index c83f476..d35245a 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1006,31 +1006,52 @@ static const char *unique_tracking_name(const char *name, unsigned char *sha1) return NULL; } -static int check_linked_checkout(struct branch_info *new, - const char *name, const char *path) +static void check_linked_checkout(struct branch_info *new, const char *id) { struct strbuf sb = STRBUF_INIT; + struct strbuf path = STRBUF_INIT; + struct strbuf gitdir = STRBUF_INIT; const char *start, *end; - if (strbuf_read_file(sb, path, 0) 0 || - !skip_prefix(sb.buf, ref:, start)) { - strbuf_release(sb); - return 0; - } + if (id) + strbuf_addf(path, %s/repos/%s/HEAD, get_git_common_dir(), id); + else + strbuf_addf(path, %s/HEAD, get_git_common_dir()); + + if (strbuf_read_file(sb, path.buf, 0) = 0 || + !skip_prefix(sb.buf, ref:, start)) + goto done; while (isspace(*start)) start++; end = start; while (*end !isspace(*end)) end++; - if (!strncmp(start, new-path, end - start) - new-path[end - start] == '\0') { - strbuf_release(sb); - new-path = NULL; /* detach */ - new-checkout = xstrdup(name); /* reason */ - return 1; - } + if (strncmp(start, new-path, end - start) || + new-path[end - start] != '\0') + goto done; + if (id) { + strbuf_reset(path); + strbuf_addf(path, %s/repos/%s/gitdir, + get_git_common_dir(), id); + if (strbuf_read_file(gitdir, path.buf, 0) = 0) + goto done; + while (gitdir.len (gitdir.buf[gitdir.len - 1] == '\n' || + gitdir.buf[gitdir.len - 1] == '\r')) + gitdir.buf[--gitdir.len] = '\0'; + } else + strbuf_addstr(gitdir, get_git_common_dir()); + if (advice_checkout_to) + die(_(%s is already checked out at %s.\n + Either use --detach or -b together with --to + or switch branch in the the other checkout.), or switch to a different branch in the other checkout. But maybe we can be even more helpful, like: %s is already checked out at %s.\n Either checkout the detached head of branch %s using\n git checkout --detach --to %s %s\n or checkout a new branch based off %s using\n git checkout --to %s -b %s newbranch %s\n or switch to a
Re: What's cooking in git.git (Jul 2014, #04; Tue, 22)
Am 22.07.2014 23:44, schrieb Junio C Hamano: * sk/mingw-uni-fix-more (2014-07-21) 14 commits - Win32: enable color output in Windows cmd.exe - Win32: patch Windows environment on startup - Win32: keep the environment sorted - Win32: use low-level memory allocation during initialization - Win32: reduce environment array reallocations - Win32: don't copy the environment twice when spawning child processes - Win32: factor out environment block creation - Win32: unify environment function names - Win32: unify environment case-sensitivity - Win32: fix environment memory leaks - Win32: Unicode environment (incoming) - Win32: Unicode environment (outgoing) - Revert Windows: teach getenv to do a case-sensitive search - tests: do not pass iso8859-1 encoded parameter Most of these are battle-tested in msysgit and are needed to complete what has been merged to 'master' already. A fix has been squashed into Unicode environ (outgoing); is this now ready to go? * sk/mingw-tests-workaround (2014-07-21) 6 commits - t800[12]: work around MSys limitation - t9902: mingw-specific fix for gitfile link files - t4210: skip command-line encoding tests on mingw - MinGW: disable legacy encoding tests - t0110/MinGW: skip tests that pass arbitrary bytes on the command line - MinGW: Skip test redirecting to fd 4 (this branch is used by jc/not-mingw-cygwin.) Make tests pass on msysgit by mostly disabling ones that are infeasible on that platform. The t0110 one has been replaced; is this now ready to go? Yes, I think both series are ready. Compiles with msysgit and MSVC (with NO_CURL=1). With the version in pu, three tests fail. t7001 is fixed with a newer 'cp'. The other two are unrelated (introduced by nd/multiple-work-trees topic). * t1501-worktree: failed 1 As of 5bbcb072 setup.c: support multi-checkout repo setup Using $TRASH_DIRECTORY doesn't work on Windows. * t2026-prune-linked-checkouts: failed 1 As of 404a45f1 prune: strategies for linked checkouts Dito. * t7001-mv: failed 6 'cp -P' doesn't work due to outdated cp.exe. -- To unsubscribe from this list: send the line 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 00/15] ref-transactions for reflogs
List, Jun, This is the next patch series for ref-transactions. It is also available at https://github.com/rsahlberg/git/tree/ref-transactions-reflog and is the same patch series that has been posted previously with one exception: This series now contains an additional patch that fixes ref handling of broken refs. This patch allows a user to delete a ref that can not be resolved and contains a broken SHA1. The main part of the patch series is to refactor the reflog handling to use transactions and eventually leads up to : * only a single place in the code where we marshall the reflog text line * atomic transaction for reflog.c for performing the reflog expire operation. This series is based on and builds on the previous series that is now in origin/pu: === commit c52f85eb59d26a7036d2149bc5b4347d0ecbbbeb Merge: 44fc7ba 282edb2 Author: Junio C Hamano gits...@pobox.com Date: Mon Jul 21 12:35:51 2014 -0700 Merge branch 'rs/ref-transaction' into jch * rs/ref-transaction: refs.c: fix handling of badly named refs refs.c: make write_ref_sha1 static fetch.c: change s_update_ref to use a ref transaction refs.c: propagate any errno==ENOTDIR from _commit back to the callers refs.c: pass a skip list to name_conflict_fn refs.c: call lock_ref_sha1_basic directly from commit refs.c: move the check for valid refname to lock_ref_sha1_basic refs.c: pass NULL as *flags to read_ref_full refs.c: pass the ref log message to _create/delete/update instead of _commit refs.c: add an err argument to delete_ref_loose wrapper.c: add a new function unlink_or_msg wrapper.c: simplify warn_if_unremovable === Ronnie Sahlberg (15): refs.c make ref_transaction_create a wrapper to ref_transaction_update refs.c: make ref_transaction_delete a wrapper for ref_transaction_update refs.c: rename the transaction functions refs.c: add a new update_type field to ref_update refs.c: add a function to append a reflog entry to a fd lockfile.c: make hold_lock_file_for_append preserve meaningful errno refs.c: add a transaction function to append a reflog entry refs.c: add a flag to allow reflog updates to truncate the log refs.c: only write reflog update if msg is non-NULL refs.c: allow multiple reflog updates during a single transaction reflog.c: use a reflog transaction when writing during expire refs.c: rename log_ref_setup to create_reflog refs.c: make unlock_ref/close_ref/commit_ref static refs.c: remove lock_any_ref_for_update refs.c: allow deleting refs with a broken sha1 branch.c | 11 +- builtin/branch.c | 6 +- builtin/checkout.c | 8 +- builtin/commit.c | 14 +- builtin/fetch.c| 12 +- builtin/receive-pack.c | 14 +- builtin/reflog.c | 84 +-- builtin/replace.c | 10 +- builtin/tag.c | 10 +- builtin/update-ref.c | 22 +-- copy.c | 20 ++- fast-import.c | 22 +-- lockfile.c | 7 +- refs.c | 392 ++--- refs.h | 110 +++--- sequencer.c| 14 +- walker.c | 16 +- 17 files changed, 461 insertions(+), 311 deletions(-) -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 04/15] refs.c: add a new update_type field to ref_update
Add a field that describes what type of update this refers to. For now the only type is UPDATE_SHA1 but we will soon add more types. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 25 + 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 91163e8..e88cb97 100644 --- a/refs.c +++ b/refs.c @@ -3372,6 +3372,10 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) return retval; } +enum transaction_update_type { + UPDATE_SHA1 = 0 +}; + /** * Information needed for a single ref update. Set new_sha1 to the * new value or to zero to delete the ref. To check the old value @@ -3379,6 +3383,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) * value or to zero to ensure the ref does not exist before update. */ struct ref_update { + enum transaction_update_type update_type; unsigned char new_sha1[20]; unsigned char old_sha1[20]; int flags; /* REF_NODEREF? */ @@ -3441,12 +3446,14 @@ void transaction_free(struct ref_transaction *transaction) } static struct ref_update *add_update(struct ref_transaction *transaction, -const char *refname) +const char *refname, +enum transaction_update_type update_type) { size_t len = strlen(refname); struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1); strcpy((char *)update-refname, refname); + update-update_type = update_type; ALLOC_GROW(transaction-updates, transaction-nr + 1, transaction-alloc); transaction-updates[transaction-nr++] = update; return update; @@ -3473,7 +3480,7 @@ int transaction_update_sha1(struct ref_transaction *transaction, return -1; } - update = add_update(transaction, refname); + update = add_update(transaction, refname, UPDATE_SHA1); hashcpy(update-new_sha1, new_sha1); update-flags = flags; update-have_old = have_old; @@ -3561,7 +3568,10 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n, struct strbuf *err) { int i; - for (i = 1; i n; i++) + for (i = 1; i n; i++) { + if (updates[i - 1]-update_type != UPDATE_SHA1 || + updates[i]-update_type != UPDATE_SHA1) + continue; if (!strcmp(updates[i - 1]-refname, updates[i]-refname)) { const char *str = Multiple updates for ref '%s' not allowed.; @@ -3570,6 +3580,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n, return 1; } + } return 0; } @@ -3599,10 +3610,12 @@ int transaction_commit(struct ref_transaction *transaction, goto cleanup; } - /* Acquire all locks while verifying old values */ + /* Acquire all ref locks while verifying old values */ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; + if (update-update_type != UPDATE_SHA1) + continue; update-lock = lock_ref_sha1_basic(update-refname, (update-have_old ? update-old_sha1 : @@ -3625,6 +3638,8 @@ int transaction_commit(struct ref_transaction *transaction, for (i = 0; i n; i++) { struct ref_update *update = updates[i]; + if (update-update_type != UPDATE_SHA1) + continue; if (!is_null_sha1(update-new_sha1)) { ret = write_ref_sha1(update-lock, update-new_sha1, update-msg); @@ -3644,6 +3659,8 @@ int transaction_commit(struct ref_transaction *transaction, for (i = 0; i n; i++) { struct ref_update *update = updates[i]; + if (update-update_type != UPDATE_SHA1) + continue; if (update-lock) { if (delete_ref_loose(update-lock, update-type, err)) ret = -1; -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 01/15] refs.c make ref_transaction_create a wrapper to ref_transaction_update
Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 18 ++ refs.h | 7 --- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/refs.c b/refs.c index 6dcb920..8f2aa3a 100644 --- a/refs.c +++ b/refs.c @@ -3490,28 +3490,14 @@ int ref_transaction_create(struct ref_transaction *transaction, int flags, const char *msg, struct strbuf *err) { - struct ref_update *update; - if (transaction-state != REF_TRANSACTION_OPEN) die(BUG: create called for transaction that is not open); if (!new_sha1 || is_null_sha1(new_sha1)) die(BUG: create ref with null new_sha1); - if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) { - strbuf_addf(err, Bad refname: %s, refname); - return -1; - } - - update = add_update(transaction, refname); - - hashcpy(update-new_sha1, new_sha1); - hashclr(update-old_sha1); - update-flags = flags; - update-have_old = 1; - if (msg) - update-msg = xstrdup(msg); - return 0; + return ref_transaction_update(transaction, refname, new_sha1, + null_sha1, flags, 1, msg, err); } int ref_transaction_delete(struct ref_transaction *transaction, diff --git a/refs.h b/refs.h index b0476c1..1c08cfd 100644 --- a/refs.h +++ b/refs.h @@ -276,9 +276,10 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); /* * Add a reference update to transaction. new_sha1 is the value that * the reference should have after the update, or zeros if it should - * be deleted. If have_old is true, then old_sha1 holds the value - * that the reference should have had before the update, or zeros if - * it must not have existed beforehand. + * be deleted. If have_old is true and old_sha is not the null_sha1 + * then the previous value of the ref must match or the update will fail. + * If have_old is true and old_sha1 is the null_sha1 then the ref must not + * already exist and a new ref will be created with new_sha1. * Function returns 0 on success and non-zero on failure. A failure to update * means that the transaction as a whole has failed and will need to be * rolled back. -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 06/15] lockfile.c: make hold_lock_file_for_append preserve meaningful errno
Update hold_lock_file_for_append and copy_fd to return a meaningful errno on failure. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- copy.c | 20 +--- lockfile.c | 7 ++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/copy.c b/copy.c index a7f58fd..5cb8679 100644 --- a/copy.c +++ b/copy.c @@ -9,10 +9,12 @@ int copy_fd(int ifd, int ofd) if (!len) break; if (len 0) { - int read_error = errno; + int save_errno = errno; close(ifd); - return error(copy-fd: read returned %s, -strerror(read_error)); + error(copy-fd: read returned %s, + strerror(save_errno)); + errno = save_errno; + return -1; } while (len) { int written = xwrite(ofd, buf, len); @@ -22,12 +24,16 @@ int copy_fd(int ifd, int ofd) } else if (!written) { close(ifd); - return error(copy-fd: write returned 0); + error(copy-fd: write returned 0); + errno = EAGAIN; + return -1; } else { - int write_error = errno; + int save_errno = errno; close(ifd); - return error(copy-fd: write returned %s, -strerror(write_error)); + error(copy-fd: write returned %s, + strerror(save_errno)); + errno = save_errno; + return -1; } } } diff --git a/lockfile.c b/lockfile.c index a921d77..32f4681 100644 --- a/lockfile.c +++ b/lockfile.c @@ -217,15 +217,20 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags) orig_fd = open(path, O_RDONLY); if (orig_fd 0) { if (errno != ENOENT) { + int save_errno = errno; if (flags LOCK_DIE_ON_ERROR) die(cannot open '%s' for copying, path); close(fd); - return error(cannot open '%s' for copying, path); + error(cannot open '%s' for copying, path); + errno = save_errno; + return -1; } } else if (copy_fd(orig_fd, fd)) { + int save_errno = errno; if (flags LOCK_DIE_ON_ERROR) exit(128); close(fd); + errno = save_errno; return -1; } return fd; -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 08/15] refs.c: add a flag to allow reflog updates to truncate the log
Add a flag that allows us to truncate the reflog before we write the update. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 17 +++-- refs.h | 10 +- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/refs.c b/refs.c index b010d6d..181c957 100644 --- a/refs.c +++ b/refs.c @@ -3747,7 +3747,12 @@ int transaction_commit(struct ref_transaction *transaction, } } - /* Update all reflog files */ + /* +* Update all reflog files +* We have already done all ref updates and deletes. +* There is not much we can do here if there are any reflog +* update errors other than complain. +*/ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; @@ -3755,7 +3760,15 @@ int transaction_commit(struct ref_transaction *transaction, continue; if (update-reflog_fd == -1) continue; - + if (update-flags REFLOG_TRUNCATE) + if (lseek(update-reflog_fd, 0, SEEK_SET) 0 || + ftruncate(update-reflog_fd, 0)) { + error(Could not truncate reflog: %s. %s, + update-refname, strerror(errno)); + rollback_lock_file(update-reflog_lock); + update-reflog_fd = -1; + continue; + } if (log_ref_write_fd(update-reflog_fd, update-old_sha1, update-new_sha1, update-committer, update-msg)) { diff --git a/refs.h b/refs.h index 32bc4ae..66cf38b 100644 --- a/refs.h +++ b/refs.h @@ -321,7 +321,15 @@ int transaction_delete_sha1(struct ref_transaction *transaction, struct strbuf *err); /* - * Append a reflog entry for refname. + * Flags controlling transaction_update_reflog(). + * REFLOG_TRUNCATE: Truncate the reflog. + * + * Flags = 0x100 are reserved for internal use. + */ +#define REFLOG_TRUNCATE 0x01 +/* + * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set + * this update will first truncate the reflog before writing the entry. */ int transaction_update_reflog(struct ref_transaction *transaction, const char *refname, -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 07/15] refs.c: add a transaction function to append a reflog entry
Define a new transaction update type, UPDATE_LOG, and a new function transaction_update_reflog. This function will lock the reflog and append an entry to it during transaction commit. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 103 +++-- refs.h | 12 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index a479ddd..b010d6d 100644 --- a/refs.c +++ b/refs.c @@ -3385,7 +3385,8 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) } enum transaction_update_type { - UPDATE_SHA1 = 0 + UPDATE_SHA1 = 0, + UPDATE_LOG = 1 }; /** @@ -3403,6 +3404,12 @@ struct ref_update { struct ref_lock *lock; int type; char *msg; + + /* used by reflog updates */ + int reflog_fd; + struct lock_file reflog_lock; + char *committer; + const char refname[FLEX_ARRAY]; }; @@ -3451,6 +3458,7 @@ void transaction_free(struct ref_transaction *transaction) for (i = 0; i transaction-nr; i++) { free(transaction-updates[i]-msg); + free(transaction-updates[i]-committer); free(transaction-updates[i]); } free(transaction-updates); @@ -3471,6 +3479,42 @@ static struct ref_update *add_update(struct ref_transaction *transaction, return update; } +int transaction_update_reflog(struct ref_transaction *transaction, + const char *refname, + const unsigned char *new_sha1, + const unsigned char *old_sha1, + const unsigned char *email, + unsigned long timestamp, int tz, + const char *msg, int flags, + struct strbuf *err) +{ + struct ref_update *update; + + if (transaction-state != REF_TRANSACTION_OPEN) + die(BUG: update_reflog called for transaction that is not + open); + + update = add_update(transaction, refname, UPDATE_LOG); + hashcpy(update-new_sha1, new_sha1); + hashcpy(update-old_sha1, old_sha1); + update-reflog_fd = -1; + if (email) { + struct strbuf buf = STRBUF_INIT; + char sign = (tz 0) ? '-' : '+'; + int zone = (tz 0) ? (-tz) : tz; + + strbuf_addf(buf, %s %lu %c%04d, email, timestamp, sign, + zone); + update-committer = xstrdup(buf.buf); + strbuf_release(buf); + } + if (msg) + update-msg = xstrdup(msg); + update-flags = flags; + + return 0; +} + int transaction_update_sha1(struct ref_transaction *transaction, const char *refname, const unsigned char *new_sha1, @@ -3646,7 +3690,28 @@ int transaction_commit(struct ref_transaction *transaction, } } - /* Perform updates first so live commits remain referenced */ + /* Lock all reflog files */ + for (i = 0; i n; i++) { + struct ref_update *update = updates[i]; + + if (update-update_type != UPDATE_LOG) + continue; + update-reflog_fd = hold_lock_file_for_append( + update-reflog_lock, + git_path(logs/%s, update-refname), + 0); + if (update-reflog_fd 0) { + const char *str = Cannot lock reflog for '%s'. %s; + + ret = -1; + if (err) + strbuf_addf(err, str, update-refname, + strerror(errno)); + goto cleanup; + } + } + + /* Perform ref updates first so live commits remain referenced */ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; @@ -3682,6 +3747,40 @@ int transaction_commit(struct ref_transaction *transaction, } } + /* Update all reflog files */ + for (i = 0; i n; i++) { + struct ref_update *update = updates[i]; + + if (update-update_type != UPDATE_LOG) + continue; + if (update-reflog_fd == -1) + continue; + + if (log_ref_write_fd(update-reflog_fd, update-old_sha1, +update-new_sha1, +update-committer, update-msg)) { + error(Could write to reflog: %s. %s, + update-refname, strerror(errno)); + rollback_lock_file(update-reflog_lock); + update-reflog_fd = -1; + } + } + +
[PATCH 11/15] reflog.c: use a reflog transaction when writing during expire
Use a transaction for all updates during expire_reflog. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/reflog.c | 84 refs.c | 4 +-- refs.h | 2 +- 3 files changed, 39 insertions(+), 51 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index e8a8fb1..f11fee3 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -32,8 +32,11 @@ struct cmd_reflog_expire_cb { int recno; }; +static struct strbuf err = STRBUF_INIT; + struct expire_reflog_cb { - FILE *newlog; + struct ref_transaction *t; + const char *refname; enum { UE_NORMAL, UE_ALWAYS, @@ -316,20 +319,18 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1, if (cb-cmd-recno --(cb-cmd-recno) == 0) goto prune; - if (cb-newlog) { - char sign = (tz 0) ? '-' : '+'; - int zone = (tz 0) ? (-tz) : tz; - fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s, - sha1_to_hex(osha1), sha1_to_hex(nsha1), - email, timestamp, sign, zone, - message); + if (cb-t) { + if (transaction_update_reflog(cb-t, cb-refname, nsha1, osha1, + email, timestamp, tz, message, 0, + err)) + return -1; hashcpy(cb-last_kept_sha1, nsha1); } if (cb-cmd-verbose) printf(keep %s, message); return 0; prune: - if (!cb-newlog) + if (!cb-t) printf(would prune %s, message); else if (cb-cmd-verbose) printf(prune %s, message); @@ -353,29 +354,26 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, { struct cmd_reflog_expire_cb *cmd = cb_data; struct expire_reflog_cb cb; - struct ref_lock *lock; - char *log_file, *newlog_path = NULL; struct commit *tip_commit; struct commit_list *tips; int status = 0; memset(cb, 0, sizeof(cb)); + cb.refname = ref; - /* -* we take the lock for the ref itself to prevent it from -* getting updated. -*/ - lock = lock_any_ref_for_update(ref, sha1, 0, NULL); - if (!lock) - return error(cannot lock ref '%s', ref); - log_file = git_pathdup(logs/%s, ref); if (!reflog_exists(ref)) goto finish; - if (!cmd-dry_run) { - newlog_path = git_pathdup(logs/%s.lock, ref); - cb.newlog = fopen(newlog_path, w); + cb.t = transaction_begin(err); + if (!cb.t) { + status |= error(%s, err.buf); + goto cleanup; + } + if (transaction_update_reflog(cb.t, cb.refname, null_sha1, null_sha1, + NULL, 0, 0, NULL, REFLOG_TRUNCATE, + err)) { + status |= error(%s, err.buf); + goto cleanup; } - cb.cmd = cmd; if (!cmd-expire_unreachable || !strcmp(ref, HEAD)) { @@ -407,7 +405,10 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, mark_reachable(cb); } - for_each_reflog_ent(ref, expire_reflog_ent, cb); + if (for_each_reflog_ent(ref, expire_reflog_ent, cb)) { + status |= error(%s, err.buf); + goto cleanup; + } if (cb.unreachable_expire_kind != UE_ALWAYS) { if (cb.unreachable_expire_kind == UE_HEAD) { @@ -420,32 +421,19 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, } } finish: - if (cb.newlog) { - if (fclose(cb.newlog)) { - status |= error(%s: %s, strerror(errno), - newlog_path); - unlink(newlog_path); - } else if (cmd-updateref - (write_in_full(lock-lock_fd, - sha1_to_hex(cb.last_kept_sha1), 40) != 40 || -write_str_in_full(lock-lock_fd, \n) != 1 || -close_ref(lock) 0)) { - status |= error(Couldn't write %s, - lock-lk-filename); - unlink(newlog_path); - } else if (rename(newlog_path, log_file)) { - status |= error(cannot rename %s to %s, - newlog_path, log_file); - unlink(newlog_path); - } else if (cmd-updateref commit_ref(lock)) { - status |= error(Couldn't set %s, lock-ref_name); - } else { -
[PATCH 03/15] refs.c: rename the transaction functions
Rename the transaction functions. Remove the leading ref_ from the names and append _sha1 to the names for functions that create/delete/ update sha1 refs. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 11 --- builtin/commit.c | 14 - builtin/fetch.c| 12 builtin/receive-pack.c | 14 - builtin/replace.c | 10 +++--- builtin/tag.c | 10 +++--- builtin/update-ref.c | 22 +++--- fast-import.c | 22 +++--- refs.c | 82 +- refs.h | 54 - sequencer.c| 14 - walker.c | 16 +- 12 files changed, 141 insertions(+), 140 deletions(-) diff --git a/branch.c b/branch.c index e0439af..6fa6d78 100644 --- a/branch.c +++ b/branch.c @@ -298,13 +298,14 @@ void create_branch(const char *head, struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, ref.buf, sha1, - null_sha1, 0, !forcing, msg, err) || - ref_transaction_commit(transaction, err)) + transaction_update_sha1(transaction, ref.buf, sha1, + null_sha1, 0, !forcing, msg, + err) || + transaction_commit(transaction, err)) die(%s, err.buf); - ref_transaction_free(transaction); + transaction_free(transaction); } if (real_ref track) diff --git a/builtin/commit.c b/builtin/commit.c index c499826..bf8d85a 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1762,17 +1762,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix) strbuf_insert(sb, 0, reflog_msg, strlen(reflog_msg)); strbuf_insert(sb, strlen(reflog_msg), : , 2); - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, HEAD, sha1, - current_head ? - current_head-object.sha1 : NULL, - 0, !!current_head, sb.buf, err) || - ref_transaction_commit(transaction, err)) { + transaction_update_sha1(transaction, HEAD, sha1, + current_head ? + current_head-object.sha1 : NULL, + 0, !!current_head, sb.buf, err) || + transaction_commit(transaction, err)) { rollback_index_files(); die(%s, err.buf); } - ref_transaction_free(transaction); + transaction_free(transaction); unlink(git_path(CHERRY_PICK_HEAD)); unlink(git_path(REVERT_HEAD)); diff --git a/builtin/fetch.c b/builtin/fetch.c index 383c385..7320395 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -414,22 +414,22 @@ static int s_update_ref(const char *action, rla = default_rla.buf; snprintf(msg, sizeof(msg), %s: %s, rla, action); - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, ref-name, ref-new_sha1, - ref-old_sha1, 0, check_old, msg, err)) + transaction_update_sha1(transaction, ref-name, ref-new_sha1, + ref-old_sha1, 0, check_old, msg, err)) goto fail; - ret = ref_transaction_commit(transaction, err); + ret = transaction_commit(transaction, err); if (ret == UPDATE_REFS_NAME_CONFLICT) df_conflict = 1; if (ret) goto fail; - ref_transaction_free(transaction); + transaction_free(transaction); return 0; fail: - ref_transaction_free(transaction); + transaction_free(transaction); error(%s, err.buf); strbuf_release(err); return df_conflict ? STORE_REF_ERROR_DF_CONFLICT diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 4752225..0565b94 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -582,20 +582,20 @@ static char *update(struct command *cmd, struct shallow_info *si) update_shallow_ref(cmd, si)) return xstrdup(shallow error); - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction,
[PATCH 05/15] refs.c: add a function to append a reflog entry to a fd
Break out the code to create the string and writing it to the file descriptor from log_ref_write and into a dedicated function log_ref_write_fd. For now this is only used from log_ref_write but later on we will call this function from reflog transactions too which means that we will end up with only a single place where we write a reflog entry to a file instead of the current two places (log_ref_write and builtin/reflog.c). Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 48 ++-- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/refs.c b/refs.c index e88cb97..a479ddd 100644 --- a/refs.c +++ b/refs.c @@ -2862,15 +2862,37 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize) return 0; } +static int log_ref_write_fd(int fd, const unsigned char *old_sha1, + const unsigned char *new_sha1, + const char *committer, const char *msg) +{ + int msglen, written; + unsigned maxlen, len; + char *logrec; + + msglen = msg ? strlen(msg) : 0; + maxlen = strlen(committer) + msglen + 100; + logrec = xmalloc(maxlen); + len = sprintf(logrec, %s %s %s\n, + sha1_to_hex(old_sha1), + sha1_to_hex(new_sha1), + committer); + if (msglen) + len += copy_msg(logrec + len - 1, msg) - 1; + + written = len = maxlen ? write_in_full(fd, logrec, len) : -1; + free(logrec); + if (written != len) + return -1; + + return 0; +} + static int log_ref_write(const char *refname, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *msg) { - int logfd, result, written, oflags = O_APPEND | O_WRONLY; - unsigned maxlen, len; - int msglen; + int logfd, result, oflags = O_APPEND | O_WRONLY; char log_file[PATH_MAX]; - char *logrec; - const char *committer; if (log_all_ref_updates 0) log_all_ref_updates = !is_bare_repository(); @@ -2882,19 +2904,9 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1, logfd = open(log_file, oflags); if (logfd 0) return 0; - msglen = msg ? strlen(msg) : 0; - committer = git_committer_info(0); - maxlen = strlen(committer) + msglen + 100; - logrec = xmalloc(maxlen); - len = sprintf(logrec, %s %s %s\n, - sha1_to_hex(old_sha1), - sha1_to_hex(new_sha1), - committer); - if (msglen) - len += copy_msg(logrec + len - 1, msg) - 1; - written = len = maxlen ? write_in_full(logfd, logrec, len) : -1; - free(logrec); - if (written != len) { + result = log_ref_write_fd(logfd, old_sha1, new_sha1, + git_committer_info(0), msg); + if (result) { int save_errno = errno; close(logfd); error(Unable to append to %s, log_file); -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 12/15] refs.c: rename log_ref_setup to create_reflog
log_ref_setup is used to do several semi-related things : * sometimes it will create a new reflog including missing parent directories and cleaning up any conflicting stale directories in the path. * fill in a filename buffer for the full path to the reflog. * unconditionally re-adjust the permissions for the file. This function is only called from two places: checkout.c where it is always used to create a reflog and refs.c:log_ref_write where it sometimes are used to create a reflog and sometimes just used to fill in the filename. Rename log_ref_setup to create_reflog and change it to only take the refname as argument to make its signature similar to delete_reflog and reflog_exists. Change create_reflog to ignore log_all_ref_updates and unconditionally create the reflog when called. Since checkout.c always wants to create a reflog we can call create_reflog directly and avoid the temp-and-log_all_ref_update dance. In log_ref_write, only call create_reflog iff we want to create a reflog and if the reflog does not yet exist. This means that for the common case where the log already exists we now only need to perform a single lstat() instead of a open(O_CREAT)+lstat()+close(). Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/checkout.c | 8 +--- refs.c | 22 +- refs.h | 8 +++- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index f1dc56e..808c58f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -583,19 +583,13 @@ static void update_refs_for_switch(const struct checkout_opts *opts, if (opts-new_branch) { if (opts-new_orphan_branch) { if (opts-new_branch_log !log_all_ref_updates) { - int temp; - char log_file[PATH_MAX]; char *ref_name = mkpath(refs/heads/%s, opts-new_orphan_branch); - temp = log_all_ref_updates; - log_all_ref_updates = 1; - if (log_ref_setup(ref_name, log_file, sizeof(log_file))) { + if (create_reflog(ref_name)) { fprintf(stderr, _(Can not do reflog for '%s'\n), opts-new_orphan_branch); - log_all_ref_updates = temp; return; } - log_all_ref_updates = temp; } } else diff --git a/refs.c b/refs.c index 4231fad..b7940e0 100644 --- a/refs.c +++ b/refs.c @@ -2819,16 +2819,16 @@ static int copy_msg(char *buf, const char *msg) } /* This function must set a meaningful errno on failure */ -int log_ref_setup(const char *refname, char *logfile, int bufsize) +int create_reflog(const char *refname) { int logfd, oflags = O_APPEND | O_WRONLY; + char logfile[PATH_MAX]; - git_snpath(logfile, bufsize, logs/%s, refname); - if (log_all_ref_updates - (starts_with(refname, refs/heads/) || -starts_with(refname, refs/remotes/) || -starts_with(refname, refs/notes/) || -!strcmp(refname, HEAD))) { + git_snpath(logfile, sizeof(logfile), logs/%s, refname); + if (starts_with(refname, refs/heads/) || + starts_with(refname, refs/remotes/) || + starts_with(refname, refs/notes/) || + !strcmp(refname, HEAD)) { if (safe_create_leading_directories(logfile) 0) { int save_errno = errno; error(unable to create directory for %s, logfile); @@ -2897,16 +2897,20 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1, static int log_ref_write(const char *refname, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *msg) { - int logfd, result, oflags = O_APPEND | O_WRONLY; + int logfd, result = 0, oflags = O_APPEND | O_WRONLY; char log_file[PATH_MAX]; if (log_all_ref_updates 0) log_all_ref_updates = !is_bare_repository(); - result = log_ref_setup(refname, log_file, sizeof(log_file)); + if (log_all_ref_updates !reflog_exists(refname)) + result = create_reflog(refname); + if (result) return result; + git_snpath(log_file, sizeof(log_file), logs/%s, refname); + logfd = open(log_file, oflags); if (logfd 0) return 0; diff --git a/refs.h b/refs.h index 9ad12fe..6763f69 100644 --- a/refs.h +++ b/refs.h @@ -196,11 +196,6 @@ extern int commit_ref(struct ref_lock *lock); /** Release any lock taken but not written. **/ extern void unlock_ref(struct ref_lock *lock); -/* - * Setup reflog
[PATCH 09/15] refs.c: only write reflog update if msg is non-NULL
When performing a reflog transaction update, only write to the reflog iff msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform an update that only truncates but does not write. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 5 +++-- refs.h | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 181c957..c431088 100644 --- a/refs.c +++ b/refs.c @@ -3769,8 +3769,9 @@ int transaction_commit(struct ref_transaction *transaction, update-reflog_fd = -1; continue; } - if (log_ref_write_fd(update-reflog_fd, update-old_sha1, -update-new_sha1, + if (update-msg + log_ref_write_fd(update-reflog_fd, +update-old_sha1, update-new_sha1, update-committer, update-msg)) { error(Could write to reflog: %s. %s, update-refname, strerror(errno)); diff --git a/refs.h b/refs.h index 66cf38b..a8b7047 100644 --- a/refs.h +++ b/refs.h @@ -330,6 +330,7 @@ int transaction_delete_sha1(struct ref_transaction *transaction, /* * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set * this update will first truncate the reflog before writing the entry. + * If msg is NULL no update will be written to the log. */ int transaction_update_reflog(struct ref_transaction *transaction, const char *refname, -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 02/15] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update
Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 17 + refs.h | 2 +- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/refs.c b/refs.c index 8f2aa3a..74fb797 100644 --- a/refs.c +++ b/refs.c @@ -3506,24 +3506,17 @@ int ref_transaction_delete(struct ref_transaction *transaction, int flags, int have_old, const char *msg, struct strbuf *err) { - struct ref_update *update; - if (transaction-state != REF_TRANSACTION_OPEN) die(BUG: delete called for transaction that is not open); if (have_old !old_sha1) die(BUG: have_old is true but old_sha1 is NULL); - update = add_update(transaction, refname); - update-flags = flags; - update-have_old = have_old; - if (have_old) { - assert(!is_null_sha1(old_sha1)); - hashcpy(update-old_sha1, old_sha1); - } - if (msg) - update-msg = xstrdup(msg); - return 0; + if (have_old is_null_sha1(old_sha1)) + die(BUG: have_old is true but old_sha1 is null_sha1); + + return ref_transaction_update(transaction, refname, null_sha1, + old_sha1, flags, have_old, msg, err); } int update_ref(const char *action, const char *refname, diff --git a/refs.h b/refs.h index 1c08cfd..f680b19 100644 --- a/refs.h +++ b/refs.h @@ -275,7 +275,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); /* * Add a reference update to transaction. new_sha1 is the value that - * the reference should have after the update, or zeros if it should + * the reference should have after the update, or null_sha1 if it should * be deleted. If have_old is true and old_sha is not the null_sha1 * then the previous value of the ref must match or the update will fail. * If have_old is true and old_sha1 is the null_sha1 then the ref must not -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 10/15] refs.c: allow multiple reflog updates during a single transaction
Allow to make multiple reflog updates to the same ref during a transaction. This means we only need to lock the reflog once, during the first update that touches the reflog, and that all further updates can just write the reflog entry since the reflog is already locked. This allows us to write code such as: t = transaction_begin() transaction_reflog_update(t, foo, REFLOG_TRUNCATE, NULL); loop-over-somehting... transaction_reflog_update(t, foo, 0, message); transaction_commit(t) where we first truncate the reflog and then build the new content one line at a time. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 46 +- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/refs.c b/refs.c index c431088..12ff916 100644 --- a/refs.c +++ b/refs.c @@ -30,6 +30,12 @@ static unsigned char refname_disposition[256] = { */ #define REF_ISPRUNING 0x0100 /* + * Only the first reflog update needs to lock the reflog file. Further updates + * just use the lock taken by the first update. + */ +#define UPDATE_REFLOG_NOLOCK 0x0200 + +/* * Try to read one refname component from the front of refname. * Return the length of the component found, or -1 if the component is * not legal. It is legal if it is something reasonable to have under @@ -3389,7 +3395,7 @@ enum transaction_update_type { UPDATE_LOG = 1 }; -/** +/* * Information needed for a single ref update. Set new_sha1 to the * new value or to zero to delete the ref. To check the old value * while locking the ref, set have_old to 1 and set old_sha1 to the @@ -3399,7 +3405,7 @@ struct ref_update { enum transaction_update_type update_type; unsigned char new_sha1[20]; unsigned char old_sha1[20]; - int flags; /* REF_NODEREF? */ + int flags; /* REF_NODEREF? or private flags */ int have_old; /* 1 if old_sha1 is valid, 0 otherwise */ struct ref_lock *lock; int type; @@ -3407,8 +3413,9 @@ struct ref_update { /* used by reflog updates */ int reflog_fd; - struct lock_file reflog_lock; + struct lock_file *reflog_lock; char *committer; + struct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */ const char refname[FLEX_ARRAY]; }; @@ -3489,12 +3496,27 @@ int transaction_update_reflog(struct ref_transaction *transaction, struct strbuf *err) { struct ref_update *update; + int i; if (transaction-state != REF_TRANSACTION_OPEN) die(BUG: update_reflog called for transaction that is not open); update = add_update(transaction, refname, UPDATE_LOG); + update-flags = flags; + for (i = 0; i transaction-nr - 1; i++) { + if (transaction-updates[i]-update_type != UPDATE_LOG) + continue; + if (!strcmp(transaction-updates[i]-refname, + update-refname)) { + update-flags |= UPDATE_REFLOG_NOLOCK; + update-orig_update = transaction-updates[i]; + break; + } + } + if (!(update-flags UPDATE_REFLOG_NOLOCK)) + update-reflog_lock = xcalloc(1, sizeof(struct lock_file)); + hashcpy(update-new_sha1, new_sha1); hashcpy(update-old_sha1, old_sha1); update-reflog_fd = -1; @@ -3510,7 +3532,6 @@ int transaction_update_reflog(struct ref_transaction *transaction, } if (msg) update-msg = xstrdup(msg); - update-flags = flags; return 0; } @@ -3696,10 +3717,15 @@ int transaction_commit(struct ref_transaction *transaction, if (update-update_type != UPDATE_LOG) continue; + if (update-flags UPDATE_REFLOG_NOLOCK) { + update-reflog_fd = update-orig_update-reflog_fd; + update-reflog_lock = update-orig_update-reflog_lock; + continue; + } update-reflog_fd = hold_lock_file_for_append( - update-reflog_lock, + update-reflog_lock, git_path(logs/%s, update-refname), - 0); + LOCK_NODEREF); if (update-reflog_fd 0) { const char *str = Cannot lock reflog for '%s'. %s; @@ -3765,7 +3791,7 @@ int transaction_commit(struct ref_transaction *transaction, ftruncate(update-reflog_fd, 0)) { error(Could not truncate reflog: %s. %s, update-refname, strerror(errno)); - rollback_lock_file(update-reflog_lock); +
[PATCH 13/15] refs.c: make unlock_ref/close_ref/commit_ref static
unlock|close|commit_ref can be made static since there are no more external callers. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 24 refs.h | 9 - 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/refs.c b/refs.c index b7940e0..b74e5ff 100644 --- a/refs.c +++ b/refs.c @@ -1969,6 +1969,16 @@ int refname_match(const char *abbrev_name, const char *full_name) return 0; } +static void unlock_ref(struct ref_lock *lock) +{ + /* Do not free lock-lk -- atexit() still looks at them */ + if (lock-lk) + rollback_lock_file(lock-lk); + free(lock-ref_name); + free(lock-orig_ref_name); + free(lock); +} + /* This function should make sure errno is meaningful on error */ static struct ref_lock *verify_lock(struct ref_lock *lock, const unsigned char *old_sha1, int mustexist) @@ -2766,7 +2776,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms return 1; } -int close_ref(struct ref_lock *lock) +static int close_ref(struct ref_lock *lock) { if (close_lock_file(lock-lk)) return -1; @@ -2774,7 +2784,7 @@ int close_ref(struct ref_lock *lock) return 0; } -int commit_ref(struct ref_lock *lock) +static int commit_ref(struct ref_lock *lock) { if (commit_lock_file(lock-lk)) return -1; @@ -2782,16 +2792,6 @@ int commit_ref(struct ref_lock *lock) return 0; } -void unlock_ref(struct ref_lock *lock) -{ - /* Do not free lock-lk -- atexit() still looks at them */ - if (lock-lk) - rollback_lock_file(lock-lk); - free(lock-ref_name); - free(lock-orig_ref_name); - free(lock); -} - /* * copy the reflog message msg to buf, which has been allocated sufficiently * large, while cleaning up the whitespaces. Especially, convert LF to space, diff --git a/refs.h b/refs.h index 6763f69..65d6360 100644 --- a/refs.h +++ b/refs.h @@ -187,15 +187,6 @@ extern struct ref_lock *lock_any_ref_for_update(const char *refname, const unsigned char *old_sha1, int flags, int *type_p); -/** Close the file descriptor owned by a lock and return the status */ -extern int close_ref(struct ref_lock *lock); - -/** Close and commit the ref locked by the lock */ -extern int commit_ref(struct ref_lock *lock); - -/** Release any lock taken but not written. **/ -extern void unlock_ref(struct ref_lock *lock); - /** Reads log for the value of ref during at_time. **/ extern int read_ref_at(const char *refname, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 15/15] refs.c: allow deleting refs with a broken sha1
Add (back?) support to make it possible to delete refs that are broken. Add a new flag REF_ALLOWBROKEN that can be passed to the functions to lock a ref. If this flag is set we allow locking the ref even if the ref points to a broken sha1. For example a sha1 that is created by : echo Broken ref .git/refs/heads/foo-broken-1 Use this flag when calling from branch.c dusing a ref delete so that we only allow locking those broken refs IFF when called during a branch delete. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/branch.c | 6 -- refs.c | 10 +++--- refs.h | 3 ++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 5c95656..6d70037 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -236,6 +236,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, name = mkpathdup(fmt, bname.buf); target = resolve_ref_unsafe(name, sha1, RESOLVE_REF_ALLOW_BAD_NAME, flags); + if (!target (flags REF_ISBROKEN)) + target = name; if (!target || (!(flags REF_ISSYMREF) is_null_sha1(sha1))) { error(remote_branch @@ -245,14 +247,14 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, continue; } - if (!(flags REF_ISSYMREF) + if (!(flags (REF_ISSYMREF|REF_ISBROKEN)) check_branch_commit(bname.buf, name, sha1, head_rev, kinds, force)) { ret = 1; continue; } - if (delete_ref(name, sha1, REF_NODEREF)) { + if (delete_ref(name, sha1, REF_NODEREF|REF_ALLOWBROKEN)) { error(remote_branch ? _(Error deleting remote branch '%s') : _(Error deleting branch '%s'), diff --git a/refs.c b/refs.c index 0ead11f..2662ef6 100644 --- a/refs.c +++ b/refs.c @@ -2122,12 +2122,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, int resolve_flags; int missing = 0; int attempts_remaining = 3; - int bad_refname; + int bad_ref; lock = xcalloc(1, sizeof(struct ref_lock)); lock-lock_fd = -1; - bad_refname = check_refname_format(refname, REFNAME_ALLOW_ONELEVEL); + bad_ref = check_refname_format(refname, REFNAME_ALLOW_ONELEVEL); resolve_flags = RESOLVE_REF_ALLOW_BAD_NAME; if (mustexist) @@ -2150,6 +2150,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, refname = resolve_ref_unsafe(orig_refname, lock-old_sha1, resolve_flags, type); } + if (!refname (flags REF_ALLOWBROKEN) (type REF_ISBROKEN)) { + bad_ref = 1; + refname = orig_refname; + } if (type_p) *type_p = type; if (!refname) { @@ -2212,7 +2216,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, else unable_to_lock_index_die(ref_file, errno); } - if (bad_refname) + if (bad_ref) return lock; return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock; diff --git a/refs.h b/refs.h index 712fc32..0172f48 100644 --- a/refs.h +++ b/refs.h @@ -174,10 +174,11 @@ extern int peel_ref(const char *refname, unsigned char *sha1); * Flags controlling transaction_update_sha1(), transaction_create_sha1(), etc. * REF_NODEREF: act on the ref directly, instead of dereferencing * symbolic references. - * + * REF_ALLOWBROKEN: allow locking refs that are broken. * Flags = 0x100 are reserved for internal use. */ #define REF_NODEREF0x01 +#define REF_ALLOWBROKEN 0x02 /** Reads log for the value of ref during at_time. **/ extern int read_ref_at(const char *refname, unsigned long at_time, int cnt, -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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 14/15] refs.c: remove lock_any_ref_for_update
No one is using this function so we can delete it. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 7 --- refs.h | 10 +- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/refs.c b/refs.c index b74e5ff..0ead11f 100644 --- a/refs.c +++ b/refs.c @@ -,13 +,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, return NULL; } -struct ref_lock *lock_any_ref_for_update(const char *refname, -const unsigned char *old_sha1, -int flags, int *type_p) -{ - return lock_ref_sha1_basic(refname, old_sha1, flags, type_p, NULL, 0); -} - /* * Write an entry to the packed-refs file for the specified refname. * If peeled is non-NULL, write it as the entry's peeled value. diff --git a/refs.h b/refs.h index 65d6360..712fc32 100644 --- a/refs.h +++ b/refs.h @@ -171,21 +171,13 @@ extern int ref_exists(const char *); extern int peel_ref(const char *refname, unsigned char *sha1); /* - * Flags controlling lock_any_ref_for_update(), transaction_update_sha1(), - * transaction_create_sha1(), etc. + * Flags controlling transaction_update_sha1(), transaction_create_sha1(), etc. * REF_NODEREF: act on the ref directly, instead of dereferencing * symbolic references. * * Flags = 0x100 are reserved for internal use. */ #define REF_NODEREF0x01 -/* - * Locks any ref (for 'HEAD' type refs) and sets errno to something - * meaningful on failure. - */ -extern struct ref_lock *lock_any_ref_for_update(const char *refname, - const unsigned char *old_sha1, - int flags, int *type_p); /** Reads log for the value of ref during at_time. **/ extern int read_ref_at(const char *refname, unsigned long at_time, int cnt, -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line unsubscribe 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] config.c: change the function signature of `git_config_string()`
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: *1* We have safe_create_leading_directories_const() that works around this for input parameter around its _const less counterpart, which is ugly but livable solution. I think it would actually be a reasonable solution to avoid casting here and there on the caller side. Ugly primarily refers to the fact that we are forced to do this in the first place by the language. I agree with you, especially if we have very many call sites, and I suspect config-get-string actually would. Another option would be to _return_ a non-const char * instead of outputing it as a by-address parameter. Here, too, I agree that it is the most C-ish interface. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] checkout --to: no auto-detach if the ref is already checked out
Michael J Gruber g...@drmicha.warpmail.net writes: +if (advice_checkout_to) +die(_(%s is already checked out at %s.\n + Either use --detach or -b together with --to + or switch branch in the the other checkout.), or switch to a different branch in the other checkout. But maybe we can be even more helpful, like: %s is already checked out at %s.\n Either checkout the detached head of branch %s using\n git checkout --detach --to %s %s\n or checkout a new branch based off %s using\n git checkout --to %s -b %s newbranch %s\n or switch to a different branch in the other checkout. if we can figure out the appropriate arguments at this point in the code. Another possible alternative is go and work there instead of creating yet another checkout, but is that too obvious without saying? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: rebase flattens history when it shouldn't?
Hi Sergei, Sergei Organov wrote: --C-- / \ / M topic,HEAD / / A---B master shouldn't $ git rebase master be a no-op here? [...] I'd expect --force-rebase to be required for this to happen: -f, --force-rebase Force the rebase even if the current branch is a descendant of the commit you are rebasing onto. Normally non-interactive rebase will exit with the message Current branch is up to date in such a situation. [...] Do you think it's worth fixing? Thanks for a clear report. After a successful 'git rebase master', the current branch is always a linear string of patches on top of 'master'. The already up to date behavior when -f is not passed is in a certain sense an optimization --- it is about git noticing that 'git rebase' wouldn't have anything to do (except for touching timestamps) and therefore doing nothing. So I don't think requiring -f for this case would be an improvement. I do agree that the documentation is misleading. Any ideas for wording that could make it clearer? 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: Unify subcommand structure; introduce double dashes for all subcommands?
Stefan Beller stefanbel...@gmail.com writes: A git command is generally setup as: git command [subcommand] [options] ... The subcommands vary wildly by the nature of the command. However all subcommands could at least follow one style. The commands bundle, notes, stash and submodule have subcommands without two leading dashes (i.e. git stash list) as opposed to all other commands (i.e. git tag --list). Sounds familiar. E.g. here is a similar thread about a year ago. http://thread.gmane.org/gmane.comp.version-control.git/231376/focus=231478 Further discussions to make the plan more concrete is very much welcomed. 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: What's cooking in git.git (Jul 2014, #04; Tue, 22)
Philip Oakley philipoak...@iee.org writes: * po/everyday-doc (2014-01-27) 1 commit - Make 'git help everyday' work This may make the said command to emit something, but the source is not meant to be formatted into a manual pages to begin with, and also its contents are a bit stale. It may be a good first step in the right direction, but needs more work to at least get the mark-up right before public consumption. Will hold. It's not clear to me which bits of mark-up are 'wrong' and must be reworked, It's been too long since I wrote the above and I left it without updates (these comments are by default carried over from one issue to the next of What's cooking report, unless there is some development on the topic). Now I read the output (admittedly, I skimmed only the HTML version), I think the formatting / mark-up is fine. I at the same time found various command sequences used there are rather classical and there are better ways to do the same with modern tools, which still makes me feel hesitant to promote this document without updating its contents, 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: What's cooking in git.git (Jul 2014, #04; Tue, 22)
Karsten Blees karsten.bl...@gmail.com writes: Am 22.07.2014 23:44, schrieb Junio C Hamano: * sk/mingw-uni-fix-more (2014-07-21) 14 commits ... * sk/mingw-tests-workaround (2014-07-21) 6 commits ... Yes, I think both series are ready. Compiles with msysgit and MSVC (with NO_CURL=1). Thanks. With the version in pu, three tests fail. t7001 is fixed with a newer 'cp'. It seems that the only use of the copy symlinks as-is in that test are to move trash/submodule/.git to trash/.git/modules/submodule; as the longer-term direction is not to rely on symlinks in .git/ (and we have got rid of HEAD - refs/heads/master long time ago), perhaps we do not even want to have -P there? The other two are unrelated (introduced by nd/multiple-work-trees topic). * t1501-worktree: failed 1 As of 5bbcb072 setup.c: support multi-checkout repo setup Using $TRASH_DIRECTORY doesn't work on Windows. * t2026-prune-linked-checkouts: failed 1 As of 404a45f1 prune: strategies for linked checkouts Dito. * t7001-mv: failed 6 'cp -P' doesn't work due to outdated cp.exe. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] config.c: fix accuracy of line number in errors
If a callback returns a negative value to `git_config*()` family, they call `die()` while printing the line number and the file name. Currently the printed line number is off by one, thus printing the wrong line number. Make `linenr` point to the line we just parsed during the call to callback to get accurate line number in error messages. Discovered-by: Tanay Abhra tanay...@gmail.com Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr --- config.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 22971e9..6db8f97 100644 --- a/config.c +++ b/config.c @@ -244,6 +244,7 @@ static int get_next_char(void) cf-linenr++; if (c == EOF) { cf-eof = 1; + cf-linenr++; c = '\n'; } return c; @@ -319,6 +320,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) { int c; char *value; + int ret; /* Get the full name */ for (;;) { @@ -341,7 +343,15 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) if (!value) return -1; } - return fn(name-buf, value, data); + /* +* We already consumed the \n, but we need linenr to point to +* the line we just parsed during the call to fn to get +* accurate line number in error messages. +*/ + cf-linenr--; + ret = fn(name-buf, value, data); + cf-linenr++; + return ret; } static int get_extended_base_var(struct strbuf *name, int c) -- 1.9.0.GIT -- To unsubscribe from this list: send the line 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 6/7] config: add `git_die_config()` to the config-set API
Add `git_die_config` that dies printing the line number and the file name of the highest priority value for the configuration variable `key`. It has usage in non-callback based config value retrieval where we can raise an error and die if there is a semantic error. For example, if (!git_config_get_value(key, value)) { /* NULL values not allowed */ if (!value) git_config_die(key); else /* do work */ } Signed-off-by: Tanay Abhra tanay...@gmail.com --- Documentation/technical/api-config.txt | 5 + cache.h| 1 + config.c | 24 ++-- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 8a86e45..14571e7 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -150,6 +150,11 @@ as well as retrieval for the queried variable, including: Similar to `git_config_get_string`, but expands `~` or `~user` into the user's home directory when found at the beginning of the path. +`void git_die_config(const char *key)`:: + + Dies printing the line number and the file name of the highest + priority value for the configuration variable `key`. + See test-config.c for usage examples. Value Parsing Helpers diff --git a/cache.h b/cache.h index 2f63fd1..fc886c3 100644 --- a/cache.h +++ b/cache.h @@ -1380,6 +1380,7 @@ extern int git_config_get_bool(const char *key, int *dest); extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest); extern int git_config_get_maybe_bool(const char *key, int *dest); extern int git_config_get_pathname(const char *key, const char **dest); +extern void git_die_config(const char *key); extern int committer_ident_sufficiently_given(void); extern int author_ident_sufficiently_given(void); diff --git a/config.c b/config.c index 43a951f..f0c9805 100644 --- a/config.c +++ b/config.c @@ -1491,8 +1491,12 @@ const struct string_list *git_config_get_value_multi(const char *key) int git_config_get_string(const char *key, const char **dest) { + int ret; git_config_check_init(); - return git_configset_get_string(the_config_set, key, dest); + ret = git_configset_get_string(the_config_set, key, dest); + if (ret 0) + git_die_config(key); + return ret; } int git_config_get_int(const char *key, int *dest) @@ -1527,8 +1531,24 @@ int git_config_get_maybe_bool(const char *key, int *dest) int git_config_get_pathname(const char *key, const char **dest) { + int ret; git_config_check_init(); - return git_configset_get_pathname(the_config_set, key, dest); + ret = git_configset_get_pathname(the_config_set, key, dest); + if (ret 0) + git_die_config(key); + return ret; +} + +void git_die_config(const char *key) +{ + const struct string_list *strptr; + struct key_value_info *kv_info; + strptr = git_config_get_value_multi(key); + kv_info = strptr-items[strptr-nr - 1].util; + if (!kv_info-linenr) + die(unable to parse command-line config); + else + die(bad config file line %d in %s,kv_info-linenr, kv_info-filename); } /* -- 1.9.0.GIT -- To unsubscribe from this list: send the line 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 3/7] add a test for semantic errors in config files
Semantic errors (for example, for alias.* variables NULL values are not allowed) in configuration files cause a die printing the line number and file name of the offending value. Add a test documenting that such errors cause a die printing the accurate line number and file name. Signed-off-by: Tanay Abhra tanay...@gmail.com --- t/t1308-config-set.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 7fdf840..bd033df 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -197,4 +197,12 @@ test_expect_success 'proper error on error in custom config files' ' test_cmp expect actual ' +test_expect_success 'check line errors for malformed values' ' + cp .git/config .git/config.old + test_when_finished mv .git/config.old .git/config + echo [alias]\n br .git/config + test_expect_code 128 git br 2result + grep fatal: bad config file line 2 in .git/config result +' + test_done -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] Rewrite `git_config()` using config-set API
This series builds on the top of 5def4132 (ta/config-set) in pu or topic[1] in the mailing list with name git config cache special querying API utilizing the cache. This series aims to do these three things, * Use the config-set API to rewrite git_config(). * Solve any legacy bugs in the previous system while at it. * To be feature complete compared to the previous git_config() implementation, which I think it is now. (added the line number and file name info just for completeness) Also, I haven't yet checked the exact improvements but still as a teaser, git status now only rereads the configuration files twice instead of four times. [1]: http://thread.gmane.org/gmane.comp.version-control.git/253862 Tanay Abhra (7): Documentation/technical/api-config.txt | 5 ++ cache.h| 1 + config.c | 93 +++--- t/t1308-config-set.sh | 17 +++ test-config.c | 10 userdiff.c | 14 - 6 files changed, 131 insertions(+), 9 deletions(-) -- 1.9.0.GIT -- To unsubscribe from this list: send the line 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 7/7] Add tests for `git_config_get_string()`
Add tests for `git_config_get_string()`, check whether it dies printing the line number and the file name if an NULL value is retrieved for the given key. Signed-off-by: Tanay Abhra tanay...@gmail.com --- t/t1308-config-set.sh | 9 + test-config.c | 10 ++ 2 files changed, 19 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index bd033df..1cb453b 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -119,6 +119,15 @@ test_expect_success 'find integer value for a key' ' check_config get_int lamb.chop 65 ' +test_expect_success 'find string value for a key' ' + check_config get_string case.baz hask +' + +test_expect_success 'check line error when NULL string is queried' ' + test_expect_code 128 test-config get_string case.foo 2result + grep fatal: bad config file line 7 in .git/config result +' + test_expect_success 'find integer if value is non parse-able' ' check_config expect_code 128 get_int lamb.head ' diff --git a/test-config.c b/test-config.c index 9dd1b22..994741a 100644 --- a/test-config.c +++ b/test-config.c @@ -16,6 +16,8 @@ * * get_bool - print bool value for the entered key or die * + * get_string - print string value for the entered key or die + * * configset_get_value - returns value with the highest priority for the entered key * from a config_set constructed from files entered as arguments. * @@ -84,6 +86,14 @@ int main(int argc, char **argv) printf(Value not found for \%s\\n, argv[2]); goto exit1; } + } else if (argc == 3 !strcmp(argv[1], get_string)) { + if (!git_config_get_string(argv[2], v)) { + printf(%s\n, v); + goto exit0; + } else { + printf(Value not found for \%s\\n, argv[2]); + goto exit1; + } } else if (!strcmp(argv[1], configset_get_value)) { for (i = 3; i argc; i++) { int err; -- 1.9.0.GIT -- To unsubscribe from this list: send the line 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 4/7] add line number and file name info to `config_set`
Store file name and line number for each key-value pair in the cache. Use the information to print line number and file name in errors raised by `git_config()` which now uses the configuration files caching layer internally. Signed-off-by: Tanay Abhra tanay...@gmail.com --- config.c | 32 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/config.c b/config.c index a7fb9a4..43a951f 100644 --- a/config.c +++ b/config.c @@ -1237,9 +1237,15 @@ static int git_config_raw(config_fn_t fn, void *data) return git_config_with_options(fn, data, NULL, 1); } +struct key_value_info { + const char *filename; + int linenr; +}; + static int configset_iter(struct config_set *cs, config_fn_t fn, void *data) { int i; + struct key_value_info *kv_info; struct string_list *strptr; struct config_set_element *entry; struct hashmap_iter iter; @@ -1247,8 +1253,15 @@ static int configset_iter(struct config_set *cs, config_fn_t fn, void *data) while ((entry = hashmap_iter_next(iter))) { strptr = entry-value_list; for (i = 0; i strptr-nr; i++) { - if (fn(entry-key, strptr-items[i].string, data) 0) - die(bad config file line in (TODO: file/line info)); + if (fn(entry-key, strptr-items[i].string, data) 0) { + kv_info = strptr-items[i].util; + if (!kv_info-linenr) + die(unable to parse command-line config); + else + die(bad config file line %d in %s, + kv_info-linenr, + kv_info-filename); + } } } return 0; @@ -1287,6 +1300,8 @@ static struct config_set_element *configset_find_element(struct config_set *cs, static int configset_add_value(struct config_set *cs, const char *key, const char *value) { struct config_set_element *e; + struct key_value_info *kv_info = xmalloc(sizeof(*kv_info)); + struct string_list_item *si; e = configset_find_element(cs, key); /* * Since the keys are being fed by git_config*() callback mechanism, they @@ -1299,7 +1314,16 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha string_list_init(e-value_list, 1); hashmap_add(cs-config_hash, e); } - string_list_append_nodup(e-value_list, value ? xstrdup(value) : NULL); + si = string_list_append_nodup(e-value_list, value ? xstrdup(value) : NULL); + if (cf) { + kv_info-filename = strintern(cf-name); + kv_info-linenr = cf-linenr; + } else { + /* for values read from `git_config_from_parameters()` */ + kv_info-filename = strintern(parameter); + kv_info-linenr = 0; + } + si-util = kv_info; return 0; } @@ -1326,7 +1350,7 @@ void git_configset_clear(struct config_set *cs) hashmap_iter_init(cs-config_hash, iter); while ((entry = hashmap_iter_next(iter))) { free(entry-key); - string_list_clear(entry-value_list, 0); + string_list_clear(entry-value_list, 1); } hashmap_free(cs-config_hash, 1); cs-hash_initialized = 0; -- 1.9.0.GIT -- To unsubscribe from this list: send the line 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/7] rewrite git_config() to use the config-set API
Of all the functions in `git_config*()` family, `git_config()` has the most invocations in the whole code base. Each `git_config()` invocation causes config file rereads which can be avoided using the config-set API. Use the config-set API to rewrite `git_config()` to use the config caching layer to avoid config file rereads on each invocation during a git process lifetime. First invocation constructs the cache, and after that for each successive invocation, `git_config()` feeds values from the config cache instead of rereading the configuration files. Signed-off-by: Tanay Abhra tanay...@gmail.com --- config.c | 29 +++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/config.c b/config.c index 6db8f97..a7fb9a4 100644 --- a/config.c +++ b/config.c @@ -1232,11 +1232,36 @@ int git_config_with_options(config_fn_t fn, void *data, return ret; } -int git_config(config_fn_t fn, void *data) +static int git_config_raw(config_fn_t fn, void *data) { return git_config_with_options(fn, data, NULL, 1); } +static int configset_iter(struct config_set *cs, config_fn_t fn, void *data) +{ + int i; + struct string_list *strptr; + struct config_set_element *entry; + struct hashmap_iter iter; + hashmap_iter_init(cs-config_hash, iter); + while ((entry = hashmap_iter_next(iter))) { + strptr = entry-value_list; + for (i = 0; i strptr-nr; i++) { + if (fn(entry-key, strptr-items[i].string, data) 0) + die(bad config file line in (TODO: file/line info)); + } + } + return 0; +} + +static void git_config_check_init(void); + +int git_config(config_fn_t fn, void *data) +{ + git_config_check_init(); + return configset_iter(the_config_set, fn, data); +} + static struct config_set_element *configset_find_element(struct config_set *cs, const char *key) { struct config_set_element k; @@ -1418,7 +1443,7 @@ static void git_config_check_init(void) if (the_config_set.hash_initialized) return; git_configset_init(the_config_set); - git_config(config_set_callback, the_config_set); + git_config_raw(config_set_callback, the_config_set); } void git_config_clear(void) -- 1.9.0.GIT -- To unsubscribe from this list: send the line 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 5/7] enforce `xfuncname` precedence over `funcname`
t4018-diff-funcname.sh fails for the new `git_config()` which uses the configuration files caching layer internally. The test introduced in commit d64d6cdc checks that whether `xfuncname` takes precedence over `funcname` variable which was not guaranteed by config API previously and worked only because values were parsed and fed in order. The new `git_config()` only guarantees precedence order for variables with the same name. Also `funcname` variable is deprecated and not documented properly. `xfuncname` is mentioned in the docs and takes precedence over `funcname`. Instead of removing `funcname` variable, enforce `xfuncname` precedence over `funcname` when the variables have the same subsection. Remove dependency that required values to be fed to userdiff_config() in parsing order for the test to succeed. Signed-off-by: Tanay Abhra tanay...@gmail.com --- Note: this the only test that failed for the new git_config() rewrite. userdiff.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/userdiff.c b/userdiff.c index fad52d6..a51bc89 100644 --- a/userdiff.c +++ b/userdiff.c @@ -2,6 +2,7 @@ #include userdiff.h #include cache.h #include attr.h +#include string-list.h static struct userdiff_driver *drivers; static int ndrivers; @@ -211,9 +212,12 @@ int userdiff_config(const char *k, const char *v) struct userdiff_driver *drv; const char *name, *type; int namelen; + char *subsection = NULL; + static struct string_list xflag = STRING_LIST_INIT_DUP; if (parse_config_key(k, diff, name, namelen, type) || !name) return 0; + subsection = xstrndup(name, namelen); drv = userdiff_find_by_namelen(name, namelen); if (!drv) { @@ -224,10 +228,16 @@ int userdiff_config(const char *k, const char *v) drv-binary = -1; } - if (!strcmp(type, funcname)) + if (!strcmp(type, funcname) !unsorted_string_list_has_string(xflag, subsection)) { + free (subsection); return parse_funcname(drv-funcname, k, v, 0); - if (!strcmp(type, xfuncname)) + } + if (!strcmp(type, xfuncname)) { + string_list_append(xflag, subsection); + free (subsection); return parse_funcname(drv-funcname, k, v, REG_EXTENDED); + } + free(subsection); if (!strcmp(type, binary)) return parse_tristate(drv-binary, k, v); if (!strcmp(type, command)) -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: confused about remote branch management
On Wed, 2014-07-23 at 15:09 +0200, Kevin wrote: On Jul 23, 2014 5:11 AM, Ross Boylan r...@biostat.ucsf.edu wrote: My local master branch is the result of a merge of upstream master and some local changes. I want to merge in more recent upstream work. git pull doesn't seem to have updated origin/master, and git checkout origin/master also doesn't seem to work. git pull with two parameters in older versions will not update remote tracking branches. That's because the last parameter expects a refspec with a source and destination and you only specify a source. My command was git pull origin master so I think it has a source as well. Doing a git fetch will update them. I thought git pull = get fetch + git merge. Are you saying that if I issued those 2 commands separately the result would have been different? Ross ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git --version git version 1.7.10.4 Version 1.8.4 changes this behavior and will update the remote tracking branches. -- To unsubscribe from this list: send the line 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/RFC] sparse: avoid sse2 code which renders sparse useless
Commit 745224e0 (refs.c: SSE2 optimizations for check_refname_\ component, 18-06-2014) introduces (on x86_64) the use of sse2 code, and associated header files, to optimize some reference handling code. This causes sparse to implode and exit with too many errors, among other things, while attempting to parse the code contained in those headers. For example, the following shows just the last few messages: $ make abspath.sp ... /usr/lib/gcc/x86_64-pc-cygwin/4.8.3//include/mmintrin.h:724:38: error: attribute '__gnu_inline__': unknown attribute /usr/lib/gcc/x86_64-pc-cygwin/4.8.3//include/mmintrin.h:732:38: error: too many errors /usr/lib/gcc/x86_64-pc-cygwin/4.8.3//include/xmmintrin.h:91:34: error: constant 0.0f is not a valid number Makefile:2297: recipe for target 'abspath.sp' failed make: *** [abspath.sp] Error 1 $ The most numerous errors (about 100 for the above file) relate to the use of the __gnu_inline__ attribute. A simple 'one line' patch to sparse (actually it is three lines), can fix this up without too much problem. However, the final error above is not as simple (and quick) to fix. The code in question (.../xmmintrin.h:91), looks like this: return __extension__ (__m128){ 0.0f, 0.0f, 0.0f, 0.0f }; Until sparse learns to parse this gcc extension (if ever), we can avoid the issue by simply not attempting to parse this code. In order to do this, use the preprocessor symbol __CHECKER__, automatically defined by sparse, in the #if conditional already used to guard the code. Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- Hi Junio, I've been sitting on this patch for some time, while I try to gauge how long it would take to fix sparse to cope with this vectorised code. Unfortunately, it would probably require adding a considerable amount of code to add the same level of support for __SSE__, __SSE2__, __MMX__, etc, that gcc provides. This is marked RFC, because this would be the first use of __CHECKER__ in the git code-base. I have been cherry-picking this on top of any branch I want to check. At first this wasn't too much of a hassle, but now commit 745224e0 has progressed to master ... (last night I cherry-picked this patch approx a dozen times, so I was getting a little irritated! :D ). ATB, Ramsay Jones git-compat-util.h | 2 +- refs.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 26e92f1..1aae883 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -731,7 +731,7 @@ void git_qsort(void *base, size_t nmemb, size_t size, #endif #endif -#if defined(__GNUC__) defined(__x86_64__) +#if defined(__GNUC__) defined(__x86_64__) !defined(__CHECKER__) #include emmintrin.h /* * This is the system memory page size; it's used so that we can read diff --git a/refs.c b/refs.c index 84b9070..ffd4016 100644 --- a/refs.c +++ b/refs.c @@ -124,7 +124,7 @@ static int check_refname_format_bytewise(const char *refname, int flags) return 0; } -#if defined(__GNUC__) defined(__x86_64__) +#if defined(__GNUC__) defined(__x86_64__) !defined(__CHECKER__) #define SSE_VECTOR_BYTES 16 /* Vectorized version of check_refname_format. */ -- 2.0.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: confused about remote branch management
I still don't know what I need to do to update origin/master in my local repo. Regarding Kevin's suggestion, I just tried git fetch origin master. It seems to have made no difference, at least judging by git show origin/master. I'm assuming the commit it show is the head of the branch. For reasons given below, Chris's theory that there was a conflict also doesn't seem to apply. On Wed, 2014-07-23 at 19:40 +1200, Chris Packham wrote: On 23/07/14 14:49, Ross Boylan wrote: My local master branch is the result of a merge of upstream master and some local changes. I want to merge in more recent upstream work. git pull doesn't seem to have updated origin/master, and git checkout origin/master also doesn't seem to work. Here's some info that may be relevant. ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git remote -v origin https://github.com/emacs-ess/ESS.git (fetch) origin https://github.com/emacs-ess/ESS.git (push) personalhttps://github.com/RossBoylan/ESS.git (fetch) personalhttps://github.com/RossBoylan/ESS.git (push) # I think I originally cloned from what is now the personal remote # and switched names later so origin refers to upstream. ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git branch -v * master 8fa569c [ahead 340] merge from origin # 340 ahead of personal is plausible, but ahead from origin seems odd ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git --version git version 1.7.10.4 ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git branch -a * master remotes/origin/S+eldoc remotes/origin/gretl remotes/origin/linewise_callbacks remotes/origin/litprog remotes/origin/master remotes/origin/transmissions remotes/personal/HEAD - personal/master remotes/personal/S+eldoc remotes/personal/gretl remotes/personal/linewise_callbacks remotes/personal/litprog remotes/personal/master remotes/personal/transmissions ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git status # On branch master # Your branch is ahead of 'personal/master' by 340 commits. # nothing to commit (working directory clean) ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git checkout origin/master Note: checking out 'origin/master'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name HEAD is now at a33a2f9... Merge branch 'trunk' ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git checkout master Previous HEAD position was a33a2f9... Merge branch 'trunk' Switched to branch 'master' ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git pull origin master # [messages] Not committing merge; use 'git commit' to complete the merge. I think this is the relevant message. By doing a git pull you are asking to merge the branch 'master' from the remote 'origin' into the current branch (which happens to also be called 'master'). What I'm guessing is in # [messages] is something about a merge conflict that needs resolving before the merge can be completed. There are various ways to resolve the conflict but probably the easiest would be Here are the full deleted messages: remote: Counting objects: 388, done. remote: Compressing objects: 100% (159/159), done. remote: Total 356 (delta 257), reused 289 (delta 190) Receiving objects: 100% (356/356), 59.99 KiB, done. Resolving deltas: 100% (257/257), completed with 29 local objects. From https://github.com/emacs-ess/ESS * branchmaster - FETCH_HEAD error: Terminal is dumb, but EDITOR unset Not committing merge; use 'git commit' to complete the merge. So the lack of commit was not from a conflict, just it didn't know how to bring up an editor (I was in a bash session under emacs). [snip discussion of merging] ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git status # On branch master # Changes to be committed: # [long list] The list had only modified and added files; no conflicts. Ross ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git show origin/master commit a33a2f9e06185a225af7d72ea3896cfd260e455e Merge: 136742f 6b22a88 Author: Vitalie Spinu spinu...@gmail.com Date: Mon Jan 20 00:43:30 2014 -0800 Merge branch 'trunk' # this was the head of origin/master BEFORE I did the pull. # I think this means it has not been updated. ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git commit -m merge in upstream, probably fe7d609..8fa569c [master 59f6841] merge in upstream, probably fe7d609..8fa569c ross@tempserver:~/UCSF/Choi/GitHub/ESS$ git show origin/master -v # no change -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to
Re: rebase flattens history when it shouldn't?
Jonathan Nieder jrnie...@gmail.com writes: Hi Sergei, Sergei Organov wrote: --C-- / \ / M topic,HEAD / / A---B master shouldn't $ git rebase master be a no-op here? [...] I'd expect --force-rebase to be required for this to happen: -f, --force-rebase Force the rebase even if the current branch is a descendant of the commit you are rebasing onto. Normally non-interactive rebase will exit with the message Current branch is up to date in such a situation. [...] Do you think it's worth fixing? Thanks for a clear report. After a successful 'git rebase master', the current branch is always a linear string of patches on top of 'master'. The already up to date behavior when -f is not passed is in a certain sense an optimization --- it is about git noticing that 'git rebase' wouldn't have anything to do (except for touching timestamps) and therefore doing nothing. So I don't think requiring -f for this case would be an improvement. What actually bothers me is the unfortunate consequence that git pull is not always a no-op when nothing was changed at the origin since the last git pull. THIS is really surprising and probably should better be fixed. Requiring -f is just one (obvious) way to fix this. I do agree that the documentation is misleading. Any ideas for wording that could make it clearer? I can't suggest anything as I don't see why -f is there in the first place. What are use cases? -- Sergey. -- To unsubscribe from this list: send the line unsubscribe 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 1/3] completion: complete unstuck `git push --recurse-submodules`
On Tue, Jul 22, 2014 at 02:17:13PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Do you want me to re-roll with this change or can you replace the patch while applying? I think I had to flip the third one to adjust to the change I suggested to this; the result will be on 'pu', so please double check when I push it out. The result on jk/more-push-completion looks good. 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 0/7] Rewrite `git_config()` using config-set API
Tanay Abhra tanay...@gmail.com writes: This series builds on the top of 5def4132 (ta/config-set) in pu or topic[1] Not exactly: 5def4132 has been replaced in pu, and it does not contain tests, hence PATCH 3 does not apply on top of 5def4132. The series applies to 0912a24, but does not compile, since you use strintern which is in master but not in 0912a24. OK, applied and compiled. Let the review begin. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] rewrite git_config() to use the config-set API
Tanay Abhra tanay...@gmail.com writes: -int git_config(config_fn_t fn, void *data) +static int git_config_raw(config_fn_t fn, void *data) As you noticed already, this change breaks several tests. You are going to repair them later in the series, but your patch series produces a non-bisectable history. The history should pass tests at each commit. If needed, you can ensure that with eg. git rebase HEAD~7 --exec make cd t/ ./t1308-config-set.sh ./t4018-diff-funcname.sh -i (or --exec 'make test', but that takes really long) So, this patch should come later in the series (not hard, just a reordering with rebase -i). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] add a test for semantic errors in config files
Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'check line errors for malformed values' ' + cp .git/config .git/config.old + test_when_finished mv .git/config.old .git/config + echo [alias]\n br .git/config + test_expect_code 128 git br 2result + grep fatal: bad config file line 2 in .git/config result +' + The test fails at this point in history. I guess the problem will disapear once you've put PATCH 2 at the right point in the series. Another option is to mark the test as test_expect_failure when you introduce it, and change it to test_expect_success when you fix it (probably not applicable here, but it's a trick I find elegant). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] rewrite git_config() to use the config-set API
Tanay Abhra tanay...@gmail.com writes: +static int configset_iter(struct config_set *cs, config_fn_t fn, void *data) +{ + int i; + struct string_list *strptr; + struct config_set_element *entry; + struct hashmap_iter iter; + hashmap_iter_init(cs-config_hash, iter); + while ((entry = hashmap_iter_next(iter))) { + strptr = entry-value_list; + for (i = 0; i strptr-nr; i++) { + if (fn(entry-key, strptr-items[i].string, data) 0) + die(bad config file line in (TODO: file/line info)); One more reason to reorder (but that will actually be slightly more than rebase -i, you'll have a few conflicts to fix) is to avoid this TODO. Put the patch after the line number patch and you'll be able to provide the right information directly. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] prune --repos: fix uninitialized access
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: There's a code path in prune_repo_dir() that does not initialize 'st' buffer, which is checked by the caller, prune_repos_dir(). Instead of leaking some prune logic out to prune_repos_dir(), move 'st' into prune_repo_dir(). Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- The return value from the function used to be should .git/repos/$x be removed if it is stale enough? and now it is just should it be removed?, so the change makes sense. It does not explain what the change to the test is about, though. Thanks. builtin/prune.c | 16 ++-- t/t2026-prune-linked-checkouts.sh | 2 +- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/builtin/prune.c b/builtin/prune.c index 28b7adf..e72c391 100644 --- a/builtin/prune.c +++ b/builtin/prune.c @@ -112,8 +112,9 @@ static void prune_object_dir(const char *path) } } -static int prune_repo_dir(const char *id, struct stat *st, struct strbuf *reason) +static int prune_repo_dir(const char *id, struct strbuf *reason) { + struct stat st; char *path; int fd, len; @@ -123,26 +124,23 @@ static int prune_repo_dir(const char *id, struct stat *st, struct strbuf *reason } if (file_exists(git_path(repos/%s/locked, id))) return 0; - if (stat(git_path(repos/%s/gitdir, id), st)) { - st-st_mtime = expire; + if (stat(git_path(repos/%s/gitdir, id), st)) { strbuf_addf(reason, _(Removing repos/%s: gitdir file does not exist), id); return 1; } fd = open(git_path(repos/%s/gitdir, id), O_RDONLY); if (fd 0) { - st-st_mtime = expire; strbuf_addf(reason, _(Removing repos/%s: unable to read gitdir file (%s)), id, strerror(errno)); return 1; } - len = st-st_size; + len = st.st_size; path = xmalloc(len + 1); read_in_full(fd, path, len); close(fd); while (len (path[len - 1] == '\n' || path[len - 1] == '\r')) len--; if (!len) { - st-st_mtime = expire; strbuf_addf(reason, _(Removing repos/%s: invalid gitdir file), id); free(path); return 1; @@ -162,7 +160,7 @@ static int prune_repo_dir(const char *id, struct stat *st, struct strbuf *reason return 1; } free(path); - return 0; + return st.st_mtime = expire; } static void prune_repos_dir(void) @@ -172,15 +170,13 @@ static void prune_repos_dir(void) DIR *dir = opendir(git_path(repos)); struct dirent *d; int ret; - struct stat st; if (!dir) return; while ((d = readdir(dir)) != NULL) { if (!strcmp(d-d_name, .) || !strcmp(d-d_name, ..)) continue; strbuf_reset(reason); - if (!prune_repo_dir(d-d_name, st, reason) || - st.st_mtime expire) + if (!prune_repo_dir(d-d_name, reason)) continue; if (show_only || verbose) printf(%s\n, reason.buf); diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh index 4ccfa4e..79d84cb 100755 --- a/t/t2026-prune-linked-checkouts.sh +++ b/t/t2026-prune-linked-checkouts.sh @@ -77,7 +77,7 @@ test_expect_success 'not prune recent checkouts' ' mkdir zz mkdir -p .git/repos/jlm echo $TRASH_DIRECTORY/zz .git/repos/jlm/gitdir - git prune --repos --verbose + git prune --repos --verbose --expire=2.days.ago test -d .git/repos/jlm ' -- To unsubscribe from this list: send the line unsubscribe 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 5/7] enforce `xfuncname` precedence over `funcname`
Tanay Abhra tanay...@gmail.com writes: Also `funcname` variable is deprecated and not documented properly. I'd say purposely undocumented (see 45d9414fa5, Brandon Casey, Sep 18 2008). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] Add tests for `git_config_get_string()`
Tanay Abhra tanay...@gmail.com writes: Add tests for `git_config_get_string()`, check whether it dies printing the line number and the file name if an NULL a NULL (no n). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] enforce `xfuncname` precedence over `funcname`
On Wed, Jul 23, 2014 at 2:42 PM, Tanay Abhra tanay...@gmail.com wrote: t4018-diff-funcname.sh fails for the new `git_config()` which uses the configuration files caching layer internally. The test introduced in commit d64d6cdc checks that whether `xfuncname` takes s/that// precedence over `funcname` variable which was not guaranteed by config API previously and worked only because values were parsed and fed in order. The new `git_config()` only guarantees precedence order for variables with the s/\s+/ / same name. Also `funcname` variable is deprecated and not documented properly. `xfuncname` is mentioned in the docs and takes precedence over `funcname`. Instead of removing `funcname` variable, enforce `xfuncname` precedence over `funcname` when the variables have the same subsection. Remove dependency that required values to be fed to userdiff_config() in parsing order for the test to succeed. Signed-off-by: Tanay Abhra tanay...@gmail.com --- Note: this the only test that failed for the new git_config() rewrite. userdiff.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/userdiff.c b/userdiff.c index fad52d6..a51bc89 100644 --- a/userdiff.c +++ b/userdiff.c @@ -2,6 +2,7 @@ #include userdiff.h #include cache.h #include attr.h +#include string-list.h static struct userdiff_driver *drivers; static int ndrivers; @@ -211,9 +212,12 @@ int userdiff_config(const char *k, const char *v) struct userdiff_driver *drv; const char *name, *type; int namelen; + char *subsection = NULL; + static struct string_list xflag = STRING_LIST_INIT_DUP; if (parse_config_key(k, diff, name, namelen, type) || !name) return 0; + subsection = xstrndup(name, namelen); drv = userdiff_find_by_namelen(name, namelen); if (!drv) { @@ -224,10 +228,16 @@ int userdiff_config(const char *k, const char *v) drv-binary = -1; } - if (!strcmp(type, funcname)) + if (!strcmp(type, funcname) !unsorted_string_list_has_string(xflag, subsection)) { + free (subsection); return parse_funcname(drv-funcname, k, v, 0); - if (!strcmp(type, xfuncname)) + } + if (!strcmp(type, xfuncname)) { + string_list_append(xflag, subsection); + free (subsection); return parse_funcname(drv-funcname, k, v, REG_EXTENDED); + } + free(subsection); if (!strcmp(type, binary)) return parse_tristate(drv-binary, k, v); if (!strcmp(type, command)) -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe 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 15/15] refs.c: allow deleting refs with a broken sha1
On Wed, Jul 23, 2014 at 1:03 PM, Ronnie Sahlberg sahlb...@google.com wrote: Add (back?) support to make it possible to delete refs that are broken. Add a new flag REF_ALLOWBROKEN that can be passed to the functions to lock a ref. If this flag is set we allow locking the ref even if the ref points to a broken sha1. For example a sha1 that is created by : echo Broken ref .git/refs/heads/foo-broken-1 Use this flag when calling from branch.c dusing a ref delete so that we Presumably: s/dusing/doing/ only allow locking those broken refs IFF when called during a branch delete. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/branch.c | 6 -- refs.c | 10 +++--- refs.h | 3 ++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 5c95656..6d70037 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -236,6 +236,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, name = mkpathdup(fmt, bname.buf); target = resolve_ref_unsafe(name, sha1, RESOLVE_REF_ALLOW_BAD_NAME, flags); + if (!target (flags REF_ISBROKEN)) + target = name; if (!target || (!(flags REF_ISSYMREF) is_null_sha1(sha1))) { error(remote_branch @@ -245,14 +247,14 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, continue; } - if (!(flags REF_ISSYMREF) + if (!(flags (REF_ISSYMREF|REF_ISBROKEN)) check_branch_commit(bname.buf, name, sha1, head_rev, kinds, force)) { ret = 1; continue; } - if (delete_ref(name, sha1, REF_NODEREF)) { + if (delete_ref(name, sha1, REF_NODEREF|REF_ALLOWBROKEN)) { error(remote_branch ? _(Error deleting remote branch '%s') : _(Error deleting branch '%s'), diff --git a/refs.c b/refs.c index 0ead11f..2662ef6 100644 --- a/refs.c +++ b/refs.c @@ -2122,12 +2122,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, int resolve_flags; int missing = 0; int attempts_remaining = 3; - int bad_refname; + int bad_ref; lock = xcalloc(1, sizeof(struct ref_lock)); lock-lock_fd = -1; - bad_refname = check_refname_format(refname, REFNAME_ALLOW_ONELEVEL); + bad_ref = check_refname_format(refname, REFNAME_ALLOW_ONELEVEL); resolve_flags = RESOLVE_REF_ALLOW_BAD_NAME; if (mustexist) @@ -2150,6 +2150,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, refname = resolve_ref_unsafe(orig_refname, lock-old_sha1, resolve_flags, type); } + if (!refname (flags REF_ALLOWBROKEN) (type REF_ISBROKEN)) { + bad_ref = 1; + refname = orig_refname; + } if (type_p) *type_p = type; if (!refname) { @@ -2212,7 +2216,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, else unable_to_lock_index_die(ref_file, errno); } - if (bad_refname) + if (bad_ref) return lock; return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock; diff --git a/refs.h b/refs.h index 712fc32..0172f48 100644 --- a/refs.h +++ b/refs.h @@ -174,10 +174,11 @@ extern int peel_ref(const char *refname, unsigned char *sha1); * Flags controlling transaction_update_sha1(), transaction_create_sha1(), etc. * REF_NODEREF: act on the ref directly, instead of dereferencing * symbolic references. - * + * REF_ALLOWBROKEN: allow locking refs that are broken. * Flags = 0x100 are reserved for internal use. */ #define REF_NODEREF0x01 +#define REF_ALLOWBROKEN 0x02 /** Reads log for the value of ref during at_time. **/ extern int read_ref_at(const char *refname, unsigned long at_time, int cnt, -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line 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: [PATCH 5/5] environment.c: fix incorrect git_graft_file initialization
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: info/grafts should be part of the common repository when accessed from a linked checkout (iow $GIT_COMMON_DIR/info/grafts, not $GIT_DIR/info/grafts). git_path(info/grafts) returns correctly, even without this fix, because it detects that $GIT_GRAFT_FILE is not set, so it goes with the common rule: anything except sparse-checkout in 'info' belongs to common repo. But get_graft_file() would return a wrong value and that one is used for setting grafts up. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Thanks Eric for sharp eyes and Duy for a quick fix. Will queue all five. environment.c | 2 +- t/t0060-path-utils.sh | 1 + t/t2025-checkout-to.sh | 18 ++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/environment.c b/environment.c index 50ed40a..d5b0788 100644 --- a/environment.c +++ b/environment.c @@ -157,7 +157,7 @@ static void setup_git_env(void) objects, git_db_env); git_index_file = git_path_from_env(INDEX_ENVIRONMENT, git_dir, index, git_index_env); - git_graft_file = git_path_from_env(GRAFT_ENVIRONMENT, git_dir, + git_graft_file = git_path_from_env(GRAFT_ENVIRONMENT, git_common_dir, info/grafts, git_graft_env); if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT)) check_replace_refs = 0; diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index da82aab..93605f4 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -269,6 +269,7 @@ test_git_path GIT_COMMON_DIR=bar logs/HEAD .git/logs/HEAD test_git_path GIT_COMMON_DIR=bar objects bar/objects test_git_path GIT_COMMON_DIR=bar objects/bar bar/objects/bar test_git_path GIT_COMMON_DIR=bar info/exclude bar/info/exclude +test_git_path GIT_COMMON_DIR=bar info/grafts bar/info/grafts test_git_path GIT_COMMON_DIR=bar info/sparse-checkout .git/info/sparse-checkout test_git_path GIT_COMMON_DIR=bar remotes/bar bar/remotes/bar test_git_path GIT_COMMON_DIR=bar branches/bar bar/branches/bar diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh index 8a00310..508993f 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-checkout-to.sh @@ -81,4 +81,22 @@ test_expect_success 'checkout from a bare repo without --to' ' ) ' +test_expect_success 'checkout with grafts' ' + test_when_finished rm .git/info/grafts + test_commit abc + SHA1=`git rev-parse HEAD` + test_commit def + test_commit xyz + echo `git rev-parse HEAD` $SHA1 .git/info/grafts + cat expected -\EOF + xyz + abc + EOF + git log --format=%s -2 actual + test_cmp expected actual + git checkout --detach --to grafted master + git --git-dir=grafted/.git log --format=%s -2 actual + test_cmp expected 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: Test failure in t9814-git-p4-rename.sh - my environment or bad test?
On Mon, Jul 7, 2014 at 11:14 PM, Junio C Hamano gits...@pobox.com wrote: Choosing any of these as the copy source is fine is a sensible way to fix the problem with this test. Would it be a better solution to avoid having multiple/ambiguous copy source candidates in the first place, by the way? I agree that this would be the best solution, although I feel more confident making a less invasive change first. Thanks, Christoph -- To unsubscribe from this list: send the line 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 p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?
The scenario in the rename test makes unnecessary assumptions about which file git file-tree will detect as a source for a copy-operations. Furthermore, copy detection is not tested by checking the resulting perforce revision history via p4 filelog, but via git diff-tree. This patch makes the test more robust by accepting each of the possible sources, and more rigorous by doing so via p4 filelog. --- t/t9814-git-p4-rename.sh | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh index 1fc1f5f..4068510 100755 --- a/t/t9814-git-p4-rename.sh +++ b/t/t9814-git-p4-rename.sh @@ -156,18 +156,16 @@ test_expect_success 'detect copies' ' git diff-tree -r -C HEAD git p4 submit p4 filelog //depot/file10 - p4 filelog //depot/file10 | grep -q branch from //depot/file + p4 filelog //depot/file10 | grep -q branch from //depot/file2 cp file2 file11 git add file11 git commit -a -m Copy file2 to file11 git diff-tree -r -C --find-copies-harder HEAD - src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) - test $src = file10 git config git-p4.detectCopiesHarder true git p4 submit p4 filelog //depot/file11 - p4 filelog //depot/file11 | grep -q branch from //depot/file + p4 filelog //depot/file11 | grep -q -E branch from //depot/file(2|10) cp file2 file12 echo some text file12 @@ -177,7 +175,7 @@ test_expect_success 'detect copies' ' level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d -f5 | sed s/C0*//) test -n $level test $level -gt 0 test $level -lt 98 src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) - test $src = file10 || test $src = file11 + test $src = file2 || test $src = file10 || test $src = file11 git config git-p4.detectCopies $(($level + 2)) git p4 submit p4 filelog //depot/file12 @@ -190,12 +188,10 @@ test_expect_success 'detect copies' ' git diff-tree -r -C --find-copies-harder HEAD level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d -f5 | sed s/C0*//) test -n $level test $level -gt 2 test $level -lt 100 - src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) - test $src = file10 || test $src = file11 || test $src = file12 git config git-p4.detectCopies $(($level - 2)) git p4 submit p4 filelog //depot/file13 - p4 filelog //depot/file13 | grep -q branch from //depot/file + p4 filelog //depot/file13 | grep -q -E branch from //depot/file(2|10|11|12) ) ' -- 2.0.1 On Mon, Jul 7, 2014 at 3:10 AM, Pete Wyckoff p...@padd.com wrote: ml.christophbon...@gmail.com wrote on Sun, 06 Jul 2014 16:32 +0200: I'm trying to get the git p4 tests to pass on my machine (OS X Mavericks) from master before making some changes. I'm experiencing a test failure in detect copies of the rename test. The test creates file2 with some content, creates a few copies (each with a commit), then does the following (no git write operations omitted): echo file2 file2 cp file2 file10 git add file2 file10 git commit -a -m Modify and copy file2 to file10 ... (some non-write-operations) ... cp file2 file11 git add file11 git commit -a -m Copy file2 to file11 git diff-tree -r -C --find-copies-harder HEAD src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) test $src = file10 This is where it fails on my machine. The git diff-tree output is this :100644 100644 22a35c17c4c0779f75142036beef6ccd58525b9c 22a35c17c4c0779f75142036beef6ccd58525b9c C100 file2 file11 so git diff-tree sees file2 as the copy source, not file10. In my opinion, the diff-tree result is legitimate (at that point, file2, file10 and file11 are identical). Later in the tests, after making more copies of file2, the conditions are more flexible, e.g. test $src = file10 || test $src = file11 || test $src = file12 IMO, the test discounts the legitimate possibility of diff-tree detecting file2 as source, making unnecessary assumptions about implementation details. Is this correct, or do I misunderstand the workings of diff-tree? I'd be grateful for advice, both on whether this is a bug, and if so, which branch to base a patch on. I think your analysis is correct. And I agree that later tests have noticed this ambiguity and added multiple comparisons like you quote. I'm not sure how to robustify this. At least doing the multiple comparisons should make the tests work again. The goal of this series of tests is to make sure that copy detection is working, not to verify that the correct copy choice was made. That should be in other (non-p4) tests. Do send patches based on Junio's master. I can ack, and they'll show up in a future git release. Thanks! -- Pete -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH v2 1/3] completion: complete unstuck `git push --recurse-submodules`
John Keeping j...@keeping.me.uk writes: On Tue, Jul 22, 2014 at 02:17:13PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Do you want me to re-roll with this change or can you replace the patch while applying? I think I had to flip the third one to adjust to the change I suggested to this; the result will be on 'pu', so please double check when I push it out. The result on jk/more-push-completion looks good. Thanks. 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: confused about remote branch management
Ross Boylan r...@biostat.ucsf.edu writes: I still don't know what I need to do to update origin/master in my local repo. Regarding Kevin's suggestion, I just tried git fetch origin master. I think Kevin's suggestion was 'To older git, git fetch origin master tells it to fetch master without updating origin/master, so it is understandable that your origin/master was not molested'. Either git fetch origin master:refs/remotes/origin/master or if you want to be more explicit and unambiguous: git fetch origin refs/heads/master:refs/remotes/origin/master should work on all versions of git. -- To unsubscribe from this list: send the line unsubscribe 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/7] Rewrite `git_config()` using config-set API
Tanay Abhra tanay...@gmail.com writes: This series builds on the top of 5def4132 (ta/config-set) in pu or topic[1] in the mailing list with name git config cache special querying API utilizing the cache. This series aims to do these three things, * Use the config-set API to rewrite git_config(). * Solve any legacy bugs in the previous system while at it. * To be feature complete compared to the previous git_config() implementation, which I think it is now. (added the line number and file name info just for completeness) Sounds promising. Are you done with the original series, or do you still want to fix the const-ness issue with the string pointer before working on follow-up topics like this one? -- To unsubscribe from this list: send the line unsubscribe 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/7] config.c: fix accuracy of line number in errors
Tanay Abhra tanay...@gmail.com writes: If a callback returns a negative value to `git_config*()` family, they call `die()` while printing the line number and the file name. Currently the printed line number is off by one, thus printing the wrong line number. Make `linenr` point to the line we just parsed during the call to callback to get accurate line number in error messages. Discovered-by: Tanay Abhra tanay...@gmail.com Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr Thanks. I am not sure what to read in these two lines. Was the fix done by you or Matthieu? --- config.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 22971e9..6db8f97 100644 --- a/config.c +++ b/config.c @@ -244,6 +244,7 @@ static int get_next_char(void) cf-linenr++; if (c == EOF) { cf-eof = 1; + cf-linenr++; c = '\n'; } return c; @@ -319,6 +320,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) { int c; char *value; + int ret; /* Get the full name */ for (;;) { @@ -341,7 +343,15 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) if (!value) return -1; } - return fn(name-buf, value, data); + /* + * We already consumed the \n, but we need linenr to point to + * the line we just parsed during the call to fn to get + * accurate line number in error messages. + */ + cf-linenr--; + ret = fn(name-buf, value, data); + cf-linenr++; + return ret; } static int get_extended_base_var(struct strbuf *name, int c) -- To unsubscribe from this list: send the line unsubscribe 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/7] rewrite git_config() to use the config-set API
Tanay Abhra tanay...@gmail.com writes: Of all the functions in `git_config*()` family, `git_config()` has the most invocations in the whole code base. Each `git_config()` invocation causes config file rereads which can be avoided using the config-set API. Use the config-set API to rewrite `git_config()` to use the config caching layer to avoid config file rereads on each invocation during a git process lifetime. First invocation constructs the cache, and after that for each successive invocation, `git_config()` feeds values from the config cache instead of rereading the configuration files. Signed-off-by: Tanay Abhra tanay...@gmail.com --- config.c | 29 +++-- 1 file changed, 27 insertions(+), 2 deletions(-) This is the natural logical conclusion ;-) diff --git a/config.c b/config.c index 6db8f97..a7fb9a4 100644 --- a/config.c +++ b/config.c @@ -1232,11 +1232,36 @@ int git_config_with_options(config_fn_t fn, void *data, return ret; } -int git_config(config_fn_t fn, void *data) +static int git_config_raw(config_fn_t fn, void *data) { return git_config_with_options(fn, data, NULL, 1); } +static int configset_iter(struct config_set *cs, config_fn_t fn, void *data) +{ + int i; + struct string_list *strptr; We know string_list would hold strings, so naming the variable strptr does not give us much information. As this is a list of values you would get by querying for a variable (or key), perhaps name it values or something? + struct config_set_element *entry; + struct hashmap_iter iter; Have a blank line between the local variable definitions (above) and the first executable statement (below); it would make it easier to read, especially because out codebase do not have decl-after-stmt. + hashmap_iter_init(cs-config_hash, iter); + while ((entry = hashmap_iter_next(iter))) { + strptr = entry-value_list; + for (i = 0; i strptr-nr; i++) { + if (fn(entry-key, strptr-items[i].string, data) 0) + die(bad config file line in (TODO: file/line info)); + } + } + return 0; +} + +static void git_config_check_init(void); + +int git_config(config_fn_t fn, void *data) +{ + git_config_check_init(); + return configset_iter(the_config_set, fn, data); +} Perhaps if you define this function at the right place in the file you do not have to add an forward decl of git_config_check_init()? static struct config_set_element *configset_find_element(struct config_set *cs, const char *key) { struct config_set_element k; @@ -1418,7 +1443,7 @@ static void git_config_check_init(void) if (the_config_set.hash_initialized) return; git_configset_init(the_config_set); - git_config(config_set_callback, the_config_set); + git_config_raw(config_set_callback, the_config_set); } void git_config_clear(void) -- To unsubscribe from this list: send the line unsubscribe 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-svn: doublecheck if really file or dir
Andrej Manduch amand...@gmail.com wrote: * this fixes 'git svn info `pwd`' buggy behaviour Good catch, the commit could use a better description, something like: --- 8 Subject: [PATCH] git-svn: info checks for dirs more carefully This avoids a Reading from filehandle failed at ... error when running git svn info `pwd`. Signed-off-by: Andrej Manduch amand...@gmail.com --- 8 While your patch avoids an error, but the output isn't right, either. I tried it running in /home/ew/ruby, the URL field is bogus: ~/ruby$ git svn info `pwd` Path: /home/ew/ruby URL: svn+ssh://svn.ruby-lang.org/ruby/trunk/home/ew/ruby Repository Root: svn+ssh://svn.ruby-lang.org/ruby Repository UUID: b2dd03c8-39d4-4d8f-98ff-823fe69b080e Revision: 46901 Node Kind: directory Schedule: normal Last Changed Author: hsbt Last Changed Rev: 46901 Last Changed Date: 2014-07-22 19:06:12 + (Tue, 22 Jul 2014) The URL should be: URL: svn+ssh://svn.ruby-lang.org/ruby/trunk It's better than an error, but it'd be nice if someone who uses this command can fix it (*hint* :). --- a/git-svn.perl +++ b/git-svn.perl @@ -2029,7 +2029,7 @@ sub find_file_type_and_diff_status { my $mode = (split(' ', $ls_tree))[0] || ; return (link, $diff_status) if $mode eq 12; - return (dir, $diff_status) if $mode eq 04; + return (dir, $diff_status) if $mode eq 04 or -d $path; or has a lower precedence than ||, so I would do the following: return (dir, $diff_status) if $mode eq 04 || -d $path; The general rule I've learned is to use || for conditionals and or for control flow (e.g. do_something() or die(...) ). I can take your patch with the above changes (no need to resend), but I'd be happier to see the URL field corrected if you want to reroll. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] add a test for semantic errors in config files
Matthieu Moy matthieu@grenoble-inp.fr writes: Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'check line errors for malformed values' ' +cp .git/config .git/config.old Should this be mv not cp? You will be overwriting the file from scratch in the later part of this test. +test_when_finished mv .git/config.old .git/config +echo [alias]\n br .git/config Is the use of \n portable? Another option is to mark the test as test_expect_failure when you introduce it, and change it to test_expect_success when you fix it (probably not applicable here, but it's a trick I find elegant). Yes, I agree that it is a good practice to document an existing breakage in an early patch #1, and then make a fix and flip expect-failure to expect-success in the patch #2. Breaking the code and documenting the breakage by expecting a failure in one patch, and then later fixing the breakage and flipping the expectation in another patch, is a bit less nice, 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: [PATCH 4/7] add line number and file name info to `config_set`
Tanay Abhra tanay...@gmail.com writes: @@ -1287,6 +1300,8 @@ static struct config_set_element *configset_find_element(struct config_set *cs, static int configset_add_value(struct config_set *cs, const char *key, const char *value) { struct config_set_element *e; + struct key_value_info *kv_info = xmalloc(sizeof(*kv_info)); + struct string_list_item *si; I have this suspicion that we may want to eventually build a custom config_value_list API that is very similar to what string_list does, with the only difference from string_list being that the util item of config_value_item (i.e. a parallel to string_list_item) would not be a void * that points at anything, but is struct key_value_info embedded within, so that we do not have to waste a pointer and fragmented allocation. I suspect such a config_value_list API must be built on top of a kind of generics which C does not allow, which would mean we would be doing some preprocessor macro tricks (similar to the way how commit-slab.h allows different kinds of payload) that lets us build a templated string-list API, discarding the existing string-list.[ch] and replacing them with something like these two liners: declare_generic_string_list(string_list, void *); /* in string-list.h */ define_generic_string_list(string_list, void *); /* in string-list.c */ And at that point, declare_generic_string_list(config_value_list, struct key_value); define_generic_string_list(config_value_list, struct key_value); would give us an API declaration and implementation that parallel that of string-list, but with struct key_value in the util field of each item. But that kind of clean-up can come much later after this series settles, and what this patch has is fine for now. -- To unsubscribe from this list: send the line unsubscribe 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-svn: Initialize SVN::Client with svn config instead of, auth for git svn branch.
Monard Vong travelingsou...@gmail.com wrote: If a client certificate is required to connect to svn, git svn branch always prompt the user for the certificate location and password, even though those parameters are stored in svn file server located in svn config dir (generally ~/.subversion). On the opposite, git svn info/init/dcommit read the config dir and do not prompt if the parameters are set. This commit initializes for git svn branch, the SVN::Client with the 'config' option instead of 'auth'. As decribed in the SVN documentation, http://search.cpan.org/~mschwern/Alien-SVN-v1.7.17.1/src/subversion/subversion/bindings/swig/perl/native/Client.pm#METHODS the SVN::Client will then read cached authentication options. Signed-off-by: Monard Vong travelingsou...@gmail.com Thanks, I do not have a good way to test this, but the idea seems correct. Your patch is whitespace mangled, and the commit message and subject needs to be improved (see Documentation/SubmittingPatches on how to describe your changes and how to send them without whitespace mangling.) Thanks again. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] enforce `xfuncname` precedence over `funcname`
Tanay Abhra tanay...@gmail.com writes: t4018-diff-funcname.sh fails for the new `git_config()` which uses the configuration files caching layer internally. The test introduced in commit d64d6cdc checks that whether `xfuncname` takes precedence over `funcname` variable which was not guaranteed by config API previously and worked only because values were parsed and fed in order. The above is hard to understand. Do you mean that the old code and test used whichever (between funcname and xfuncname) appeared last in the configuration file? For example, if I had diff.foo.xfuncname Beer$ in my ~/.gitconfig and then for a particular project I wanted to use a custom pattern in its .git/config but it was sufficient to define the pattern without using extended regexp, I would be tempted to say diff.foo.funcname Wine$ With the last one of xfuncname or funcname wins rule, I do not have to worry about having xfuncname in ~/.gitconfig when I do so, but with xfuncname is stronger than funcname rule, it suddenly forces me to go back and check if I have something stronger in my lower-precedence configuration file (i.e. ~/.gitconfig) when I am trying to override with a higher-precedence configuration file (i.e. the project-specific .git/config). If that is what this patch is trying to change, I am not sure if I agree that is a good change. They are two ways to spell the same information, and I would find it more natural if a later funcname spelled in BRE allowed me to override an earlier xfuncname spelled in ERE. -- To unsubscribe from this list: send the line unsubscribe 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 01/15] refs.c make ref_transaction_create a wrapper to ref_transaction_update
Ronnie Sahlberg sahlb...@google.com writes: Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 18 ++ refs.h | 7 --- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/refs.c b/refs.c index 6dcb920..8f2aa3a 100644 --- a/refs.c +++ b/refs.c @@ -3490,28 +3490,14 @@ int ref_transaction_create(struct ref_transaction *transaction, int flags, const char *msg, struct strbuf *err) { - struct ref_update *update; - if (transaction-state != REF_TRANSACTION_OPEN) die(BUG: create called for transaction that is not open); if (!new_sha1 || is_null_sha1(new_sha1)) die(BUG: create ref with null new_sha1); - if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) { - strbuf_addf(err, Bad refname: %s, refname); - return -1; - } - - update = add_update(transaction, refname); - - hashcpy(update-new_sha1, new_sha1); - hashclr(update-old_sha1); - update-flags = flags; - update-have_old = 1; - if (msg) - update-msg = xstrdup(msg); - return 0; + return ref_transaction_update(transaction, refname, new_sha1, + null_sha1, flags, 1, msg, err); } Makes sense, but at the same time makes me wonder why no code is moved to ref_transaction_update() while removing code from here, which would only mean that code in ref_transaction_update() was added redundantly in the first place. An ideal series would have had only update code in _update() when the function is added, and later with a patch like this would lose code from _create() while adding some code to _update(), I would think. Or if all the code in _update() was necessary from day one, then perhaps this change should have been part of the same patch. It's not a big deal either way, though. Thanks. int ref_transaction_delete(struct ref_transaction *transaction, diff --git a/refs.h b/refs.h index b0476c1..1c08cfd 100644 --- a/refs.h +++ b/refs.h @@ -276,9 +276,10 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); /* * Add a reference update to transaction. new_sha1 is the value that * the reference should have after the update, or zeros if it should - * be deleted. If have_old is true, then old_sha1 holds the value - * that the reference should have had before the update, or zeros if - * it must not have existed beforehand. + * be deleted. If have_old is true and old_sha is not the null_sha1 + * then the previous value of the ref must match or the update will fail. + * If have_old is true and old_sha1 is the null_sha1 then the ref must not + * already exist and a new ref will be created with new_sha1. * Function returns 0 on success and non-zero on failure. A failure to update * means that the transaction as a whole has failed and will need to be * rolled back. -- To unsubscribe from this list: send the line unsubscribe 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 p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?
ml.christophbon...@gmail.com wrote on Wed, 23 Jul 2014 23:28 +0200: The scenario in the rename test makes unnecessary assumptions about which file git file-tree will detect as a source for a copy-operations. Furthermore, copy detection is not tested by checking the resulting perforce revision history via p4 filelog, but via git diff-tree. This patch makes the test more robust by accepting each of the possible sources, and more rigorous by doing so via p4 filelog. I see what you're doing here, and it all looks good. The diff-tree checks were mainly debugging, and if p4 filelog shows the right branch from, that means it works. You might be able to firm up the branch from lines for file8 and file9 too, but those aren't likely to fail. Indeed, as noted in the other thread, it would be better to make these not so flaky, but your change here fixes a problem, and doesn't do any harm. And gives you an opportunity to fix it more later. :) Be sure to fix the word-wrapping you have on two of the lines below. And be careful not to top post. Here's my ack for when you decide to send it back to the list, cc junio. Acked-by: Pete Wyckoff p...@padd.com --- t/t9814-git-p4-rename.sh | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh index 1fc1f5f..4068510 100755 --- a/t/t9814-git-p4-rename.sh +++ b/t/t9814-git-p4-rename.sh @@ -156,18 +156,16 @@ test_expect_success 'detect copies' ' git diff-tree -r -C HEAD git p4 submit p4 filelog //depot/file10 - p4 filelog //depot/file10 | grep -q branch from //depot/file + p4 filelog //depot/file10 | grep -q branch from //depot/file2 cp file2 file11 git add file11 git commit -a -m Copy file2 to file11 git diff-tree -r -C --find-copies-harder HEAD - src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) - test $src = file10 git config git-p4.detectCopiesHarder true git p4 submit p4 filelog //depot/file11 - p4 filelog //depot/file11 | grep -q branch from //depot/file + p4 filelog //depot/file11 | grep -q -E branch from //depot/file(2|10) cp file2 file12 echo some text file12 @@ -177,7 +175,7 @@ test_expect_success 'detect copies' ' level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d -f5 | sed s/C0*//) test -n $level test $level -gt 0 test $level -lt 98 src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) - test $src = file10 || test $src = file11 + test $src = file2 || test $src = file10 || test $src = file11 git config git-p4.detectCopies $(($level + 2)) git p4 submit p4 filelog //depot/file12 @@ -190,12 +188,10 @@ test_expect_success 'detect copies' ' git diff-tree -r -C --find-copies-harder HEAD level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | cut -d -f5 | sed s/C0*//) test -n $level test $level -gt 2 test $level -lt 100 - src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) - test $src = file10 || test $src = file11 || test $src = file12 git config git-p4.detectCopies $(($level - 2)) git p4 submit p4 filelog //depot/file13 - p4 filelog //depot/file13 | grep -q branch from //depot/file + p4 filelog //depot/file13 | grep -q -E branch from //depot/file(2|10|11|12) ) ' -- 2.0.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 00/15] ref-transactions for reflogs
Ronnie Sahlberg sahlb...@google.com writes: This is the next patch series for ref-transactions. It is also available at https://github.com/rsahlberg/git/tree/ref-transactions-reflog and is the same patch series that has been posted previously with one exception: I've received, queued and started reading it, but it has fairly heavy interactions with other topics in flight when merged to 'pu', so today's integration result will not have this topic anywhere. https://github.com/gitster/git/ is a mirror of my primary working space and rs/ref-transaction-reflog topic can be seen there, 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: confused about remote branch management
On Wed, 2014-07-23 at 14:41 -0700, Junio C Hamano wrote: Ross Boylan r...@biostat.ucsf.edu writes: I still don't know what I need to do to update origin/master in my local repo. Regarding Kevin's suggestion, I just tried git fetch origin master. I think Kevin's suggestion was 'To older git, git fetch origin master tells it to fetch master without updating origin/master, so it is understandable that your origin/master was not molested'. Either git fetch origin master:refs/remotes/origin/master Great; that works. Is that procedure supposed to be the usual way I track upstream in this (1.7) version of git? It seems arcane. I had thought the usual workflow was supposed to be one of 2 alternatives, either git checkout remotes/origin/master git pull origin master git checkout master git merge remotes/origin/master But that failed on the first step. Or # assuming we are on the local master branch git fetch origin master # and everything is updated. Is the problem that I called the local branch with my modifications master instead of something else? or if you want to be more explicit and unambiguous: git fetch origin refs/heads/master:refs/remotes/origin/master should work on all versions of git. After studying man git-fetch's discussion of the refspec parameter, especially the second note: You never do your own development on branches that appear on the right hand side of a refspec colon on Pull: lines; they are to be updated by git fetch. If you intend to do development derived from a remote branch B, have a Pull: line to track it (i.e. Pull: B:remote-B), and have a separate branch my-B to do your development on top of it. The latter is created by git branch my-B remote-B (or its equivalent git checkout -b my-B remote-B). Run git fetch to keep track of the progress of the remote side, and when you see something new on the remote branch, merge it into your development branch with git pull . remote-B, while you are on my-B branch. I'm even more confused. Is Pull: lines a reference to log messages during the fetch, a configuration file, or something else? The docs refer to a pull: line in $GIT_DIR/remotes, but I have no such directory. I do have .git/config, which includes [remote personal] url = https://github.com/RossBoylan/ESS.git fetch = +refs/heads/*:refs/remotes/personal/* [branch master] remote = personal merge = refs/heads/master [remote origin] url = https://github.com/emacs-ess/ESS.git fetch = +refs/heads/*:refs/remotes/origin/* Ah! branch master is associated with the personal remote; is that why updating it from origin's master branch has no effect on origin/master? I also don't know what the . in git pull . remote-B does; the git-pull manpage doesn't indicate it's legal as far as I can see. Ross -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: confused about remote branch management
Ross Boylan r...@biostat.ucsf.edu writes: Either git fetch origin master:refs/remotes/origin/master Great; that works. Is that procedure supposed to be the usual way I track upstream in this (1.7) version of git? It seems arcane. No, and no. The command is designed so that most of the time you can just say git fetchENTER without anything extra, which will let the configured remote.*.fetch to kick in as the default refspec to slurp updates to all the branches. This is because the branches of a single project are supposed to be related, and a single git fetch goes over a single network connection, establishment of which is expected to be a large overhead. Letting a single invocation of fetch to slurp updates to _all_ the branches is supposed not to be too much overhead over grabbing updates to everything (let alone invoking a git fetch per each individual branch), and is the normal mode of operation. A single-shot git fetch origin master to explicitly decline following of everything other than 'master' *is* the special case. And it was a very conscious design decision not to molest the remote tracking branch when this kind of explicit command line request is made, so that you do not lose track of what commit you _saw_ before you ran the command. That way git log origin/master..FETCH_HEAD can be used to inspect what got changed since you fetched last time. Over the years, with reflogs enabled for everybody, preserving the remote tracking branches when the user does not explicitly ask to store the result has become much less important. For this reason, modern Git applies, when it sees git fetch origin master, the configured remote.*.fetch as refmap to map the name master, i.e. the only branch you are fetching, to the remote tracking branch you use to store the result, i.e. refs/remotes/origin/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: confused about remote branch management
On Wed, 2014-07-23 at 16:51 -0700, Junio C Hamano wrote: Ross Boylan r...@biostat.ucsf.edu writes: Either git fetch origin master:refs/remotes/origin/master Great; that works. Is that procedure supposed to be the usual way I track upstream in this (1.7) version of git? It seems arcane. No, and no. Good. How should I handle getting updates from origin? The command is designed so that most of the time you can just say git fetchENTER without anything extra, which will let the configured remote.*.fetch to kick in as the default refspec to slurp updates to all the branches. This is because the branches of a single project are supposed to be related, and a single git fetch goes over a single network connection, establishment of which is expected to be a large overhead. Letting a single invocation of fetch to slurp updates to _all_ the branches is supposed not to be too much overhead over grabbing updates to everything (let alone invoking a git fetch per each individual branch), and is the normal mode of operation. A single-shot git fetch origin master to explicitly decline following of everything other than 'master' *is* the special case. And it was a very conscious design decision not to molest the remote tracking branch when this kind of explicit command line request is made, so that you do not lose track of what commit you _saw_ before you ran the command. That way git log origin/master..FETCH_HEAD can be used to inspect what got changed since you fetched last time. Over the years, with reflogs enabled for everybody, preserving the remote tracking branches when the user does not explicitly ask to store the result has become much less important. For this reason, modern Git applies, when it sees git fetch origin master, the configured remote.*.fetch as refmap to map the name master, i.e. the only branch you are fetching, to the remote tracking branch you use to store the result, i.e. refs/remotes/origin/master. For this case I think get fetch will attempt to retrieve from the personal remote. Will get fetch origin (with no other arguments) update all the branches in origin, updating the remote tracking branches, particularly in git 1.7? When I tried git fetch origin nothing happened (it returned immediately with no messages and git branch -v -a showed the same heads as before). It's quite possible none of the other branches have changed since I last got them, so I don't think the exercise proves much. Ross -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] coverity mixed bag
Since Stefan has recently started feeding git builds to coverity, I spent a few minutes poking through the results. There are tons of false positives, so there is some work to be done there with tweaking our coverity models. But there are some real issues, too. Here are fixes for the handful that I looked at. [1/5]: receive-pack: don't copy dir parameter [2/5]: free ref string returned by dwim_ref [3/5]: transport: fix leaks in refs_from_alternate_cb [4/5]: fix memory leak parsing core.commentchar [5/5]: apply: avoid possible bogus pointer -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] receive-pack: don't copy dir parameter
We used to do this so could pass a mutable string to enter_repo. But since 1c64b48 (enter_repo: do not modify input, 2011-10-04), this is not necessary. The resulting code is simpler, and it fixes a minor leak. Signed-off-by: Jeff King p...@peff.net --- If you are wondering whether upload-pack needs the same treatment, we already did it in 6379dd0. builtin/receive-pack.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 92561bf..f93ac45 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1122,7 +1122,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix) int advertise_refs = 0; int stateless_rpc = 0; int i; - char *dir = NULL; + const char *dir = NULL; struct command *commands; struct sha1_array shallow = SHA1_ARRAY_INIT; struct sha1_array ref = SHA1_ARRAY_INIT; @@ -1157,7 +1157,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix) } if (dir) usage(receive_pack_usage); - dir = xstrdup(arg); + dir = arg; } if (!dir) usage(receive_pack_usage); -- 2.0.0.566.gfe3e6b2 -- To unsubscribe from this list: send the line 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/5] free ref string returned by dwim_ref
A call to dwim_ref(name, len, flags, ref) will allocate a new string in ref to return the exact ref we found. We do not consistently free it in all code paths, leading to small leaks. The worst is in get_sha1_basic, which may be called many times (e.g., by cat-file --batch), though it is relatively unlikely, as it only triggers on a bogus reflog specification. Signed-off-by: Jeff King p...@peff.net --- builtin/rev-parse.c | 1 + builtin/show-branch.c | 1 + sha1_name.c | 4 +++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 8102aaa..d85e08c 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -151,6 +151,7 @@ static void show_rev(int type, const unsigned char *sha1, const char *name) error(refname '%s' is ambiguous, name); break; } + free(full); } else { show_with_type(type, name); } diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 5fd4e4e..298c95e 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -777,6 +777,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) sprintf(nth_desc, %s@{%d}, *av, base+i); append_ref(nth_desc, sha1, 1); } + free(ref); } else if (all_heads + all_remotes) snarf_refs(all_heads, all_remotes); diff --git a/sha1_name.c b/sha1_name.c index 6ccd3a5..63ee66f 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -540,8 +540,10 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) char *tmp = xstrndup(str + at + 2, reflog_len); at_time = approxidate_careful(tmp, errors); free(tmp); - if (errors) + if (errors) { + free(real_ref); return -1; + } } if (read_ref_at(real_ref, at_time, nth, sha1, NULL, co_time, co_tz, co_cnt)) { -- 2.0.0.566.gfe3e6b2 -- To unsubscribe from this list: send the line 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 3/5] transport: fix leaks in refs_from_alternate_cb
The function starts by creating a copy of the static buffer returned by real_path, but forgets to free it in the error code paths. We can solve this by jumping to the cleanup code that is already there. Signed-off-by: Jeff King p...@peff.net --- transport.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/transport.c b/transport.c index 3e42570..297e981 100644 --- a/transport.c +++ b/transport.c @@ -1359,11 +1359,11 @@ static int refs_from_alternate_cb(struct alternate_object_database *e, while (other[len-1] == '/') other[--len] = '\0'; if (len 8 || memcmp(other + len - 8, /objects, 8)) - return 0; + goto out; /* Is this a git repository with refs? */ memcpy(other + len - 8, /refs, 6); if (!is_directory(other)) - return 0; + goto out; other[len - 8] = '\0'; remote = remote_get(other); transport = transport_get(remote, other); @@ -1372,6 +1372,7 @@ static int refs_from_alternate_cb(struct alternate_object_database *e, extra = extra-next) cb-fn(extra, cb-data); transport_disconnect(transport); +out: free(other); return 0; } -- 2.0.0.566.gfe3e6b2 -- To unsubscribe from this list: send the line 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 4/5] fix memory leak parsing core.commentchar
When we see the core.commentchar config option, we extract the string with git_config_string, which does two things: 1. It complains via config_error_nonbool if there is no string value. 2. It makes a copy of the string. Since we immediately parse the string into its single-character value, we only care about (1). And in fact (2) is a detriment, as it means we leak the copy. Instead, let's just check the pointer value ourselves, and parse directly from the const string we already have. Signed-off-by: Jeff King p...@peff.net --- config.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/config.c b/config.c index 9767c4b..058505c 100644 --- a/config.c +++ b/config.c @@ -817,14 +817,12 @@ static int git_default_core_config(const char *var, const char *value) return git_config_string(editor_program, var, value); if (!strcmp(var, core.commentchar)) { - const char *comment; - int ret = git_config_string(comment, var, value); - if (ret) - return ret; - else if (!strcasecmp(comment, auto)) + if (!value) + return config_error_nonbool(var); + else if (!strcasecmp(value, auto)) auto_comment_line_char = 1; - else if (comment[0] !comment[1]) { - comment_line_char = comment[0]; + else if (value[0] !value[1]) { + comment_line_char = value[0]; auto_comment_line_char = 0; } else return error(core.commentChar should only be one character); -- 2.0.0.566.gfe3e6b2 -- To unsubscribe from this list: send the line 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 5/5] apply: avoid possible bogus pointer
When parsing index lines from a git-diff, we look for a space followed by the mode. If we don't have a space, then we set our pointer to the end-of-line. However, we don't double-check that our end-of-line pointer is valid (e.g., if we got a truncated diff input), which could lead to some wrap-around pointer arithmetic. In most cases this would probably get caught by our 40 len check later in the function, but to be on the safe side, let's just use strchrnul to treat end-of-string the same as end-of-line. Signed-off-by: Jeff King p...@peff.net --- builtin/apply.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/apply.c b/builtin/apply.c index 9f8f5ba..be2b4ce 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -1075,7 +1075,7 @@ static int gitdiff_index(const char *line, struct patch *patch) line = ptr + 2; ptr = strchr(line, ' '); - eol = strchr(line, '\n'); + eol = strchrnul(line, '\n'); if (!ptr || eol ptr) ptr = eol; -- 2.0.0.566.gfe3e6b2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html