* [RFC/PATCH 1/3] remote: use a local variable in match_push_refs()
From: Felipe Contreras @ 2012-02-17 19:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1329505957-24595-1-git-send-email-felipe.contreras@gmail.com>
So that we can reuse src later on.
Will be useful in next patches.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
remote.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/remote.c b/remote.c
index 73a3809..55d68d1 100644
--- a/remote.c
+++ b/remote.c
@@ -1157,7 +1157,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
int send_mirror = flags & MATCH_REFS_MIRROR;
int errs;
static const char *default_refspec[] = { ":", NULL };
- struct ref **dst_tail = tail_ref(dst);
+ struct ref *ref, **dst_tail = tail_ref(dst);
if (!nr_refspec) {
nr_refspec = 1;
@@ -1167,14 +1167,14 @@ int match_push_refs(struct ref *src, struct ref **dst,
errs = match_explicit_refs(src, *dst, &dst_tail, rs, nr_refspec);
/* pick the remainder */
- for ( ; src; src = src->next) {
+ for (ref = src; ref; ref = ref->next) {
struct ref *dst_peer;
const struct refspec *pat = NULL;
char *dst_name;
- if (src->peer_ref)
+ if (ref->peer_ref)
continue;
- pat = check_pattern_match(rs, nr_refspec, src);
+ pat = check_pattern_match(rs, nr_refspec, ref);
if (!pat)
continue;
@@ -1184,13 +1184,14 @@ int match_push_refs(struct ref *src, struct ref **dst,
* including refs outside refs/heads/ hierarchy, but
* that does not make much sense these days.
*/
- if (!send_mirror && prefixcmp(src->name, "refs/heads/"))
+ if (!send_mirror && prefixcmp(ref->name, "refs/heads/"))
continue;
- dst_name = xstrdup(src->name);
+ dst_name = xstrdup(ref->name);
+
} else {
const char *dst_side = pat->dst ? pat->dst : pat->src;
- if (!match_name_with_pattern(pat->src, src->name,
+ if (!match_name_with_pattern(pat->src, ref->name,
dst_side, &dst_name))
die("Didn't think it matches any more");
}
@@ -1211,9 +1212,9 @@ int match_push_refs(struct ref *src, struct ref **dst,
/* Create a new one and link it */
dst_peer = make_linked_ref(dst_name, &dst_tail);
- hashcpy(dst_peer->new_sha1, src->new_sha1);
+ hashcpy(dst_peer->new_sha1, ref->new_sha1);
}
- dst_peer->peer_ref = copy_ref(src);
+ dst_peer->peer_ref = copy_ref(ref);
dst_peer->force = pat->force;
free_name:
free(dst_name);
--
1.7.9.1
^ permalink raw reply related
* [RFC/PATCH 2/3] remote: reorganize check_pattern_match()
From: Felipe Contreras @ 2012-02-17 19:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1329505957-24595-1-git-send-email-felipe.contreras@gmail.com>
There's a lot of code that can be consolidated there, and will be useful
for next patches.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
remote.c | 59 ++++++++++++++++++++++++++++++-----------------------------
1 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/remote.c b/remote.c
index 55d68d1..019aafc 100644
--- a/remote.c
+++ b/remote.c
@@ -1110,10 +1110,11 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
return errs;
}
-static const struct refspec *check_pattern_match(const struct refspec *rs,
- int rs_nr,
- const struct ref *src)
+static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref *ref,
+ int send_mirror, const struct refspec **ret_pat)
{
+ const struct refspec *pat;
+ char *name;
int i;
int matching_refs = -1;
for (i = 0; i < rs_nr; i++) {
@@ -1123,14 +1124,31 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
continue;
}
- if (rs[i].pattern && match_name_with_pattern(rs[i].src, src->name,
- NULL, NULL))
- return rs + i;
+ if (rs[i].pattern) {
+ const char *dst_side = rs[i].dst ? rs[i].dst : rs[i].src;
+ if (match_name_with_pattern(rs[i].src, ref->name, dst_side, &name)) {
+ matching_refs = i;
+ break;
+ }
+ }
}
- if (matching_refs != -1)
- return rs + matching_refs;
- else
+ if (matching_refs == -1)
return NULL;
+
+ pat = rs + matching_refs;
+ if (pat->matching) {
+ /*
+ * "matching refs"; traditionally we pushed everything
+ * including refs outside refs/heads/ hierarchy, but
+ * that does not make much sense these days.
+ */
+ if (!send_mirror && prefixcmp(ref->name, "refs/heads/"))
+ return NULL;
+ name = xstrdup(ref->name);
+ }
+ if (ret_pat)
+ *ret_pat = pat;
+ return name;
}
static struct ref **tail_ref(struct ref **head)
@@ -1171,36 +1189,19 @@ int match_push_refs(struct ref *src, struct ref **dst,
struct ref *dst_peer;
const struct refspec *pat = NULL;
char *dst_name;
+
if (ref->peer_ref)
continue;
- pat = check_pattern_match(rs, nr_refspec, ref);
- if (!pat)
+ dst_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, &pat);
+ if (!dst_name)
continue;
- if (pat->matching) {
- /*
- * "matching refs"; traditionally we pushed everything
- * including refs outside refs/heads/ hierarchy, but
- * that does not make much sense these days.
- */
- if (!send_mirror && prefixcmp(ref->name, "refs/heads/"))
- continue;
- dst_name = xstrdup(ref->name);
-
-
- } else {
- const char *dst_side = pat->dst ? pat->dst : pat->src;
- if (!match_name_with_pattern(pat->src, ref->name,
- dst_side, &dst_name))
- die("Didn't think it matches any more");
- }
dst_peer = find_ref_by_name(*dst, dst_name);
if (dst_peer) {
if (dst_peer->peer_ref)
/* We're already sending something to this ref. */
goto free_name;
-
} else {
if (pat->matching && !(send_all || send_mirror))
/*
--
1.7.9.1
^ permalink raw reply related
* [RFC/PATCH 3/3] push: add 'prune' option
From: Felipe Contreras @ 2012-02-17 19:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1329505957-24595-1-git-send-email-felipe.contreras@gmail.com>
This will allow us to remove refs from the remote that have been removed
locally.
It's useful to conveniently synchronize all the local branches to
certain remote.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/push.c | 2 ++
remote.c | 29 ++++++++++++++++++++++++++---
remote.h | 3 ++-
transport.c | 2 ++
transport.h | 1 +
5 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 35cce53..46b99b1 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -261,6 +261,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
TRANSPORT_PUSH_SET_UPSTREAM),
OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
+ OPT_BIT('p', "prune", &flags, "prune locally removed refs",
+ TRANSPORT_PUSH_PRUNE),
OPT_END()
};
diff --git a/remote.c b/remote.c
index 019aafc..0900bb5 100644
--- a/remote.c
+++ b/remote.c
@@ -1111,7 +1111,7 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
}
static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref *ref,
- int send_mirror, const struct refspec **ret_pat)
+ int send_mirror, int dir, const struct refspec **ret_pat)
{
const struct refspec *pat;
char *name;
@@ -1126,7 +1126,12 @@ static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref
if (rs[i].pattern) {
const char *dst_side = rs[i].dst ? rs[i].dst : rs[i].src;
- if (match_name_with_pattern(rs[i].src, ref->name, dst_side, &name)) {
+ int match;
+ if (dir == 0)
+ match = match_name_with_pattern(rs[i].src, ref->name, dst_side, &name);
+ else
+ match = match_name_with_pattern(dst_side, ref->name, rs[i].src, &name);
+ if (match) {
matching_refs = i;
break;
}
@@ -1173,6 +1178,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
struct refspec *rs;
int send_all = flags & MATCH_REFS_ALL;
int send_mirror = flags & MATCH_REFS_MIRROR;
+ int send_prune = flags & MATCH_REFS_PRUNE;
int errs;
static const char *default_refspec[] = { ":", NULL };
struct ref *ref, **dst_tail = tail_ref(dst);
@@ -1193,7 +1199,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
if (ref->peer_ref)
continue;
- dst_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, &pat);
+ dst_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, 0, &pat);
if (!dst_name)
continue;
@@ -1220,6 +1226,23 @@ int match_push_refs(struct ref *src, struct ref **dst,
free_name:
free(dst_name);
}
+ if (send_prune) {
+ /* check for missing refs on the remote */
+ for (ref = *dst; ref; ref = ref->next) {
+ char *src_name;
+
+ if (ref->peer_ref)
+ /* We're already sending something to this ref. */
+ continue;
+
+ src_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, 1, NULL);
+ if (src_name) {
+ if (!find_ref_by_name(src, src_name))
+ ref->peer_ref = try_explicit_object_name("");
+ free(src_name);
+ }
+ }
+ }
if (errs)
return -1;
return 0;
diff --git a/remote.h b/remote.h
index b395598..341142c 100644
--- a/remote.h
+++ b/remote.h
@@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
enum match_refs_flags {
MATCH_REFS_NONE = 0,
MATCH_REFS_ALL = (1 << 0),
- MATCH_REFS_MIRROR = (1 << 1)
+ MATCH_REFS_MIRROR = (1 << 1),
+ MATCH_REFS_PRUNE = (1 << 2),
};
/* Reporting of tracking info */
diff --git a/transport.c b/transport.c
index cac0c06..c20267c 100644
--- a/transport.c
+++ b/transport.c
@@ -1028,6 +1028,8 @@ int transport_push(struct transport *transport,
match_flags |= MATCH_REFS_ALL;
if (flags & TRANSPORT_PUSH_MIRROR)
match_flags |= MATCH_REFS_MIRROR;
+ if (flags & TRANSPORT_PUSH_PRUNE)
+ match_flags |= MATCH_REFS_PRUNE;
if (match_push_refs(local_refs, &remote_refs,
refspec_nr, refspec, match_flags)) {
diff --git a/transport.h b/transport.h
index 059b330..5d30328 100644
--- a/transport.h
+++ b/transport.h
@@ -101,6 +101,7 @@ struct transport {
#define TRANSPORT_PUSH_MIRROR 8
#define TRANSPORT_PUSH_PORCELAIN 16
#define TRANSPORT_PUSH_SET_UPSTREAM 32
+#define TRANSPORT_PUSH_PRUNE 64
#define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
--
1.7.9.1
^ permalink raw reply related
* Mixing and matching multiple projects
From: Junio C Hamano @ 2012-02-17 19:19 UTC (permalink / raw)
To: git
Imagine that there is an open source embedded appliance project "xyzzy"
that consists of two submodules and the top-level superproject that binds
them and serves as the scaffolding, perhaps organized like this:
/s/xyzzy/.git -- the superproject
/s/xyzzy/os/.git -- the "os" submodule
/s/xyzzy/u/.git -- the "userspace" submodule
The "os" submodule may contain various subdirectories, among which there
may be a device drivers directory. Further imagine that the Makefile in
that directory, /s/xyzzy/os/drivers/Makefile, is written in such a way
that it can run make in any subdirectory of drivers/ and can link with
resulting *.o files in these subdirectories.
Now suppose somebody wanted to add an add-on device driver for frotz
device that does not appear in the open source tree. Further suppose that
the open source folks do not want to accept support for "frotz" driver in
their tree for whatever reason.
At the working tree level, he can add /s/xyzzy/os/drivers/frotz/ directory
and populate its sources there. Everything else is already set up to build
and link to it properly. But how would he add that directory to git?
He could fork the /s/xyzzy/os/.git submodule and add "drivers/frotz/"
directory, and maintain his own fork of the submodule, which in turn means
that he has to fork the top-level superproject /s/xyzzy/.git because its
tree has to point at a commit in /s/xyzzy/os/.git repository that has the
"drivers/frotz" directory in it, which is different from the open source
version. A proprietary fork has to happen somewhere; that is a given.
But I wonder if we can do without forking the /s/xyzzy/os/.git submodule?
If we *could* do this:
cd /s/xyzzy
mkdir os/drivers/frotz && populate the directory with sources
git add os/drivers/frotz ;# to the top-level superproject!!
to add the "frotz driver" submodule directly to the superproject, then we
could leave /s/xyzzy/os/.git intact, letting it follow the open source
world. Because the superproject must be forked in order to keep track of
what happens in the appliance that supports the "frotz" driver anyway,
this could result in the minimum amount of forking from the end user's
point of view.
People who know the Git data structure of course can immediately see that
this is not supported (and I strongly suspect unsupportable, but my
suspicion has a history of being wrong from time to time, and that is why
I am sending this message). The index at /s/xyzzy/.git/index has to be
able to record "os" as a gitlink and "os/drivers/frotz/frotz.c" as a
regular blob at the same time, which is a D/F conflict.
Even if you modify the index D/F conflict check to allow this, you cannot
write the top-level tree object for the superproject, as "os" needs to be
recorded as a gitlink to bind the /s/xyzzy/os/.git submodule, while you
also have to have another "os" in the same top-level tree object as a tree
object that has a single entry "drivers" (which is a tree) in it, under
which all the "os/drivers/frotz" subdirectory hang. All tree traversal
code would be very unhappy to see such a tree with "duplicate" entry,
including the unpack-trees machinery used for merging and switching
branches, connectivity machinery used for fsck, object enumeration, and
object transfer. I imagine that it *could* be done, but it won't be a
mere two-weekend hack; it has to first destroy pretty much everything in
the current codebase and rebuild it to add such a support.
The "frotz" submodule could be added as /s/xyzzy/frotz/.git and the build
infrastructure at /s/xyzzy/.git (which we are going to fork anyway) could
be tweaked so that it creates a symbolic link in /s/xyzzy/os/drivers to
point there, pretending that there is a 'frotz' subdirectory. While that
would certainly be a workaround, I am wondering if people who have used
submodules and/or nested projects more than I have better solutions to
this puzzle.
Comments?
^ permalink raw reply
* Re: How to use git attributes to configure server-side checks?
From: Junio C Hamano @ 2012-02-17 19:26 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git discussion list
In-Reply-To: <4F3E9F86.3070802@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> I was just alerted by Scott Chacon's blog [1] to the fact that one can
> set GIT_INDEX_FILE to an arbitrary filename, thereby causing the index
> to be read/written from that file instead of $GIT_DIR/index.
That's very old fashioned. For almost five years, the preferred way to
say that has been "git read-tree --index-output=file" ;-)
But what you outlined should work.
^ permalink raw reply
* Re: Mixing and matching multiple projects
From: Junio C Hamano @ 2012-02-17 19:49 UTC (permalink / raw)
To: git
In-Reply-To: <7vhayptght.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> But I wonder if we can do without forking the /s/xyzzy/os/.git submodule?
>
> If we *could* do this:
>
> cd /s/xyzzy
> mkdir os/drivers/frotz && populate the directory with sources
A crucial step was missing here for my story to make *any* sense. Sorry.
A step to create a repository at /s/xyzzy/os/drivers/frotz/.git must be
here, like this:
(cd os/drivers/frotz && git init && git add . && git commit)
And then that is added as a submodule to the superproject.
> git add os/drivers/frotz ;# to the top-level superproject!!
>
> to add the "frotz driver" submodule directly to the superproject, then we
> could leave /s/xyzzy/os/.git intact, letting it follow the open source
> world. Because the superproject must be forked in order to keep track of
> what happens in the appliance that supports the "frotz" driver anyway,
> this could result in the minimum amount of forking from the end user's
> point of view.
>
> People who know the Git data structure of course can immediately see that
> this is not supported (and I strongly suspect unsupportable, but my
> suspicion has a history of being wrong from time to time, and that is why
> I am sending this message). The index at /s/xyzzy/.git/index has to be
> able to record "os" as a gitlink and "os/drivers/frotz/frotz.c" as a
> regular blob at the same time, which is a D/F conflict.
>
> Even if you modify the index D/F conflict check to allow this, you cannot
> write the top-level tree object for the superproject, as "os" needs to be
> recorded as a gitlink to bind the /s/xyzzy/os/.git submodule, while you
> also have to have another "os" in the same top-level tree object as a tree
> object that has a single entry "drivers" (which is a tree) in it, under
> which all the "os/drivers/frotz" subdirectory hang. All tree traversal
> code would be very unhappy to see such a tree with "duplicate" entry,
> including the unpack-trees machinery used for merging and switching
> branches, connectivity machinery used for fsck, object enumeration, and
> object transfer. I imagine that it *could* be done, but it won't be a
> mere two-weekend hack; it has to first destroy pretty much everything in
> the current codebase and rebuild it to add such a support.
>
> The "frotz" submodule could be added as /s/xyzzy/frotz/.git and the build
> infrastructure at /s/xyzzy/.git (which we are going to fork anyway) could
> be tweaked so that it creates a symbolic link in /s/xyzzy/os/drivers to
> point there, pretending that there is a 'frotz' subdirectory. While that
> would certainly be a workaround, I am wondering if people who have used
> submodules and/or nested projects more than I have better solutions to
> this puzzle.
>
> Comments?
^ permalink raw reply
* Re: How to use git attributes to configure server-side checks?
From: Junio C Hamano @ 2012-02-17 19:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git discussion list
In-Reply-To: <7vd39dtg5t.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> I was just alerted by Scott Chacon's blog [1] to the fact that one can
>> set GIT_INDEX_FILE to an arbitrary filename, thereby causing the index
>> to be read/written from that file instead of $GIT_DIR/index.
>
> That's very old fashioned. For almost five years, the preferred way to
> say that has been "git read-tree --index-output=file" ;-)
Ah, I take that back.
There is any benefit to be had by using "read-tree --index-output" in the
way used in your check-attr example. Setting up a single GIT_INDEX_FILE
and using it throughout as you did is indeed much more preferrable.
> But what you outlined should work.
^ permalink raw reply
* Re: Mixing and matching multiple projects
From: Seth Robertson @ 2012-02-17 20:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhayptght.fsf@alter.siamese.dyndns.org>
In message <7vhayptght.fsf@alter.siamese.dyndns.org>, Junio C Hamano writes:
/s/xyzzy/.git -- the superproject
/s/xyzzy/os/.git -- the "os" submodule
/s/xyzzy/u/.git -- the "userspace" submodule
At the working tree level, he can add /s/xyzzy/os/drivers/frotz/ directory
and populate its sources there. Everything else is already set up to build
and link to it properly. But how would he add that directory to git?
But I wonder if we can do without forking the /s/xyzzy/os/.git submodule?
I am wondering if people who have used submodules and/or nested
projects more than I have better solutions to this puzzle.
Using gitslave [http://gitslave.sf.net] instead of git-submodules, you
can *almost* get there.
Specifically, gitslave allows the superproject /s/xyzzy to attach a
subproject at /s/xyzzy/os/drivers/frotz (either recursively through
/os which you don't want to do or directly).
However, the problem is the .gitignore in the os subproject. If we
didn't have some way to ignore drivers/frotz, various git commands
would get annoyed. Adding it in the os subproject requires forking
which you said you didn't want to do. However, you could use
core.excludesfile in the os subproject to ignore the necessary file.
gitslave doesn't support automatically using core.excludesfile (and
indeed would automatically make an entry in os/.gitignore when you set
up the hierarchy). However, it isn't a lot of work to revert that
.gitignore commit and set up core.excludesfile; but setting up
core.excludesfile on every clone might get annoying. It probably
wouldn't be too difficult to tearch gitslave how to do that
automagically.
Is using this third party tool and making the necessary changes or
manually doing the work after every clone worth it? Do you need the
git-submodule semantics of freezing the subprojects at a particular
SHA instead of floating at the tip of a branch? Are the branches used
between xyzzy and os desynchronized? Only you can judge. Actually it
seems possible that you could use gitslave for os/drivers/frotz and
git-submodules for os. Whether that is wise…
I would be open to moving gitslave into contrib similar to what
git-subtree is under the process of doing, but gitslave would
certainly need some cleanup since it grew rather organically.
-Seth Robertson
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Jeff King @ 2012-02-17 20:37 UTC (permalink / raw)
To: Piotr Krukowiecki; +Cc: Thomas Rast, Git Mailing List, Nguyen Thai Ngoc Duy
In-Reply-To: <CAA01Csq6vSekW=Fa236bB0H3LVtN43Gb2aLMVE+A1wVyUqYJ7A@mail.gmail.com>
On Fri, Feb 17, 2012 at 06:19:06PM +0100, Piotr Krukowiecki wrote:
> "git update-index --refresh" with dropped cache took
> real 0m3.726s
> user 0m0.024s
> sys 0m0.404s
> [...]
> The diff-index after dropping cache takes
> real 0m14.095s
> user 0m0.268s
> sys 0m0.564s
OK, that suggests to me that the real culprit is the I/O we spend in
accessing the object db, since that is the main I/O that happens in the
second command but not the first.
> > Mostly reading (we keep a sorted index and access the packfiles via
> > mmap, so we only touch the pages we need). But you're also paying to
> > lstat() the directory tree, too. And you're paying to load (probably)
> > the whole index into memory, although it's relatively compact compared
> > to the actual file data.
>
> If the index is the objects/pack/*.idx files than it's 21MB
Yes, that's it. Though we don't necessarily read the whole thing. The
sorted list of sha1s is only a part of that. And we mmap and
binary-search that, so we only have to fault in pages that are actually
used in our binary search.
However, we're faulting in random pages of the index in series, so it
may actually have a lot of latency. You can see how expensive the I/O on
the index is with something like this:
[whole operation, for reference]
$ sudo sh -c 'echo 3 >/proc/sys/vm/drop_caches'
$ time git diff-index HEAD
real 0m2.636s
user 0m0.248s
sys 0m0.392s
[prime the cache with just the index]
$ sudo sh -c 'echo 3 >/proc/sys/vm/drop_caches'
$ time cat .git/objects/pack/*.idx >/dev/null
real 0m0.288s
user 0m0.000s
sys 0m0.028s
$ time git diff-index HEAD
real 0m2.175s
user 0m0.272s
sys 0m0.320s
So roughly 20% of the I/O time in my case went to faulting in the index.
You could pre-fault in the index, which would give the OS a chance to do
read-ahead caching. You can see that the combined cat and diff-index
times are still lower than the raw diff-index time. You could also do
them in parallel, but that will create some additional seeks as the
threads fight for the disk, but may be a win in the long run because we
can read bigger chunks. You can roughly simulate it by running the "cat"
and the "diff-index" above in parallel. I get:
real 0m2.464s
user 0m0.284s
sys 0m0.372s
which is almost exactly the same as doing them separately (though note
that this is on an SSD, so seeking is very cheap).
But the bulk of the time still goes to actually retrieving the object
data, so that's probably a more interesting area to focus, anyway (and
if we can reduce object accesses, we reduce their lookup, too :) ).
> If I understand correctly, you only need to compute sha1 on the
> workdir files and compare it with sha1 files recorded in index/gitdir.
> It seems that to get the sha1 from index/gitdir I need to read the
> packfiles? Maybe it'd be possible to cache/index it somehow, for
> example in separate and smaller file?
There are two diffs going on in "git status". One is a comparison
between index and worktree. In that one, you need to lstat each file to
make sure the cached sha1 we have in the index is up to date. Assuming
it is, you don't need to touch the file data at all. Then you compare
that sha1 to the stage 0 sha1 (i.e., what we typically think of as
"staged for commit"). If they match, you don't need to do more work.
But the expensive diff-index we've been doing above is comparing the
index to the HEAD tree. And doing that is a little trickier. The index
is a flat list of files with their sha1s. But the HEAD tree is stored
hierarchically. So to get the sha1 of foo/bar/baz, we have to access the
root tree object, find the "foo" entry, access its tree object, find the
"bar" entry, access its tree object, and then find the "baz" entry. Then
we compare the sha1 of the "baz" entry to what's in the index.
So what's where your I/O comes from: accessing each of the tree objects.
And that fact that it isn't just "compare the HEAD and index sha1s" is
that the index is stored as a list of flat files.
That being said, we do have an index extension to store the tree sha1 of
whole directories (i.e., we populate it when we write a whole tree or
subtree into the index from the object db, and it becomes invalidated
when a file becomes modified). This optimization is used by things like
"git commit" to avoid having to recreate the same sub-trees over and
over when creating tree objects from the index. But we could also use it
here to avoid having to even read the sub-tree objects from the object
db.
> No, it's ext4 and the disk Seagate Barracuda 7200.12 500GB, as it
> reads on the cover :)
>
> But IMO faster disk won't help with this - times will be smaller, but
> you'll still have to read the same data, so the subdir times will be
> just 2x faster than whole repo, won't it? So maybe in my case it will
> go down to e.g. 2s on subdir, but for someone with larger repository
> it will still be 10s...
Sure. But a certain amount of I/O is going to be unavoidable to get the
answer to your question. So you will never be able to achieve the
warm-cache case. I'm not saying we can't improve (e.g., I think the
index extension thing I mentioned above is a promising approach). But we
have to be realistic about what will make things faster; if I/O is your
problem, faster disk is one possible solution (especially because some
of this is related to seeking and latency, an SSD is a nice improvement
for cold-cache times).
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 3/3] push: add 'prune' option
From: Jeff King @ 2012-02-17 22:25 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano
In-Reply-To: <1329505957-24595-4-git-send-email-felipe.contreras@gmail.com>
On Fri, Feb 17, 2012 at 09:12:37PM +0200, Felipe Contreras wrote:
> This will allow us to remove refs from the remote that have been removed
> locally.
>
> It's useful to conveniently synchronize all the local branches to
> certain remote.
Thanks for picking up this topic again.
There's one subtlety in the motivation above that you may find helpful
when you end up writing the documentation: "push --mirror" already does
this sort of pruning, but it _also_ implies that we are pushing
"refs/*:refs/*". So this is really about giving access to the pruning
half, but still being able to use custom refspecs. So the features
together might end up being explained something like:
--prune::
... prune things that no longer exist locally ...
--mirror::
... turn on --prune, and also match all refs ...
At least that is my understanding of how the code is meant to work.
> builtin/push.c | 2 ++
> remote.c | 29 ++++++++++++++++++++++++++---
> remote.h | 3 ++-
> transport.c | 2 ++
> transport.h | 1 +
> 5 files changed, 33 insertions(+), 4 deletions(-)
I've just given a quick read to the patches so far, but I did notice
this:
> static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref *ref,
> - int send_mirror, const struct refspec **ret_pat)
> + int send_mirror, int dir, const struct refspec **ret_pat)
The "dir" flag looks like it is meant to be short for "direction". But
the callers only pass 0 or 1. I'm not clear which direction is which.
Either symbolic constants for directions, or perhaps giving it a more
boolean name like "match_to_dst" might make it more clear.
I'll try to take a closer look later tonight.
-Peff
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Junio C Hamano @ 2012-02-17 22:25 UTC (permalink / raw)
To: Jeff King
Cc: Piotr Krukowiecki, Thomas Rast, Git Mailing List,
Nguyen Thai Ngoc Duy
In-Reply-To: <20120217203755.GA30114@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> That being said, we do have an index extension to store the tree sha1 of
> whole directories (i.e., we populate it when we write a whole tree or
> subtree into the index from the object db, and it becomes invalidated
> when a file becomes modified). This optimization is used by things like
> "git commit" to avoid having to recreate the same sub-trees over and
> over when creating tree objects from the index. But we could also use it
> here to avoid having to even read the sub-tree objects from the object
> db.
Like b65982b (Optimize "diff-index --cached" using cache-tree, 2009-05-20)
perhaps?
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Jeff King @ 2012-02-17 22:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Piotr Krukowiecki, Thomas Rast, Git Mailing List,
Nguyen Thai Ngoc Duy
In-Reply-To: <7vaa4hrtbe.fsf@alter.siamese.dyndns.org>
On Fri, Feb 17, 2012 at 02:25:25PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > That being said, we do have an index extension to store the tree sha1 of
> > whole directories (i.e., we populate it when we write a whole tree or
> > subtree into the index from the object db, and it becomes invalidated
> > when a file becomes modified). This optimization is used by things like
> > "git commit" to avoid having to recreate the same sub-trees over and
> > over when creating tree objects from the index. But we could also use it
> > here to avoid having to even read the sub-tree objects from the object
> > db.
>
> Like b65982b (Optimize "diff-index --cached" using cache-tree, 2009-05-20)
> perhaps?
That's what I get for speaking before running "git log".
So yeah, we may be about as reasonably fast as we can go. Or maybe that
optimization isn't kicking in for some reason. I think going further
would require Piotr to do more profiling.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 1/3] remote: use a local variable in match_push_refs()
From: Junio C Hamano @ 2012-02-17 22:31 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <1329505957-24595-2-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> So that we can reuse src later on.
>
> Will be useful in next patches.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
If the 2/3 were much less complex, it might have made sense to squash this
into that, but I think it makes it easier to see what is happening if an
obvious and safe no-op change like this is done as a separate patch like
this series did.
Looking good so far.
^ permalink raw reply
* Re: [RFC/PATCH 2/3] remote: reorganize check_pattern_match()
From: Junio C Hamano @ 2012-02-17 22:34 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <1329505957-24595-3-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> There's a lot of code that can be consolidated there, and will be useful
> for next patches.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> remote.c | 59 ++++++++++++++++++++++++++++++-----------------------------
> 1 files changed, 30 insertions(+), 29 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 55d68d1..019aafc 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1110,10 +1110,11 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
> return errs;
> }
>
> -static const struct refspec *check_pattern_match(const struct refspec *rs,
> - int rs_nr,
> - const struct ref *src)
> +static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref *ref,
> + int send_mirror, const struct refspec **ret_pat)
> {
For a change that not just adds parameters but removes an existing one,
this is way under-described with neither in-code comment nor log message.
So I'll have to think aloud in this review. Take it as a chance to learn
how the thought process of other people with lessor intelligence than you
have might work, and how to help those slower than you are ;-)
> + const struct refspec *pat;
> + char *name;
> int i;
> int matching_refs = -1;
> for (i = 0; i < rs_nr; i++) {
> @@ -1123,14 +1124,31 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
> continue;
> }
>
> - if (rs[i].pattern && match_name_with_pattern(rs[i].src, src->name,
> - NULL, NULL))
> - return rs + i;
> + if (rs[i].pattern) {
> + const char *dst_side = rs[i].dst ? rs[i].dst : rs[i].src;
> + if (match_name_with_pattern(rs[i].src, ref->name, dst_side, &name)) {
> + matching_refs = i;
> + break;
We used to discard what match_name_with_pattern() finds out by matching a
wildcard refspec against the ref by passing two NULLs. This updates the
code to capture what destination ref ref->name is mapped to, by using the
same logic as the original and only caller, i.e. 'foo' without destination
maps to the same 'foo' destination, 'foo:bar' maps to the named 'bar'.
This function is not used by fetching side of the codepath, so we do not
have to worry about its need to use different dst_side selection logic
(i.e. 'foo' without destination maps to "do not store anywhere other than
FETCH_HEAD"). Good.
> + }
> + }
> }
> -...
> + if (matching_refs == -1)
> return NULL;
> +
> + pat = rs + matching_refs;
> + if (pat->matching) {
> + /*
> + * "matching refs"; traditionally we pushed everything
> + * including refs outside refs/heads/ hierarchy, but
> + * that does not make much sense these days.
> + */
> + if (!send_mirror && prefixcmp(ref->name, "refs/heads/"))
> + return NULL;
> + name = xstrdup(ref->name);
> + }
So you are moving some code from what the sole caller of this function
does after calling us, and that is where the new parameters come from.
And by doing so, you do not have to run the same match_name_with_pattern()
again. OK.
> + if (ret_pat)
> + *ret_pat = pat;
> + return name;
> }
You did not initialize name to anything at the beginning, but if the
earlier match_name_with_pattern() didn't find anything, we could only come
here after matching_refs is set by the if (rs[i].matching) codepath; by
the time we come here, we would have xstrdup(ref->name) in name, so we
would never return a garbage pointer to the caller. OK.
> static struct ref **tail_ref(struct ref **head)
> @@ -1171,36 +1189,19 @@ int match_push_refs(struct ref *src, struct ref **dst,
> struct ref *dst_peer;
> const struct refspec *pat = NULL;
> char *dst_name;
> +
> if (ref->peer_ref)
> continue;
>
> - pat = check_pattern_match(rs, nr_refspec, ref);
> - if (!pat)
> + dst_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, &pat);
> + if (!dst_name)
> continue;
>
> - if (pat->matching) {
> - /*
> - * "matching refs"; traditionally we pushed everything
> - * including refs outside refs/heads/ hierarchy, but
> - * that does not make much sense these days.
> - */
> - if (!send_mirror && prefixcmp(ref->name, "refs/heads/"))
> - continue;
> - dst_name = xstrdup(ref->name);
> -
> -
> - } else {
> - const char *dst_side = pat->dst ? pat->dst : pat->src;
> - if (!match_name_with_pattern(pat->src, ref->name,
> - dst_side, &dst_name))
> - die("Didn't think it matches any more");
> - }
> dst_peer = find_ref_by_name(*dst, dst_name);
> if (dst_peer) {
> if (dst_peer->peer_ref)
> /* We're already sending something to this ref. */
> goto free_name;
> -
> } else {
> if (pat->matching && !(send_all || send_mirror))
> /*
OK, it is easy to tell that the patch is trivially correct, once a reader
figures out that the patch is really about:
Move code to check_pattern_match() from its sole caller to make it
unnecessary to call match_name_with_pattern() twice.
Saying so in the log message would have prepared the reader, instead of
the "There's a lot of code that can be consolidated there." which does not
give hints on what to look for in the patch.
Also this changes the semantics (because it changed the meaning of its
return value) of check_pattern_match() so much that it would deserve a
rename, which I will address in my review of 3/3.
Otherwise this step looks good.
^ permalink raw reply
* Re: [RFC/PATCH 3/3] push: add 'prune' option
From: Junio C Hamano @ 2012-02-17 22:34 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <1329505957-24595-4-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> This will allow us to remove refs from the remote that have been removed
> locally.
Can you enhance this a bit more to summarize the gist of what the semantic
of this new feature is, perhaps like this:
After pushing refs, "git push --prune" will remove refs from the
remote that existed before the push and would have been pushed
from us if we had some local refs that would have matched the
refspecs used. For example,
$ git push --prune remote refs/heads/*:refs/remotes/repo1/*
will push all local branches in our repository to refs with
corresponding names under refs/remotes/repo1/ at the remote, and
removes remote's refs in refs/remotes/repo1/ that no longer have
corresponding local branches in our repository. The refs at the
remote outside refs/remotes/repo1/ are not affected.
In order to alley the worries raised in the previous discussion, something
to the effect of the last sentence above is crucial to have, I would think.
> It's useful to conveniently synchronize all the local branches to
> certain remote.
When an update to this patch comes with a documentation update to
illustrate how to exercise this useful feature with an example, it will
start to make sense to write "this is useful" in the log message. I know
you haven't gotten around the documentation part while the patch is marked
as RFC, and that is OK.
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> builtin/push.c | 2 ++
> remote.c | 29 ++++++++++++++++++++++++++---
> remote.h | 3 ++-
> transport.c | 2 ++
> transport.h | 1 +
> 5 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 35cce53..46b99b1 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -261,6 +261,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
> OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
> TRANSPORT_PUSH_SET_UPSTREAM),
> OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
> + OPT_BIT('p', "prune", &flags, "prune locally removed refs",
> + TRANSPORT_PUSH_PRUNE),
Please refrain from squatting on a short-and-sweet one letter option
before this new feature proves to be popular and useful in a few cycles,
especially when there already is a long option that begins with 'p'.
> OPT_END()
> };
>
> diff --git a/remote.c b/remote.c
> index 019aafc..0900bb5 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1111,7 +1111,7 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
> }
>
> static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref *ref,
> - int send_mirror, const struct refspec **ret_pat)
> + int send_mirror, int dir, const struct refspec **ret_pat)
Can we name this a bit better? I first thought "Huh? directory?", and had
to scratch my head, wondering if it is an offset into refs/heads/* string
or something....
> {
> const struct refspec *pat;
> char *name;
> @@ -1126,7 +1126,12 @@ static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref
>
> if (rs[i].pattern) {
> const char *dst_side = rs[i].dst ? rs[i].dst : rs[i].src;
> - if (match_name_with_pattern(rs[i].src, ref->name, dst_side, &name)) {
> + int match;
> + if (dir == 0)
> + match = match_name_with_pattern(rs[i].src, ref->name, dst_side, &name);
> + else
> + match = match_name_with_pattern(dst_side, ref->name, rs[i].src, &name);
....until the code told us that it is some sort of direction of the
matching. A symbolic constant or two would be even better.
Originally this funcion was fed a list of refs in the source (i.e. on our
end, as this is only used in 'push') and matched them against the source
side of the refspec, rs[i].src, to see under what name the destination
side will store it (i.e. give dst_side as value to find out the result in
&name). This patch adds a new caller, who feeds a list of refs in the
destination (i.e. on the remote end) to find out how they map to the names
on our end (i.e. source). So "direction" is not necessarily incorrect; it
is the direction this function maps the names (either src-to-dst for the
original caller, or dst-to-src for the new caller).
Perhaps "enum map_direction { SRC_TO_DST, DST_TO_SRC }" or something?
> + if (match) {
> matching_refs = i;
> break;
> }
So what is the updated semantics of this function? Is it still
appropriate to name it "check_pattern_match()"?
It seems that by now this does a lot more than just "check if a pattern
matches". Since your patch 2/3, it is a function that finds out the
refname in the remote that the given one refspec would try to update, and
with this patch, it can also map in the reverse direction, given the list
of remote refs, finding out which local ref a refspec would use to update
them.
At the same time, to reduce risk of future breakage, we probably should
rename this function to make it clear that this function is to be only
used by the push side.
Perhaps rename this to "map_push_refs()" or something in the patch 2/3?
> @@ -1173,6 +1178,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
> struct refspec *rs;
> int send_all = flags & MATCH_REFS_ALL;
> int send_mirror = flags & MATCH_REFS_MIRROR;
> + int send_prune = flags & MATCH_REFS_PRUNE;
> int errs;
> static const char *default_refspec[] = { ":", NULL };
> struct ref *ref, **dst_tail = tail_ref(dst);
> @@ -1193,7 +1199,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
> if (ref->peer_ref)
> continue;
>
> - dst_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, &pat);
> + dst_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, 0, &pat);
> if (!dst_name)
> continue;
>
> @@ -1220,6 +1226,23 @@ int match_push_refs(struct ref *src, struct ref **dst,
> free_name:
> free(dst_name);
> }
> + if (send_prune) {
> + /* check for missing refs on the remote */
> + for (ref = *dst; ref; ref = ref->next) {
> + char *src_name;
> +
> + if (ref->peer_ref)
> + /* We're already sending something to this ref. */
> + continue;
> +
> + src_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, 1, NULL);
> + if (src_name) {
> + if (!find_ref_by_name(src, src_name))
> + ref->peer_ref = try_explicit_object_name("");
Yuck. You do not want it to "try" as its name says. You just want to
trigger its "delete" codepath.
Please extract the body of "if (!*name) { ... }" block out of that
function into a separate helper function, i.e.
static struct ref *deleted_ref(void)
{
struct ref *ref = alloc_ref("(delete)");
hashclr(ref->new_sha1);
return ref;
}
then update try_explicit_...() to call it, and call the same helper here.
This is not for runtime efficiency; feeding a constant to a function that
says try_foo() or check_bar() that makes decision on the parameter only to
trigger a partial codepath hurts readability.
> + free(src_name);
> + }
> + }
> + }
> if (errs)
> return -1;
> return 0;
> diff --git a/remote.h b/remote.h
> index b395598..341142c 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
> enum match_refs_flags {
> MATCH_REFS_NONE = 0,
> MATCH_REFS_ALL = (1 << 0),
> - MATCH_REFS_MIRROR = (1 << 1)
> + MATCH_REFS_MIRROR = (1 << 1),
> + MATCH_REFS_PRUNE = (1 << 2),
> };
>
> /* Reporting of tracking info */
> diff --git a/transport.c b/transport.c
> index cac0c06..c20267c 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1028,6 +1028,8 @@ int transport_push(struct transport *transport,
> match_flags |= MATCH_REFS_ALL;
> if (flags & TRANSPORT_PUSH_MIRROR)
> match_flags |= MATCH_REFS_MIRROR;
> + if (flags & TRANSPORT_PUSH_PRUNE)
> + match_flags |= MATCH_REFS_PRUNE;
Does it make sense to specify --prune when --mirror is in effect? If so,
how would it behave differently from a vanilla --mirror? If not, should
it be detected as an error?
I couldn't infer from the context shown in the patch, but how in general
does this new feature interact with the codepath for --mirror?
> if (match_push_refs(local_refs, &remote_refs,
> refspec_nr, refspec, match_flags)) {
> diff --git a/transport.h b/transport.h
> index 059b330..5d30328 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -101,6 +101,7 @@ struct transport {
> #define TRANSPORT_PUSH_MIRROR 8
> #define TRANSPORT_PUSH_PORCELAIN 16
> #define TRANSPORT_PUSH_SET_UPSTREAM 32
> +#define TRANSPORT_PUSH_PRUNE 64
> #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
Hrm...?
> #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
^ permalink raw reply
* Re: [PATCH v3 0/3]
From: Junio C Hamano @ 2012-02-17 23:28 UTC (permalink / raw)
To: Thomas Rast, Jehan Bing; +Cc: git
In-Reply-To: <7v62f5v1d1.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Rast <trast@student.ethz.ch> writes:
> ...
> I seem to be getting intermittent test failures, and every time the
> failing tests are different, when these three are queued to 'pu'. I didn't
> look for what goes wrong and how.
False alarm. I suspect that it is jb/required-filter topic that is causing
intermittent failures from convert.c depending on the timing of how fast
filter subprocess dies vs how fast we consume its result or something.
Repeatedly running t0021 like this:
$ cd t
$ while sh t0021-conversion.sh ; do :; done
under load seems to make it fail every once in a while.
test_must_fail: died by signal: git add test.fc
Are we dying on SIGPIPE or something?
^ permalink raw reply
* Re: [PATCH v2] Add a setting to require a filter to be successful
From: Junio C Hamano @ 2012-02-18 0:07 UTC (permalink / raw)
To: jehan; +Cc: git, Johannes Sixt
In-Reply-To: <4F3DFCD0.6070002@viscovery.net>
A few test in t0021 use 'false' as the filter, which can exit without
reading any byte from us, before we start writing and causes us to die
with SIGPIPE, leading to intermittent test failure. I think treating this
as a failure of running the filter (the end user's filter should read what
is fed in full, produce its output and write the result back to us) is the
right thing to do, and this patch needs more work to handle such a
situation better, probably by using sigchain_push(SIGPIPE) or something.
^ permalink raw reply
* git-svn won't remember pem password
From: Igor @ 2012-02-18 0:36 UTC (permalink / raw)
To: git; +Cc: Eric Wong
I'm running into an issue where I have to enter my pem certificate password every time I git-svn fetch or git-svn dcommit. Vanilla svn uses OS X KeyChain and remembers my password just fine. Is there a known solution for this? Other users have ran into same issue as described here: http://stackoverflow.com/questions/605519/does-git-svn-store-svn-passwords. However, that solution of removing .subversion folder did not work for me.
Thanks,
Igor
^ permalink raw reply
* Re: [PATCH v2] Add a setting to require a filter to be successful
From: Jehan Bing @ 2012-02-18 0:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Sixt
In-Reply-To: <7vd39dqa1i.fsf@alter.siamese.dyndns.org>
On 2012-02-17 16:07, Junio C Hamano wrote:
> A few test in t0021 use 'false' as the filter, which can exit without
> reading any byte from us, before we start writing and causes us to die
> with SIGPIPE, leading to intermittent test failure. I think treating this
> as a failure of running the filter (the end user's filter should read what
> is fed in full, produce its output and write the result back to us) is the
> right thing to do, and this patch needs more work to handle such a
> situation better, probably by using sigchain_push(SIGPIPE) or something.
If I understand what you're saying, current version of git already have
the problem: if a filter fails without reading anything, git will die
instead of using the unfiltered content. My patch has only made the
issue apparent by testing with a failing filter.
Am I understanding correctly?
^ permalink raw reply
* Re: [PATCH v3 0/3]
From: Jeff King @ 2012-02-18 0:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, Jehan Bing, git
In-Reply-To: <7vk43lqbt8.fsf@alter.siamese.dyndns.org>
On Fri, Feb 17, 2012 at 03:28:51PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Thomas Rast <trast@student.ethz.ch> writes:
> > ...
> > I seem to be getting intermittent test failures, and every time the
> > failing tests are different, when these three are queued to 'pu'. I didn't
> > look for what goes wrong and how.
>
> False alarm. I suspect that it is jb/required-filter topic that is causing
> intermittent failures from convert.c depending on the timing of how fast
> filter subprocess dies vs how fast we consume its result or something.
>
> Repeatedly running t0021 like this:
>
> $ cd t
> $ while sh t0021-conversion.sh ; do :; done
>
> under load seems to make it fail every once in a while.
>
> test_must_fail: died by signal: git add test.fc
>
> Are we dying on SIGPIPE or something?
I would be unsurprised if that is the case. Joey Hess mentioned similar
issues with hooks a month or two ago. And I have been seeing
intermittent failures of t5541 under load that I traced back to SIGPIPE.
I've been meaning to dig further and come up with a good solution.
Here's some previous discussion:
http://article.gmane.org/gmane.comp.version-control.git/186291
I'd be happy if we just ignored SIGPIPE everywhere, but turned it on for
the log family.
-Peff
^ permalink raw reply
* Re: git-status does not propagate -uall to submodules
From: Jens Lehmann @ 2012-02-17 22:11 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <874nupq0la.fsf@thomas.inf.ethz.ch>
Am 17.02.2012 10:18, schrieb Thomas Rast:
> Hi,
>
> While helping with the submodule display on #git I noticed that if you
> have a submodule with status.showuntrackedfiles=no, and run 'git status
> -uall' from the superproject, then this does not propagate into the
> submodule's status. In code:
>
> $ (cd bar && git config status.showuntrackedfiles)
> no
> $ git ls-files -s
> 100644 926c01b7259c489a422442a8dc5cb5ea7c58f60c 0 .gitmodules
> 160000 eb5af46e1a938d064c9f7bae9561013654a43316 0 bar
> $ (cd bar && git status -s -unormal)
> ?? otheruntracked
> ?? untracked
> $ git status
> # On branch master
> nothing to commit (working directory clean)
>
> So far that's expected; after all the submodule is configured not to
> display untracked files. But with -uall:
>
> $ git status -uall
> # On branch master
> nothing to commit (working directory clean)
>
> Shouldn't the -uall propagate, since the user is explicitly asking for
> it? That is, the display should summarize what git-status *with the
> same arguments* would show inside the submodules?
Yes, that makes sense. In 3bfc45047 (git status: ignoring untracked
files must apply to submodules too) I added that using -uno will
propagate into submodules. But -uall (and I suspect -unormal too)
should also be passed to the status commands forked inside the
submodules (even though in both cases -unormal should suffice as only
the presence or absence of untracked files will be shown anyway).
But opposed to -uno, which overrides any diff.ignoreSubmodules or
submodule.<name>.ignore settings for the submodules, what should
-unormal and -uall do? These options are used to override the
status.showuntrackedfiles setting, so I suspect even when these
options are given submodules which are configured to ignore untracked
files via diff.ignoreSubmodules or submodule.<name>.ignore should
still be dropped, right?
^ permalink raw reply
* Does pack v4 do anything to commits?
From: Nguyen Thai Ngoc Duy @ 2012-02-18 4:44 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Git Mailing List, Shawn O. Pearce
Hi Nico,
I had an experiment on speeding up "rev-list --all". If I cache sha-1
of tree and parent, and committer date of single-parent commits, in
binary form, rev-list can be sped up significantly. On linux-2.6.git,
it goes from 14s to 4s (2s to 0.8 for git.git). Profiling shows that
commit parsing (get_sha1_hex, parse_commit_date) dominates rev-list
time.
>From what I remember, pack v4 is mainly about changing tree
representation so that we can traverse object DAG as fast as possible.
Do you do anything to commit representation too? Maybe it's worth
storing the above info along with the compressed commit objects in
pack to shave some more seconds.
By the way, is latest packv4 code available somewhere to fetch?
--
Duy
^ permalink raw reply
* Re: [PATCH] git-send-email: allow overriding smtp-encryption config to 'none'
From: Brian Norris @ 2012-02-18 5:27 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120216004903.GA21170@sigill.intra.peff.net>
On Wed, Feb 15, 2012 at 4:49 PM, Jeff King <peff@peff.net> wrote:
> Ah, I see. I misunderstood the original problem you were trying to solve
> (I thought your example was "see? Encryption is off, so the server won't
> do AUTH, demonstrating that the patch works.").
Yeah, I got a little bit off track on what my actual goal was...
> Overriding the smtp user from the config is a separate issue, and I
> don't think that is currently possible. The usual way to spell an option
> like that in git is "--no-smtp-user", but it seems that we use perl's
> GetOptions, which does not understand that syntax. So you'd have to add
> a "--no-smtp-user" by hand.
I think the "--no-smtp-user" is what I really wanted. I've written a
different patch that actually targets the user name properly, but I've
also found a current solution that can work for scripting purposes:
just redirect the $GIT_CONFIG environment variable to /dev/null
temporarily. Perhaps I'll send my new patch sometime, but it's not
pressing and I'm not sure what kind of use it would actually get.
Thanks for the pointers.
Brian
^ permalink raw reply
* Re: [PATCH] git-send-email: allow overriding smtp-encryption config to 'none'
From: Jeff King @ 2012-02-18 6:24 UTC (permalink / raw)
To: Brian Norris; +Cc: git
In-Reply-To: <CAN8TOE-vek=ooq4DRcNF0iCg+rJMt6SUhMi4+_dOWaRJ44KLLA@mail.gmail.com>
On Fri, Feb 17, 2012 at 09:27:44PM -0800, Brian Norris wrote:
> > Overriding the smtp user from the config is a separate issue, and I
> > don't think that is currently possible. The usual way to spell an option
> > like that in git is "--no-smtp-user", but it seems that we use perl's
> > GetOptions, which does not understand that syntax. So you'd have to add
> > a "--no-smtp-user" by hand.
>
> I think the "--no-smtp-user" is what I really wanted. I've written a
> different patch that actually targets the user name properly, but I've
> also found a current solution that can work for scripting purposes:
> just redirect the $GIT_CONFIG environment variable to /dev/null
> temporarily.
Just FYI, the fact that doing so works is somewhat accidental. Long ago,
GIT_CONFIG was respected everywhere as an override to stop reading any
other config. Later, it was dropped, but retained its meaning only for
the git-config command, mostly for historical reasons (although these
days one would do better to use "git config -f $file" instead).
So the reason it works for git-send-email is that send-email in turn
calls git-config to actually look at config values, because send-email
is a perl script and not a C program. In other words, the fact that
GIT_CONFIG is respected is a coincidence of some implementation
decisions, not an intended behavior.
I don't think we have any plans for those implementation details to
change in the near future. So by all means, use it if you like for the
time being. But know that it's not a behavior which is guaranteed not to
change in future versions.
> Perhaps I'll send my new patch sometime, but it's not pressing and I'm
> not sure what kind of use it would actually get.
I think the ideal case would be a patch that teaches the send-email
option parsing code to understand a "--no-*" counterpart for every
option, without having to modify each option individually. I haven't
looked at how easy or hard that would be, though.
-Peff
^ permalink raw reply
* Re: [PATCH v2] Add a setting to require a filter to be successful
From: Junio C Hamano @ 2012-02-18 7:27 UTC (permalink / raw)
To: Jehan Bing; +Cc: git, Johannes Sixt
In-Reply-To: <4F3EF43D.2040102@orb.com>
Jehan Bing <jehan@orb.com> writes:
> If I understand what you're saying, current version of git already
> have the problem: if a filter fails without reading anything, git will
> die instead of using the unfiltered content. My patch has only made
> the issue apparent by testing with a failing filter.
> Am I understanding correctly?
Yes.
^ 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