Re: [PATCH] Prevent buffer overflows when path is too long
On Tue, Nov 26, 2013 at 8:50 PM, Junio C Hamano gits...@pobox.com wrote:
Antoine Pelisse apeli...@gmail.com writes:
Some buffers created with PATH_MAX length are not checked when being
written, and can overflow if PATH_MAX is not big enough to hold the
path.
Perhaps it is time to update all of them to use strbuf? The callers
of prefix_filename() aren't that many, and all of them are prepared
to stash the returned value away when they keep it longer term, so
they would not notice if we used static struct strbuf path and
gave back path.buf (without strbuf_detach() on it). The buffer
used in clear_ce_flags() and passed to clear_ce_flags_{1,dir} are
not seen outside the callchain, and can safely become strbuf, I
think.
Let's do that, but shouldn't we also modify those that are currently
safe, like absolute_path() just above prefix_filename() ?
abspath.c| 10 --
diffcore-order.c | 14 +-
unpack-trees.c | 2 ++
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/abspath.c b/abspath.c
index e390994..29a5f9d 100644
--- a/abspath.c
+++ b/abspath.c
@@ -216,11 +216,16 @@ const char *absolute_path(const char *path)
const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
{
static char path[PATH_MAX];
+
+ if (pfx_len = PATH_MAX)
+ die(Too long prefix path: %s, pfx);
I do not think this is needed, and will reject a valid input that
used to be accepted (e.g. arg is absolute so pfx does not matter).
My mistake
- strcpy(path + pfx_len, arg);
+ if (strlcpy(path + pfx_len, arg, PATH_MAX - pfx_len) PATH_MAX)
+ die(Too long path: %s, path);
for (p = path + pfx_len; *p; p++)
if (*p == '\\')
*p = '/';
The above is curious. Unless we are doing the short-cut for no
prefix so we can just return arg codepath, we know that the
resulting length is always pfx_len + strlen(arg), no?
If you mean that the test should be more like the following:
+ if (strlcpy(path + pfx_len, arg, PATH_MAX - pfx_len) PATH_MAX - pfx_len)
Then of course, you are right, that's my mistake.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Git merge: conflict is expected, but not detected
Hi! Let's say I have two identical branches: master and topic. In master I remove some code, i.e. function bar(). In topic I do the same (commit) and after some time I realize I need bar() and revert previous commit with removal. So I end with master with no bar() and topic with bar() in its original state. When I merge I get code without bar() and no merge conflict (recursive or resolve strategies). Is it possible to detect such situations as conflicts? When bar() is C++ virtual there is no possibility to catch this with compiler. Please, CC me since I'm not subscribed. Thanks in advance! -- Cheers, Evgeniy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: gitk refresh keeps showing dangling commits
Hi, my Ubuntu saucy version of gitk is 1.8.3.2-1 I believe. I want to report what I believe is a bug. I have been using gitk for 3 years, and I use it to verify what I am doing in the shell. In the version I use now, the behavior has changed. When I do mkdir temp cd temp git init touch foo git add foo git commit -m 'foo' echo bar foo git add foo git commit -m 'foo' gitk then I see gitk showing the foo and bar commits, so far so good. Then, leaving gitk open, I do: git reset --hard HEAD~1 and in gitk, I select Reload(Ctrl-F5), and I still see both commits, not just commit foo. This is very annoying for me for doing rebases, as I don't care about all the dangling commits left by my rebases, and now I have to restart gitk each time to see a clean history. I believe Refresh(F5) may still display such commits, but a reload should display what I would get if I restarted gitk, which is just displaying one commit in the above case. cheers, Thibault -- To unsubscribe from this list: send the line 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 request: consider picked commits in branch --contains
Hi, there are development workflows where a feature or fix is by definition implemented in the dev branch and perhaps picked into a stable/rc branch. A question often being asked in such a workflow is Is this fix in this stable branch?. Usually I would call git branch -r --contains $commit to figure this out, but this command does not work in such workflows because it's a different commit since it was picked. git-log knows the --cherry-pick option to identify commits with identical changes. Wouldn't it be nice to say git branch --cherry-pick --contains $commit to see branches containing a commit even if the commit was picked?! Ralf -- To unsubscribe from this list: send the line 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/WIP] Repair DF conflicts during fetch.
I encountered a directory/file conflict when running `git fetch --prune origin`. I figured passing --prune would automatically fix DF conflicts. After looking in the code I found that prune is called after fetching. It seemed to be intentional according historical commits. I made this patch to change it, which seems to work as I expected it to. This patch doesn't have any tests and it breaks the output when it does prune branches. I'm looking for guidance to help with fixing the broken output. I tried to figure out a way to do it on my own but I realize that I don't have the expertise with the codebase or C. Thanks, for any help that I may recieve in advaned this is my first time posting. If I have submitted this wrong I applogize and look forward to any advice that I may recieve in correcting my mistakes. Tom Miller (1): Repair DF conflicts during fetch. builtin/fetch.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) -- 1.8.5.rc3.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP] Repair DF conflicts during fetch.
When a DF conflict occurs during a fetch, --prune should be able to fix
it. When fetching with --prune, the fetching process happens before
pruning causing the DF conflict to persist and report an error. This
patch prunes before fetching, thus correcting DF conflicts during a
fetch.
Signed-off-by: Tom Miller jacker...@gmail.com
---
builtin/fetch.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bd7a101..f7959d0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -824,11 +824,6 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
- if (fetch_refs(transport, ref_map)) {
- free_refs(ref_map);
- retcode = 1;
- goto cleanup;
- }
if (prune) {
/*
* If --tags was specified, pretend that the user gave us
@@ -857,6 +852,11 @@ static int do_fetch(struct transport *transport,
prune_refs(transport-remote-fetch,
transport-remote-fetch_refspec_nr, ref_map);
}
}
+ if (fetch_refs(transport, ref_map)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
free_refs(ref_map);
/* if neither --no-tags nor --tags was specified, do automated tag
--
1.8.5.rc3.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git merge: conflict is expected, but not detected
On Fri, Nov 29, 2013 at 06:26:25PM +0400, Evgeniy Ivanov wrote: Hi! Let's say I have two identical branches: master and topic. In master I remove some code, i.e. function bar(). In topic I do the same (commit) and after some time I realize I need bar() and revert previous commit with removal. So I end with master with no bar() and topic with bar() in its original state. When I merge I get code without bar() and no merge conflict (recursive or resolve strategies). Is it possible to detect such situations as conflicts? When bar() is C++ virtual there is no possibility to catch this with compiler. I don't believe so. The problem you're seeing is that by default, git considers only a small set of points for merges: the heads of the two branches and the merge base. So if one side has changed but the other has not, the changed code takes effect. This is not specifically a git problem, but a three-way merge problem in general. If you rebase instead of merge, then the code ends up the way you want it, but this may or may not be appropriate for your workflow. -- 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/WIP] Repair DF conflicts during fetch.
Tom Miller jacker...@gmail.com writes:
When a DF conflict occurs during a fetch, --prune should be able to fix
it. When fetching with --prune, the fetching process happens before
pruning causing the DF conflict to persist and report an error. This
patch prunes before fetching, thus correcting DF conflicts during a
fetch.
Signed-off-by: Tom Miller jacker...@gmail.com
---
builtin/fetch.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
Good catch.
I can't comment on the correctness of the patch right now, but here's a
test you could steal. It just reproduces what you describe, and I did
verify that it confirms the fix ;-)
diff --git i/t/t5510-fetch.sh w/t/t5510-fetch.sh
index 5d4581d..a981125 100755
--- i/t/t5510-fetch.sh
+++ w/t/t5510-fetch.sh
@@ -614,4 +614,18 @@ test_expect_success 'all boundary commits are excluded' '
test_bundle_object_count .git/objects/pack/pack-${pack##pack}.pack 3
'
+test_expect_success 'branchname D/F conflict resolved by --prune' '
+ git branch dir/file
+ git clone . prune-df-conflict
+ git branch -D dir/file
+ git branch dir
+ (
+ cd prune-df-conflict
+ git fetch --prune
+ git rev-parse origin/dir ../actual
+ )
+ git rev-parse dir expect
+ test_cmp expect actual
+'
+
test_done
--
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line 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] stash: handle specifying stashes with spaces
When trying to pop/apply a stash specified with an argument containing
spaces the user will see an error:
$ git stash pop 'stash@{two hours ago}'
Too many revisions specified: stash@{two hours ago}
This happens because word splitting is used to count non-option
arguments. Instead shift the positional arguments as the options are
processed; the number of arguments left is then the number we're after.
Add quotes where necessary.
Also add a test that verifies correct behaviour.
Signed-off-by: Øystein Walle oys...@gmail.com
---
This is perhaps an esoteric use case but it's still worth fixing in my opinion.
It also saves a fork/exec so I see it as a win-win :)
Comments welcome, of course. Especially on the test. I couldn't use a relative
date spec since the commits and stashes are made with bogus timestamps and the
spec is compared to the local time. It looks a bit icky the way it's written
now.
git-stash.sh | 20 ++--
t/t3903-stash.sh | 11 +++
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 1e541a2..0a48d42 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -358,26 +358,25 @@ parse_flags_and_rev()
i_tree=
u_tree=
- REV=$(git rev-parse --no-flags --symbolic $@) || exit 1
-
FLAGS=
for opt
do
case $opt in
-q|--quiet)
GIT_QUIET=-t
+ shift
;;
--index)
INDEX_OPTION=--index
+ shift
;;
-*)
FLAGS=${FLAGS}${FLAGS:+ }$opt
+ shift
;;
esac
done
- set -- $REV
-
case $# in
0)
have_stash || die $(gettext No stash found.)
@@ -387,17 +386,18 @@ parse_flags_and_rev()
:
;;
*)
- die $(eval_gettext Too many revisions specified:
\$REV)
+ refs=$*
+ die $(eval_gettext Too many revisions specified:
\$refs)
;;
esac
- REV=$(git rev-parse --quiet --symbolic --verify $1 2/dev/null) || {
+ REV=$(git rev-parse --quiet --symbolic --verify $1 2/dev/null) || {
reference=$1
die $(eval_gettext \$reference is not valid reference)
}
- i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null)
- set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: 2/dev/null)
+ i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null)
+ set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2:
2/dev/null)
s=$1
w_commit=$1
b_commit=$2
@@ -408,8 +408,8 @@ parse_flags_and_rev()
test $ref_stash = $(git rev-parse --symbolic-full-name ${REV%@*})
IS_STASH_REF=t
- u_commit=$(git rev-parse --quiet --verify $REV^3 2/dev/null)
- u_tree=$(git rev-parse $REV^3: 2/dev/null)
+ u_commit=$(git rev-parse --quiet --verify $REV^3 2/dev/null)
+ u_tree=$(git rev-parse $REV^3: 2/dev/null)
}
is_stash_like()
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index debda7a..0568ec5 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -673,4 +673,15 @@ test_expect_success 'store updates stash ref and reflog' '
grep quux bazzy
'
+test_expect_success 'handle stash specification with spaces' '
+ git stash clear
+ echo pig file
+ git stash
+ test_tick
+ echo cow file
+ git stash
+ git stash apply stash@{Thu Apr 7 15:17:13 2005 -0700}
+ grep pig file
+'
+
test_done
--
1.8.5.1.g359345f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] commit-slab: document clear_$slabname()
Jonathan Nieder jrnie...@gmail.com writes: Thomas Rast wrote: + * + * - void clear_indegree(struct indegree *); + * + * Free the slab's data structures. Tense shift (previous descriptions were in the present tense, while this one is in the imperative). More importantly, this doesn't answer the questions I'd have if I were in a hurry, which are what exactly is being freed (has the slab taken ownership of any memory from the user, e.g. when elemtype is a pointer?) and whether the slab needs to be init_ ed again. Maybe something like the following would work? [...] Ok, I see that while I was procrastinating, you sorted this out and Junio merged it to next. Thanks, both. -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line unsubscribe 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] stash: handle specifying stashes with spaces
Øystein Walle oys...@gmail.com writes:
When trying to pop/apply a stash specified with an argument containing
spaces the user will see an error:
$ git stash pop 'stash@{two hours ago}'
Too many revisions specified: stash@{two hours ago}
This happens because word splitting is used to count non-option
arguments. Instead shift the positional arguments as the options are
processed; the number of arguments left is then the number we're after.
[...]
for opt
do
case $opt in
-q|--quiet)
GIT_QUIET=-t
+ shift
;;
--index)
INDEX_OPTION=--index
+ shift
;;
-*)
FLAGS=${FLAGS}${FLAGS:+ }$opt
+ shift
;;
esac
done
- set -- $REV
-
But this isn't correct any more, is it? You unconditionally shift off
arguments when you see something of the form -*, even if what you shift
is not what you're currently looking at.
For example, without this patch:
$ g stash apply stash@{0} --index
On branch next
Your branch is ahead of 'origin/next' by 41 commits.
(use git push to publish your local commits)
[blah blah]
but with this patch:
$ g stash apply stash@{0} --index
--index is not valid reference
Granted, git-stash is extremely inconsistent in its handling of options.
For example, 'git stash save foo -k' does _not_ treat -k as an option.
If you set out to unify this (not just randomly (un)break one
subroutine) I'd be all for it.
--
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line 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] stash: handle specifying stashes with spaces
When trying to pop/apply a stash specified with an argument containing
spaces git-stash will throw an error:
$ git stash pop 'stash@{two hours ago}'
Too many revisions specified: stash@{two hours ago}
This happens because word splitting is used to count non-option
arguments. Make use of rev-parse's --sq option to quote the arguments
for us to ensure a correct count. Add quotes where necessary.
Also add a test that verifies correct behaviour.
Helped-by: Thomas Rast t...@thomasrast.ch
Signed-off-by: Øystein Walle oys...@gmail.com
---
Many thanks to Thomas Rast for helping me with this approach.
git-stash.sh | 14 +++---
t/t3903-stash.sh | 11 +++
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 1e541a2..f0a94ab 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -358,7 +358,7 @@ parse_flags_and_rev()
i_tree=
u_tree=
- REV=$(git rev-parse --no-flags --symbolic $@) || exit 1
+ REV=$(git rev-parse --no-flags --symbolic --sq $@) || exit 1
FLAGS=
for opt
@@ -376,7 +376,7 @@ parse_flags_and_rev()
esac
done
- set -- $REV
+ eval set -- $REV
case $# in
0)
@@ -391,13 +391,13 @@ parse_flags_and_rev()
;;
esac
- REV=$(git rev-parse --quiet --symbolic --verify $1 2/dev/null) || {
+ REV=$(git rev-parse --quiet --symbolic --verify $1 2/dev/null) || {
reference=$1
die $(eval_gettext \$reference is not valid reference)
}
- i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null)
- set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: 2/dev/null)
+ i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null)
+ set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2:
2/dev/null)
s=$1
w_commit=$1
b_commit=$2
@@ -408,8 +408,8 @@ parse_flags_and_rev()
test $ref_stash = $(git rev-parse --symbolic-full-name ${REV%@*})
IS_STASH_REF=t
- u_commit=$(git rev-parse --quiet --verify $REV^3 2/dev/null)
- u_tree=$(git rev-parse $REV^3: 2/dev/null)
+ u_commit=$(git rev-parse --quiet --verify $REV^3 2/dev/null)
+ u_tree=$(git rev-parse $REV^3: 2/dev/null)
}
is_stash_like()
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index debda7a..0568ec5 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -673,4 +673,15 @@ test_expect_success 'store updates stash ref and reflog' '
grep quux bazzy
'
+test_expect_success 'handle stash specification with spaces' '
+ git stash clear
+ echo mook file
+ git stash
+ test_tick
+ echo kleb file
+ git stash
+ git stash apply stash@{Thu Apr 7 15:17:13 2005 -0700}
+ grep mook file
+'
+
test_done
--
1.8.5.rc0.23.gaa27064.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 10/21] pack-bitmap: add support for bitmap indexes
TLDR: nitpicks. Thanks for a very nice read.
I do think it's worth fixing the syntax pedantry at the end so that we
can keep supporting arcane compilers, but otherwise, meh.
+static int open_pack_bitmap_1(struct packed_git *packfile)
This goes somewhat against the naming convention (if you can call it
that) used elsewhere in git. Usually foo_1() is an implementation
detail of foo(), used because it is convenient to wrap the main part in
another function, e.g. so that it can consistently free resources or
some such. But this one operates on one pack file, so in the terms of
the rest of git, it should probably be called open_pack_bitmap_one().
+static void show_object(struct object *object, const struct name_path *path,
+ const char *last, void *data)
+{
+ struct bitmap *base = data;
+ int bitmap_pos;
+
+ bitmap_pos = bitmap_position(object-sha1);
+
+ if (bitmap_pos 0) {
+ char *name = path_name(path, last);
+ bitmap_pos = ext_index_add_object(object, name);
+ free(name);
+ }
+
+ bitmap_set(base, bitmap_pos);
+}
+
+static void show_commit(struct commit *commit, void *data)
+{
+}
A bit unfortunate that you inherit the strange show_* naming from
builtin/pack-objects.c, which seems to have stolen some code from
builtin/rev-list.c at some point without worrying about better naming...
+static void show_objects_for_type(
+ struct bitmap *objects,
+ struct ewah_bitmap *type_filter,
+ enum object_type object_type,
+ show_reachable_fn show_reach)
+{
[...]
+ while (i objects-word_alloc ewah_iterator_next(filter, it)) {
+ eword_t word = objects-words[i] filter;
+
+ for (offset = 0; offset BITS_IN_WORD; ++offset) {
+ const unsigned char *sha1;
+ struct revindex_entry *entry;
+ uint32_t hash = 0;
+
+ if ((word offset) == 0)
+ break;
+
+ offset += ewah_bit_ctz64(word offset);
+
+ if (pos + offset bitmap_git.reuse_objects)
+ continue;
+
+ entry = bitmap_git.reverse_index-revindex[pos +
offset];
+ sha1 = nth_packed_object_sha1(bitmap_git.pack,
entry-nr);
+
+ show_reach(sha1, object_type, 0, hash, bitmap_git.pack,
entry-offset);
+ }
You have a very nice bitmap_each_bit() function in ewah/bitmap.c, why
not use it here?
+int reuse_partial_packfile_from_bitmap(struct packed_git **packfile,
+uint32_t *entries,
+off_t *up_to)
+{
+ /*
+ * Reuse the packfile content if we need more than
+ * 90% of its objects
+ */
+ static const double REUSE_PERCENT = 0.9;
Curious: is this based on some measurements or just a guess?
diff --git a/pack-bitmap.h b/pack-bitmap.h
[...]
+static const char BITMAP_IDX_SIGNATURE[] = {'B', 'I', 'T', 'M'};;
There's a stray ; at the end of the line that is technically not
permitted:
pack-bitmap.h:22:65: warning: ISO C does not allow extra ‘;’ outside of a
function [-Wpedantic]
+enum pack_bitmap_opts {
+ BITMAP_OPT_FULL_DAG = 1,
And I think this trailing comma on the last enum item is also strictly
speaking not allowed, even though it is very nice to have:
pack-bitmap.h:28:27: warning: comma at end of enumerator list [-Wpedantic]
--
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line unsubscribe 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] stash: handle specifying stashes with spaces
Thanks for looking into this!
Øystein Walle oys...@gmail.com writes:
- REV=$(git rev-parse --quiet --symbolic --verify $1 2/dev/null) || {
+ REV=$(git rev-parse --quiet --symbolic --verify $1 2/dev/null) || {
reference=$1
It's somewhat ironic that the one place where the original did use
quoting is where it's actually not required.
- i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null)
- set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: 2/dev/null)
+ i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null)
+ set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2:
2/dev/null)
Thanks for being careful here.
I wonder what we would lose by dropping the --symbolic in the line I
quoted above (which is the second parsing pass), so that it resolves to
a SHA1. We would gain some robustness, as I'm not sure $REV: works
correctly in the face of weird revision expressions like :/foo.
--
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH] submodule recursion in git-archive
On Wed, Nov 27, 2013 at 11:43:44AM -0800, Junio C Hamano wrote: Nick Townsend nick.towns...@mac.com writes: * The .gitmodules file can be dirty (easy to flag, but should we allow archive to proceed?) As we are discussing archive, which takes a tree object from the top-level project that is recorded in the object database, the information _about_ the submodule in question should come from the given tree being archived. There is no reason for the .gitmodules file that happens to be sitting in the working tree of the top-level project to be involved in the decision, so its dirtyness should not matter, I think. If the tree being archived has a submodule whose name is kernel at path linux/ (relative to the top-level project), its repository should be at .git/modules/kernel in the layout recent git-submodule prepares, and we should find that path-and-name mapping from .gitmodules recorded in that tree object we are archiving. The version that happens to be checked out to the working tree may have moved the submodule to a new path linux-3.0/ and linux-3.0/.git may have gitdir: .git/modules/kernel in it, but when archiving a tree that has the submodule at linux/, it would not help---we would not know to look at linux-3.0/.git to learn that information anyway because .gitmodules in the working tree would say that the submodule at path linux-3.0/ is with name kernel, and would not tell us anything about linux/. * Users can mess with settings both prior to git submodule init and before git submodule update. I think this is irrelevant for exactly the same reason as above. What makes this tricker, however, is how to deal with an old-style repository, where the submodule repositories are embedded in the working tree that happens to be checked out. In that case, we may have to read .gitmodules from two places, i.e. (1) We are archiving a tree with a submodule at linux/; (2) We read .gitmodules from that tree and learn that the submodule has name kernel; (3) There is no .git/modules/kernel because the repository uses the old layout (if the user never was interested in this submodule, .git/modules/kernel may also be missing, and we should tell these two cases apart by checking .git/config to see if a corresponding entry for the kernel submodule exists there); (4) In a repository that uses the old layout, there must be the repository somewhere embedded in the current working tree (this inability to remove is why we use the new layout these days). We can learn where it is by looking at .gitmodules in the working tree---map the name kernel we learned earlier, and map it to the current path (linux-3.0/ if you have been following this example so far). And in that fallback context, I would say that reading from a dirty (or messed with by the user) .gitmodules is the right thing to do. Perhaps the user may be in the process of moving the submodule in his working tree with $ mv linux-3.0 linux-3.2 $ git config -f .gitmodules submodule.kernel.path linux-3.2 but hasn't committed the change yet. For those reasons I deliberately decided not to reproduce the above logic all by myself. As I already hinted, I agree that the how to find the location of submodule repository, given a particular tree in the top-level project the submodule belongs to and the path to the submodule in question deserves a separate thread to discuss with area experts. FYI, I already started to implement this lookup of submodule paths early this year[1] but have not found the time to proceed on that yet. I am planning to continue on that topic soonish. We need it to implement a correct recursive fetch with clone on-demand as a basis for the future recursive checkout. During the work on this I hit too many open questions. Thats why I am currently working on a complete plan[2] so we can discuss and define how this needs to be implemented. It is an asciidoc document which I will send out once I am finished with it. Cheers Heiko [1] http://article.gmane.org/gmane.comp.version-control.git/217020 [2] https://github.com/hvoigt/git/wiki/submodule-fetch-config -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Git issues with submodules
On Mon, Nov 25, 2013 at 12:53:45PM -0800, Junio C Hamano wrote: Heiko Voigt hvo...@hvoigt.net writes: What I think needs fixing here first is that the ignore setting should not apply to any diffs between HEAD and index. IMO, it should only apply to the diff between worktree and index. Hmph. How about git diff $commit, the diff between the worktree and a named commit (which may or may not be HEAD)? Thats an interesting question. My first thought was that I would expect it to not show submodules since it involves the worktree, but then I could also argue that it should only show differences between whats in the index and the given commit. That would make matters more complicated but I image the use case (floating submodules) involves not caring about submodules except for some integration points when submodule sha1's are explicitly recorded. I would expect to only see diffs between these integration points. But then I am not a big user (none at all at the moment) of the floating model. Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/WIP PATCH v2] disable complete ignorance of submodules for index - HEAD diff
If the value of ignore for submodules is set to all we would not show
whats actually committed during status or diff. This can result in the
user committing unexpected submodule references. Lets be nicer and always
show whats in the index.
Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
On Thu, Nov 28, 2013 at 08:10:01AM +0100, Heiko Voigt wrote:
On Mon, Nov 25, 2013 at 03:01:34PM +0600, Sergey Sharybin wrote:
Tested the patch. `git status` now shows the changes to the
submodules, which is nice :)
However, is it possible to make it so `git commit` lists submodules in
changes to be committed section, so you'll see what's gonna to be in
the commit while typing the commit message as well?
Yes, of course that should be shown. Will add in the next iteration.
Which will hopefully be a much simpler implementation. Possibly getting
rid of this new flag.
Here is an updated version of this patch. The code is a little bit more
simplified and I changed the existing tests to account for the new
behavior we are discussing. If everyone agrees that this a desired
change in behavior I would continue adding more tests so commit, status
and so on are more explicitly tested.
Cheers Heiko
P.S.: This is still work in progress, the complete series should contain
both my patches from this thread.
builtin/diff.c| 2 ++
diff-lib.c| 3 +++
diff.h| 2 +-
submodule.c | 16 ++--
submodule.h | 1 +
t/t4027-diff-submodule.sh | 12 +---
t/t7508-status.sh | 6 +-
7 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..c47614d 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -162,6 +162,8 @@ static int builtin_diff_tree(struct rev_info *revs,
if (argc 1)
usage(builtin_diff_usage);
+ enforce_no_complete_ignore_submodule(revs-diffopt);
+
/*
* We saw two trees, ent0 and ent1. If ent1 is uninteresting,
* swap them.
diff --git a/diff-lib.c b/diff-lib.c
index 346cac6..c5219cb 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -483,6 +483,9 @@ int run_diff_index(struct rev_info *revs, int cached)
{
struct object_array_entry *ent;
+ if (cached)
+ enforce_no_complete_ignore_submodule(revs-diffopt);
+
ent = revs-pending.objects;
if (diff_cache(revs, ent-item-sha1, ent-name, cached))
exit(128);
diff --git a/diff.h b/diff.h
index e342325..81561b3 100644
--- a/diff.h
+++ b/diff.h
@@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct
diff_options *opt, void *data)
#define DIFF_OPT_FIND_COPIES_HARDER (1 6)
#define DIFF_OPT_FOLLOW_RENAMES (1 7)
#define DIFF_OPT_RENAME_EMPTY(1 8)
-/* (1 9) unused */
+#define DIFF_OPT_NO_IGNORE_SUBMODULE (1 9)
#define DIFF_OPT_HAS_CHANGES (1 10)
#define DIFF_OPT_QUICK (1 11)
#define DIFF_OPT_NO_INDEX(1 12)
diff --git a/submodule.c b/submodule.c
index 1905d75..e0719b6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -294,6 +294,16 @@ int parse_submodule_config_option(const char *var, const
char *value)
return 0;
}
+void enforce_no_complete_ignore_submodule(struct diff_options *diffopt)
+{
+ DIFF_OPT_SET(diffopt, NO_IGNORE_SUBMODULE);
+ if (DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG)
+ DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)) {
+ DIFF_OPT_CLR(diffopt, IGNORE_SUBMODULES);
+ DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
+ }
+}
+
void handle_ignore_submodules_arg(struct diff_options *diffopt,
const char *arg)
{
@@ -301,9 +311,11 @@ void handle_ignore_submodules_arg(struct diff_options
*diffopt,
DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
- if (!strcmp(arg, all))
+ if (!strcmp(arg, all)) {
+ if (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))
+ return;
DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
- else if (!strcmp(arg, untracked))
+ } else if (!strcmp(arg, untracked))
DIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
else if (!strcmp(arg, dirty))
DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
diff --git a/submodule.h b/submodule.h
index 7beec48..2c8087e 100644
--- a/submodule.h
+++ b/submodule.h
@@ -20,6 +20,7 @@ void set_diffopt_flags_from_submodule_config(struct
diff_options *diffopt,
int submodule_config(const char *var, const char *value, void *cb);
void gitmodules_config(void);
int parse_submodule_config_option(const char *var, const char *value);
+void enforce_no_complete_ignore_submodule(struct diff_options *diffopt);
void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
int
git-svn troubles with calendarserver SVN repo
Hi, I am trying to git-svn clone https://svn.calendarserver.org/repository/calendarserver/CalendarServer/ and I cannot say I am much succesful. Every couple of hundred of commits I get this: RA layer request failed: REPORT of '/repository/calendarserver/!svn/vcc/default': could not connect to server (https://svn.calendarserver.org) at /usr/local/share/perl5/Git/SVN/Ra.pm line 290. and git-svn stops. When restarting git svn fetch, it fetches another couple of hundred commits and then fails again. Given that there are 11000+ commits just in the trunk, I am afraid of a long long night. Did anybody managed to clone this repo? Or is there some way how to make git-svn more patient (this error means the SVN server is a bit flakey, right)? Best, Matěj -- http://www.ceplovi.cz/matej/, Jabber: mc...@ceplovi.cz GPG Finger: 89EF 4BC6 288A BF43 1BAB 25C3 E09F EF25 D964 84AC A day without sunshine is like night. signature.asc Description: OpenPGP digital signature
Re: Can I enter feature requests in git
On 2013-11-29, 15:53 GMT, Thomas Ferris Nicolaisen wrote: https://github.com/schacon/ticgit - with all its forks and descendants. Not sure which ones are the most active. Probably the most active is https://github.com/glogiotatidis/gitissius/, but generally whole landscape of the distributed issue trackers looks something like http://is.gd/xCNU4G The only other still developed DIT is http://bugseverywhere.org/ which is probably the most mature and reliable one. Best, Matěj Cepl -- http://www.ceplovi.cz/matej/, Jabber: mceplatceplovi.cz GPG Finger: 89EF 4BC6 288A BF43 1BAB 25C3 E09F EF25 D964 84AC Faithful love is what people look for in a person; ... -- Proverbs 19:22 (NJB) pgpYlAdiZXUVu.pgp Description: PGP signature
Re: Git merge: conflict is expected, but not detected
On Nov 29, 2013, at 06:26, Evgeniy Ivanov wrote: Let's say I have two identical branches: master and topic. In master I remove some code, i.e. function bar(). In topic I do the same (commit) and after some time I realize I need bar() and revert previous commit with removal. So I end with master with no bar() and topic with bar() in its original state. When I merge I get code without bar() and no merge conflict (recursive or resolve strategies). Is it possible to detect such situations as conflicts? When bar() is C++ virtual there is no possibility to catch this with compiler. You can do something like: git checkout master git merge -s ours --no-commit topic # conflicts, if any, will happen during cherry-pick git cherry-pick --no-commit ..topic git commit -m Merge branch 'topic' Which will give you a merge commit as though using git merge but it will have restored the bar() function. However, depending on what's happened on the topic branch, you might have to wade through some conflicts that would not happen with a real git merge since cherry- pick will replay all the commits from the topic branch that aren't in master. Maybe some day git merge will grow a --cherry-pick option. -- To unsubscribe from this list: send the line 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] gettext.c: only work around the vsnprintf bug on glibc 2.17
Bug 6530 [1] causes git show v0.99.6~1 to fail with error your
vsnprintf is broken. The workaround avoids that, but it corrupts
system error messages in non-C locales.
The bug has been fixed since 2.17. If git is built with glibc, it can
know running libc version with gnu_get_libc_version() and avoid the
workaround on fixed versions. As a side effect, the workaround is
dropped for all non-glibc systems.
Tested on Gentoo Linux, glibc 2.17, amd64.
[1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
We could even dlopen and look for gnu_get_libc_version at runtime and
drop USE_GLIBC define. But there may be other problems with dl* on
other platforms..
Somebody should test for the other two USE_GLIBC = YesPlease I
added. I don't have Debian/FreeBSD nor any non-Linux GNU systems.
Makefile | 5 +
config.mak.uname | 3 +++
gettext.c| 34 --
3 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index af847f8..8df6d6d 100644
--- a/Makefile
+++ b/Makefile
@@ -66,6 +66,8 @@ all::
# Define LIBC_CONTAINS_LIBINTL if your gettext implementation doesn't
# need -lintl when linking.
#
+# Define USE_GLIBC if your libc version is glibc = 2.1.
+#
# Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt
# doesn't support GNU extensions like --check and --statistics
#
@@ -1203,6 +1205,9 @@ ifndef NO_GETTEXT
ifndef LIBC_CONTAINS_LIBINTL
EXTLIBS += -lintl
endif
+ifdef USE_GLIBC
+ BASIC_CFLAGS += -DUSE_GLIBC
+endif
endif
ifdef NEEDS_SOCKET
EXTLIBS += -lsocket
diff --git a/config.mak.uname b/config.mak.uname
index 82d549e..ffb01e0 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -33,6 +33,7 @@ ifeq ($(uname_S),Linux)
HAVE_PATHS_H = YesPlease
LIBC_CONTAINS_LIBINTL = YesPlease
HAVE_DEV_TTY = YesPlease
+ USE_GLIBC = YesPlease
endif
ifeq ($(uname_S),GNU/kFreeBSD)
NO_STRLCPY = YesPlease
@@ -40,6 +41,7 @@ ifeq ($(uname_S),GNU/kFreeBSD)
HAVE_PATHS_H = YesPlease
DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
LIBC_CONTAINS_LIBINTL = YesPlease
+ USE_GLIBC = YesPlease
endif
ifeq ($(uname_S),UnixWare)
CC = cc
@@ -236,6 +238,7 @@ ifeq ($(uname_S),GNU)
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
LIBC_CONTAINS_LIBINTL = YesPlease
+ USE_GLIBC = YesPlease
endif
ifeq ($(uname_S),IRIX)
NO_SETENV = YesPlease
diff --git a/gettext.c b/gettext.c
index 71e9545..91e679d 100644
--- a/gettext.c
+++ b/gettext.c
@@ -18,6 +18,13 @@
# endif
#endif
+#ifdef USE_GLIBC
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include gnu/libc-version.h
+#endif
+
#ifdef GETTEXT_POISON
int use_gettext_poison(void)
{
@@ -30,6 +37,7 @@ int use_gettext_poison(void)
#ifndef NO_GETTEXT
static const char *charset;
+static int vsnprintf_workaround;
static void init_gettext_charset(const char *domain)
{
/*
@@ -99,9 +107,7 @@ static void init_gettext_charset(const char *domain)
$ LANGUAGE= LANG=de_DE.utf8 ./test
test: Kein passendes Ger?t gefunden
- In the long term we should probably see about getting that
- vsnprintf bug in glibc fixed, and audit our code so it won't
- fall apart under a non-C locale.
+ The vsnprintf bug has been fixed since 2.17.
Then we could simply set LC_CTYPE from the environment, which would
make things like the external perror(3) messages work.
@@ -112,20 +118,36 @@ static void init_gettext_charset(const char *domain)
1. http://sourceware.org/bugzilla/show_bug.cgi?id=6530
2. E.g. Content-Type: text/plain; charset=UTF-8\n in po/is.po
*/
- setlocale(LC_CTYPE, );
+ if (vsnprintf_workaround)
+ setlocale(LC_CTYPE, );
charset = locale_charset();
bind_textdomain_codeset(domain, charset);
- setlocale(LC_CTYPE, C);
+ if (vsnprintf_workaround)
+ setlocale(LC_CTYPE, C);
}
void git_setup_gettext(void)
{
const char *podir = getenv(GIT_TEXTDOMAINDIR);
+#ifdef USE_GLIBC
+ int major, minor;
+ const char *version = gnu_get_libc_version();
+
+ if (version sscanf(version, %d.%d, major, minor) == 2
+ (major 2 || (major == 2 minor = 17)))
+ vsnprintf_workaround = 0;
+ else
+ vsnprintf_workaround = 1;
+#endif
+
if (!podir)
podir = GIT_LOCALE_PATH;
bindtextdomain(git, podir);
- setlocale(LC_MESSAGES, );
+ if (vsnprintf_workaround)
+ setlocale(LC_MESSAGES, );
+ else
+ setlocale(LC_ALL, );
init_gettext_charset(git);
textdomain(git);
}
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More
