Re: [PATCH v2 1/3] test: use unambigous leading path (/foo) for mingw
On 2013-09-13 21.51, Junio C Hamano wrote: > Jiang Xin writes: > >> In test cases for relative_path, path with one leading character >> (such as /a, /x) may be recogonized as "a:/" or "x:/" if there is >> such doc drive on MINGW platform. Use an umambigous leading path >> "/foo" instead. > > "DOS drive", you mean? > > Are they really spelled as /a or /x (not e.g. //a or something)? > > Just double-checking. Yes, there is a directoctory structure in / like this: /usr /bin /lib Then we have the drive letters mapped to single letters: /c/Documents and Settings /c/temp As an alternative c:/temp can be used or the DOS style "c:\temp" And the // or "\\" is used for the UNC names (Universal Name Convention) //Servername/ShareName/Directory /Torsten -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] Trivial cleanups
Felipe Contreras (5): branch: trivial style fix sha1-name: trivial style cleanup merge: simplify ff-only option t: replace pulls with merges pull: cleanup documentation Documentation/git-pull.txt | 4 ++-- builtin/branch.c | 3 +-- builtin/merge.c| 11 ++- sha1_name.c| 1 - t/annotate-tests.sh| 2 +- t/t4200-rerere.sh | 2 +- t/t9114-git-svn-dcommit-merge.sh | 2 +- t/t9500-gitweb-standalone-no-errors.sh | 2 +- 8 files changed, 9 insertions(+), 18 deletions(-) -- 1.8.4-fc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] branch: trivial style fix
Signed-off-by: Felipe Contreras
---
builtin/branch.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 0836890..c5aa0ba 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -968,9 +968,8 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
die(_("no such branch '%s'"), argv[0]);
}
- if (!branch_has_merge_config(branch)) {
+ if (!branch_has_merge_config(branch))
die(_("Branch '%s' has no upstream information"),
branch->name);
- }
strbuf_addf(&buf, "branch.%s.remote", branch->name);
git_config_set_multivar(buf.buf, NULL, NULL, 1);
--
1.8.4-fc
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] sha1-name: trivial style cleanup
Signed-off-by: Felipe Contreras
---
sha1_name.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sha1_name.c b/sha1_name.c
index 65ad066..9f3e340 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -343,7 +343,6 @@ static int get_short_sha1(const char *name, int len,
unsigned char *sha1,
return status;
}
-
int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)
{
char hex_pfx[40];
--
1.8.4-fc
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] t: replace pulls with merges
This is what the code intended. No functional changes. Signed-off-by: Felipe Contreras --- t/annotate-tests.sh| 2 +- t/t4200-rerere.sh | 2 +- t/t9114-git-svn-dcommit-merge.sh | 2 +- t/t9500-gitweb-standalone-no-errors.sh | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index d4e7f47..01deece 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -92,7 +92,7 @@ test_expect_success 'blame 2 authors + 1 branch2 author' ' ' test_expect_success 'merge branch1 & branch2' ' - git pull . branch1 + git merge branch1 ' test_expect_success 'blame 2 authors + 2 merged-in authors' ' diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 7ff..cf19eb7 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -172,7 +172,7 @@ test_expect_success 'first postimage wins' ' git show second^:a1 | sed "s/To die: t/To die! T/" >a1 && git commit -q -a -m third && - test_must_fail git pull . first && + test_must_fail git merge first && # rerere kicked in ! grep "^===\$" a1 && test_cmp expect a1 diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh index f524d2f..d33d714 100755 --- a/t/t9114-git-svn-dcommit-merge.sh +++ b/t/t9114-git-svn-dcommit-merge.sh @@ -62,7 +62,7 @@ test_expect_success 'setup git mirror and merge' ' echo friend > README && cat tmp >> README && git commit -a -m "friend" && - git pull . merge + git merge merge ' test_debug 'gitk --all & sleep 1' diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh index 6fca193..3864388 100755 --- a/t/t9500-gitweb-standalone-no-errors.sh +++ b/t/t9500-gitweb-standalone-no-errors.sh @@ -328,7 +328,7 @@ test_expect_success \ git add b && git commit -a -m "On branch" && git checkout master && -git pull . b && +git merge b && git tag merge_commit' test_expect_success \ -- 1.8.4-fc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] pull: cleanup documentation
'origin/master' is very clear, no need to specify the 'remotes/' prefix, or babysit the user. Signed-off-by: Felipe Contreras --- Documentation/git-pull.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 6ef8d59..4dd941d 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -39,7 +39,7 @@ Assume the following history exists and the current branch is "`master`": - A---B---C master on origin + A---B---C origin/master / D---E---F---G master @@ -51,7 +51,7 @@ result in a new commit along with the names of the two parent commits and a log message from the user describing the changes. - A---B---C remotes/origin/master + A---B---C origin/master / \ D---E---F---G---H master -- 1.8.4-fc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] merge: simplify ff-only option
No functional changes.
Signed-off-by: Felipe Contreras
---
builtin/merge.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 34a6166..da9fc08 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -186,13 +186,6 @@ static int option_parse_n(const struct option *opt,
return 0;
}
-static int option_parse_ff_only(const struct option *opt,
- const char *arg, int unset)
-{
- fast_forward = FF_ONLY;
- return 0;
-}
-
static struct option builtin_merge_options[] = {
{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
N_("do not show a diffstat at the end of the merge"),
@@ -210,9 +203,9 @@ static struct option builtin_merge_options[] = {
OPT_BOOL('e', "edit", &option_edit,
N_("edit message before committing")),
OPT_SET_INT(0, "ff", &fast_forward, N_("allow fast-forward (default)"),
FF_ALLOW),
- { OPTION_CALLBACK, 0, "ff-only", NULL, NULL,
+ { OPTION_SET_INT, 0, "ff-only", &fast_forward, NULL,
N_("abort if fast-forward is not possible"),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG, option_parse_ff_only },
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },
OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
OPT_BOOL(0, "verify-signatures", &verify_signatures,
N_("Verify that the named commit has a valid GPG signature")),
--
1.8.4-fc
--
To unsubscribe from this list: send the line "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 2/3] version-gen: avoid messing the version
If the version is 'v1.8.4-rc1' that is the version, and there's no need
to change it to anything else, like 'v1.8.4.rc1'.
If RedHat, or somebody else, needs a specific version, they can use the
'version' file, like everybody else.
Signed-off-by: Felipe Contreras
---
GIT-VERSION-GEN | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index e96538d..19902e9 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -26,10 +26,8 @@ describe () {
if test -f version
then
VN=$(cat version) || VN="$DEF_VER"
-elif test -d ${GIT_DIR:-.git} -o -f .git && describe
+elif test ! -d ${GIT_DIR:-.git} -a ! -f .git || ! describe
then
- VN=$(echo "$VN" | sed -e 's/-/./g')
-else
VN="$DEF_VER"
fi
--
1.8.4-fc
--
To unsubscribe from this list: send the line "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 3/3] build: fix rpm versioning
The current versioning scheme doesn't work properly: git-1.8.4 < git-1.8.4.rc1 git-1.8.4 < git-1.8.4-rc1 git-1.8.4 > git-1.8.4~rc1 Since v1.8.4 final is obviously greater than v1.8.4-rc1, we need to use a tilde so RPM detects it properly as a greater version number. This works in rpm-4.10, in older versions, a tilde would work as bad as anything else. Signed-off-by: Felipe Contreras --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3588ca1..7a8bee7 100644 --- a/Makefile +++ b/Makefile @@ -2426,7 +2426,7 @@ quick-install-html: ### Maintainer's dist rules git.spec: git.spec.in GIT-VERSION-FILE - sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+ + sed -e 's/@@VERSION@@/$(subst -,~,$(GIT_VERSION))/g' < $< > $@+ mv $@+ $@ GIT_TARNAME = git-$(GIT_VERSION) -- 1.8.4-fc -- To unsubscribe from this list: send the line "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 0/3] Version fixes and cleanups
Felipe Contreras (3): version-gen: cleanup version-gen: avoid messing the version build: fix rpm versioning GIT-VERSION-GEN | 36 +++- Makefile| 2 +- 2 files changed, 20 insertions(+), 18 deletions(-) -- 1.8.4-fc -- To unsubscribe from this list: send the line "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 1/3] version-gen: cleanup
No functional changes.
Signed-off-by: Felipe Contreras
---
GIT-VERSION-GEN | 36
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 06026ea..e96538d 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -6,22 +6,29 @@ DEF_VER=v1.8.4
LF='
'
-# First see if there is a version file (included in release tarballs),
-# then try git-describe, then default.
-if test -f version
-then
- VN=$(cat version) || VN="$DEF_VER"
-elif test -d ${GIT_DIR:-.git} -o -f .git &&
- VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) &&
+describe () {
+ VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) ||
return 1
case "$VN" in
- *$LF*) (exit 1) ;;
+ *$LF*)
+ return 1
+ ;;
v[0-9]*)
git update-index -q --refresh
test -z "$(git diff-index --name-only HEAD --)" ||
- VN="$VN-dirty" ;;
+ VN="$VN-dirty"
+ return 0
+ ;;
esac
+}
+
+# First see if there is a version file (included in release tarballs),
+# then try 'git describe', then default.
+if test -f version
+then
+ VN=$(cat version) || VN="$DEF_VER"
+elif test -d ${GIT_DIR:-.git} -o -f .git && describe
then
- VN=$(echo "$VN" | sed -e 's/-/./g');
+ VN=$(echo "$VN" | sed -e 's/-/./g')
else
VN="$DEF_VER"
fi
@@ -34,9 +41,6 @@ then
else
VC=unset
fi
-test "$VN" = "$VC" || {
- echo >&2 "GIT_VERSION = $VN"
- echo "GIT_VERSION = $VN" >$GVF
-}
-
-
+test "$VN" = "$VC" && exit
+echo >&2 "GIT_VERSION = $VN"
+echo "GIT_VERSION = $VN" >$GVF
--
1.8.4-fc
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] relative_path should honor dos_drive_prefix
On 13.09.13 07:08, Jiang Xin wrote:
> Tvangeste found that the "relative_path" function could not work
> properly on Windows if "in" and "prefix" have dos driver prefix.
> ($gmane/234434)
>
> e.g., When execute: test-path-utils relative_path "C:/a/b" "D:/x/y",
> should return "C:/a/b", but returns "../../C:/a/b", which is wrong.
>
> So make relative_path honor dos_drive_prefix, and add test cases
> for it in t0060.
>
> Reported-by: Tvangeste
> Helped-by: Johannes Sixt
> Signed-off-by: Jiang Xin
> ---
> path.c| 20
> t/t0060-path-utils.sh | 4
> 2 files changed, 24 insertions(+)
>
> diff --git a/path.c b/path.c
> index 9fd28bcd..65d376d 100644
> --- a/path.c
> +++ b/path.c
> @@ -434,6 +434,16 @@ int adjust_shared_perm(const char *path)
> return 0;
> }
>
> +static int have_same_root(const char *path1, const char *path2)
> +{
> + int is_abs1, is_abs2;
> +
> + is_abs1 = is_absolute_path(path1);
> + is_abs2 = is_absolute_path(path2);
> + return (is_abs1 && is_abs2 && tolower(path1[0]) == tolower(path2[0])) ||
> +(!is_abs1 && !is_abs2);
> +}
> +
I think the name of the fuction is somewhat misleading, as we are not sure if
they really have the same root.
And that is investigated further down.
may_have_same_root() could be a better name.
[snip]
> while (i < prefix_len && j < in_len && prefix[i] == in[j]) {
> if (is_dir_sep(prefix[i])) {
> while (is_dir_sep(prefix[i]))
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 82a6f21..0187d11 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -210,6 +210,10 @@ relative_path foo/a/b/ foo/a/b ./
> relative_path foo/a foo/a/b ../
> relative_path foo/x/yfoo/a/b ../../x/y
> relative_path foo/a/cfoo/a/b ../c
> +relative_path foo/a/b/foo/x/yfoo/a/b
> +relative_path /foo/a/b foo/x/y /foo/a/b
> +relative_path d:/a/b D:/a/c ../bMINGW
> +relative_path C:/a/b D:/a/c C:/a/b MINGW
Side question:
What happens if we feed in a relative path with a dos drive?
like "c:foo" which is different from c:/foo.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] version-gen: avoid messing the version
On Fri, Sep 13, 2013 at 1:16 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> If the version is 'v1.8.4-rc1' that is the version, and there's no need >> to change it to anything else, like 'v1.8.4.rc1'. >> >> If RedHat, or somebody else, needs a specific version, they can use the >> 'version' file, like everybody else. >> >> Signed-off-by: Felipe Contreras >> --- > > I already explained to you why this is a bad change. No, you did not. All you did is throw a non sequitur argument which I already exposed as such. > When we say "we try to avoid regressions", we really mean it. Maybe by "regressions" you mean progress. > Before coming up with a change to pay Paul by robbing Peter, we must > make an honest effort to see if there is a way to pay Paul without > robbing anybody. There is, because Peter, the RedHat maintainer, can do exactly the same that Alice does; use the 'version' file. In fact, Peter, cannot possibly use Git's version because of this: % rpmdev-vercmp git-1.8.4 git-1.8.4.rc1 git-1.8.4 < git-1.8.4.rc1 % rpmdev-vercmp git-1.8.4 git-1.8.4-rc1 git-1.8.4 < git-1.8.4-rc1 % rpmdev-vercmp git-1.8.4 git-1.8.4~rc1 git-1.8.4 > git-1.8.4~rc1 Fedora's guideline[1] is to use a format like git-1.8.4-1.rc1, then the final release would be git-1.8.4-2, in other words, the '.rc1' is mostly informative, but that has nothing to do with Git's internal version (git-1.8.4.rc1). openSUSE's guideline[1] prefers to use git-1.8.4~rc1. Either way, none of them could possibly use git-1.8.4.rc1, because that's greater than git-1.8.4. > This change forces existing users who depend on how dashes are > mangled into dots to change their tooling. No, it does not. You just say that, and you assume because you said it, it must be true; it's not. First of all, nor RedHat, nor any other RPM-based distribution needs this any more, because as a simple search demonstrates, they don't use release candidates any more. http://www.rpmfind.net/linux/rpm2html/search.php?query=git&submit=Search+... Essentially, Peter is a figment of your imagination. The closest thing to Peter 1) doesn't use release candidates, and 2) if he did, he wouldn't use a version like git-1.8.4.rc1. > We do occasionally make deliberate regressions that force existing > users to change the way they work, but only when there is no other > way, and when the benefit of the change far outweighs the cost of > such an adjustment, and with careful planning to ease the pain of > transition. The updates to "git add" and "git push" planned for 2.0 > fall into that category. > > There has to be a benefit that far outweighs the inconvenience this > patch imposes on existing users, but I do not see there is any. "If > somebody needs a specific version, they can use the 'version' file" > does not justify it at all; it equally applies to those who want to > use a version name with a dash. > > Besides, the patch does not even do what it claims to do; if the > version is "v1.8.4-rc1", what you get out of the updated code is > "1.8.4-rc1", still losing the leading "v". Wrong. % git tag -m '' v1.8.5-rc1 % ./GIT-VERSION-GEN GIT_VERSION = 1.8.5-rc1-dirty [1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines#NonNumericRelease [2] http://en.opensuse.org/openSUSE:Package_naming_guidelines -- Felipe Contreras -- To unsubscribe from this list: send the line "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] t7508: avoid non-portable sed expression
2556b996 (status: disable display of '#' comment prefix by default;
2013-09-06) introduced tests which fail on Mac OS X due to unportable
use of \t (for TAB) in a sed expression. POSIX [1][2] also disallows
it. Fix this.
[1]:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html#tag_20_116_13_02
[2]:
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03_02
Signed-off-by: Eric Sunshine
---
This applies against 'next'.
t/t7508-status.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 7d52dbf..6fb59f3 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -61,7 +61,8 @@ test_expect_success 'status (1)' '
'
strip_comments () {
- sed "s/^\# //; s/^\#$//; s/^#\t/\t/" <"$1" >"$1".tmp &&
+ tab=' '
+ sed "s/^\# //; s/^\#$//; s/^#$tab/$tab/" <"$1" >"$1".tmp &&
rm "$1" && mv "$1".tmp "$1"
}
--
1.8.4.535.g7b94f8e
--
To unsubscribe from this list: send the line "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/4] pack v4: add v4_size to struct delta_base_cache_entry
On Fri, 13 Sep 2013, Nicolas Pitre wrote: > On Fri, 13 Sep 2013, Duy Nguyen wrote: > > > On Fri, Sep 13, 2013 at 8:27 PM, Nicolas Pitre wrote: > > > However I can see that, as you say, the same base object is repeatedly > > > referenced. This means that we need to parse it over and over again > > > just to find the right offset where the needed entries start. We > > > probably end up skipping over the first entries in a tree object > > > multiple times. And that would be true even when the core code learns > > > to walk pv4 trees directly. > > > > > > So here's the beginning of a tree offset cache to mitigate this problem. > > > It is incomplete as the cache function is not implemented, etc. But > > > that should give you the idea. > > > > Thanks. I'll have a closer look and maybe complete your patch. > > I've committed an almost final patch and pushed it out. It is disabled > right now due to some bug I've not found yet. But you can have a look. Found the bug. The cache is currently updated by the caller. The caller may ask for a copy of 2 entries from a base object, but that base object may itself copy those objects from somewhere else in a larger chunk. Let's consider this example: tree A -- 0 (0) copy 2 entries from tree B starting at entry 0 1 (2) copy 1 entry from tree B starting at entry 3 tree B -- 0 (0) copy 6 entries from tree C starting at entry 0 1 (6) entry "foo.txt" 2 (7) entry "bar.txt" Right now, the code calls decode_entries() to decode 2 entries from tree B but those entries are part of a copy from tree C. When that call returns, the cache is updated as if tree B entry #2 would start at offset 1 but this is wrong because offset 0 in tree B covers 6 entries and therefore offset 1 is for entry #6. So this needs a rethink. I won't be able to work on this for a few days. Nicolas -- To unsubscribe from this list: send the line "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] pack-objects: no crc check when the cached version is used
On Sat, 14 Sep 2013, Duy Nguyen wrote: > On Sat, Sep 14, 2013 at 4:26 AM, Thomas Rast wrote: > > I tried the perf script below, but at least for the git repo the only > > thing I can see is noise. > > --stdout does not set do_check_packed_object_crc, you need to run > pack-objects without --stdout (i.e. the real use case is repack) And for those who might wonder why, you can have a look at the description for commit 0e8189e2708b. This was probably one of my best commit logs ever. ;-) This commit also provides a hint about the cost of over-checking the CRC. Nicolas -- To unsubscribe from this list: send the line "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/4] pack v4: add v4_size to struct delta_base_cache_entry
On Fri, 13 Sep 2013, Duy Nguyen wrote: > On Fri, Sep 13, 2013 at 8:27 PM, Nicolas Pitre wrote: > > On Thu, 12 Sep 2013, Nguyễn Thái Ngọc Duy wrote: > > > >> The intention is to store flat v4 trees in delta base cache to avoid > >> repeatedly expanding copy sequences in v4 trees. When the user needs > >> to unpack a v4 tree and the tree is found in the cache, the tree will > >> be converted back to canonical format. Future tree_desc interface may > >> skip canonical format and read v4 trees directly. > >> > >> For that to work we need to keep track of v4 tree size after all copy > >> sequences are expanded, which is the purpose of this new field. > > > > Hmmm I think this is going in a wrong direction. > > Good thing you caught me early. I was planning to implement a better > version of this on the weekend. And you are not wrong about code > maintainability, unpack_entry() so far looks very close to a real > mess. Yes. We might have to break it into sub-functions. However this has the potential to recurse rather deeply so it is necessary to limit its stack footprint as well. > > Yet, pavkv4 tree walking shouldn't need a cache since there is nothing > > to expand in the end. Entries should be advanced one by one as they are > > needed. Granted when converting back to a canonical object we need all > > of them, but eventually this shouldn't be the main mode of operation. > > There's another case where one of the base tree is not v4 (the packer > is inefficient, like my index-pack --fix-thin). For trees with leading > zeros in entry mode, we can just do a lossy conversion to v4, but I > wonder if there is a case where we can't even convert to v4 and the v4 > treewalk interface has to fall back to canonical format.. I guess that > can't happen. Well... There is little gain in converting loose tree objects into pv4 format so there will always be a place for the canolical tree parser. Eventually the base trees added by index-pack should be pv4 trees. The only case where this might not work is for zero padded mode trees and we don't have to optimize for that i.e. a fallback to the canonical tree format will be good enough. > > However I can see that, as you say, the same base object is repeatedly > > referenced. This means that we need to parse it over and over again > > just to find the right offset where the needed entries start. We > > probably end up skipping over the first entries in a tree object > > multiple times. And that would be true even when the core code learns > > to walk pv4 trees directly. > > > > So here's the beginning of a tree offset cache to mitigate this problem. > > It is incomplete as the cache function is not implemented, etc. But > > that should give you the idea. > > Thanks. I'll have a closer look and maybe complete your patch. I've committed an almost final patch and pushed it out. It is disabled right now due to some bug I've not found yet. But you can have a look. Nicolas
Re: [PATCH] pack-objects: no crc check when the cached version is used
On Sat, Sep 14, 2013 at 4:26 AM, Thomas Rast wrote: > Junio C Hamano writes: > >> Nguyễn Thái Ngọc Duy writes: >> >>> Current code makes pack-objects always do check_pack_crc() in >>> unpack_entry() even if right after that we find out there's a cached >>> version and pack access is not needed. Swap two code blocks, search >>> for cached version first, then check crc. > [...] >> >> Interesting. >> >> This is only triggered inside pack-objects, which would read a lot >> of data from existing packs, and the overhead for looking up the >> entry from the revindex, faulting in the actual packdata, and >> computing and comparing the crc would not be trivial, especially as >> the cost is incurred over many objects we need to untangle in the >> delta chain. If you have interesting numbers to show how much this >> improves the performance, I am curious to see it. No I don't have any timing numbers. I just updated the code to see how many times crc is checked and how many times we find a cached version after crc is checked. The numbers with git.git are 353535 and 113257 respectively. IOW we could reduce the number of crc checks by 30%. > > I can't see anything wrong with the patch, but then I haven't stared too > hard. (It seems that my conversion around abe601b (sha1_file: remove > recursion in unpack_entry, 2013-03-27) was faithful on this point, the > problem has existed for longer than that.) > > I tried the perf script below, but at least for the git repo the only > thing I can see is noise. --stdout does not set do_check_packed_object_crc, you need to run pack-objects without --stdout (i.e. the real use case is repack) > > --- 8< --- t/perf/p5300-pack-object.sh --- 8< --- > #!/bin/sh > > test_description="Tests object packing performance" > > . ./perf-lib.sh > > test_perf_default_repo > > test_perf 'pack-objects on commits in HEAD' ' > git rev-list HEAD | > git pack-objects --stdout >/dev/null > ' > > test_perf 'pack-objects on all of HEAD' ' > git rev-list --objects HEAD | > git pack-objects --stdout >/dev/null > ' > > test_done -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Junio C Hamano writes: > You can explicitly include the system header from your compatibility > layer, i.e. > ... > and then in config.mak.uname, do something like this: > ... > COMPAT_CFLAGS += -DSYSTEM_STRING_H_HEADER=$(SYSTEM_STRING_H_HEADER) You need to have one level of quoting to keep "" from being eaten; it should be sufficient to see how SHA1_HEADER that is included in cache.h is handled and imitate it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 3/3] doc: command line interface (cli) dot-repository dwimmery
Philip Oakley writes: > The Git cli will accept dot '.' (period) as the relative path > to the current repository. Explain this action. > > Signed-off-by: Philip Oakley > --- > Documentation/gitcli.txt | 4 > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt > index 7d54b77..b065c0e 100644 > --- a/Documentation/gitcli.txt > +++ b/Documentation/gitcli.txt > @@ -58,6 +58,10 @@ the paths in the index that match the pattern to be > checked out to your > working tree. After running `git add hello.c; rm hello.c`, you will _not_ > see `hello.c` in your working tree with the former, but with the latter > you will. > ++ > +Just as the filesystem '.' (period) refers to the current directory, > +using a '.' as a repository name in Git (a dot-repository) is a relative > +path for your current repository. Looks good to 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 V2 2/3] config doc: update dot-repository notes
Philip Oakley writes: > branch..remote can be set to '.' (period) as the repository > path (URL) as part of the remote name dwimmery. Tell the reader. > > Such relative paths are not 'special'. Correct the branch..merge > note. Looks good. It naturally follows that this is also valid: [branch "master"] merge = refs/heads/master remote = git://git.kernel.org/pub/scm/git/git.git and running "git pull" while on your 'master'. This is because branch..remote usually is spelled with a nickname that refers to the [remote ] section, but it does not have to be; it can use a URL to refer to the remote repository. > > Signed-off-by: Philip Oakley > --- > Documentation/config.txt | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 599ca52..da63043 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -718,6 +718,8 @@ branch..remote:: > overridden by `branch..pushremote`. If no remote is > configured, or if you are not on any branch, it defaults to > `origin` for fetching and `remote.pushdefault` for pushing. > + Additionally, `.` (a period) is the current local repository > + (a dot-repository), see `branch..merge`'s final note below. > > branch..pushremote:: > When on branch , it overrides `branch..remote` for > @@ -743,8 +745,8 @@ branch..merge:: > Specify multiple values to get an octopus merge. > If you wish to setup 'git pull' so that it merges into from > another branch in the local repository, you can point > - branch..merge to the desired branch, and use the special setting > - `.` (a period) for branch..remote. > + branch..merge to the desired branch, and use the relative path > + setting `.` (a period) for branch..remote. > > branch..mergeoptions:: > Sets default options for merging into branch . The syntax and -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/3] Doc URLs: relative paths imply the dot-respository
Philip Oakley writes: > Signed-off-by: Philip Oakley > --- > Documentation/urls.txt | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/urls.txt b/Documentation/urls.txt > index 9ccb246..5350a63 100644 > --- a/Documentation/urls.txt > +++ b/Documentation/urls.txt > @@ -55,6 +55,13 @@ These two syntaxes are mostly equivalent, except the > former implies > --local option. > endif::git-clone[] > > +Relative paths are relative to the `$GIT_DIR`, thus the path: Is it? git init src dst cd src git commit --allow-empty -m initial cd ../dst git fetch ../src HEAD:refs/heads/copy would work, but if it is relative to $GIT_DIR, the last one would need to be written as git fetch ../../src HEAD:refs/heads/copy wouldn't it? > + > +- '.' > + > +is the current repository and acts as if it were a repository > +named `'.'`. > + > When Git doesn't know how to handle a certain transport protocol, it > attempts to use the 'remote-' remote helper, if one > exists. To explicitly request a remote helper, the following syntax -- To unsubscribe from this list: send the line "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] sequencer: trivial cleanup
Consider that the return values of allow_empty() could either be negative, zero, or one. However, there is no reason to be overtly conservative about it: we might as well return positive values as well since the callsite has no problems with it. Signed-off-by: Ramkumar Ramachandra --- sequencer.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/sequencer.c b/sequencer.c index 351548f..ae25b5b 100644 --- a/sequencer.c +++ b/sequencer.c @@ -463,13 +463,7 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit) if (opts->keep_redundant_commits) return 1; - empty_commit = is_original_commit_empty(commit); - if (empty_commit < 0) - return empty_commit; - if (!empty_commit) - return 0; - else - return 1; + return is_original_commit_empty(commit); } static int do_pick_commit(struct commit *commit, struct replay_opts *opts) -- 1.8.4.299.gb3e7d24.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 1/3] name-hash: refactor polymorphic index_name_exists()
On Fri, Sep 13, 2013 at 6:16 PM, Junio C Hamano wrote: > Eric Sunshine writes: > >> Given the above. How should I proceed? Do you still feel that it is >> advisable to keep an index_name_exists() around for compatibility >> reasons in case any new callers are introduced? Regardless of that >> answer, do you want index_name_exists() renamed to >> index_file_exists()? > > Renaming *_name_exists() to *_file_exists() without keeping a > compatibility one will force new topics to be rebased on this > series. Alternatively we could merge them to 'pu' (and later 'next' > and 'master') with evil merges to adjust the change in the semantics > of the called function. That increases the risk of accidental > breakages, I think. > > It is safer to keep index_name_exists() around with the older > semantics, if we can, and rename your "file only" one to a different > name. That way, even if a new topic still uses index_name_exists() > expecting the traditional behaviour, it will not break immediately > and we do not need to risk evil merges making mistakes. > > Later, we can "git grep _name_exists" to spot them and convert such > old-style calls to either "directory only" or "file only" variants > after this series and these follow-on topics hit 'master' (and we do > not know at this point in what order that happens). Thanks. That's what I needed to know. I'll re-roll with the suggested changes. (And, I'm looking into the Mac-only test breakages not related to this patch series.) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Sep 2013, #04; Fri, 13)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The third batch of topics are now in 'master'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * jc/commit-is-spelled-with-two-ems (2013-09-05) 2 commits (merged to 'next' on 2013-09-05 at 982aef2) + typofix: cherry is spelled with two ars + typofix: commit is spelled with two ems * jc/pager-configuration-doc (2013-08-29) 1 commit (merged to 'next' on 2013-09-05 at 3169083) + config: rewrite core.pager documentation It was unclear in the documentation how various configurations and environment variables determine which pager is eventually used. * jk/config-int-range-check (2013-09-09) 5 commits (merged to 'next' on 2013-09-09 at 9ab779d) + git-config: always treat --int as 64-bit internally + config: make numeric parsing errors more clear + config: set errno in numeric git_parse_* functions + config: properly range-check integer values + config: factor out integer parsing from range checks "git config" did not provide a way to set or access numbers larger than a native "int" on the platform; it now provides 64-bit signed integers on all platforms. * mm/fast-import-feature-doc (2013-08-25) 1 commit (merged to 'next' on 2013-09-05 at 83802e2) + Documentation/fast-import: clarify summary for `feature` command * mm/mediawiki-dumb-push-fix (2013-09-03) 4 commits (merged to 'next' on 2013-09-05 at f8313f4) + git-remote-mediawiki: no need to update private ref in non-dumb push + git-remote-mediawiki: use no-private-update capability on dumb push + transport-helper: add no-private-update capability + git-remote-mediawiki: add test and check Makefile targets * mm/remote-helpers-doc (2013-08-26) 1 commit (merged to 'next' on 2013-09-05 at c181b35) + Documentation/remote-helpers: document common use-case for private ref * mn/doc-pack-heu-remove-dead-pastebin (2013-08-23) 1 commit (merged to 'next' on 2013-09-05 at 5caecec) + remove dead pastebin link from pack-heuristics document -- [New Topics] * jc/url-match (2013-09-12) 1 commit (merged to 'next' on 2013-09-13 at 7b94f8e) + urlmatch.c: recompute pointer after append_normalized_escapes While normalizing a URL, we forgot that the buffer that holds it could be relocated when it grows, which was a brown-paper-bag bug that can lead to a crash introduced on 'master' post 1.8.4 release. Will merge to 'master' in the fourth batch. * jx/relative-path-regression-fix (2013-09-13) 3 commits - Use simpler relative_path when set_git_dir - relative_path should honor dos_drive_prefix - test: use unambigous leading path (/foo) for mingw (this branch uses jx/clean-interactive.) * nd/unpack-entry-optim-in-pack-objects (2013-09-13) 1 commit - pack-objects: no crc check when the cached version is used The codepath to use data from packfiles that is only exercised in pack-objects unnecessarily checked crc checksum of the pack data, even when it ends up using in-core copy that it got by reading from the pack (at which point the checksum was validated). Will merge to 'next'. -- [Stalled] * jc/ref-excludes (2013-09-03) 2 commits - document --exclude option - revision: introduce --exclude= to tame wildcards People often wished a way to tell "git log --branches" (and "git log --remotes --not --branches") to exclude some local branches from the expansion of "--branches" (similarly for "--tags", "--all" and "--glob="). Now they have one. Needs a matching change to rev-parse. * rv/send-email-cache-generated-mid (2013-08-21) 2 commits - git-send-email: Cache generated message-ids, use them when prompting - git-send-email: add optional 'choices' parameter to the ask sub * rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits - ### DONTMERGE: needs better explanation on what config they need - pack-refs.c: Add missing call to git_config() - show-ref.c: Add missing call to git_config() The changes themselves are probably good, but it is unclear what basic setting needs to be read for which exact operation. Waiting for clarification. $gmane/228294 * jh/shorten-refname (2013-05-07) 4 commits - t1514: refname shortening is done after dereferencing symbolic refs - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin - t1514: Demonstrate failure to correctly shorten "refs/remotes/origin/HEAD" - t1514: Add tests of shortening refnames in strict/loose mode When remotes/origin/HEAD is not a symbolic ref, "rev-parse --abbrev-ref remotes/origin/HEAD" ought to show "origin", not "origin/HEAD", which is fixed with this series
Re: [PATCH 1/3] name-hash: refactor polymorphic index_name_exists()
Eric Sunshine writes: > Given the above. How should I proceed? Do you still feel that it is > advisable to keep an index_name_exists() around for compatibility > reasons in case any new callers are introduced? Regardless of that > answer, do you want index_name_exists() renamed to > index_file_exists()? Renaming *_name_exists() to *_file_exists() without keeping a compatibility one will force new topics to be rebased on this series. Alternatively we could merge them to 'pu' (and later 'next' and 'master') with evil merges to adjust the change in the semantics of the called function. That increases the risk of accidental breakages, I think. It is safer to keep index_name_exists() around with the older semantics, if we can, and rename your "file only" one to a different name. That way, even if a new topic still uses index_name_exists() expecting the traditional behaviour, it will not break immediately and we do not need to risk evil merges making mistakes. Later, we can "git grep _name_exists" to spot them and convert such old-style calls to either "directory only" or "file only" variants after this series and these follow-on topics hit 'master' (and we do not know at this point in what order that happens). Hmm? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth writes: > On Fri, Sep 13, 2013 at 10:01 PM, Junio C Hamano wrote: > >>> I don't like the idea of introducing a compat/mingw/string.h because >>> of two reasons: You would have to add a conditional to include that >>> string.h instead of the system one anyway, >> >> With -Icompat/mingw passed to the compiler, which is a bog-standard >> technique we already use to supply headers the system forgot to >> supply or override buggy headers the system is shipped with, you do >> not have to change any "#include ". >> >> Am I mistaken? > > Ah, that would work I guess, but you'd still need the include_next. You can explicitly include the system header from your compatibility layer, i.e. === compat/mingw/string.h === #define __NO_INLINE__ #ifdef SYSTEM_STRING_H_HEADER #include SYSTEM_STRING_H_HEADER #else #include_next #endif and then in config.mak.uname, do something like this: ifneq (,$(findstring MINGW,$(uname_S))) ifndef SYSTEM_STRING_H_HEADER SYSTEM_STRING_H_HEADER = "C:\\llvm\include\string.h" endif COMPAT_CFLAGS += -DSYSTEM_STRING_H_HEADER=$(SYSTEM_STRING_H_HEADER) endif People who have the system header file at different paths can further override SYSTEM_STRING_H_HEADER in their config.mak. That would help compilers targetting mingw that do not support "#include_next" without spreading the damage to other people's systems, I think. -- To unsubscribe from this list: send the line "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/3] name-hash: refactor polymorphic index_name_exists()
On Fri, Sep 13, 2013 at 3:41 PM, Junio C Hamano wrote:
> On Fri, Sep 13, 2013 at 12:29 PM, Eric Sunshine
> wrote:
>> On Fri, Sep 13, 2013 at 2:40 PM, Junio C Hamano wrote:
>>> Eric Sunshine writes:
>>>
Since these two modes of operations are disjoint and have no code in
common (one searches index_state.name_hash; the other dir_hash), they
can be represented more naturally as distinct functions: one to search
for a file, and one for a directory.
>>>
>>> Good thinking. Thanks for working on this; I agree with the general
>>> direction this series is going.
>>>
>>> I however wonder if it would be a good idea to rename the other one
>>> to {cache|index}_file_exists(), and even keep the *_name_exists()
>>> variant that switches between the two based on the trailing slash,
>>> to avoid breaking other topics in flight that may have added new
>>> callsites to *_name_exists(). Otherwise the new callsites will feed
>>> a path with a trailing slash to *_name_exists() and get a false
>>> result, no?
>>
>> An assert("no trailing /") was added to index_name_exists() precisely
>> for the purpose of catching cases of code incorrectly calling
>> index_name_exists() to search for a directory. No existing code in
>> 'master' trips the assertion (at least according to the tests),
>> however, the assertion may be annoying for topics in flight which do.
>>
>> Other than plopping these patches atop 'pu' and seeing if anything
>> breaks, what would be the "git way" of detecting in-flight topics
>> which add callers of index_name_exists()? (Excuse my git ignorance.)
>
> I would do a quick
>
> $ git diff master..pu | grep '^+.*_name_exists'
>
> which would be more conservative (it would also show an invocation
> moved from one place to another).
There appear to be no topics in flight which add new
index_name_exists() callers (which is not to say that no new callers
will be introduced before this topic lands in 'master').
I also plopped the patches atop 'pu' and there are no new tests
failures on Linux or Mac OS X, so the series does not seem to break
anything in flight. (There are a few test failures on 'pu' on Mac OS
X, but they are not related to this series.)
Given the above. How should I proceed? Do you still feel that it is
advisable to keep an index_name_exists() around for compatibility
reasons in case any new callers are introduced? Regardless of that
answer, do you want index_name_exists() renamed to
index_file_exists()?
--
To unsubscribe from this list: send the line "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 0/3] Extend dot repository documentation
This is an update to my patch series on 19 May documenting the dot repository notation. The earlier threads start at: $gmane/224870/ [0/2] Extend dot repository documentation $gmane/224868/ [1/2] config doc: add dot-repository note $gmane/224869/ [2/2] doc: command line interface (cli) dot-repository dwimmery The base documentation is now the URLs section (first patch) as suggested by Jonathan, with a clarification in config(1) branch..remote, and finally a note in cli(7). The patches can be squashed together if appropriate. Philip -- Philip Oakley (3): Doc URLs: relative paths imply the dot-respository config doc: update dot-repository notes doc: command line interface (cli) dot-repository dwimmery Documentation/config.txt | 6 -- Documentation/gitcli.txt | 4 Documentation/urls.txt | 7 +++ 3 files changed, 15 insertions(+), 2 deletions(-) -- 1.8.1.msysgit.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 3/3] doc: command line interface (cli) dot-repository dwimmery
The Git cli will accept dot '.' (period) as the relative path to the current repository. Explain this action. Signed-off-by: Philip Oakley --- Documentation/gitcli.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt index 7d54b77..b065c0e 100644 --- a/Documentation/gitcli.txt +++ b/Documentation/gitcli.txt @@ -58,6 +58,10 @@ the paths in the index that match the pattern to be checked out to your working tree. After running `git add hello.c; rm hello.c`, you will _not_ see `hello.c` in your working tree with the former, but with the latter you will. ++ +Just as the filesystem '.' (period) refers to the current directory, +using a '.' as a repository name in Git (a dot-repository) is a relative +path for your current repository. Here are the rules regarding the "flags" that you should follow when you are scripting Git: -- 1.8.1.msysgit.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 1/3] Doc URLs: relative paths imply the dot-respository
Signed-off-by: Philip Oakley --- Documentation/urls.txt | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/urls.txt b/Documentation/urls.txt index 9ccb246..5350a63 100644 --- a/Documentation/urls.txt +++ b/Documentation/urls.txt @@ -55,6 +55,13 @@ These two syntaxes are mostly equivalent, except the former implies --local option. endif::git-clone[] +Relative paths are relative to the `$GIT_DIR`, thus the path: + +- '.' + +is the current repository and acts as if it were a repository +named `'.'`. + When Git doesn't know how to handle a certain transport protocol, it attempts to use the 'remote-' remote helper, if one exists. To explicitly request a remote helper, the following syntax -- 1.8.1.msysgit.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 2/3] config doc: update dot-repository notes
branch..remote can be set to '.' (period) as the repository path (URL) as part of the remote name dwimmery. Tell the reader. Such relative paths are not 'special'. Correct the branch..merge note. Signed-off-by: Philip Oakley --- Documentation/config.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 599ca52..da63043 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -718,6 +718,8 @@ branch..remote:: overridden by `branch..pushremote`. If no remote is configured, or if you are not on any branch, it defaults to `origin` for fetching and `remote.pushdefault` for pushing. + Additionally, `.` (a period) is the current local repository + (a dot-repository), see `branch..merge`'s final note below. branch..pushremote:: When on branch , it overrides `branch..remote` for @@ -743,8 +745,8 @@ branch..merge:: Specify multiple values to get an octopus merge. If you wish to setup 'git pull' so that it merges into from another branch in the local repository, you can point - branch..merge to the desired branch, and use the special setting - `.` (a period) for branch..remote. + branch..merge to the desired branch, and use the relative path + setting `.` (a period) for branch..remote. branch..mergeoptions:: Sets default options for merging into branch . The syntax and -- 1.8.1.msysgit.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pack-objects: no crc check when the cached version is used
Junio C Hamano writes: > Nguyễn Thái Ngọc Duy writes: > >> Current code makes pack-objects always do check_pack_crc() in >> unpack_entry() even if right after that we find out there's a cached >> version and pack access is not needed. Swap two code blocks, search >> for cached version first, then check crc. [...] > > Interesting. > > This is only triggered inside pack-objects, which would read a lot > of data from existing packs, and the overhead for looking up the > entry from the revindex, faulting in the actual packdata, and > computing and comparing the crc would not be trivial, especially as > the cost is incurred over many objects we need to untangle in the > delta chain. If you have interesting numbers to show how much this > improves the performance, I am curious to see it. I can't see anything wrong with the patch, but then I haven't stared too hard. (It seems that my conversion around abe601b (sha1_file: remove recursion in unpack_entry, 2013-03-27) was faithful on this point, the problem has existed for longer than that.) I tried the perf script below, but at least for the git repo the only thing I can see is noise. --- 8< --- t/perf/p5300-pack-object.sh --- 8< --- #!/bin/sh test_description="Tests object packing performance" . ./perf-lib.sh test_perf_default_repo test_perf 'pack-objects on commits in HEAD' ' git rev-list HEAD | git pack-objects --stdout >/dev/null ' test_perf 'pack-objects on all of HEAD' ' git rev-list --objects HEAD | git pack-objects --stdout >/dev/null ' 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: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 10:01 PM, Junio C Hamano wrote: >> I don't like the idea of introducing a compat/mingw/string.h because >> of two reasons: You would have to add a conditional to include that >> string.h instead of the system one anyway, > > With -Icompat/mingw passed to the compiler, which is a bog-standard > technique we already use to supply headers the system forgot to > supply or override buggy headers the system is shipped with, you do > not have to change any "#include ". > > Am I mistaken? Ah, that would work I guess, but you'd still need the include_next. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 9:56 PM, Linus Torvalds wrote: >> +#ifdef __MINGW32__ >> +#ifdef __NO_INLINE__ > > Why do you want to push this insane workaround for a clear Mingw bug? To be frank, because Git is picking up patches much quicker than MinGW does, and I want a solution ASAP. Although I of course agree that fixing the real issue upstream in MinGW is the better solution. > Please have mingw just fix the nasty bug, and the git patch with the I'll try to come up with a MinGW patch in parallel. > trivial wrapper looks much simpler than just saying "don't inline > anything" and that crazy block of nasty mingw magic #defines/. It may look simpler, but as outlines in this thread it's less maintainable because you need to remember to use the wrapper. And people tend to forget that no matter how loudly you document that. If we can make the code more fool proof we IMHO should do so. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth writes: > I don't like the idea of introducing a compat/mingw/string.h because > of two reasons: You would have to add a conditional to include that > string.h instead of the system one anyway, With -Icompat/mingw passed to the compiler, which is a bog-standard technique we already use to supply headers the system forgot to supply or override buggy headers the system is shipped with, you do not have to change any "#include ". Am I mistaken? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 12:53 PM, Sebastian Schuberth wrote: > > +#ifdef __MINGW32__ > +#ifdef __NO_INLINE__ Why do you want to push this insane workaround for a clear Mingw bug? Please have mingw just fix the nasty bug, and the git patch with the trivial wrapper looks much simpler than just saying "don't inline anything" and that crazy block of nasty mingw magic #defines/. And then document loudly that the wrapper is due to the mingw bug. Linus -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 4:37 PM, Junio C Hamano wrote: > Which means people who do want to see that macro defined will be > broken after that section of the header file which unconditionally > undefs it, right? Right, but luckily you've fixed that in our proposed patch :-) > That is certainly better than the unconditional one, but I wonder if > it is an option to add compat/mingw/string.h without doing the > above, though. I don't like the idea of introducing a compat/mingw/string.h because of two reasons: You would have to add a conditional to include that string.h instead of the system one anyway, so we could just as well keep the conditional in git-compat-util.h along with the logic. And I don't like the include_next GCC-ism, especially as I was planning to take a look at compiling Git with LLVM / clang under Windows. So how about this: --- a/git-compat-util.h +++ b/git-compat-util.h @@ -85,6 +85,25 @@ #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 +#ifdef __MINGW32__ +#ifdef __NO_INLINE__ +#define __NO_INLINE_ALREADY_DEFINED +#else +#define __NO_INLINE__ /* do not inline strcasecmp() */ +#endif +#endif +#include +#ifdef __MINGW32__ +#ifdef __NO_INLINE_ALREADY_DEFINED +#undef __NO_INLINE_ALREADY_DEFINED +#else +#undef __NO_INLINE__ +#endif +#endif +#ifdef HAVE_STRINGS_H +#include /* for strcasecmp() */ +#endif + #ifdef WIN32 /* Both MinGW and MSVC */ #define _WIN32_WINNT 0x0502 #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ @@ -99,10 +118,6 @@ #include #include #include -#include -#ifdef HAVE_STRINGS_H -#include /* for strcasecmp() */ -#endif #include #include #ifdef NEEDS_SYS_PARAM_H -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] test: use unambigous leading path (/foo) for mingw
Jiang Xin writes: > In test cases for relative_path, path with one leading character > (such as /a, /x) may be recogonized as "a:/" or "x:/" if there is > such doc drive on MINGW platform. Use an umambigous leading path > "/foo" instead. "DOS drive", you mean? Are they really spelled as /a or /x (not e.g. //a or something)? Just double-checking. > Signed-off-by: Jiang Xin > --- > t/t0060-path-utils.sh | 56 > +-- > 1 file changed, 28 insertions(+), 28 deletions(-) > > diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh > index 3a48de2..82a6f21 100755 > --- a/t/t0060-path-utils.sh > +++ b/t/t0060-path-utils.sh > @@ -190,33 +190,33 @@ test_expect_success SYMLINKS 'real path works on > symlinks' ' > test "$sym" = "$(test-path-utils real_path "$dir2/syml")" > ' > > -relative_path /a/b/c//a/b/ c/ > -relative_path /a/b/c//a/bc/ > -relative_path /a//b//c/ //a/b// c/ POSIX > -relative_path /a/b /a/b./ > -relative_path /a/b/ /a/b./ > -relative_path /a /a/b../ > -relative_path / /a/b/ ../../ > -relative_path /a/c /a/b/ ../c > -relative_path /a/c /a/b../c > -relative_path /x/y /a/b/ ../../x/y > -relative_path /a/b "" /a/b > -relative_path /a/b ""/a/b > -relative_path a/b/c/ a/b/c/ > -relative_path a/b/c/ a/b c/ > -relative_path a/b//c a//bc > -relative_path a/b/ a/b/./ > -relative_path a/b/ a/b ./ > -relative_path a a/b ../ > -relative_path x/ya/b ../../x/y > -relative_path a/ca/b ../c > -relative_path a/b"" a/b > -relative_path a/b""a/b > -relative_path "" /a/b./ > -relative_path "" "" ./ > -relative_path "" ""./ > -relative_path "" "" ./ > -relative_path "" ""./ > -relative_path "" /a/b./ > +relative_path /foo/a/b/c//foo/a/b/ c/ > +relative_path /foo/a/b/c//foo/a/bc/ > +relative_path /foo/a//b//c/ //foo/a/b// c/ POSIX > +relative_path /foo/a/b /foo/a/b./ > +relative_path /foo/a/b/ /foo/a/b./ > +relative_path /foo/a /foo/a/b../ > +relative_path / /foo/a/b/ ../../../ > +relative_path /foo/a/c /foo/a/b/ ../c > +relative_path /foo/a/c /foo/a/b../c > +relative_path /foo/x/y /foo/a/b/ ../../x/y > +relative_path /foo/a/b "" /foo/a/b > +relative_path /foo/a/b ""/foo/a/b > +relative_path foo/a/b/c/ foo/a/b/c/ > +relative_path foo/a/b/c/ foo/a/b c/ > +relative_path foo/a/b//c foo/a//bc > +relative_path foo/a/b/ foo/a/b/./ > +relative_path foo/a/b/ foo/a/b ./ > +relative_path foo/a foo/a/b ../ > +relative_path foo/x/yfoo/a/b ../../x/y > +relative_path foo/a/cfoo/a/b ../c > +relative_path foo/a/b"" foo/a/b > +relative_path foo/a/b""foo/a/b > +relative_path "" /foo/a/b./ > +relative_path "" "" ./ > +relative_path "" ""./ > +relative_path "" "" ./ > +relative_path "" ""./ > +relative_path "" /foo/a/b./ > > 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: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Fri, Sep 13, 2013 at 4:26 PM, Junio C Hamano wrote: >>> Perhaps the real issue is that the header file does not give an >>> equivalent "those who want to take the address of strcasecmp will >>> get the address of _stricmp instead" macro, e.g. >>> >>> #define strcasecmp _stricmp >>> >>> or something? >> >> Now it's you who puzzles me, because the header file *does* have >> exactly the macro that you suggest. > > Then why does your platform have problem with the code that takes > the address of strcasecmp and stores it in the variable? It is not > me, but your platform that is puzzling us. > > There is something else going on, like you do not have that #define > "enabled" under some condition, or something silly like that. Exactly. That define is only enabled if __NO_INLINE__ is defined. Which is what my patch is all about: Define __NO_INLINE__ so that we get "#define strcasecmp _stricmp". -- Sebastian Schuberth -- To unsubscribe from this list: send the line "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/3] name-hash: refactor polymorphic index_name_exists()
On Fri, Sep 13, 2013 at 2:40 PM, Junio C Hamano wrote:
> Eric Sunshine writes:
>
>> Depending upon the absence or presence of a trailing '/' on the incoming
>> pathname, index_name_exists() checks either if a file is present in the
>> index or if a directory is represented within the index. Each caller
>> explicitly chooses the mode of operation by adding or removing a
>> trailing '/' before invoking index_name_exists().
>>
>> Since these two modes of operations are disjoint and have no code in
>> common (one searches index_state.name_hash; the other dir_hash), they
>> can be represented more naturally as distinct functions: one to search
>> for a file, and one for a directory.
>>
>> Splitting index searching into two functions relieves callers of the
>> artificial burden of having to add or remove a slash to select the mode
>> of operation; instead they just call the desired function. A subsequent
>> patch will take advantage of this benefit in order to eliminate the
>> requirement that the incoming pathname for a directory search must have
>> a trailing slash.
>
> Good thinking. Thanks for working on this; I agree with the general
> direction this series is going.
>
> I however wonder if it would be a good idea to rename the other one
> to {cache|index}_file_exists(), and even keep the *_name_exists()
> variant that switches between the two based on the trailing slash,
> to avoid breaking other topics in flight that may have added new
> callsites to *_name_exists(). Otherwise the new callsites will feed
> a path with a trailing slash to *_name_exists() and get a false
> result, no?
An assert("no trailing /") was added to index_name_exists() precisely
for the purpose of catching cases of code incorrectly calling
index_name_exists() to search for a directory. No existing code in
'master' trips the assertion (at least according to the tests),
however, the assertion may be annoying for topics in flight which do.
Other than plopping these patches atop 'pu' and seeing if anything
breaks, what would be the "git way" of detecting in-flight topics
which add callers of index_name_exists()? (Excuse my git ignorance.)
--
To unsubscribe from this list: send the line "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/3] name-hash: refactor polymorphic index_name_exists()
Eric Sunshine writes:
> Depending upon the absence or presence of a trailing '/' on the incoming
> pathname, index_name_exists() checks either if a file is present in the
> index or if a directory is represented within the index. Each caller
> explicitly chooses the mode of operation by adding or removing a
> trailing '/' before invoking index_name_exists().
>
> Since these two modes of operations are disjoint and have no code in
> common (one searches index_state.name_hash; the other dir_hash), they
> can be represented more naturally as distinct functions: one to search
> for a file, and one for a directory.
>
> Splitting index searching into two functions relieves callers of the
> artificial burden of having to add or remove a slash to select the mode
> of operation; instead they just call the desired function. A subsequent
> patch will take advantage of this benefit in order to eliminate the
> requirement that the incoming pathname for a directory search must have
> a trailing slash.
Good thinking. Thanks for working on this; I agree with the general
direction this series is going.
I however wonder if it would be a good idea to rename the other one
to {cache|index}_file_exists(), and even keep the *_name_exists()
variant that switches between the two based on the trailing slash,
to avoid breaking other topics in flight that may have added new
callsites to *_name_exists(). Otherwise the new callsites will feed
a path with a trailing slash to *_name_exists() and get a false
result, no?
> Signed-off-by: Eric Sunshine
> ---
> cache.h | 2 ++
> dir.c| 7 ---
> name-hash.c | 47 ---
> read-cache.c | 2 +-
> 4 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 9ef778a..03ec24c 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -314,6 +314,7 @@ extern void free_name_hash(struct index_state *istate);
> #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL,
> NULL)
> #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st),
> (options))
> #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st),
> (options))
> +#define cache_dir_exists(name, namelen) index_dir_exists(&the_index, (name),
> (namelen))
> #define cache_name_exists(name, namelen, igncase)
> index_name_exists(&the_index, (name), (namelen), (igncase))
> #define cache_name_is_other(name, namelen) index_name_is_other(&the_index,
> (name), (namelen))
> #define resolve_undo_clear() resolve_undo_clear_index(&the_index)
> @@ -463,6 +464,7 @@ extern int write_index(struct index_state *, int newfd);
> extern int discard_index(struct index_state *);
> extern int unmerged_index(const struct index_state *);
> extern int verify_path(const char *path);
> +extern struct cache_entry *index_dir_exists(struct index_state *istate,
> const char *name, int namelen);
> extern struct cache_entry *index_name_exists(struct index_state *istate,
> const char *name, int namelen, int igncase);
> extern int index_name_pos(const struct index_state *, const char *name, int
> namelen);
> #define ADD_CACHE_OK_TO_ADD 1/* Ok to add */
> diff --git a/dir.c b/dir.c
> index b439ff0..0080673 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -860,7 +860,8 @@ static struct dir_entry *dir_entry_new(const char
> *pathname, int len)
>
> static struct dir_entry *dir_add_name(struct dir_struct *dir, const char
> *pathname, int len)
> {
> - if (cache_name_exists(pathname, len, ignore_case))
> + if ((len == 0 || pathname[len - 1] != '/') &&
> + cache_name_exists(pathname, len, ignore_case))
> return NULL;
>
> ALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);
> @@ -885,11 +886,11 @@ enum exist_status {
> /*
> * Do not use the alphabetically sorted index to look up
> * the directory name; instead, use the case insensitive
> - * name hash.
> + * directory hash.
> */
> static enum exist_status directory_exists_in_index_icase(const char
> *dirname, int len)
> {
> - const struct cache_entry *ce = cache_name_exists(dirname, len + 1,
> ignore_case);
> + const struct cache_entry *ce = cache_dir_exists(dirname, len + 1);
> unsigned char endchar;
>
> if (!ce)
> diff --git a/name-hash.c b/name-hash.c
> index 617c86c..5b01554 100644
> --- a/name-hash.c
> +++ b/name-hash.c
> @@ -222,11 +222,35 @@ static int same_name(const struct cache_entry *ce,
> const char *name, int namelen
> return slow_same_name(name, namelen, ce->name, len);
> }
>
> +struct cache_entry *index_dir_exists(struct index_state *istate, const char
> *name, int namelen)
> +{
> + struct cache_entry *ce;
> + struct dir_entry *dir;
> +
> + lazy_init_name_hash(istate);
> + dir = find_dir_entry(istate, name, namelen);
> + if (dir && dir->nr)
> + return dir->ce;
> +
> + /*
> + * It might be a submodule. Unlike plain directories, which are
Re: [PATCH v2 2/2] version-gen: avoid messing the version
Felipe Contreras writes:
> If the version is 'v1.8.4-rc1' that is the version, and there's no need
> to change it to anything else, like 'v1.8.4.rc1'.
>
> If RedHat, or somebody else, needs a specific version, they can use the
> 'version' file, like everybody else.
>
> Signed-off-by: Felipe Contreras
> ---
I already explained to you why this is a bad change.
When we say "we try to avoid regressions", we really mean it.
Before coming up with a change to pay Paul by robbing Peter, we must
make an honest effort to see if there is a way to pay Paul without
robbing anybody.
This change forces existing users who depend on how dashes are
mangled into dots to change their tooling.
We do occasionally make deliberate regressions that force existing
users to change the way they work, but only when there is no other
way, and when the benefit of the change far outweighs the cost of
such an adjustment, and with careful planning to ease the pain of
transition. The updates to "git add" and "git push" planned for 2.0
fall into that category.
There has to be a benefit that far outweighs the inconvenience this
patch imposes on existing users, but I do not see there is any. "If
somebody needs a specific version, they can use the 'version' file"
does not justify it at all; it equally applies to those who want to
use a version name with a dash.
Besides, the patch does not even do what it claims to do; if the
version is "v1.8.4-rc1", what you get out of the updated code is
"1.8.4-rc1", still losing the leading "v".
I'd be more receptive if the patch were like this instead, though.
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index b444c18..c6d78ec 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -3,12 +3,16 @@
GVF=GIT-VERSION-FILE
DEF_VER=v1.8.4.GIT
+READ_RAW_VERSION=
LF='
'
# First see if there is a version file (included in release tarballs),
# then try git-describe, then default.
-if test -f version
+if test -f raw-version && VN=$(cat raw-version)
+then
+ READ_RAW_VERSION=yes
+elif test -f version
then
VN=$(cat version) || VN="$DEF_VER"
elif test -d ${GIT_DIR:-.git} -o -f .git &&
@@ -26,7 +30,10 @@ else
VN="$DEF_VER"
fi
-VN=$(expr "$VN" : v*'\(.*\)')
+if test -z "$READ_RAW_VERSION"
+then
+ VN=$(expr "$VN" : v*'\(.*\)')
+fi
if test -r $GVF
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] pack-objects: no crc check when the cached version is used
Nguyễn Thái Ngọc Duy writes:
> Current code makes pack-objects always do check_pack_crc() in
> unpack_entry() even if right after that we find out there's a cached
> version and pack access is not needed. Swap two code blocks, search
> for cached version first, then check crc.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
Interesting.
This is only triggered inside pack-objects, which would read a lot
of data from existing packs, and the overhead for looking up the
entry from the revindex, faulting in the actual packdata, and
computing and comparing the crc would not be trivial, especially as
the cost is incurred over many objects we need to untangle in the
delta chain. If you have interesting numbers to show how much this
improves the performance, I am curious to see it.
Good spotting ;-)
> sha1_file.c | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 8c2d1ed..4955724 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2126,6 +2126,16 @@ void *unpack_entry(struct packed_git *p, off_t
> obj_offset,
> int i;
> struct delta_base_cache_entry *ent;
>
> + ent = get_delta_base_cache_entry(p, curpos);
> + if (eq_delta_base_cache_entry(ent, p, curpos)) {
> + type = ent->type;
> + data = ent->data;
> + size = ent->size;
> + clear_delta_base_cache_entry(ent);
> + base_from_cache = 1;
> + break;
> + }
> +
> if (do_check_packed_object_crc && p->index_version > 1) {
> struct revindex_entry *revidx = find_pack_revindex(p,
> obj_offset);
> unsigned long len = revidx[1].offset - obj_offset;
> @@ -2140,16 +2150,6 @@ void *unpack_entry(struct packed_git *p, off_t
> obj_offset,
> }
> }
>
> - ent = get_delta_base_cache_entry(p, curpos);
> - if (eq_delta_base_cache_entry(ent, p, curpos)) {
> - type = ent->type;
> - data = ent->data;
> - size = ent->size;
> - clear_delta_base_cache_entry(ent);
> - base_from_cache = 1;
> - break;
> - }
> -
> type = unpack_object_header(p, &w_curs, &curpos, &size);
> if (type != OBJ_OFS_DELTA && type != OBJ_REF_DELTA)
> break;
--
To unsubscribe from this list: send the line "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: Fwd: Fwd: git-daemon access-hook race condition
> > For now I'm trying to do the following: > > access-hook.bash has: > > delayed-notify.bash $@ & > > delayed-notify.bash has: > > sleep 10 > ... > curl ... > > I'm expecting access-hook to spawn new process and return without > waiting for it to finish to let the service to do its job. But when i > do push - it sleeps for 10 seconds anyway. Am i missing something > obvious here? > > Any help is much appreciated! > > Thanks, > Eugene I found a following solution to make it happen while waiting for somebody to be generous enough to take on the --post-service-hook (unfortunately i'm not a C guy): It is using 'at' command. The access-hook script has: echo "delayed-notify.bash $@" | at now while delayed-notify.bash has: sleep 10 curl ... This is not perfect and in certain situations can still fail because the delay is not long enough but this will at least resolve 90% of issues. I hope that might be helpful for someone. Thanks, Eugene -- To unsubscribe from this list: send the line "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: Removing all notes containing a specific string
On Fri, Sep 13, 2013 at 10:12 AM, Johan Herland wrote: > On Fri, Sep 13, 2013 at 8:51 AM, Francis Moreau > wrote: >> Hello, >> >> I'd like to know if that's possible to parse all notes to detect a >> special string and if it's the case, remove the note like "git-notes >> remove" would do. > > Hi, > > There's no built-in command/option to do this, but the following shell > one-liner should do the job: > > git grep -l $mystring refs/notes/commits | cut -d':' -f2 | tr -d '/' > | xargs git notes remove > Looks "magic" to me but does the trick Thanks ! -- Francis -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth writes: > Well, if by "everywhere" in (1) you mean "on all platforms" then > you're right. But my patch does not define __NO_INLINE__ globally, but > only at the time string.h / strings.h is included. Afterwards > __NO_INLINE__ is undefined. In that sense, __NO_INLINE__ is not > defined "everywhere". Which means people who do want to see that macro defined will be broken after that section of the header file which unconditionally undefs it, right? That is exactly why that change should not appear in the platform neutral part of the header file. > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -85,12 +85,16 @@ > #define _NETBSD_SOURCE 1 > #define _SGI_SOURCE 1 > > +#ifdef __MINGW32__ > #define __NO_INLINE__ /* do not inline strcasecmp() */ > +#endif > #include > +#ifdef __MINGW32__ > +#undef __NO_INLINE__ > +#endif That is certainly better than the unconditional one, but I wonder if it is an option to add compat/mingw/string.h without doing the above, though. That header file can do the "no-inline" dance before including the real thing with "#include_next", and nobody else would notice, no? #ifdef __NO_INLINE__ #define __NO_INLINE_WAS_THERE 1 #else #define __NO_INLINE__ #define __NO_INLINE_WAS_THERE 0 #endif #include_next #if !__NO_INLINE_WAS_THERE #undef __NO_INLINE__ #endif or something like that. That of course assumes nobody compiles for _MINGW32_ with a compiler that does not understrand "#include_next" and I do not know if that restriction is a showstopper or not. > #ifdef HAVE_STRINGS_H > #include /* for strcasecmp() */ > #endif > -#undef __NO_INLINE__ > > #ifdef WIN32 /* Both MinGW and MSVC */ > #ifndef _WIN32_WINNT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth writes: > On Thu, Sep 12, 2013 at 10:08 PM, Junio C Hamano wrote: > >>> I'm not too happy with the wording either. As I see it, even on MinGW >>> runtime version 4.0 it's not true that "string.h has _only_ inline >>> definition of strcasecmp"; there's also "#define strncasecmp >>> _strnicmp" which effectively provides a non-inline definition of >>> strncasecmp aka _strnicmp. >> >> I do not get this part. Sure, string.h would have definitions of >> things other than strcasecmp, such as strncasecmp. So what? > > Sorry, I mixed up "strcasecmp" and "strncasecmp". OK. >> Does it "effectively" provide a non-inline definition of strcasecmp? > > Yes, if __NO_INLINE__ is defined string.h provides non-inline > definition of both "strcasecmp" and "strncasecmp" by defining them to > "_stricmp" and "_strnicmp" respectively. > >> Perhaps the real issue is that the header file does not give an >> equivalent "those who want to take the address of strcasecmp will >> get the address of _stricmp instead" macro, e.g. >> >> #define strcasecmp _stricmp >> >> or something? > > Now it's you who puzzles me, because the header file *does* have > exactly the macro that you suggest. Then why does your platform have problem with the code that takes the address of strcasecmp and stores it in the variable? It is not me, but your platform that is puzzling us. There is something else going on, like you do not have that #define "enabled" under some condition, or something silly like that. -- To unsubscribe from this list: send the line "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] relative_path should honor dos_drive_prefix
On 13.09.13 06:55, Jiang Xin wrote: > 2013/9/13 Junio C Hamano : >> For systems that need POSIX escape hatch for Apollo Domain ;-), we >> would need a bit more work. When both path1 and path2 begin with a >> double-dash, we would need to check if they match up to the next >> slash, so that >> >> - //host1/usr/src and //host1/usr/lib share the same root and the >>former can be made to ../src relative to the latter; >> >> - //host1/usr/src and //host2/usr/lib are of separate roots. >> >> or something. >> >> > But how could we know which platform supports network pathnames and > needs such implementation. Similar to the has_dos_drive_prefix: For Windows/Mingw we do like this mingw.h #define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':') And all other platforms defines has_dos_drive_prefix() to be 0 here git-compat-util.h #ifndef has_dos_drive_prefix #define has_dos_drive_prefix(path) 0 #endif mingw.h: #define has_unc_path_prefix(path) ((path)[0] == '/' && (path)[1] == '/') (Or may be) #define has_unc_path_prefix(path) (is_dir_sep((path)[0]) && is_dir_sep((path)[1])) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] pack v4: add v4_size to struct delta_base_cache_entry
On Fri, Sep 13, 2013 at 8:27 PM, Nicolas Pitre wrote: > On Thu, 12 Sep 2013, Nguyễn Thái Ngọc Duy wrote: > >> The intention is to store flat v4 trees in delta base cache to avoid >> repeatedly expanding copy sequences in v4 trees. When the user needs >> to unpack a v4 tree and the tree is found in the cache, the tree will >> be converted back to canonical format. Future tree_desc interface may >> skip canonical format and read v4 trees directly. >> >> For that to work we need to keep track of v4 tree size after all copy >> sequences are expanded, which is the purpose of this new field. > > Hmmm I think this is going in a wrong direction. Good thing you caught me early. I was planning to implement a better version of this on the weekend. And you are not wrong about code maintainability, unpack_entry() so far looks very close to a real mess. > Yet, pavkv4 tree walking shouldn't need a cache since there is nothing > to expand in the end. Entries should be advanced one by one as they are > needed. Granted when converting back to a canonical object we need all > of them, but eventually this shouldn't be the main mode of operation. There's another case where one of the base tree is not v4 (the packer is inefficient, like my index-pack --fix-thin). For trees with leading zeros in entry mode, we can just do a lossy conversion to v4, but I wonder if there is a case where we can't even convert to v4 and the v4 treewalk interface has to fall back to canonical format.. I guess that can't happen. > However I can see that, as you say, the same base object is repeatedly > referenced. This means that we need to parse it over and over again > just to find the right offset where the needed entries start. We > probably end up skipping over the first entries in a tree object > multiple times. And that would be true even when the core code learns > to walk pv4 trees directly. > > So here's the beginning of a tree offset cache to mitigate this problem. > It is incomplete as the cache function is not implemented, etc. But > that should give you the idea. Thanks. I'll have a closer look and maybe complete your patch. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] pack v4: add v4_size to struct delta_base_cache_entry
On Thu, 12 Sep 2013, Nguyễn Thái Ngọc Duy wrote:
> The intention is to store flat v4 trees in delta base cache to avoid
> repeatedly expanding copy sequences in v4 trees. When the user needs
> to unpack a v4 tree and the tree is found in the cache, the tree will
> be converted back to canonical format. Future tree_desc interface may
> skip canonical format and read v4 trees directly.
>
> For that to work we need to keep track of v4 tree size after all copy
> sequences are expanded, which is the purpose of this new field.
Hmmm I think this is going in a wrong direction.
If we want a cache for pv4 objects, it would be best to keep it separate
from the current delta cache in sha1_file.c for code maintainability
reasons. I'd suggest creating another cache in packv4-parse.c instead.
Yet, pavkv4 tree walking shouldn't need a cache since there is nothing
to expand in the end. Entries should be advanced one by one as they are
needed. Granted when converting back to a canonical object we need all
of them, but eventually this shouldn't be the main mode of operation.
However I can see that, as you say, the same base object is repeatedly
referenced. This means that we need to parse it over and over again
just to find the right offset where the needed entries start. We
probably end up skipping over the first entries in a tree object
multiple times. And that would be true even when the core code learns
to walk pv4 trees directly.
So here's the beginning of a tree offset cache to mitigate this problem.
It is incomplete as the cache function is not implemented, etc. But
that should give you the idea.
diff --git a/packv4-parse.c b/packv4-parse.c
index ae3e6a5..39b4f31 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -339,9 +339,9 @@ void *pv4_get_commit(struct packed_git *p, struct
pack_window **w_curs,
return dst;
}
-static int copy_canonical_tree_entries(struct packed_git *p, off_t offset,
- unsigned int start, unsigned int count,
- unsigned char **dstp, unsigned long
*sizep)
+static off_t copy_canonical_tree_entries(struct packed_git *p, off_t offset,
+ unsigned int start, unsigned int count,
+ unsigned char **dstp, unsigned long *sizep)
{
void *data;
const unsigned char *from, *end;
@@ -369,13 +369,19 @@ static int copy_canonical_tree_entries(struct packed_git
*p, off_t offset,
if (end - from > *sizep) {
free(data);
- return -1;
+ return 0;
}
memcpy(*dstp, from, end - from);
*dstp += end - from;
*sizep -= end - from;
free(data);
- return 0;
+
+
+ /*
+* We won't cache this tree's current offset so let's just
+* indicate success with a non-zero return value.
+*/
+ return 1;
}
static int tree_entry_prefix(unsigned char *buf, unsigned long size,
@@ -404,39 +410,56 @@ static int tree_entry_prefix(unsigned char *buf, unsigned
long size,
return len;
}
-static int decode_entries(struct packed_git *p, struct pack_window **w_curs,
- off_t offset, unsigned int start, unsigned int count,
- unsigned char **dstp, unsigned long *sizep, int hdr)
+static off_t decode_entries(struct packed_git *p, struct pack_window **w_curs,
+ off_t offset, unsigned int start, unsigned int count,
+ unsigned char **dstp, unsigned long *sizep, int hdr)
{
- unsigned long avail;
- unsigned int nb_entries;
+ unsigned long avail = 0;
const unsigned char *src, *scp;
off_t copy_objoffset = 0;
- src = use_pack(p, w_curs, offset, &avail);
- scp = src;
-
if (hdr) {
+ unsigned int nb_entries;
+
+ src = use_pack(p, w_curs, offset, &avail);
+ scp = src;
+
/* we need to skip over the object header */
while (*scp & 128)
if (++scp - src >= avail - 20)
- return -1;
+ return 0;
+
/* is this a canonical tree object? */
if ((*scp & 0xf) == OBJ_TREE)
return copy_canonical_tree_entries(p, offset,
start, count,
dstp, sizep);
+
/* let's still make sure this is actually a pv4 tree */
if ((*scp++ & 0xf) != OBJ_PV4_TREE)
- return -1;
- }
+ return 0;
+
+ nb_entries = decode_varint(&scp);
+ if (scp == src || start > nb_entries ||
+ count > nb_entries - start)
+ return 0;
+
+ /*
+* If thi
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Thu, Sep 12, 2013 at 10:29 PM, Junio C Hamano wrote: > Jeff King writes: > >> I think there are basically three classes of solution: >> >> 1. Declare __NO_INLINE__ everywhere. I'd worry this might affect other >> environments, who would then not inline and lose performance (but >> since it's a non-standard macro, we don't really know what it will >> do in other places; possibly nothing). >> >> 2. Declare __NO_INLINE__ on mingw. Similar to above, but we know it >> only affects mingw, and we know the meaning of NO_INLINE there. >> >> 3. Try to impact only the uses as a function pointer (e.g., by using >> a wrapper function as suggested in the thread). >> >> Your patch does (1), I believe. Junio's patch does (3), but is a >> maintenance burden in that any new callsites will need to remember to do >> the same trick. Well, if by "everywhere" in (1) you mean "on all platforms" then you're right. But my patch does not define __NO_INLINE__ globally, but only at the time string.h / strings.h is included. Afterwards __NO_INLINE__ is undefined. In that sense, __NO_INLINE__ is not defined "everywhere". > Agreed. If that #define __NO_INLINE__ does not appear in the common > part of our header files like git-compat-util.h but is limited to > somewhere in compat/, that would be the perfect outcome. It's not that easy to move the definition of __NO_INLINE__ into compat/ because git-compat-util.h includes string.h / strings.h before anything of compat/. More over, defining __NO_INLINE__ in somewhere in compat/ would not limit its definition to the string.h / strings.h headers only. So how about something like this on top of my original patch: --- a/git-compat-util.h +++ b/git-compat-util.h @@ -85,12 +85,16 @@ #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 +#ifdef __MINGW32__ #define __NO_INLINE__ /* do not inline strcasecmp() */ +#endif #include +#ifdef __MINGW32__ +#undef __NO_INLINE__ +#endif #ifdef HAVE_STRINGS_H #include /* for strcasecmp() */ #endif -#undef __NO_INLINE__ #ifdef WIN32 /* Both MinGW and MSVC */ #ifndef _WIN32_WINNT -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Thu, Sep 12, 2013 at 10:08 PM, Junio C Hamano wrote: >> I'm not too happy with the wording either. As I see it, even on MinGW >> runtime version 4.0 it's not true that "string.h has _only_ inline >> definition of strcasecmp"; there's also "#define strncasecmp >> _strnicmp" which effectively provides a non-inline definition of >> strncasecmp aka _strnicmp. > > I do not get this part. Sure, string.h would have definitions of > things other than strcasecmp, such as strncasecmp. So what? Sorry, I mixed up "strcasecmp" and "strncasecmp". > Does it "effectively" provide a non-inline definition of strcasecmp? Yes, if __NO_INLINE__ is defined string.h provides non-inline definition of both "strcasecmp" and "strncasecmp" by defining them to "_stricmp" and "_strnicmp" respectively. > Perhaps the real issue is that the header file does not give an > equivalent "those who want to take the address of strcasecmp will > get the address of _stricmp instead" macro, e.g. > > #define strcasecmp _stricmp > > or something? Now it's you who puzzles me, because the header file *does* have exactly the macro that you suggest. Anyway, I think Peff's reply to my other mail summed it up nicely. I will come up with another patch. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "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-Transmission of blobs?
On Fri, Sep 13, 2013 at 3:06 AM, Pyeron, Jason J CTR (US) wrote: > But, again if the connection drops, we have already lost the delta advantage. > I would think the scenario would go like this: > > git clone url://blah/blah > [fail] > cd blah > git clone --resume #uses normal methods > [fail] > while ! git clone --resume --HitItWithAStick > > replace clone with fetch for that use case too Sorry if I missed something in this thread. But I think we could stablize the transferred pack so that --resume works. The sender constructs exactly the same pack as in the first "git clone" then it starts sending from the offset given by the client. For that to work, the first "git clone" must also be "git clone --resume". I started working on that but now my focus is pack v4, so that has to wait. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] version-gen: cleanup
On Fri, Sep 13, 2013 at 1:10 AM, Junio C Hamano wrote:
> Felipe Contreras writes:
>
>> +describe () {
>> + VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) ||
>> return 1
>> case "$VN" in
>> + *$LF*)
>> + return 1
>> + ;;
>> v[0-9]*)
>> git update-index -q --refresh
>> test -z "$(git diff-index --name-only HEAD --)" ||
>> + VN="$VN-dirty"
>> + return 0
>> + ;;
>> esac
>> +}
>> +
>> +# First see if there is a version file (included in release tarballs),
>> +# then try 'git describe', then default.
>> +if test -f version
>> +then
>> + VN=$(cat version) || VN="$DEF_VER"
>> +elif test -d ${GIT_DIR:-.git} -o -f .git && describe
>> then
>
> Makes sense; using a helper function makes the primary logic easier
> to read.
>
>> -test "$VN" = "$VC" || {
>> - echo >&2 "GIT_VERSION = $VN"
>> - echo "GIT_VERSION = $VN" >$GVF
>> -}
>> -
>> -
>> +test "$VN" = "$VC" && exit
>> +echo >&2 "GIT_VERSION = $VN"
>> +echo "GIT_VERSION = $VN" >$GVF
>
> The point of this part is "if the version file is already up to
> date, do not rewrite it only to smudge the mtime to confuse make",
> so it would be much easier to read to write it like so:
>
> if test "$VN" != "$VC"
> then
> ... two echoes ...
> fi
>
> compared to "exiting in the middle" which is harder to follow,
> especially if we consider that we may have to grow the remaining two
> lines in the future.
No, it's much easier to follow, the code clearly says "if the version
is up to date, there's nothing else to do".
--
Felipe Contreras
--
To unsubscribe from this list: send the line "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-Transmission of blobs?
> -Original Message- > From: Josef Wolf > Sent: Friday, September 13, 2013 6:23 > > On Thu, Sep 12, 2013 at 08:06:35PM +, Pyeron, Jason J CTR > (US) wrote: > > > Yes, but it is those awfully slow connections (slower that > the looping > > issue) which happen to always drop while cloning from our > office. And > > the round trip should be mitigated by http-keep-alives. > [ ... ] > > But, again if the connection drops, we have already lost the delta > > advantage. I would think the scenario would go like this: > > > > git clone url://blah/blah > > [fail] > > cd blah > > git clone --resume #uses normal methods I am using the mythical --resume, where it would fetch packs and indexes. > > [fail] > > while ! git clone --resume --HitItWithAStick > > > > replace clone with fetch for that use case too > > Last time I checked, cloning could not be resumed: > > http://git.661346.n2.nabble.com/git-clone-and-unreliable-links-td7570652.html > > If you're on a slow/unreliable link, you've lost. That is kind of the point. It should be possible. -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- - - - Jason Pyeron PD Inc. http://www.pdinc.us - - Principal Consultant 10 West 24th Street #100- - +1 (443) 269-1555 x333Baltimore, Maryland 21218 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- This message is copyright PD Inc, subject to license 20080407P00. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] dir: revert work-around for retired dangerous behavior
directory_exists_in_index_icase() dangerously assumed that it could
access one character beyond the end of its directory argument, and that
that character would unconditionally be '/'. 2eac2a4c (ls-files -k: a
directory only can be killed if the index has a non-directory,
2013-08-15) added a caller which did not respect this undocumented
assumption, and 680be044 (dir.c::test_one_path(): work around
directory_exists_in_index_icase() breakage, 2013-08-23) added a
work-around which temporarily appends a '/' before invoking
directory_exists_in_index_icase().
Since the dangerous behavior of directory_exists_in_index_icase() has
been eliminated, the work-around is now redundant, so retire it (but not
the tests added by the same commit).
Signed-off-by: Eric Sunshine
---
dir.c | 18 +++---
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/dir.c b/dir.c
index 69d04a0..ef95160 100644
--- a/dir.c
+++ b/dir.c
@@ -1161,21 +1161,9 @@ static enum path_treatment treat_one_path(struct
dir_struct *dir,
*/
if ((dir->flags & DIR_COLLECT_KILLED_ONLY) &&
(dtype == DT_DIR) &&
- !has_path_in_index) {
- /*
-* NEEDSWORK: directory_exists_in_index_icase()
-* assumes that one byte past the given path is
-* readable and has '/', which needs to be fixed, but
-* until then, work it around in the caller.
-*/
- strbuf_addch(path, '/');
- if (directory_exists_in_index(path->buf, path->len - 1) ==
- index_nonexistent) {
- strbuf_setlen(path, path->len - 1);
- return path_none;
- }
- strbuf_setlen(path, path->len - 1);
- }
+ !has_path_in_index &&
+ (directory_exists_in_index(path->buf, path->len) ==
index_nonexistent))
+ return path_none;
exclude = is_excluded(dir, path->buf, &dtype);
--
1.8.4.457.g424cb08
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] name-hash: refactor polymorphic index_name_exists()
Depending upon the absence or presence of a trailing '/' on the incoming
pathname, index_name_exists() checks either if a file is present in the
index or if a directory is represented within the index. Each caller
explicitly chooses the mode of operation by adding or removing a
trailing '/' before invoking index_name_exists().
Since these two modes of operations are disjoint and have no code in
common (one searches index_state.name_hash; the other dir_hash), they
can be represented more naturally as distinct functions: one to search
for a file, and one for a directory.
Splitting index searching into two functions relieves callers of the
artificial burden of having to add or remove a slash to select the mode
of operation; instead they just call the desired function. A subsequent
patch will take advantage of this benefit in order to eliminate the
requirement that the incoming pathname for a directory search must have
a trailing slash.
Signed-off-by: Eric Sunshine
---
cache.h | 2 ++
dir.c| 7 ---
name-hash.c | 47 ---
read-cache.c | 2 +-
4 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/cache.h b/cache.h
index 9ef778a..03ec24c 100644
--- a/cache.h
+++ b/cache.h
@@ -314,6 +314,7 @@ extern void free_name_hash(struct index_state *istate);
#define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL,
NULL)
#define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st),
(options))
#define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st),
(options))
+#define cache_dir_exists(name, namelen) index_dir_exists(&the_index, (name),
(namelen))
#define cache_name_exists(name, namelen, igncase)
index_name_exists(&the_index, (name), (namelen), (igncase))
#define cache_name_is_other(name, namelen) index_name_is_other(&the_index,
(name), (namelen))
#define resolve_undo_clear() resolve_undo_clear_index(&the_index)
@@ -463,6 +464,7 @@ extern int write_index(struct index_state *, int newfd);
extern int discard_index(struct index_state *);
extern int unmerged_index(const struct index_state *);
extern int verify_path(const char *path);
+extern struct cache_entry *index_dir_exists(struct index_state *istate, const
char *name, int namelen);
extern struct cache_entry *index_name_exists(struct index_state *istate, const
char *name, int namelen, int igncase);
extern int index_name_pos(const struct index_state *, const char *name, int
namelen);
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
diff --git a/dir.c b/dir.c
index b439ff0..0080673 100644
--- a/dir.c
+++ b/dir.c
@@ -860,7 +860,8 @@ static struct dir_entry *dir_entry_new(const char
*pathname, int len)
static struct dir_entry *dir_add_name(struct dir_struct *dir, const char
*pathname, int len)
{
- if (cache_name_exists(pathname, len, ignore_case))
+ if ((len == 0 || pathname[len - 1] != '/') &&
+ cache_name_exists(pathname, len, ignore_case))
return NULL;
ALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);
@@ -885,11 +886,11 @@ enum exist_status {
/*
* Do not use the alphabetically sorted index to look up
* the directory name; instead, use the case insensitive
- * name hash.
+ * directory hash.
*/
static enum exist_status directory_exists_in_index_icase(const char *dirname,
int len)
{
- const struct cache_entry *ce = cache_name_exists(dirname, len + 1,
ignore_case);
+ const struct cache_entry *ce = cache_dir_exists(dirname, len + 1);
unsigned char endchar;
if (!ce)
diff --git a/name-hash.c b/name-hash.c
index 617c86c..5b01554 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -222,11 +222,35 @@ static int same_name(const struct cache_entry *ce, const
char *name, int namelen
return slow_same_name(name, namelen, ce->name, len);
}
+struct cache_entry *index_dir_exists(struct index_state *istate, const char
*name, int namelen)
+{
+ struct cache_entry *ce;
+ struct dir_entry *dir;
+
+ lazy_init_name_hash(istate);
+ dir = find_dir_entry(istate, name, namelen);
+ if (dir && dir->nr)
+ return dir->ce;
+
+ /*
+* It might be a submodule. Unlike plain directories, which are stored
+* in the dir-hash, submodules are stored in the name-hash, so check
+* there, as well.
+*/
+ ce = index_name_exists(istate, name, namelen - 1, 1);
+ if (ce && S_ISGITLINK(ce->ce_mode))
+ return ce;
+
+ return NULL;
+}
+
struct cache_entry *index_name_exists(struct index_state *istate, const char
*name, int namelen, int icase)
{
unsigned int hash = hash_name(name, namelen);
struct cache_entry *ce;
+ assert(namelen == 0 || name[namelen - 1] != '/');
+
lazy_init_name_hash(istate);
ce = lookup_hash(hash, &istate->name_hash);
@@ -237,29 +261,6 @@ struct cache_entry *index_name_exists(struct index_state
*istat
[PATCH 2/3] name-hash: stop storing trailing '/' on paths in index_state.dir_hash
When 5102c617 (Add case insensitivity support for directories when using
git status, 2010-10-03) added directories to the name-hash there was
only a single hash table in which both real cache entries and leading
directory prefixes were registered. To distinguish between the two types
of entries, directories were stored with a trailing '/'.
2092678c (name-hash.c: fix endless loop with core.ignorecase=true,
2013-02-28), however, moved directories to a separate hash table
(index_state.dir_hash) but retained the now-redundant trailing '/', thus
callers still bear the burden of ensuring its presence before searching
via index_dir_exists(). Eliminate this redundancy by storing paths in
the dir-hash without the trailing '/'.
An important benefit of this change is that it eliminates undocumented
and dangerous behavior of dir.c:directory_exists_in_index_icase() in
which it assumes not only that it can validly access one character
beyond the end of its incoming directory argument, but also that that
character will unconditionally be a '/'. This perilous behavior was
"tolerated" because the string passed in by its lone caller always had a
'/' in that position, however, things broke [1] when 2eac2a4c (ls-files
-k: a directory only can be killed if the index has a non-directory,
2013-08-15) added a new caller which failed to respect the undocumented
assumption.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/232727
Signed-off-by: Eric Sunshine
---
dir.c| 2 +-
name-hash.c | 9 +
read-cache.c | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index 0080673..69d04a0 100644
--- a/dir.c
+++ b/dir.c
@@ -890,7 +890,7 @@ enum exist_status {
*/
static enum exist_status directory_exists_in_index_icase(const char *dirname,
int len)
{
- const struct cache_entry *ce = cache_dir_exists(dirname, len + 1);
+ const struct cache_entry *ce = cache_dir_exists(dirname, len);
unsigned char endchar;
if (!ce)
diff --git a/name-hash.c b/name-hash.c
index 5b01554..2bae75d 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -58,9 +58,9 @@ static struct dir_entry *hash_dir_entry(struct index_state
*istate,
{
/*
* Throw each directory component in the hash for quick lookup
-* during a git status. Directory components are stored with their
+* during a git status. Directory components are stored without their
* closing slash. Despite submodules being a directory, they never
-* reach this point, because they are stored without a closing slash
+* reach this point, because they are stored
* in index_state.name_hash (as ordinary cache_entries).
*
* Note that the cache_entry stored with the dir_entry merely
@@ -78,6 +78,7 @@ static struct dir_entry *hash_dir_entry(struct index_state
*istate,
namelen--;
if (namelen <= 0)
return NULL;
+ namelen--;
/* lookup existing entry for that directory */
dir = find_dir_entry(istate, ce->name, namelen);
@@ -97,7 +98,7 @@ static struct dir_entry *hash_dir_entry(struct index_state
*istate,
}
/* recursively add missing parent directories */
- dir->parent = hash_dir_entry(istate, ce, namelen - 1);
+ dir->parent = hash_dir_entry(istate, ce, namelen);
}
return dir;
}
@@ -237,7 +238,7 @@ struct cache_entry *index_dir_exists(struct index_state
*istate, const char *nam
* in the dir-hash, submodules are stored in the name-hash, so check
* there, as well.
*/
- ce = index_name_exists(istate, name, namelen - 1, 1);
+ ce = index_name_exists(istate, name, namelen, 1);
if (ce && S_ISGITLINK(ce->ce_mode))
return ce;
diff --git a/read-cache.c b/read-cache.c
index a59644a..8990b61 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -643,7 +643,7 @@ int add_to_index(struct index_state *istate, const char
*path, struct stat *st,
if (*ptr == '/') {
struct cache_entry *foundce;
++ptr;
- foundce = index_dir_exists(istate, ce->name,
ptr - ce->name);
+ foundce = index_dir_exists(istate, ce->name,
ptr - ce->name - 1);
if (foundce) {
memcpy((void *)startPtr, foundce->name
+ (startPtr - ce->name), ptr - startPtr);
startPtr = ptr;
--
1.8.4.457.g424cb08
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] stop storing trailing slash in dir-hash
This series alters name-hash so that it no longer stores the (redundant) trailing slash with index_state.dir_hash entries. As an intentional side-effect, the series fixes [1] in a cleaner way (suggested by Junio [2]) than either [3] (680be044 in master) or [4]. As noted by Peff [5], this change is at a fairly fundamental level, so care has been taken to ensure that all tests still pass (thus it at least does not break anything covered by the tests). [1]: http://thread.gmane.org/gmane.comp.version-control.git/232727 [2]: http://thread.gmane.org/gmane.comp.version-control.git/232727/focus=232813 [3]: http://thread.gmane.org/gmane.comp.version-control.git/232796 [4]: http://thread.gmane.org/gmane.comp.version-control.git/232833 [5]: http://thread.gmane.org/gmane.comp.version-control.git/232727/focus=232822 Eric Sunshine (3): name-hash: refactor polymorphic index_name_exists() name-hash: stop storing trailing '/' on paths in index_state.dir_hash dir: revert work-around for retired dangerous behavior cache.h | 2 ++ dir.c| 25 +++-- name-hash.c | 54 -- read-cache.c | 2 +- 4 files changed, 38 insertions(+), 45 deletions(-) -- 1.8.4.457.g424cb08 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] version-gen: cleanup
Felipe Contreras writes:
> +describe () {
> + VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) ||
> return 1
> case "$VN" in
> + *$LF*)
> + return 1
> + ;;
> v[0-9]*)
> git update-index -q --refresh
> test -z "$(git diff-index --name-only HEAD --)" ||
> + VN="$VN-dirty"
> + return 0
> + ;;
> esac
> +}
> +
> +# First see if there is a version file (included in release tarballs),
> +# then try 'git describe', then default.
> +if test -f version
> +then
> + VN=$(cat version) || VN="$DEF_VER"
> +elif test -d ${GIT_DIR:-.git} -o -f .git && describe
> then
Makes sense; using a helper function makes the primary logic easier
to read.
> -test "$VN" = "$VC" || {
> - echo >&2 "GIT_VERSION = $VN"
> - echo "GIT_VERSION = $VN" >$GVF
> -}
> -
> -
> +test "$VN" = "$VC" && exit
> +echo >&2 "GIT_VERSION = $VN"
> +echo "GIT_VERSION = $VN" >$GVF
The point of this part is "if the version file is already up to
date, do not rewrite it only to smudge the mtime to confuse make",
so it would be much easier to read to write it like so:
if test "$VN" != "$VC"
then
... two echoes ...
fi
compared to "exiting in the middle" which is harder to follow,
especially if we consider that we may have to grow the remaining two
lines in the future.
--
To unsubscribe from this list: send the line "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] pack-objects: no crc check when the cached version is used
Current code makes pack-objects always do check_pack_crc() in
unpack_entry() even if right after that we find out there's a cached
version and pack access is not needed. Swap two code blocks, search
for cached version first, then check crc.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
sha1_file.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 8c2d1ed..4955724 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2126,6 +2126,16 @@ void *unpack_entry(struct packed_git *p, off_t
obj_offset,
int i;
struct delta_base_cache_entry *ent;
+ ent = get_delta_base_cache_entry(p, curpos);
+ if (eq_delta_base_cache_entry(ent, p, curpos)) {
+ type = ent->type;
+ data = ent->data;
+ size = ent->size;
+ clear_delta_base_cache_entry(ent);
+ base_from_cache = 1;
+ break;
+ }
+
if (do_check_packed_object_crc && p->index_version > 1) {
struct revindex_entry *revidx = find_pack_revindex(p,
obj_offset);
unsigned long len = revidx[1].offset - obj_offset;
@@ -2140,16 +2150,6 @@ void *unpack_entry(struct packed_git *p, off_t
obj_offset,
}
}
- ent = get_delta_base_cache_entry(p, curpos);
- if (eq_delta_base_cache_entry(ent, p, curpos)) {
- type = ent->type;
- data = ent->data;
- size = ent->size;
- clear_delta_base_cache_entry(ent);
- base_from_cache = 1;
- break;
- }
-
type = unpack_object_header(p, &w_curs, &curpos, &size);
if (type != OBJ_OFS_DELTA && type != OBJ_REF_DELTA)
break;
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "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-Transmission of blobs?
On Thu, Sep 12, 2013 at 08:06:35PM +, Pyeron, Jason J CTR (US) wrote: > Yes, but it is those awfully slow connections (slower that the looping > issue) which happen to always drop while cloning from our office. And the > round trip should be mitigated by http-keep-alives. [ ... ] > But, again if the connection drops, we have already lost the delta > advantage. I would think the scenario would go like this: > > git clone url://blah/blah > [fail] > cd blah > git clone --resume #uses normal methods > [fail] > while ! git clone --resume --HitItWithAStick > > replace clone with fetch for that use case too Last time I checked, cloning could not be resumed: http://git.661346.n2.nabble.com/git-clone-and-unreliable-links-td7570652.html If you're on a slow/unreliable link, you've lost. :-( :-( :-( -- Josef Wolf j...@raven.inka.de -- To unsubscribe from this list: send the line "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-Transmission of blobs?
On Thu, Sep 12, 2013 at 03:44:53PM -0400, Jeff King wrote: > On Thu, Sep 12, 2013 at 12:35:32PM +0200, Josef Wolf wrote: > > > I'm not sure I understand correctly. I see that bitmaps can be used to > > implement set operations. But how comes that walking the graph requires a > > lot > > of CPU? Isn't it O(n)? > > Yes and no. Your "n" there is the entirety of history. Is this really true? > (and each one needs to be pulled off of the disk, > decompressed, and reconstructed from deltas). While you need to unpack commits/trees to traverse further down, I can't see any reason to unpack/reconstruct blobs just to see whether you need to send it. The SHA is all you need to know, isn't it? > Secondly, the graph traversal ends up seeing the same sha1s over and > over again in tree entries (because most entries in the tree don't > change from commit to commit). Whenever you see an object (whether commit or tree) that you already have seen, you can stop traversing further down this part of the graph/tree, as everything you will see on this part has already be seen before. Why would you see the same commits/trees over and over again? You'd stop traversing on the boundary of the already-seen-territory, leaving the vast majority of the "duplicated" structure under the carpet. Somehow I fail to see the problem here. -- Josef Wolf j...@raven.inka.de -- To unsubscribe from this list: send the line "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: Converting repo from HG, `git filter-branch --prune-empty -- --all` is extremely slow and errors out.
On Thu, Sep 12, 2013 at 7:47 PM, Felipe Contreras wrote: > Indeed, I remember writing my own simplified version of 'git > filter-branch' that was much faster. If I recall correctly, the trick > was avoiding 'git write-tree' which can be done if you are not using > any tree filter, but 'git filter-branch' is not that smart. > > If all you want to do is prune empty commits, it should be easy to > write a script that simply does 'git commit-tree'. I might decide to > do that based on my script if I have time today. Here it is, it's straightforward and should be easy to understand. -- Felipe Contreras filter-branch Description: Binary data
Your registration code (E3O6M6Y)
Your registration code (E3O6M6Y) This is to update you regarding your CARD ATM of US$1.8Million Contact the Atm Director with your delivery address for more information via their details below. MD. Ms. Kieffer Robert Email:(dhlcom_p...@yahoo.fr) Tel:+229 98298252 Mr. Ahamad Yaki -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] pathspec: strip multiple trailing slashes from submodules
On Fri, Sep 13, 2013 at 08:28:24AM +0700, Duy Nguyen wrote: > On Fri, Sep 13, 2013 at 3:21 AM, John Keeping wrote: > > On Thu, Sep 12, 2013 at 12:48:10PM -0700, Junio C Hamano wrote: > >> John Keeping writes: > >> > >> > This allows us to replace the submodule path trailing slash removal in > >> > builtin/rm.c with the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to > >> > parse_pathspec() without changing the behaviour with respect to multiple > >> > trailing slashes. > >> > >> Where does prefix_pathspec()'s input, which could have an unwanted > >> trailing slash, come from? > >> > >> If it is read from some of our internal data structure and known to > >> have at most one, then this change makes me feel very uneasy to cope > >> with potentially sloppy end-user input and data generated by ourselves > >> with the same logic. It will allow our internal to be sloppy without > >> forcing us notice and fix that sloppiness. > >> > >> If it is coming from an end-user input, then I would not object to > >> the change, though. > > > > I added this in response to Duy's comment on v1 [1]. > > > > [1] http://article.gmane.org/gmane.comp.version-control.git/234548 > > Looks like I add more noise to this thread than anything valuable. > Yes, once argv goes through parse_pathspec it's normalized so we do > not need to strip consecutive slashes any more. I'm not entirely sure > if it also converts Windows '\\' to '/'.. If it goes through normalize_path_copy_len then it does. Junio, can you please drop the first two patches in this series? I can resend the final two unmodified if necessary, but I suspect it's easier for you to just drop the commits. > > Looking more closely, this does come from user input (via the argv > > passed into parse_pathspec) but does (some of the time) go through > > prefix_path_gently which calls normalize_path_copy_len. > > > > It's not immediately clear to me when prefix_pathspec goes through this > > particular code path, but I think we may be able to drop this (and the > > previous patch) without affecting the user. -- To unsubscribe from this list: send the line "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: Removing all notes containing a specific string
On Fri, Sep 13, 2013 at 8:51 AM, Francis Moreau wrote: > Hello, > > I'd like to know if that's possible to parse all notes to detect a > special string and if it's the case, remove the note like "git-notes > remove" would do. Hi, There's no built-in command/option to do this, but the following shell one-liner should do the job: git grep -l $mystring refs/notes/commits | cut -d':' -f2 | tr -d '/' | xargs git notes remove ...Johan -- Johan Herland, www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
