* Re: [PATCH v1 11/45] parse_pathspec: support stripping submodule trailing slashes
From: Junio C Hamano @ 2013-03-17 21:55 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1363327620-29017-12-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> This flag is equivalent to builtin/ls-files.c:strip_trailing_slashes()
> and is intended to replace that function when ls-files is converted to
> use parse_pathspec.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> pathspec.c | 9 +++++++++
> pathspec.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/pathspec.c b/pathspec.c
> index b2446c3..2da8bc9 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -204,6 +204,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
> *raw = item->match = match;
> item->original = elt;
> item->len = strlen(item->match);
> +
> + if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
I see that having cheap and expensive variant at these steps is a
way to achieve a bug-for-bug compatible rewrite of the existing
code, but in the longer term should we lose one of them? After all,
a path that points at the working tree of another repository that is
beyond a symlink cannot be added to us as a submodule, so CHEAP can
be used only when we know that any intermediate component on the
path is not a symlink pointing elsewhere, and the user in the
codepath of ls-files may know it.
To put it differently, shouldn't CHEAP and EXPENSIVE be accompanied
by in-code comment and updates to Documentation/technical/api-*.txt
to help users of the API to decide which one to use?
^ permalink raw reply
* Re: [PATCH v1 14/45] Guard against new pathspec magic in pathspec matching code
From: Junio C Hamano @ 2013-03-17 22:00 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1363327620-29017-15-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> GUARD_PATHSPEC() marks pathspec-sensitive code (basically anything in
> 'struct pathspec' except fields "nr" and "original"). GUARD_PATHSPEC()
> is not supposed to fail. The steps for a new pathspec magic or
> optimization would be:
>
> - update parse_pathspec, add extra information to struct pathspec
>
> - grep GUARD_PATHSPEC() and update all relevant code (or note those
> that won't work with your new stuff). Update GUARD_PATHSPEC mask
> accordingly.
>
> - update parse_pathspec calls to disable new magic early at command
> parsing level. Make sure parse_pathspec() catches unsupported
> syntax, not until GUARD_PATHSPEC catches it.
>
> - add tests to verify supported/unsupported commands both work as
> expected.
I think Documentation/technical/api-*.txt wants to have all of the
above. It is not clear from the description what the second bitmask
is supposed to mean, without reading the implementation; I am guessing
that you are supposed to list the kinds of magic that are supported
in the codepath?
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> builtin/diff.c | 2 ++
> dir.c | 2 ++
> pathspec.h | 7 +++++++
> tree-diff.c | 19 +++++++++++++++++++
> tree-walk.c | 2 ++
> 5 files changed, 32 insertions(+)
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index 8c2af6c..d237e0a 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -371,6 +371,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
> die(_("unhandled object '%s' given."), name);
> }
> if (rev.prune_data.nr) {
> + /* builtin_diff_b_f() */
> + GUARD_PATHSPEC(&rev.prune_data, PATHSPEC_FROMTOP);
> if (!path)
> path = rev.prune_data.items[0].match;
> paths += rev.prune_data.nr;
> diff --git a/dir.c b/dir.c
> index 1e9db41..6094ba8 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -297,6 +297,8 @@ int match_pathspec_depth(const struct pathspec *ps,
> {
> int i, retval = 0;
>
> + GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
> +
> if (!ps->nr) {
> if (!ps->recursive ||
> !(ps->magic & PATHSPEC_MAXDEPTH) ||
> diff --git a/pathspec.h b/pathspec.h
> index 1cef9c6..ed5d3a6 100644
> --- a/pathspec.h
> +++ b/pathspec.h
> @@ -27,6 +27,13 @@ struct pathspec {
> } *items;
> };
>
> +#define GUARD_PATHSPEC(ps, mask) \
> + do { \
> + if ((ps)->magic & ~(mask)) \
> + die("BUG:%s:%d: unsupported magic %x", \
> + __FILE__, __LINE__, (ps)->magic & ~(mask)); \
> + } while (0)
> +
> /* parse_pathspec flags */
> #define PATHSPEC_PREFER_CWD (1<<0) /* No args means match cwd */
> #define PATHSPEC_PREFER_FULL (1<<1) /* No args means match everything */
> diff --git a/tree-diff.c b/tree-diff.c
> index 826512e..5a87614 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -198,6 +198,25 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
> const char *paths[1];
> int i;
>
> + /*
> + * follow-rename code is very specific, we need exactly one
> + * path. Magic that matches more than one path is not
> + * supported.
> + */
> + GUARD_PATHSPEC(&opt->pathspec, PATHSPEC_FROMTOP);
> +#if 0
> + /*
> + * We should reject wildcards as well. Unfortunately we
> + * haven't got a reliable way to detect that 'foo\*bar' in
> + * fact has no wildcards. nowildcard_len is merely a hint for
> + * optimization. Let it slip for now until wildmatch is taught
> + * about dry-run mode and returns wildcard info.
> + */
> + if (opt->pathspec.has_wildcard)
> + die("BUG:%s:%d: wildcards are not supported",
> + __FILE__, __LINE__);
> +#endif
> +
> /* Remove the file creation entry from the diff queue, and remember it */
> choice = q->queue[0];
> q->nr = 0;
> diff --git a/tree-walk.c b/tree-walk.c
> index d399ca9..37b157e 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -636,6 +636,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
> enum interesting never_interesting = ps->has_wildcard ?
> entry_not_interesting : all_entries_not_interesting;
>
> + GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
> +
> if (!ps->nr) {
> if (!ps->recursive ||
> !(ps->magic & PATHSPEC_MAXDEPTH) ||
^ permalink raw reply
* Re: [PATCH/RFC] http_init: only initialize SSL for https
From: Daniel Stenberg @ 2013-03-17 22:11 UTC (permalink / raw)
To: Antoine Pelisse
Cc: Jeff King, Junio C Hamano, Johannes Schindelin, kusmabite, git,
msysgit
In-Reply-To: <CALWbr2wQNM=7vUcoragNmKGpSeXkOCsmsM5y1AMhj95i15A4bw@mail.gmail.com>
On Sun, 17 Mar 2013, Antoine Pelisse wrote:
>> With redirects taken into account, I can't think of any really good way
>> around avoiding this init...
>
> Is there any way for curl to initialize SSL on-demand ?
Yes, but not without drawbacks.
If you don't call curl_global_init() at all, libcurl will notice that on first
use and then libcurl will call global_init by itself with a default bitmask.
That automatic call of course will prevent the application from being able to
set its own bitmask choice, and also the global_init function is not
(necessarily) thread safe while all other libcurl functions are so the
internal call to global_init from an otherwise thread-safe function is
unfortunate.
--
/ daniel.haxx.se
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* Re: [PATCH] remote.<name>.pushurl does not consider aliases when pushing
From: Junio C Hamano @ 2013-03-17 22:14 UTC (permalink / raw)
To: Rob Hoelz; +Cc: git, josh
In-Reply-To: <20130317224002.366f54f7@hoelz.ro>
Rob Hoelz <rob@hoelz.ro> writes:
> Hi everyone! I found a bug in Git today and wrote up a fix; I did my best to conform to the rules layed out in Documentation/SubmittingPatches, but please let me know if I need to change anything to get my work merged. =) I have CC'ed Josh Triplet, as
> he was the last one to touch the line I modified. I hope my commit messages explain the problem I encountered well enough; if not,
> I can always go back and amend them.
>
> Patches follow.
>
> -Rob
Please read Documentation/SubmittingPatches and follow it. The
above is most likely to be the cover letter of a two-patch series
(meaning you will be sending three pieces of e-mail messages), or
perhaps out of band comment below the three-dash line of a single
patch (you will send only one piece of e-mail message).
See recent patches on the list from list regulars for good examples,
e.g.
http://thread.gmane.org/gmane.comp.version-control.git/218350
http://thread.gmane.org/gmane.comp.version-control.git/218177/focus=218361
> From 5007b11e86c0835807632cb41e6cfa75ce9a1aa1 Mon Sep 17 00:00:00 2001
> From: Rob Hoelz <rob@hoelz.ro>
> Date: Sun, 17 Mar 2013 21:49:20 +0100
> Subject: [PATCH 1/2] Add test for pushInsteadOf + pushurl
>
> git push currently doesn't consider pushInsteadOf when
> using pushurl; this test tests that.
>
> Signed-off-by: Rob Hoelz <rob@hoelz.ro>
> ---
> t/t5500-push-pushurl.sh | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
> create mode 100644 t/t5500-push-pushurl.sh
The number 5500 is already taken. Please do not add a duplicate.
I also wonder if we need to waste a new test number for this;
perhaps adding new tests to 5516 that already tests insteadOf might
be a better fit, but I didn't carefully read it.
> diff --git a/t/t5500-push-pushurl.sh b/t/t5500-push-pushurl.sh
> new file mode 100644
Test scripts are supposed to be executable.
> +test_expect_success 'test commit and push' '
> + test_commit one &&
> + git push origin master:master
> +'
> +
> +test_expect_success 'check for commits in rw repo' '
> + cd ../rw/repo &&
> + git log --pretty=oneline | grep -q .
> +'
Are both expected to succeed in patch 1/2 without any code change?
If you were doing a large code change, it is a good series structure
to have tests first that are marked as "expect_failure" in an early
patch, and then in a later patch that changes the code to fix it,
update the tests that start to pass to "expect_success".
I personally do not think you need such a two-step approach for
something small like this; instead you can just have a single patch
that adds a set of tests that expect success and code change.
Thanks.
^ permalink raw reply
* [PATCH] t1507: Test that branchname@{upstream} is interpreted as branch
From: Kacper Kornet @ 2013-03-17 22:17 UTC (permalink / raw)
To: git
In-Reply-To: <1363459903-32358-1-git-send-email-draenog@pld-linux.org>
Syntax branchname@{upstream} should interpret its argument as a name of
a branch. Add the test to check that it doesn't try to interpret it as a
refname if the branch in question does not exist.
Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
---
Maybe I'm too cautious adding this test. But just in case here it is.
t/t1507-rev-parse-upstream.sh | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index d6e5761..b27a720 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -54,6 +54,10 @@ test_expect_success 'my-side@{upstream} resolves to correct full name' '
test refs/remotes/origin/side = "$(full_name my-side@{u})"
'
+test_expect_success 'refs/heads/my-side@{upstream} does not resolve to my-side{upstream}' '
+ test_must_fail full_name refs/heads/my-side@{upstream}
+'
+
test_expect_success 'my-side@{u} resolves to correct commit' '
git checkout side &&
test_commit 5 &&
--
1.8.2
^ permalink raw reply related
* Re: [PATCH/RFC] http_init: only initialize SSL for https
From: Junio C Hamano @ 2013-03-17 22:27 UTC (permalink / raw)
To: Daniel Stenberg
Cc: Antoine Pelisse, Jeff King, Johannes Schindelin, kusmabite, git,
msysgit
In-Reply-To: <alpine.DEB.2.00.1303172305230.21738@tvnag.unkk.fr>
Daniel Stenberg <daniel@haxx.se> writes:
> On Sun, 17 Mar 2013, Antoine Pelisse wrote:
>
>>> With redirects taken into account, I can't think of any really good way
>>> around avoiding this init...
>>
>> Is there any way for curl to initialize SSL on-demand ?
>
> Yes, but not without drawbacks.
>
> If you don't call curl_global_init() at all, libcurl will notice that
> on first use and then libcurl will call global_init by itself with a
> default bitmask.
>
> That automatic call of course will prevent the application from being
> able to set its own bitmask choice, and also the global_init function
> is not (necessarily) thread safe while all other libcurl functions are
> so the internal call to global_init from an otherwise thread-safe
> function is unfortunate.
So in short, unless you are writing a custom application to talk to
servers that you know will never redirect you to HTTPS, passing
custom masks such as ALL&~SSL to global-init is not going to be a
valid optimization.
I think that is a reasonable API; your custom application may want
to go around your intranet servers all of which serve their status
over plain HTTP, and it is a valid optimization to initialize the
library with ALL&~SSL. It is just that such an optimization does
not apply to us---we let our users go to random hosts we have no
control over, and they may redirect us in ways we cannot anticipate.
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* Re: [PATCH] combine-diff: coalesce lost lines optimally
From: Junio C Hamano @ 2013-03-17 22:37 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git
In-Reply-To: <1363525436-21667-1-git-send-email-apelisse@gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> +/* Coalesce new lines into base by finding LCS */
> +static struct lline *coalesce_lines(struct lline *base, int *lenbase,
> + struct lline *new, int lennew,
> + unsigned long parent)
> +{
Don't you want to pass flags so that you can use match_string_spaces()
at places like this:
> + for (i = 1, baseend = base; i < origbaselen + 1; i++) {
> + for (j = 1, newend = new; j < lennew + 1; j++) {
> + if (baseend->len == newend->len &&
> + !memcmp(baseend->line, newend->line, baseend->len)) {
IOW, it looks to me that this wants to be rebuilt on top of
fa04ae0be8cc (Allow combined diff to ignore white-spaces,
2013-03-14).
^ permalink raw reply
* Re: [PATCH] remote.<name>.pushurl does not consider aliases when pushing
From: Rob Hoelz @ 2013-03-17 22:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, josh
In-Reply-To: <7vppyxptbr.fsf@alter.siamese.dyndns.org>
On Sun, 17 Mar 2013 15:14:32 -0700
Junio C Hamano <gitster@pobox.com> wrote:
> Rob Hoelz <rob@hoelz.ro> writes:
>
> > Hi everyone! I found a bug in Git today and wrote up a fix; I did
> > my best to conform to the rules layed out in
> > Documentation/SubmittingPatches, but please let me know if I need
> > to change anything to get my work merged. =) I have CC'ed Josh
> > Triplet, as he was the last one to touch the line I modified. I
> > hope my commit messages explain the problem I encountered well
> > enough; if not, I can always go back and amend them.
> >
> > Patches follow.
> >
> > -Rob
>
>
> Please read Documentation/SubmittingPatches and follow it. The
> above is most likely to be the cover letter of a two-patch series
> (meaning you will be sending three pieces of e-mail messages), or
> perhaps out of band comment below the three-dash line of a single
> patch (you will send only one piece of e-mail message).
>
> See recent patches on the list from list regulars for good examples,
> e.g.
>
> http://thread.gmane.org/gmane.comp.version-control.git/218350
> http://thread.gmane.org/gmane.comp.version-control.git/218177/focus=218361
>
> > From 5007b11e86c0835807632cb41e6cfa75ce9a1aa1 Mon Sep 17 00:00:00
> > 2001 From: Rob Hoelz <rob@hoelz.ro>
> > Date: Sun, 17 Mar 2013 21:49:20 +0100
> > Subject: [PATCH 1/2] Add test for pushInsteadOf + pushurl
> >
> > git push currently doesn't consider pushInsteadOf when
> > using pushurl; this test tests that.
> >
> > Signed-off-by: Rob Hoelz <rob@hoelz.ro>
> > ---
> > t/t5500-push-pushurl.sh | 37 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> > create mode 100644 t/t5500-push-pushurl.sh
>
> The number 5500 is already taken. Please do not add a duplicate.
>
> I also wonder if we need to waste a new test number for this;
> perhaps adding new tests to 5516 that already tests insteadOf might
> be a better fit, but I didn't carefully read it.
>
> > diff --git a/t/t5500-push-pushurl.sh b/t/t5500-push-pushurl.sh
> > new file mode 100644
>
> Test scripts are supposed to be executable.
>
> > +test_expect_success 'test commit and push' '
> > + test_commit one &&
> > + git push origin master:master
> > +'
> > +
> > +test_expect_success 'check for commits in rw repo' '
> > + cd ../rw/repo &&
> > + git log --pretty=oneline | grep -q .
> > +'
>
> Are both expected to succeed in patch 1/2 without any code change?
>
> If you were doing a large code change, it is a good series structure
> to have tests first that are marked as "expect_failure" in an early
> patch, and then in a later patch that changes the code to fix it,
> update the tests that start to pass to "expect_success".
>
> I personally do not think you need such a two-step approach for
> something small like this; instead you can just have a single patch
> that adds a set of tests that expect success and code change.
>
> Thanks.
>
Thanks for the feeback; another reply with the new patch follows.
^ permalink raw reply
* [PATCH] push: Alias pushurl from push rewrites
From: Rob Hoelz @ 2013-03-17 22:50 UTC (permalink / raw)
To: git; +Cc: josh
git push currently doesn't consider pushInsteadOf when
using pushurl; this tests and fixes that.
If you use pushurl with an alias that has a pushInsteadOf configuration
value, Git does not take advantage of it. For example:
[url "git://github.com/"]
insteadOf = github:
[url "git://github.com/myuser/"]
insteadOf = mygithub:
[url "git@github.com:myuser/"]
pushInsteadOf = mygithub:
[remote "origin"]
url = github:organization/project
pushurl = mygithub:project
Signed-off-by: Rob Hoelz <rob@hoelz.ro>
---
remote.c | 2 +-
t/t5516-fetch-push.sh | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index ca1f8f2..de7a915 100644
--- a/remote.c
+++ b/remote.c
@@ -465,7 +465,7 @@ static void alias_all_urls(void)
if (!remotes[i])
continue;
for (j = 0; j < remotes[i]->pushurl_nr; j++) {
- remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
+ remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites_push);
}
add_pushurl_aliases = remotes[i]->pushurl_nr == 0;
for (j = 0; j < remotes[i]->url_nr; j++) {
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..272e225 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -244,6 +244,26 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
)
'
+test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf does rewrite in this case)' '
+ mk_empty &&
+ TRASH="$(pwd)/" &&
+ mkdir ro &&
+ mkdir rw &&
+ git init --bare rw/testrepo &&
+ git config "url.file://$TRASH/ro/.insteadOf" ro: &&
+ git config "url.file://$TRASH/rw/.pushInsteadOf" rw: &&
+ git config remote.r.url ro:wrong &&
+ git config remote.r.pushurl rw:testrepo &&
+ git push r refs/heads/master:refs/remotes/origin/master &&
+ (
+ cd rw/testrepo &&
+ r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+ test "z$r" = "z$the_commit" &&
+
+ test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+ )
+'
+
test_expect_success 'push with matching heads' '
mk_test heads/master &&
--
1.8.2
^ permalink raw reply related
* Re: [PATCH] push: Alias pushurl from push rewrites
From: Junio C Hamano @ 2013-03-17 23:35 UTC (permalink / raw)
To: Rob Hoelz; +Cc: git, josh
In-Reply-To: <20130317235040.4cdc9ef2@hoelz.ro>
Rob Hoelz <rob@hoelz.ro> writes:
> git push currently doesn't consider pushInsteadOf when
> using pushurl; this tests and fixes that.
>
> If you use pushurl with an alias that has a pushInsteadOf configuration
> value, Git does not take advantage of it. For example:
>
> [url "git://github.com/"]
> insteadOf = github:
> [url "git://github.com/myuser/"]
> insteadOf = mygithub:
> [url "git@github.com:myuser/"]
> pushInsteadOf = mygithub:
> [remote "origin"]
> url = github:organization/project
> pushurl = mygithub:project
Incomplete sentence? For example [this is an example configuration]
and then what happens? Something like "with the sample
configuration, 'git push origin' should follow pushurl and then turn
it into X, but instead it ends up accessing Y".
If there is no pushInsteadOf, does it still follow insteadOf? Is it
tested already?
Wouldn't you want to cover all the combinations to negative cases
(i.e. making sure the codepath to support a new case does not affect
behaviour of the code outside the new case)? A remote with and
without pushurl (two combinations) and a pseudo URL scheme with and
without pushInsteadOf (again, two combinations) will give you four
cases.
Thanks.
>
> Signed-off-by: Rob Hoelz <rob@hoelz.ro>
> ---
> remote.c | 2 +-
> t/t5516-fetch-push.sh | 20 ++++++++++++++++++++
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index ca1f8f2..de7a915 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -465,7 +465,7 @@ static void alias_all_urls(void)
> if (!remotes[i])
> continue;
> for (j = 0; j < remotes[i]->pushurl_nr; j++) {
> - remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
> + remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites_push);
> }
> add_pushurl_aliases = remotes[i]->pushurl_nr == 0;
> for (j = 0; j < remotes[i]->url_nr; j++) {
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index b5417cc..272e225 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -244,6 +244,26 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
> )
> '
>
> +test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf does rewrite in this case)' '
> + mk_empty &&
> + TRASH="$(pwd)/" &&
> + mkdir ro &&
> + mkdir rw &&
> + git init --bare rw/testrepo &&
> + git config "url.file://$TRASH/ro/.insteadOf" ro: &&
> + git config "url.file://$TRASH/rw/.pushInsteadOf" rw: &&
> + git config remote.r.url ro:wrong &&
> + git config remote.r.pushurl rw:testrepo &&
> + git push r refs/heads/master:refs/remotes/origin/master &&
> + (
> + cd rw/testrepo &&
> + r=$(git show-ref -s --verify refs/remotes/origin/master) &&
> + test "z$r" = "z$the_commit" &&
> +
> + test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
> + )
> +'
> +
> test_expect_success 'push with matching heads' '
>
> mk_test heads/master &&
^ permalink raw reply
* Re: [PATCH v1 11/45] parse_pathspec: support stripping submodule trailing slashes
From: Duy Nguyen @ 2013-03-18 0:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy5dlpu7i.fsf@alter.siamese.dyndns.org>
On Mon, Mar 18, 2013 at 4:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> This flag is equivalent to builtin/ls-files.c:strip_trailing_slashes()
>> and is intended to replace that function when ls-files is converted to
>> use parse_pathspec.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> pathspec.c | 9 +++++++++
>> pathspec.h | 1 +
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/pathspec.c b/pathspec.c
>> index b2446c3..2da8bc9 100644
>> --- a/pathspec.c
>> +++ b/pathspec.c
>> @@ -204,6 +204,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>> *raw = item->match = match;
>> item->original = elt;
>> item->len = strlen(item->match);
>> +
>> + if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
>
> I see that having cheap and expensive variant at these steps is a
> way to achieve a bug-for-bug compatible rewrite of the existing
> code, but in the longer term should we lose one of them? After all,
> a path that points at the working tree of another repository that is
> beyond a symlink cannot be added to us as a submodule, so CHEAP can
> be used only when we know that any intermediate component on the
> path is not a symlink pointing elsewhere, and the user in the
> codepath of ls-files may know it.
I did not concentrate on the future part. But yes only one should
remain in long term. Cheap vs expensive does not say much.
> To put it differently, shouldn't CHEAP and EXPENSIVE be accompanied
> by in-code comment and updates to Documentation/technical/api-*.txt
> to help users of the API to decide which one to use?
Will do (also for the comment on 14/45)
--
Duy
^ permalink raw reply
* Re: git: how the pack-objects.c find the object's delta
From: Duy Nguyen @ 2013-03-18 0:54 UTC (permalink / raw)
To: 方栋; +Cc: Git Mailing List
In-Reply-To: <tencent_0A91BD9B7F2294EF42EFC942@qq.com>
Hi,
I can't say from a first glance. Maybe git@vger can help?
On Sun, Mar 17, 2013 at 10:08 PM, 方栋 <fangdong@pipul.org> wrote:
> hello
>
> i don't understand this:
> src in builtin/pack-objects.c find_deltas() function, line 1800:
>
>
> static void find_deltas(struct object_entry **list, unsigned *list_size,
> int window, int depth, unsigned *processed)
> {
> ......
>
> /*
> * Move the best delta base up in the window, after the
> * currently deltified object, to keep it longer. It will
> * be the first base object to be attempted next.
> */
> if (entry->delta) {
> struct unpacked swap = array[best_base];
> int dist = (window + idx - best_base) % window;
> int dst = best_base;
> while (dist--) {
> int src = (dst + 1) % window;
> array[dst] = array[src];
> dst = src;
> }
> array[dst] = swap;
> }
> ......
> }
>
> what this code block use for?
>
> thx
>
>
>
>
> ------------------
> 祝好
> --方（tyut）
>
--
Duy
^ permalink raw reply
* Re: [PATCH v2 4/4] pack-refs: add fully-peeled trait
From: Michael Haggerty @ 2013-03-18 3:12 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20130317082829.GD29550@sigill.intra.peff.net>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
and ACK for the whole series, once Junio's points are addressed.
Regarding Junio's readability suggestion: I agree that his versions are
a bit more readable, albeit at the expense of having to evaluate a bit
more logic for each reference rather than just once when the header line
is handled. So I don't have a preference either way.
Michael
On 03/17/2013 09:28 AM, Jeff King wrote:
> From: Michael Haggerty <mhagger@alum.mit.edu>
>
> Older versions of pack-refs did not write peel lines for
> refs outside of refs/tags. This meant that on reading the
> pack-refs file, we might set the REF_KNOWS_PEELED flag for
> such a ref, even though we do not know anything about its
> peeled value.
>
> The previous commit updated the writer to always peel, no
> matter what the ref is. That means that packed-refs files
> written by newer versions of git are fine to be read by both
> old and new versions of git. However, we still have the
> problem of reading packed-refs files written by older
> versions of git, or by other implementations which have not
> yet learned the same trick.
>
> The simplest fix would be to always unset the
> REF_KNOWS_PEELED flag for refs outside of refs/tags that do
> not have a peel line (if it has a peel line, we know it is
> valid, but we cannot assume a missing peel line means
> anything). But that loses an important optimization, as
> upload-pack should not need to load the object pointed to by
> refs/heads/foo to determine that it is not a tag.
>
> Instead, we add a "fully-peeled" trait to the packed-refs
> file. If it is set, we know that we can trust a missing peel
> line to mean that a ref cannot be peeled. Otherwise, we fall
> back to assuming nothing.
>
> [commit message and tests by Jeff King <peff@peff.net>]
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This uses Michael's approach for managing the flags within
> read_packed_refs, which is more readable. As I picked up his
> code and comments, I realized that there was basically
> nothing of mine left, so I switched the authorship. But do
> note:
>
> 1. It should have Michael's signoff, which was not present
> in the commit I lifted the code from.
>
> 2. I tweaked the big comment above read_packed_refs to
> reduce some ambiguities. Please double-check that I am
> not putting inaccurate words in your mouth. :)
>
> pack-refs.c | 2 +-
> refs.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> t/t3211-peel-ref.sh | 22 ++++++++++++++++++++++
> 3 files changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/pack-refs.c b/pack-refs.c
> index ebde785..4461f71 100644
> --- a/pack-refs.c
> +++ b/pack-refs.c
> @@ -128,7 +128,7 @@ int pack_refs(unsigned int flags)
> die_errno("unable to create ref-pack file structure");
>
> /* perhaps other traits later as well */
> - fprintf(cbdata.refs_file, "# pack-refs with: peeled \n");
> + fprintf(cbdata.refs_file, "# pack-refs with: peeled fully-peeled \n");
>
> for_each_ref(handle_one_ref, &cbdata);
> if (ferror(cbdata.refs_file))
> diff --git a/refs.c b/refs.c
> index 175b9fc..bdeac28 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -803,11 +803,39 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
> return line;
> }
>
> +/*
> + * Read f, which is a packed-refs file, into dir.
> + *
> + * A comment line of the form "# pack-refs with: " may contain zero or
> + * more traits. We interpret the traits as follows:
> + *
> + * No traits:
> + *
> + * Probably no references are peeled. But if the file contains a
> + * peeled value for a reference, we will use it.
> + *
> + * peeled:
> + *
> + * References under "refs/tags/", if they *can* be peeled, *are*
> + * peeled in this file. References outside of "refs/tags/" are
> + * probably not peeled even if they could have been, but if we find
> + * a peeled value for such a reference we will use it.
> + *
> + * fully-peeled:
> + *
> + * All references in the file that can be peeled are peeled.
> + * Inversely (and this is more important, any references in the
> + * file for which no peeled value is recorded is not peelable. This
> + * trait should typically be written alongside "fully-peeled" for
> + * compatibility with older clients, but we do not require it
> + * (i.e., "peeled" is a no-op if "fully-peeled" is set).
> + */
> static void read_packed_refs(FILE *f, struct ref_dir *dir)
> {
> struct ref_entry *last = NULL;
> char refline[PATH_MAX];
> int flag = REF_ISPACKED;
> + int refs_tags_peeled = 0;
>
> while (fgets(refline, sizeof(refline), f)) {
> unsigned char sha1[20];
> @@ -816,8 +844,10 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
>
> if (!strncmp(refline, header, sizeof(header)-1)) {
> const char *traits = refline + sizeof(header) - 1;
> - if (strstr(traits, " peeled "))
> + if (strstr(traits, " fully-peeled "))
> flag |= REF_KNOWS_PEELED;
> + else if (strstr(traits, " peeled "))
> + refs_tags_peeled = 1;
> /* perhaps other traits later as well */
> continue;
> }
> @@ -825,6 +855,8 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
> refname = parse_ref_line(refline, sha1);
> if (refname) {
> last = create_ref_entry(refname, sha1, flag, 1);
> + if (refs_tags_peeled && !prefixcmp(refname, "refs/tags/"))
> + last->flag |= REF_KNOWS_PEELED;
> add_ref(dir, last);
> continue;
> }
> @@ -832,8 +864,15 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
> refline[0] == '^' &&
> strlen(refline) == 42 &&
> refline[41] == '\n' &&
> - !get_sha1_hex(refline + 1, sha1))
> + !get_sha1_hex(refline + 1, sha1)) {
> hashcpy(last->u.value.peeled, sha1);
> + /*
> + * Regardless of what the file header said,
> + * we definitely know the value of *this*
> + * reference:
> + */
> + last->flag |= REF_KNOWS_PEELED;
> + }
> }
> }
>
> diff --git a/t/t3211-peel-ref.sh b/t/t3211-peel-ref.sh
> index 85f09be..d4d7792 100755
> --- a/t/t3211-peel-ref.sh
> +++ b/t/t3211-peel-ref.sh
> @@ -39,4 +39,26 @@ test_expect_success 'refs are peeled outside of refs/tags (packed)' '
> test_cmp expect actual
> '
>
> +test_expect_success 'create old-style pack-refs without fully-peeled' '
> + # Git no longer writes without fully-peeled, so we just write our own
> + # from scratch; we could also munge the existing file to remove the
> + # fully-peeled bits, but that seems even more prone to failure,
> + # especially if the format ever changes again. At least this way we
> + # know we are emulating exactly what an older git would have written.
> + {
> + echo "# pack-refs with: peeled " &&
> + print_ref "refs/heads/master" &&
> + print_ref "refs/outside/foo" &&
> + print_ref "refs/tags/base" &&
> + print_ref "refs/tags/foo" &&
> + echo "^$(git rev-parse "refs/tags/foo^{}")"
> + } >tmp &&
> + mv tmp .git/packed-refs
> +'
> +
> +test_expect_success 'refs are peeled outside of refs/tags (old packed)' '
> + git show-ref -d >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
>
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: Tag peeling peculiarities
From: Michael Haggerty @ 2013-03-18 3:17 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git discussion list
In-Reply-To: <514475C4.7020901@alum.mit.edu>
On 03/16/2013 02:38 PM, Michael Haggerty wrote:
> On 03/16/2013 10:34 AM, Jeff King wrote:
>> On Sat, Mar 16, 2013 at 09:48:42AM +0100, Michael Haggerty wrote:
>>
>>> My patch series is nearly done. I will need another day or two to
>>> review and make it submission-ready, but I wanted to give you an idea of
>>> what I'm up to and I could also use your feedback on some points.
I will wait for the dust to settle on Peff's "peel-ref optimization
fixes" patches, then rebase my series on top of his before submitting.
The rest of my series is not so urgent so I don't think the delay will
be a problem.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Git build fails on `make`, undefined references in libcrypto.a.
From: zero modulo @ 2013-03-18 4:03 UTC (permalink / raw)
To: git
$ LDFLAGS="-L/sandbox/builds/lib" CPPFLAGS="-I/sandbox/builds/include"
./configure --prefix=$PREFIX
$ make
[…]
/sandbox/builds/lib/libcrypto.a(dso_dlfcn.o): In function `dlfcn_globallookup':
dso_dlfcn.c:(.text+0x1b): undefined reference to `dlopen'
dso_dlfcn.c:(.text+0x31): undefined reference to `dlsym'
dso_dlfcn.c:(.text+0x3b): undefined reference to `dlclose'
/sandbox/builds/lib/libcrypto.a(dso_dlfcn.o): In function `dlfcn_bind_func':
dso_dlfcn.c:(.text+0x3c1): undefined reference to `dlsym'
dso_dlfcn.c:(.text+0x4a0): undefined reference to `dlerror'
/sandbox/builds/lib/libcrypto.a(dso_dlfcn.o): In function `dlfcn_bind_var':
dso_dlfcn.c:(.text+0x521): undefined reference to `dlsym'
dso_dlfcn.c:(.text+0x600): undefined reference to `dlerror'
/sandbox/builds/lib/libcrypto.a(dso_dlfcn.o): In function `dlfcn_load':
dso_dlfcn.c:(.text+0x678): undefined reference to `dlopen'
dso_dlfcn.c:(.text+0x6e8): undefined reference to `dlclose'
dso_dlfcn.c:(.text+0x72d): undefined reference to `dlerror'
/sandbox/builds/lib/libcrypto.a(dso_dlfcn.o): In function `dlfcn_pathbyaddr':
dso_dlfcn.c:(.text+0x7e1): undefined reference to `dladdr'
dso_dlfcn.c:(.text+0x849): undefined reference to `dlerror'
/sandbox/builds/lib/libcrypto.a(dso_dlfcn.o): In function `dlfcn_unload':
dso_dlfcn.c:(.text+0x8aa): undefined reference to `dlclose'
collect2: error: ld returned 1 exit status
make: *** [git-imap-send] Error 1
^ permalink raw reply
* git branch based hook desigh
From: Joydeep Bakshi @ 2013-03-18 4:27 UTC (permalink / raw)
To: git@vger.kernel.org
Hello list,
I have implemented git pre-received hook successfully. And it works on the repo level.
Could anyone suggest how to call branch level hook please ?
^ permalink raw reply
* Recovering from a loss of sync between git and svn
From: Jon Seymour @ 2013-03-18 5:03 UTC (permalink / raw)
To: Git Mailing List
G'day,
I managed to lose sync between the git-svn repo that I am using to
track an svn repo. In particular, the git-svn repo lost the content of
about 5 commits with the net result that the git-svn repo and the svn
repo it tracks have a difference of opinion about what the contents of
trunk are for the files involved in the missing commits.
The situation arose because I used --ignore-paths trunk on a
git-svn-fetch when I was trying to deal with an SVN user that had
copied trunk into the same SVN tag twice (which caused the source tree
to be duplicated under the trunk directory of the SVN tag).
I was hoping that --ignore-paths trunk would cause the git-svn copy of
the tag to exclude the unwanted copy of the trunk directory in the
tag. Instead, it appears to have caused my fetches of subsequent
commits to SVN trunk to be empty, resulting in divergence between by
git-svn repo and the SVN repo itself.
Does anyone have any tips about how I can fix this other than pulling
the entire SVN repo again?
jon.
^ permalink raw reply
* Re: building git ; need suggestion
From: Joydeep Bakshi @ 2013-03-18 5:44 UTC (permalink / raw)
To: Magnus Bäck; +Cc: Fredrik Gustafsson, git
In-Reply-To: <20130315131403.GA27022@google.com>
On 15-Mar-2013, at 6:44 PM, Magnus Bäck <baeck@google.com> wrote:
>>
>
> Right, but that's R/W permissions. Almost any piece of Git hosting
> software supports restriction of pushes. Discriminating *read* access
> between developers and maintenance people sounds like a disaster if it's
> the same organisation.
Just restriction on push access is what required.
^ permalink raw reply
* Re: Recovering from a loss of sync between git and svn
From: Jon Seymour @ 2013-03-18 6:09 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <CAH3Anrq4y54YA=qeEbO5z1FKB7Adg4G8vf4jmPLzLWa6tky0wQ@mail.gmail.com>
Ah, answering my own question....
I can see git svn reset does exactly what I need.
jon.
On Mon, Mar 18, 2013 at 4:03 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> G'day,
>
> I managed to lose sync between the git-svn repo that I am using to
> track an svn repo. In particular, the git-svn repo lost the content of
> about 5 commits with the net result that the git-svn repo and the svn
> repo it tracks have a difference of opinion about what the contents of
> trunk are for the files involved in the missing commits.
>
> The situation arose because I used --ignore-paths trunk on a
> git-svn-fetch when I was trying to deal with an SVN user that had
> copied trunk into the same SVN tag twice (which caused the source tree
> to be duplicated under the trunk directory of the SVN tag).
>
> I was hoping that --ignore-paths trunk would cause the git-svn copy of
> the tag to exclude the unwanted copy of the trunk directory in the
> tag. Instead, it appears to have caused my fetches of subsequent
> commits to SVN trunk to be empty, resulting in divergence between by
> git-svn repo and the SVN repo itself.
>
> Does anyone have any tips about how I can fix this other than pulling
> the entire SVN repo again?
>
> jon.
^ permalink raw reply
* Re: Git build fails on `make`, undefined references in libcrypto.a.
From: Konstantin Khomoutov @ 2013-03-18 7:29 UTC (permalink / raw)
To: zero modulo; +Cc: git
In-Reply-To: <CAA8xkY==7021SyDmeiOcHMzXbX9L0GgG9yTTED5u1r+tfAPGqg@mail.gmail.com>
On Sun, Mar 17, 2013 at 10:03:37PM -0600, zero modulo wrote:
> $ LDFLAGS="-L/sandbox/builds/lib" CPPFLAGS="-I/sandbox/builds/include"
> ./configure --prefix=$PREFIX
>
> $ make
> […]
> /sandbox/builds/lib/libcrypto.a(dso_dlfcn.o): In function `dlfcn_globallookup':
> dso_dlfcn.c:(.text+0x1b): undefined reference to `dlopen'
[...]
> make: *** [git-imap-send] Error 1
FYI, I've already tried to answer this exact question [1] with no
comments from the OP.
1. http://serverfault.com/a/488604/118848
^ permalink raw reply
* Re: Make GIT_USE_LOOKUP default?
From: Jeff King @ 2013-03-18 7:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ingo Molnar, Jonathan Nieder, Duy Nguyen, Git Mailing List
In-Reply-To: <7vd2uxrdh7.fsf@alter.siamese.dyndns.org>
[+cc Ingo and Jonathan, as this revisits the "open-code hashcmp" thread
referenced below]
On Sun, Mar 17, 2013 at 01:13:56PM -0700, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
> > This env comes from jc/sha1-lookup in 2008 (merge commit e9f9d4f), 5
> > years ago. I wonder if it's good enough to turn on by default and keep
> > improving from there, or is it still experimental?
>
> The algorithm has been used in production in other codepaths like
> patch-ids and replace-object, so correctness-wise it should be fine
> to turn it on. I think nobody has bothered to benchmark with and
> without the environment to see if it is really worth the complexity.
>
> It may be a good idea to try doing so, now you have noticed it ;-).
The only benchmarking I could find in the list archive (besides the ones
in the commit itself, showing little change, but fewer page faults) is:
http://article.gmane.org/gmane.comp.version-control.git/123832
which actually indicates that GIT_USE_LOOKUP is slower (despite having
fewer page faults).
By the way, looking at that made me think for a few minutes about
hashcmp, and I was surprised to find that we use an open-coded
comparison loop. That dates back to this thread by Ingo:
http://article.gmane.org/gmane.comp.version-control.git/172286
I could not replicate his benchmarks at all. In fact, my measurements
showed a slight slowdown with 1a812f3 (hashcmp(): inline memcmp() by
hand to optimize, 2011-04-28).
Here are my best-of-five numbers for running "git rev-list --objects
--all >/dev/null" on linux-2.6.git:
[current master, compiled with -O2]
real 0m45.612s
user 0m45.140s
sys 0m0.300s
[current master, compiled with -O3 for comparison]
real 0m45.588s
user 0m45.088s
sys 0m0.312s
[revert 1a812f3 (i.e., go back to memcmp), -O2]
real 0m44.358s
user 0m43.876s
sys 0m0.316s
[open-code first byte, fall back to memcmp, -O2]
real 0m43.963s
user 0m43.568s
sys 0m0.284s
I wonder why we get such different numbers. Ingo said his tests are on a
Nehalem CPU, as are mine (mine is an i7-840QM). I wonder if we should be
wrapping the optimization in an #ifdef, but I'm not sure which flag we
should be checking.
Note that I didn't run all of my measurements using "git gc" as Ingo
did, which I think conflates a lot of unrelated performance issues (like
writing out a packfile). The interesting bits for hashcmp in "gc" are
the "Counting objects" phase of pack-objects, and "git prune"
determining reachability. Those are both basically the same as "rev-list
--objects --all".
I did do a quick check of `git gc`, though, and it showed results that
matched my rev-lists above (i.e., a very slight speedup by going back to
memcmp).
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
From: Ramkumar Ramachandra @ 2013-03-18 8:24 UTC (permalink / raw)
To: Thomas Rast
Cc: Junio C Hamano, Karsten Blees, Duy Nguyen, Git List,
Torsten Bögershausen, Robert Zeh, Jeff King, Erik Faye-Lund,
Drew Northup
In-Reply-To: <87ehffv30f.fsf@pctrast.inf.ethz.ch>
Junio C Hamano wrote:
> Yes, and you would need one inotify per directory but you do not
> have an infinite supply of outstanding inotify watch (wasn't the
> limit like 8k per a single uid or something?), so the daemon must be
> prepared to say "I'll watch this, that and that directories, but the
> consumers should check other directories themselves."
>
> FWIW, I share your suspicion that an effort in the direction this
> thread suggests may end up duplicating what the caching vfs layer
> already does, and doing so poorly.
Thomas Rast wrote:
> $ cat /proc/sys/fs/inotify/max_user_watches
> 65536
> $ cat /proc/sys/fs/inotify/max_user_instancest
> 128
>From Junio's and Thomas' observations, I'm inclined to think that
inotify is ill-suited for the problem we are trying to solve. It is
designed as a per-directory watch, because VFS can quickly supply the
inodes for a directory entry. As such, I think the ideal usecase for
inotify is to execute something immediately when a change takes place
in a directory: it's well-suited for solutions like Dropbox (which I
think is poorly designed to begin with, but that's offtopic). It
doesn't substitute of augment VFS caching. I suspect the VFS cache
works by caching the inodes in a frequently used directory entry, thus
optimizing calls like lstat() on them.
The correct solution for our problem is to get VFS to recognize our
repository as a unit: the repository is not a bunch of frequently-used
directory entries, but a frequently-used unit in itself. We need an
optimization that will work on recursively on a directory entry.
However, since the repository is a special usecase, I suspect adding
an rwatch() system call (or similar) will be necessary to register the
repository with VFS. The design of this feature should be transparent
to userland, and their filesystem calls will be optimized magically.
We certainly don't need something as fine-grained as inotify to
perform these optimizations: if the tree hash of a registered
repository changes frequently enough, we have to optimize operations
on that directory tree (recursively).
Inputs from btrfs/ vfs hackers would be appreciated. I'll take out
some time to look at them myself this week.
^ permalink raw reply
* Re: Proposal: sharing .git/config
From: Ramkumar Ramachandra @ 2013-03-18 9:00 UTC (permalink / raw)
To: Jeff King; +Cc: Duy Nguyen, Git List
In-Reply-To: <20130312085342.GA11340@sigill.intra.peff.net>
Jeff King wrote:
> I don't think you can avoid the 3-step problem and retain the safety in
> the general case. Forgetting implementation details for a minute, you
> have either a 1-step system:
>
> 1. Fetch and start using config from the remote.
>
> which is subject to fetching and executing malicious config, or:
>
> 1. Fetch config from remote.
> 2. Inspect it.
> 3. Integrate it into the current config.
I don't understand your emphasis on step 2. Isn't the configuration
written by me? Why would it be malicious?
I've just started thinking about how to design something that will
allow us to share configuration elegantly [1]. Essentially, the
metadata repository will consist of *.layout files, one for each
repository to clone, containing the .git/config to write after cloning
that repository. So, a git.layout might look like:
[layout]
directory = git
[remote "origin"]
url = git://github.com/git/git
[remote "ram"]
url = git@github.com:artagnon/git
[remote "junio"]
url = git://github.com/gitster/git
As you can see the [layout] is a special section which will tell our
fetcher where to place the repository. Everything else is meant to be
inserted into the repository's .git/config. However, I can foresee a
problem in scaling: when I ask a specific directory like a/b/c to be
populated (equivalent of repo sync `a/b/c`), it'll have to parse the
layout.directory variable of all the .layout files, and this can be
slow. So, maybe we should have a special _manifest.layout listing all
the paths?
Further, I see this as a way to work with projects that would
otherwise require nested submodules like the Android project. What do
you think?
[1]: https://github.com/artagnon/src.layout
^ permalink raw reply
* Re: [RFC/PATCH] Introduce remote.pushdefault
From: Ramkumar Ramachandra @ 2013-03-18 9:02 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git List, Jonathan Nieder
In-Reply-To: <20130317054803.GB16070@sigill.intra.peff.net>
Jeff King wrote:
> So the push lookup list is (in order of precedence):
>
> 1. branch.*.pushremote
> 2. remote.pushdefault
> 3. branch.*.remote
> 4. remote.default
> 5. origin
>
> and it solves Junio's issue because the way to say "override my
> remote.pushdefault for this branch" is not to set "branch.*.remote", but
> to set "branch.*.pushremote".
Right, thanks for clearing that up Jeff. I'll re-roll with
remote.pushdefault overriding branch.<name>.remote.
Ram
^ permalink raw reply
* Re: [PATCH] push: Alias pushurl from push rewrites
From: Rob Hoelz @ 2013-03-18 10:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, josh
In-Reply-To: <7vd2uxppk0.fsf@alter.siamese.dyndns.org>
On 3/18/13 12:35 AM, Junio C Hamano wrote:
> Rob Hoelz <rob@hoelz.ro> writes:
>
>> git push currently doesn't consider pushInsteadOf when
>> using pushurl; this tests and fixes that.
>>
>> If you use pushurl with an alias that has a pushInsteadOf configuration
>> value, Git does not take advantage of it. For example:
>>
>> [url "git://github.com/"]
>> insteadOf = github:
>> [url "git://github.com/myuser/"]
>> insteadOf = mygithub:
>> [url "git@github.com:myuser/"]
>> pushInsteadOf = mygithub:
>> [remote "origin"]
>> url = github:organization/project
>> pushurl = mygithub:project
> Incomplete sentence? For example [this is an example configuration]
> and then what happens? Something like "with the sample
> configuration, 'git push origin' should follow pushurl and then turn
> it into X, but instead it ends up accessing Y".
>
> If there is no pushInsteadOf, does it still follow insteadOf? Is it
> tested already?
>
> Wouldn't you want to cover all the combinations to negative cases
> (i.e. making sure the codepath to support a new case does not affect
> behaviour of the code outside the new case)? A remote with and
> without pushurl (two combinations) and a pseudo URL scheme with and
> without pushInsteadOf (again, two combinations) will give you four
> cases.
>
>
> Thanks.
Sorry; that's a good point. With the above configuration, the following
fails:
$ git push origin master
With the following message:
fatal: remote error:
You can't push to git://github.com/myuser/project.git
Use git@github.com:myuser/project.git
So you can see that pushurl is being followed (it's not attempting to
push to git://github.com/organization/project), but insteadOf values are
being used as opposed to pushInsteadOf values for expanding the pushurl
alias.
I haven't tried without pushInsteadOf; I will add a test for it later
today. I assume that if pushInsteadOf isn't found, insteadOf should be
used? I will also consider the other test cases you described.
Thanks again for the feedback!
>
>> Signed-off-by: Rob Hoelz <rob@hoelz.ro>
>> ---
>> remote.c | 2 +-
>> t/t5516-fetch-push.sh | 20 ++++++++++++++++++++
>> 2 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/remote.c b/remote.c
>> index ca1f8f2..de7a915 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -465,7 +465,7 @@ static void alias_all_urls(void)
>> if (!remotes[i])
>> continue;
>> for (j = 0; j < remotes[i]->pushurl_nr; j++) {
>> - remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
>> + remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites_push);
>> }
>> add_pushurl_aliases = remotes[i]->pushurl_nr == 0;
>> for (j = 0; j < remotes[i]->url_nr; j++) {
>> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
>> index b5417cc..272e225 100755
>> --- a/t/t5516-fetch-push.sh
>> +++ b/t/t5516-fetch-push.sh
>> @@ -244,6 +244,26 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
>> )
>> '
>>
>> +test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf does rewrite in this case)' '
>> + mk_empty &&
>> + TRASH="$(pwd)/" &&
>> + mkdir ro &&
>> + mkdir rw &&
>> + git init --bare rw/testrepo &&
>> + git config "url.file://$TRASH/ro/.insteadOf" ro: &&
>> + git config "url.file://$TRASH/rw/.pushInsteadOf" rw: &&
>> + git config remote.r.url ro:wrong &&
>> + git config remote.r.pushurl rw:testrepo &&
>> + git push r refs/heads/master:refs/remotes/origin/master &&
>> + (
>> + cd rw/testrepo &&
>> + r=$(git show-ref -s --verify refs/remotes/origin/master) &&
>> + test "z$r" = "z$the_commit" &&
>> +
>> + test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
>> + )
>> +'
>> +
>> test_expect_success 'push with matching heads' '
>>
>> mk_test heads/master &&
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox