[PATCH v2 2/2] bisect: make bisection refs per-worktree
Using the new refs/worktree/ refs, make bisection per-worktree.
Signed-off-by: David Turner dtur...@twopensource.com
---
Documentation/git-bisect.txt | 4 ++--
Documentation/rev-list-options.txt | 14 +++---
bisect.c | 2 +-
builtin/rev-parse.c| 6 --
revision.c | 2 +-
t/t6030-bisect-porcelain.sh| 20 ++--
6 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index e97f2de..f0c52d1 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -82,7 +82,7 @@ to ask for the next commit that needs testing.
Eventually there will be no more revisions left to inspect, and the
command will print out a description of the first bad commit. The
-reference `refs/bisect/bad` will be left pointing at that commit.
+reference `refs/worktree/bisect/bad` will be left pointing at that commit.
Bisect reset
@@ -373,7 +373,7 @@ on a single line.
$ git bisect start HEAD known-good-commit [ boundary-commit ... ]
--no-checkout
$ git bisect run sh -c '
- GOOD=$(git for-each-ref --format=%(objectname) refs/bisect/good-*)
+ GOOD=$(git for-each-ref --format=%(objectname)
refs/worktree/bisect/good-*)
git rev-list --objects BISECT_HEAD --not $GOOD tmp.$$
git pack-objects --stdout /dev/null tmp.$$
rc=$?
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index a9b808f..1175960 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -183,9 +183,9 @@ explicitly.
ifndef::git-rev-list[]
--bisect::
- Pretend as if the bad bisection ref `refs/bisect/bad`
+ Pretend as if the bad bisection ref `refs/worktree/bisect/bad`
was listed and as if it was followed by `--not` and the good
- bisection refs `refs/bisect/good-*` on the command
+ bisection refs `refs/worktree/bisect/good-*` on the command
line. Cannot be combined with --first-parent.
endif::git-rev-list[]
@@ -548,10 +548,10 @@ Bisection Helpers
--bisect::
Limit output to the one commit object which is roughly halfway between
included and excluded commits. Note that the bad bisection ref
- `refs/bisect/bad` is added to the included commits (if it
- exists) and the good bisection refs `refs/bisect/good-*` are
+ `refs/worktree/bisect/bad` is added to the included commits (if it
+ exists) and the good bisection refs `refs/worktree/bisect/good-*` are
added to the excluded commits (if they exist). Thus, supposing there
- are no refs in `refs/bisect/`, if
+ are no refs in `refs/worktree/bisect/`, if
+
---
$ git rev-list --bisect foo ^bar ^baz
@@ -571,7 +571,7 @@ one. Cannot be combined with --first-parent.
--bisect-vars::
This calculates the same as `--bisect`, except that refs in
- `refs/bisect/` are not used, and except that this outputs
+ `refs/worktree/bisect/` are not used, and except that this outputs
text ready to be eval'ed by the shell. These lines will assign the
name of the midpoint revision to the variable `bisect_rev`, and the
expected number of commits to be tested after `bisect_rev` is tested
@@ -584,7 +584,7 @@ one. Cannot be combined with --first-parent.
--bisect-all::
This outputs all the commit objects between the included and excluded
commits, ordered by their distance to the included and excluded
- commits. Refs in `refs/bisect/` are not used. The farthest
+ commits. Refs in `refs/worktree/bisect/` are not used. The farthest
from them is displayed first. (This is the only one displayed by
`--bisect`.)
+
diff --git a/bisect.c b/bisect.c
index 33ac88d..dbe3461 100644
--- a/bisect.c
+++ b/bisect.c
@@ -425,7 +425,7 @@ static int register_ref(const char *refname, const struct
object_id *oid,
static int read_bisect_refs(void)
{
- return for_each_ref_in(refs/bisect/, register_ref, NULL);
+ return for_each_ref_in(refs/worktree/bisect/, register_ref, NULL);
}
static void read_bisect_paths(struct argv_array *array)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 02d747d..3240ddf 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -663,8 +663,10 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
if (!strcmp(arg, --bisect)) {
- for_each_ref_in(refs/bisect/bad,
show_reference, NULL);
- for_each_ref_in(refs/bisect/good,
anti_reference, NULL);
+ for_each_ref_in(refs/worktree/bisect/bad,
+
[PATCH v2 1/2] refs: refs/worktree/* become per-worktree
We need a place to stick refs for bisects in progress that is not
shared between worktrees. So we use the refs/worktree/ hierarchy.
The is_per_worktree_ref function and associated docs learn that
refs/worktree/ is per-worktree, as does the git_path code in path.c
The ref-packing functions learn that refs beginning with
refs/worktree/ should not be packed (since packed-refs is common
rather than per-worktree).
Signed-off-by: David Turner dtur...@twopensource.com
---
This implements the very simple solution of making refs/worktree/
per-worktree, as we discussed on the PATCH/RFC first version of this
patch.
Note that git for-each-ref may have inconsistent behavior (I think; I
haven't confirmed this), sometimes showing refs/worktree/* and sometimes
not. In the long run, we should fix this, but right now, I don't know
that it matters, since the only refs affected are these bisect refs.
---
Documentation/glossary-content.txt | 5 +++--
path.c | 15 ---
refs.c | 7 ++-
t/t1400-update-ref.sh | 16
t/t3210-pack-refs.sh | 7 +++
5 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/Documentation/glossary-content.txt
b/Documentation/glossary-content.txt
index 8c6478b..5c707e6 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -413,8 +413,9 @@ exclude;;
[[def_per_worktree_ref]]per-worktree ref::
Refs that are per-def_working_tree,worktree, rather than
- global. This is presently only def_HEAD,HEAD, but might
- later include other unusual refs.
+ global. This is presently only def_HEAD,HEAD and any refs
+ that start with `refs/worktree/`, but might later include other
+ unusual refs.
[[def_pseudoref]]pseudoref::
Pseudorefs are a class of files under `$GIT_DIR` which behave
diff --git a/path.c b/path.c
index 10f4cbf..da0f767 100644
--- a/path.c
+++ b/path.c
@@ -92,8 +92,9 @@ static void replace_dir(struct strbuf *buf, int len, const
char *newdir)
}
static const char *common_list[] = {
+ /refs, /* special case, since refs/worktree/ is per-worktree */
/branches, /hooks, /info, !/logs, /lost-found,
- /objects, /refs, /remotes, /worktrees, /rr-cache, /svn,
+ /objects, /remotes, /worktrees, /rr-cache, /svn,
config, !gc.pid, packed-refs, shallow,
NULL
};
@@ -116,8 +117,16 @@ static void update_common_dir(struct strbuf *buf, int
git_dir_len)
is_dir = 1;
}
if (is_dir dir_prefix(base, path)) {
- replace_dir(buf, git_dir_len, get_git_common_dir());
- return;
+ /*
+* The first entry in common_list is refs, and
+* refs/worktree is *not* common.
+*/
+
+ if (p != common_list ||
+ !dir_prefix(base, refs/worktree)) {
+ replace_dir(buf, git_dir_len,
get_git_common_dir());
+ return;
+ }
}
if (!is_dir !strcmp(base, path)) {
replace_dir(buf, git_dir_len, get_git_common_dir());
diff --git a/refs.c b/refs.c
index e6fc3fe..d43bfe1 100644
--- a/refs.c
+++ b/refs.c
@@ -2656,6 +2656,10 @@ static int pack_if_possible_fn(struct ref_entry *entry,
void *cb_data)
struct ref_entry *packed_entry;
int is_tag_ref = starts_with(entry-name, refs/tags/);
+ /* Do not pack per-worktree refs: */
+ if (starts_with(entry-name, refs/worktree/))
+ return 0;
+
/* ALWAYS pack tags */
if (!(cb-flags PACK_REFS_ALL) !is_tag_ref)
return 0;
@@ -2850,7 +2854,8 @@ static int delete_ref_loose(struct ref_lock *lock, int
flag, struct strbuf *err)
static int is_per_worktree_ref(const char *refname)
{
- return !strcmp(refname, HEAD);
+ return !strcmp(refname, HEAD) ||
+ starts_with(refname, refs/worktree/);
}
static int is_pseudoref_syntax(const char *refname)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 9d21c19..c9fd1ca 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1131,4 +1131,20 @@ test_expect_success ULIMIT_FILE_DESCRIPTORS 'large
transaction deleting branches
)
'
+test_expect_success 'handle per-worktree refs in refs/worktree' '
+ git commit --allow-empty -m initial commit
+ git worktree add -b branch worktree
+ (
+ cd worktree
+ git commit --allow-empty -m test commit
+ git update-ref refs/worktree/something HEAD
+ git rev-parse refs/worktree/something ../worktree-head
+ )
+ ! test -e .git/refs/worktree
+ test_must_fail git rev-parse refs/worktree/something
[PATCH v5 0/5] Improve guessing of repository names
This is version 5 of my patch series which aims to improve guessed directory names in several cases. This version now includes the patches from Jeff which were previously a separate patch series. Besides fixing behavior when cloning a repository with trailing '.git' suffix they also add a new test suite that verifies guessed directory names. I've amended 'clone: add tests for output directory' to add additional tests (as proposed by Junio). Changes to my own patches include improved commit messages detailing why certain changes are done the way they are done and a change to authentication-data-stripping, such that we now strip greedily until the last '@' sign in the host part (proposed by Junio, as well). Jeff King (2): clone: add tests for output directory clone: use computed length in guess_dir_name Patrick Steinhardt (3): clone: do not include authentication data in guessed dir clone: do not use port number as dir name clone: abort if no dir name could be guessed builtin/clone.c | 65 +- t/t5603-clone-dirname.sh | 103 +++ 2 files changed, 157 insertions(+), 11 deletions(-) create mode 100755 t/t5603-clone-dirname.sh -- 2.5.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
[PATCH v5 3/5] clone: do not include authentication data in guessed dir
If the URI contains authentication data and the URI's path
component is empty we fail to guess a sensible directory name.
E.g. cloning a repository 'ssh://user:passw...@example.com/' we
guess a directory name 'passw...@example.com' where we would want
the hostname only, e.g. 'example.com'.
The naive way of just adding '@' as a path separator would break
cloning repositories like 'foo/b...@baz.git' (which would
currently become 'bar@baz' but would then become 'baz' only).
Instead fix this by first dropping the scheme and then greedily
scanning for an '@' sign until we find the first path separator.
Signed-off-by: Patrick Steinhardt p...@pks.im
---
builtin/clone.c | 41 +++--
t/t5603-clone-dirname.sh | 4 ++--
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index bf45199..da51792 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -146,30 +146,51 @@ static char *get_repo_path(const char *repo, int
*is_bundle)
static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
{
- const char *end = repo + strlen(repo), *start;
+ const char *end = repo + strlen(repo), *start, *ptr;
size_t len;
char *dir;
/*
+* Skip scheme.
+*/
+ start = strstr(repo, ://);
+ if (start == NULL)
+ start = repo;
+ else
+ start += 3;
+
+ /*
+* Skip authentication data. The stripping does happen
+* greedily, such that we strip up to the last '@' inside
+* the host part.
+*/
+ for (ptr = start; ptr end !is_dir_sep(*ptr); ptr++) {
+ if (*ptr == '@')
+ start = ptr + 1;
+ }
+
+ /*
* Strip trailing spaces, slashes and /.git
*/
- while (repo end (is_dir_sep(end[-1]) || isspace(end[-1])))
+ while (start end (is_dir_sep(end[-1]) || isspace(end[-1])))
end--;
- if (end - repo 5 is_dir_sep(end[-5])
+ if (end - start 5 is_dir_sep(end[-5])
!strncmp(end - 4, .git, 4)) {
end -= 5;
- while (repo end is_dir_sep(end[-1]))
+ while (start end is_dir_sep(end[-1]))
end--;
}
/*
-* Find last component, but be prepared that repo could have
-* the form remote.example.com:foo.git, i.e. no slash
-* in the directory part.
+* Find last component. To remain backwards compatible we
+* also regard colons as path separators, such that
+* cloning a repository 'foo:bar.git' would result in a
+* directory 'bar' being guessed.
*/
- start = end;
- while (repo start !is_dir_sep(start[-1]) start[-1] != ':')
- start--;
+ ptr = end;
+ while (start ptr !is_dir_sep(ptr[-1]) ptr[-1] != ':')
+ ptr--;
+ start = ptr;
/*
* Strip .{bundle,git}.
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index 10f5d09..a9aba72 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -74,11 +74,11 @@ test_clone_dir host:foo/.git/// foo
# omitting the path should default to the hostname
test_clone_dir ssh://host/ host
test_clone_dir ssh://host:1234/ host fail
-test_clone_dir ssh://user@host/ host fail
+test_clone_dir ssh://user@host/ host
test_clone_dir host:/ host fail
# auth materials should be redacted
-test_clone_dir ssh://user:password@host/ host fail
+test_clone_dir ssh://user:password@host/ host
test_clone_dir ssh://user:password@host:1234/ host fail
test_clone_dir ssh://user:passw@rd@host:1234/ host fail
test_clone_dir user@host:/ host fail
--
2.5.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
[PATCH v5 4/5] clone: do not use port number as dir name
If the URI contains a port number and the URI's path component is
empty we fail to guess a sensible directory name. E.g. cloning a
repository 'ssh://example.com:/' we guess a directory name
'' where we would want the hostname only, e.g. 'example.com'.
We need to take care to not drop trailing port-like numbers in
certain cases. E.g. when cloning a repository 'foo/bar:.git'
we want to guess the directory name '' instead of 'bar'.
Thus, we have to first check the stripped URI for path separators
and only strip port numbers if there are path separators present.
This heuristic breaks when cloning a repository 'bar:.git',
though.
Signed-off-by: Patrick Steinhardt p...@pks.im
---
builtin/clone.c | 17 +
t/t5603-clone-dirname.sh | 14 +++---
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index da51792..e7f16ff 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -182,6 +182,23 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
}
/*
+* Strip trailing port number if we've got only a
+* hostname (that is, there is no dir separator but a
+* colon). This check is required such that we do not
+* strip URI's like '/foo/bar:.git', which should
+* result in a dir '' being guessed due to backwards
+* compatibility.
+*/
+ if (memchr(start, '/', end - start) == NULL
+memchr(start, ':', end - start) != NULL) {
+ ptr = end;
+ while (start ptr isdigit(ptr[-1]) ptr[-1] != ':')
+ ptr--;
+ if (start ptr ptr[-1] == ':')
+ end = ptr - 1;
+ }
+
+ /*
* Find last component. To remain backwards compatible we
* also regard colons as path separators, such that
* cloning a repository 'foo:bar.git' would result in a
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index a9aba72..664d0ab 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -73,17 +73,17 @@ test_clone_dir host:foo/.git/// foo
# omitting the path should default to the hostname
test_clone_dir ssh://host/ host
-test_clone_dir ssh://host:1234/ host fail
+test_clone_dir ssh://host:1234/ host
test_clone_dir ssh://user@host/ host
-test_clone_dir host:/ host fail
+test_clone_dir host:/ host
# auth materials should be redacted
test_clone_dir ssh://user:password@host/ host
-test_clone_dir ssh://user:password@host:1234/ host fail
-test_clone_dir ssh://user:passw@rd@host:1234/ host fail
-test_clone_dir user@host:/ host fail
-test_clone_dir user:password@host:/ host fail
-test_clone_dir user:pass@wrd@host:/ host fail
+test_clone_dir ssh://user:password@host:1234/ host
+test_clone_dir ssh://user:passw@rd@host:1234/ host
+test_clone_dir user@host:/ host
+test_clone_dir user:password@host:/ host
+test_clone_dir user:pass@wrd@host:/ host
# auth-like material should not be dropped
test_clone_dir ssh://host/foo@bar foo@bar
--
2.5.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
[PATCH v5 2/5] clone: use computed length in guess_dir_name
From: Jeff King p...@peff.net
Commit 7e837c6 (clone: simplify string handling in
guess_dir_name(), 2015-07-09) changed clone to use
strip_suffix instead of hand-rolled pointer manipulation.
However, strip_suffix will strip from the end of a
NUL-terminated string, and we may have already stripped some
characters (like directory separators, or /.git). This
leads to commands like:
git clone host:foo.git/
failing to strip the .git.
We must instead convert our pointer arithmetic into a
computed length and feed that to strip_suffix_mem, which will
then reduce the length further for us.
It would be nicer if we could drop the pointer manipulation
entirely, and just continually strip using strip_suffix. But
that doesn't quite work for two reasons:
1. The early suffixes we're stripping are not constant; we
need to look for is_dir_sep, which could be one of
several characters.
2. Mid-way through the stripping we compute the pointer
start, which shows us the beginning of the pathname.
Which really give us two lengths to work with: the
offset from the start of the string, and from the start
of the path. By using pointers for the early part, we
can just compute the length from start when we need
it.
Signed-off-by: Jeff King p...@peff.net
---
builtin/clone.c | 3 ++-
t/t5603-clone-dirname.sh | 12 ++--
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 303a3a7..bf45199 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -174,7 +174,8 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
/*
* Strip .{bundle,git}.
*/
- strip_suffix(start, is_bundle ? .bundle : .git , len);
+ len = end - start;
+ strip_suffix_mem(start, len, is_bundle ? .bundle : .git);
if (is_bare)
dir = xstrfmt(%.*s.git, (int)len, start);
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index b009a87..10f5d09 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -46,7 +46,7 @@ test_clone_dir host:foo foo.git bare
test_clone_dir host:foo.git foo
test_clone_dir host:foo.git foo.git bare
test_clone_dir host:foo/.git foo
-test_clone_dir host:foo/.git foo.git bare fail
+test_clone_dir host:foo/.git foo.git bare
# similar, but using ssh URL rather than host:path syntax
test_clone_dir ssh://host/foo foo
@@ -54,20 +54,20 @@ test_clone_dir ssh://host/foo foo.git bare
test_clone_dir ssh://host/foo.git foo
test_clone_dir ssh://host/foo.git foo.git bare
test_clone_dir ssh://host/foo/.git foo
-test_clone_dir ssh://host/foo/.git foo.git bare fail
+test_clone_dir ssh://host/foo/.git foo.git bare
# we should remove trailing slashes and .git suffixes
test_clone_dir ssh://host/foo/ foo
test_clone_dir ssh://host/foo/// foo
-test_clone_dir ssh://host/foo.git/ foo fail
-test_clone_dir ssh://host/foo.git/// foo fail
+test_clone_dir ssh://host/foo.git/ foo
+test_clone_dir ssh://host/foo.git/// foo
test_clone_dir ssh://host/foo///.git/ foo
test_clone_dir ssh://host/foo/.git/// foo
test_clone_dir host:foo/ foo
test_clone_dir host:foo/// foo
-test_clone_dir host:foo.git/ foo fail
-test_clone_dir host:foo.git/// foo fail
+test_clone_dir host:foo.git/ foo
+test_clone_dir host:foo.git/// foo
test_clone_dir host:foo///.git/ foo
test_clone_dir host:foo/.git/// foo
--
2.5.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
[PATCH v5 5/5] clone: abort if no dir name could be guessed
Due to various components of the URI being stripped off it may happen that we fail to guess a directory name. We currently error out with a message that it is impossible to create the working tree '' in such cases. Instead, error out early with a sensible error message hinting that a directory name should be specified manually on the command line. Signed-off-by: Patrick Steinhardt p...@pks.im --- builtin/clone.c | 4 1 file changed, 4 insertions(+) diff --git a/builtin/clone.c b/builtin/clone.c index e7f16ff..5169746 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -215,6 +215,10 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) len = end - start; strip_suffix_mem(start, len, is_bundle ? .bundle : .git); + if (!len || (len == 1 *start == '/')) + die(No directory name could be guessed.\n + Please specify a directory on the command line); + if (is_bare) dir = xstrfmt(%.*s.git, (int)len, start); else -- 2.5.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
[PATCH v5 1/5] clone: add tests for output directory
From: Jeff King p...@peff.net
When we run git clone $url, clone guesses from the $url
what to name the local output directory. We don't have any
test coverage of this, so let's add some basic tests.
This reveals a few problems:
- cloning foo.git/ does not properly remove the .git;
this is a recent regression from 7e837c6 (clone:
simplify string handling in guess_dir_name(), 2015-07-09)
- likewise, cloning foo/.git does not seem to handle the
bare case (we should end up in foo.git, but we try to
use foo/.git on the local end), which also comes from
7e837c6.
- cloning the root is not very smart about URL parsing,
and usernames and port numbers may end up in the
directory name
All of these tests are marked as failures.
Signed-off-by: Jeff King p...@peff.net
---
t/t5603-clone-dirname.sh | 103 +++
1 file changed, 103 insertions(+)
create mode 100755 t/t5603-clone-dirname.sh
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
new file mode 100755
index 000..b009a87
--- /dev/null
+++ b/t/t5603-clone-dirname.sh
@@ -0,0 +1,103 @@
+#!/bin/sh
+
+test_description='check output directory names used by git-clone'
+. ./test-lib.sh
+
+# we use a fake ssh wrapper that ignores the arguments
+# entirely; we really only care that we get _some_ repo,
+# as the real test is what clone does on the local side
+test_expect_success 'setup ssh wrapper' '
+ write_script $TRASH_DIRECTORY/ssh-wrapper -\EOF
+ git upload-pack $TRASH_DIRECTORY
+ EOF
+ GIT_SSH=$TRASH_DIRECTORY/ssh-wrapper
+ export GIT_SSH
+ export TRASH_DIRECTORY
+'
+
+# make sure that cloning $1 results in local directory $2
+test_clone_dir () {
+ url=$1; shift
+ dir=$1; shift
+ expect=success
+ bare=non-bare
+ clone_opts=
+ for i in $@; do
+ case $i in
+ fail)
+ expect=failure
+ ;;
+ bare)
+ bare=bare
+ clone_opts=--bare
+ ;;
+ esac
+ done
+ test_expect_$expect clone of $url goes to $dir ($bare)
+ rm -rf $dir
+ git clone $clone_opts $url
+ test_path_is_dir $dir
+
+}
+
+# basic syntax with bare and non-bare variants
+test_clone_dir host:foo foo
+test_clone_dir host:foo foo.git bare
+test_clone_dir host:foo.git foo
+test_clone_dir host:foo.git foo.git bare
+test_clone_dir host:foo/.git foo
+test_clone_dir host:foo/.git foo.git bare fail
+
+# similar, but using ssh URL rather than host:path syntax
+test_clone_dir ssh://host/foo foo
+test_clone_dir ssh://host/foo foo.git bare
+test_clone_dir ssh://host/foo.git foo
+test_clone_dir ssh://host/foo.git foo.git bare
+test_clone_dir ssh://host/foo/.git foo
+test_clone_dir ssh://host/foo/.git foo.git bare fail
+
+# we should remove trailing slashes and .git suffixes
+test_clone_dir ssh://host/foo/ foo
+test_clone_dir ssh://host/foo/// foo
+test_clone_dir ssh://host/foo.git/ foo fail
+test_clone_dir ssh://host/foo.git/// foo fail
+test_clone_dir ssh://host/foo///.git/ foo
+test_clone_dir ssh://host/foo/.git/// foo
+
+test_clone_dir host:foo/ foo
+test_clone_dir host:foo/// foo
+test_clone_dir host:foo.git/ foo fail
+test_clone_dir host:foo.git/// foo fail
+test_clone_dir host:foo///.git/ foo
+test_clone_dir host:foo/.git/// foo
+
+# omitting the path should default to the hostname
+test_clone_dir ssh://host/ host
+test_clone_dir ssh://host:1234/ host fail
+test_clone_dir ssh://user@host/ host fail
+test_clone_dir host:/ host fail
+
+# auth materials should be redacted
+test_clone_dir ssh://user:password@host/ host fail
+test_clone_dir ssh://user:password@host:1234/ host fail
+test_clone_dir ssh://user:passw@rd@host:1234/ host fail
+test_clone_dir user@host:/ host fail
+test_clone_dir user:password@host:/ host fail
+test_clone_dir user:pass@wrd@host:/ host fail
+
+# auth-like material should not be dropped
+test_clone_dir ssh://host/foo@bar foo@bar
+test_clone_dir ssh://host/f...@bar.git foo@bar
+test_clone_dir ssh://user:password@host/foo@bar foo@bar
+test_clone_dir ssh://user:passw@rd@host/f...@bar.git foo@bar
+
+test_clone_dir host:/foo@bar foo@bar
+test_clone_dir host:/f...@bar.git foo@bar
+test_clone_dir user:password@host:/foo@bar foo@bar
+test_clone_dir user:passw@rd@host:/f...@bar.git foo@bar
+
+# trailing port-like numbers should not be stripped for paths
+test_clone_dir ssh://user:password@host/test:1234 1234
+test_clone_dir ssh://user:password@host/test:1234.git 1234
+
+test_done
--
2.5.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: Feature: git stash pop --always-drop
Junio C Hamano gitster at pobox.com writes: Yes, my use case is that I get confused about whether the stash has been dropped or not and whether I might have stashed something else in the meantime. So for me plain 'git stash drop' feels a bit dangerous. Then git stash apply followed by git stash drop would be a pair of good workflow elements for you, no? I like ordinary 'git stash pop' when it applies cleanly. Only in the cases where it has conflicts and leaves the stash in place does it get a bit awkward. I manually resolve the conflicts and then 'git stash drop', but that last step is a bit dangerous because it might drop an unrelated stash if I have done some other stashing in the meantime. If 'git stash pop' (and 'apply') would always print the name of the stash, then it would be easy to drop that particular stash afterwards. Running one too many or one too few 'git stash drop' commands would no longer cause problems. Printing the name of the stash would, for me, largely remove the need for an --always-drop option to git stash, which is what I at first suggested. -- Ed Avis e...@waniasset.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pager_in_use: make sure output is still going to pager
Hi Peff,
On 2015-08-10 07:23, Jeff King wrote:
diff --git a/compat/pipe-id.c b/compat/pipe-id.c
new file mode 100644
index 000..4764c5f
--- /dev/null
+++ b/compat/pipe-id.c
@@ -0,0 +1,25 @@
+#include git-compat-util.h
+#include compat/pipe-id.h
+#include strbuf.h
+
+const char *pipe_id_get(int fd)
+{
+ static struct strbuf id = STRBUF_INIT;
+ struct stat st;
+
+ if (fstat(fd, st) 0 || !S_ISFIFO(st.st_mode))
+ return NULL;
Just a quick note: it seems that this check is not really working on Windows. I
tested this by running this test case manually (because TTY is not set on
Windows):
+test_expect_success TTY 'no color when paged program writes to pipe' '
+ test_config alias.externallog !git log | cat log.out
+ test_config color.ui auto
+ test_terminal env TERM=vt100 git -p externallog
+ test_line_count = 0 paginated.out
+ test -s log.out
+ ! colorful log.out
+'
The output is colorful ;-)
I hope to find some time tomorrow to figure out some workaround that makes this
work on Windows.
Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: File Hash for Windows executable
Hi Ryan, On 2015-08-10 15:54, Kiser, Ryan Lee wrote: I've downloaded the Windows installer for Git, Which one. but can't seem to find hashes published anywhere for verification. Since the downloaded file doesn't seem to be signed, The newest ones from https://git-for-windows.github.io/ are signed. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe 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/17] removing questionable uses of git_path
Jeff King p...@peff.net writes: The problem is that git_path uses a static buffer that gets overwritten by subsequent calls. As the rotating static buffer pattern used in get_pathname() was modeled after sha1_to_hex(), we have the same issue there. They are troubles waiting to happen unless the callers are careful. producing a fairly tame-looking set of function calls. It's OK to pass the result of git_path() to a system call, or something that is a thin wrapper around one (e.g., strbuf_read_file). That is a short and good rule to follow, but the problem is that not everybody has good taste to interpret the above rule and apply it with an eye toward maintainability X-. Along the way, there are a few cleanups (e.g., I polished off the recent hold_lock_file_for_append topic which was on the list, as it had some problematic calls). [01/17]: cache.h: clarify documentation for git_path, et al [02/17]: cache.h: complete set of git_path_submodule helpers [03/17]: t5700: modernize style [04/17]: add_to_alternates_file: don't add duplicate entries [05/17]: remove hold_lock_file_for_append [06/17]: prefer git_pathdup to git_path in some possibly-dangerous cases [07/17]: prefer mkpathdup to mkpath in assignments [08/17]: remote.c: drop extraneous local variable from migrate_file [09/17]: refs.c: remove extra git_path calls from read_loose refs [10/17]: path.c: drop git_path_submodule [11/17]: refs.c: simplify strbufs in reflog setup and writing [12/17]: refs.c: avoid repeated git_path calls in rename_tmp_log [13/17]: refs.c: avoid git_path assignment in lock_ref_sha1_basic [14/17]: refs.c: remove_empty_directories can take a strbuf [15/17]: find_hook: keep our own static buffer [16/17]: get_repo_path: refactor path-allocation [17/17]: memoize common git-path constant files Nice. 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: resolving a (possibly remote) branch HEAD to a hash
per...@pluto.rain.com (Perry Hutchison) writes: ... we do not say append 'refs/remotes/anything/' for various values of anything and see if such a ref exists when resolving an abbreviated refname 'master'. checkout appears to. You are referring to this part of the documentation: If branch is not found but there does exist a tracking branch in exactly one remote (call it remote) with a matching name, treat as equivalent to $ git checkout -b branch --track remote/branch A reader needs to read this part of the documentation a bit more carefully in order to notice that it never says it is equivalent to: $ git checkout -b branch -t branch ;# NOT CORRECT This behaviour was brought in by somebody who thought that, in the context of checkout (and only in that context), it is clear that missing branch that can only mean the sole remote/branch and make that signal something more than what the user told checkout to do: If you want to check out a branch, and it does not exist yet, you must wanted to create your own branch and start it at the same commit as somebody else has at the tip of his branch. This clever dwim is very specific to the way you interact with checkout and generally does not apply when you want to run anything other than checkout, e.g. rev-parse or log. But it is _so_ convenient a short-cut, that it lets new people form into an illusion that branch could be naming remote/branch. That is an incorrect perception. The rationale behind signal something more above goes like this: if the user wanted to detach the head at the same commit as somebody else's branch, she would explicitly have written $ git checkout remote/branch to do so. Because remote/branch is the shortest valid way to name that remote-tracking branch (i.e. exactly because branch is not a valid abbreviation for remote/branch), we can treat $ git checkout branch when branch is not a local branch name specially. It is sad and ironic that this checkout-specific DWIM works only because branch does not mean remote/branch, but presence of the DWIM gives a wrong illusion that it does X-. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git blame breaking on repository with CRLF files
Torsten Bögershausen tbo...@web.de writes: Actually I could reproduce the following: CRLF in repo, CRLF in working tree, core.autocrlf= true. What should happen in such a case? Wouldn't autocrlf=true want to strip CRLF down to LF? Shouldn't it? And if so, blame is correct to say that you are changing the line endings of all your lines, as what you _would_ commit if you were to commit the tracked files in your working tree would be different from what is in the index, no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pager_in_use: make sure output is still going to pager
On Mon, Aug 10, 2015 at 06:38:10PM +0200, Johannes Schindelin wrote:
+const char *pipe_id_get(int fd)
+{
+ static struct strbuf id = STRBUF_INIT;
+ struct stat st;
+
+ if (fstat(fd, st) 0 || !S_ISFIFO(st.st_mode))
+ return NULL;
Just a quick note: it seems that this check is not really working on
Windows. I tested this by running this test case manually (because TTY
is not set on Windows):
Yeah, I'm not too surprised. I'm guessing your st_ino for pipes are all
just the same or something. Or maybe S_ISFIFO doesn't pass (we don't
technically need it, I don't think, and could just drop that check).
Anyway, I had planned that we would need to stick a big #ifdef WINDOWS
around these two functions.
I hope to find some time tomorrow to figure out some workaround that
makes this work on Windows.
Cool. I can't comment on Windows-specific stuff, but I'm happy to review
the rest of it. :)
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/17] removing questionable uses of git_path
On Mon, Aug 10, 2015 at 10:31:32AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: The problem is that git_path uses a static buffer that gets overwritten by subsequent calls. As the rotating static buffer pattern used in get_pathname() was modeled after sha1_to_hex(), we have the same issue there. They are troubles waiting to happen unless the callers are careful. Yeah, I think Michael mentioned that to me off-list. I don't _think_ sha1_to_hex is nearly such a problem, because we tend to store sha1s in their binary form. So sha1_to_hex is almost always an argument to fprintf() or similar. Of course there are some exceptions. :) I notice we pass one return value to add_pending_object, which was almost certainly horribly broken before 31faeb2 started strdup'ing object_array names. So certainly it would be nice to audit them all, but there are over 600 calls. Given the likelihood of finding a useful bug, I'm not sure it's the greatest use of developer time. producing a fairly tame-looking set of function calls. It's OK to pass the result of git_path() to a system call, or something that is a thin wrapper around one (e.g., strbuf_read_file). That is a short and good rule to follow, but the problem is that not everybody has good taste to interpret the above rule and apply it with an eye toward maintainability X-. Yeah. Part of me wants to eradicate git_path entirely. My series takes out over half of the calls, but there are still close to 100, I think. I think it would make the code worse to convert all of them naively. We could provide format-aware wrappers for some filesystem functions, like: git_stat(st, %s, refname); or something. That feels horribly coupled compared to: stat(git_path(%s, refname), st); but it makes it very clear what the memory ownership/lifetime rules are. Anyway, part of my goal with the series was not just to clean up the existing suspicious spots, but to raise awareness of the issue. At least for me, I know it's something I will look for more carefully when reviewing patches. Once bitten, twice shy. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Error when cloning with weird local directory
Torsten Bögershausen tbo...@web.de writes: So I think that git clone can be slighty more consistant here. Sure. I _think_ taking notice of word:// (with doubled slashes) and treating it specially will not introduce any new issue; while it is still OK for users to have a local directory called word:, if they meant a subdirectory of it, they wouldn't have typed double-slashes there. -- To unsubscribe from this list: send the line unsubscribe 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] submodule: implement `module_name` as a builtin helper
On Fri, Aug 7, 2015 at 11:20 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: ... if you really want to go the thread route, the first thing to try would be to see if a few places we already use threads for parallelism (namely, grep, pack-objects, preload-index and index-pack) can be factored out and model your new API around the commonality among them. And obviously, doing your pool API around threads will allow you to throw future per-thread function that do not involve run_command() at all at your API, and it will make it easy to adapt the current threaded parts of the system to the API. Just a few random thoughts before going to bed and going offline for the weekend... Eventually, we would want to do submodule update of a top-level project that has 500 submodules underneath, but obviously we would not want to blindly spawn 500 threads, each of which runs fetch, all at the same time. We'd want to limit the parallelism to a sane limit (say, 16 or 32), stuff 500 work units to a queue, from which that many number of worker bees grab work unit one by one to process and then come back to ask for more work. This is exactly what is currently implemented and works in patch [RFC PATCH 2/4] Add a workdispatcher to get work done in parallel And we would eventually want to be able to do this even when these 500 submodules are spread across multiple levels of nested submodules (e.g. top-level may have 8 submodules, and they have 16 nested subsubmodules each on average, each of which may have 4 nested subsubsubmodules on average). Specifying -j16 at the top level and apportioning the parallelism to recursive invoation of submodule update in such a way that the overall process is efficient and without waste would be a bit tricky. In such a nested submodule case, we may want to instead try to enumerate these 500 submodules upfront with unbounded parallelism The problem here is we need to have finished cloning at least one top level submodule before we can add any further sub-submodules into the work queue. So if there are only 4 top level submodules we'd have a slow start. If we have only one top level work queue, the deeper nesting levels will not explode unbound, but eventually we will reach the 16 threads and keep working with them, and once git submodule is ported to C we don't need to have process invocations, but can rely on the just the threading done right. And then we it should be rather easy to only use one top level task queue. (e.g. the top-level will ask 4 worker bees to process immediate 8 submodules, and they each spawn 4 worker bees to process their immediate 16 submodules, and so on---it is unbounded because we do not know upfront how deep the nesting is). Let's call that a recursive module_list. You would want out of a recursive module_list: - the path to the submodule (or . for the top-level) to indicate where in the nested hierarchy the information came from; - the information the flat module_list gives for that location. I only see value in this recursive module_list approach for updating the work tree (fetch --recurse-submodules=yes, instead of cloning) as you already have most of the submodules there (there may be new submodules after fetching). Also collecting 500 submodule information is in the order of reading 500 files. But then we need to do 500 fetches. And doing a fetch takes some orders of magnitude longer than reading a file, so I am not convinced a parallel collection of work to be done is a good first step? Since you already have module_list() function natively callable from C and also it is available via git submodule--helper module_list, implementing a recursive module_list would be a good first proof of concept exercise for your thread pool engine. You can employ the dual implementation trick to call - a version that tells the thread to run the native C version of module_list(), - another version that tells the thread to run_command() submodule--helper module_list in the top-level and nested submodules. and collect and compare their results and performance. That will not just be a good proof of concept for the pool implementation. Once you have such a recursive module_list, you can use it as a way to easily obtain such a unified view list of all submodules. That can be used to stuff a flat work unit queue to implement reasonably bounded parallelism. Your recursive submoule update implementation could be - Run recursive module_list to stuff the work queue with these 500 submodules (possibly spread across in top-level and in nested submodules, or all 500 in the flat top-level); - Start N worker bees, and tell them to pick from that work queue, each element of which tells them to process which submodule that resides in where (either in the top-level project or in a submodule). And each work element would essentially be to
[PATCH v6] notes: handle multiple worktrees
Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using
find_shared_symref and die if we find one. This prevents simultaneous
merges to the same notes branch from different worktrees.
Signed-off-by: David Turner dtur...@twopensource.com
---
This reroll addresses Eric Sunshine's comments on v5.
---
builtin/notes.c | 6
t/t3320-notes-merge-worktrees.sh | 72
2 files changed, 78 insertions(+)
create mode 100755 t/t3320-notes-merge-worktrees.sh
diff --git a/builtin/notes.c b/builtin/notes.c
index 63f95fc..0423480 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -19,6 +19,7 @@
#include string-list.h
#include notes-merge.h
#include notes-utils.h
+#include branch.h
static const char * const git_notes_usage[] = {
N_(git notes [--ref notes-ref] [list [object]]),
@@ -825,10 +826,15 @@ static int merge(int argc, const char **argv, const char
*prefix)
update_ref(msg.buf, default_notes_ref(), result_sha1, NULL,
0, UPDATE_REFS_DIE_ON_ERR);
else { /* Merge has unresolved conflicts */
+ char *existing;
/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */
update_ref(msg.buf, NOTES_MERGE_PARTIAL, result_sha1, NULL,
0, UPDATE_REFS_DIE_ON_ERR);
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
+ existing = find_shared_symref(NOTES_MERGE_REF,
default_notes_ref());
+ if (existing)
+ die(_(A notes merge into %s is already in-progress at
%s),
+ default_notes_ref(), existing);
if (create_symref(NOTES_MERGE_REF, default_notes_ref(), NULL))
die(Failed to store link to current notes ref (%s),
default_notes_ref());
diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh
new file mode 100755
index 000..a7beef2
--- /dev/null
+++ b/t/t3320-notes-merge-worktrees.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+#
+# Copyright (c) 2015 Twitter, Inc
+#
+
+test_description='Test merging of notes trees in multiple worktrees'
+
+. ./test-lib.sh
+
+test_expect_success 'setup commit' '
+ test_commit tantrum
+'
+
+commit_tantrum=$(git rev-parse tantrum^{commit})
+
+test_expect_success 'setup notes ref (x)' '
+ git config core.notesRef refs/notes/x
+ git notes add -m x notes on tantrum tantrum
+'
+
+test_expect_success 'setup local branch (y)' '
+ git update-ref refs/notes/y refs/notes/x
+ git config core.notesRef refs/notes/y
+ git notes remove tantrum
+'
+
+test_expect_success 'setup remote branch (z)' '
+ git update-ref refs/notes/z refs/notes/x
+ git config core.notesRef refs/notes/z
+ git notes add -f -m conflicting notes on tantrum tantrum
+'
+
+test_expect_success 'modify notes ref ourselves (x)' '
+ git config core.notesRef refs/notes/x
+ git notes add -f -m more conflicting notes on tantrum tantrum
+'
+
+test_expect_success 'create some new worktrees' '
+ git worktree add -b newbranch worktree master
+ git worktree add -b newbranch2 worktree2 master
+'
+
+test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '
+ git config core.notesRef refs/notes/y
+ test_must_fail git notes merge z
+ echo ref: refs/notes/y expect
+ test_cmp .git/NOTES_MERGE_REF expect
+'
+
+test_expect_success 'merge z into y while mid-merge in another workdir fails' '
+ (
+ cd worktree
+ git config core.notesRef refs/notes/y
+ test_must_fail git notes merge z 2err
+ grep A notes merge into refs/notes/y is already in-progress
at err
+ )
+ test_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF
+'
+
+test_expect_success 'merge z into x while mid-merge on y succeeds' '
+ (
+ cd worktree2
+ git config core.notesRef refs/notes/x
+ test_must_fail git notes merge z 21 out
+ grep Automatic notes merge failed out
+ grep -v A notes merge into refs/notes/x is already in-progress
in out
+ )
+ echo ref: refs/notes/x expect
+ test_cmp .git/worktrees/worktree2/NOTES_MERGE_REF expect
+'
+
+test_done
--
2.0.4.315.gad8727a-twtrsrc
--
To unsubscribe from this list: send the line 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 v6 1/2] worktrees: add find_shared_symref
Add a new function, find_shared_symref, which contains the heart of
die_if_checked_out, but works for any symref, not just HEAD. Refactor
die_if_checked_out to use the same infrastructure as
find_shared_symref.
Soon, we will use find_shared_symref to protect notes merges in
worktrees.
Signed-off-by: David Turner dtur...@twopensource.com
---
Please disregard v6.
This version addresses Eric Sunshine's comments on v5. It fixes an error
message and cleans up the code.
---
branch.c | 46 ++
branch.h | 8
2 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/branch.c b/branch.c
index c85be07..07c8b1e 100644
--- a/branch.c
+++ b/branch.c
@@ -311,21 +311,23 @@ void remove_branch_state(void)
unlink(git_path(SQUASH_MSG));
}
-static void check_linked_checkout(const char *branch, const char *id)
+static char *find_linked_symref(const char *symref, const char *branch,
+ const char *id)
{
struct strbuf sb = STRBUF_INIT;
struct strbuf path = STRBUF_INIT;
struct strbuf gitdir = STRBUF_INIT;
+ char *existing = NULL;
/*
-* $GIT_COMMON_DIR/HEAD is practically outside
-* $GIT_DIR so resolve_ref_unsafe() won't work (it
-* uses git_path). Parse the ref ourselves.
+* $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside
+* $GIT_DIR so resolve_ref_unsafe() won't work (it uses
+* git_path). Parse the ref ourselves.
*/
if (id)
- strbuf_addf(path, %s/worktrees/%s/HEAD,
get_git_common_dir(), id);
+ strbuf_addf(path, %s/worktrees/%s/%s, get_git_common_dir(),
id, symref);
else
- strbuf_addf(path, %s/HEAD, get_git_common_dir());
+ strbuf_addf(path, %s/%s, get_git_common_dir(), symref);
if (!strbuf_readlink(sb, path.buf, 0)) {
if (!starts_with(sb.buf, refs/) ||
@@ -347,33 +349,53 @@ static void check_linked_checkout(const char *branch,
const char *id)
strbuf_rtrim(gitdir);
} else
strbuf_addstr(gitdir, get_git_common_dir());
- skip_prefix(branch, refs/heads/, branch);
strbuf_strip_suffix(gitdir, .git);
- die(_('%s' is already checked out at '%s'), branch, gitdir.buf);
+
+ existing = strbuf_detach(gitdir, NULL);
done:
strbuf_release(path);
strbuf_release(sb);
strbuf_release(gitdir);
+
+ return existing;
}
-void die_if_checked_out(const char *branch)
+char *find_shared_symref(const char *symref, const char *target)
{
struct strbuf path = STRBUF_INIT;
DIR *dir;
struct dirent *d;
+ char *existing;
- check_linked_checkout(branch, NULL);
+ if ((existing = find_linked_symref(symref, target, NULL)))
+ return existing;
strbuf_addf(path, %s/worktrees, get_git_common_dir());
dir = opendir(path.buf);
strbuf_release(path);
if (!dir)
- return;
+ return NULL;
while ((d = readdir(dir)) != NULL) {
if (!strcmp(d-d_name, .) || !strcmp(d-d_name, ..))
continue;
- check_linked_checkout(branch, d-d_name);
+ existing = find_linked_symref(symref, target, d-d_name);
+ if (existing)
+ goto done;
}
+done:
closedir(dir);
+
+ return existing;
+}
+
+void die_if_checked_out(const char *branch)
+{
+ char *existing;
+
+ existing = find_shared_symref(HEAD, branch);
+ if (existing) {
+ skip_prefix(branch, refs/heads/, branch);
+ die(_('%s' is already checked out at '%s'), branch, existing);
+ }
}
diff --git a/branch.h b/branch.h
index 58aa45f..d3446ed 100644
--- a/branch.h
+++ b/branch.h
@@ -59,4 +59,12 @@ extern int read_branch_desc(struct strbuf *, const char
*branch_name);
*/
extern void die_if_checked_out(const char *branch);
+/*
+ * Check if a per-worktree symref points to a ref in the main worktree
+ * or any linked worktree, and return the path to the exising worktree
+ * if it is. Returns NULL if there is no existing ref. The caller is
+ * responsible for freeing the returned path.
+ */
+extern char *find_shared_symref(const char *symref, const char *target);
+
#endif
--
2.0.4.315.gad8727a-twtrsrc
--
To unsubscribe from this list: send the line 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 v6 2/2] notes: handle multiple worktrees
Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using
find_shared_symref and die if we find one. This prevents simultaneous
merges to the same notes branch from different worktrees.
Signed-off-by: David Turner dtur...@twopensource.com
---
builtin/notes.c | 6
t/t3320-notes-merge-worktrees.sh | 72
2 files changed, 78 insertions(+)
create mode 100755 t/t3320-notes-merge-worktrees.sh
diff --git a/builtin/notes.c b/builtin/notes.c
index 63f95fc..0423480 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -19,6 +19,7 @@
#include string-list.h
#include notes-merge.h
#include notes-utils.h
+#include branch.h
static const char * const git_notes_usage[] = {
N_(git notes [--ref notes-ref] [list [object]]),
@@ -825,10 +826,15 @@ static int merge(int argc, const char **argv, const char
*prefix)
update_ref(msg.buf, default_notes_ref(), result_sha1, NULL,
0, UPDATE_REFS_DIE_ON_ERR);
else { /* Merge has unresolved conflicts */
+ char *existing;
/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */
update_ref(msg.buf, NOTES_MERGE_PARTIAL, result_sha1, NULL,
0, UPDATE_REFS_DIE_ON_ERR);
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
+ existing = find_shared_symref(NOTES_MERGE_REF,
default_notes_ref());
+ if (existing)
+ die(_(A notes merge into %s is already in-progress at
%s),
+ default_notes_ref(), existing);
if (create_symref(NOTES_MERGE_REF, default_notes_ref(), NULL))
die(Failed to store link to current notes ref (%s),
default_notes_ref());
diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh
new file mode 100755
index 000..a7beef2
--- /dev/null
+++ b/t/t3320-notes-merge-worktrees.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+#
+# Copyright (c) 2015 Twitter, Inc
+#
+
+test_description='Test merging of notes trees in multiple worktrees'
+
+. ./test-lib.sh
+
+test_expect_success 'setup commit' '
+ test_commit tantrum
+'
+
+commit_tantrum=$(git rev-parse tantrum^{commit})
+
+test_expect_success 'setup notes ref (x)' '
+ git config core.notesRef refs/notes/x
+ git notes add -m x notes on tantrum tantrum
+'
+
+test_expect_success 'setup local branch (y)' '
+ git update-ref refs/notes/y refs/notes/x
+ git config core.notesRef refs/notes/y
+ git notes remove tantrum
+'
+
+test_expect_success 'setup remote branch (z)' '
+ git update-ref refs/notes/z refs/notes/x
+ git config core.notesRef refs/notes/z
+ git notes add -f -m conflicting notes on tantrum tantrum
+'
+
+test_expect_success 'modify notes ref ourselves (x)' '
+ git config core.notesRef refs/notes/x
+ git notes add -f -m more conflicting notes on tantrum tantrum
+'
+
+test_expect_success 'create some new worktrees' '
+ git worktree add -b newbranch worktree master
+ git worktree add -b newbranch2 worktree2 master
+'
+
+test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '
+ git config core.notesRef refs/notes/y
+ test_must_fail git notes merge z
+ echo ref: refs/notes/y expect
+ test_cmp .git/NOTES_MERGE_REF expect
+'
+
+test_expect_success 'merge z into y while mid-merge in another workdir fails' '
+ (
+ cd worktree
+ git config core.notesRef refs/notes/y
+ test_must_fail git notes merge z 2err
+ grep A notes merge into refs/notes/y is already in-progress
at err
+ )
+ test_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF
+'
+
+test_expect_success 'merge z into x while mid-merge on y succeeds' '
+ (
+ cd worktree2
+ git config core.notesRef refs/notes/x
+ test_must_fail git notes merge z 21 out
+ grep Automatic notes merge failed out
+ grep -v A notes merge into refs/notes/x is already in-progress
in out
+ )
+ echo ref: refs/notes/x expect
+ test_cmp .git/worktrees/worktree2/NOTES_MERGE_REF expect
+'
+
+test_done
--
2.0.4.315.gad8727a-twtrsrc
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 1/2] config: add '--names-only' option to list only variable names
SZEDER Gábor sze...@ira.uka.de writes:
'git config' can only show values or name-value pairs, so if a shell
script needs the names of set config variables it has to run 'git config
--list' or '--get-regexp' and parse the output to separate config
variable names from their values. However, such a parsing can't cope
with multi-line values. Though 'git config' can produce null-terminated
output for newline-safe parsing, that's of no use in such a case, becase
s/becase/because/;
shells can't cope with null characters.
Even our own bash completion script suffers from these issues.
Help the completion script, and shell scripts in general, by introducing
the '--names-only' option to modify the output of '--list' and
'--get-regexp' to list only the names of config variables, so they don't
have to perform error-prone post processing to separate variable names
from their values anymore.
I agree with Peff that --names-only has a subtle difference with
an existing and well known subcommand option and it would be a bit
irritating to remember which options is for which command.
diff --git a/builtin/config.c b/builtin/config.c
index 7188405f7e..307980ab50 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int omit_values;
static int use_key_regexp;
static int do_all;
static int do_not_match;
...
@@ -91,7 +93,7 @@ static void check_argc(int argc, int min, int max) {
static int show_all_config(const char *key_, const char *value_, void *cb)
{
- if (value_)
+ if (!omit_values value_)
H. As we have show_keys,
if (show_values value_)
would be a lot more intuitive, no?
@@ -117,6 +119,10 @@ static int format_config(struct strbuf *buf, const char
*key_, const char *value
strbuf_addstr(buf, key_);
must_print_delim = 1;
}
+ if (omit_values) {
+ strbuf_addch(buf, term);
+ return 0;
+ }
This hunk makes me wonder what the assignment to must_print_delim
is about. When the code is told to show only keys and not values,
it shouldn't even have to worry about key_delim, but that assignment
is done to control exactly that. It happens that you are lucky that
you can return 0 early here so that the assignment does not have
any effect, but still conceptually the code structure is made ugly
by this patch.
Isn't it more like the existing show_keys can be replaced/enhanced
with a single show tri-state toggle that chooses one among:
* show both keys and values (for --list)
* show only keys (for your new feature)
* show only value (for --get)
perhaps?
I see get_urlmatch() abuses show_keys variable in a strange way, and
it may not be as trivial as removing show_keys and replacing it with
a new tri-state toggle, 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] sha1_file.c: rename move_temp_to_file() to finalize_temp_file()
Jeff King p...@peff.net writes: On Fri, Aug 07, 2015 at 05:24:29PM -0700, Junio C Hamano wrote: Since 5a688fe4 (core.sharedrepository = 0mode should set, not loosen, 2009-03-25), we kept reminding ourselves: NEEDSWORK: this should be renamed to finalize_temp_file() as moving is only a part of what it does, when no patch between master to pu changes the call sites of this function. without doing anything about it. Let's do so. The purpose of this function was not to move but to finalize. The detail of the primarily implementation of finalizing was to link the temporary file to its final name and then to unlink, which wasn't even moving. The alternative implementation did move by calling rename(2), which is a fun tangent. This is definitely a better name. But while we are touching the area, my other pet peeve about this function is that it is really specific to moving _object_ temp files around. It started life as static-local to sha1-file.c, so not mentioning objects is OK, but when it became a global, it's a bit confusing. Maybe finalize_object_file() would be a better name? -Peff OK, will fix. -- To unsubscribe from this list: send the line unsubscribe 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 v6] notes: handle multiple worktrees
Sorry, that should have included the first patch as well. Will re-send
as .v7
On Mon, 2015-08-10 at 13:43 -0400, David Turner wrote:
Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using
find_shared_symref and die if we find one. This prevents simultaneous
merges to the same notes branch from different worktrees.
Signed-off-by: David Turner dtur...@twopensource.com
---
This reroll addresses Eric Sunshine's comments on v5.
---
builtin/notes.c | 6
t/t3320-notes-merge-worktrees.sh | 72
2 files changed, 78 insertions(+)
create mode 100755 t/t3320-notes-merge-worktrees.sh
diff --git a/builtin/notes.c b/builtin/notes.c
index 63f95fc..0423480 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -19,6 +19,7 @@
#include string-list.h
#include notes-merge.h
#include notes-utils.h
+#include branch.h
static const char * const git_notes_usage[] = {
N_(git notes [--ref notes-ref] [list [object]]),
@@ -825,10 +826,15 @@ static int merge(int argc, const char **argv, const
char *prefix)
update_ref(msg.buf, default_notes_ref(), result_sha1, NULL,
0, UPDATE_REFS_DIE_ON_ERR);
else { /* Merge has unresolved conflicts */
+ char *existing;
/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */
update_ref(msg.buf, NOTES_MERGE_PARTIAL, result_sha1, NULL,
0, UPDATE_REFS_DIE_ON_ERR);
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
+ existing = find_shared_symref(NOTES_MERGE_REF,
default_notes_ref());
+ if (existing)
+ die(_(A notes merge into %s is already in-progress at
%s),
+ default_notes_ref(), existing);
if (create_symref(NOTES_MERGE_REF, default_notes_ref(), NULL))
die(Failed to store link to current notes ref (%s),
default_notes_ref());
diff --git a/t/t3320-notes-merge-worktrees.sh
b/t/t3320-notes-merge-worktrees.sh
new file mode 100755
index 000..a7beef2
--- /dev/null
+++ b/t/t3320-notes-merge-worktrees.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+#
+# Copyright (c) 2015 Twitter, Inc
+#
+
+test_description='Test merging of notes trees in multiple worktrees'
+
+. ./test-lib.sh
+
+test_expect_success 'setup commit' '
+ test_commit tantrum
+'
+
+commit_tantrum=$(git rev-parse tantrum^{commit})
+
+test_expect_success 'setup notes ref (x)' '
+ git config core.notesRef refs/notes/x
+ git notes add -m x notes on tantrum tantrum
+'
+
+test_expect_success 'setup local branch (y)' '
+ git update-ref refs/notes/y refs/notes/x
+ git config core.notesRef refs/notes/y
+ git notes remove tantrum
+'
+
+test_expect_success 'setup remote branch (z)' '
+ git update-ref refs/notes/z refs/notes/x
+ git config core.notesRef refs/notes/z
+ git notes add -f -m conflicting notes on tantrum tantrum
+'
+
+test_expect_success 'modify notes ref ourselves (x)' '
+ git config core.notesRef refs/notes/x
+ git notes add -f -m more conflicting notes on tantrum tantrum
+'
+
+test_expect_success 'create some new worktrees' '
+ git worktree add -b newbranch worktree master
+ git worktree add -b newbranch2 worktree2 master
+'
+
+test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '
+ git config core.notesRef refs/notes/y
+ test_must_fail git notes merge z
+ echo ref: refs/notes/y expect
+ test_cmp .git/NOTES_MERGE_REF expect
+'
+
+test_expect_success 'merge z into y while mid-merge in another workdir
fails' '
+ (
+ cd worktree
+ git config core.notesRef refs/notes/y
+ test_must_fail git notes merge z 2err
+ grep A notes merge into refs/notes/y is already in-progress
at err
+ )
+ test_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF
+'
+
+test_expect_success 'merge z into x while mid-merge on y succeeds' '
+ (
+ cd worktree2
+ git config core.notesRef refs/notes/x
+ test_must_fail git notes merge z 21 out
+ grep Automatic notes merge failed out
+ grep -v A notes merge into refs/notes/x is already in-progress
in out
+ )
+ echo ref: refs/notes/x expect
+ test_cmp .git/worktrees/worktree2/NOTES_MERGE_REF expect
+'
+
+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: File Hash for Windows executable
Ah, perfect. Thank you. I was looking at 1.9.5 from http://www.git-scm.com/download Thanks for pointing me at the right place. -Ryan -Original Message- From: Johannes Schindelin [mailto:johannes.schinde...@gmx.de] Sent: Monday, August 10, 2015 12:10 PM To: Kiser, Ryan Lee rlki...@iu.edu Cc: git@vger.kernel.org Subject: Re: File Hash for Windows executable Hi Ryan, On 2015-08-10 15:54, Kiser, Ryan Lee wrote: I've downloaded the Windows installer for Git, Which one. but can't seem to find hashes published anywhere for verification. Since the downloaded file doesn't seem to be signed, The newest ones from https://git-for-windows.github.io/ are signed. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe 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 v5 0/5] Improve guessing of repository names
Patrick Steinhardt p...@pks.im writes: This is version 5 of my patch series which aims to improve guessed directory names in several cases. This version now includes the patches from Jeff which were previously a separate patch series. Besides fixing behavior when cloning a repository with trailing '.git' suffix they also add a new test suite that verifies guessed directory names. I've amended 'clone: add tests for output directory' to add additional tests (as proposed by Junio). Changes to my own patches include improved commit messages detailing why certain changes are done the way they are done and a change to authentication-data-stripping, such that we now strip greedily until the last '@' sign in the host part (proposed by Junio, as well). Jeff King (2): clone: add tests for output directory clone: use computed length in guess_dir_name Patrick Steinhardt (3): clone: do not include authentication data in guessed dir clone: do not use port number as dir name clone: abort if no dir name could be guessed builtin/clone.c | 65 +- t/t5603-clone-dirname.sh | 103 +++ 2 files changed, 157 insertions(+), 11 deletions(-) create mode 100755 t/t5603-clone-dirname.sh Looks good. I'll forge your sign-off for two patches from Peff and queue the whole thing. 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: Feature: git stash pop --always-drop
Ed Avis e...@waniasset.com writes: Yes, my use case is that I get confused about whether the stash has been dropped or not and whether I might have stashed something else in the meantime. So for me plain 'git stash drop' feels a bit dangerous. Then git stash apply followed by git stash drop would be a pair of good workflow elements for you, no? -- To unsubscribe from this list: send the line unsubscribe 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 v5 0/4] submodule config lookup API
On Mon, Jun 15, 2015 at 2:48 PM, Junio C Hamano gits...@pobox.com wrote:
Thanks. Will replace and wait for comments from others.
I have reviewed the patches carefully and they look good to me.
As Git is a large project and I was active in other parts until now,
I noticed that there are subtle differences in style as when compared
to the refs code. One example would be the way comments are written.
In d378e35d256348f (Patch 1, implement submodule config API for
lookup of .gitmodules values) the comments for the data structures in
submodule-config.c seem to have a non exposed headline and if more
is needed proper sentences with capitalized starts and punctuation at the
end. In the refs code there are only sentences IIRC. Most of the commits
touching submodule.{c,h} do not prefix their commit message with
submodule:
The style is no show stopper of course, just an observation from someone
moving into a different area of code.
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: git blame breaking on repository with CRLF files
On 2015-08-10 20.48, Junio C Hamano wrote:
Torsten Bögershausen tbo...@web.de writes:
Actually I could reproduce the following:
CRLF in repo, CRLF in working tree, core.autocrlf= true.
What should happen in such a case? Wouldn't autocrlf=true want to
strip CRLF down to LF? Shouldn't it?
A problem is, that git status would report a file as changed,
when it have been committed with CRLF and core.autocrlf was false.
The only change that git status would trigger on would be the EOL
normalization.
So if core.autocrlf would be set true later,
git status reports files as changed.
Long story short:
Once commited with CRLF, the files will not be normalized in a modern git:
From convert.c:
if (crlf_action == CRLF_GUESS) {
/*
* If the file in the index has any CR in it, do not convert.
* This is the new safer autocrlf handling.
*/
if (has_cr_in_index(path))
return 0;
}
-
commit fd6cce9e89ab5ac1125a3b5f5611048ad22379e7
Author: Eyvind Bernhardsen eyvind.bernhard...@gmail.com
Date: Wed May 19 22:43:10 2010 +0200
Add per-repository eol normalization
Change the semantics of the crlf attribute so that it enables
end-of-line normalization when it is set, regardless of core.autocrlf.
Add a new setting for crlf: auto, which enables end-of-line
conversion but does not override the automatic text file detection.
Add a new attribute eol with possible values crlf and lf. When
set, this attribute enables normalization and forces git to use CRLF or
LF line endings in the working directory, respectively.
The line ending style to be used for normalized text files in the
working directory is set using core.autocrlf. When it is set to
true, CRLFs are used in the working directory; when set to input or
false, LFs are used.
-
So git status is somewhat improved, but git blame is not.
(My feeling/suspicion is that has_cr_in_index() should be replaced
by has_cr_in_latest_commit() to have git status consistent
with git blame, but more analyzes may be needed.)
A different approach could be to ignore the EOL differences
completely in git blame (when core.autocrlf is set and the file
is text, or when the text attribute is set).
And if so, blame is correct
to say that you are changing the line endings of all your lines, as
what you _would_ commit if you were to commit the tracked files in
your working tree would be different from what is in the index, no?
--
To unsubscribe from this list: send the line 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 v3] worktree: add 'list' command
Differences from [v2](http://www.mail-archive.com/git@vger.kernel.org/msg75467.html) - removed unintended whitespace changes - cleanup based on comments from v2 Michael Rappazzo (1): worktree: add 'list' command Documentation/git-worktree.txt | 6 +++- builtin/worktree.c | 67 ++ t/t2027-worktree-list.sh | 30 +++ 3 files changed, 102 insertions(+), 1 deletion(-) create mode 100755 t/t2027-worktree-list.sh -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] config.mak.uname: Cygwin: Use renames for creation
Adam Dinwoodie a...@dinwoodie.org writes: If the desired goal is that Cygwin's link(2) acts like POSIX link(2) on network drives, I'm not convinced that's possible, at least not by emulating `core.createObject = rename` in the Cygwin library layer. The way core.createObject=rename makes things work is by avoiding link(2) in the first place and using rename(2) instead. You might be able to emulate rename(2) of A to B by doing a link(2) of A to B and then unlink(2) of A, but I do not think it is reasonable for the system call emulation layer to detect a sequence of link followed by unlink and use rename, i.e. emulationg the other way around. So I suspect fix in Cygwin is a non-starter. But in the end, I'd prefer the choice of the compiled-in default up to the port maintainers. You earlier said: This problem was reported on the Cygwin mailing list at https://cygwin.com/ml/cygwin/2015-08/msg00102.html (amongst others) and is being applied as a manual patch to the Cygwin builds until the patch is taken here. so my preference is to see Cygwin continue to do so for a couple release cycles of ours to make sure all Cygwin end-users are happy and consider the flip of the default a good change for them, and then the official Cygwin packager of Git sends a patch our way. https://cygwin.com/ml/cygwin/2015-08/msg00102.html seems to indicate that somebody called Adam Dinwoodie is wearing Git maintainer hat, so perhaps you may be that official Cygwin packager of Git ;-) I agree with everything you said in that message to Peter---the patch should be included when you hear reports of `git config core.createObject rename` helping more people. After versions of Cygwin Git package with such a change proves good, let's reduce the workload of Cygwin Git maintainer by upstreaming that change to my tree. But not before. 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
Your Donation to you is ready, contact melc...@gmail.com for more details -- To unsubscribe from this list: send the line 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 v3] worktree: add 'list' command
'git worktree list' will list the main worktree followed by any linked
worktrees which were created using 'git worktree add'.
Signed-off-by: Michael Rappazzo rappa...@gmail.com
---
Documentation/git-worktree.txt | 6 +++-
builtin/worktree.c | 67 ++
t/t2027-worktree-list.sh | 30 +++
3 files changed, 102 insertions(+), 1 deletion(-)
create mode 100755 t/t2027-worktree-list.sh
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 3387e2f..efb8e9d 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
[verse]
'git worktree add' [-f] [--detach] [-b new-branch] path [branch]
'git worktree prune' [-n] [-v] [--expire expire]
+'git worktree list'
DESCRIPTION
---
@@ -59,6 +60,10 @@ prune::
Prune working tree information in $GIT_DIR/worktrees.
+list::
+
+List the main worktree followed by all of the linked worktrees.
+
OPTIONS
---
@@ -167,7 +172,6 @@ performed manually, such as:
- `remove` to remove a linked worktree and its administrative files (and
warn if the worktree is dirty)
- `mv` to move or rename a worktree and update its administrative files
-- `list` to list linked worktrees
- `lock` to prevent automatic pruning of administrative files (for instance,
for a worktree on a portable device)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6a264ee..d90bdee 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -10,6 +10,7 @@
static const char * const worktree_usage[] = {
N_(git worktree add [options] path branch),
N_(git worktree prune [options]),
+ N_(git worktree list [options]),
NULL
};
@@ -316,6 +317,70 @@ static int add(int ac, const char **av, const char *prefix)
return add_worktree(path, cmd.argv);
}
+static int list(int ac, const char **av, const char *prefix)
+{
+ int main_only = 0;
+ struct option options[] = {
+ OPT_BOOL(0, main-only, main_only, N_(only list the main
worktree)),
+ OPT_END()
+ };
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac)
+ usage_with_options(worktree_usage, options);
+
+ struct strbuf main_path = STRBUF_INIT;
+ const char* common_dir = get_git_common_dir();
+ int is_bare = is_bare_repository();
+ if (is_bare) {
+ strbuf_addstr(main_path, absolute_path(common_dir));
+ strbuf_strip_suffix(main_path, /.);
+ } else if (!strcmp(common_dir, .git)) {
+ /* within a worktree, the common dir only returns .git */
+ strbuf_addstr(main_path, get_git_work_tree());
+ } else {
+ strbuf_addstr(main_path, common_dir);
+ strbuf_strip_suffix(main_path, /.git);
+ }
+
+ printf(%s\n, main_path.buf);
+
+ if (!is_bare) {
+ strbuf_addstr(main_path, /.git);
+ }
+ strbuf_addstr(main_path, /worktrees);
+
+ if (is_directory(main_path.buf)) {
+ DIR *dir = opendir(main_path.buf);
+ if (dir) {
+ struct dirent *d;
+ struct stat st;
+ struct strbuf path = STRBUF_INIT;
+ struct strbuf other_worktree_path = STRBUF_INIT;
+ while ((d = readdir(dir)) != NULL) {
+ if (!strcmp(d-d_name, .) ||
!strcmp(d-d_name, ..))
+ continue;
+ strbuf_reset(other_worktree_path);
+ strbuf_addf(other_worktree_path,
%s/%s/gitdir, main_path.buf, d-d_name);
+
+ if (stat(other_worktree_path.buf, st))
+ continue;
+
+ strbuf_reset(path);
+ strbuf_read_file(path,
other_worktree_path.buf, st.st_size);
+ strbuf_strip_suffix(path, /.git\n);
+
+ printf(%s\n, path.buf);
+ }
+ strbuf_release(other_worktree_path);
+ strbuf_release(path);
+ }
+ closedir(dir);
+ }
+ strbuf_release(main_path);
+ return 0;
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -328,5 +393,7 @@ int cmd_worktree(int ac, const char **av, const char
*prefix)
return add(ac - 1, av + 1, prefix);
if (!strcmp(av[1], prune))
return prune(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], list))
+ return list(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
new file mode 100755
index
Re: [PATCH v3] worktree: add 'list' command
Michael Rappazzo rappa...@gmail.com writes:
+static int list(int ac, const char **av, const char *prefix)
+{
+ int main_only = 0;
+ struct option options[] = {
+ OPT_BOOL(0, main-only, main_only, N_(only list the main
worktree)),
+ OPT_END()
+ };
Hmm, main-only is still there?
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac)
+ usage_with_options(worktree_usage, options);
+
+ struct strbuf main_path = STRBUF_INIT;
+ const char* common_dir = get_git_common_dir();
Asterisks stick to the variable names, not types.
+ int is_bare = is_bare_repository();
Please do not introduce decl-after-stmt.
+ if (is_bare) {
+ strbuf_addstr(main_path, absolute_path(common_dir));
Hmm, interesting.
Because .git/config is shared, core.bare read from that tells us if
the main one is bare, even if you start this command from one of
its linked worktrees. So in that sense, this test of is_bare
correctly tells if main one is a bare repository.
But that by itself feels wrong. Doesn't the presense of a working
tree mean that you should not get is_bare==true in such a case
(i.e. your main one is bare, you are in a linked worktree of it
that has the index and the working tree)?
Duy? Eric? What do you guys think?
There are many codepaths that change their behaviour (e.g. if we
create reflogs by default) based on the return value of
is_bare_repository(). If I am reading this correctly, I _think_ a
new working area that was prepared with git worktree add out of a
bare repository would not work well, as these operations behave as
if we do not have a working tree. Perhaps is_bare_repository() in
such a working area _should_ say No, even if core.bare in the
shared bare one is set to true.
+ strbuf_strip_suffix(main_path, /.);
In any case, what is that stripping of /. about? Who is adding
that extra trailing string?
What I am getting at is (1) perhaps it shouldn't be adding that in
the first place, and (2) if some other code is randomly adding /.
at the end, what guarantees you that you would need to strip it only
once here---if the other code added that twice, don't you have to
repeatedly remove /. from the end?
--
To unsubscribe from this list: send the line unsubscribe 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 v6 1/2] worktrees: add find_shared_symref
On Mon, Aug 10, 2015 at 1:52 PM, David Turner dtur...@twopensource.com wrote: worktrees: add find_shared_symref s/worktrees/branch/ perhaps? Add a new function, find_shared_symref, which contains the heart of die_if_checked_out, but works for any symref, not just HEAD. Refactor die_if_checked_out to use the same infrastructure as find_shared_symref. Soon, we will use find_shared_symref to protect notes merges in worktrees. Signed-off-by: David Turner dtur...@twopensource.com --- This version addresses Eric Sunshine's comments on v5. It fixes an error message and cleans up the code. All issues identified in previous versions seem to have been addressed, and nothing else pops out at me. 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 05/17] remove hold_lock_file_for_append
Jeff King p...@peff.net writes: No users of hold_lock_file_for_append remain, so remove it. This does not seem to have anything to do with rotating static buffers used in get_pathname(); the only effect it has is to conflict heavily with Michael's tempfile topic X-. Perhaps this should be part of Michael's tempfile topic? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Error pushing new branch: value too great for base (error token is...
On Mon, Aug 10, 2015 at 6:29 PM, Jacob Keller jacob.kel...@gmail.com wrote: On Mon, Aug 10, 2015 at 2:54 AM, Gaurav Chhabra varuag.chha...@gmail.com wrote: Apologies for the delay in reply! I tried your suggestion and it works. Thanks! :) I'm curious why integer comparison is throwing error. Shouldn't i be comparing numbers with numeric operator? Yes, but shell doesn't treat hex numbers as numbers. So it will work only if the string is a decimal number. This particular case deserves a bit more explanation. The expression in question was this: if [[ $new_sha -eq $NULL ]]; then where 'new_sha' was 9226289d2416af4cb7365d7aaa5e382bdb3d9a89. In Bash, inside the [[ .. ]], it did attempt evaluating the SHA1 as a *decimal* number, however, when it encountered the d, it complained that it was outside the allowed range of decimal digits (0...9). Had the SHA1 been prefixed by a 0x, the [[...]] context would have dealt with it just fine. Outside the [[...]] context, arguments to -eq do need to be base-10 integers. Nevertheless, a SHA1 is effective an opaque value. There's little, if anything, to be gained by treating it as a numeric quantity, hence string '=' makes more sense than numeric '-eq'. -- To unsubscribe from this list: send the line unsubscribe 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 10/17] path.c: drop git_path_submodule
Jeff King p...@peff.net writes: There are no callers of the slightly-dangerous static-buffer git_path_submodule left. Let's drop it. There are a few callers added on 'pu', 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: git blame breaking on repository with CRLF files
On Mon, Aug 10, 2015 at 08:36:35AM +, Benkstein, Frank wrote: You are correct that it is also wrong in git v1.7.0. However, it is correct in v2.4.0. Another bisect gave me this commit which was included in v2.0.1: commit 4d4813a52f3722854a54bab046f4abfec13ef6ae Author: brian m. carlson sand...@crustytoothpaste.net Date: Sat Apr 26 23:10:40 2014 + blame: correctly handle files regardless of autocrlf So this still looks like a regression v2.5.0 to me. This commit was reverted because it was decided that it wasn't the right way to handle the problem and it broke other things. The complexity of the CRLF handling is a bit beyond me, to be honest. I'm sure I'd understand it better if I used it more, but I'm a Unix guy. I stand by my earlier statement that we should improve the documentation in this area, because it's a common source of confusion. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v2] send-email: provide whitelist of SMTP AUTH mechanisms
On Mon, Aug 10, 2015 at 6:06 AM, Jan Viktorin vikto...@rehivetech.com wrote: On Sun, 9 Aug 2015 14:13:33 -0400 Eric Sunshine sunsh...@sunshineco.com wrote: One possibility which comes to mind is to create a fake Authen::SASL::Perl which merely dumps its input mechanisms to a file, and arrange for the Perl search path to find the fake one instead. You could then check the output file to see if it reflects your expectations. However, this may be overkill and perhaps not worth the effort (especially if you're not a Perl programmer). I think that Authen::SASL::Perl mock would not help. I wanted to create some fake sendmail (but this is impossible as stated above because then the perl modules are not used). So the only way would be to provide some fake socket with a static content on the other side. This is really an overkill to just test the few lines of code. Agreed. So, what more can I do for this feature? I don't have any further suggestions. Other than the unwanted Supported: line in the documentation and the couple style issues[1], the patch seems sufficiently complete, as-is. The validation regex gets a meh from me merely because it's not clear how beneficial it will be in practice, but that's not an outright objection; I don't feel strongly about it either way. [1]: http://article.gmane.org/gmane.comp.version-control.git/275150 I think that the basic regex test is OK. It can accept lowercase letters and do an explicit uppercase call. I do not like to rely on internals of the SASL library. As you could see, the SASL::Perl does not check its inputs in a very good way and its code is quite unclear (strange for a library providing security features). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Error pushing new branch: value too great for base (error token is...
On Mon, Aug 10, 2015 at 2:54 AM, Gaurav Chhabra varuag.chha...@gmail.com wrote: Apologies for the delay in reply! I tried your suggestion and it works. Thanks! :) I'm curious why integer comparison is throwing error. Shouldn't i be comparing numbers with numeric operator? Yes, but shell doesn't treat hex numbers as numbers. So it will work only if the string is a decimal number. Regards, Jake -- To unsubscribe from this list: send the line unsubscribe 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 v6 1/2] worktrees: add find_shared_symref
On Mon, 2015-08-10 at 18:30 -0400, Eric Sunshine wrote: On Mon, Aug 10, 2015 at 1:52 PM, David Turner dtur...@twopensource.com wrote: worktrees: add find_shared_symref s/worktrees/branch/ perhaps? Do you mean this is in branch.c, so should be labeled with branch? Because this change is mostly about non-branch refs, so I think saying branch is confusing. That's why I labelled it worktrees; that's the broad topic that's being addressed. Add a new function, find_shared_symref, which contains the heart of die_if_checked_out, but works for any symref, not just HEAD. Refactor die_if_checked_out to use the same infrastructure as find_shared_symref. Soon, we will use find_shared_symref to protect notes merges in worktrees. Signed-off-by: David Turner dtur...@twopensource.com --- This version addresses Eric Sunshine's comments on v5. It fixes an error message and cleans up the code. All issues identified in previous versions seem to have been addressed, and nothing else pops out at me. 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 v6 1/2] worktrees: add find_shared_symref
On Mon, Aug 10, 2015 at 6:42 PM, David Turner dtur...@twopensource.com wrote: On Mon, 2015-08-10 at 18:30 -0400, Eric Sunshine wrote: On Mon, Aug 10, 2015 at 1:52 PM, David Turner dtur...@twopensource.com wrote: worktrees: add find_shared_symref s/worktrees/branch/ perhaps? Do you mean this is in branch.c, so should be labeled with branch? Because this change is mostly about non-branch refs, so I think saying branch is confusing. That's why I labelled it worktrees; that's the broad topic that's being addressed. Okay. -- To unsubscribe from this list: send the line unsubscribe 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 10/17] path.c: drop git_path_submodule
Junio C Hamano gits...@pobox.com writes:
Jeff King p...@peff.net writes:
There are no callers of the slightly-dangerous static-buffer
git_path_submodule left. Let's drop it.
There are a few callers added on 'pu', though.
Actually there is only one. Here is a proposed evil merge.
diff --git a/submodule.c b/submodule.c
index dfe8b7b..7ab08a1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -120,34 +120,35 @@ void stage_updated_gitmodules(void)
static int add_submodule_odb(const char *path)
{
struct alternate_object_database *alt_odb;
- const char *objects_directory;
+ struct strbuf objects_directory = STRBUF_INIT;
int ret = 0;
- objects_directory = git_path_submodule(path, objects/);
- if (!is_directory(objects_directory)) {
+ strbuf_git_path_submodule(objects_directory, objects/, %s, path);
+ if (!is_directory(objects_directory.buf)) {
ret = -1;
goto done;
}
/* avoid adding it twice */
for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb-next)
- if (alt_odb-name - alt_odb-base == strlen(objects_directory)
- !strcmp(alt_odb-base, objects_directory))
+ if (alt_odb-name - alt_odb-base == objects_directory.len
+ !strcmp(alt_odb-base, objects_directory.buf))
goto done;
- alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));
+ alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));
alt_odb-next = alt_odb_list;
- strcpy(alt_odb-base, objects_directory);
- alt_odb-name = alt_odb-base + strlen(objects_directory);
+ strcpy(alt_odb-base, objects_directory.buf);
+ alt_odb-name = alt_odb-base + objects_directory.len;
alt_odb-name[2] = '/';
alt_odb-name[40] = '\0';
alt_odb-name[41] = '\0';
alt_odb_list = alt_odb;
/* add possible alternates from the submodule */
- read_info_alternates(objects_directory, 0);
+ read_info_alternates(objects_directory.buf, 0);
prepare_alt_odb();
done:
+ strbuf_release(objects_directory);
return ret;
}
--
To unsubscribe from this list: send the line unsubscribe 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 10/17] path.c: drop git_path_submodule
Junio C Hamano gits...@pobox.com writes:
Junio C Hamano gits...@pobox.com writes:
Jeff King p...@peff.net writes:
There are no callers of the slightly-dangerous static-buffer
git_path_submodule left. Let's drop it.
There are a few callers added on 'pu', though.
Actually there is only one. Here is a proposed evil merge.
Sorry, that didn't work X-.
diff --git a/submodule.c b/submodule.c
index dfe8b7b..0cdaeb8 100644
--- a/submodule.c
+++ b/submodule.c
@@ -120,10 +120,10 @@ void stage_updated_gitmodules(void)
static int add_submodule_odb(const char *path)
{
struct alternate_object_database *alt_odb;
- const char *objects_directory;
+ char *objects_directory;
int ret = 0;
- objects_directory = git_path_submodule(path, objects/);
+ objects_directory = git_pathdup_submodule(path, objects/);
if (!is_directory(objects_directory)) {
ret = -1;
goto done;
@@ -148,6 +148,7 @@ static int add_submodule_odb(const char *path)
read_info_alternates(objects_directory, 0);
prepare_alt_odb();
done:
+ free(objects_directory);
return ret;
}
--
To unsubscribe from this list: send the line unsubscribe 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 04/17] add_to_alternates_file: don't add duplicate entries
On 08/10/2015 11:34 AM, Jeff King wrote: The add_to_alternates_file function blindly uses hold_lock_file_for_append to copy the existing contents, and then adds the new line to it. This has two minor problems: 1. We might add duplicate entries, which are ugly and inefficient. 2. We do not check that the file ends with a newline, in which case we would bogusly append to the final line. This is quite unlikely in practice, though, as we call this function only from git-clone, so presumably we are the only writers of the file (and we always add a newline). Instead of using hold_lock_file_for_append, let's copy the file line by line, which ensures all records are properly terminated. If we see an extra line, we can simply abort the update (there is no point in even copying the rest, as we know that it would be identical to the original). Do we have reason to expect that a lot of people have alternates files that already contain duplicate lines? You say that this function is only called from `git clone`, so I guess the answer is no. But if I'm wrong, it might be friendly to de-dup the existing lines while copying them. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe 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 04/10] branch: roll show_detached HEAD into regular ref_list
On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak karthik@gmail.com wrote:
Remove show_detached() and make detached HEAD to be rolled into
regular ref_list by adding REF_DETACHED_HEAD as a kind of branch and
supporting the same in append_ref(). This eliminates the need for an
extra function and helps in easier porting of branch.c to use
ref-filter APIs.
Before show_detached() used to check if the HEAD branch satisfies the
'--contains' option, now that is taken care by append_ref().
Signed-off-by: Karthik Nayak karthik@gmail.com
---
diff --git a/builtin/branch.c b/builtin/branch.c
index 65f6d0d..81815c9 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -535,6 +540,7 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
int color;
struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
const char *prefix = ;
+ const char *desc = item-name;
if (item-ignore)
return;
@@ -547,6 +553,10 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
color = BRANCH_COLOR_REMOTE;
prefix = remote_prefix;
break;
+ case REF_DETACHED_HEAD:
+ color = BRANCH_COLOR_CURRENT;
+ desc = get_head_description();
+ break;
default:
color = BRANCH_COLOR_PLAIN;
break;
@@ -558,7 +568,7 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
color = BRANCH_COLOR_CURRENT;
}
- strbuf_addf(name, %s%s, prefix, item-name);
+ strbuf_addf(name, %s%s, prefix, desc);
if (verbose) {
int utf8_compensation = strlen(name.buf) -
utf8_strwidth(name.buf);
strbuf_addf(out, %c %s%-*s%s, c, branch_get_color(color),
@@ -581,6 +591,8 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
}
strbuf_release(name);
strbuf_release(out);
+ if (item-kind == REF_DETACHED_HEAD)
+ free((void *)desc);
This would be cleaner, and more easily extended to other cases if you
used a 'to_free' variable. For instance, something like this:
const char *desc = item-name;
char *to_free = NULL;
...
case REF_DETACHED_HEAD:
...
desc = to_free = get_head_description();
break;
...
strbuf_addf(name, %s%s, prefix, desc);
...
free(to_free);
Note that it's safe to free 'to_free' when it's NULL, so you don't
need to protect the free() with that ugly specialized 'if' which
checks for REF_DETACHED_HEAD. You can also do away with the cast to
non-const when freeing.
}
@@ -642,7 +638,14 @@ static int print_ref_list(int kinds, int detached, int
verbose, int abbrev, stru
cb.ref_list = ref_list;
cb.pattern = pattern;
cb.ret = 0;
+ /*
+* First we obtain all regular branch refs and then if the
s/then//
+* HEAD is detached then we insert that ref to the end of the
+* ref_fist so that it can be printed first.
+*/
for_each_rawref(append_ref, cb);
+ if (detached)
+ head_ref(append_ref, cb);
--
To unsubscribe from this list: send the line unsubscribe 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 v3] worktree: add 'list' command
On Mon, 2015-08-10 at 16:53 -0400, Michael Rappazzo wrote:
+ while ((d = readdir(dir)) != NULL) {
I think it would be useful to break this loop out into a
for_each_worktree function.
While looking into per-worktree ref stuff, I have just noticed that git
prune will delete objects that are only referenced in a different
worktree's detached HEAD. To fix this, git prune will need to walk over
each worktree, looking at that worktree's HEAD (and other per-worktree
refs). It would be useful to be able to reuse some of this code for
that task.
--
To unsubscribe from this list: send the line unsubscribe 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 02/10] branch: refactor width computation
On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak karthik@gmail.com wrote:
Remove unnecessary variables from ref_list and ref_item which were
used for width computation. This is to make ref_item similar to
ref-filter's ref_array_item. This will ensure a smooth port of
branch.c to use ref-filter APIs in further patches.
Previously the maxwidth was computed when inserting the refs into the
ref_list. Now, we obtain the entire ref_list and then compute
maxwidth.
Signed-off-by: Karthik Nayak karthik@gmail.com
---
diff --git a/builtin/branch.c b/builtin/branch.c
index 4fc8beb..b058b74 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -557,16 +552,21 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
-static int calc_maxwidth(struct ref_list *refs)
+static int calc_maxwidth(struct ref_list *refs, int remote_bonus)
{
- int i, w = 0;
+ int i, max = 0;
for (i = 0; i refs-index; i++) {
+ struct ref_item *it = refs-list[i];
+ int w = utf8_strwidth(it-name);
+
if (refs-list[i].ignore)
continue;
Nit: Unnecessary work. You compute the width and then immediately
throw it away when 'ignore' is true.
Also, you use 'it' elsewhere rather than 'refs-list[i]' but not this
line, which makes it seem out of place. I would have expected to see:
if (it-ignore)
continue;
(Despite the noisier diff, the end result will be more consistent.)
- if (refs-list[i].width w)
- w = refs-list[i].width;
+ if (it-kind == REF_REMOTE_BRANCH)
+ w += remote_bonus;
+ if (w max)
+ max = w;
}
- return w;
+ return max;
}
On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak karthik@gmail.com wrote:
From: Karthik Nayak karthik@gmail.com
Remove unnecessary variables from ref_list and ref_item which were
used for width computation. This is to make ref_item similar to
ref-filter's ref_array_item. This will ensure a smooth port of
branch.c to use ref-filter APIs in further patches.
Previously the maxwidth was computed when inserting the refs into the
ref_list. Now, we obtain the entire ref_list and then compute
maxwidth.
Based-on-patch-by: Jeff King p...@peff.net
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/branch.c | 61
+---
1 file changed, 32 insertions(+), 29 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 4fc8beb..b058b74 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -282,14 +282,14 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
struct ref_item {
char *name;
char *dest;
- unsigned int kind, width;
+ unsigned int kind;
struct commit *commit;
int ignore;
};
struct ref_list {
struct rev_info revs;
- int index, alloc, maxwidth, verbose, abbrev;
+ int index, alloc, verbose, abbrev;
struct ref_item *list;
struct commit_list *with_commit;
int kinds;
@@ -386,15 +386,8 @@ static int append_ref(const char *refname, const struct
object_id *oid, int flag
newitem-name = xstrdup(refname);
newitem-kind = kind;
newitem-commit = commit;
- newitem-width = utf8_strwidth(refname);
newitem-dest = resolve_symref(orig_refname, prefix);
newitem-ignore = 0;
- /* adjust for remotes/ */
- if (newitem-kind == REF_REMOTE_BRANCH
- ref_list-kinds != REF_REMOTE_BRANCH)
- newitem-width += 8;
- if (newitem-width ref_list-maxwidth)
- ref_list-maxwidth = newitem-width;
return 0;
}
@@ -505,11 +498,12 @@ static void add_verbose_info(struct strbuf *out, struct
ref_item *item,
}
static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
- int abbrev, int current, char *prefix)
+ int abbrev, int current, const char *remote_prefix)
{
char c;
int color;
struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
+ const char *prefix = ;
if (item-ignore)
return;
@@ -520,6 +514,7 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
break;
case REF_REMOTE_BRANCH:
color = BRANCH_COLOR_REMOTE;
+ prefix = remote_prefix;
break;
default:
color = BRANCH_COLOR_PLAIN;
@@ -557,16 +552,21 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
strbuf_release(out);
}
-static int
Re: [PATCH 03/10] branch: bump get_head_description() to the top
On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak karthik@gmail.com wrote: This is a preperatory patch for 'roll show_detached HEAD into regular ref_list'. This patch moves get_head_descrition() to the top so that s/descrition/description/ it can be used in print_ref_item(). Signed-off-by: Karthik Nayak karthik@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report Windows 10
On Mon, 10 Aug 2015 14:26:44 +0200 MS-Informatique i...@ms-informatique.be wrote: My Windows notebook got updated to Windows 10 and now my Git Bash doesn't start and when I open an existing repository from Git Gui, I am getting next error: 0 [main] us 0 init_cheap: VirtualAlloc pointer is null, Win32 error 487 AllocationBase 0x0, BaseAddress 0x6857, RegionSize 0x41, State 0x1 C:\Program Files (x86)\Git\bin\sh.exe: *** Couldn't reserve space for cygwin's heap, Win32 error 0 I am running GIT version 1.9.5 (latest build for Windows). This version is essentially abandoned because the development happens on packaging the 2.x series for Windows, and using the updated runtime (MSYS2 instead of MSYS). So please try this: 1) Fetch the latest version of the new Git for Windows available -- currently its 2.4.6 RC5 [1]. Supposedly you should try 64-bit version. 2) If the bug still does manifest itself, prease report it to the project's tracker [2]. 1. https://github.com/git-for-windows/git/releases/tag/v2.4.6.windows.1 2. https://github.com/git-for-windows/git/issues/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature: git stash pop --always-drop
On Mon, Aug 10, 2015 at 10:42:30AM +, Ed Avis wrote: I would find it useful to ask 'git stash pop' to always drop the stash after applying it to the working tree, even if there were conflicts. (Only if there was some hard error, such as an I/O error updating some of the files, should the stash be left on the stack.) Hmm. That seems rather dangerous, but hey, if it's not the default, I guess it is your own foot that you are shooting. Would a patch for such an --always-drop flag be accepted? I doubt you will get a good answer to that question; the attitude here is usually well, we would have to see the patch. For instance, I don't know how easy it will be to tell merge conflicts apart from I/O errors. Figuring that out will probably be part of a rough draft patch. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature: git stash pop --always-drop
An alternative would be for git stash to always print the name of the stash it is applying. Then you can drop it afterwards by name and be sure you got the right one. Printing the name of the stash sounds like a reasonable bit of chatter to add anyway, do you agree? -- Ed Avis e...@waniasset.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report Windows 10
Hi, On 2015-08-10 14:26, MS-Informatique wrote: My Windows notebook got updated to Windows 10 and now my Git Bash doesn't start and when I open an existing repository from Git Gui, I am getting next error: 0 [main] us 0 init_cheap: VirtualAlloc pointer is null, Win32 error 487 AllocationBase 0x0, BaseAddress 0x6857, RegionSize 0x41, State 0x1 C:\Program Files (x86)\Git\bin\sh.exe: *** Couldn't reserve space for cygwin's heap, Win32 error 0 I am running GIT version 1.9.5 (latest build for Windows). Can someone help me? First of all, the home page of Git for Windows has hints where to report bugs (and where to look for possible resolutions first). Second, this issue is so common that I wrote a wiki page about it: https://github.com/git-for-windows/git/wiki/32-bit-issues Short version: reinstall Git for Windows. Preferably a 64-bit Git for Windows 2.x from https://git-for-windows.github.io/. And you might want to heed the advice given in http://git-for-windows.github.io/#contribute when you want to report bugs ;-) Ciao, Johannes -- To unsubscribe from this list: send the line 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/17] cache.h: complete set of git_path_submodule helpers
The git_path function has git_pathdup and
strbuf_git_path variants, but git_submodule_path only
comes in the dangerous, static-buffer variant. That makes
refactoring callers to use the safer functions hard (since
they don't exist).
Since we're already using a strbuf behind the scenes, it's
easy to expose all three of these interfaces with thin
wrappers.
Signed-off-by: Jeff King p...@peff.net
---
cache.h | 5 +
path.c | 35 ++-
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index 8db884e..6f74f33 100644
--- a/cache.h
+++ b/cache.h
@@ -724,10 +724,15 @@ extern char *mksnpath(char *buf, size_t n, const char
*fmt, ...)
__attribute__((format (printf, 3, 4)));
extern void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
+extern void strbuf_git_path_submodule(struct strbuf *sb, const char *path,
+ const char *fmt, ...)
+ __attribute__((format (printf, 3, 4)));
extern char *git_pathdup(const char *fmt, ...)
__attribute__((format (printf, 1, 2)));
extern char *mkpathdup(const char *fmt, ...)
__attribute__((format (printf, 1, 2)));
+extern char *git_pathdup_submodule(const char *path, const char *fmt, ...)
+ __attribute__((format (printf, 2, 3)));
extern void report_linked_checkout_garbage(void);
diff --git a/path.c b/path.c
index 10f4cbf..9aad9a1 100644
--- a/path.c
+++ b/path.c
@@ -224,11 +224,10 @@ const char *mkpath(const char *fmt, ...)
return cleanup_path(pathname-buf);
}
-const char *git_path_submodule(const char *path, const char *fmt, ...)
+static void do_submodule_path(struct strbuf *buf, const char *path,
+ const char *fmt, va_list args)
{
- struct strbuf *buf = get_pathname();
const char *git_dir;
- va_list args;
strbuf_addstr(buf, path);
if (buf-len buf-buf[buf-len - 1] != '/')
@@ -242,13 +241,39 @@ const char *git_path_submodule(const char *path, const
char *fmt, ...)
}
strbuf_addch(buf, '/');
- va_start(args, fmt);
strbuf_vaddf(buf, fmt, args);
- va_end(args);
strbuf_cleanup_path(buf);
+}
+
+const char *git_path_submodule(const char *path, const char *fmt, ...)
+{
+ va_list args;
+ struct strbuf *buf = get_pathname();
+ va_start(args, fmt);
+ do_submodule_path(buf, path, fmt, args);
+ va_end(args);
return buf-buf;
}
+char *git_pathdup_submodule(const char *path, const char *fmt, ...)
+{
+ va_list args;
+ struct strbuf buf = STRBUF_INIT;
+ va_start(args, fmt);
+ do_submodule_path(buf, path, fmt, args);
+ va_end(args);
+ return strbuf_detach(buf, NULL);
+}
+
+void strbuf_git_path_submodule(struct strbuf *buf, const char *path,
+ const char *fmt, ...)
+{
+ va_list args;
+ va_start(args, fmt);
+ do_submodule_path(buf, path, fmt, args);
+ va_end(args);
+}
+
int validate_headref(const char *path)
{
struct stat st;
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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 03/17] t5700: modernize style
The early part of this test is rather old, and does not
follow our usual style guidelines. In particular:
- the tests liberally chdir, and expect out-of-test cd
commands to return them to a sane state
- test commands aren't indented at all
- there are a lot of minor formatting nits, like the
opening quote of the test block on the wrong line,
spaces after , etc
This patch fixes the style issues, and uses a few helper
functions, along with subshells and git -C, to avoid
changing the cwd of the main script.
Signed-off-by: Jeff King p...@peff.net
---
t/t5700-clone-reference.sh | 193 +++--
1 file changed, 81 insertions(+), 112 deletions(-)
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 3e783fc..51d131a 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -10,49 +10,51 @@ base_dir=`pwd`
U=$base_dir/UPLOAD_LOG
-test_expect_success 'preparing first repository' \
-'test_create_repo A cd A
-echo first file1
-git add file1
-git commit -m initial'
-
-cd $base_dir
-
-test_expect_success 'preparing second repository' \
-'git clone A B cd B
-echo second file2
-git add file2
-git commit -m addition
-git repack -a -d
-git prune'
-
-cd $base_dir
-
-test_expect_success 'cloning with reference (-l -s)' \
-'git clone -l -s --reference B A C'
-
-cd $base_dir
-
-test_expect_success 'existence of info/alternates' \
-'test_line_count = 2 C/.git/objects/info/alternates'
-
-cd $base_dir
+# create a commit in repo $1 with name $2
+commit_in () {
+ (
+ cd $1
+ echo $2 $2
+ git add $2
+ git commit -m $2
+ )
+}
+
+# check that there are $2 loose objects in repo $1
+test_objcount () {
+ echo $2 expect
+ git -C $1 count-objects actual.raw
+ cut -d' ' -f1 actual.raw actual
+ test_cmp expect actual
+}
+
+test_expect_success 'preparing first repository' '
+ test_create_repo A
+ commit_in A file1
+'
-test_expect_success 'pulling from reference' \
-'cd C
-git pull ../B master'
+test_expect_success 'preparing second repository' '
+ git clone A B
+ commit_in B file2
+ git -C B repack -ad
+ git -C B prune
+'
-cd $base_dir
+test_expect_success 'cloning with reference (-l -s)' '
+ git clone -l -s --reference B A C
+'
-test_expect_success 'that reference gets used' \
-'cd C
-echo 0 objects, 0 kilobytes expected
-git count-objects current
-test_cmp expected current'
+test_expect_success 'existence of info/alternates' '
+ test_line_count = 2 C/.git/objects/info/alternates
+'
-cd $base_dir
+test_expect_success 'pulling from reference' '
+ git -C C pull ../B master
+'
-rm -f $U.D
+test_expect_success 'that reference gets used' '
+ test_objcount C 0
+'
test_expect_success 'cloning with reference (no -l -s)' '
GIT_TRACE_PACKET=$U.D git clone --reference B file://$(pwd)/A D
@@ -63,95 +65,64 @@ test_expect_success 'fetched no objects' '
! grep want $U.D
'
-cd $base_dir
-
-test_expect_success 'existence of info/alternates' \
-'test_line_count = 1 D/.git/objects/info/alternates'
-
-cd $base_dir
-
-test_expect_success 'pulling from reference' \
-'cd D git pull ../B master'
-
-cd $base_dir
-
-test_expect_success 'that reference gets used' \
-'cd D echo 0 objects, 0 kilobytes expected
-git count-objects current
-test_cmp expected current'
-
-cd $base_dir
+test_expect_success 'existence of info/alternates' '
+ test_line_count = 1 D/.git/objects/info/alternates
+'
-test_expect_success 'updating origin' \
-'cd A
-echo third file3
-git add file3
-git commit -m update
-git repack -a -d
-git prune'
+test_expect_success 'pulling from reference' '
+ git -C D pull ../B master
+'
-cd $base_dir
+test_expect_success 'that reference gets used' '
+ test_objcount D 0
+'
-test_expect_success 'pulling changes from origin' \
-'cd C
-git pull origin'
+test_expect_success 'updating origin' '
+ commit_in A file3
+ git -C A repack -ad
+ git -C A prune
+'
-cd $base_dir
+test_expect_success 'pulling changes from origin' '
+ git -C C pull origin
+'
# the 2 local objects are commit and tree from the merge
-test_expect_success 'that alternate to origin gets used' \
-'cd C
-echo 2 objects expected
-git count-objects | cut -d, -f1 current
-test_cmp expected current'
-
-cd $base_dir
-
-test_expect_success 'pulling changes from origin' \
-'cd D
-git pull origin'
+test_expect_success 'that alternate to origin gets used' '
+ test_objcount C 2
+'
-cd $base_dir
+test_expect_success 'pulling changes from origin' '
+ git -C D pull origin
+'
# the 5 local objects are expected; file3 blob, commit in A to add it
# and its tree, and 2 are our tree and the merge commit.
-test_expect_success 'check objects expected to exist locally' \
-'cd D
-echo 5 objects expected
-git
[PATCH 01/17] cache.h: clarify documentation for git_path, et al
The comment above these functions actually describes sha1_file_name, and comes from the very first revision of git. Commit 723c31f (Add git_path() and head_ref() helper functions., 2005-07-05) added git_path, pushing the comment away from the function it describes; later commits added more functions in this block. Let's fix the comment to describe these related functions in more detail. Let's also make sure to point out their safer alternatives (and move those alternatives below, which makes more sense when reading the file). Note that we do not need to move the existing comment to sha1_file_name. Commit d40d535 (sha1_file.c: document a bunch of functions defined in the file, 2014-02-21) already added a much more descriptive comment to it. Signed-off-by: Jeff King p...@peff.net --- cache.h | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/cache.h b/cache.h index 6bb7119..8db884e 100644 --- a/cache.h +++ b/cache.h @@ -708,6 +708,18 @@ extern int check_repository_format(void); #define DATA_CHANGED0x0020 #define TYPE_CHANGED0x0040 +/* + * Return a statically allocated filename, either generically (mkpath), in + * the repository directory (git_path), or in a submodule's repository + * directory (git_path_submodule). In all cases, note that the result + * may be overwritten by another call to _any_ of the functions. Consider + * using the safer dup or strbuf formats below. + */ +extern const char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2))); +extern const char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2))); +extern const char *git_path_submodule(const char *path, const char *fmt, ...) + __attribute__((format (printf, 2, 3))); + extern char *mksnpath(char *buf, size_t n, const char *fmt, ...) __attribute__((format (printf, 3, 4))); extern void strbuf_git_path(struct strbuf *sb, const char *fmt, ...) @@ -717,11 +729,6 @@ extern char *git_pathdup(const char *fmt, ...) extern char *mkpathdup(const char *fmt, ...) __attribute__((format (printf, 1, 2))); -/* Return a statically allocated filename matching the sha1 signature */ -extern const char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2))); -extern const char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2))); -extern const char *git_path_submodule(const char *path, const char *fmt, ...) - __attribute__((format (printf, 2, 3))); extern void report_linked_checkout_garbage(void); /* -- 2.5.0.414.g670f2a4 -- To unsubscribe from this list: send the line 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/17] prefer mkpathdup to mkpath in assignments
As with the previous commit to git_path, assigning the
result of mkpath is suspicious, since it is not clear
whether we will still depend on the value after it may have
been overwritten by subsequent calls. This patch converts
low-hanging fruit to use mkpathdup instead of mkpath (with
the downside that we must remember to free the result).
Signed-off-by: Jeff King p...@peff.net
---
builtin/repack.c | 24 +---
refs.c | 6 --
2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index af7340c..70b9b1e 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -285,8 +285,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
failed = 0;
for_each_string_list_item(item, names) {
for (ext = 0; ext ARRAY_SIZE(exts); ext++) {
- const char *fname_old;
- char *fname;
+ char *fname, *fname_old;
fname = mkpathdup(%s/pack-%s%s, packdir,
item-string, exts[ext].name);
if (!file_exists(fname)) {
@@ -294,7 +293,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
continue;
}
- fname_old = mkpath(%s/old-%s%s, packdir,
+ fname_old = mkpathdup(%s/old-%s%s, packdir,
item-string, exts[ext].name);
if (file_exists(fname_old))
if (unlink(fname_old))
@@ -302,10 +301,12 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
if (!failed rename(fname, fname_old)) {
free(fname);
+ free(fname_old);
failed = 1;
break;
} else {
string_list_append(rollback, fname);
+ free(fname_old);
}
}
if (failed)
@@ -314,13 +315,13 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
if (failed) {
struct string_list rollback_failure = STRING_LIST_INIT_DUP;
for_each_string_list_item(item, rollback) {
- const char *fname_old;
- char *fname;
+ char *fname, *fname_old;
fname = mkpathdup(%s/%s, packdir, item-string);
- fname_old = mkpath(%s/old-%s, packdir, item-string);
+ fname_old = mkpathdup(%s/old-%s, packdir,
item-string);
if (rename(fname_old, fname))
string_list_append(rollback_failure, fname);
free(fname);
+ free(fname_old);
}
if (rollback_failure.nr) {
@@ -368,13 +369,14 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
/* Remove the old- files */
for_each_string_list_item(item, names) {
for (ext = 0; ext ARRAY_SIZE(exts); ext++) {
- const char *fname;
- fname = mkpath(%s/old-%s%s,
- packdir,
- item-string,
- exts[ext].name);
+ char *fname;
+ fname = mkpathdup(%s/old-%s%s,
+ packdir,
+ item-string,
+ exts[ext].name);
if (remove_path(fname))
warning(_(removing '%s' failed), fname);
+ free(fname);
}
}
diff --git a/refs.c b/refs.c
index 93b250e..e70941a 100644
--- a/refs.c
+++ b/refs.c
@@ -3380,7 +3380,7 @@ static int commit_ref_update(struct ref_lock *lock,
int create_symref(const char *ref_target, const char *refs_heads_master,
const char *logmsg)
{
- const char *lockpath;
+ char *lockpath = NULL;
char ref[1000];
int fd, len, written;
char *git_HEAD = git_pathdup(%s, ref_target);
@@ -3407,7 +3407,7 @@ int create_symref(const char *ref_target, const char
*refs_heads_master,
error(refname too long: %s, refs_heads_master);
goto error_free_return;
}
- lockpath = mkpath(%s.lock, git_HEAD);
+ lockpath = mkpathdup(%s.lock, git_HEAD);
fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);
if (fd 0) {
error(Unable to open %s for writing, lockpath);
@@ -3427,9 +3427,11 @@ int create_symref(const char *ref_target, const char
[PATCH 06/17] prefer git_pathdup to git_path in some possibly-dangerous cases
Because git_path uses a static buffer that is shared with
calls to git_path, mkpath, etc, it can be dangerous to
assign the result to a variable or pass it to a non-trivial
function. The value may change unexpectedly due to other
calls.
None of the cases changed here has a known bug, but they're
worth converting away from git_path because:
1. It's easy to use git_pathdup in these cases.
2. They use constructs (like assignment) that make it
hard to tell whether they're safe or not.
The extra malloc overhead should be trivial, as an
allocation should be an order of magnitude cheaper than a
system call (which we are clearly about to make, since we
are constructing a filename). The real cost is that we must
remember to free the result.
Signed-off-by: Jeff King p...@peff.net
---
builtin/fsck.c | 4 +++-
fast-import.c | 4 +++-
http-backend.c | 3 ++-
notes-merge.c | 3 ++-
refs.c | 14 --
unpack-trees.c | 4 +++-
6 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index f4b87e9..0794703 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -243,13 +243,14 @@ static void check_unreachable_object(struct object *obj)
printf(dangling %s %s\n, typename(obj-type),
sha1_to_hex(obj-sha1));
if (write_lost_and_found) {
- const char *filename = git_path(lost-found/%s/%s,
+ char *filename = git_pathdup(lost-found/%s/%s,
obj-type == OBJ_COMMIT ? commit : other,
sha1_to_hex(obj-sha1));
FILE *f;
if (safe_create_leading_directories_const(filename)) {
error(Could not create lost-found);
+ free(filename);
return;
}
if (!(f = fopen(filename, w)))
@@ -262,6 +263,7 @@ static void check_unreachable_object(struct object *obj)
if (fclose(f))
die_errno(Could not finish '%s',
filename);
+ free(filename);
}
return;
}
diff --git a/fast-import.c b/fast-import.c
index 2ad4fee..ad8848b 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -407,7 +407,7 @@ static void dump_marks_helper(FILE *, uintmax_t, struct
mark_set *);
static void write_crash_report(const char *err)
{
- const char *loc = git_path(fast_import_crash_%PRIuMAX, (uintmax_t)
getpid());
+ char *loc = git_pathdup(fast_import_crash_%PRIuMAX, (uintmax_t)
getpid());
FILE *rpt = fopen(loc, w);
struct branch *b;
unsigned long lu;
@@ -415,6 +415,7 @@ static void write_crash_report(const char *err)
if (!rpt) {
error(can't write crash report %s: %s, loc, strerror(errno));
+ free(loc);
return;
}
@@ -488,6 +489,7 @@ static void write_crash_report(const char *err)
fputs(---\n, rpt);
fputs(END OF CRASH REPORT\n, rpt);
fclose(rpt);
+ free(loc);
}
static void end_packfile(void);
diff --git a/http-backend.c b/http-backend.c
index b977c00..bac40ef 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -164,7 +164,7 @@ static void send_strbuf(const char *type, struct strbuf
*buf)
static void send_local_file(const char *the_type, const char *name)
{
- const char *p = git_path(%s, name);
+ char *p = git_pathdup(%s, name);
size_t buf_alloc = 8192;
char *buf = xmalloc(buf_alloc);
int fd;
@@ -191,6 +191,7 @@ static void send_local_file(const char *the_type, const
char *name)
}
close(fd);
free(buf);
+ free(p);
}
static void get_text_file(char *name)
diff --git a/notes-merge.c b/notes-merge.c
index 0b2b82c..b3d1dab 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -295,7 +295,7 @@ static void write_buf_to_worktree(const unsigned char *obj,
const char *buf, unsigned long size)
{
int fd;
- const char *path = git_path(NOTES_MERGE_WORKTREE /%s,
sha1_to_hex(obj));
+ char *path = git_pathdup(NOTES_MERGE_WORKTREE /%s, sha1_to_hex(obj));
if (safe_create_leading_directories_const(path))
die_errno(unable to create directory for '%s', path);
if (file_exists(path))
@@ -320,6 +320,7 @@ static void write_buf_to_worktree(const unsigned char *obj,
}
close(fd);
+ free(path);
}
static void write_note_to_worktree(const unsigned char *obj,
diff --git a/refs.c b/refs.c
index 2db2975..93b250e 100644
--- a/refs.c
+++ b/refs.c
@@ -1288,12 +1288,12 @@ static void read_packed_refs(FILE *f, struct ref_dir
*dir)
*/
static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache
[PATCH 08/17] remote.c: drop extraneous local variable from migrate_file
It's an anti-pattern to assign the result of git_path to a
variable, since other calls may reuse our buffer. In this
case, we feed the result to unlink_or_warn immediately
afterwards, so it's OK. However, it's nice to avoid
assignment entirely, which makes it more obvious that
there's no bug.
We can just pass the result directly to unlink_or_warn,
which is a known-simple function. As a bonus, the code flow
is a little more obvious, as we eliminate an extra
conditional (a reader does not have to wonder any more
under which circumstances is 'path' set?).
Signed-off-by: Jeff King p...@peff.net
---
builtin/remote.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index cc3c741..181668d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -581,7 +581,6 @@ static int migrate_file(struct remote *remote)
{
struct strbuf buf = STRBUF_INIT;
int i;
- const char *path = NULL;
strbuf_addf(buf, remote.%s.url, remote-name);
for (i = 0; i remote-url_nr; i++)
@@ -601,11 +600,9 @@ static int migrate_file(struct remote *remote)
return error(_(Could not append '%s' to '%s'),
remote-fetch_refspec[i], buf.buf);
if (remote-origin == REMOTE_REMOTES)
- path = git_path(remotes/%s, remote-name);
+ unlink_or_warn(git_path(remotes/%s, remote-name));
else if (remote-origin == REMOTE_BRANCHES)
- path = git_path(branches/%s, remote-name);
- if (path)
- unlink_or_warn(path);
+ unlink_or_warn(git_path(branches/%s, remote-name));
return 0;
}
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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 09/17] refs.c: remove extra git_path calls from read_loose_refs
In iterating over the loose refs in refs/foo/, we keep a
running strbuf with refs/foo/one, refs/foo/two, etc. But
we also need to access these files in the filesystem, as
.git/refs/foo/one, etc. For this latter purpose, we make a
series of independent calls to git_path(). These are safe
(we only use the result to call stat()), but assigning the
result of git_path is a suspicious pattern that we'd rather
avoid.
This patch keeps a running buffer with .git/refs/foo/, and
we can just append/reset each directory element as we loop.
This matches how we handle the refnames. It should also be
more efficient, as we do not keep formatting the same
.git/refs/foo prefix (which can be arbitrarily deep).
Technically we are dropping a call to strbuf_cleanup() on
each generated filename, but that's OK; it wasn't doing
anything, as we are putting in single-level names we read
from the filesystem (so it could not possibly be cleaning up
cruft like ./ in this instance).
A clever reader may also note that the running refname
buffer (refs/foo/) is actually a subset of the filesystem
path buffer (.git/refs/foo/). We could get by with one
buffer, indexing the length of $GIT_DIR when we want the
refname. However, having tried this, the resulting code
actually ends up a little more confusing, and the efficiency
improvement is tiny (and almost certainly dwarfed by the
system calls we are making).
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index e70941a..06f95c4 100644
--- a/refs.c
+++ b/refs.c
@@ -1352,19 +1352,23 @@ static void read_loose_refs(const char *dirname, struct
ref_dir *dir)
{
struct ref_cache *refs = dir-ref_cache;
DIR *d;
- const char *path;
struct dirent *de;
int dirnamelen = strlen(dirname);
struct strbuf refname;
+ struct strbuf path = STRBUF_INIT;
+ size_t path_baselen;
if (*refs-name)
- path = git_path_submodule(refs-name, %s, dirname);
+ strbuf_git_path_submodule(path, refs-name, %s, dirname);
else
- path = git_path(%s, dirname);
+ strbuf_git_path(path, %s, dirname);
+ path_baselen = path.len;
- d = opendir(path);
- if (!d)
+ d = opendir(path.buf);
+ if (!d) {
+ strbuf_release(path);
return;
+ }
strbuf_init(refname, dirnamelen + 257);
strbuf_add(refname, dirname, dirnamelen);
@@ -1373,17 +1377,14 @@ static void read_loose_refs(const char *dirname, struct
ref_dir *dir)
unsigned char sha1[20];
struct stat st;
int flag;
- const char *refdir;
if (de-d_name[0] == '.')
continue;
if (ends_with(de-d_name, .lock))
continue;
strbuf_addstr(refname, de-d_name);
- refdir = *refs-name
- ? git_path_submodule(refs-name, %s, refname.buf)
- : git_path(%s, refname.buf);
- if (stat(refdir, st) 0) {
+ strbuf_addstr(path, de-d_name);
+ if (stat(path.buf, st) 0) {
; /* silently ignore */
} else if (S_ISDIR(st.st_mode)) {
strbuf_addch(refname, '/');
@@ -1430,8 +1431,10 @@ static void read_loose_refs(const char *dirname, struct
ref_dir *dir)
create_ref_entry(refname.buf, sha1,
flag, 0));
}
strbuf_setlen(refname, dirnamelen);
+ strbuf_setlen(path, path_baselen);
}
strbuf_release(refname);
+ strbuf_release(path);
closedir(d);
}
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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 05/17] remove hold_lock_file_for_append
From: Jim Hill gjth...@gmail.com
No users of hold_lock_file_for_append remain, so remove it.
hold_lock_file_for_append copies its target file internally.
This makes it too heavyweight for true append-only logging
and too limited for anything else (which probably wants to
process the contents). It shouldn't be used.
[jk: tweaked commit message, and dropped declaration and
documentation, too]
Signed-off-by: Jim Hill gjth...@gmail.com
Signed-off-by: Jeff King p...@peff.net
---
And this is the obvious continuation of the last patch.
Documentation/technical/api-lockfile.txt | 26 --
lockfile.c | 38
lockfile.h | 7 ++
3 files changed, 11 insertions(+), 60 deletions(-)
diff --git a/Documentation/technical/api-lockfile.txt
b/Documentation/technical/api-lockfile.txt
index 93b5f23..0f5c481 100644
--- a/Documentation/technical/api-lockfile.txt
+++ b/Documentation/technical/api-lockfile.txt
@@ -40,7 +40,7 @@ The caller:
* Attempts to create a lockfile by passing that variable and the path
of the final destination (e.g. `$GIT_DIR/index`) to
- `hold_lock_file_for_update` or `hold_lock_file_for_append`.
+ `hold_lock_file_for_update`.
* Writes new content for the destination file by either:
@@ -64,8 +64,7 @@ When finished writing, the caller can:
Even after the lockfile is committed or rolled back, the `lock_file`
object must not be freed or altered by the caller. However, it may be
-reused; just pass it to another call of `hold_lock_file_for_update` or
-`hold_lock_file_for_append`.
+reused; just pass it to another call of `hold_lock_file_for_update`.
If the program exits before you have called one of `commit_lock_file`,
`commit_lock_file_to`, `rollback_lock_file`, or `close_lock_file`, an
@@ -111,8 +110,7 @@ appropriately, do their best to roll back the lockfile, and
return -1.
Flags
-
-The following flags can be passed to `hold_lock_file_for_update` or
-`hold_lock_file_for_append`:
+The following flags can be passed to `hold_lock_file_for_update`:
LOCK_NO_DEREF::
@@ -141,12 +139,6 @@ hold_lock_file_for_update::
above). Attempt to create a lockfile for the destination and
return the file descriptor for writing to the file.
-hold_lock_file_for_append::
-
- Like `hold_lock_file_for_update`, but before returning copy
- the existing contents of the file (if any) to the lockfile and
- position its write pointer at the end of the file.
-
fdopen_lock_file::
Associate a stdio stream with the lockfile. Return NULL
@@ -162,8 +154,8 @@ get_locked_file_path::
commit_lock_file::
Take a pointer to the `struct lock_file` initialized with an
- earlier call to `hold_lock_file_for_update` or
- `hold_lock_file_for_append`, close the file descriptor, and
+ earlier call to `hold_lock_file_for_update`,
+ close the file descriptor, and
rename the lockfile to its final destination. Return 0 upon
success. On failure, roll back the lock file and return -1,
with `errno` set to the value from the failing call to
@@ -180,8 +172,8 @@ commit_lock_file_to::
rollback_lock_file::
Take a pointer to the `struct lock_file` initialized with an
- earlier call to `hold_lock_file_for_update` or
- `hold_lock_file_for_append`, close the file descriptor and
+ earlier call to `hold_lock_file_for_update`,
+ close the file descriptor and
remove the lockfile. It is a NOOP to call
`rollback_lock_file()` for a `lock_file` object that has
already been committed or rolled back.
@@ -189,8 +181,8 @@ rollback_lock_file::
close_lock_file::
Take a pointer to the `struct lock_file` initialized with an
- earlier call to `hold_lock_file_for_update` or
- `hold_lock_file_for_append`. Close the file descriptor (and
+ earlier call to `hold_lock_file_for_update`.
+ Close the file descriptor (and
the file pointer if it has been opened using
`fdopen_lock_file`). Return 0 upon success. On failure to
`close(2)`, return a negative value and roll back the lock
diff --git a/lockfile.c b/lockfile.c
index 993bb82..b1ceec6 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -249,44 +249,6 @@ int hold_lock_file_for_update_timeout(struct lock_file
*lk, const char *path,
return fd;
}
-int hold_lock_file_for_append(struct lock_file *lk, const char *path, int
flags)
-{
- int fd, orig_fd;
-
- fd = lock_file(lk, path, flags);
- if (fd 0) {
- if (flags LOCK_DIE_ON_ERROR)
- unable_to_lock_die(path, errno);
- return fd;
- }
-
- orig_fd = open(path, O_RDONLY);
- if (orig_fd 0) {
- if (errno != ENOENT) {
- int save_errno = errno;
-
- if (flags LOCK_DIE_ON_ERROR)
-
[PATCHv3 2/2] completion: list variable names reliably with 'git config --names-only'
Recenty I created a multi-line branch description with '.' and '='
characters on one of the lines, and noticed that fragments of that line
show up when completing set variable names for 'git config', e.g.:
$ git config --get branch.b.description
Branch description to fool the completion script with a
second line containing dot . and equals = characters.
$ git config --unset TAB
...
second line containing dot . and equals
...
The completion script runs 'git config --list' and processes its output
to strip the values and keep only the variable names. It does so by
looking for lines containing '.' and '=' and outputting everything
before the '=', which was fooled by my multi-line branch description.
A similar issue exists with aliases and pretty format aliases with
multi-line values, but in that case 'git config --get-regexp' is run and
lines in its output are simply stripped after the first space, so
subsequent lines don't even have to contain '.' and '=' to fool the
completion script.
Use the new '--names-only' option added in the previous commit to list
config variable names reliably in both cases, without error-prone post
processing.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-completion.bash | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6eaab141e2..7200828fc4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -744,9 +744,8 @@ __git_compute_porcelain_commands ()
__git_get_config_variables ()
{
local section=$1 i IFS=$'\n'
- for i in $(git --git-dir=$(__gitdir) config --get-regexp
^$section\..* 2/dev/null); do
- i=${i#$section.}
- echo ${i/ */}
+ for i in $(git --git-dir=$(__gitdir) config --names-only --get-regexp
^$section\..* 2/dev/null); do
+ echo ${i#$section.}
done
}
@@ -1774,15 +1773,7 @@ __git_config_get_set_variables ()
c=$((--c))
done
- git --git-dir=$(__gitdir) config $config_file --list 2/dev/null |
- while read -r line
- do
- case $line in
- *.*=*)
- echo ${line/=*/}
- ;;
- esac
- done
+ git --git-dir=$(__gitdir) config $config_file --names-only --list
2/dev/null
}
_git_config ()
--
2.5.0.245.gff6622b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/16] Introduce a tempfile module
This is a re-roll of the tempfile patch series [1]. I'm sorry for the long delay getting v2 out. Thanks to Junio and Johannes Sixt for their feedback about v1. I think I have addressed all of their points. This version is very similar to v1 in spirit, though quite a few details have changed. The main difference is that I add some more wrapper functions for both lockfile and tempfile (a) to add some abstraction and (b) so that users of the former don't need to know that it is based on the latter: * Add new lockfile wrappers around the corresponding tempfile functions: * lockfile: * fdopen_lock_file() * close_lock_file() * reopen_lock_file() * Add accessors: * lockfile: * get_lock_file_path() * get_lock_file_fd() * get_lock_file_fp() * tempfile: * is_tempfile_active() * get_tempfile_path() * get_tempfile_fd() * get_tempfile_fp() Other changes in this version: * Make some trivial wrapper functions inline. * Change create_bundle() to dup() the file descriptor that it passes to write_pack_data() so that it doesn't have to tinker with lock-tempfile.fd to prevent the file from being closed twice. * Move some docs about the implementation from tempfile.h to tempfile.c. * Rename register_tempfile_object() to prepare_tempfile_object() to reduce confusion with register_tempfile(). Remove its path parameter and add a docstring. * Simplify some `die(BUG:...)` error messages. This series applies to the same commit as v1, namely v2.4.3-368-g7974889. There is one small conflict when merging to master or next or (pu minus gitster/mh/tempfile). This patch series is also available from my GitHub fork [2] as branch tempfile. [1] http://thread.gmane.org/gmane.comp.version-control.git/270998 [2] https://github.com/mhagger/git Michael Haggerty (16): Move lockfile documentation to lockfile.h and lockfile.c create_bundle(): duplicate file descriptor to avoid closing it twice lockfile: add accessors get_lock_file_fd() and get_lock_file_fp() lockfile: add accessor get_lock_file_path() commit_lock_file(): use get_locked_file_path() tempfile: a new module for handling temporary files prepare_tempfile_object(): new function, extracted from create_tempfile() tempfile: add several functions for creating temporary files register_tempfile(): new function to handle an existing temporary file write_shared_index(): use tempfile module setup_temporary_shallow(): use tempfile module diff: use tempfile module lock_repo_for_gc(): compute the path to gc.pid only once gc: use tempfile module to handle gc.pid file credential-cache--daemon: delete socket from main() credential-cache--daemon: use tempfile module Documentation/technical/api-lockfile.txt | 220 Makefile | 1 + builtin/commit.c | 15 +- builtin/gc.c | 32 +-- bundle.c | 26 ++- config.c | 14 +- credential-cache--daemon.c | 25 +-- credential-store.c | 2 +- diff.c | 29 +-- lockfile.c | 205 +++ lockfile.h | 336 +-- read-cache.c | 40 +--- refs.c | 18 +- shallow.c| 41 +--- tempfile.c | 305 tempfile.h | 271 + 16 files changed, 951 insertions(+), 629 deletions(-) delete mode 100644 Documentation/technical/api-lockfile.txt create mode 100644 tempfile.c create mode 100644 tempfile.h -- 2.5.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
[PATCHv3 1/2] config: add '--names-only' option to list only variable names
'git config' can only show values or name-value pairs, so if a shell
script needs the names of set config variables it has to run 'git config
--list' or '--get-regexp' and parse the output to separate config
variable names from their values. However, such a parsing can't cope
with multi-line values. Though 'git config' can produce null-terminated
output for newline-safe parsing, that's of no use in such a case, becase
shells can't cope with null characters.
Even our own bash completion script suffers from these issues.
Help the completion script, and shell scripts in general, by introducing
the '--names-only' option to modify the output of '--list' and
'--get-regexp' to list only the names of config variables, so they don't
have to perform error-prone post processing to separate variable names
from their values anymore.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
Documentation/git-config.txt | 10 +++---
builtin/config.c | 14 --
contrib/completion/git-completion.bash | 1 +
t/t1300-repo-config.sh | 22 ++
4 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096faa..ba76097c06 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -14,13 +14,13 @@ SYNOPSIS
'git config' [file-option] [type] --replace-all name value [value_regex]
'git config' [file-option] [type] [-z|--null] --get name [value_regex]
'git config' [file-option] [type] [-z|--null] --get-all name [value_regex]
-'git config' [file-option] [type] [-z|--null] --get-regexp name_regex
[value_regex]
+'git config' [file-option] [type] [-z|--null] [--names-only] --get-regexp
name_regex [value_regex]
'git config' [file-option] [type] [-z|--null] --get-urlmatch name URL
'git config' [file-option] --unset name [value_regex]
'git config' [file-option] --unset-all name [value_regex]
'git config' [file-option] --rename-section old_name new_name
'git config' [file-option] --remove-section name
-'git config' [file-option] [-z|--null] -l | --list
+'git config' [file-option] [-z|--null] [--names-only] -l | --list
'git config' [file-option] --get-color name [default]
'git config' [file-option] --get-colorbool name [stdout-is-tty]
'git config' [file-option] -e | --edit
@@ -159,7 +159,7 @@ See also FILES.
-l::
--list::
- List all variables set in config file.
+ List all variables set in config file, along with their values.
--bool::
'git config' will ensure that the output is true or false
@@ -190,6 +190,10 @@ See also FILES.
output without getting confused e.g. by values that
contain line breaks.
+--names-only::
+ Output only the names of config variables for `--list` or
+ `--get-regexp`.
+
--get-colorbool name [stdout-is-tty]::
Find the color setting for `name` (e.g. `color.diff`) and output
diff --git a/builtin/config.c b/builtin/config.c
index 7188405f7e..307980ab50 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int omit_values;
static int use_key_regexp;
static int do_all;
static int do_not_match;
@@ -78,6 +79,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, path, types, N_(value is a path (file or directory
name)), TYPE_PATH),
OPT_GROUP(N_(Other)),
OPT_BOOL('z', null, end_null, N_(terminate values with NUL byte)),
+ OPT_BOOL(0, names-only, omit_values, N_(names only)),
OPT_BOOL(0, includes, respect_includes, N_(respect include
directives on lookup)),
OPT_END(),
};
@@ -91,7 +93,7 @@ static void check_argc(int argc, int min, int max) {
static int show_all_config(const char *key_, const char *value_, void *cb)
{
- if (value_)
+ if (!omit_values value_)
printf(%s%c%s%c, key_, delim, value_, term);
else
printf(%s%c, key_, term);
@@ -117,6 +119,10 @@ static int format_config(struct strbuf *buf, const char
*key_, const char *value
strbuf_addstr(buf, key_);
must_print_delim = 1;
}
+ if (omit_values) {
+ strbuf_addch(buf, term);
+ return 0;
+ }
if (types == TYPE_INT)
sprintf(value, %PRId64,
git_config_int64(key_, value_ ? value_ : ));
@@ -549,7 +555,11 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
default:
usage_with_options(builtin_config_usage,
builtin_config_options);
}
-
+ if (omit_values
+ !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
+ error(--names-only is only applicable to --list or
--get-regexp);
+ usage_with_options(builtin_config_usage,
builtin_config_options);
+
[PATCHv3 0/2] 'git config --names-only' to help the completion script
This is a reroll of 'sg/config-name-only'. * Instead of the two new listing options of the previous round add one new option '--names-only' to modify the output of '--list' and '--get-regexp' options, as suggested in previous discussions. * Reorganized the commit messages: don't go into details of the completion bug in the first patch modifying builtin/config.c, talk about that in the second patch. SZEDER Gábor (2): config: add '--names-only' option to list only variable names completion: list variable names reliably with 'git config --names-only' Documentation/git-config.txt | 10 +++--- builtin/config.c | 14 -- contrib/completion/git-completion.bash | 16 t/t1300-repo-config.sh | 22 ++ 4 files changed, 45 insertions(+), 17 deletions(-) -- 2.5.0.245.gff6622b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/16] create_bundle(): duplicate file descriptor to avoid closing it twice
write_pack_data() passes bundle_fd to start_command() to be used as
the stdout of pack-objects. But start_command() closes its stdout if
it is 1. This is a problem if bundle_fd is the fd of a lock_file,
because commit_lock_file() will also try to close the fd.
So the old code suppressed commit_lock_file()'s usual behavior of
closing the file descriptor by setting the lock_file object's fd field
to -1.
But this is not really kosher. Code here shouldn't be mutating fields
within the lock_file object.
Instead, duplicate the file descriptor before passing it to
write_pack_data(). Then that function can close its copy without
closing the copy held in the lock_file object.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
This is new since v1. I like that it is better decoupled than the old
code, but let me know if you think otherwise.
Actually, it seems to me that start_command()'s special case of not
closing fd==0 is weird. I suppose that is because fd==0 is used to
mean no redirections whereas 0 also happens to be the fd for stdin.
But I don't want to dig into that now.
bundle.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/bundle.c b/bundle.c
index f732c92..b9dacc0 100644
--- a/bundle.c
+++ b/bundle.c
@@ -235,7 +235,9 @@ out:
return result;
}
-static int write_pack_data(int bundle_fd, struct lock_file *lock, struct
rev_info *revs)
+
+/* Write the pack data to bundle_fd, then close it if it is 1. */
+static int write_pack_data(int bundle_fd, struct rev_info *revs)
{
struct child_process pack_objects = CHILD_PROCESS_INIT;
int i;
@@ -250,13 +252,6 @@ static int write_pack_data(int bundle_fd, struct lock_file
*lock, struct rev_inf
if (start_command(pack_objects))
return error(_(Could not spawn pack-objects));
- /*
-* start_command closed bundle_fd if it was 1
-* so set the lock fd to -1 so commit_lock_file()
-* won't fail trying to close it.
-*/
- lock-fd = -1;
-
for (i = 0; i revs-pending.nr; i++) {
struct object *object = revs-pending.objects[i].item;
if (object-flags UNINTERESTING)
@@ -416,10 +411,21 @@ int create_bundle(struct bundle_header *header, const
char *path,
bundle_to_stdout = !strcmp(path, -);
if (bundle_to_stdout)
bundle_fd = 1;
- else
+ else {
bundle_fd = hold_lock_file_for_update(lock, path,
LOCK_DIE_ON_ERROR);
+ /*
+* write_pack_data() will close the fd passed to it,
+* but commit_lock_file() will also try to close the
+* lockfile's fd. So make a copy of the file
+* descriptor to avoid trying to close it twice.
+*/
+ bundle_fd = dup(bundle_fd);
+ if (bundle_fd 0)
+ die_errno(unable to dup file descriptor);
+ }
+
/* write signature */
write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
@@ -445,7 +451,7 @@ int create_bundle(struct bundle_header *header, const char
*path,
return -1;
/* write pack */
- if (write_pack_data(bundle_fd, lock, revs))
+ if (write_pack_data(bundle_fd, revs))
return -1;
if (!bundle_to_stdout) {
--
2.5.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
[PATCH v2 15/16] credential-cache--daemon: delete socket from main()
main() is responsible for cleaning up the socket in the case of errors, so it is reasonable to also make it responsible for cleaning it up when there are no errors. This change also makes the next step easier. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- credential-cache--daemon.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c index c2f0049..a671b2b 100644 --- a/credential-cache--daemon.c +++ b/credential-cache--daemon.c @@ -221,7 +221,6 @@ static void serve_cache(const char *socket_path, int debug) ; /* nothing */ close(fd); - unlink(socket_path); } static const char permissions_advice[] = @@ -280,5 +279,7 @@ int main(int argc, const char **argv) serve_cache(socket_path, debug); + unlink(socket_path); + return 0; } -- 2.5.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: git blame breaking on repository with CRLF files
Hello Torsten, Torsten Bögershausen, Sonntag, 9. August 2015 22:20: On 2015-08-08 07.58, Torsten Bögershausen wrote: On 2015-08-07 18.32, Benkstein, Frank wrote: I am working working on Linux and am examining code in a git repository I do not know much about. I am only looking at files, not changing anything. On some files in the repository I get (Not Committed Yet for all lines when running git blame. I checked with git status, git reset, git clean that the files are indeed in the repository and unmodified. I noticed that this only happens with git v2.5.0. With git v2.4.0 it looks correct, i.e. the output has proper commit ids, Author names and dates.. With git bisect I tracked this down to the following commit: commit 4bf256d67a85bed1e175ecc2706322eafe4489ca (HEAD, refs/bisect/bad) Author: Torsten Bögershausen tbo...@web.de Date: Sun May 3 18:38:01 2015 +0200 blame: CRLF in the working tree and LF in the repo Digging further, it seems that most files in the repository are checked in with CRLF line endings. In my working tree these are checked out as LF Do I understand it right that you have files in the repo with CRLF ? And these files are checked out with LF in the working tree ? Are the files marked with .gitattributes ? Or does the file have mixed line endings ? (Unless I missed something: Git never strips CRLF into LF at checkout, so I wonder how you ended up in this situation) You were right. They are CRLF in my working tree. My editor tricked me. Is there a way to reproduce it? Actually I could reproduce the following: CRLF in repo, CRLF in working tree, core.autocrlf= true. This is an old limitation (or call it bug), which has been there for a long time, (I tested with Git v1.7.0 from 2010). Thanks for the report, we will see if anybody is able to fix it. I can probably contribute some test cases. You are correct that it is also wrong in git v1.7.0. However, it is correct in v2.4.0. Another bisect gave me this commit which was included in v2.0.1: commit 4d4813a52f3722854a54bab046f4abfec13ef6ae Author: brian m. carlson sand...@crustytoothpaste.net Date: Sat Apr 26 23:10:40 2014 + blame: correctly handle files regardless of autocrlf So this still looks like a regression v2.5.0 to me. Regards, Frank.
[PATCH 0/17] removing questionable uses of git_path
Recently Michael and I were working on a patch series (not yet
published), which did something like:
const char *path = git_path(foo);
... do stuff with path ...
for_each_ref(some_callback, NULL);
... do some other stuff ...
unlink(path);
Clever readers may have spotted the bug immediately, but we did not,
until we found that random loose refs were being deleted from the
repository.
The problem is that git_path uses a static buffer that gets overwritten
by subsequent calls. The ref code uses it to iterate over all of the
loose refs in a directory, so our original path is trashed before
for_each_ref returns. Except to make it even more exciting, git_path
actually has a ring of _four_ buffers, so any trivial test you write
will probably work just fine; it's only when you use a real repository
that it causes problems (and then, only if the code path is such that
the loose refs were not previously accessed and cached!).
Michael likened git_path to a hand-grenade with the pin pulled out,
and I tend to agree. On the other hand, it's pretty darn useful to be
able to get a quick path without having to deal with memory allocation
and ownership. This patch series tries to document the danger, and
remove some of the more questionable uses. I don't know whether this is
fixing any actual latent bugs; I traced a number of the code paths
manually, but never found a bug. There were some near misses, though,
which make me believe that seemingly-unrelated refactoring could
introduce a bug.
I stopped short of trying to eradicate git_path entirely, and settled
for:
git grep -E '[^_](git_|mk)path\('
producing a fairly tame-looking set of function calls. It's OK to pass
the result of git_path() to a system call, or something that is a thin
wrapper around one (e.g., strbuf_read_file).
I think this takes us most of the way there. I left out a few cases
where introducing allocations would have been awkward, and I verified
that there were no bugs (e.g., rerere_path). And I left out a few spots
that conflict with topics in next (and luckily, in all cases what is
in next makes the problem go away, so we do not have to follow-up for
those sites).
Along the way, there are a few cleanups (e.g., I polished off the recent
hold_lock_file_for_append topic which was on the list, as it had some
problematic calls).
[01/17]: cache.h: clarify documentation for git_path, et al
[02/17]: cache.h: complete set of git_path_submodule helpers
[03/17]: t5700: modernize style
[04/17]: add_to_alternates_file: don't add duplicate entries
[05/17]: remove hold_lock_file_for_append
[06/17]: prefer git_pathdup to git_path in some possibly-dangerous cases
[07/17]: prefer mkpathdup to mkpath in assignments
[08/17]: remote.c: drop extraneous local variable from migrate_file
[09/17]: refs.c: remove extra git_path calls from read_loose refs
[10/17]: path.c: drop git_path_submodule
[11/17]: refs.c: simplify strbufs in reflog setup and writing
[12/17]: refs.c: avoid repeated git_path calls in rename_tmp_log
[13/17]: refs.c: avoid git_path assignment in lock_ref_sha1_basic
[14/17]: refs.c: remove_empty_directories can take a strbuf
[15/17]: find_hook: keep our own static buffer
[16/17]: get_repo_path: refactor path-allocation
[17/17]: memoize common git-path constant files
-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 04/17] add_to_alternates_file: don't add duplicate entries
The add_to_alternates_file function blindly uses
hold_lock_file_for_append to copy the existing contents, and
then adds the new line to it. This has two minor problems:
1. We might add duplicate entries, which are ugly and
inefficient.
2. We do not check that the file ends with a newline, in
which case we would bogusly append to the final line.
This is quite unlikely in practice, though, as we call
this function only from git-clone, so presumably we are
the only writers of the file (and we always add a
newline).
Instead of using hold_lock_file_for_append, let's copy the
file line by line, which ensures all records are properly
terminated. If we see an extra line, we can simply abort the
update (there is no point in even copying the rest, as we
know that it would be identical to the original).
As a bonus, we also get rid of some calls to the
static-buffer mkpath and git_path functions.
Signed-off-by: Jeff King p...@peff.net
---
This is a polishing of the thread at:
http://thread.gmane.org/gmane.comp.version-control.git/270341
sha1_file.c| 47 +++---
t/t5700-clone-reference.sh | 5 +
2 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 1cee438..3400b8b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -404,13 +404,46 @@ void read_info_alternates(const char * relative_base, int
depth)
void add_to_alternates_file(const char *reference)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int fd = hold_lock_file_for_append(lock,
git_path(objects/info/alternates), LOCK_DIE_ON_ERROR);
- const char *alt = mkpath(%s\n, reference);
- write_or_die(fd, alt, strlen(alt));
- if (commit_lock_file(lock))
- die(could not close alternates file);
- if (alt_odb_tail)
- link_alt_odb_entries(alt, strlen(alt), '\n', NULL, 0);
+ char *alts = git_pathdup(objects/info/alternates);
+ FILE *in, *out;
+
+ hold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR);
+ out = fdopen_lock_file(lock, w);
+ if (!out)
+ die_errno(unable to fdopen alternates lockfile);
+
+ in = fopen(alts, r);
+ if (in) {
+ struct strbuf line = STRBUF_INIT;
+ int found = 0;
+
+ while (strbuf_getline(line, in, '\n') != EOF) {
+ if (!strcmp(reference, line.buf)) {
+ found = 1;
+ break;
+ }
+ fprintf_or_die(out, %s\n, line.buf);
+ }
+
+ strbuf_release(line);
+ fclose(in);
+
+ if (found) {
+ rollback_lock_file(lock);
+ lock = NULL;
+ }
+ }
+ else if (errno != ENOENT)
+ die_errno(unable to read alternates file);
+
+ if (lock) {
+ fprintf_or_die(out, %s\n, reference);
+ if (commit_lock_file(lock))
+ die_errno(unable to move new alternates file into
place);
+ if (alt_odb_tail)
+ link_alt_odb_entries(reference, strlen(reference),
'\n', NULL, 0);
+ }
+ free(alts);
}
int foreach_alt_odb(alt_odb_fn fn, void *cb)
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 51d131a..ef1779f 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -120,6 +120,11 @@ test_expect_success 'cloning with reference being subset
of source (-l -s)' '
git clone -l -s --reference A B E
'
+test_expect_success 'cloning with multiple references drops duplicates' '
+ git clone -s --reference B --reference A --reference B A dups
+ test_line_count = 2 dups/.git/objects/info/alternates
+'
+
test_expect_success 'clone with reference from a tagged repository' '
(
cd A git tag -a -m tagged HEAD
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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 16/17] get_repo_path: refactor path-allocation
The get_repo_path function calls mkpath() and then does some
non-trivial operations on it, like calling
is_git_directory() and read_gitfile(). These are actually
OK (they do not use more pathname static buffers
themselves), but it takes a fair bit of work to verify.
Let's use our own strbuf to store the path, and we can
simply reuse it for each iteration of the loop (we can even
avoid rewriting the beginning part, since we are trying a
series of suffixes).
To make the strbuf cleanup easier, we split out a thin
wrapper. As a bonus, this wrapper can factor out the
canonicalization that happens in all of the early-return
code paths.
Signed-off-by: Jeff King p...@peff.net
---
builtin/clone.c | 43 +--
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 303a3a7..5864ad1 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -99,51 +99,66 @@ static const char *argv_submodule[] = {
submodule, update, --init, --recursive, NULL
};
-static char *get_repo_path(const char *repo, int *is_bundle)
+static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
{
static char *suffix[] = { /.git, , .git/.git, .git };
static char *bundle_suffix[] = { .bundle, };
+ size_t baselen = path-len;
struct stat st;
int i;
for (i = 0; i ARRAY_SIZE(suffix); i++) {
- const char *path;
- path = mkpath(%s%s, repo, suffix[i]);
- if (stat(path, st))
+ strbuf_setlen(path, baselen);
+ strbuf_addstr(path, suffix[i]);
+ if (stat(path-buf, st))
continue;
- if (S_ISDIR(st.st_mode) is_git_directory(path)) {
+ if (S_ISDIR(st.st_mode) is_git_directory(path-buf)) {
*is_bundle = 0;
- return xstrdup(absolute_path(path));
+ return path-buf;
} else if (S_ISREG(st.st_mode) st.st_size 8) {
/* Is it a gitfile? */
char signature[8];
- int len, fd = open(path, O_RDONLY);
+ const char *dst;
+ int len, fd = open(path-buf, O_RDONLY);
if (fd 0)
continue;
len = read_in_full(fd, signature, 8);
close(fd);
if (len != 8 || strncmp(signature, gitdir: , 8))
continue;
- path = read_gitfile(path);
- if (path) {
+ dst = read_gitfile(path-buf);
+ if (dst) {
*is_bundle = 0;
- return xstrdup(absolute_path(path));
+ return dst;
}
}
}
for (i = 0; i ARRAY_SIZE(bundle_suffix); i++) {
- const char *path;
- path = mkpath(%s%s, repo, bundle_suffix[i]);
- if (!stat(path, st) S_ISREG(st.st_mode)) {
+ strbuf_setlen(path, baselen);
+ strbuf_addstr(path, bundle_suffix[i]);
+ if (!stat(path-buf, st) S_ISREG(st.st_mode)) {
*is_bundle = 1;
- return xstrdup(absolute_path(path));
+ return path-buf;
}
}
return NULL;
}
+static char *get_repo_path(const char *repo, int *is_bundle)
+{
+ struct strbuf path = STRBUF_INIT;
+ const char *raw;
+ char *canon;
+
+ strbuf_addstr(path, repo);
+ raw = get_repo_path_1(path, is_bundle);
+ canon = raw ? xstrdup(absolute_path(raw)) : NULL;
+ strbuf_release(path);
+ return canon;
+}
+
static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
{
const char *end = repo + strlen(repo), *start;
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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 13/17] refs.c: avoid git_path assignment in lock_ref_sha1_basic
Assigning the result of git_path is a bad pattern, because
it's not immediately obvious how long you expect the content
to stay valid (and it may be overwritten by subsequent
calls). Let's use a function-local strbuf here instead,
which we know is safe (we just have to remember to free it
in all code paths).
As a bonus, we get rid of a confusing variable-reuse
(ref_file is used for two distinct purposes).
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 32 +++-
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index 61a318f..8566677 100644
--- a/refs.c
+++ b/refs.c
@@ -2408,7 +2408,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
unsigned int flags, int *type_p,
struct strbuf *err)
{
- const char *ref_file;
+ struct strbuf ref_file = STRBUF_INIT;
+ struct strbuf orig_ref_file = STRBUF_INIT;
const char *orig_refname = refname;
struct ref_lock *lock;
int last_errno = 0;
@@ -2432,20 +2433,19 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
refname = resolve_ref_unsafe(refname, resolve_flags,
lock-old_oid.hash, type);
if (!refname errno == EISDIR) {
- /* we are trying to lock foo but we used to
+ /*
+* we are trying to lock foo but we used to
* have foo/bar which now does not exist;
* it is normal for the empty directory 'foo'
* to remain.
*/
- ref_file = git_path(%s, orig_refname);
- if (remove_empty_directories(ref_file)) {
+ strbuf_git_path(orig_ref_file, %s, orig_refname);
+ if (remove_empty_directories(orig_ref_file.buf)) {
last_errno = errno;
-
if (!verify_refname_available(orig_refname, extras,
skip,
get_loose_refs(ref_cache), err))
strbuf_addf(err, there are still refs under
'%s',
orig_refname);
-
goto error_return;
}
refname = resolve_ref_unsafe(orig_refname, resolve_flags,
@@ -2485,10 +2485,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
}
lock-ref_name = xstrdup(refname);
lock-orig_ref_name = xstrdup(orig_refname);
- ref_file = git_path(%s, refname);
+ strbuf_git_path(ref_file, %s, refname);
retry:
- switch (safe_create_leading_directories_const(ref_file)) {
+ switch (safe_create_leading_directories_const(ref_file.buf)) {
case SCLD_OK:
break; /* success */
case SCLD_VANISHED:
@@ -2497,11 +2497,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
/* fall through */
default:
last_errno = errno;
- strbuf_addf(err, unable to create directory for %s, ref_file);
+ strbuf_addf(err, unable to create directory for %s,
+ ref_file.buf);
goto error_return;
}
- if (hold_lock_file_for_update(lock-lk, ref_file, lflags) 0) {
+ if (hold_lock_file_for_update(lock-lk, ref_file.buf, lflags) 0) {
last_errno = errno;
if (errno == ENOENT --attempts_remaining 0)
/*
@@ -2511,7 +2512,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
*/
goto retry;
else {
- unable_to_lock_message(ref_file, errno, err);
+ unable_to_lock_message(ref_file.buf, errno, err);
goto error_return;
}
}
@@ -2519,12 +2520,17 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
last_errno = errno;
goto error_return;
}
- return lock;
+ goto out;
error_return:
unlock_ref(lock);
+ lock = NULL;
+
+ out:
+ strbuf_release(ref_file);
+ strbuf_release(orig_ref_file);
errno = last_errno;
- return NULL;
+ return lock;
}
/*
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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/17] refs.c: avoid repeated git_path calls in rename_tmp_log
Because it's not safe to store the static-buffer results of
git_path for a long time, we end up formatting the same
filename over and over. We can fix this by using a
function-local strbuf to store the formatted pathname and
avoid repeating ourselves.
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 3666132..61a318f 100644
--- a/refs.c
+++ b/refs.c
@@ -2930,9 +2930,13 @@ out:
static int rename_tmp_log(const char *newrefname)
{
int attempts_remaining = 4;
+ struct strbuf path = STRBUF_INIT;
+ int ret = -1;
retry:
- switch (safe_create_leading_directories_const(git_path(logs/%s,
newrefname))) {
+ strbuf_reset(path);
+ strbuf_git_path(path, logs/%s, newrefname);
+ switch (safe_create_leading_directories_const(path.buf)) {
case SCLD_OK:
break; /* success */
case SCLD_VANISHED:
@@ -2941,19 +2945,19 @@ static int rename_tmp_log(const char *newrefname)
/* fall through */
default:
error(unable to create directory for %s, newrefname);
- return -1;
+ goto out;
}
- if (rename(git_path(TMP_RENAMED_LOG), git_path(logs/%s, newrefname)))
{
+ if (rename(git_path(TMP_RENAMED_LOG), path.buf)) {
if ((errno==EISDIR || errno==ENOTDIR) --attempts_remaining
0) {
/*
* rename(a, b) when b is an existing
* directory ought to result in ISDIR, but
* Solaris 5.8 gives ENOTDIR. Sheesh.
*/
- if (remove_empty_directories(git_path(logs/%s,
newrefname))) {
+ if (remove_empty_directories(path.buf)) {
error(Directory not empty: logs/%s,
newrefname);
- return -1;
+ goto out;
}
goto retry;
} else if (errno == ENOENT --attempts_remaining 0) {
@@ -2966,10 +2970,13 @@ static int rename_tmp_log(const char *newrefname)
} else {
error(unable to move logfile TMP_RENAMED_LOG to
logs/%s: %s,
newrefname, strerror(errno));
- return -1;
+ goto out;
}
}
- return 0;
+ ret = 0;
+out:
+ strbuf_release(path);
+ return ret;
}
static int rename_ref_available(const char *oldname, const char *newname)
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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/17] path.c: drop git_path_submodule
There are no callers of the slightly-dangerous static-buffer
git_path_submodule left. Let's drop it.
Signed-off-by: Jeff King p...@peff.net
---
cache.h | 5 ++---
path.c | 10 --
2 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/cache.h b/cache.h
index 6f74f33..7a4fa90 100644
--- a/cache.h
+++ b/cache.h
@@ -713,12 +713,11 @@ extern int check_repository_format(void);
* the repository directory (git_path), or in a submodule's repository
* directory (git_path_submodule). In all cases, note that the result
* may be overwritten by another call to _any_ of the functions. Consider
- * using the safer dup or strbuf formats below.
+ * using the safer dup or strbuf formats below (in some cases, the
+ * unsafe versions have already been removed).
*/
extern const char *mkpath(const char *fmt, ...) __attribute__((format (printf,
1, 2)));
extern const char *git_path(const char *fmt, ...) __attribute__((format
(printf, 1, 2)));
-extern const char *git_path_submodule(const char *path, const char *fmt, ...)
- __attribute__((format (printf, 2, 3)));
extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
diff --git a/path.c b/path.c
index 9aad9a1..94d7ec2 100644
--- a/path.c
+++ b/path.c
@@ -245,16 +245,6 @@ static void do_submodule_path(struct strbuf *buf, const
char *path,
strbuf_cleanup_path(buf);
}
-const char *git_path_submodule(const char *path, const char *fmt, ...)
-{
- va_list args;
- struct strbuf *buf = get_pathname();
- va_start(args, fmt);
- do_submodule_path(buf, path, fmt, args);
- va_end(args);
- return buf-buf;
-}
-
char *git_pathdup_submodule(const char *path, const char *fmt, ...)
{
va_list args;
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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 11/17] refs.c: simplify strbufs in reflog setup and writing
Commit 1a83c24 (git_snpath(): retire and replace with
strbuf_git_path(), 2014-11-30) taught log_ref_setup and
log_ref_write_1 to take a strbuf parameter, rather than a
bare string. It then makes an alias to the strbuf's buf
field under the original name.
This made the original diff much shorter, but the resulting
code is more complicated that it needs to be. Since we've
aliased the pointer, we drop our reference to the strbuf to
ensure we don't accidentally change it. But if we simply
drop our alias and use logfile.buf directly, we do not
have to worry about this aliasing. It's a larger diff, but
the resulting code is simpler.
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 38 +++---
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/refs.c b/refs.c
index 06f95c4..3666132 100644
--- a/refs.c
+++ b/refs.c
@@ -3150,46 +3150,42 @@ static int should_autocreate_reflog(const char *refname)
* should_autocreate_reflog returns non-zero. Otherwise, create it
* regardless of the ref name. Fill in *err and return -1 on failure.
*/
-static int log_ref_setup(const char *refname, struct strbuf *sb_logfile,
struct strbuf *err, int force_create)
+static int log_ref_setup(const char *refname, struct strbuf *logfile, struct
strbuf *err, int force_create)
{
int logfd, oflags = O_APPEND | O_WRONLY;
- char *logfile;
- strbuf_git_path(sb_logfile, logs/%s, refname);
- logfile = sb_logfile-buf;
- /* make sure the rest of the function can't change logfile */
- sb_logfile = NULL;
+ strbuf_git_path(logfile, logs/%s, refname);
if (force_create || should_autocreate_reflog(refname)) {
- if (safe_create_leading_directories(logfile) 0) {
+ if (safe_create_leading_directories(logfile-buf) 0) {
strbuf_addf(err, unable to create directory for %s:
- %s, logfile, strerror(errno));
+ %s, logfile-buf, strerror(errno));
return -1;
}
oflags |= O_CREAT;
}
- logfd = open(logfile, oflags, 0666);
+ logfd = open(logfile-buf, oflags, 0666);
if (logfd 0) {
if (!(oflags O_CREAT) (errno == ENOENT || errno == EISDIR))
return 0;
if (errno == EISDIR) {
- if (remove_empty_directories(logfile)) {
+ if (remove_empty_directories(logfile-buf)) {
strbuf_addf(err, There are still logs under
- '%s', logfile);
+ '%s', logfile-buf);
return -1;
}
- logfd = open(logfile, oflags, 0666);
+ logfd = open(logfile-buf, oflags, 0666);
}
if (logfd 0) {
strbuf_addf(err, unable to append to %s: %s,
- logfile, strerror(errno));
+ logfile-buf, strerror(errno));
return -1;
}
}
- adjust_shared_perm(logfile);
+ adjust_shared_perm(logfile-buf);
close(logfd);
return 0;
}
@@ -3233,36 +3229,32 @@ static int log_ref_write_fd(int fd, const unsigned char
*old_sha1,
static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
const unsigned char *new_sha1, const char *msg,
- struct strbuf *sb_log_file, int flags,
+ struct strbuf *log_file, int flags,
struct strbuf *err)
{
int logfd, result, oflags = O_APPEND | O_WRONLY;
- char *log_file;
if (log_all_ref_updates 0)
log_all_ref_updates = !is_bare_repository();
- result = log_ref_setup(refname, sb_log_file, err, flags
REF_FORCE_CREATE_REFLOG);
+ result = log_ref_setup(refname, log_file, err, flags
REF_FORCE_CREATE_REFLOG);
if (result)
return result;
- log_file = sb_log_file-buf;
- /* make sure the rest of the function can't change log_file */
- sb_log_file = NULL;
- logfd = open(log_file, oflags);
+ logfd = open(log_file-buf, oflags);
if (logfd 0)
return 0;
result = log_ref_write_fd(logfd, old_sha1, new_sha1,
git_committer_info(0), msg);
if (result) {
- strbuf_addf(err, unable to append to %s: %s, log_file,
+ strbuf_addf(err, unable to append to %s: %s, log_file-buf,
strerror(errno));
close(logfd);
return -1;
}
if (close(logfd)) {
- strbuf_addf(err, unable to append to %s: %s,
[PATCH 15/17] find_hook: keep our own static buffer
The find_hook function returns the results of git_path,
which is a static buffer shared by other path-related calls.
Returning such a buffer is slightly dangerous, because it
can be overwritten by seemingly unrelated functions.
Let's at least keep our _own_ static buffer, so you can
only get in trouble by calling find_hook in quick
succession, which is less likely to happen and more obvious
to notice.
While we're at it, let's add some documentation of the
function's limitations.
Signed-off-by: Jeff King p...@peff.net
---
run-command.c | 10 ++
run-command.h | 5 +
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/run-command.c b/run-command.c
index 4d73e90..28e1d55 100644
--- a/run-command.c
+++ b/run-command.c
@@ -797,11 +797,13 @@ int finish_async(struct async *async)
const char *find_hook(const char *name)
{
- const char *path = git_path(hooks/%s, name);
- if (access(path, X_OK) 0)
- path = NULL;
+ static struct strbuf path = STRBUF_INIT;
- return path;
+ strbuf_reset(path);
+ strbuf_git_path(path, hooks/%s, name);
+ if (access(path.buf, X_OK) 0)
+ return NULL;
+ return path.buf;
}
int run_hook_ve(const char *const *env, const char *name, va_list args)
diff --git a/run-command.h b/run-command.h
index 1103805..5b4425a 100644
--- a/run-command.h
+++ b/run-command.h
@@ -52,6 +52,11 @@ int start_command(struct child_process *);
int finish_command(struct child_process *);
int run_command(struct child_process *);
+/*
+ * Returns the path to the hook file, or NULL if the hook is missing
+ * or disabled. Note that this points to static storage that will be
+ * overwritten by further calls to find_hook and run_hook_*.
+ */
extern const char *find_hook(const char *name);
LAST_ARG_MUST_BE_NULL
extern int run_hook_le(const char *const *env, const char *name, ...);
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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/17] refs.c: remove_empty_directories can take a strbuf
The first thing we do in this function is copy the input
into a strbuf. Of the 4 callers, 3 of them already have a
strbuf we could use. Let's just take the strbuf, and convert
the remaining caller to use a strbuf, rather than a raw
git_path. This is safer, anyway, as remove_dir_recursively
is a non-trivial function that might use the pathname
buffers itself (this is _probably_ OK, as the likely culprit
would be calling resolve_gitlink_ref, but we do not pass the
proper flags to ask it to avoid blowing away gitlinks).
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 34 +++---
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/refs.c b/refs.c
index 8566677..ec1d06c 100644
--- a/refs.c
+++ b/refs.c
@@ -2290,25 +2290,14 @@ static int verify_lock(struct ref_lock *lock,
return 0;
}
-static int remove_empty_directories(const char *file)
+static int remove_empty_directories(struct strbuf *path)
{
- /* we want to create a file but there is a directory there;
+ /*
+* we want to create a file but there is a directory there;
* if that is an empty directory (or a directory that contains
* only empty directories), remove them.
*/
- struct strbuf path;
- int result, save_errno;
-
- strbuf_init(path, 20);
- strbuf_addstr(path, file);
-
- result = remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY);
- save_errno = errno;
-
- strbuf_release(path);
- errno = save_errno;
-
- return result;
+ return remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY);
}
/*
@@ -2440,7 +2429,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
* to remain.
*/
strbuf_git_path(orig_ref_file, %s, orig_refname);
- if (remove_empty_directories(orig_ref_file.buf)) {
+ if (remove_empty_directories(orig_ref_file)) {
last_errno = errno;
if (!verify_refname_available(orig_refname, extras,
skip,
get_loose_refs(ref_cache), err))
@@ -2961,7 +2950,7 @@ static int rename_tmp_log(const char *newrefname)
* directory ought to result in ISDIR, but
* Solaris 5.8 gives ENOTDIR. Sheesh.
*/
- if (remove_empty_directories(path.buf)) {
+ if (remove_empty_directories(path)) {
error(Directory not empty: logs/%s,
newrefname);
goto out;
}
@@ -3046,7 +3035,14 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
if (!read_ref_full(newrefname, RESOLVE_REF_READING, sha1, NULL)
delete_ref(newrefname, sha1, REF_NODEREF)) {
if (errno==EISDIR) {
- if (remove_empty_directories(git_path(%s,
newrefname))) {
+ struct strbuf path = STRBUF_INIT;
+ int result;
+
+ strbuf_git_path(path, %s, newrefname);
+ result = remove_empty_directories(path);
+ strbuf_release(path);
+
+ if (result) {
error(Directory not empty: %s, newrefname);
goto rollback;
}
@@ -3183,7 +3179,7 @@ static int log_ref_setup(const char *refname, struct
strbuf *logfile, struct str
return 0;
if (errno == EISDIR) {
- if (remove_empty_directories(logfile-buf)) {
+ if (remove_empty_directories(logfile)) {
strbuf_addf(err, There are still logs under
'%s', logfile-buf);
return -1;
--
2.5.0.414.g670f2a4
--
To unsubscribe from this list: send the line 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 17/17] memoize common git-path constant files
One of the most common uses of git_path() is to pass a
constant, like git_path(MERGE_MSG). This has two
drawbacks:
1. The return value is a static buffer, and the lifetime
is dependent on other calls to git_path, etc.
2. There's no compile-time checking of the pathname. This
is OK for a one-off (after all, we have to spell it
correctly at least once), but many of these constant
strings appear throughout the code.
This patch introduces a series of functions to memoize
these strings, which are essentially globals for the
lifetime of the program. We compute the value once, take
ownership of the buffer, and return the cached value for
subsequent calls. cache.h provides a helper macro for
defining these functions as one-liners, and defines a few
common ones for global use.
Using a macro is a little bit gross, but it does nicely
document the purpose of the functions. If we need to touch
them all later (e.g., because we learned how to change the
git_dir variable at runtime, and need to invalidate all of
the stored values), it will be much easier to have the
complete list.
Note that the shared-global functions have separate, manual
declarations. We could do something clever with the macros
(e.g., expand it to a declaration in some places, and a
declaration _and_ a definition in path.c). But there aren't
that many, and it's probably better to stay away from
too-magical macros.
Likewise, if we abandon the C preprocessor in favor of
generating these with a script, we could get much fancier.
E.g., normalizing FOO/BAR-BAZ into git_path_foo_bar_baz.
But the small amount of saved typing is probably not worth
the resulting confusion to readers who want to grep for the
function's definition.
Signed-off-by: Jeff King p...@peff.net
---
This one is probably the most contentious, as it has very far-reaching
effects. But it really reduces the number of sites that need audited by
quite a lot.
attr.c | 4 +-
bisect.c | 7 ++-
branch.c | 14 +++---
builtin/blame.c| 7 ++-
builtin/commit.c | 32 ++---
builtin/fetch.c| 4 +-
builtin/merge.c| 30 ++--
builtin/reset.c| 2 +-
cache.h| 26 ++
contrib/examples/builtin-fetch--tool.c | 4 +-
dir.c | 4 +-
fetch-pack.c | 2 +-
path.c | 10
rerere.c | 19
sequencer.c| 87 --
shallow.c | 10 ++--
wt-status.c| 8 ++--
17 files changed, 151 insertions(+), 119 deletions(-)
diff --git a/attr.c b/attr.c
index 8f2ac6c..086c08d 100644
--- a/attr.c
+++ b/attr.c
@@ -490,6 +490,8 @@ static int git_attr_system(void)
return !git_env_bool(GIT_ATTR_NOSYSTEM, 0);
}
+static GIT_PATH_FUNC(git_path_info_attributes, INFOATTRIBUTES_FILE)
+
static void bootstrap_attr_stack(void)
{
struct attr_stack *elem;
@@ -531,7 +533,7 @@ static void bootstrap_attr_stack(void)
debug_push(elem);
}
- elem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1);
+ elem = read_attr_from_file(git_path_info_attributes(), 1);
if (!elem)
elem = xcalloc(1, sizeof(*elem));
elem-origin = NULL;
diff --git a/bisect.c b/bisect.c
index 03d5cd9..7a6498c 100644
--- a/bisect.c
+++ b/bisect.c
@@ -420,10 +420,13 @@ static int read_bisect_refs(void)
return for_each_ref_in(refs/bisect/, register_ref, NULL);
}
+static GIT_PATH_FUNC(git_path_bisect_names, BISECT_NAMES)
+static GIT_PATH_FUNC(git_path_bisect_expected_rev, BISECT_EXPECTED_REV)
+
static void read_bisect_paths(struct argv_array *array)
{
struct strbuf str = STRBUF_INIT;
- const char *filename = git_path(BISECT_NAMES);
+ const char *filename = git_path_bisect_names();
FILE *fp = fopen(filename, r);
if (!fp)
@@ -644,7 +647,7 @@ static void exit_if_skipped_commits(struct commit_list
*tried,
static int is_expected_rev(const struct object_id *oid)
{
- const char *filename = git_path(BISECT_EXPECTED_REV);
+ const char *filename = git_path_bisect_expected_rev();
struct stat st;
struct strbuf str = STRBUF_INIT;
FILE *fp;
diff --git a/branch.c b/branch.c
index b002435..e283683 100644
--- a/branch.c
+++ b/branch.c
@@ -302,11 +302,11 @@ void create_branch(const char *head,
void remove_branch_state(void)
{
- unlink(git_path(CHERRY_PICK_HEAD));
- unlink(git_path(REVERT_HEAD));
- unlink(git_path(MERGE_HEAD));
- unlink(git_path(MERGE_RR));
- unlink(git_path(MERGE_MSG));
- unlink(git_path(MERGE_MODE));
-
[PATCH v2 10/16] write_shared_index(): use tempfile module
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
read-cache.c | 38 ++
1 file changed, 6 insertions(+), 32 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 96cb9a3..89be226 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -5,6 +5,7 @@
*/
#define NO_THE_INDEX_COMPATIBILITY_MACROS
#include cache.h
+#include tempfile.h
#include lockfile.h
#include cache-tree.h
#include refs.h
@@ -2136,54 +2137,27 @@ static int write_split_index(struct index_state *istate,
return ret;
}
-static char *temporary_sharedindex;
-
-static void remove_temporary_sharedindex(void)
-{
- if (temporary_sharedindex) {
- unlink_or_warn(temporary_sharedindex);
- free(temporary_sharedindex);
- temporary_sharedindex = NULL;
- }
-}
-
-static void remove_temporary_sharedindex_on_signal(int signo)
-{
- remove_temporary_sharedindex();
- sigchain_pop(signo);
- raise(signo);
-}
+static struct tempfile temporary_sharedindex;
static int write_shared_index(struct index_state *istate,
struct lock_file *lock, unsigned flags)
{
struct split_index *si = istate-split_index;
- static int installed_handler;
int fd, ret;
- temporary_sharedindex = git_pathdup(sharedindex_XX);
- fd = mkstemp(temporary_sharedindex);
+ fd = mks_tempfile(temporary_sharedindex,
git_path(sharedindex_XX));
if (fd 0) {
- free(temporary_sharedindex);
- temporary_sharedindex = NULL;
hashclr(si-base_sha1);
return do_write_locked_index(istate, lock, flags);
}
- if (!installed_handler) {
- atexit(remove_temporary_sharedindex);
- sigchain_push_common(remove_temporary_sharedindex_on_signal);
- }
move_cache_to_base_index(istate);
ret = do_write_index(si-base, fd, 1);
- close(fd);
if (ret) {
- remove_temporary_sharedindex();
+ delete_tempfile(temporary_sharedindex);
return ret;
}
- ret = rename(temporary_sharedindex,
-git_path(sharedindex.%s, sha1_to_hex(si-base-sha1)));
- free(temporary_sharedindex);
- temporary_sharedindex = NULL;
+ ret = rename_tempfile(temporary_sharedindex,
+ git_path(sharedindex.%s,
sha1_to_hex(si-base-sha1)));
if (!ret)
hashcpy(si-base_sha1, si-base-sha1);
return ret;
--
2.5.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
[PATCH v2 07/16] prepare_tempfile_object(): new function, extracted from create_tempfile()
This makes the next step easier.
The old code used to use path to set the initial length of
tempfile-filename. This was not helpful because path was usually
relative whereas the value stored to filename will be absolute. So
just initialize the length to 0.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
tempfile.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/tempfile.c b/tempfile.c
index d835818..d840f04 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -85,11 +85,11 @@ static void remove_tempfiles_on_signal(int signo)
raise(signo);
}
-/* Make sure errno contains a meaningful value on error */
-int create_tempfile(struct tempfile *tempfile, const char *path)
+/*
+ * Initialize *tempfile if necessary and add it to tempfile_list.
+ */
+static void prepare_tempfile_object(struct tempfile *tempfile)
{
- size_t pathlen = strlen(path);
-
if (!tempfile_list) {
/* One-time initialization */
sigchain_push_common(remove_tempfiles_on_signal);
@@ -97,21 +97,27 @@ int create_tempfile(struct tempfile *tempfile, const char
*path)
}
if (tempfile-active)
- die(BUG: create_tempfile called for active object);
+ die(BUG: prepare_tempfile_object called for active object);
if (!tempfile-on_list) {
/* Initialize *tempfile and add it to tempfile_list: */
tempfile-fd = -1;
tempfile-fp = NULL;
tempfile-active = 0;
tempfile-owner = 0;
- strbuf_init(tempfile-filename, pathlen);
+ strbuf_init(tempfile-filename, 0);
tempfile-next = tempfile_list;
tempfile_list = tempfile;
tempfile-on_list = 1;
} else if (tempfile-filename.len) {
/* This shouldn't happen, but better safe than sorry. */
- die(BUG: create_tempfile called for improperly-reset object);
+ die(BUG: prepare_tempfile_object called for improperly-reset
object);
}
+}
+
+/* Make sure errno contains a meaningful value on error */
+int create_tempfile(struct tempfile *tempfile, const char *path)
+{
+ prepare_tempfile_object(tempfile);
strbuf_add_absolute_path(tempfile-filename, path);
tempfile-fd = open(tempfile-filename.buf, O_RDWR | O_CREAT | O_EXCL,
0666);
--
2.5.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
[PATCH v2 03/16] lockfile: add accessors get_lock_file_fd() and get_lock_file_fp()
We are about to move those members, so change client code to read them
through accessor functions.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
credential-store.c | 2 +-
lockfile.c | 14 ++
lockfile.h | 3 +++
read-cache.c | 2 +-
refs.c | 12 +++-
5 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/credential-store.c b/credential-store.c
index f692509..00aea3a 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -52,7 +52,7 @@ static void print_entry(struct credential *c)
static void print_line(struct strbuf *buf)
{
strbuf_addch(buf, '\n');
- write_or_die(credential_lock.fd, buf-buf, buf-len);
+ write_or_die(get_lock_file_fd(credential_lock), buf-buf, buf-len);
}
static void rewrite_credential_file(const char *fn, struct credential *c,
diff --git a/lockfile.c b/lockfile.c
index 2369eff..df9c704 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -364,6 +364,20 @@ FILE *fdopen_lock_file(struct lock_file *lk, const char
*mode)
return lk-fp;
}
+int get_lock_file_fd(struct lock_file *lk)
+{
+ if (!lk-active)
+ die(BUG: get_lock_file_fd() called for unlocked object);
+ return lk-fd;
+}
+
+FILE *get_lock_file_fp(struct lock_file *lk)
+{
+ if (!lk-active)
+ die(BUG: get_lock_file_fp() called for unlocked object);
+ return lk-fp;
+}
+
char *get_locked_file_path(struct lock_file *lk)
{
if (!lk-active)
diff --git a/lockfile.h b/lockfile.h
index a483cc9..d9dfbc9 100644
--- a/lockfile.h
+++ b/lockfile.h
@@ -203,6 +203,9 @@ extern NORETURN void unable_to_lock_die(const char *path,
int err);
*/
extern FILE *fdopen_lock_file(struct lock_file *lk, const char *mode);
+extern int get_lock_file_fd(struct lock_file *lk);
+extern FILE *get_lock_file_fp(struct lock_file *lk);
+
/*
* Return the path of the file that is locked by the specified
* lock_file object. The caller must free the memory.
diff --git a/read-cache.c b/read-cache.c
index 723d48d..96cb9a3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2112,7 +2112,7 @@ static int commit_locked_index(struct lock_file *lk)
static int do_write_locked_index(struct index_state *istate, struct lock_file
*lock,
unsigned flags)
{
- int ret = do_write_index(istate, lock-fd, 0);
+ int ret = do_write_index(istate, get_lock_file_fd(lock), 0);
if (ret)
return ret;
assert((flags (COMMIT_LOCK | CLOSE_LOCK)) !=
diff --git a/refs.c b/refs.c
index a742d79..0f49a62 100644
--- a/refs.c
+++ b/refs.c
@@ -3162,6 +3162,7 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
{
static char term = '\n';
struct object *o;
+ int fd;
o = parse_object(sha1);
if (!o) {
@@ -3178,8 +3179,9 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
- if (write_in_full(lock-lk-fd, sha1_to_hex(sha1), 40) != 40 ||
- write_in_full(lock-lk-fd, term, 1) != 1 ||
+ fd = get_lock_file_fd(lock-lk);
+ if (write_in_full(fd, sha1_to_hex(sha1), 40) != 40 ||
+ write_in_full(fd, term, 1) != 1 ||
close_ref(lock) 0) {
int save_errno = errno;
error(Couldn't write %s, lock-lk-filename.buf);
@@ -4264,10 +4266,10 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
status |= error(couldn't write %s: %s, log_file,
strerror(errno));
} else if (update
- (write_in_full(lock-lk-fd,
+ (write_in_full(get_lock_file_fd(lock-lk),
sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_str_in_full(lock-lk-fd, \n) != 1 ||
-close_ref(lock) 0)) {
+ write_str_in_full(get_lock_file_fd(lock-lk), \n)
!= 1 ||
+ close_ref(lock) 0)) {
status |= error(couldn't write %s,
lock-lk-filename.buf);
rollback_lock_file(reflog_lock);
--
2.5.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
[PATCH v2 13/16] lock_repo_for_gc(): compute the path to gc.pid only once
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/gc.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 36fe333..c41354b 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -199,6 +199,7 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
uintmax_t pid;
FILE *fp;
int fd;
+ char *pidfile_path;
if (pidfile)
/* already locked */
@@ -207,12 +208,13 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
if (gethostname(my_host, sizeof(my_host)))
strcpy(my_host, unknown);
- fd = hold_lock_file_for_update(lock, git_path(gc.pid),
+ pidfile_path = git_pathdup(gc.pid);
+ fd = hold_lock_file_for_update(lock, pidfile_path,
LOCK_DIE_ON_ERROR);
if (!force) {
static char locking_host[128];
int should_exit;
- fp = fopen(git_path(gc.pid), r);
+ fp = fopen(pidfile_path, r);
memset(locking_host, 0, sizeof(locking_host));
should_exit =
fp != NULL
@@ -236,6 +238,7 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
if (fd = 0)
rollback_lock_file(lock);
*ret_pid = pid;
+ free(pidfile_path);
return locking_host;
}
}
@@ -246,7 +249,7 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
strbuf_release(sb);
commit_lock_file(lock);
- pidfile = git_pathdup(gc.pid);
+ pidfile = pidfile_path;
sigchain_push_common(remove_pidfile_on_signal);
atexit(remove_pidfile);
--
2.5.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
[PATCH v2 04/16] lockfile: add accessor get_lock_file_path()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/commit.c | 15 ---
config.c | 14 +++---
lockfile.c | 7 +++
lockfile.h | 6 ++
refs.c | 6 +++---
shallow.c| 6 +++---
6 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 254477f..96aee0c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -324,6 +324,7 @@ static const char *prepare_index(int argc, const char
**argv, const char *prefix
struct string_list partial;
struct pathspec pathspec;
int refresh_flags = REFRESH_QUIET;
+ const char *ret;
if (is_status)
refresh_flags |= REFRESH_UNMERGED;
@@ -344,7 +345,7 @@ static const char *prepare_index(int argc, const char
**argv, const char *prefix
die(_(unable to create temporary index));
old_index_env = getenv(INDEX_ENVIRONMENT);
- setenv(INDEX_ENVIRONMENT, index_lock.filename.buf, 1);
+ setenv(INDEX_ENVIRONMENT, get_lock_file_path(index_lock), 1);
if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
die(_(interactive add failed));
@@ -355,7 +356,7 @@ static const char *prepare_index(int argc, const char
**argv, const char *prefix
unsetenv(INDEX_ENVIRONMENT);
discard_cache();
- read_cache_from(index_lock.filename.buf);
+ read_cache_from(get_lock_file_path(index_lock));
if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
if (reopen_lock_file(index_lock) 0)
die(_(unable to write index file));
@@ -365,7 +366,7 @@ static const char *prepare_index(int argc, const char
**argv, const char *prefix
warning(_(Failed to update main cache tree));
commit_style = COMMIT_NORMAL;
- return index_lock.filename.buf;
+ return get_lock_file_path(index_lock);
}
/*
@@ -388,7 +389,7 @@ static const char *prepare_index(int argc, const char
**argv, const char *prefix
if (write_locked_index(the_index, index_lock, CLOSE_LOCK))
die(_(unable to write new_index file));
commit_style = COMMIT_NORMAL;
- return index_lock.filename.buf;
+ return get_lock_file_path(index_lock);
}
/*
@@ -475,9 +476,9 @@ static const char *prepare_index(int argc, const char
**argv, const char *prefix
die(_(unable to write temporary index file));
discard_cache();
- read_cache_from(false_lock.filename.buf);
-
- return false_lock.filename.buf;
+ ret = get_lock_file_path(false_lock);
+ read_cache_from(ret);
+ return ret;
}
static int run_status(FILE *fp, const char *index_file, const char *prefix,
int nowarn,
diff --git a/config.c b/config.c
index ab46462..adf8b53 100644
--- a/config.c
+++ b/config.c
@@ -2056,9 +2056,9 @@ int git_config_set_multivar_in_file(const char
*config_filename,
MAP_PRIVATE, in_fd, 0);
close(in_fd);
- if (chmod(lock-filename.buf, st.st_mode 0) 0) {
+ if (chmod(get_lock_file_path(lock), st.st_mode 0) 0) {
error(chmod on %s failed: %s,
- lock-filename.buf, strerror(errno));
+ get_lock_file_path(lock), strerror(errno));
ret = CONFIG_NO_WRITE;
goto out_free;
}
@@ -2138,7 +2138,7 @@ out_free:
return ret;
write_err_out:
- ret = write_error(lock-filename.buf);
+ ret = write_error(get_lock_file_path(lock));
goto out_free;
}
@@ -2239,9 +2239,9 @@ int git_config_rename_section_in_file(const char
*config_filename,
fstat(fileno(config_file), st);
- if (chmod(lock-filename.buf, st.st_mode 0) 0) {
+ if (chmod(get_lock_file_path(lock), st.st_mode 0) 0) {
ret = error(chmod on %s failed: %s,
- lock-filename.buf, strerror(errno));
+ get_lock_file_path(lock), strerror(errno));
goto out;
}
@@ -2262,7 +2262,7 @@ int git_config_rename_section_in_file(const char
*config_filename,
}
store.baselen = strlen(new_name);
if (!store_write_section(out_fd, new_name)) {
- ret = write_error(lock-filename.buf);
+ ret =
write_error(get_lock_file_path(lock));
goto out;
}
/*
@@ -2288,7 +2288,7 @@ int
[PATCH v2 11/16] setup_temporary_shallow(): use tempfile module
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
shallow.c | 35 +++
1 file changed, 7 insertions(+), 28 deletions(-)
diff --git a/shallow.c b/shallow.c
index 7973e74..2ba29a5 100644
--- a/shallow.c
+++ b/shallow.c
@@ -1,4 +1,5 @@
#include cache.h
+#include tempfile.h
#include lockfile.h
#include commit.h
#include tag.h
@@ -208,50 +209,28 @@ int write_shallow_commits(struct strbuf *out, int
use_pack_protocol,
return write_shallow_commits_1(out, use_pack_protocol, extra, 0);
}
-static struct strbuf temporary_shallow = STRBUF_INIT;
-
-static void remove_temporary_shallow(void)
-{
- if (temporary_shallow.len) {
- unlink_or_warn(temporary_shallow.buf);
- strbuf_reset(temporary_shallow);
- }
-}
-
-static void remove_temporary_shallow_on_signal(int signo)
-{
- remove_temporary_shallow();
- sigchain_pop(signo);
- raise(signo);
-}
+static struct tempfile temporary_shallow;
const char *setup_temporary_shallow(const struct sha1_array *extra)
{
struct strbuf sb = STRBUF_INIT;
int fd;
- if (temporary_shallow.len)
- die(BUG: attempt to create two temporary shallow files);
-
if (write_shallow_commits(sb, 0, extra)) {
- strbuf_addstr(temporary_shallow, git_path(shallow_XX));
- fd = xmkstemp(temporary_shallow.buf);
-
- atexit(remove_temporary_shallow);
- sigchain_push_common(remove_temporary_shallow_on_signal);
+ fd = xmks_tempfile(temporary_shallow,
git_path(shallow_XX));
if (write_in_full(fd, sb.buf, sb.len) != sb.len)
die_errno(failed to write to %s,
- temporary_shallow.buf);
- close(fd);
+ get_tempfile_path(temporary_shallow));
+ close_tempfile(temporary_shallow);
strbuf_release(sb);
- return temporary_shallow.buf;
+ return get_tempfile_path(temporary_shallow);
}
/*
* is_repository_shallow() sees empty string as no shallow
* file.
*/
- return temporary_shallow.buf;
+ return get_tempfile_path(temporary_shallow);
}
void setup_alternate_shallow(struct lock_file *shallow_lock,
--
2.5.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
[PATCH v2 14/16] gc: use tempfile module to handle gc.pid file
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/gc.c | 25 +
1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index c41354b..bfe589f 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -11,6 +11,7 @@
*/
#include builtin.h
+#include tempfile.h
#include lockfile.h
#include parse-options.h
#include run-command.h
@@ -42,20 +43,7 @@ static struct argv_array prune = ARGV_ARRAY_INIT;
static struct argv_array prune_worktrees = ARGV_ARRAY_INIT;
static struct argv_array rerere = ARGV_ARRAY_INIT;
-static char *pidfile;
-
-static void remove_pidfile(void)
-{
- if (pidfile)
- unlink(pidfile);
-}
-
-static void remove_pidfile_on_signal(int signo)
-{
- remove_pidfile();
- sigchain_pop(signo);
- raise(signo);
-}
+static struct tempfile pidfile;
static void git_config_date_string(const char *key, const char **output)
{
@@ -201,7 +189,7 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
int fd;
char *pidfile_path;
- if (pidfile)
+ if (is_tempfile_active(pidfile))
/* already locked */
return NULL;
@@ -248,11 +236,8 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
write_in_full(fd, sb.buf, sb.len);
strbuf_release(sb);
commit_lock_file(lock);
-
- pidfile = pidfile_path;
- sigchain_push_common(remove_pidfile_on_signal);
- atexit(remove_pidfile);
-
+ register_tempfile(pidfile, pidfile_path);
+ free(pidfile_path);
return NULL;
}
--
2.5.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
[PATCH v2 05/16] commit_lock_file(): use get_locked_file_path()
First beef up the sanity checking in get_locked_file_path() to match
that in commit_lock_file(). Then rewrite commit_lock_file() to use
get_locked_file_path() for its pathname computation.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
lockfile.c | 28
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index 5e954ba..3904803 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -389,8 +389,10 @@ char *get_locked_file_path(struct lock_file *lk)
{
if (!lk-active)
die(BUG: get_locked_file_path() called for unlocked object);
- if (lk-filename.len = LOCK_SUFFIX_LEN)
+ if (lk-filename.len = LOCK_SUFFIX_LEN ||
+ strcmp(lk-filename.buf + lk-filename.len - LOCK_SUFFIX_LEN,
LOCK_SUFFIX))
die(BUG: get_locked_file_path() called for malformed lock
object);
+ /* remove .lock: */
return xmemdupz(lk-filename.buf, lk-filename.len - LOCK_SUFFIX_LEN);
}
@@ -458,22 +460,16 @@ int commit_lock_file_to(struct lock_file *lk, const char
*path)
int commit_lock_file(struct lock_file *lk)
{
- static struct strbuf result_file = STRBUF_INIT;
- int err;
+ char *result_path = get_locked_file_path(lk);
- if (!lk-active)
- die(BUG: attempt to commit unlocked object);
-
- if (lk-filename.len = LOCK_SUFFIX_LEN ||
- strcmp(lk-filename.buf + lk-filename.len - LOCK_SUFFIX_LEN,
LOCK_SUFFIX))
- die(BUG: lockfile filename corrupt);
-
- /* remove .lock: */
- strbuf_add(result_file, lk-filename.buf,
- lk-filename.len - LOCK_SUFFIX_LEN);
- err = commit_lock_file_to(lk, result_file.buf);
- strbuf_reset(result_file);
- return err;
+ if (commit_lock_file_to(lk, result_path)) {
+ int save_errno = errno;
+ free(result_path);
+ errno = save_errno;
+ return -1;
+ }
+ free(result_path);
+ return 0;
}
void rollback_lock_file(struct lock_file *lk)
--
2.5.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
[PATCH v2 08/16] tempfile: add several functions for creating temporary files
Add several functions for creating temporary files with
automatically-generated names, analogous to mkstemps(), but also
arranging for the files to be deleted on program exit.
The functions are named according to a pattern depending how they
operate. They will be used to replace many places in the code where
temporary files are created and cleaned up ad-hoc.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
tempfile.c | 53 ++
tempfile.h | 96 ++
2 files changed, 149 insertions(+)
diff --git a/tempfile.c b/tempfile.c
index d840f04..0b5d8ce 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -137,6 +137,59 @@ int create_tempfile(struct tempfile *tempfile, const char
*path)
return tempfile-fd;
}
+int mks_tempfile_sm(struct tempfile *tempfile,
+ const char *template, int suffixlen, int mode)
+{
+ prepare_tempfile_object(tempfile);
+
+ strbuf_add_absolute_path(tempfile-filename, template);
+ tempfile-fd = git_mkstemps_mode(tempfile-filename.buf, suffixlen,
mode);
+ if (tempfile-fd 0) {
+ strbuf_reset(tempfile-filename);
+ return -1;
+ }
+ tempfile-owner = getpid();
+ tempfile-active = 1;
+ return tempfile-fd;
+}
+
+int mks_tempfile_tsm(struct tempfile *tempfile,
+const char *template, int suffixlen, int mode)
+{
+ const char *tmpdir;
+
+ prepare_tempfile_object(tempfile);
+
+ tmpdir = getenv(TMPDIR);
+ if (!tmpdir)
+ tmpdir = /tmp;
+
+ strbuf_addf(tempfile-filename, %s/%s, tmpdir, template);
+ tempfile-fd = git_mkstemps_mode(tempfile-filename.buf, suffixlen,
mode);
+ if (tempfile-fd 0) {
+ strbuf_reset(tempfile-filename);
+ return -1;
+ }
+ tempfile-owner = getpid();
+ tempfile-active = 1;
+ return tempfile-fd;
+}
+
+int xmks_tempfile_m(struct tempfile *tempfile, const char *template, int mode)
+{
+ int fd;
+ struct strbuf full_template = STRBUF_INIT;
+
+ strbuf_add_absolute_path(full_template, template);
+ fd = mks_tempfile_m(tempfile, full_template.buf, mode);
+ if (fd 0)
+ die_errno(Unable to create temporary file '%s',
+ full_template.buf);
+
+ strbuf_release(full_template);
+ return fd;
+}
+
FILE *fdopen_tempfile(struct tempfile *tempfile, const char *mode)
{
if (!tempfile-active)
diff --git a/tempfile.h b/tempfile.h
index bcc229f..a30e12c 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -92,6 +92,102 @@ struct tempfile {
*/
extern int create_tempfile(struct tempfile *tempfile, const char *path);
+
+/*
+ * mks_tempfile functions
+ *
+ * The following functions attempt to create and open temporary files
+ * with names derived automatically from a template, in the manner of
+ * mkstemps(), and arrange for them to be deleted if the program ends
+ * before they are deleted explicitly. There is a whole family of such
+ * functions, named according to the following pattern:
+ *
+ * x?mks_tempfile_t?s?m?()
+ *
+ * The optional letters have the following meanings:
+ *
+ * x - die if the temporary file cannot be created.
+ *
+ * t - create the temporary file under $TMPDIR (as opposed to
+ * relative to the current directory). When these variants are
+ * used, template should be the pattern for the filename alone,
+ * without a path.
+ *
+ * s - template includes a suffix that is suffixlen characters long.
+ *
+ * m - the temporary file should be created with the specified mode
+ * (otherwise, the mode is set to 0600).
+ *
+ * None of these functions modify template. If the caller wants to
+ * know the (absolute) path of the file that was created, it can be
+ * read from tempfile-filename.
+ *
+ * On success, the functions return a file descriptor that is open for
+ * writing the temporary file. On errors, they return -1 and set errno
+ * appropriately (except for the x variants, which die() on errors).
+ */
+
+/* See mks_tempfile functions above. */
+extern int mks_tempfile_sm(struct tempfile *tempfile,
+ const char *template, int suffixlen, int mode);
+
+/* See mks_tempfile functions above. */
+static inline int mks_tempfile_s(struct tempfile *tempfile,
+const char *template, int suffixlen)
+{
+ return mks_tempfile_sm(tempfile, template, suffixlen, 0600);
+}
+
+/* See mks_tempfile functions above. */
+static inline int mks_tempfile_m(struct tempfile *tempfile,
+const char *template, int mode)
+{
+ return mks_tempfile_sm(tempfile, template, 0, mode);
+}
+
+/* See mks_tempfile functions above. */
+static inline int mks_tempfile(struct tempfile *tempfile,
+ const char *template)
+{
+ return mks_tempfile_sm(tempfile, template, 0,
[PATCH v2 01/16] Move lockfile documentation to lockfile.h and lockfile.c
Rearrange/rewrite it somewhat to fit its new environment. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/technical/api-lockfile.txt | 220 --- lockfile.c | 53 ++ lockfile.h | 290 --- 3 files changed, 283 insertions(+), 280 deletions(-) delete mode 100644 Documentation/technical/api-lockfile.txt diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt deleted file mode 100644 index 93b5f23..000 --- a/Documentation/technical/api-lockfile.txt +++ /dev/null @@ -1,220 +0,0 @@ -lockfile API - - -The lockfile API serves two purposes: - -* Mutual exclusion and atomic file updates. When we want to change a - file, we create a lockfile `filename.lock`, write the new file - contents into it, and then rename the lockfile to its final - destination `filename`. We create the `filename.lock` file with - `O_CREAT|O_EXCL` so that we can notice and fail if somebody else has - already locked the file, then atomically rename the lockfile to its - final destination to commit the changes and unlock the file. - -* Automatic cruft removal. If the program exits after we lock a file - but before the changes have been committed, we want to make sure - that we remove the lockfile. This is done by remembering the - lockfiles we have created in a linked list and setting up an - `atexit(3)` handler and a signal handler that clean up the - lockfiles. This mechanism ensures that outstanding lockfiles are - cleaned up if the program exits (including when `die()` is called) - or if the program dies on a signal. - -Please note that lockfiles only block other writers. Readers do not -block, but they are guaranteed to see either the old contents of the -file or the new contents of the file (assuming that the filesystem -implements `rename(2)` atomically). - - -Calling sequence - - -The caller: - -* Allocates a `struct lock_file` either as a static variable or on the - heap, initialized to zeros. Once you use the structure to call the - `hold_lock_file_*` family of functions, it belongs to the lockfile - subsystem and its storage must remain valid throughout the life of - the program (i.e. you cannot use an on-stack variable to hold this - structure). - -* Attempts to create a lockfile by passing that variable and the path - of the final destination (e.g. `$GIT_DIR/index`) to - `hold_lock_file_for_update` or `hold_lock_file_for_append`. - -* Writes new content for the destination file by either: - - * writing to the file descriptor returned by the `hold_lock_file_*` -functions (also available via `lock-fd`). - - * calling `fdopen_lock_file` to get a `FILE` pointer for the open -file and writing to the file using stdio. - -When finished writing, the caller can: - -* Close the file descriptor and rename the lockfile to its final - destination by calling `commit_lock_file` or `commit_lock_file_to`. - -* Close the file descriptor and remove the lockfile by calling - `rollback_lock_file`. - -* Close the file descriptor without removing or renaming the lockfile - by calling `close_lock_file`, and later call `commit_lock_file`, - `commit_lock_file_to`, `rollback_lock_file`, or `reopen_lock_file`. - -Even after the lockfile is committed or rolled back, the `lock_file` -object must not be freed or altered by the caller. However, it may be -reused; just pass it to another call of `hold_lock_file_for_update` or -`hold_lock_file_for_append`. - -If the program exits before you have called one of `commit_lock_file`, -`commit_lock_file_to`, `rollback_lock_file`, or `close_lock_file`, an -`atexit(3)` handler will close and remove the lockfile, rolling back -any uncommitted changes. - -If you need to close the file descriptor you obtained from a -`hold_lock_file_*` function yourself, do so by calling -`close_lock_file`. You should never call `close(2)` or `fclose(3)` -yourself! Otherwise the `struct lock_file` structure would still think -that the file descriptor needs to be closed, and a commit or rollback -would result in duplicate calls to `close(2)`. Worse yet, if you close -and then later open another file descriptor for a completely different -purpose, then a commit or rollback might close that unrelated file -descriptor. - - -Error handling --- - -The `hold_lock_file_*` functions return a file descriptor on success -or -1 on failure (unless `LOCK_DIE_ON_ERROR` is used; see below). On -errors, `errno` describes the reason for failure. Errors can be -reported by passing `errno` to one of the following helper functions: - -unable_to_lock_message:: - - Append an appropriate error message to a `strbuf`. - -unable_to_lock_error:: - - Emit an appropriate error message using `error()`. - -unable_to_lock_die:: - - Emit an appropriate error message and `die()`. - -Similarly,
[PATCH v2 06/16] tempfile: a new module for handling temporary files
A lot of work went into defining the state diagram for lockfiles and
ensuring correct, race-resistant cleanup in all circumstances.
Most of that infrastructure can be applied directly to *any* temporary
file. So extract a new tempfile module from the lockfile module.
Reimplement lockfile on top of tempfile.
Subsequent commits will add more users of the new module.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
Makefile | 1 +
lockfile.c | 261 -
lockfile.h | 73 +++--
tempfile.c | 238 +++
tempfile.h | 167 +++
5 files changed, 470 insertions(+), 270 deletions(-)
create mode 100644 tempfile.c
create mode 100644 tempfile.h
diff --git a/Makefile b/Makefile
index 54ec511..2573f89 100644
--- a/Makefile
+++ b/Makefile
@@ -786,6 +786,7 @@ LIB_OBJS += string-list.o
LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
+LIB_OBJS += tempfile.o
LIB_OBJS += trace.o
LIB_OBJS += trailer.o
LIB_OBJS += transport.o
diff --git a/lockfile.c b/lockfile.c
index 3904803..e1d68f7 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -2,90 +2,8 @@
* Copyright (c) 2005, Junio C Hamano
*/
-/*
- * State diagram and cleanup
- * -
- *
- * This module keeps track of all locked files in `lock_file_list` for
- * use at cleanup. This list and the `lock_file` objects that comprise
- * it must be kept in self-consistent states at all time, because the
- * program can be interrupted any time by a signal, in which case the
- * signal handler will walk through the list attempting to clean up
- * any open lock files.
- *
- * The possible states of a `lock_file` object are as follows:
- *
- * - Uninitialized. In this state the object's `on_list` field must be
- * zero but the rest of its contents need not be initialized. As
- * soon as the object is used in any way, it is irrevocably
- * registered in `lock_file_list`, and `on_list` is set.
- *
- * - Locked, lockfile open (after `hold_lock_file_for_update()`,
- * `hold_lock_file_for_append()`, or `reopen_lock_file()`). In this
- * state:
- *
- * - the lockfile exists
- * - `active` is set
- * - `filename` holds the filename of the lockfile
- * - `fd` holds a file descriptor open for writing to the lockfile
- * - `fp` holds a pointer to an open `FILE` object if and only if
- * `fdopen_lock_file()` has been called on the object
- * - `owner` holds the PID of the process that locked the file
- *
- * - Locked, lockfile closed (after successful `close_lock_file()`).
- * Same as the previous state, except that the lockfile is closed
- * and `fd` is -1.
- *
- * - Unlocked (after `commit_lock_file()`, `commit_lock_file_to()`,
- * `rollback_lock_file()`, a failed attempt to lock, or a failed
- * `close_lock_file()`). In this state:
- *
- * - `active` is unset
- * - `filename` is empty (usually, though there are transitory
- * states in which this condition doesn't hold). Client code should
- * *not* rely on the filename being empty in this state.
- * - `fd` is -1
- * - the object is left registered in the `lock_file_list`, and
- * `on_list` is set.
- *
- * A lockfile is owned by the process that created it. The `lock_file`
- * has an `owner` field that records the owner's PID. This field is
- * used to prevent a forked process from closing a lockfile created by
- * its parent.
- */
-
#include cache.h
#include lockfile.h
-#include sigchain.h
-
-static struct lock_file *volatile lock_file_list;
-
-static void remove_lock_files(int skip_fclose)
-{
- pid_t me = getpid();
-
- while (lock_file_list) {
- if (lock_file_list-owner == me) {
- /* fclose() is not safe to call in a signal handler */
- if (skip_fclose)
- lock_file_list-fp = NULL;
- rollback_lock_file(lock_file_list);
- }
- lock_file_list = lock_file_list-next;
- }
-}
-
-static void remove_lock_files_on_exit(void)
-{
- remove_lock_files(0);
-}
-
-static void remove_lock_files_on_signal(int signo)
-{
- remove_lock_files(1);
- sigchain_pop(signo);
- raise(signo);
-}
/*
* path = absolute or relative path name
@@ -154,60 +72,17 @@ static void resolve_symlink(struct strbuf *path)
/* Make sure errno contains a meaningful value on error */
static int lock_file(struct lock_file *lk, const char *path, int flags)
{
- size_t pathlen = strlen(path);
-
- if (!lock_file_list) {
- /* One-time initialization */
- sigchain_push_common(remove_lock_files_on_signal);
- atexit(remove_lock_files_on_exit);
- }
+ int fd;
+ struct strbuf filename = STRBUF_INIT;
- if (lk-active)
- die(BUG: cannot lock_file(\%s\) using active
[PATCH v2 12/16] diff: use tempfile module
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
diff.c | 29 +++--
1 file changed, 7 insertions(+), 22 deletions(-)
diff --git a/diff.c b/diff.c
index 7500c55..dc95247 100644
--- a/diff.c
+++ b/diff.c
@@ -2,6 +2,7 @@
* Copyright (C) 2005 Junio C Hamano
*/
#include cache.h
+#include tempfile.h
#include quote.h
#include diff.h
#include diffcore.h
@@ -312,7 +313,7 @@ static struct diff_tempfile {
const char *name; /* filename external diff should read from */
char hex[41];
char mode[10];
- char tmp_path[PATH_MAX];
+ struct tempfile tempfile;
} diff_temp[2];
typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
@@ -564,25 +565,16 @@ static struct diff_tempfile *claim_diff_tempfile(void) {
die(BUG: diff is failing to clean up its tempfiles);
}
-static int remove_tempfile_installed;
-
static void remove_tempfile(void)
{
int i;
for (i = 0; i ARRAY_SIZE(diff_temp); i++) {
- if (diff_temp[i].name == diff_temp[i].tmp_path)
- unlink_or_warn(diff_temp[i].name);
+ if (is_tempfile_active(diff_temp[i].tempfile))
+ delete_tempfile(diff_temp[i].tempfile);
diff_temp[i].name = NULL;
}
}
-static void remove_tempfile_on_signal(int signo)
-{
- remove_tempfile();
- sigchain_pop(signo);
- raise(signo);
-}
-
static void print_line_count(FILE *file, int count)
{
switch (count) {
@@ -2817,8 +2809,7 @@ static void prep_temp_blob(const char *path, struct
diff_tempfile *temp,
strbuf_addstr(template, XX_);
strbuf_addstr(template, base);
- fd = git_mkstemps(temp-tmp_path, PATH_MAX, template.buf,
- strlen(base) + 1);
+ fd = mks_tempfile_ts(temp-tempfile, template.buf, strlen(base) + 1);
if (fd 0)
die_errno(unable to create temp-file);
if (convert_to_working_tree(path,
@@ -2828,8 +2819,8 @@ static void prep_temp_blob(const char *path, struct
diff_tempfile *temp,
}
if (write_in_full(fd, blob, size) != size)
die_errno(unable to write temp-file);
- close(fd);
- temp-name = temp-tmp_path;
+ close_tempfile(temp-tempfile);
+ temp-name = get_tempfile_path(temp-tempfile);
strcpy(temp-hex, sha1_to_hex(sha1));
temp-hex[40] = 0;
sprintf(temp-mode, %06o, mode);
@@ -2854,12 +2845,6 @@ static struct diff_tempfile *prepare_temp_file(const
char *name,
return temp;
}
- if (!remove_tempfile_installed) {
- atexit(remove_tempfile);
- sigchain_push_common(remove_tempfile_on_signal);
- remove_tempfile_installed = 1;
- }
-
if (!S_ISGITLINK(one-mode)
(!one-sha1_valid ||
reuse_worktree_file(name, one-sha1, 1))) {
--
2.5.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
[PATCH v2 16/16] credential-cache--daemon: use tempfile module
Use the tempfile module to ensure that the socket file gets deleted on
program exit.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
credential-cache--daemon.c | 26 ++
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index a671b2b..eef6fce 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -1,23 +1,11 @@
#include cache.h
+#include tempfile.h
#include credential.h
#include unix-socket.h
#include sigchain.h
#include parse-options.h
-static const char *socket_path;
-
-static void cleanup_socket(void)
-{
- if (socket_path)
- unlink(socket_path);
-}
-
-static void cleanup_socket_on_signal(int sig)
-{
- cleanup_socket();
- sigchain_pop(sig);
- raise(sig);
-}
+static struct tempfile socket_file;
struct credential_cache_entry {
struct credential item;
@@ -256,6 +244,7 @@ static void check_socket_directory(const char *path)
int main(int argc, const char **argv)
{
+ const char *socket_path;
static const char *usage[] = {
git-credential-cache--daemon [opts] socket_path,
NULL
@@ -272,14 +261,11 @@ int main(int argc, const char **argv)
if (!socket_path)
usage_with_options(usage, options);
- check_socket_directory(socket_path);
-
- atexit(cleanup_socket);
- sigchain_push_common(cleanup_socket_on_signal);
+ check_socket_directory(socket_path);
+ register_tempfile(socket_file, socket_path);
serve_cache(socket_path, debug);
-
- unlink(socket_path);
+ delete_tempfile(socket_file);
return 0;
}
--
2.5.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
Feature: git stash pop --always-drop
I would find it useful to ask 'git stash pop' to always drop the stash after applying it to the working tree, even if there were conflicts. (Only if there was some hard error, such as an I/O error updating some of the files, should the stash be left on the stack.) Would a patch for such an --always-drop flag be accepted? -- Ed Avis e...@waniasset.com -- To unsubscribe from this list: send the line unsubscribe 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] send-email: provide whitelist of SMTP AUTH mechanisms
On Sun, 9 Aug 2015 14:13:33 -0400
Eric Sunshine sunsh...@sunshineco.com wrote:
On Wed, Aug 5, 2015 at 3:17 AM, Jan Viktorin
vikto...@rehivetech.com wrote:
Do I understand well that you are complaining about too
narrow commmit message?
Yes, I'm a complainer. ;-) It's minor, though, not a big deal, and
certainly not worth a re-roll if that was the only issue. In fact,
other than the undesirable Supported: line in the documentation, all
comments on v2 were minor and not demanding of a re-roll.
:)
I am trying to figure out how to write a test. It is
not very clear to me, what the testing suite does. My
attempt looks this way at the moment:
1657 do_smtp_auth_test() {
1658 git send-email \
1659 --from=Example nob...@example.com \
1660 --to=some...@example.com \
1661 --smtp-server=$(pwd)/fake.sendmail \
1662 --smtp-auth=$1 \
1663 -v \
1664 0001-*.patch \
1665 2errors out
1666 }
1667
1668 test_expect_success $PREREQ 'accepts SMTP AUTH mechanisms (see
RFC-4422, p. 8)' ' 1669 do_smtp_auth_test PLAIN LOGIN
CRAM-MD5 DIGEST-MD5 GSSAPI EXTERNAL ANONYMOUS 1670
do_smtp_auth_test ABCDEFGHIKLMNOPQRSTUVWXYZ 0123456789_-
Wouldn't this one fail the regex check you added which limits the
length to 20 characters?
Yes, it would fail. But it does not work anyway...
1671 '
1672
1673 test_expect_success $PREREQ 'does not accept non-RFC-4422
strings for SMTP AUTH' ' 1674 test_must_fail
do_smtp_auth_test ../ATTACK 1675 test_must_fail
do_smtp_auth_test TOO-LONG-BUT-VALID-STRING 1676
test_must_fail do_smtp_auth_test no-lower-case-sorry 1677 '
* I do not know yet, what to check after each do_smtp_auth_test
call.
If you were able somehow to capture the interaction with
Auth::SASL::Perl, then you'd probably want to test if it received the
whitelisted mechanisms specified via --smtp-auth, however... (see
below)
--smtp-debug
* Perhaps, each case should have its own test_expect_success call?
The grouping seems okay as-is.
* Why send-email -v does not generate any output?
As far as I know, git-send-email doesn't accept a -v flag.
True, I confused it with --smtp-debug. However, what I did not
understand was the testing framework. The TAP harness discards
everything (I expected some automatic redirection to a file for each
test.). Later I found the --verbose option that allows to see some
output from tests.
(I found a directory 'trash directory.t9001-send-email', however,
the errors file is always empty.)
Was it empty even for the cases which should have triggered the
validation regex to invoke die()?
* Is there any other place where the files out, errors are placed?
No.
* I have no idea what the fake.sendmail does (I could see its
contents but still...). Is it suitable for my tests?
It dumps its command-line arguments to one file (commandline) and
its stdin to another (msgtxt), but otherwise does no work. This is
useful for tests which need to make sure that the command-line and/or
message content gets augmented in some way, but won't help your case
since it can't capture the script's interaction with
Authen::SASL::Perl.
I can see it now. Either Perl implementation or a sendmail binary is
used. Unfortunately, this is very unfriendly for such testing.
* Should I check the behaviour '--smtp-auth overrides
sendemail.smtpAuth'?
That would be nice, but there doesn't seem to be a good way to do it
via an existing testing mechanism since you can't check the
git-sendemail's interaction with Auth::SASL::Perl. The same holds for
your question above about what to check after each do_smtp_auth_test()
call.
One possibility which comes to mind is to create a fake
Authen::SASL::Perl which merely dumps its input mechanisms to a file,
and arrange for the Perl search path to find the fake one instead. You
could then check the output file to see if it reflects your
expectations. However, this may be overkill and perhaps not worth the
effort (especially if you're not a Perl programmer).
I think that Authen::SASL::Perl mock would not help. I wanted to create
some fake sendmail (but this is impossible as stated above because
then the perl modules are not used). So the only way would be to
provide some fake socket with a static content on the other side. This
is really an overkill to just test the few lines of code.
So, what more can I do for this feature?
I think that the basic regex test is OK. It can accept lowercase
letters and do an explicit uppercase call. I do not like to rely on
internals of the SASL library. As you could see, the SASL::Perl does
not check its inputs in a very good way and its code is quite unclear
(strange for a library providing security features).
--
Jan ViktorinE-mail: vikto...@rehivetech.com
System
Re: Error pushing new branch: value too great for base (error token is...
Apologies for the delay in reply! I tried your suggestion and it works. Thanks! :) I'm curious why integer comparison is throwing error. Shouldn't i be comparing numbers with numeric operator? On Mon, Aug 10, 2015 at 3:23 PM, Gaurav Chhabra varuag.chha...@gmail.com wrote: Apologies for the delay in reply! I tried your suggestion and it works. Thanks! :) I'm curious why integer comparison is throwing error. Shouldn't i be comparing numbers with numeric operator? On Wed, Aug 5, 2015 at 11:23 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Aug 5, 2015 at 1:32 PM, Gaurav Chhabra varuag.chha...@gmail.com wrote: I had written the following code to check whether a push is for branch deletion: #!/bin/bash NULL= if [[ $new_sha -eq $NULL ]]; then # Line 17 remote: Stdin: [] [9226289d2416af4cb7365d7aaa5e382bdb3d9a89] [refs/heads/rel-a] remote: remote: hooks/pre-receive: line 17: [[: 9226289d2416af4cb7365d7aaa5e382bdb3d9a89: value too great for base (error token is 922628 9d2416af4cb7365d7aaa5e382bdb3d9a89) Although the new branch gets pushed to remote but i'm not sure why i'm getting this error and how can i fix it. I checked online and i get few links where folks had similar issue but in each such case, the error token was 08 or 09. I still tried the suggestion of using 10# in front of my $new_sha variable but to no avail. Any suggestions? Yes, try using the string comparison '=' operator rather than the numeric comparison operator '-eq'. if [[ $new_sha = $NULL ]]; then -- To unsubscribe from this list: send the line unsubscribe 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 11/17] refs.c: simplify strbufs in reflog setup and writing
On 08/10/2015 11:36 AM, Jeff King wrote:
Commit 1a83c24 (git_snpath(): retire and replace with
strbuf_git_path(), 2014-11-30) taught log_ref_setup and
log_ref_write_1 to take a strbuf parameter, rather than a
bare string. It then makes an alias to the strbuf's buf
field under the original name.
This made the original diff much shorter, but the resulting
code is more complicated that it needs to be. Since we've
aliased the pointer, we drop our reference to the strbuf to
ensure we don't accidentally change it. But if we simply
drop our alias and use logfile.buf directly, we do not
have to worry about this aliasing. It's a larger diff, but
the resulting code is simpler.
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 38 +++---
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/refs.c b/refs.c
index 06f95c4..3666132 100644
--- a/refs.c
+++ b/refs.c
@@ -3150,46 +3150,42 @@ static int should_autocreate_reflog(const char
*refname)
* should_autocreate_reflog returns non-zero. Otherwise, create it
* regardless of the ref name. Fill in *err and return -1 on failure.
*/
-static int log_ref_setup(const char *refname, struct strbuf *sb_logfile,
struct strbuf *err, int force_create)
+static int log_ref_setup(const char *refname, struct strbuf *logfile, struct
strbuf *err, int force_create)
{
[...]
@@ -3233,36 +3229,32 @@ static int log_ref_write_fd(int fd, const unsigned
char *old_sha1,
static int log_ref_write_1(const char *refname, const unsigned char
*old_sha1,
const unsigned char *new_sha1, const char *msg,
-struct strbuf *sb_log_file, int flags,
+struct strbuf *log_file, int flags,
struct strbuf *err)
{
[...]
Nice change.
How about taking this opportunity to name the parameters (logfile vs.
log_file) consistently?
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 1/2] config: add '--names-only' option to list only variable names
On Mon, Aug 10, 2015 at 11:46:06AM +0200, SZEDER Gábor wrote: 'git config' can only show values or name-value pairs, so if a shell script needs the names of set config variables it has to run 'git config --list' or '--get-regexp' and parse the output to separate config variable names from their values. However, such a parsing can't cope with multi-line values. Though 'git config' can produce null-terminated output for newline-safe parsing, that's of no use in such a case, becase shells can't cope with null characters. Even our own bash completion script suffers from these issues. Help the completion script, and shell scripts in general, by introducing the '--names-only' option to modify the output of '--list' and '--get-regexp' to list only the names of config variables, so they don't have to perform error-prone post processing to separate variable names from their values anymore. Nice. The whole thing looks very neatly done. I have only one minor nit: the option --names-only is _almost_ the same as the --name-only diff option which is somewhat similar. Obviously they do different things and do not need to match, but I wonder if it would create less annoyance to just give them the same name. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/2] completion: list variable names reliably with 'git config --names-only'
On Mon, Aug 10, 2015 at 11:46:07AM +0200, SZEDER Gábor wrote: Use the new '--names-only' option added in the previous commit to list config variable names reliably in both cases, without error-prone post processing. Signed-off-by: SZEDER Gábor sze...@ira.uka.de This looks like a pretty straight-forward application of part 1, and the resulting code is much nicer to read. Both patches: Acked-by: Jeff King p...@peff.net though I do not think my ack on completion code is worth anything. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature: git stash pop --always-drop
On Mon, Aug 10, 2015 at 12:50:51PM +, Ed Avis wrote:
An alternative would be for git stash to always print the name of the stash
it is applying. Then you can drop it afterwards by name and be sure you got
the right one. Printing the name of the stash sounds like a reasonable
bit of chatter to add anyway, do you agree?
Yeah, I agree that makes sense. We already show something like:
Dropped refs/stash@{0} (31cb86c3d700d241e315d989f460e3e83f84fa19)
when dropping. Perhaps model the message after that:
Applying refs/stash@{0} (31cb86c3d700d241e315d989f460e3e83f84fa19)
Or maybe it would be useful to actually show the stash subject, though
the defaults are not very informative (just WIP on master, and then
some totally irrelevant commit subject).
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature: git stash pop --always-drop
Jeff King peff at peff.net writes:
An alternative would be for git stash to always print the name of the stash
it is applying.
Applying refs/stash@{0} (31cb86c3d700d241e315d989f460e3e83f84fa19)
Yes, that's the one.
Or maybe it would be useful to actually show the stash subject,
That could be nice to see, but is not a substitute for the SHA.
If the stash pop failed because of conflicts then it could even print
To drop this stash manually, run 'git stash drop abcde...'
Another feature I would like to see is a kind of atomic stash apply, where
either the whole change can be applied to the working tree without conflicts,
or nothing happens.
--
Ed Avis e...@waniasset.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
