Re: [PATCH 2/3] combine-diff: suppress a clang warning
On Thu, Feb 07, 2013 at 01:12:59PM +0900, Miles Bader wrote: John Keeping j...@keeping.me.uk writes: I generally like to get rid of the pointless warnings so that the useful ones can't hide in the noise. Perhaps CFLAGS += -Wno-string-plus-int would be better for this particular warning, but when there's only one bit of code that triggers it, tweaking that seemed simpler. An even better approach would be to file a bug against clang ... it really is a very ill-considered warning -- PTR + OFFS is not just valid C, it's _idiomatic_ in C for getting interior pointers into arrays -- and such a warning should never be enabled by default, or by any standard warning options. It doesn't warn of PTR + OFFS, only STRING_LITERAL + OFFS. I agree that it's not a particularly useful warning but it was clearly introduced intentionally and appears to find real bugs [1] so I don't intend to argue about it with the Clang developers. [1] http://article.gmane.org/gmane.comp.compilers.clang.scm/47203 John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
Junio C Hamano venit, vidit, dixit 06.02.2013 17:53:
Michael J Gruber g...@drmicha.warpmail.net writes:
Currently, diff and cat-file for blobs obey --textconv options
(with the former defaulting to --textconv and the latter to
--no-textconv) whereas show does not obey this option, even though
it takes diff options.
Make show on blobs behave like diff, i.e. obey --textconv by
default and --no-textconv when given.
What does log -p do currently, and what should it do? Does/should
it also use --textconv?
It invokes --textconv by default, and allows to override with
--no-textconv. That's what I meant to say but seem to have failed to.
I think at some point we decided to have human output on by default
for all things diff (porcelain only, of course) unless the diff is meant
for machine consumption, such as in the case of format-patch.
The --textconv is a natural extension of what --ext-diff provides us,
so I think it should trigger the same way as how --ext-diff triggers.
This series' aim is to make the textconv behavior the same for all
human output commands. I don't see textconv and ext-diff to be
strongly related, and if then the other way round:
textconv is about converting blobs to a (possibly lossy) human
consumable text.
ext-diff is about computing diffs with an external diff tool (not in
the sense of diff-tool).
One way of diffing is textconving blobs then using internal diff on the
resulting text blobs, but ext-diff is more general and, really,
orthogonal in a way. (It used to be used often for what textconv does
when that didn't exist.)
We apply --ext-diff for diff by default but not for log -p and
show; I suspect this may have been for a good reason but I do not
recall the discussion that led to the current behaviour offhand.
I don't think I changed anything ext-diff related, did I? 1/4 is about
showing blobs, not diffs.
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
builtin/log.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..f83870d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -402,10 +402,28 @@ static void show_tagger(char *buf, int len, struct
rev_info *rev)
strbuf_release(out);
}
-static int show_blob_object(const unsigned char *sha1, struct rev_info *rev)
+static int show_blob_object(const unsigned char *sha1, struct rev_info
*rev, const char *obj_name)
{
+unsigned char sha1c[20];
+struct object_context obj_context;
+char *buf;
+unsigned long size;
+
fflush(stdout);
-return stream_blob_to_fd(1, sha1, NULL, 0);
+if (!DIFF_OPT_TST(rev-diffopt, ALLOW_TEXTCONV))
+return stream_blob_to_fd(1, sha1, NULL, 0);
+
+if (get_sha1_with_context(obj_name, 0, sha1c, obj_context))
+die(Not a valid object name %s, obj_name);
+if (!obj_context.path[0] ||
+!textconv_object(obj_context.path, obj_context.mode, sha1c, 1,
buf, size))
+return stream_blob_to_fd(1, sha1, NULL, 0);
+
+if (!buf)
+die(git show %s: bad file, obj_name);
+
+write_or_die(1, buf, size);
+return 0;
}
static int show_tag_object(const unsigned char *sha1, struct rev_info *rev)
@@ -491,7 +509,7 @@ int cmd_show(int argc, const char **argv, const char
*prefix)
const char *name = objects[i].name;
switch (o-type) {
case OBJ_BLOB:
-ret = show_blob_object(o-sha1, NULL);
+ret = show_blob_object(o-sha1, rev, name);
break;
case OBJ_TAG: {
struct tag *t = (struct tag *)o;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
Jeff King venit, vidit, dixit 06.02.2013 23:06:
On Wed, Feb 06, 2013 at 04:08:50PM +0100, Michael J Gruber wrote:
diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..f83870d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -402,10 +402,28 @@ static void show_tagger(char *buf, int len, struct
rev_info *rev)
strbuf_release(out);
}
-static int show_blob_object(const unsigned char *sha1, struct rev_info *rev)
+static int show_blob_object(const unsigned char *sha1, struct rev_info
*rev, const char *obj_name)
Should this maybe just take the whole object_array_entry as a cleanup?
It's just a question of one or two/three pointers (I can't count), but
yes, that would be possible.
{
+unsigned char sha1c[20];
+struct object_context obj_context;
+char *buf;
+unsigned long size;
+
fflush(stdout);
-return stream_blob_to_fd(1, sha1, NULL, 0);
+if (!DIFF_OPT_TST(rev-diffopt, ALLOW_TEXTCONV))
+return stream_blob_to_fd(1, sha1, NULL, 0);
+
+if (get_sha1_with_context(obj_name, 0, sha1c, obj_context))
+die(Not a valid object name %s, obj_name);
It seems a little hacky that we have to look up the sha1 again. What
should happen in the off chance that hashcmp(sha1, sha1c) != 0 due to
a race with a simultaneous update of a ref?
I thought about a check here but didn't bother to because I knew the
refactoring would come up again...
Would it be better if object_array_entry replaced its mode member with
an object_context?
Do all callers/users want to deal with object_context?
I'm wondering why o_c has a mode at all, since it is mostly used in
conjunction with an object, isn't it?
The only downside I see is that we might waste a
significant amount of memory (each context has a PATH_MAX buffer in it).
That's why I used a reference to the struct, see my other reply.
Michael
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
Jeff King venit, vidit, dixit 06.02.2013 23:36: On Wed, Feb 06, 2013 at 04:08:53PM +0100, Michael J Gruber wrote: -add_object_array(object, arg, list); +add_object_array_with_context(object, arg, list, xmemdupz(oc, sizeof(struct object_context))); If we go this route, this new _with_context variant should be used in patch 1, too. @@ -265,9 +260,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj objects[nr].item = obj; objects[nr].name = name; objects[nr].mode = mode; +objects[nr].context = context; array-nr = ++nr; } This seems a little gross. Who is responsible for allocating the context? Who frees it? It looks like we duplicate it in cmd_grep. Which Well, who is responsible for allocating and freeing name and item? I didn't want to introduce a new member which is a struct when all other complex members are pointers. Wouldn't that be confusing? I think is OK, but it means all of this context infrastructure in object.[ch] is just bolted-on junk waiting for somebody to use it wrong or get confused. It does not get set, for example, by the regular setup_revisions code path. Sure, it's NULL when there is no context info, just like in many other cases. It would be nice if we could just always have the context available, then setup_revisions could set it up by default (and replace the mode parameter entirely). But we'd need to do something to avoid the PATH_MAX-sized buffer for each entry, as some code paths may have a large number of pending objects. If the information is always available even if we don't need it then it always takes space. The only way out would be pointing into a pool of path names rather having a copy in each entry. It's not like I hadn't talked about providing virtual (blob) objects for path names keyed by their sha1 before... It's just that I want my grep --textconv now ;) Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
On Thu, Feb 07, 2013 at 10:05:26AM +0100, Michael J Gruber wrote: Would it be better if object_array_entry replaced its mode member with an object_context? Do all callers/users want to deal with object_context? Wouldn't it just mean replacing entry-mode with entry-oc.mode at each user? I'm wondering why o_c has a mode at all, since it is mostly used in conjunction with an object, isn't it? Just as we record the path from the surrounding tree, we record the mode. It's that mode which gets put into the pending object list by the revision parser (see the very end of handle_revision_arg). Storing an object_context instead of the mode would be a strict superset of what we store now (right now we just throw the rest away). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
On Thu, Feb 07, 2013 at 10:05:57AM +0100, Michael J Gruber wrote: @@ -265,9 +260,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj objects[nr].item = obj; objects[nr].name = name; objects[nr].mode = mode; + objects[nr].context = context; array-nr = ++nr; } This seems a little gross. Who is responsible for allocating the context? Who frees it? It looks like we duplicate it in cmd_grep. Which Well, who is responsible for allocating and freeing name and item? I didn't want to introduce a new member which is a struct when all other complex members are pointers. Wouldn't that be confusing? We cheat on those two. item is always a pointer to a struct object, which lasts forever and never gets freed. When name is set by setup_revisions, it comes from the argv list, which is assumed to last forever (and when we add pending blobs for a --objects traversal, it is the empty string (literal). I'd be OK if we had an exterior object_context that could be handled in the same way. But how do we tell setup_revisions that we are interested in seeing the object_context from each parsed item, where does the allocation come from (is it malloc'd by setup_revisions?), and who is responsible for freeing it when we pop pending objects in get_revisions and similar? I don't think it's as clear cut. I wonder, though...what we really care about here is just the pathname. But if it is a pending object that comes from a blob revision argument, won't it always be of the form treeish:path? Could we not even resolve the sha1 again, but instead just parse out the :path bit? That is sort of like what the repeated call to get_sha1_with_context does in your first patch. Except that we do not actually want to lookup the sha1, and it is harmful to do so (e.g., if the ref had moved on to a new tree that does not have that path, get_sha1 would fail, but we do not even care what is in the tree; we only want the parsing side effects of get_sha1). Hmm. -Peff PS By the way, while looking at the object_array code (which I have not really used much before), I noticed that add_pending_commit_list sets the name field to the result of sha1_to_hex. Which means that it is likely to be completely bogus by the time you read it. I'm not even sure where it gets read or if this matters. And obviously it's completely unrelated to what we were discussing; just something I noticed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
Jeff King venit, vidit, dixit 07.02.2013 10:11: On Thu, Feb 07, 2013 at 10:05:26AM +0100, Michael J Gruber wrote: Would it be better if object_array_entry replaced its mode member with an object_context? Do all callers/users want to deal with object_context? Wouldn't it just mean replacing entry-mode with entry-oc.mode at each user? Yes, I meant at the time of creation, i.e. when someone has to create and pass an o_a_e and maybe only knows a mode, and thus would have to set the path to NULL or . I'm wondering why o_c has a mode at all, since it is mostly used in conjunction with an object, isn't it? Just as we record the path from the surrounding tree, we record the mode. It's that mode which gets put into the pending object list by the revision parser (see the very end of handle_revision_arg). Storing an object_context instead of the mode would be a strict superset of what we store now (right now we just throw the rest away). Sure. But why does object_context have a mode member at all? Maybe it is not alway used together with another struct which has the mode already, then that's a reason. Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] connect.c: Tell *PLink to always use ssh protocol
Am 07.02.2013 00:22 schrieb Jeff King:
On Wed, Feb 06, 2013 at 10:58:49PM +0100, Sven Strickroth wrote:
Default values for *plink can be set using PuTTY. If a user makes
telnet the default in PuTTY this breaks ssh clones in git.
Since git clones of the type user@host:path use ssh, tell *plink
to use ssh and override PuTTY defaults for the protocol to use.
---
connect.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/connect.c b/connect.c
index 49e56ba..d337b6f 100644
--- a/connect.c
+++ b/connect.c
@@ -625,6 +625,8 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
if (!ssh) ssh = ssh;
*arg++ = ssh;
+if (putty)
+*arg++ = -ssh;
if (putty !strcasestr(ssh, tortoiseplink))
*arg++ = -batch;
if (port) {
Just for the completeness: This might have an unwanted side effect...
Using the -ssh parameter sets the protocol to ssh AND the port number to
22. This might break a setting where a user stores a PuTTY default for
ssh, but with a different port number (e.g. because a user always pushes
to a remote ssh repository which resides on a different port).
PuTTY settings for a named session still work, it only affects the
Default Settings session - so users can set up specific sessons in
PuTTY if he wants to change the default port.
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
On Thu, Feb 07, 2013 at 10:34:26AM +0100, Michael J Gruber wrote: Just as we record the path from the surrounding tree, we record the mode. It's that mode which gets put into the pending object list by the revision parser (see the very end of handle_revision_arg). Storing an object_context instead of the mode would be a strict superset of what we store now (right now we just throw the rest away). Sure. But why does object_context have a mode member at all? Maybe it is not alway used together with another struct which has the mode already, then that's a reason. Exactly. It's purely for pulling information out of get_sha1_with_context, and does not know that you are going to put its output into an object_array_entry (and many call sites do not). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
Jeff King venit, vidit, dixit 07.02.2013 10:26: On Thu, Feb 07, 2013 at 10:05:57AM +0100, Michael J Gruber wrote: @@ -265,9 +260,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj objects[nr].item = obj; objects[nr].name = name; objects[nr].mode = mode; + objects[nr].context = context; array-nr = ++nr; } This seems a little gross. Who is responsible for allocating the context? Who frees it? It looks like we duplicate it in cmd_grep. Which Well, who is responsible for allocating and freeing name and item? I didn't want to introduce a new member which is a struct when all other complex members are pointers. Wouldn't that be confusing? We cheat on those two. item is always a pointer to a struct object, which lasts forever and never gets freed. When name is set by setup_revisions, it comes from the argv list, which is assumed to last forever (and when we add pending blobs for a --objects traversal, it is the empty string (literal). I see, so they are really different. I'd be OK if we had an exterior object_context that could be handled in the same way. But how do we tell setup_revisions that we are interested in seeing the object_context from each parsed item, where does the allocation come from (is it malloc'd by setup_revisions?), and who is responsible for freeing it when we pop pending objects in get_revisions and similar? Do we really need all of tree, path and mode in object_context (I mean not just here, but other users), or only the path? I'd try and resurrect the virtual path name objects then, they would be just like item storage-wise. I don't think it's as clear cut. I wonder, though...what we really care about here is just the pathname. But if it is a pending object that comes from a blob revision argument, won't it always be of the form treeish:path? Could we not even resolve the sha1 again, but instead just parse out the :path bit? Do we have that, and in what form (e.g. magic expanded etc.)? That is sort of like what the repeated call to get_sha1_with_context does in your first patch. Except that we do not actually want to lookup the sha1, and it is harmful to do so (e.g., if the ref had moved on to a new tree that does not have that path, get_sha1 would fail, but we do not even care what is in the tree; we only want the parsing side effects of get_sha1). Hmm. -Peff PS By the way, while looking at the object_array code (which I have not really used much before), I noticed that add_pending_commit_list sets the name field to the result of sha1_to_hex. Which means that it is likely to be completely bogus by the time you read it. I'm not even sure where it gets read or if this matters. And obviously it's completely unrelated to what we were discussing; just something I noticed. Another thing I noted is that our path mangling at least for grep has some issues: (cd t git grep GET_SHA1_QUIETLY HEAD:../cache.h) ../HEAD:../cache.h:#define GET_SHA1_QUIETLY01 Taking everything right of : could still work. Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
On Thu, Feb 07, 2013 at 10:47:55AM +0100, Michael J Gruber wrote: I'd be OK if we had an exterior object_context that could be handled in the same way. But how do we tell setup_revisions that we are interested in seeing the object_context from each parsed item, where does the allocation come from (is it malloc'd by setup_revisions?), and who is responsible for freeing it when we pop pending objects in get_revisions and similar? Do we really need all of tree, path and mode in object_context (I mean not just here, but other users), or only the path? I'd try and resurrect the virtual path name objects then, they would be just like item storage-wise. We need at least mode, since that is how the mode parameter of object_array_entry gets set. I do not know off-hand who uses tree. I suspect the intent was to do .gitattributes lookups inside that tree, but I do not think we actually do in-tree lookups currently. I don't think it's as clear cut. I wonder, though...what we really care about here is just the pathname. But if it is a pending object that comes from a blob revision argument, won't it always be of the form treeish:path? Could we not even resolve the sha1 again, but instead just parse out the :path bit? Do we have that, and in what form (e.g. magic expanded etc.)? Ah, I should have mentioned that. :) We should have the original rev name in the object_array_entry's name field, shouldn't we? It's just a matter of re-parsing it. Another thing I noted is that our path mangling at least for grep has some issues: (cd t git grep GET_SHA1_QUIETLY HEAD:../cache.h) ../HEAD:../cache.h:#define GET_SHA1_QUIETLY01 Yuck. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
On Thu, Feb 7, 2013 at 1:16 AM, Jeff King p...@peff.net wrote: On Wed, Feb 06, 2013 at 04:12:10PM -0800, Junio C Hamano wrote: Ævar Arnfjörð Bjarmason ava...@gmail.com writes: I think there's a simpler way to do this, which is that: * New clients supporting v2 of the protocol send some piece of data that would break old servers. * If that fails the new client goes oh jeeze, I guess it's an old server, and try again with the old protocol. * The client then saves a date (or the version the server gave us) indicating that it tried the new protocol on that remote, tries again sometime later. For that to work, the new server needs to wait for the client to speak first. How would that server handle old clients who expect to be spoken first? Wait with a read timeout (no timeout is the right timeout for everybody)? If the new client can handle the old-style server's response, then the server can start blasting out refs (optionally after a timeout) and stop when the client interrupts with hey, wait, I can speak the new protocol. The server just has to include you can interrupt me in its capability advertisement (obviously it would have to send out at least the first ref with the capabilities before the timeout). Can't this also be handled by passing an extra argument to upload-pack? Whether you're talking http, ssh + normal shell, ssh + git-shell or git:// you pass some argument that older clients would reject on but would cause newer clients that know about that argument to wait for you to speak before blasting refs at you. It would mean that older clients (e.g. older git-shell) would reject your initial connection, but you could just try again, and save away info about that remote's version. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
Jeff King venit, vidit, dixit 07.02.2013 10:55: On Thu, Feb 07, 2013 at 10:47:55AM +0100, Michael J Gruber wrote: I'd be OK if we had an exterior object_context that could be handled in the same way. But how do we tell setup_revisions that we are interested in seeing the object_context from each parsed item, where does the allocation come from (is it malloc'd by setup_revisions?), and who is responsible for freeing it when we pop pending objects in get_revisions and similar? Do we really need all of tree, path and mode in object_context (I mean not just here, but other users), or only the path? I'd try and resurrect the virtual path name objects then, they would be just like item storage-wise. We need at least mode, since that is how the mode parameter of object_array_entry gets set. I do not know off-hand who uses tree. I suspect the intent was to do .gitattributes lookups inside that tree, but I do not think we actually do in-tree lookups currently. I don't think it's as clear cut. I wonder, though...what we really care about here is just the pathname. But if it is a pending object that comes from a blob revision argument, won't it always be of the form treeish:path? Could we not even resolve the sha1 again, but instead just parse out the :path bit? Do we have that, and in what form (e.g. magic expanded etc.)? Ah, I should have mentioned that. :) We should have the original rev name in the object_array_entry's name field, shouldn't we? It's just a matter of re-parsing it. Another thing I noted is that our path mangling at least for grep has some issues: (cd t git grep GET_SHA1_QUIETLY HEAD:../cache.h) ../HEAD:../cache.h:#define GET_SHA1_QUIETLY01 Yuck. And even more yuck: (cd t git grep --full-name GET_SHA1_QUIETLY HEAD:../cache.h) HEAD:../cache.h:#define GET_SHA1_QUIETLY01 Someone does not expect a rev: to be in there, it seems ;) Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 0/5] Make git-send-email use git-credential
From: Michal Nazarewicz min...@mina86.com Minor fixes as suggested in emails. Michal Nazarewicz (5): Git.pm: allow command_close_bidi_pipe to be called as method Git.pm: fix example in command_close_bidi_pipe documentation Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe Git.pm: add interface for git credential command git-send-email: use git credential to obtain password Documentation/git-send-email.txt | 4 +- git-send-email.perl | 59 ++ perl/Git.pm | 129 +-- 3 files changed, 161 insertions(+), 31 deletions(-) -- 1.8.1.2.549.g1d13f9f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 1/5] Git.pm: allow command_close_bidi_pipe to be called as method
From: Michal Nazarewicz min...@mina86.com
The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter. Using _maybe_self()
fixes this.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
sub command_close_bidi_pipe {
local $?;
- my ($pid, $in, $out, $ctx) = @_;
+ my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
--
1.8.1.2.549.g1d13f9f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 2/5] Git.pm: fix example in command_close_bidi_pipe documentation
From: Michal Nazarewicz min...@mina86.com
File handle goes as the first argument when calling print on it.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..11f310a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -418,7 +418,7 @@ and it is the fourth value returned by
Ccommand_bidi_pipe(). The call idiom
is:
my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file
--batch-check');
- print 0\n $out;
+ print $out 0\n;
while ($in) { ... }
$r-command_close_bidi_pipe($pid, $in, $out, $ctx);
--
1.8.1.2.549.g1d13f9f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 4/5] Git.pm: add interface for git credential command
From: Michal Nazarewicz min...@mina86.com
Add a credential() function which is an interface to the git
credential command. The code is heavily based on credential_*
functions in contrib/mw-to-git/git-remote-mediawiki.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
perl/Git.pm | 110 +++-
1 file changed, 109 insertions(+), 1 deletion(-)
On Thu, Feb 07 2013, Jeff King p...@peff.net wrote:
On Wed, Feb 06, 2013 at 09:47:05PM +0100, Michal Nazarewicz wrote:
+sub _credential_read {
+ my %credential;
+ my ($reader, $op) = (@_);
+ while ($reader) {
+ chomp;
+ my ($key, $value) = /([^=]*)=(.*)/;
Empty keys are not valid. Can we make this:
/^([^=]+)=(.*)/
to fail the regex? Otherwise, I think this check:
+ if (not defined $key) {
+ throw Error::Simple(unable to parse git credential $op
response:\n$_\n);
+ }
would not pass because $key would be the empty string.
Right, fixed.
+sub _credential_write {
+ my ($credential, $writer) = @_;
+
+ for my $key (sort {
+ # url overwrites other fields, so it must come first
+ return -1 if $a eq 'url';
+ return 1 if $b eq 'url';
+ return $a cmp $b;
+ } keys %$credential) {
+ if (defined $credential-{$key} length $credential-{$key}) {
+ print $writer $key, '=', $credential-{$key}, \n;
+ }
+ }
There are a few disallowed characters, like \n in key or value, and
= in a key. They should never happen unless the caller is buggy, but
should we check and catch them here?
I left it as is for now since it's not entairly clear to me what to
do in all cases. In particular:
- when reading, what to do if the line is foo = bar ,
- when reading, what to do if the line is foo= (ie. empty value),
- when writing, what to do if value is a single space,
- when writing, what to do if value ends with a new line,
- when writing, what to do if value is empty (currently not printed at all),
On Thu, Feb 07 2013, Matthieu Moy matthieu@grenoble-inp.fr wrote:
I think you should credit git-remote-mediawiki for the code in the
commit message. Perhaps have a first copy/paste commit, and then an
adaptation commit to add sort, ^ anchor in regexp, doc and your
callback mechanism, but I won't insist on that.
Good point. Creating additional commit is a bit too much for my
licking, but added note in commit message.
diff --git a/perl/Git.pm b/perl/Git.pm
index 9dded54..b4adead 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,7 +59,8 @@ require Exporter;
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
remote_refs prompt
-temp_acquire temp_release temp_reset temp_path);
+temp_acquire temp_release temp_reset temp_path
+credential);
=head1 DESCRIPTION
@@ -1013,6 +1014,113 @@ sub _close_cat_blob {
}
+sub _credential_read {
+ my %credential;
+ my ($reader, $op) = (@_);
+ while ($reader) {
+ if (!/^([^=\s]+)=(.*?)\s*$/) {
+ throw Error::Simple(unable to parse git credential $op
response:\n$_);
+ }
+ $credential{$1} = $2;
+ }
+ return %credential;
+}
+
+sub _credential_write {
+ my ($credential, $writer) = @_;
+
+ for my $key (sort {
+ # url overwrites other fields, so it must come first
+ return -1 if $a eq 'url';
+ return 1 if $b eq 'url';
+ return $a cmp $b;
+ } keys %$credential) {
+ if (defined $credential-{$key} length $credential-{$key}) {
+ print $writer $key, '=', $credential-{$key}, \n;
+ }
+ }
+ print $writer \n;
+}
+
+sub _credential_run {
+ my ($self, $credential, $op) = _maybe_self(@_);
+
+ my ($pid, $reader, $writer, $ctx) = command_bidi_pipe('credential',
$op);
+
+ _credential_write $credential, $writer;
+ close $writer;
+
+ if ($op eq fill) {
+ %$credential = _credential_read $reader, $op;
+ } elsif ($reader) {
+ throw Error::Simple(unexpected output from git credential $op
response:\n$_\n);
+ }
+
+ command_close_bidi_pipe($pid, $reader, undef, $ctx);
+}
+
+=item credential( CREDENTIAL_HASH [, OPERATION ] )
+
+=item credential( CREDENTIAL_HASH, CODE )
+
+Executes Cgit credential for a given set of credentials and
+specified operation. In both form CCREDENTIAL_HASH needs to be
+a reference to a hash which stores credentials. Under certain
+conditions the hash can change.
+
+In the first form, COPERATION can be C'fill' (or omitted),
+C'approve' or C'reject', and function will execute corresponding
+Cgit credential
[PATCHv2 3/5] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
From: Michal Nazarewicz min...@mina86.com
The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe(). With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
perl/Git.pm | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
On Wed, Feb 06, 2013 at 09:47:04PM +0100, Michal Nazarewicz wrote:
This allows for something like:
my ($pid, $in, $out, $ctx) = command_bidi_pipe(...);
print $out write data;
close $out;
# ... do stuff with $in
command_close_bidi_pipe($pid, $in, undef, $ctx);
On Thu, Feb 07 2013, Jeff King p...@peff.net wrote:
Should this part go into the documentation for command_close_bidi_pipe
in Git.pm?
Done.
diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..9dded54 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -426,13 +426,26 @@ Note that you should not rely on whatever actually is in
CCTX;
currently it is simply the command name but in future the context might
have more complicated structure.
+CPIPE_IN and CPIPE_OUT may be Cundef if they have been closed prior to
+calling this function. This may be useful in a query-response type of
+commands where caller first writes a query and later reads response, eg:
+
+ my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file
--batch-check');
+ print $out 0\n;
+ close $out;
+ while ($in) { ... }
+ $r-command_close_bidi_pipe($pid, $in, undef, $ctx);
+
+This idiom may prevent potential dead locks caused by data sent to the output
+pipe not being flushed and thus not reaching the executed command.
+
=cut
sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
- unless (close $fh) {
+ if (defined $fh !close $fh) {
if ($!) {
carp error closing pipe: $!;
} elsif ($? 8) {
--
1.8.1.2.549.g1d13f9f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 5/5] git-send-email: use git credential to obtain password
From: Michal Nazarewicz min...@mina86.com
If smtp_user is provided but smtp_pass is not, instead of
prompting for password, make git-send-email use git
credential command instead.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
Documentation/git-send-email.txt | 4 +--
git-send-email.perl | 59 +++-
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
Furthermore, passwords need not be specified in configuration files
or on the command line. If a username has been specified (with
'--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
--smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
}
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+ if (!defined $smtp_authuser || $auth) {
+ return 1;
+ }
+
+ # Workaround AUTH PLAIN/LOGIN interaction defect
+ # with Authen::SASL::Cyrus
+ eval {
+ require Authen::SASL;
+ Authen::SASL-import(qw(Perl));
+ };
+
+ # TODO: Authentication may fail not because credentials were
+ # invalid but due to other reasons, in which we should not
+ # reject credentials.
+ $auth = Git::credential({
+ 'protocol' = 'smtp',
+ 'host' = join(':', $smtp_server, $smtp_server_port),
+ 'username' = $smtp_authuser,
+ # if there's no password, git credential fill will
+ # give us one, otherwise it'll just pass this one.
+ 'password' = $smtp_authpass
+ }, sub {
+ my $cred = shift;
+ return !!$smtp-auth($cred-{'username'}, $cred-{'password'});
+ });
+
+ return $auth;
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ?
port=$smtp_server_port : ;
}
- if (defined $smtp_authuser) {
- # Workaround AUTH PLAIN/LOGIN interaction defect
- # with Authen::SASL::Cyrus
- eval {
- require Authen::SASL;
- Authen::SASL-import(qw(Perl));
- };
-
- if (!defined $smtp_authpass) {
-
- system stty -echo;
-
- do {
- print Password: ;
- $_ = STDIN;
- print \n;
- } while (!defined $_);
-
- chomp($smtp_authpass = $_);
-
- system stty echo;
- }
-
- $auth ||= $smtp-auth( $smtp_authuser, $smtp_authpass )
or die $smtp-message;
- }
+ smtp_auth_maybe or die $smtp-message;
$smtp-mail( $raw_from ) or die $smtp-message;
$smtp-to( @recipients ) or die $smtp-message;
--
1.8.1.2.549.g1d13f9f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-send-email: add ~/.authinfo parsing
On Thu, 07 Feb 2013 08:08:59 +0100 Matthieu Moy matthieu@grenoble-inp.fr wrote: MM Plus, read/write has already been used for a while in the C API, so I'd MM rather keep the same names for the Perl equivalent. That makes perfect sense. Ted -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: overriding/removing inherited credential.helper, Do not add an empty value from config credential.helper
On Sat, 10 Nov 2012 23:12:50 +0800 乙酸鋰 ch3co...@gmail.com wrote: In credential.c, line 67: if (!strcmp(key, helper)) string_list_append(c-helpers, value); In global config, I add one credential helper. But I do not want to use any credential helper in a specific repository. Currently there is no way in local config to override and remove inherited credential helper. Of course, I do not want to change global config. Could you suggest a patch? On Sat, 10 Nov 2012 23:08:04 +0800 乙酸鋰 ch3co...@gmail.com wrote: Below is current git message when a local config credential.helper has an empty value. Please skip an empty value. $ git push --force origin master git: 'credential-' is not a git command. See 'git --help'. Did you mean this? credential Total 0 (delta 0), reused 0 (delta 0) To https://u...@github.com/user/myrepo.git + d23aa6a...3405990 master - master (forced update) I would like that too (needed it today). Maybe the empty string (as suggested) or none could be acceptable. none seems friendlier and is very unlikely to collide with an actual credential helper. Ted -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Credentials and the Secrets API...
On Thu, 27 Oct 2011 12:05:03 -0400 John Szakmeister j...@szakmeister.net wrote: JS Just wanted to keep folks in the loop. It turns out that the Secrets JS API is still to young. I asked about the format to store credentials JS in (as far as attributes), and got a response from a KDE developer JS that says it's still to young on their front. They hope to have JS support in the next release of KDE. But there's still the issue of JS what attributes to use. JS With that information, I went ahead and created a JS gnome-credential-keyring that uses Gnome's Keyring facility. I still JS need to do a few more things (mainly run it against Jeff's tests), but JS it's generally working. Just wanted to keep folks in the loop. JS Hopefully, I can get patches out this weekend. Do you think the Secrets API has matured enough? KDE has had a new release since your post... Thanks Ted -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
Michael Haggerty mhag...@alum.mit.edu writes: A first weakness of your proposal is that even though the hidden refs are (optionally) fetchable, there is *no* way to discover them remotely or to bulk-download them; they would have to be retrieved one by one using out-of-band information. And if I understand correctly, there would be no way to push hidden references remotely (whether manually or from some automated process). Such processes would have to be local to the machine holding the repository. I'm the author of git-fat [1], a smudge/clean filter system for managing large files. I currently store files in the file system (.git/fat/objects) and transfer them via rsync because I want to be able to transfer exact subsets requested by the user. I would like to put this data in a git repository so that I can take advantage of packfile compression when applicable and so that I can use existing access control, but I would need to store a separate reference to each blob (so that I can transfer exact subsets). My refs would be named like 'fat-SHA1_OF_SMUDGED_DATA' and are known on the client side because they are in the cleaned blob (which contains only this SHA1 and the number of bytes [2]). I believe that my use case would be well supported if git could push and pull unadvertised refs, as long as basic operations were not slowed down by the existence of a very large number of such refs. [1] https://github.com/jedbrown/git-fat [2] We could eliminate the performance problem of needing to buffer the entire file if the smudge filter could be passed the object size as an argument and if we could forward that size in a stream to 'git hash-object --stdin'. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: branch.name.remotepush
On 02/07/2013 05:14 PM, Ramkumar Ramachandra wrote: This has been annoying me for a really long time, but I never really got around to scratching this particular itch. I have a very common scenario where I fork a project on GitHub. I have two configured remotes: origin which points to git://upstream and mine which points to ssh://mine. By default, I always want to pull `master` from origin and push to mine. Unfortunately, there's only a branch.name.remote which specifies which remote to use for both pulling and pushing. There's also a remote.name.pushurl, but I get the feeling that this exists for an entirely different reason: when I have a server with a highly-available read-only mirror of the repository at git://anongit.*, and a less-available committer-only mirror at ssh://*. How about a branch.name.remotepush that specifies a special remote for pushing, falling back to branch.name.remote? Additionally, it would be nice to have branch.name.push or similar to configure a default destination branch for push. Gerrit users usually want to track refs/heads/master but push to refs/for/master for example. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
Michael J Gruber g...@drmicha.warpmail.net writes: (cd t git grep GET_SHA1_QUIETLY HEAD:../cache.h) ../HEAD:../cache.h:#define GET_SHA1_QUIETLY01 Yuck. And even more yuck: (cd t git grep --full-name GET_SHA1_QUIETLY HEAD:../cache.h) HEAD:../cache.h:#define GET_SHA1_QUIETLY01 Someone does not expect a rev: to be in there, it seems ;) I think stepping outside of $(cwd) is an afterthought the code does not anticipate. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] count-objects: report garbage files in .git/objects/pack directory too
Duy Nguyen pclo...@gmail.com writes:
I thought about that, but we may need to do extra stat() for loose
garbage as well. As it is now, garbage is complained loudly, which
gives me enough motivation to clean up, even without looking at how
much disk space it uses.
I wouldn't call a single line garbage: 4 exactly *loud*. I also
think that this is not about *motivating* you, but about giving
more information to the users to help them assess the health of
their repository themselves.
By the way, I wonder if we also want to notice .git/objects/garbage
or .git/objects/ga/rbage if we are to do this?
-- 8 --
Subject: count-objects: report garbage pack directory too
prepare_packed_git_one() is modified to allow count-objects to hook a
report function to so we don't need to duplicate the pack searching
logic in count-objects.c. When report_pack_garbage is NULL, the
overhead is insignificant.
diff --git a/Documentation/git-count-objects.txt
b/Documentation/git-count-objects.txt
index e816823..1611d7c 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
prune-packable: the number of loose objects that are also present in
the packs. These objects could be pruned using `git prune-packed`.
+
-garbage: the number of files in loose object database that are not
-valid loose objects
+garbage: the number of files in object database that are not valid
+loose objects nor valid packs
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..7fdd508 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -9,6 +9,8 @@
#include builtin.h
#include parse-options.h
+static unsigned long garbage;
+
static void count_objects(DIR *d, char *path, int len, int verbose,
unsigned long *loose,
off_t *loose_size,
@@ -65,6 +67,16 @@ static void count_objects(DIR *d, char *path, int len, int
verbose,
}
}
+extern void (*report_pack_garbage)(const char *path, int len, const char
*name);
+static void real_report_pack_garbage(const char *path, int len, const char
*name)
+{
+ if (len)
+ error(garbage found: %.*s/%s, len, path, name);
+ else
+ error(garbage found: %s, path);
+ garbage++;
+}
+
static char const * const count_objects_usage[] = {
N_(git count-objects [-v]),
NULL
@@ -76,7 +88,7 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
- unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
+ unsigned long loose = 0, packed = 0, packed_loose = 0;
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
@@ -87,6 +99,8 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
+ if (verbose)
+ report_pack_garbage = real_report_pack_garbage;
memcpy(path, objdir, len);
if (len objdir[len-1] != '/')
path[len++] = '/';
diff --git a/sha1_file.c b/sha1_file.c
index 40b2329..5b70e55 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -21,6 +21,7 @@
#include sha1-lookup.h
#include bulk-checkin.h
#include streaming.h
+#include dir.h
#ifndef O_NOATIME
#if defined(__linux__) (defined(__i386__) || defined(__PPC__))
@@ -1000,15 +1001,19 @@ void install_packed_git(struct packed_git *pack)
packed_git = pack;
}
+void (*report_pack_garbage)(const char *path, int len, const char *name);
+
static void prepare_packed_git_one(char *objdir, int local)
{
/* Ensure that this buffer is large enough so that we can
append /pack/ without clobbering the stack even if
strlen(objdir) were PATH_MAX. */
char path[PATH_MAX + 1 + 4 + 1 + 1];
- int len;
+ int i, len;
DIR *dir;
struct dirent *de;
+ struct packed_git *p;
+ struct string_list garbage = STRING_LIST_INIT_DUP;
sprintf(path, %s/pack, objdir);
len = strlen(path);
@@ -1024,14 +1029,33 @@ static void prepare_packed_git_one(char *objdir, int
local)
int namelen = strlen(de-d_name);
struct packed_git *p;
- if (!has_extension(de-d_name, .idx))
+ if (len + namelen + 1 sizeof(path)) {
+ if (report_pack_garbage)
+ report_pack_garbage(path, len - 1, de-d_name);
continue;
+ }
- if (len + namelen + 1 sizeof(path))
+ strcpy(path + len, de-d_name);
+
+ if
Re: overriding/removing inherited credential.helper, Do not add an empty value from config credential.helper
Ted Zlatanov t...@lifelogs.com writes: Below is current git message when a local config credential.helper has an empty value. Please skip an empty value. $ git push --force origin master git: 'credential-' is not a git command. See 'git --help'. Did you mean this? credential Why isn't do not add empty string, or any random string that ends up referring to a helper that you do not have a solution? Total 0 (delta 0), reused 0 (delta 0) To https://u...@github.com/user/myrepo.git + d23aa6a...3405990 master - master (forced update) I would like that too (needed it today). Maybe the empty string (as suggested) or none could be acceptable. Whatever you do, I do not think introducing a per-variable hack [credential] helper = none ;# or helper = clear helper = mine ;# this is the only thing I use like that is a sane way to go. Clear everything you saw so far would be useful for variables other than credential.helper; shouldn't it be done by adding a general syntax to the configuration file format and teach the configuration parser to clear the cumulative definitions so far? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Hiding refs
Jeff King p...@peff.net writes: If the new client can handle the old-style server's response, then the server can start blasting out refs (optionally after a timeout) and stop when the client interrupts with hey, wait, I can speak the new protocol. The server just has to include you can interrupt me in its capability advertisement (obviously it would have to send out at least the first ref with the capabilities before the timeout). Yeah, I would prefer people to come up with a way to share the port and autodetect. It is *not* a requirement for the updated server to run on a separate port at all. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
Torsten Bögershausen wrote: t0070 and t1301 fail when running the test suite under cygwin. Skip the failing tests by unsetting POSIXPERM. t1301 does not fail for me. (WIN XP (SP3) on NTFS) [It's so long since I looked, but I'm pretty sure that the failure in t0070 is caused by *git*, not by cygwin not supporting POSIXPERM] Signed-off-by: Torsten Bögershausen tbo...@web.de --- t/test-lib.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 1a6c4ab..94b097e 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -669,7 +669,6 @@ case $(uname -s) in test_set_prereq SED_STRIPS_CR ;; *CYGWIN*) - test_set_prereq POSIXPERM test_set_prereq EXECKEEPSPID test_set_prereq NOT_MINGW test_set_prereq SED_STRIPS_CR So, I'm not in favour of this, FWIW. ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: overriding/removing inherited credential.helper, Do not add an empty value from config credential.helper
On Thu, 07 Feb 2013 10:23:20 -0800 Junio C Hamano gits...@pobox.com wrote: JCH Clear everything you saw so far would be useful for variables JCH other than credential.helper; shouldn't it be done by adding a JCH general syntax to the configuration file format and teach the JCH configuration parser to clear the cumulative definitions so far? That's a question for the configuration parser, right? I don't have an informed opinion on how it should be implemented, and am OK with any syntax to clear a variable. Ted -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] git-send-email: use git credential to obtain password
Michal Nazarewicz m...@google.com writes:
From: Michal Nazarewicz min...@mina86.com
If smtp_user is provided but smtp_pass is not, instead of
prompting for password, make git-send-email use git
credential command instead.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
Nice ;-)
I'd expect reviews on 4/5 from Peff and Matthiew which may result in
either Reviewed-by:'s or another round, but everything else looks in
good order.
Thanks to all three of you for working on this.
Documentation/git-send-email.txt | 4 +--
git-send-email.perl | 59
+++-
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-send-email.txt
b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
Furthermore, passwords need not be specified in configuration files
or on the command line. If a username has been specified (with
'--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
--smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
}
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+ if (!defined $smtp_authuser || $auth) {
+ return 1;
+ }
+
+ # Workaround AUTH PLAIN/LOGIN interaction defect
+ # with Authen::SASL::Cyrus
+ eval {
+ require Authen::SASL;
+ Authen::SASL-import(qw(Perl));
+ };
+
+ # TODO: Authentication may fail not because credentials were
+ # invalid but due to other reasons, in which we should not
+ # reject credentials.
+ $auth = Git::credential({
+ 'protocol' = 'smtp',
+ 'host' = join(':', $smtp_server, $smtp_server_port),
+ 'username' = $smtp_authuser,
+ # if there's no password, git credential fill will
+ # give us one, otherwise it'll just pass this one.
+ 'password' = $smtp_authpass
+ }, sub {
+ my $cred = shift;
+ return !!$smtp-auth($cred-{'username'}, $cred-{'password'});
+ });
+
+ return $auth;
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ?
port=$smtp_server_port : ;
}
- if (defined $smtp_authuser) {
- # Workaround AUTH PLAIN/LOGIN interaction defect
- # with Authen::SASL::Cyrus
- eval {
- require Authen::SASL;
- Authen::SASL-import(qw(Perl));
- };
-
- if (!defined $smtp_authpass) {
-
- system stty -echo;
-
- do {
- print Password: ;
- $_ = STDIN;
- print \n;
- } while (!defined $_);
-
- chomp($smtp_authpass = $_);
-
- system stty echo;
- }
-
- $auth ||= $smtp-auth( $smtp_authuser, $smtp_authpass )
or die $smtp-message;
- }
+ smtp_auth_maybe or die $smtp-message;
$smtp-mail( $raw_from ) or die $smtp-message;
$smtp-to( @recipients ) or die $smtp-message;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
Michal Nazarewicz m...@google.com writes: From: Michal Nazarewicz min...@mina86.com Add a credential() function which is an interface to the git credential command. The code is heavily based on credential_* functions in contrib/mw-to-git/git-remote-mediawiki. I'm no perl expert, so I cannot comment much on style (there are many small changes compared to the mediawiki code that look like improvement though), but: Reviewed-by: Matthieu Moy matthieu@imag.fr -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Preventing merges from one branch into others
[tried IRC to no avail] I've been trying to find a way to prevent myself from merging a client-specific branch back into my dev/master branches. Is there an easy/straightforward way to do this (perhaps via a hook)? I didn't see any sort of pre-merge hook script. Visualized: A - B - C [dev] \ - Q - R - S [customer-specific] and I want to ensure that changes Q/R/S never find their way back into dev. So I was hoping some sort of hey, you're an idiot for trying to merge $CUSTOMER_BRANCH back into dev/master warning. :-) -tkc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Makefile: extract perl-related rules to make them available from other dirs
Matthieu Moy matthieu@imag.fr writes: The final goal is to make it easy to write Git commands in perl in the contrib/ directory. It is currently possible to do so, but without the benefits of Git's Makefile: adapt first line with $(PERL_PATH), hardcode the path to Git.pm, ... We make the perl-related part of the Makefile available from directories other than the toplevel so that: * Developers can include it, to avoid code duplication * Users can get a consistent behavior of make install Signed-off-by: Matthieu Moy matthieu@imag.fr The goal may be worthy, but the split does not look quite right. What business do contrib/ scripts have knowing how gitweb and git-instaweb are built and what they depend on, for example? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] Makefile: factor common configuration in git-default-config.mak
Matthieu Moy matthieu@imag.fr writes: Similarly to the extraction of perl-related code in perl.mak, we extract general default configuration from the Makefile to make it available from directories other than the toplevel. This is required to make perl.mak usable because it requires $(pathsep) to be set. Signed-off-by: Matthieu Moy matthieu@imag.fr --- I really think this is going in a wrong direction. Whatever you happen to have chosen in this patch will be available to others, while whatever are left out will not be. When adding new things, people need to ask if it needs to be sharable or not, and the right answer to that question will even change over time. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script
Matthieu Moy matthieu@imag.fr writes: The configuration of the install directory is not reused from the toplevel Makefile: we assume Git is already built, hence just call git --exec-path. This avoids too much surgery in the toplevel Makefile. git-remote-mediawiki.perl can now use Git;. Signed-off-by: Matthieu Moy matthieu@imag.fr --- Continuing to the comment on 3/4, I wonder if it would be a lot simpler and more maintainable if you replaced 1/4 to 3/4 with a smaller patch to the top-level Makefile to teach it to munge arbitrary path/to/foo.perl to path/to/foo the same way as we do to other path/tool.perl that are known to the top-level Makefile (similarly, another target to install the resulting path/to/foo at an arbitrary place). Then do something like all:: $(MAKE) -C ../.. \ PERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \ build-perl-script install:: $(MAKE) -C ../.. \ PERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \ install-perl-script in this step. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
Ramsay Jones ram...@ramsay1.demon.co.uk writes: Torsten Bögershausen wrote: t0070 and t1301 fail when running the test suite under cygwin. Skip the failing tests by unsetting POSIXPERM. t1301 does not fail for me. (WIN XP (SP3) on NTFS) Others run Cygwin with vfat or some other filesystem, and some of them do not cope will with POSIXPERM, perhaps? Not having POSIXPERM by default for Cygwin may be a saner default than having one, if we have to pick one. It may be debatable to have this default as platform attribute, though. Signed-off-by: Torsten Bögershausen tbo...@web.de --- t/test-lib.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 1a6c4ab..94b097e 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -669,7 +669,6 @@ case $(uname -s) in test_set_prereq SED_STRIPS_CR ;; *CYGWIN*) -test_set_prereq POSIXPERM test_set_prereq EXECKEEPSPID test_set_prereq NOT_MINGW test_set_prereq SED_STRIPS_CR So, I'm not in favour of this, FWIW. ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Preventing merges from one branch into others
Tim Chase g...@tim.thechases.com writes: ... I didn't see any sort of pre-merge hook script. http://thread.gmane.org/gmane.comp.version-control.git/94111/focus=71069 I think yours is a canonical example of I do not want to run command X when these conditions hold, when these conditions can be checked locally _before_ you decide to do X. While it is theoretically possible to give X a pre-X hook in such a case, we do not want it to be the first choice to avoid hook bloat. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Use __VA_ARGS__ for all of error's arguments
From: Matt Kraai matt.kr...@amo.abbott.com QNX 6.3.2 uses GCC 2.95.3 by default, and GCC 2.95.3 doesn't remove the comma if the error macro's variable argument is left out. Instead of testing for a sufficiently recent version of GCC, make __VA_ARGS__ match all of the arguments. Since this should work on any C99-compliant compiler, also remove the tests around the macro definition. Signed-off-by: Matt Kraai matt.kr...@amo.abbott.com --- git-compat-util.h | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index cc2abee..df1681f 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -305,14 +305,9 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2))) /* * Let callers be aware of the constant return value; this can help - * gcc with -Wuninitialized analysis. We have to restrict this trick to - * gcc, though, because of the variadic macro and the magic ## comma pasting - * behavior. But since we're only trying to help gcc, anyway, it's OK; other - * compilers will fall back to using the function as usual. + * gcc with -Wuninitialized analysis. */ -#if defined(__GNUC__) ! defined(__clang__) -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1) -#endif +#define error(...) (error(__VA_ARGS__), -1) extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params)); extern void set_error_routine(void (*routine)(const char *err, va_list params)); -- 1.8.1.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: branch.name.remotepush
Ramkumar Ramachandra wrote: Ramkumar Ramachandra wrote: And yes, a regular `git push origin refs/for/master` is just retarded. Actually a git config remote.origin.push refs/heads/*:refs/for/* makes more sense here. Sorry about all that confusion. The first line should be `git push origin master:refs/for/master`, but a rule like refs/head/*:refs/for/* is insufficient: what if I want refs/head/*:refs/heads/* for one set of branches (private ones that I don't send for review), and refs/heads/*:refs/for/* for another set (which I send for review)? That certainly won't play well will the existing remote.origin.push; it'd have to behave as an override. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] diff.c: use diff_line_prefix() where applicable
Signed-off-by: John Keeping j...@keeping.me.uk
---
diff.c | 115 -
1 file changed, 20 insertions(+), 95 deletions(-)
diff --git a/diff.c b/diff.c
index ed14d5d..f441f6c 100644
--- a/diff.c
+++ b/diff.c
@@ -402,12 +402,7 @@ static void emit_line_0(struct diff_options *o, const char
*set, const char *res
int nofirst;
FILE *file = o-file;
- if (o-output_prefix) {
- struct strbuf *msg = NULL;
- msg = o-output_prefix(o, o-output_prefix_data);
- assert(msg);
- fwrite(msg-buf, msg-len, 1, file);
- }
+ fputs(diff_line_prefix(o), file);
if (len == 0) {
has_trailing_newline = (first == '\n');
@@ -625,13 +620,7 @@ static void emit_rewrite_diff(const char *name_a,
char *data_one, *data_two;
size_t size_one, size_two;
struct emit_callback ecbdata;
- char *line_prefix = ;
- struct strbuf *msgbuf;
-
- if (o o-output_prefix) {
- msgbuf = o-output_prefix(o, o-output_prefix_data);
- line_prefix = msgbuf-buf;
- }
+ const char *line_prefix = diff_line_prefix(o);
if (diff_mnemonic_prefix DIFF_OPT_TST(o, REVERSE_DIFF)) {
a_prefix = o-b_prefix;
@@ -827,18 +816,14 @@ static void fn_out_diff_words_aux(void *priv, char *line,
unsigned long len)
int minus_first, minus_len, plus_first, plus_len;
const char *minus_begin, *minus_end, *plus_begin, *plus_end;
struct diff_options *opt = diff_words-opt;
- struct strbuf *msgbuf;
- char *line_prefix = ;
+ const char *line_prefix;
if (line[0] != '@' || parse_hunk_header(line, len,
minus_first, minus_len, plus_first, plus_len))
return;
assert(opt);
- if (opt-output_prefix) {
- msgbuf = opt-output_prefix(opt, opt-output_prefix_data);
- line_prefix = msgbuf-buf;
- }
+ line_prefix = diff_line_prefix(opt);
/* POSIX requires that first be decremented by one if len == 0... */
if (minus_len) {
@@ -962,14 +947,10 @@ static void diff_words_show(struct diff_words_data
*diff_words)
struct diff_words_style *style = diff_words-style;
struct diff_options *opt = diff_words-opt;
- struct strbuf *msgbuf;
- char *line_prefix = ;
+ const char *line_prefix;
assert(opt);
- if (opt-output_prefix) {
- msgbuf = opt-output_prefix(opt, opt-output_prefix_data);
- line_prefix = msgbuf-buf;
- }
+ line_prefix = diff_line_prefix(opt);
/* special case: only removal */
if (!diff_words-plus.text.size) {
@@ -1155,13 +1136,7 @@ static void fn_out_consume(void *priv, char *line,
unsigned long len)
const char *plain = diff_get_color(ecbdata-color_diff, DIFF_PLAIN);
const char *reset = diff_get_color(ecbdata-color_diff, DIFF_RESET);
struct diff_options *o = ecbdata-opt;
- char *line_prefix = ;
- struct strbuf *msgbuf;
-
- if (o o-output_prefix) {
- msgbuf = o-output_prefix(o, o-output_prefix_data);
- line_prefix = msgbuf-buf;
- }
+ const char *line_prefix = diff_line_prefix(o);
if (ecbdata-header) {
fprintf(ecbdata-opt-file, %s, ecbdata-header-buf);
@@ -1485,16 +1460,11 @@ static void show_stats(struct diffstat_t *data, struct
diff_options *options)
const char *reset, *add_c, *del_c;
const char *line_prefix = ;
int extra_shown = 0;
- struct strbuf *msg = NULL;
if (data-nr == 0)
return;
- if (options-output_prefix) {
- msg = options-output_prefix(options,
options-output_prefix_data);
- line_prefix = msg-buf;
- }
-
+ line_prefix = diff_line_prefix(options);
count = options-stat_count ? options-stat_count : data-nr;
reset = diff_get_color_opt(options, DIFF_RESET);
@@ -1746,12 +1716,7 @@ static void show_shortstats(struct diffstat_t *data,
struct diff_options *option
dels += deleted;
}
}
- if (options-output_prefix) {
- struct strbuf *msg = NULL;
- msg = options-output_prefix(options,
- options-output_prefix_data);
- fprintf(options-file, %s, msg-buf);
- }
+ fprintf(options-file, %s, diff_line_prefix(options));
print_stat_summary(options-file, total_files, adds, dels);
}
@@ -1765,12 +1730,7 @@ static void show_numstat(struct diffstat_t *data, struct
diff_options *options)
for (i = 0; i data-nr; i++) {
struct diffstat_file *file = data-files[i];
- if (options-output_prefix) {
- struct strbuf *msg = NULL;
- msg =
[PATCH 3/6] diff.c: make constant string arguments const
Signed-off-by: John Keeping j...@keeping.me.uk
---
diff.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index bf95235..73ae02d 100644
--- a/diff.c
+++ b/diff.c
@@ -2123,7 +2123,8 @@ static unsigned char *deflate_it(char *data,
return deflated;
}
-static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two,
char *prefix)
+static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two,
+ const char *prefix)
{
void *cp;
void *delta;
@@ -2184,7 +2185,8 @@ static void emit_binary_diff_body(FILE *file, mmfile_t
*one, mmfile_t *two, char
free(data);
}
-static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two, char
*prefix)
+static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two,
+const char *prefix)
{
fprintf(file, %sGIT binary patch\n, prefix);
emit_binary_diff_body(file, one, two, prefix);
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] diff: write prefix to the correct file
Write the prefix for an output line to the same file as the actual
content.
Signed-off-by: John Keeping j...@keeping.me.uk
---
diff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index 348f71b..bf95235 100644
--- a/diff.c
+++ b/diff.c
@@ -4485,7 +4485,7 @@ void diff_flush(struct diff_options *options)
struct strbuf *msg = NULL;
msg = options-output_prefix(options,
options-output_prefix_data);
- fwrite(msg-buf, msg-len, 1, stdout);
+ fwrite(msg-buf, msg-len, 1, options-file);
}
putc(options-line_termination, options-file);
if (options-stat_sep) {
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] graph: output padding for merge subsequent parents
When showing merges in git-log, the same commit is shown once for each
parent. Combined with --graph this results in graph_show_commit()
being called once for each parent without graph_update() being called.
Currently graph_show_commit() does not print anything on subsequent
invocations for the same commit (this was changed by commit 656197a -
graph.c: infinite loop in git whatchanged --graph -m from the previous
behaviour of looping infinitely).
Change this so that if the graph code believes it has already shown the
commit it prints a single padding line.
Signed-off-by: John Keeping j...@keeping.me.uk
---
graph.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/graph.c b/graph.c
index 391a712..2a3fc5c 100644
--- a/graph.c
+++ b/graph.c
@@ -1227,6 +1227,16 @@ void graph_show_commit(struct git_graph *graph)
if (!graph)
return;
+ /*
+* When showing a diff of a merge against each of its parents, we
+* are called once for each parent without graph_update having been
+* called. In this case, simply output a single padding line.
+*/
+ if (graph_is_commit_finished(graph)) {
+ graph_show_padding(graph);
+ shown_commit_line = 1;
+ }
+
while (!shown_commit_line !graph_is_commit_finished(graph)) {
shown_commit_line = graph_next_line(graph, msgbuf);
fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] diff: add diff_line_prefix function
This is a helper function to call the diff output_prefix function and
return its value as a C string, allowing us to greatly simplify
everywhere that needs to get the output prefix.
Signed-off-by: John Keeping j...@keeping.me.uk
---
diff.c | 10 ++
diff.h | 3 +++
2 files changed, 13 insertions(+)
diff --git a/diff.c b/diff.c
index 73ae02d..ed14d5d 100644
--- a/diff.c
+++ b/diff.c
@@ -1105,6 +1105,16 @@ const char *diff_get_color(int diff_use_color, enum
color_diff ix)
return ;
}
+const char *diff_line_prefix(struct diff_options *opt)
+{
+ struct strbuf *msgbuf;
+ if (!opt-output_prefix)
+ return ;
+
+ msgbuf = opt-output_prefix(opt, opt-output_prefix_data);
+ return msgbuf-buf;
+}
+
static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line,
unsigned long len)
{
const char *cp;
diff --git a/diff.h b/diff.h
index a47bae4..76830e2 100644
--- a/diff.h
+++ b/diff.h
@@ -174,6 +174,9 @@ const char *diff_get_color(int diff_use_color, enum
color_diff ix);
diff_get_color((o)-use_color, ix)
+const char *diff_line_prefix(struct diff_options *);
+
+
extern const char mime_boundary_leader[];
extern void diff_tree_setup_paths(const char **paths, struct diff_options *);
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] Improve git log --graph output of merges
This series changes a couple of places that do not currently indent their output when being shown with a graph. The first patch was already posted [1] and addresses the output of git log --graph -c -p. Patch 2 is an independent fix I noticed while working on the later patches. Patches 3-5 introduce a helper function and change existing sites using diff_options-output_prefix to use it, resulting in a net reduction by about 60 lines. There is no functional change here. The final patch uses the helper introduced in patch 4 to make combined diffs should the output prefix before each line. This affects the output of git log --graph --cc [-p|--raw]. [1] http://article.gmane.org/gmane.comp.version-control.git/215630 John Keeping (6): graph: output padding for merge subsequent parents diff: write prefix to the correct file diff.c: make constant string arguments const diff: add diff_line_prefix function diff.c: use diff_line_prefix() where applicable combine-diff.c: teach combined diffs about line prefix combine-diff.c | 47 + diff.c | 131 +++-- diff.h | 3 ++ graph.c| 10 + 4 files changed, 77 insertions(+), 114 deletions(-) -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] combine-diff.c: teach combined diffs about line prefix
When running git log --graph --cc -p the diff output for merges is not
indented by the graph structure, unlike the diffs of non-merge commits
(added in commit 7be5761 - diff.c: Output the text graph padding before
each diff line).
Fix this by teaching the combined diff code to output diff_line_prefix()
before each line.
Signed-off-by: John Keeping j...@keeping.me.uk
---
combine-diff.c | 47 ++-
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index bb1cc96..35c817b 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -526,7 +526,8 @@ static void show_line_to_eol(const char *line, int len,
const char *reset)
saw_cr_at_eol ? \r : );
}
-static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
+static void dump_sline(struct sline *sline, const char *line_prefix,
+ unsigned long cnt, int num_parent,
int use_color, int result_deleted)
{
unsigned long mark = (1ULnum_parent);
@@ -582,7 +583,7 @@ static void dump_sline(struct sline *sline, unsigned long
cnt, int num_parent,
rlines -= null_context;
}
- fputs(c_frag, stdout);
+ printf(%s%s, line_prefix, c_frag);
for (i = 0; i = num_parent; i++) putchar(combine_marker);
for (i = 0; i num_parent; i++)
show_parent_lno(sline, lno, hunk_end, i, null_context);
@@ -614,7 +615,7 @@ static void dump_sline(struct sline *sline, unsigned long
cnt, int num_parent,
struct sline *sl = sline[lno++];
ll = (sl-flag no_pre_delete) ? NULL : sl-lost_head;
while (ll) {
- fputs(c_old, stdout);
+ printf(%s%s, line_prefix, c_old);
for (j = 0; j num_parent; j++) {
if (ll-parent_map (1ULj))
putchar('-');
@@ -627,6 +628,7 @@ static void dump_sline(struct sline *sline, unsigned long
cnt, int num_parent,
if (cnt lno)
break;
p_mask = 1;
+ fputs(line_prefix, stdout);
if (!(sl-flag (mark-1))) {
/*
* This sline was here to hang the
@@ -680,11 +682,13 @@ static void reuse_combine_diff(struct sline *sline,
unsigned long cnt,
static void dump_quoted_path(const char *head,
const char *prefix,
const char *path,
+const char *line_prefix,
const char *c_meta, const char *c_reset)
{
static struct strbuf buf = STRBUF_INIT;
strbuf_reset(buf);
+ strbuf_addstr(buf, line_prefix);
strbuf_addstr(buf, c_meta);
strbuf_addstr(buf, head);
quote_two_c_style(buf, prefix, path, 0);
@@ -696,6 +700,7 @@ static void show_combined_header(struct combine_diff_path
*elem,
int num_parent,
int dense,
struct rev_info *rev,
+const char *line_prefix,
int mode_differs,
int show_file_header)
{
@@ -714,8 +719,8 @@ static void show_combined_header(struct combine_diff_path
*elem,
show_log(rev);
dump_quoted_path(dense ? diff --cc : diff --combined ,
-, elem-path, c_meta, c_reset);
- printf(%sindex , c_meta);
+, elem-path, line_prefix, c_meta, c_reset);
+ printf(%s%sindex , line_prefix, c_meta);
for (i = 0; i num_parent; i++) {
abb = find_unique_abbrev(elem-parent[i].sha1,
abbrev);
@@ -734,11 +739,12 @@ static void show_combined_header(struct combine_diff_path
*elem,
DIFF_STATUS_ADDED)
added = 0;
if (added)
- printf(%snew file mode %06o,
- c_meta, elem-mode);
+ printf(%s%snew file mode %06o,
+ line_prefix, c_meta, elem-mode);
else {
if (deleted)
- printf(%sdeleted file , c_meta);
+ printf(%s%sdeleted file ,
+ line_prefix, c_meta);
printf(mode );
for (i = 0; i num_parent; i++) {
printf(%s%06o, i ? , : ,
@@ -755,16 +761,16 @@ static void show_combined_header(struct combine_diff_path
*elem,
Re: [PATCH 0/6] Improve git log --graph output of merges
John Keeping j...@keeping.me.uk writes: This series changes a couple of places that do not currently indent their output when being shown with a graph. The first patch was already posted [1] and addresses the output of git log --graph -c -p. Patch 2 is an independent fix I noticed while working on the later patches. Patches 3-5 introduce a helper function and change existing sites using diff_options-output_prefix to use it, resulting in a net reduction by about 60 lines. There is no functional change here. The final patch uses the helper introduced in patch 4 to make combined diffs should the output prefix before each line. This affects the output of git log --graph --cc [-p|--raw]. [1] http://article.gmane.org/gmane.comp.version-control.git/215630 Overall direction looks sensible. It is a bit scary that we are doing the the fifth one now, as opposed to catching this repetitiveness during the initial review of patch series that added the graph code to the diff codepath. Thanks. John Keeping (6): graph: output padding for merge subsequent parents diff: write prefix to the correct file diff.c: make constant string arguments const diff: add diff_line_prefix function diff.c: use diff_line_prefix() where applicable combine-diff.c: teach combined diffs about line prefix combine-diff.c | 47 + diff.c | 131 +++-- diff.h | 3 ++ graph.c| 10 + 4 files changed, 77 insertions(+), 114 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
Matt Kraai kr...@ftbfs.org writes: -#if defined(__GNUC__) ! defined(__clang__) -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1) -#endif +#define error(...) (error(__VA_ARGS__), -1) Before your change, we only define error() macro for GCC variants, but with your patch that no longer is the case. Does every compiler that compiles Git correctly today support this style of varargs macros? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
On Thu, Feb 07, 2013 at 01:05:19PM -0800, Junio C Hamano wrote: Matt Kraai kr...@ftbfs.org writes: -#if defined(__GNUC__) ! defined(__clang__) -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1) -#endif +#define error(...) (error(__VA_ARGS__), -1) Before your change, we only define error() macro for GCC variants, but with your patch that no longer is the case. Does every compiler that compiles Git correctly today support this style of varargs macros? At the very least the ! defined(__clang__) was only recently added [1], although it shouldn't be needed once Clang 3.3 is released. [1] http://article.gmane.org/gmane.comp.version-control.git/213787 John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
On Thu, Feb 07, 2013 at 01:05:19PM -0800, Junio C Hamano wrote: Matt Kraai kr...@ftbfs.org writes: -#if defined(__GNUC__) ! defined(__clang__) -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1) -#endif +#define error(...) (error(__VA_ARGS__), -1) Before your change, we only define error() macro for GCC variants, but with your patch that no longer is the case. Does every compiler that compiles Git correctly today support this style of varargs macros? I don't know and I don't think it's likely I can confirm this. I'll submit a new patch that just changes the definition, since I don't know of any problems other than mine with the current situation. -- Matt -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Use __VA_ARGS__ for all of error's arguments
From: Matt Kraai matt.kr...@amo.abbott.com QNX 6.3.2 uses GCC 2.95.3 by default, and GCC 2.95.3 doesn't remove the comma if the error macro's variable argument is left out. Instead of testing for a sufficiently recent version of GCC, make __VA_ARGS__ match all of the arguments. Signed-off-by: Matt Kraai matt.kr...@amo.abbott.com --- git-compat-util.h | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index cc2abee..2e960a9 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -305,13 +305,10 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2))) /* * Let callers be aware of the constant return value; this can help - * gcc with -Wuninitialized analysis. We have to restrict this trick to - * gcc, though, because of the variadic macro and the magic ## comma pasting - * behavior. But since we're only trying to help gcc, anyway, it's OK; other - * compilers will fall back to using the function as usual. + * gcc with -Wuninitialized analysis. */ #if defined(__GNUC__) ! defined(__clang__) -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1) +#define error(...) (error(__VA_ARGS__), -1) #endif extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params)); -- 1.8.1.2.546.gfc9f004 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: branch.name.remotepush
Hi Ram, Ramkumar Ramachandra wrote: And yes, a regular `git push origin refs/for/master` is just retarded. The usual incantation is git push gerrit HEAD:refs/for/master. Is the code review creation push that uses a different branchname from the branch the integrator pulls what seems backward, or is it the need to specify a refname at all on the command line? I agree that a [branch master] pushremote configuration would be handy. pushremote instead of remotepush to be less surprising to people who have already seen pushurl. Good luck, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
Matthieu Moy matthieu@grenoble-inp.fr writes: Michal Nazarewicz m...@google.com writes: From: Michal Nazarewicz min...@mina86.com Add a credential() function which is an interface to the git credential command. The code is heavily based on credential_* functions in contrib/mw-to-git/git-remote-mediawiki. I'm no perl expert, so I cannot comment much on style (there are many small changes compared to the mediawiki code that look like improvement though), but: Reviewed-by: Matthieu Moy matthieu@imag.fr Thanks. I'd actually be more worried about the error checking issue Peff raised during his review. I have a feeling that when in doubt, do not cause harm is a more prudent way to go than I do not know, so I'll let anything pass. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: branch.name.remotepush
Jonathan Nieder jrnie...@gmail.com writes: The usual incantation is git push gerrit HEAD:refs/for/master. Is the code review creation push that uses a different branchname from the branch the integrator pulls what seems backward, or is it the need to specify a refname at all on the command line? I agree that a [branch master] pushremote configuration would be handy. pushremote instead of remotepush to be less surprising to people who have already seen pushurl. I'd actually see this as Gerrit being weird. If it wants to quarantine a commit destined to the master branch, couldn't it just let people push to master and then internally update for/master instead? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
Ted Zlatanov t...@lifelogs.com writes: Add Git credential helper that can parse netrc/authinfo files. I think this line is redundant; we already know it on the Subject: line. This credential helper supports multiple files, returning the first one that matches. It checks file permissions and owner. For *.gpg files, it will run GPG to decrypt the file. Signed-off-by: Ted Zlatanov t...@lifelogs.com --- ... diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile new file mode 100644 index 000..18a924f --- /dev/null +++ b/contrib/credential/netrc/Makefile @@ -0,0 +1,12 @@ +test_netrc: + @(echo bad data | ./git-credential-netrc -f A -d -v) || echo Bad invocation test, ignoring failure + @echo = Silent invocation... nothing should show up here with a missing file + @echo bad data | ./git-credential-netrc -f A get + @echo = Back to noisy: -v and -d used below, missing file + echo bad data | ./git-credential-netrc -f A -d -v get + @echo = Look for any entry in the default file set + echo | ./git-credential-netrc -d -v get + @echo = Look for github.com in the default file set + echo host=google.com | ./git-credential-netrc -d -v get + @echo = Look for a nonexistent machine in the default file set + echo host=korovamilkbar | ./git-credential-netrc -d -v get Whose netrc is this reading? Don't we want all of them to have -f A and ship A (rename it to something more reasonable), so that anybody can notice when he tries to improve it and breaks it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] count-objects: report garbage files in .git/objects/pack directory too
On Fri, Feb 8, 2013 at 1:12 AM, Junio C Hamano gits...@pobox.com wrote:
Duy Nguyen pclo...@gmail.com writes:
I thought about that, but we may need to do extra stat() for loose
garbage as well. As it is now, garbage is complained loudly, which
gives me enough motivation to clean up, even without looking at how
much disk space it uses.
I wouldn't call a single line garbage: 4 exactly *loud*. I also
think that this is not about *motivating* you, but about giving
more information to the users to help them assess the health of
their repository themselves.
That's not the only line it prints:
error: garbage found:
.git/objects/pack/pack-8074dfd2b01f494a30f02d0374baa57632d26fea.commits
error: garbage found:
.git/objects/pack/pack-834c9dccca7634c2b225db1b5050a980cb2c2de0.commits
error: garbage found: .git/objects/pack/tmp_pack_G235da
error: garbage found:
.git/objects/pack/pack-8bdf298e9252573289cd4f1e83e80c9f261882a2.commits
count: 604
size: 5576
in-pack: 172307
packs: 4
size-pack: 50421
prune-packable: 4
garbage: 4
By the way, I wonder if we also want to notice .git/objects/garbage
or .git/objects/ga/rbage if we are to do this?
I prefer not (for code simplicity) because we may need to catch
.git/objects/pack/info/garbage too if we do that.
+ if (!has_extension(de-d_name, .pack)) {
+ report_pack_garbage(path, 0, NULL);
+ continue;
+ }
Didn't I already say the logic should be inverted to whitelist the
known ones? Saying Anything that is not '.pack' is bad here is a
direct opposite, I think.
I must have missed it. Will do.
Add A '.keep' file is OK to this codeflow and see how it goes.
OK
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v1.8.1.3
The latest maintenance release Git v1.8.1.3 is now available at the usual places. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 29ed9047263f9835726200226451339276641779 git-1.8.1.3.tar.gz 6b1e57bde2f2b0a86532390c15bfa7b181c50db2 git-htmldocs-1.8.1.3.tar.gz 12aaa8a0428e64d194665379ab0335d786728930 git-manpages-1.8.1.3.tar.gz Also the following public repositories all have a copy of the v1.8.1.3 tag and the maint branch that the tag points at: url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git 1.8.1.3 Release Notes = Fixes since v1.8.1.2 * The attribute mechanism didn't allow limiting attributes to be applied to only a single directory itself with path/ like the exclude mechanism does. The fix for this in 1.8.1.2 had performance degradations. * Command line completion code was inadvertently made incompatible with older versions of bash by using a newer array notation. * Scripts to test bash completion was inherently flaky as it was affected by whatever random things the user may have on $PATH. * A fix was added to the build procedure to work around buggy versions of ccache broke the auto-generation of dependencies, which unfortunately is still relevant because some people use ancient distros. * We used to stuff user@ and then append what we read from /etc/mailname to come up with a default e-mail ident, but a bug lost the user@ part. * git am did not parse datestamp correctly from Hg generated patch, when it is run in a locale outside C (or en). * Attempt to branch --edit-description an existing branch, while being on a detached HEAD, errored out. * git cherry-pick did not replay a root commit to an unborn branch. * We forgot to close the file descriptor reading from gpg output, killing git log --show-signature on a long history. * git rebase --preserve-merges lost empty merges in recent versions of Git. * Rebasing the history of superproject with change in the submodule has been broken since v1.7.12. * A failure to push due to non-ff while on an unborn branch dereferenced a NULL pointer when showing an error message. Also contains various documentation fixes. Changes since v1.8.1.2 are as follows: Brandon Casey (3): git-completion.bash: replace zsh notation that breaks bash 3.X git-p4.py: support Python 2.5 git-p4.py: support Python 2.4 Dmitry V. Levin (1): am: invoke perl's strftime in C locale Fraser Tweedale (1): push: fix segfault when HEAD points nowhere John Keeping (1): git-cvsimport.txt: cvsps-2 is deprecated Jonathan Nieder (2): ident: do not drop username when reading from /etc/mailname Makefile: explicitly set target name for autogenerated dependencies Junio C Hamano (17): Which merge_file() function do you mean? merge-tree: lose unused flags from merge_list merge-tree: lose unused resolve_directories merge-tree: add comments to clarify what these functions are doing merge-tree: fix d/f conflicts Documentation: update howto maintain git howto/maintain: mark titles for asciidoc help: include common-cmds.h only in one file t9902: protect test from stray build artifacts howto/maintain: document ### match next convention in jch/pu branch README: update stale and/or incorrect information INSTALL: git-p4 does not support Python 3 git-am: record full index line in the patch used while rebasing apply: simplify build_fake_ancestor() apply: diagnose incomplete submodule object name better Start preparing for 1.8.1.3 Git 1.8.1.3 Martin von Zweigbergk (2): tests: move test_cmp_rev to test-lib-functions learn to pick/revert into unborn branch Nguyễn Thái Ngọc Duy (4): attr: fix off-by-one directory component length calculation test-lib.sh: unfilter GIT_PERF_* attr: avoid calling find_basename() twice per path branch: no detached HEAD check when editing another branch's description Phil Hord (1): rebase --preserve-merges: keep all merge commits including empty ones Ramsay Allan Jones (1): Makefile: Replace merge-file.h with merge-blobs.h in LIB_H Stephen Boyd (1): gpg: close stderr once finished with it in verify_signed_buffer() Torsten Bögershausen (3): t0050: known breakage vanished in merge (case change) t0050: honor CASE_INSENSITIVE_FS in add (with different case) t0050: Use TAB for indentation -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org
[PATCH] git-mergetool: print filename when it contains %
Before this change, if git-mergetool was invoked with regard to files with a percent sign (%) in their names, it would print an error. For example, if you were calling mergetool on a file called %2F: printf: %2F: invalid directive This changes the behavior to pass %s to printf as its first argument to avoid processing the filename as a format string. Signed-off-by: Asheesh Laroia ashe...@asheesh.org --- git-mergetool.sh |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index c50e18a..d2b9289 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -440,7 +440,7 @@ then fi printf Merging:\n -printf $files\n +printf %s $files\n IFS=' ' -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
On Fri, Feb 08 2013, Junio C Hamano wrote: I'd actually be more worried about the error checking issue Peff raised during his review. I have a feeling that when in doubt, do not cause harm is a more prudent way to go than I do not know, so I'll let anything pass. I can implement whatever checking you wish, just tell me what to do in corner cases I've listed. ;) -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo-- pgpkH3LocTxhw.pgp Description: PGP signature
Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
On Thu, 07 Feb 2013 15:52:41 -0800 Junio C Hamano gits...@pobox.com wrote: +@echo = Look for any entry in the default file set +echo | ./git-credential-netrc -d -v get +@echo = Look for github.com in the default file set +echo host=google.com | ./git-credential-netrc -d -v get +@echo = Look for a nonexistent machine in the default file set +echo host=korovamilkbar | ./git-credential-netrc -d -v get JCH Whose netrc is this reading? JCH Don't we want all of them to have -f A and ship A (rename it to JCH something more reasonable), so that anybody can notice when he tries JCH to improve it and breaks it? I agree this Makefile is not a good test to ship out. It was my quickie test rig that I should have reworked before adding to the patch. Sorry. I see contrib/subtree/t and contrib/mw-to-git/t that I could copy. The test will have a few files to parse, and will be able to compare the expected to the actual output. Does that sound like a good plan? Ted -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/3] count-objects improvements
This series: - updates count-objects -v documentation, describe each line in detail - counts garbage files in pack directory in addition to loose odb - shows how much disk space consumed by garbage files Nguyễn Thái Ngọc Duy (3): git-count-objects.txt: describe each line in -v output count-objects: report garbage files in pack directory too count-objects: report how much disk space taken by garbage files Documentation/git-count-objects.txt | 22 +++--- builtin/count-objects.c | 41 ++- sha1_file.c | 81 +++-- 3 files changed, 127 insertions(+), 17 deletions(-) -- 1.8.1.2.495.g3fdf5d5.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] git-count-objects.txt: describe each line in -v output
The current description requires a bit of guessing (what clause corresponds to what printed line?) and lacks information, such as the unit of size and size-pack. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-count-objects.txt | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt index 23c80ce..e816823 100644 --- a/Documentation/git-count-objects.txt +++ b/Documentation/git-count-objects.txt @@ -20,11 +20,21 @@ OPTIONS --- -v:: --verbose:: - In addition to the number of loose objects and disk - space consumed, it reports the number of in-pack - objects, number of packs, disk space consumed by those packs, - and number of objects that can be removed by running - `git prune-packed`. + Report in more detail: ++ +count: the number of loose objects ++ +size: disk space consumed by loose objects, in KiB ++ +in-pack: the number of in-pack objects ++ +size-pack: disk space consumed by the packs, in KiB ++ +prune-packable: the number of loose objects that are also present in +the packs. These objects could be pruned using `git prune-packed`. ++ +garbage: the number of files in loose object database that are not +valid loose objects GIT --- -- 1.8.1.2.495.g3fdf5d5.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] count-objects: report garbage files in pack directory too
prepare_packed_git_one() is modified to allow count-objects to hook a
report function to so we don't need to duplicate the pack searching
logic in count-objects.c. When report_pack_garbage is NULL, the
overhead is insignificant.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/git-count-objects.txt | 4 +-
builtin/count-objects.c | 18 -
sha1_file.c | 81 +++--
3 files changed, 97 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-count-objects.txt
b/Documentation/git-count-objects.txt
index e816823..1611d7c 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
prune-packable: the number of loose objects that are also present in
the packs. These objects could be pruned using `git prune-packed`.
+
-garbage: the number of files in loose object database that are not
-valid loose objects
+garbage: the number of files in object database that are not valid
+loose objects nor valid packs
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..118b2ae 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -9,6 +9,20 @@
#include builtin.h
#include parse-options.h
+static unsigned long garbage;
+
+extern void (*report_pack_garbage)(const char *path, int len, const char
*name);
+static void real_report_pack_garbage(const char *path, int len, const char
*name)
+{
+ if (len name)
+ error(garbage found: %.*s/%s, len, path, name);
+ else if (!len name)
+ error(garbage found: %s%s, path, name);
+ else
+ error(garbage found: %s, path);
+ garbage++;
+}
+
static void count_objects(DIR *d, char *path, int len, int verbose,
unsigned long *loose,
off_t *loose_size,
@@ -76,7 +90,7 @@ int cmd_count_objects(int argc, const char **argv, const char
*prefix)
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
- unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
+ unsigned long loose = 0, packed = 0, packed_loose = 0;
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
@@ -87,6 +101,8 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
+ if (verbose)
+ report_pack_garbage = real_report_pack_garbage;
memcpy(path, objdir, len);
if (len objdir[len-1] != '/')
path[len++] = '/';
diff --git a/sha1_file.c b/sha1_file.c
index 40b2329..cc6ef03 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -21,6 +21,7 @@
#include sha1-lookup.h
#include bulk-checkin.h
#include streaming.h
+#include dir.h
#ifndef O_NOATIME
#if defined(__linux__) (defined(__i386__) || defined(__PPC__))
@@ -1000,6 +1001,54 @@ void install_packed_git(struct packed_git *pack)
packed_git = pack;
}
+/* A hook for count-objects to report invalid files in pack directory */
+void (*report_pack_garbage)(const char *path, int len, const char *name);
+
+static const char *known_pack_extensions[] = { .pack, .keep, NULL };
+
+static void report_garbage(struct string_list *list)
+{
+ struct strbuf sb = STRBUF_INIT;
+ struct packed_git *p;
+ int i;
+
+ if (!report_pack_garbage)
+ return;
+
+ sort_string_list(list);
+
+ for (p = packed_git; p; p = p-next) {
+ struct string_list_item *item;
+ if (!p-pack_local)
+ continue;
+ strbuf_reset(sb);
+ strbuf_add(sb, p-pack_name,
+ strlen(p-pack_name) - strlen(.pack));
+ item = string_list_lookup(list, sb.buf);
+ if (!item)
+ continue;
+ /*
+* string_list_lookup does not guarantee to return the
+* first matched string if it's duplicated.
+*/
+ while (item - list-items
+ !strcmp(item[-1].string, item-string))
+ item--;
+ while (item - list-items list-nr
+ !strcmp(item-string, sb.buf)) {
+ item-util = NULL; /* non-garbage mark */
+ item++;
+ }
+ }
+ for (i = 0; i list-nr; i++) {
+ struct string_list_item *item = list-items + i;
+ if (!item-util)
+ continue;
+ report_pack_garbage(item-string, 0, item-util);
+ }
+ strbuf_release(sb);
+}
+
static void
[PATCH v2 3/3] count-objects: report how much disk space taken by garbage files
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
We may do some redundant stat() here, but I don't think it can slow
count-objects down much to worry about.
Documentation/git-count-objects.txt | 2 ++
builtin/count-objects.c | 29 ++---
2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-count-objects.txt
b/Documentation/git-count-objects.txt
index 1611d7c..da6e72e 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -35,6 +35,8 @@ the packs. These objects could be pruned using `git
prune-packed`.
+
garbage: the number of files in object database that are not valid
loose objects nor valid packs
++
+size-garbage: disk space consumed by garbage files, in KiB
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 118b2ae..90d476d 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -10,24 +10,33 @@
#include parse-options.h
static unsigned long garbage;
+static off_t size_garbage;
extern void (*report_pack_garbage)(const char *path, int len, const char
*name);
static void real_report_pack_garbage(const char *path, int len, const char
*name)
{
+ struct strbuf sb = STRBUF_INIT;
+ struct stat st;
+
if (len name)
- error(garbage found: %.*s/%s, len, path, name);
+ strbuf_addf(sb, %.*s/%s, len, path, name);
else if (!len name)
- error(garbage found: %s%s, path, name);
+ strbuf_addf(sb, %s%s, path, name);
else
- error(garbage found: %s, path);
+ strbuf_addf(sb, %s, path);
+ error(_(garbage found: %s), sb.buf);
+
+ if (!stat(sb.buf, st))
+ size_garbage += st.st_size;
+
garbage++;
+ strbuf_release(sb);
}
static void count_objects(DIR *d, char *path, int len, int verbose,
unsigned long *loose,
off_t *loose_size,
- unsigned long *packed_loose,
- unsigned long *garbage)
+ unsigned long *packed_loose)
{
struct dirent *ent;
while ((ent = readdir(d)) != NULL) {
@@ -59,11 +68,8 @@ static void count_objects(DIR *d, char *path, int len, int
verbose,
(*loose_size) += xsize_t(on_disk_bytes(st));
}
if (bad) {
- if (verbose) {
- error(garbage found: %.*s/%s,
- len + 2, path, ent-d_name);
- (*garbage)++;
- }
+ if (verbose)
+ report_pack_garbage(path, len + 2, ent-d_name);
continue;
}
(*loose)++;
@@ -113,7 +119,7 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
if (!d)
continue;
count_objects(d, path, len, verbose,
- loose, loose_size, packed_loose, garbage);
+ loose, loose_size, packed_loose);
closedir(d);
}
if (verbose) {
@@ -138,6 +144,7 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
printf(size-pack: %lu\n, (unsigned long) (size_pack / 1024));
printf(prune-packable: %lu\n, packed_loose);
printf(garbage: %lu\n, garbage);
+ printf(size-garbage: %lu\n, (unsigned long) (size_garbage /
1024));
}
else
printf(%lu objects, %lu kilobytes\n,
--
1.8.1.2.495.g3fdf5d5.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
On Thu, Feb 07, 2013 at 01:30:32PM -0800, Matt Kraai wrote: From: Matt Kraai matt.kr...@amo.abbott.com QNX 6.3.2 uses GCC 2.95.3 by default, and GCC 2.95.3 doesn't remove the comma if the error macro's variable argument is left out. Instead of testing for a sufficiently recent version of GCC, make __VA_ARGS__ match all of the arguments. Thanks, this looks better than the original (we do not assume a C99 compiler, so just doing this unconditionally would probably break some other older systems which do not use gcc). /* * Let callers be aware of the constant return value; this can help - * gcc with -Wuninitialized analysis. We have to restrict this trick to - * gcc, though, because of the variadic macro and the magic ## comma pasting - * behavior. But since we're only trying to help gcc, anyway, it's OK; other - * compilers will fall back to using the function as usual. + * gcc with -Wuninitialized analysis. */ #if defined(__GNUC__) ! defined(__clang__) -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1) +#define error(...) (error(__VA_ARGS__), -1) Should you be dropping most of the comment like this? I would expect it to be more like: We have to restrict this trick to gcc, though, because we do not assume all compilers support variadic macros. But since... Other than that, I think it is OK. The compiler will still catch error() with no arguments and generate the appropriate diagnostic (in fact, it is better, because the error is now passing too few args to a function, not to the macro). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script
On Thu, Feb 07, 2013 at 11:28:31AM -0800, Junio C Hamano wrote: Matthieu Moy matthieu@imag.fr writes: The configuration of the install directory is not reused from the toplevel Makefile: we assume Git is already built, hence just call git --exec-path. This avoids too much surgery in the toplevel Makefile. git-remote-mediawiki.perl can now use Git;. Signed-off-by: Matthieu Moy matthieu@imag.fr --- Continuing to the comment on 3/4, I wonder if it would be a lot simpler and more maintainable if you replaced 1/4 to 3/4 with a smaller patch to the top-level Makefile to teach it to munge arbitrary path/to/foo.perl to path/to/foo the same way as we do to other path/tool.perl that are known to the top-level Makefile (similarly, another target to install the resulting path/to/foo at an arbitrary place). Then do something like all:: $(MAKE) -C ../.. \ PERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \ build-perl-script install:: $(MAKE) -C ../.. \ PERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \ install-perl-script in this step. That seems much cleaner to me. If done right, it could also let people put: CONTRIB_PERL += contrib/mw-to-git/git-remote-mediawiki or similar into their config.mak, and just get specific contrib bits built and installed along with the rest of git. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
On Thu, Feb 07, 2013 at 11:24:28PM -0500, Jeff King wrote: Should you be dropping most of the comment like this? I would expect it to be more like: We have to restrict this trick to gcc, though, because we do not assume all compilers support variadic macros. But since... I'll submit a new patch with this change tomorrow. Other than that, I think it is OK. The compiler will still catch error() with no arguments and generate the appropriate diagnostic (in fact, it is better, because the error is now passing too few args to a function, not to the macro). Great, thanks for the review. -- Matt -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: branch.name.remotepush
On Thu, Feb 07, 2013 at 09:44:59PM +0530, Ramkumar Ramachandra wrote: This has been annoying me for a really long time, but I never really got around to scratching this particular itch. I have a very common scenario where I fork a project on GitHub. I have two configured remotes: origin which points to git://upstream and mine which points to ssh://mine. By default, I always want to pull `master` from origin and push to mine. Same here. Even without GitHub, working on git.git I treat Junio as my origin, but push to a publishing point. Unfortunately, there's only a branch.name.remote which specifies which remote to use for both pulling and pushing. There's also a remote.name.pushurl, but I get the feeling that this exists for an entirely different reason: when I have a server with a highly-available read-only mirror of the repository at git://anongit.*, and a less-available committer-only mirror at ssh://*. Yeah, you don't want to use pushurl. It makes the assumption that you are pushing to the same remote, so when you, e.g., push to the remote's refs/heads/master, it will update refs/remotes/origin/master. But that's not right; that ref should be tracking the true origin, not what you pushed to. How about a branch.name.remotepush that specifies a special remote for pushing, falling back to branch.name.remote? Sure, though I wonder if you really want a per-branch config, or if you just want remote.pushDefault or similar, so that you do not have to configure each branch independently as you create it. I'm imagining lookup rules something like: 1. If we are on branch $b, check branch.$b.pushRemote. 2. If not set, check remote.pushDefault. 3. If not set, check branch.$b.remote. 4. If not set, check remote.default (there was a proposal for this a few months ago, but it got stalled). 5. If not set, use origin. And then fetching could do the same, with s/push/fetch/. In both cases, if you are not using the new variables, the behavior is the same as the current behavior. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
On Thu, Feb 07, 2013 at 03:01:20PM +0100, Michal Nazarewicz wrote:
There are a few disallowed characters, like \n in key or value, and
= in a key. They should never happen unless the caller is buggy, but
should we check and catch them here?
I left it as is for now since it's not entairly clear to me what to
do in all cases. In particular:
- when reading, what to do if the line is foo = bar ,
According to the spec, whitespace (except for the final newline) is not
significant, and that parses key= foo , value= bar . The spec could
ignore whitespace on the key side, but I intentionally did not in an
attempt to keep the protocol simple. Your original implementation did
the right thing already.
- when reading, what to do if the line is foo= (ie. empty value),
The empty string is a valid value.
- when writing, what to do if value is a single space,
Then it's a single space. It's the caller's problem whether that is an
issue or not.
- when writing, what to do if value ends with a new line,
That's bogus. We cannot represent that value. I'd suggest to simply die,
as it is a bug in the caller (we _could_ try to be nice and assume the
caller accidentally forgot to chomp, but I'd rather be careful than
nice).
- when writing, what to do if value is empty (currently not printed at all),
I think you should still print it. It's unlikely to matter, but
technically a helper response may override keys (or set them to blank),
and the intermediate state gets sent on to the next helper, if there are
multiple.
On Thu, Feb 07 2013, Matthieu Moy matthieu@grenoble-inp.fr wrote:
I think you should credit git-remote-mediawiki for the code in the
commit message. Perhaps have a first copy/paste commit, and then an
adaptation commit to add sort, ^ anchor in regexp, doc and your
callback mechanism, but I won't insist on that.
Good point. Creating additional commit is a bit too much for my
licking, but added note in commit message.
I think that's fine.
+sub _credential_read {
+ my %credential;
+ my ($reader, $op) = (@_);
+ while ($reader) {
+ if (!/^([^=\s]+)=(.*?)\s*$/) {
+ throw Error::Simple(unable to parse git credential $op
response:\n$_);
+ }
+ $credential{$1} = $2;
I think this is worse than your previous version. The spec really is as
simple as:
while ($reader) {
last if /^$/; # blank line is OK as end-of-credential
/^([^=]+)=(.*)/
or throw Error::Simple(...);
$credential{$1} = {$2};
}
(actually, the spec as written does not explicitly forbid an empty key,
but it is nonsensical, and it might be worth updating the docs).
+sub _credential_write {
+ my ($credential, $writer) = @_;
+
+ for my $key (sort {
+ # url overwrites other fields, so it must come first
+ return -1 if $a eq 'url';
+ return 1 if $b eq 'url';
+ return $a cmp $b;
+ } keys %$credential) {
+ if (defined $credential-{$key} length $credential-{$key}) {
+ print $writer $key, '=', $credential-{$key}, \n;
+ }
When I mentioned error-checking the format, I really just meant
something like:
$key =~ /[=\n\0]/
and die BUG: credential key contains invalid characters: $key;
if (defined $credential-{$key}) {
$credential-{$key} =~ /[\n\0]/
and die BUG: credential value contains invalid
characters: $credential-{key};
print $writer $key, '=', $credential-{$key}, \n;
}
Those dies should never happen, and are indicative of a bug in the
caller. We can't even represent them in the protocol, so we might as
well alert the user and die rather than trying to guess what the caller
intended.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns
On 08.02.13 03:10, Jiang Xin wrote:
Since command usages can be translated, they may not align well especially
when they are translated to CJK. A wrapper utf8_fprintf can help to return
the correct columns required.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
parse-options.c | 5 +++--
utf8.c | 22 ++
utf8.h | 1 +
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 67e98..a6ce9e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
#include cache.h
#include commit.h
#include color.h
+#include utf8.h
static int parse_options_usage(struct parse_opt_ctx_t *ctx,
const char * const *usagestr,
@@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE
*outfile)
s = literal ? [%s] : [%s];
else
s = literal ? %s : %s;
- return fprintf(outfile, s, opts-argh ? _(opts-argh) : _(...));
+ return utf8_fprintf(outfile, s, opts-argh ? _(opts-argh) : _(...));
}
#define USAGE_OPTS_WIDTH 24
@@ -541,7 +542,7 @@ static int usage_with_options_internal(struct
parse_opt_ctx_t *ctx,
if (opts-long_name)
pos += fprintf(outfile, --%s, opts-long_name);
if (opts-type == OPTION_NUMBER)
- pos += fprintf(outfile, -NUM);
+ pos += utf8_fprintf(outfile, _(-NUM));
if ((opts-flags PARSE_OPT_LITERAL_ARGHELP) ||
!(opts-flags PARSE_OPT_NOARG))
diff --git a/utf8.c b/utf8.c
index a4ee6..05925 100644
--- a/utf8.c
+++ b/utf8.c
@@ -430,6 +430,28 @@ int same_encoding(const char *src, const char *dst)
}
/*
+ * Wrapper for fprintf and returns the total number of columns required
+ * for the printed string, assuming that the string is utf8.
+ */
+int utf8_fprintf(FILE *stream, const char *format, ...)
+{
+ struct strbuf buf = STRBUF_INIT;
+ va_list arg;
+ int columns;
+
+ va_start (arg, format);
+ strbuf_vaddf(buf, format, arg);
+ va_end (arg);
+
+ columns = fputs(buf.buf, stream);
+ /* If no error occurs, returns columns really required with
utf8_strwidth. */
+ if (0 = columns)
+ columns = utf8_strwidth(buf.buf);
+ strbuf_release(buf);
+ return columns;
+}
+
I don't think we handle the return code from fputs() correctly.
Please dee below for specifications on fprintf(),
something like the following could do:
int utf8_fprintf(FILE *stream, const char *format, ...)
{
struct strbuf buf = STRBUF_INIT;
va_list arg;
int columns = 0;
va_start (arg, format);
strbuf_vaddf(buf, format, arg);
va_end (arg);
if (EOF != fputs(buf.buf, stream))
columns = utf8_strwidth(buf.buf);
strbuf_release(buf);
return columns;
}
And as a side note: would fprintf_strwidth() be a better name?
Linux:
RETURN VALUE
fputc(), putc() and putchar() return the character written as an
unsigned char cast to an int or EOF on error.
puts() and fputs() return a nonnegative number on success, or EOF on
error.
Mac OS:
COMPATIBILITY
fputs() now returns a non-negative number (as opposed to 0) on successful
completion. As a result, many tests (e.g., fputs() == 0, fputs() !=
0) do not give the desired result. Use fputs() != EOF or fputs() ==
EOF to determine success or failure.
Posix:
RETURN VALUE
Upon successful completion, fputs() shall return a non-negative number.
Otherwise, it shall return EOF, set an error indicator for the stream,
and set errno to indicate the error.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: branch.name.remotepush
Jeff King p...@peff.net writes: On Thu, Feb 07, 2013 at 09:44:59PM +0530, Ramkumar Ramachandra wrote: This has been annoying me for a really long time, but I never really got around to scratching this particular itch. I have a very common scenario where I fork a project on GitHub. I have two configured remotes: origin which points to git://upstream and mine which points to ssh://mine. By default, I always want to pull `master` from origin and push to mine. Same here. Even without GitHub, working on git.git I treat Junio as my origin, but push to a publishing point. The you fetch from and push to the same place semantics that associates a branch to a single remote was primarily done for people coming from CVS/SVN background [*1*]. I think the triangle arrangement where you want to have this is where I fetch from and integrate with, and that is where I publish is more common among the Git users these days. How best to express the triangle is somewhat tricky, but I think it is sensible to say you have origin that points to your upstream (i.e. me), and peff that points to your publishing point, in other words, make it explicit that the user deals with two remotes. Then have push.default name the remote peff, so that git push goes to that remote by default (and have git fetch/pull go to origin). You will have two sets of remote tracking branches (one from origin that your push will never pretend to have fetched immediately after finishing, the other from peff that keeps track of what you pushed the last time). Of course, some people may have I use this and that branches to interact with upstream X while I use these other branches to interacct with upstream Y, and all of them push to different places, and supporting that may need complex per branch On this branch fetch from and integrate with remote X, and push to remote Z settings, but as you said, I fetch from and integrate with X, and result is pushed out to Y should be the most common, and it would be desirable to have a simple way to express it with just a single new configuration variable. [Footnote] *1* It also happens to work reasonably well for people like Linus and I with the I pull from random places, I locally integrate and I publish the results workflow, because we are trained to think that it is not just being lazy but simply meaningless to say git pull without saying fetch and integrate _what_ and from _whom_, and that is only because we do not have a fixed upstream. Linus and I would practically never fetch from origin, i.e. from ourselves. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns
(Sorry for confusing: I should have written:) Please see below for specifications on fputs() Linux: RETURN VALUE fputc(), putc() and putchar() return the character written as an unsigned char cast to an int or EOF on error. puts() and fputs() return a nonnegative number on success, or EOF on error. Mac OS: COMPATIBILITY fputs() now returns a non-negative number (as opposed to 0) on successful completion. As a result, many tests (e.g., fputs() == 0, fputs() != 0) do not give the desired result. Use fputs() != EOF or fputs() == EOF to determine success or failure. Posix: RETURN VALUE Upon successful completion, fputs() shall return a non-negative number. Otherwise, it shall return EOF, set an error indicator for the stream, and set errno to indicate the error. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
Ted Zlatanov t...@lifelogs.com writes: I agree this Makefile is not a good test to ship out. It was my quickie test rig that I should have reworked before adding to the patch. Sorry. Nothing to be sorry about. Starting with quick-and-dirty and polishing for public consumption is what the review cycle is about, and we are here to help that process. I see contrib/subtree/t and contrib/mw-to-git/t that I could copy. The test will have a few files to parse, and will be able to compare the expected to the actual output. Does that sound like a good plan? Yup. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
On Tue, Feb 05, 2013 at 07:38:58PM -0500, Ted Zlatanov wrote:
Add Git credential helper that can parse netrc/authinfo files.
This credential helper supports multiple files, returning the first one
that matches. It checks file permissions and owner. For *.gpg files,
it will run GPG to decrypt the file.
Signed-off-by: Ted Zlatanov t...@lifelogs.com
+ # the following check is copied from Net::Netrc, for non-GPG files
+ # OS/2 and Win32 do not handle stat in a way compatable with this check
:-(
s/compatable/compatible/
You mention os/2 and Win32 here, but the check has more:
+ unless ($gpgmode || $options{insecure} ||
+ $^O eq 'os2'
+ || $^O eq 'MSWin32'
+ || $^O eq 'MacOS'
+ || $^O =~ /^cygwin/) {
Does MacOS really not handle stat? Or is this old MacOS, not OS X?
+sub load_netrc {
[...]
+ foreach my $nentry (@netrc_entries) {
+ my %entry;
+ my $num_port;
+
+ if (!defined $nentry-{machine}) {
+ next;
+ }
+ if (defined $nentry-{port} $nentry-{port} =~ m/^\d+$/) {
+ $num_port = $nentry-{port};
+ delete $nentry-{port};
+ }
+
+ # create the new entry for the credential helper protocol
+ $entry{$options{tmap}-{$_}} = $nentry-{$_} foreach keys
%$nentry;
+
+ # for host X port Y where Y is an integer (captured by
+ # $num_port above), set the host to X:Y
+ if (defined $entry{host} defined $num_port) {
+ $entry{host} = join(':', $entry{host}, $num_port);
+ }
So this will convert:
machine foo port smtp
in the netrc into (protocol = smtp, host = foo), but:
machine foo port 25
into (protocol = undef, host = foo:25), right? That makes sense to
me.
+sub net_netrc_loader {
[...]
I won't comment here, as I know very little about netrc (I always
thought it was line-oriented, too!) and Junio has covered it.
+# takes the search tokens and then a list of entries
+# each entry is a hash reference
+sub find_netrc_entry {
+ my $query = shift @_;
+
+ENTRY:
+ foreach my $entry (@_)
+ {
+ my $entry_text = join ', ', map { $_=$entry-{$_} } keys
%$entry;
+ foreach my $check (sort keys %$query) {
+ if (defined $query-{$check}) {
+ log_debug(compare %s [%s] to [%s] (entry: %s),
+ $check,
+ $entry-{$check},
+ $query-{$check},
+ $entry_text);
+ unless ($query-{$check} eq $entry-{$check}) {
+ next ENTRY;
+ }
+ } else {
+ log_debug(OK: any value satisfies check
$check);
+ }
This looks right to me.
+sub print_credential_data {
I don't know if you want to take the hit of relying on Git.pm (it is
nice for the helper to be totally standalone and copy-able), but one
obvious possible refactor would be to use the credential read/write
functions recently added there. I'm OK with not doing that, though.
+ my $entry = shift @_;
+ my $query = shift @_;
+
+ log_debug(entry has passed all the search checks);
+ TOKEN:
+ foreach my $git_token (sort keys %$entry) {
+ log_debug(looking for useful token $git_token);
+ # don't print unknown (to the credential helper protocol) tokens
+ next TOKEN unless exists $query-{$git_token};
+
+ # don't print things asked in the query (the entry matches them)
+ next TOKEN if defined $query-{$git_token};
+
+ log_debug(FOUND: $git_token=$entry-{$git_token});
+ printf %s=%s\n, $git_token, $entry-{$git_token};
+ }
Printf? Bleh, isn't this supposed to be perl? :P
I don't see anything wrong from the credential-handling side of things.
As I said, I didn't look closely at the netrc parsing bits. From my
reading of perldoc macos, the answer to my question above is yes,
stat doesn't work on MacOS Classic. So I think the script itself is
fine.
In your tests:
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,12 @@
+test_netrc:
+ @(echo bad data | ./git-credential-netrc -f A -d -v) || echo Bad
invocation test, ignoring
failure
+ @echo = Silent invocation... nothing should show up here with a
missing file
+ @echo bad data | ./git-credential-netrc -f A get
+ @echo = Back to noisy: -v and -d used below, missing file
+ echo bad data | ./git-credential-netrc -f A -d -v get
+ @echo = Look for any entry in the default file set
+ echo | ./git-credential-netrc -d -v get
+ @echo = Look
Re: How to diff 2 file revisions with gitk
On Wed, Feb 6, 2013 at 9:27 PM, R. Diez rdiezmail-buspir...@yahoo.de wrote: Hi there: I asked a few days ago whether I could easily diff 2 file revisions with the mouse in gitk, but I got no reply yet, see here: How to diff two file revisions with the mouse (with gitk) https://groups.google.com/forum/#!topic/git-users/9znsQsTB0dE I am hoping that it was the wrong mailing list, and this one the right one. 8-) Here is the full question text again: 8888 I would like to start gitk, select with the mouse 2 revisions of some file and then compare them, hopefully with an external diff tool, very much like I am used to with WinCVS. The closest I got is to start gitk with a filename as an argument, in order to restrict the log to that one file. Then I right-click on a commit (a file revision) and choose Mark this commit. However, if I right-click on another commit and choose Compare with marked commit, I get a full commit diff with all files, and not just the file I specified on the command-line arguments. Selecting a filename in the Tree view and choosing Highlight this only, as I found on the Internet, does not seem to help. I have git 1.7.9 (on Cygwin). Can someone help? By the way, it would be nice if gitk could launch the external diff tool from the Compare with marked commit option too. I don't know if I misunderstood the whole question because the answer is very simple. - start gitk - left click the newer commit - scroll to the older commit - right click the older commit and choose Diff this - selected - in the bottom right pane, pick any file, right click, and choose External diff. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: branch.name.remotepush
On Thu, Feb 07, 2013 at 10:08:48PM -0800, Junio C Hamano wrote: How best to express the triangle is somewhat tricky, but I think it is sensible to say you have origin that points to your upstream (i.e. me), and peff that points to your publishing point, in other words, make it explicit that the user deals with two remotes. Then have push.default name the remote peff, so that git push goes to that remote by default (and have git fetch/pull go to origin). You will have two sets of remote tracking branches (one from origin that your push will never pretend to have fetched immediately after finishing, the other from peff that keeps track of what you pushed the last time). Exactly. That is what I have set up now, except that I have to type git push peff because there is no such push.default (with the minor nit that push.default does something else, so the config should be called remote.pushDefault or something). The entirety of the feature would be saving the user from the annoyance of: $ git push fatal: remote error: You can't push to git://github.com/gitster/git.git Use g...@github.com:gitster/git.git [doh! Stupid git, why don't you do what I mean, not what I say?] $ git push peff ... it works ... Of course, some people may have I use this and that branches to interact with upstream X while I use these other branches to interacct with upstream Y, and all of them push to different places, and supporting that may need complex per branch On this branch fetch from and integrate with remote X, and push to remote Z settings, but as you said, I fetch from and integrate with X, and result is pushed out to Y should be the most common, and it would be desirable to have a simple way to express it with just a single new configuration variable. Right. Frankly, I do not care that much about the per-branch push remote myself. In the rules I gave earlier, that was my complete backwards-compatible vision, so that we do not paint ourselves into a corner compatibility-wise when somebody wants it later. Just implementing the default push remote part would be a fine first step. I also indicated in my rules that we could have a branch.*.fetchRemote, as well, but I do not think it is strictly necessary. I think the non-specific branch.*.remote could continue to be used for fetching, and as a backup when the push-specific variables are not set. *1* It also happens to work reasonably well for people like Linus and I with the I pull from random places, I locally integrate and I publish the results workflow, because we are trained to think that it is not just being lazy but simply meaningless to say git pull without saying fetch and integrate _what_ and from _whom_, and that is only because we do not have a fixed upstream. Linus and I would practically never fetch from origin, i.e. from ourselves. Right, I think git pull is more useful in a centralized repo setting where there is one branch and one repo, so there is no what and whom to specify. Personally I do not use it much at all, as I do a separate fetch, inspect, and merge, but that is somewhat orthogonal to your reasons. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: branch.name.remotepush
Junio C Hamano gits...@pobox.com writes:
I think the triangle
arrangement where you want to have this is where I fetch from and
integrate with, and that is where I publish is more common among
the Git users these days.
Another thing to know about is that the recent move to change the
behaviour of git push to work only on one branch per default may
have to be polished and strengthened a bit.
Originally, the encouraged workflow was to perfect _everything_ that
you would push out and then with a single git push to publish
everything at the same time. Both the matching behaviour of git
push which was the default, and the set of push refspecs that is to
be defined per remote, were ways to discourage Work on one branch,
think it is OK, hastily push only that branch out, switch to another
branch, rinse, repeat.
To support a triangular arrangement well, there may need some
thinking on what $branch@{upstream} means. The original intent of
the upstream mode specified for push.default is push the result
back to what you based your work on, but in a triangular arrangement
that is no longer true. You may be keeping up with my 'master' by
constantly rebasing and then pushing out the result to your 'frotz'
topic. You want to have a lazy git fetch to fetch from my
'master' (i.e. upstream), and have remotes/origin/master to keep
track of it. You want to see git rebase to pay attention to the
updates to remotes/origin/master when figuring out where you forked.
But at the same time, you want a lazy git push to go to your
push.defaultTo repository (i.e. your publish point) and update your
'frotz' branch there---remotes/origin/master should not come into
the picture at all. But the upstream and simple modes want to pay
attention to branch.$name.merge, which is all about the fetch and
integrate side of the equation.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] Introduce branch.name.pushremote
This new configuration variable overrides the remote in
`branch.name.remote` for pushes. It is useful in the typical
scenario, where the remote I'm pulling from is not the remote I'm
pushing to. Although `remote.name.pushurl` is similar, it does not
serve the purpose as the URL would lack corresponding remote tracking
branches.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
This is a first cut. There's code duplication at the moment, but I'm
currently trying to figure out which other remote_get() calls to
replace with pushremote_get(). Comments are welcome.
I will leave it to future patches to do the following things:
1. Fix the status output to be more meaningful when pushremote is
set. At the moment, I'm thinking statuses like [pull: 4 behind,
push: 3 ahead] will make sense.
2. Introduce a remote.pushDefault (peff)
3. Introduce a remote.default (peff)
Documentation/config.txt | 6 ++
builtin/push.c | 2 +-
remote.c | 41 +
remote.h | 2 ++
4 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9b11597..0b3b1f8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -727,6 +727,12 @@ branch.name.remote::
remote to fetch from/push to. It defaults to `origin` if no remote is
configured. `origin` is also used if you are not on any branch.
+branch.name.pushremote::
+ When in branch name, it tells 'git push' which remote to
+ push to. It falls back to `branch.name.remote`, and
+ defaults to `origin` if no remote is configured. `origin` is
+ also used if you are not on any branch.
+
branch.name.merge::
Defines, together with branch.name.remote, the upstream branch
for the given branch. It tells 'git fetch'/'git pull'/'git rebase' which
diff --git a/builtin/push.c b/builtin/push.c
index 42b129d..d447a80 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport,
int flags)
static int do_push(const char *repo, int flags)
{
int i, errs;
- struct remote *remote = remote_get(repo);
+ struct remote *remote = pushremote_get(repo);
const char **url;
int url_nr;
diff --git a/remote.c b/remote.c
index e53a6eb..d6fcfc0 100644
--- a/remote.c
+++ b/remote.c
@@ -48,6 +48,7 @@ static int branches_nr;
static struct branch *current_branch;
static const char *default_remote_name;
+static const char *pushremote_name;
static int explicit_default_remote_name;
static struct rewrites rewrites;
@@ -363,6 +364,12 @@ static int handle_config(const char *key, const char
*value, void *cb)
default_remote_name = branch-remote_name;
explicit_default_remote_name = 1;
}
+ } else if (!strcmp(subkey, .pushremote)) {
+ if (!value)
+ return config_error_nonbool(key);
+ branch-pushremote_name = xstrdup(value);
+ if (branch == current_branch)
+ pushremote_name = branch-pushremote_name;
} else if (!strcmp(subkey, .merge)) {
if (!value)
return config_error_nonbool(key);
@@ -700,6 +707,40 @@ struct remote *remote_get(const char *name)
return ret;
}
+struct remote *pushremote_get(const char *name)
+{
+ struct remote *ret;
+ int name_given = 0;
+
+ read_config();
+ if (name)
+ name_given = 1;
+ else {
+ if (pushremote_name) {
+ name = pushremote_name;
+ name_given = 1;
+ } else {
+ name = default_remote_name;
+ name_given = explicit_default_remote_name;
+ }
+ }
+
+ ret = make_remote(name, 0);
+ if (valid_remote_nick(name)) {
+ if (!valid_remote(ret))
+ read_remotes_file(ret);
+ if (!valid_remote(ret))
+ read_branches_file(ret);
+ }
+ if (name_given !valid_remote(ret))
+ add_url_alias(ret, name);
+ if (!valid_remote(ret))
+ return NULL;
+ ret-fetch = parse_fetch_refspec(ret-fetch_refspec_nr,
ret-fetch_refspec);
+ ret-push = parse_push_refspec(ret-push_refspec_nr, ret-push_refspec);
+ return ret;
+}
+
int remote_is_configured(const char *name)
{
int i;
diff --git a/remote.h b/remote.h
index 251d8fd..aa42ff5 100644
--- a/remote.h
+++ b/remote.h
@@ -51,6 +51,7 @@ struct remote {
};
struct remote *remote_get(const char *name);
+struct remote *pushremote_get(const char *name);
int remote_is_configured(const char *name);
typedef int
Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns
2013/2/8 Torsten Bögershausen tbo...@web.de:
On 08.02.13 03:10, Jiang Xin wrote:
+ /* If no error occurs, returns columns really required with
utf8_strwidth. */
+ if (0 = columns)
+ columns = utf8_strwidth(buf.buf);
+ strbuf_release(buf);
+ return columns;
+}
+
I don't think we handle the return code from fputs() correctly.
Please dee below for specifications on fprintf(),
something like the following could do:
int utf8_fprintf(FILE *stream, const char *format, ...)
{
struct strbuf buf = STRBUF_INIT;
va_list arg;
int columns = 0;
va_start (arg, format);
strbuf_vaddf(buf, format, arg);
va_end (arg);
if (EOF != fputs(buf.buf, stream))
columns = utf8_strwidth(buf.buf);
strbuf_release(buf);
return columns;
}
As fputs() returns a non-negative number (as opposed to 0) on
successful completion,
Test fputs() return value as fputs() =0 is correct, while fputs()
== 0, fputs() != 0
are wrong. I think it's OK, must I send a new re-roll for this?
EOF is defined as (-1) in stdio.h:
#define EOF (-1)
And as a side note: would fprintf_strwidth() be a better name?
This is a nice candidate.
--
Jiang Xin
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: branch.name.remotepush
Junio C Hamano wrote: I'd actually see this as Gerrit being weird. If it wants to quarantine a commit destined to the master branch, couldn't it just let people push to master and then internally update for/master instead? It is because pushing doesn't update refs/heads/master. Instead, it starts a code review. Suppose Gerrit allows starting a new code review by pushing to refs/heads/master. It sounds okay if I squint --- it's just a very slow asynchronous ref update, right? Let's see: $ git clone gerrit server test Cloning into 'test'... $ echo hi greeting $ git add greeting $ git commit -q -m 'hello' $ git push origin master [...] remote: New Changes: remote: gerrit server/r/1234 remote: To url ea4cb77b..9117390e master - master $ : walk away, forget what I was doing $ git fetch origin From url + 9117390...ea4cb77 master - origin/master (forced update) Wait, why did the remote rewind? Regards, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
