Re: What's cooking in git.git (Jul 2014, #04; Tue, 22)

2014-07-23 Thread Philip Oakley

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

2014-07-23 Thread Chris Packham
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

2014-07-23 Thread Duy Nguyen
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()`

2014-07-23 Thread Matthieu Moy
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

2014-07-23 Thread Nguyễn Thái Ngọc Duy
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

2014-07-23 Thread Nguyễn Thái Ngọc Duy
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/

2014-07-23 Thread Nguyễn Thái Ngọc Duy
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

2014-07-23 Thread Nguyễn Thái Ngọc Duy
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

2014-07-23 Thread Nguyễn Thái Ngọc Duy
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()

2014-07-23 Thread Nguyễn Thái Ngọc Duy
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?

2014-07-23 Thread 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.
-- 
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

2014-07-23 Thread Duy Nguyen
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

2014-07-23 Thread Stefan Beller
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

2014-07-23 Thread Stefan Beller
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?

2014-07-23 Thread Karsten Blees
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

2014-07-23 Thread Kevin
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?

2014-07-23 Thread Sergei Organov
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?

2014-07-23 Thread Stefan Beller
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

2014-07-23 Thread Michael J Gruber
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)

2014-07-23 Thread Karsten Blees
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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

2014-07-23 Thread Ronnie Sahlberg
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()`

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Junio C Hamano
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?

2014-07-23 Thread Jonathan Nieder
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?

2014-07-23 Thread Junio C Hamano
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)

2014-07-23 Thread Junio C Hamano
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)

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Tanay Abhra
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

2014-07-23 Thread Tanay Abhra
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

2014-07-23 Thread Tanay Abhra
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

2014-07-23 Thread Tanay Abhra
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()`

2014-07-23 Thread Tanay Abhra
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`

2014-07-23 Thread Tanay Abhra
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

2014-07-23 Thread Tanay Abhra
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`

2014-07-23 Thread Tanay Abhra
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

2014-07-23 Thread Ross Boylan
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

2014-07-23 Thread Ramsay Jones

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

2014-07-23 Thread Ross Boylan
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?

2014-07-23 Thread Sergei Organov
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`

2014-07-23 Thread John Keeping
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

2014-07-23 Thread Matthieu Moy
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

2014-07-23 Thread Matthieu Moy
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

2014-07-23 Thread Matthieu Moy
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

2014-07-23 Thread Matthieu Moy
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

2014-07-23 Thread Junio C Hamano
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`

2014-07-23 Thread Matthieu Moy
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()`

2014-07-23 Thread Matthieu Moy
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`

2014-07-23 Thread Eric Sunshine
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

2014-07-23 Thread Eric Sunshine
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

2014-07-23 Thread Junio C Hamano
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?

2014-07-23 Thread Christoph Bonitz
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?

2014-07-23 Thread Christoph Bonitz
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`

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Eric Wong
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

2014-07-23 Thread Junio C Hamano
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`

2014-07-23 Thread Junio C Hamano
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.

2014-07-23 Thread Eric Wong
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`

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Junio C Hamano
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?

2014-07-23 Thread Pete Wyckoff
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

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Ross Boylan
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

2014-07-23 Thread Junio C Hamano
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

2014-07-23 Thread Ross Boylan
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

2014-07-23 Thread Jeff King
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

2014-07-23 Thread Jeff King
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

2014-07-23 Thread Jeff King
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

2014-07-23 Thread Jeff King
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

2014-07-23 Thread Jeff King
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

2014-07-23 Thread Jeff King
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