Git development
 help / color / mirror / Atom feed
* Re: RFC: git sync
From: Sergio @ 2010-02-10 16:39 UTC (permalink / raw)
  To: git
In-Reply-To: <m3wryl8lkp.fsf@localhost.localdomain>

> Why not use the recpie from GitFaq:
> "How would I use "git push" to sync out of a host that I cannot pull from?"
>
http://git.wiki.kernel.org/index.php/GitFaq#How_would_I_use_.22git_push.22_to_sync_out_of_a_host_that_I_cannot_pull_from.3F
> 

Hi, to follow that recipe with more than one satellite, all sharing the same
config, it would be just great to be able to put $hostname in .git-config, e.g.

[remote "mothership"]
   url = <url>
   push = master:refs/remotes/$hostname/master

Would there be any potential problem with that?

^ permalink raw reply

* Re: git-svn taking a long time
From: Andrew Myrick @ 2010-02-10 16:59 UTC (permalink / raw)
  To: David Kågedal; +Cc: Git Mailing List
In-Reply-To: <87hbppp8k7.fsf@krank.kagedal.org>

Give 1.7.0-rc2 a try.  It includes commit 8bff7c5383ed833bd1df9c8d85c00a27af3e5b02, which attempts to persistently cache a lot of the processing that git-svn has to do on subversion's merge tickets, which has improved my fetch times significantly.  

Note 1: git did not support subversion merge tickets before git-1.6.6, so there would be no slowdown if you use prior versions.

Note 2: you shouldn't need to clone your subversion repository from scratch.  git will create the caches the first time you fetch after upgrading (so the first fetch will be slow), and it will continue to use and update the caches thereafter.

-Andrew

On Feb 10, 2010, at 4:41 AM, David Kågedal wrote:

> I compiled the perl bindings from subversion 1.6.x, and git git version
> 1.7.0.rc0.52.g64ba.
> 
> I have imported a largish svn repo using "git svn fetch", which takes
> quite a long time (at least a day).
> 
> But my problems is that if I rerun "git svn fetch" again right after
> fetching, it will chew for hours before figuring out there are just a
> few new revisions to import. And the same if I try again.
> 
> I have another import from the same repo (on another machine, using
> slightly different versions of stuff) that doesn't have this problem.
> 
> So, does anyone have any suggestions for what could be done about this?
> 
> This is what the entry i .git/config looks like (url edited to protect
> the innocent)
> 
> [svn-remote "svn"]
>        url = svn://svn.server
>        branches = foo/branches/*:refs/remotes/svn/*
>        tags = foo/tags/*:refs/remotes/svn/tags/*
> 
> -- 
> David Kågedal
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] Documentation: reword --thin description
From: Stephen Boyd @ 2010-02-10 16:52 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.LFD.2.00.1002101037300.1681@xanadu.home>

On 02/10/2010 08:07 AM, Nicolas Pitre wrote:
> Both the old and the new text are bollocks.
>
> There is no extra cycles involved here.  And linking this to a slow 
> connection is misleading.
>
> The point of --thin is to create a pack containing delta objects while 
> excluding the base objects they depend on when those objects are known 
> to exist in the receiver's repository already.  Because base objects 
> are usually significantly bigger than delta objects, this results in a 
> large reduction in the amount of data to transfer.

Thanks. Maybe this would more accurately describe the option?

Create a pack containing only delta objects when the base objects the
delta objects depend upon are present in the receiver's repository. This
typically results in less data being transferred. Default: on

^ permalink raw reply

* Re: A Visual Git Reference
From: Dilip M @ 2010-02-10 17:02 UTC (permalink / raw)
  To: Mark Lodato; +Cc: git list
In-Reply-To: <ca433831002081134m698f531bwa22f0474db0cdcb@mail.gmail.com>

Hello Mark,


On Tue, Feb 9, 2010 at 1:04 AM, Mark Lodato <lodatom@gmail.com> wrote:
> All,
>
> I put together a "Visual Git Reference" containing visualizations of
> the most common git commands, for people who prefer to see images over
> text.  It is designed as a reference, not a tutorial, so readers need
> to have some amount of experience before the page will become useful.
>
> URL: http://marklodato.github.com/visual-git-guide/
> Git repo: http://github.com/marklodato/visual-git-guide/
>
> If you have any feedback or suggestions, please let me know!

This is very helpful for git users...:) Thanks...


-- 
Dilip

Everyone is gifted - but some people never open their package!

^ permalink raw reply

* Re: [PATCH] Fix signal handler
From: Markus Elfring @ 2010-02-10 17:08 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <4B71A2EE.8070708@web.de>

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

Hello,

How do Git software developers think about the appended update suggestion?
Would you like to integrate such adjustments into your source code
repository?

Regards,
Markus

[-- Attachment #2: 0001-Fix-a-signal-handler.patch --]
[-- Type: text/x-patch, Size: 3457 bytes --]

>From c37d8dafef11168d8302d40c8d1453943a058d95 Mon Sep 17 00:00:00 2001
From: Markus Elfring <Markus.Elfring@web.de>
Date: Wed, 10 Feb 2010 17:05:45 +0100
Subject: [PATCH] Fix a signal handler

A global flag can only be set by a signal handler in a portable way if it has got the data type "sig_atomic_t". The previously used assignment of a function pointer in the function "early_output" was moved to another variable in the function "setup_early_output".
The involved software design details were also mentioned on the mailing list.
---
 builtin-log.c |   12 +++---------
 revision.c    |   14 ++++++--------
 revision.h    |    3 ++-
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 8d16832..358c98b 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -123,7 +123,7 @@ static void show_early_header(struct rev_info *rev, const char *stage, int nr)
 
 static struct itimerval early_output_timer;
 
-static void log_show_early(struct rev_info *revs, struct commit_list *list)
+extern void log_show_early(struct rev_info *revs, struct commit_list *list)
 {
 	int i = revs->early_output;
 	int show_header = 1;
@@ -170,20 +170,14 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list)
 
 static void early_output(int signal)
 {
-	show_early_output = log_show_early;
+	show_early_output = 1;
 }
 
 static void setup_early_output(struct rev_info *rev)
 {
 	struct sigaction sa;
 
-	/*
-	 * Set up the signal handler, minimally intrusively:
-	 * we only set a single volatile integer word (not
-	 * using sigatomic_t - trying to avoid unnecessary
-	 * system dependencies and headers), and using
-	 * SA_RESTART.
-	 */
+	early_output_function = &log_show_early;
 	memset(&sa, 0, sizeof(sa));
 	sa.sa_handler = early_output;
 	sigemptyset(&sa.sa_mask);
diff --git a/revision.c b/revision.c
index 3ba6d99..62402fb 100644
--- a/revision.c
+++ b/revision.c
@@ -13,7 +13,8 @@
 #include "decorate.h"
 #include "log-tree.h"
 
-volatile show_early_output_fn_t show_early_output;
+sig_atomic_t show_early_output = 0;
+show_early_output_fn_t early_output_function = NULL;
 
 char *path_name(const struct name_path *path, const char *name)
 {
@@ -654,7 +655,6 @@ static int limit_list(struct rev_info *revs)
 		struct commit_list *entry = list;
 		struct commit *commit = list->item;
 		struct object *obj = &commit->object;
-		show_early_output_fn_t show;
 
 		list = list->next;
 		free(entry);
@@ -680,12 +680,10 @@ static int limit_list(struct rev_info *revs)
 		date = commit->date;
 		p = &commit_list_insert(commit, p)->next;
 
-		show = show_early_output;
-		if (!show)
-			continue;
-
-		show(revs, newlist);
-		show_early_output = NULL;
+		if (show_early_output) {
+			(*early_output_function)(revs, newlist);
+			show_early_output = 0;
+		}
 	}
 	if (revs->cherry_pick)
 		cherry_pick_list(newlist, revs);
diff --git a/revision.h b/revision.h
index a14deef..93a8ffc 100644
--- a/revision.h
+++ b/revision.h
@@ -135,7 +135,8 @@ struct rev_info {
 
 /* revision.c */
 typedef void (*show_early_output_fn_t)(struct rev_info *, struct commit_list *);
-extern volatile show_early_output_fn_t show_early_output;
+extern show_early_output_fn_t early_output_function;
+extern sig_atomic_t show_early_output;
 
 extern void init_revisions(struct rev_info *revs, const char *prefix);
 extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
-- 
1.6.6.1


^ permalink raw reply related

* Re: git-svn: Cannot lock the ref 'refs/remotes/tags/autotag_for_.'.
From: Tzafrir Cohen @ 2010-02-10 17:07 UTC (permalink / raw)
  To: git
In-Reply-To: <20100209213929.GL3599@xorcom.com>

Small update,

On Tue, Feb 09, 2010 at 11:39:29PM +0200, Tzafrir Cohen wrote:
> Hi
> 
> I'm using git-svn for watching over SVN repositories in
> svn.asterisk.org . The largest one there is
> http://svn.asterisk.org/svn/asterisk .
> 
> This worked fine up until recently. I was using Debian Stable with Git
> 1.5.6 .
> 
> Recently I upgraded my system to Debian Testing with Git 1.6 (I
> currently have 1.6.6.1-1). Today I tried to update the repository (git
> svn rebase --fetch-all) and got the following error:
> 
>   fatal: Cannot lock the ref 'refs/remotes/tags/autotag_for_.'.
> 
> IIRC it followed an automatic garbage-collection in the repository.
> 
> I decided to re-clone the SVN repository (I was planning on doing that
> for quite some time, with a proper AUTHORS file this time. But it's a
> repo of some 200,000 revisions and takes a few days to clone)
> 
> I eventually got the same error.
> 
> The specific revision that triggered it:
> 
>   http://svnview.digium.com/svn/asterisk?view=revision&revision=47394
> 
> Should git-svn mangle SVN tag names to be legal git ref names? Or just
> panic before it makes the git repo invalid?
> 
> Is there a way for me to skip some tags? I can avoid that specific tag.

Here's one that did not work:

$ git svn clone -s --ignore-paths='^/tags/autotag_for_.$' http://svn.asterisk.org/svn/asterisk

I again stress that git-svn manages to fetch a faulty repository.
Another sub-question is: how do I remove that faulty reference git-svn
added?

> 
> Please CC your replies to me as I don't actively follow this list.

-- 
               Tzafrir Cohen
icq#16849755              jabber:tzafrir.cohen@xorcom.com
+972-50-7952406           mailto:tzafrir.cohen@xorcom.com
http://www.xorcom.com  iax:guest@local.xorcom.com/tzafrir

^ permalink raw reply

* Re: [PATCH] Fix signal handler
From: Shawn O. Pearce @ 2010-02-10 17:14 UTC (permalink / raw)
  To: Markus Elfring; +Cc: Jeff King, git
In-Reply-To: <4B72E81B.3020900@web.de>

Markus Elfring <Markus.Elfring@web.de> wrote:
> How do Git software developers think about the appended update suggestion?
> Would you like to integrate such adjustments into your source code
> repository?

Finally, a concrete patch we can comment on!
 
> Subject: [PATCH] Fix a signal handler
> 
> A global flag can only be set by a signal handler in a portable way if it has got the data type "sig_atomic_t". The previously used assignment of a function pointer in the function "early_output" was moved to another variable in the function "setup_early_output".
> The involved software design details were also mentioned on the mailing list.

Please line wrap your commit messages at ~70 characters per line.
This improves readability when reading the messages with tools like
`git log` and `gitk` where the lines aren't reflowed.

Please read Documentation/SubmittingPatches and add a Signed-off-by
line if you agree to the Developer's Certificate of Origin.


> +	early_output_function = &log_show_early;
...
> -volatile show_early_output_fn_t show_early_output;
> +sig_atomic_t show_early_output = 0;
> +show_early_output_fn_t early_output_function = NULL;
...
> +		if (show_early_output) {
> +			(*early_output_function)(revs, newlist);
> +			show_early_output = 0;
> +		}

The function pointer isn't necessary.  AFAIK its only called in
this one call site.  So you can make a direct reference to the
log_show_early function.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 8/6] receive-pack: Send internal errors over side-band #2
From: Shawn O. Pearce @ 2010-02-10 17:17 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4B725CB1.1080908@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> wrote:
> Shawn O. Pearce schrieb:
> > +static void rp_error(const char *err, ...) __attribute__((format (printf, 1, 2)));
> > +static void rp_warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
> > +
> > +static void report_message(const char *prefix, const char *err, va_list params)
> > +{
> > +	int sz = strlen(prefix);
> > +	char msg[4096];
> > +
> > +	strncpy(msg, prefix, sz);
> > +	sz += vsnprintf(msg + sz, sizeof(msg) - (sz + 1), err, params);
> > +	msg[sz++] = '\n';
> 
> This writes beyond the buffer if it is too small because the return value
> tells how many characters *would* have been written if it were
> sufficiently large, no?

Ugh.  I don't code C often enough anymore.

Thank you for catching that.

 
> > +static void rp_warning(const char *err, ...)
> > ...
> > +static void rp_error(const char *err, ...)
> > ...
> 
> Looks like we need set_report_routine().
> 
> Or did you replace only selected error() and warning() calls by rp_error()
> and rp_warning()?

As described by others in the thread... I only replaced selected
calls.  Well, most of them, maybe too many.  But I didn't want to
expose everything to the client.  So I added new functions.  Yes, it
was painful.  I wasn't happy about it.  But I also wasn't happy about
exposing every message to the client over the side-band channel.

-- 
Shawn.

^ permalink raw reply

* Re: git-svn taking a long time
From: David Kågedal @ 2010-02-10 17:29 UTC (permalink / raw)
  To: Andrew Myrick; +Cc: Git Mailing List
In-Reply-To: <6D721095-7A04-4097-8D86-1A2B915182DF@apple.com>

Andrew Myrick <amyrick@apple.com> writes:

> Give 1.7.0-rc2 a try.  It includes commit 8bff7c5383ed833bd1df9c8d85c00a27af3e5b02, which attempts to persistently cache a lot of the processing that git-svn has to do on subversion's merge tickets, which has improved my fetch times significantly.  

By "merge tickets", are you talking about the merge functionality that
appeared in subversion 1.5? We don't use that.

But I had another idea. I pecularity of our subversion repo is that we
no longer use the foo/trunk branch, but only foo/branches/*. But we did
once upon a time have a foo/trunk. And since I didn't include a "fetch =
foo/trunk:refs/remotes/svn/trunk" in my config, it might need to refetch
that information every time. For instance, the first revision is on
trunk.

I'm rerunning the fetch now with the trunk added, so see if it helps.

And another note is that "git svn fetch --parent" was always quick.

-- 
David Kågedal

^ permalink raw reply

* Re: [PATCH] Fix signal handler
From: Jeff King @ 2010-02-10 17:33 UTC (permalink / raw)
  To: Markus Elfring; +Cc: git
In-Reply-To: <4B72E81B.3020900@web.de>

On Wed, Feb 10, 2010 at 06:08:43PM +0100, Markus Elfring wrote:

> A global flag can only be set by a signal handler in a portable way if
> it has got the data type "sig_atomic_t". The previously used
> assignment of a function pointer in the function "early_output" was
> moved to another variable in the function "setup_early_output".
>
> The involved software design details were also mentioned on the
> mailing list.

Keep in mind commit messages will be read much later through "git log"
and the like.  Mentioning the mailing list is usually not very helpful
there. It is usually a good idea instead to summarize what was said on
the list for later readers of the commit (though in this case, I think
your first paragraph really says everything that needs to be said).

> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -123,7 +123,7 @@ static void show_early_header(struct rev_info *rev, const char *stage, int nr)
>  
>  static struct itimerval early_output_timer;
>  
> -static void log_show_early(struct rev_info *revs, struct commit_list *list)
> +extern void log_show_early(struct rev_info *revs, struct commit_list *list)

Why does this need to become extern? It looks like we are still just
assigning the function pointer from within this file.

> -volatile show_early_output_fn_t show_early_output;
> +sig_atomic_t show_early_output = 0;
> +show_early_output_fn_t early_output_function = NULL;

Good. I was worried from the above s/static/extern/ that you were going
to make log_show_early the only possible early output function, but the
way you did it is definitely the right way.

Overall, this change looks sane to me. You still haven't provided any
evidence that this is a problem in practice, but these changes are not
particularly cumbersome, so it is probably better to be on the safe
side.

-Peff

^ permalink raw reply

* [PATCH 8/6 v2] receive-pack: Send internal errors over side-band #2
From: Shawn O. Pearce @ 2010-02-10 17:34 UTC (permalink / raw)
  To: Johannes Sixt, Junio C Hamano; +Cc: git
In-Reply-To: <4B726A8C.6010600@viscovery.net>

If the client has requested side-band-64k capability, send any
of the internal error or warning messages in the muxed side-band
stream using the same band as our hook output, band #2.  By putting
everything in one stream we ensure all messages are processed by
the side-band demuxer, avoiding interleaving between our own stderr
and the side-band demuxer's stderr buffers.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 - Buffer overflow fixed.
 - Kept the two corruption errors on the server side.

 builtin-receive-pack.c  |   64 ++++++++++++++++++++++++++++++++++++----------
 t/t5401-update-hooks.sh |    3 +-
 2 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index da1c26b..a5543f9 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -139,6 +139,42 @@ static struct command *commands;
 static const char pre_receive_hook[] = "hooks/pre-receive";
 static const char post_receive_hook[] = "hooks/post-receive";
 
+static void rp_error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+static void rp_warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
+
+static void report_message(const char *prefix, const char *err, va_list params)
+{
+	int sz = strlen(prefix);
+	char msg[4096];
+
+	strncpy(msg, prefix, sz);
+	sz += vsnprintf(msg + sz, sizeof(msg) - sz, err, params);
+	if (sz > (sizeof(msg) - 1))
+		sz = sizeof(msg) - 1;
+	msg[sz++] = '\n';
+
+	if (use_sideband)
+		send_sideband(1, 2, msg, sz, use_sideband);
+	else
+		xwrite(2, msg, sz);
+}
+
+static void rp_warning(const char *err, ...)
+{
+	va_list params;
+	va_start(params, err);
+	report_message("warning: ", err, params);
+	va_end(params);
+}
+
+static void rp_error(const char *err, ...)
+{
+	va_list params;
+	va_start(params, err);
+	report_message("error: ", err, params);
+	va_end(params);
+}
+
 static int copy_to_sideband(int in, int out, void *arg)
 {
 	char data[128];
@@ -276,7 +312,7 @@ static void warn_unconfigured_deny(void)
 {
 	int i;
 	for (i = 0; i < ARRAY_SIZE(warn_unconfigured_deny_msg); i++)
-		warning("%s", warn_unconfigured_deny_msg[i]);
+		rp_warning("%s", warn_unconfigured_deny_msg[i]);
 }
 
 static char *warn_unconfigured_deny_delete_current_msg[] = {
@@ -302,7 +338,7 @@ static void warn_unconfigured_deny_delete_current(void)
 	for (i = 0;
 	     i < ARRAY_SIZE(warn_unconfigured_deny_delete_current_msg);
 	     i++)
-		warning("%s", warn_unconfigured_deny_delete_current_msg[i]);
+		rp_warning("%s", warn_unconfigured_deny_delete_current_msg[i]);
 }
 
 static const char *update(struct command *cmd)
@@ -314,7 +350,7 @@ static const char *update(struct command *cmd)
 
 	/* only refs/... are allowed */
 	if (prefixcmp(name, "refs/") || check_ref_format(name + 5)) {
-		error("refusing to create funny ref '%s' remotely", name);
+		rp_error("refusing to create funny ref '%s' remotely", name);
 		return "funny refname";
 	}
 
@@ -324,12 +360,12 @@ static const char *update(struct command *cmd)
 			break;
 		case DENY_UNCONFIGURED:
 		case DENY_WARN:
-			warning("updating the current branch");
+			rp_warning("updating the current branch");
 			if (deny_current_branch == DENY_UNCONFIGURED)
 				warn_unconfigured_deny();
 			break;
 		case DENY_REFUSE:
-			error("refusing to update checked out branch: %s", name);
+			rp_error("refusing to update checked out branch: %s", name);
 			return "branch is currently checked out";
 		}
 	}
@@ -342,7 +378,7 @@ static const char *update(struct command *cmd)
 
 	if (!is_null_sha1(old_sha1) && is_null_sha1(new_sha1)) {
 		if (deny_deletes && !prefixcmp(name, "refs/heads/")) {
-			error("denying ref deletion for %s", name);
+			rp_error("denying ref deletion for %s", name);
 			return "deletion prohibited";
 		}
 
@@ -354,10 +390,10 @@ static const char *update(struct command *cmd)
 			case DENY_UNCONFIGURED:
 				if (deny_delete_current == DENY_UNCONFIGURED)
 					warn_unconfigured_deny_delete_current();
-				warning("deleting the current branch");
+				rp_warning("deleting the current branch");
 				break;
 			case DENY_REFUSE:
-				error("refusing to delete the current branch: %s", name);
+				rp_error("refusing to delete the current branch: %s", name);
 				return "deletion of the current branch prohibited";
 			}
 		}
@@ -387,23 +423,23 @@ static const char *update(struct command *cmd)
 				break;
 		free_commit_list(bases);
 		if (!ent) {
-			error("denying non-fast-forward %s"
-			      " (you should pull first)", name);
+			rp_error("denying non-fast-forward %s"
+				 " (you should pull first)", name);
 			return "non-fast-forward";
 		}
 	}
 	if (run_update_hook(cmd)) {
-		error("hook declined to update %s", name);
+		rp_error("hook declined to update %s", name);
 		return "hook declined";
 	}
 
 	if (is_null_sha1(new_sha1)) {
 		if (!parse_object(old_sha1)) {
-			warning ("Allowing deletion of corrupt ref.");
+			rp_warning("Allowing deletion of corrupt ref.");
 			old_sha1 = NULL;
 		}
 		if (delete_ref(name, old_sha1, 0)) {
-			error("failed to delete %s", name);
+			rp_error("failed to delete %s", name);
 			return "failed to delete";
 		}
 		return NULL; /* good */
@@ -411,7 +447,7 @@ static const char *update(struct command *cmd)
 	else {
 		lock = lock_any_ref_for_update(name, old_sha1, 0);
 		if (!lock) {
-			error("failed to lock %s", name);
+			rp_error("failed to lock %s", name);
 			return "failed to lock";
 		}
 		if (write_ref_sha1(lock, new_sha1, "push")) {
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 7240fab..17bcb0b 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -124,6 +124,7 @@ remote: STDOUT update refs/heads/master
 remote: STDERR update refs/heads/master
 remote: STDOUT update refs/heads/tofail
 remote: STDERR update refs/heads/tofail
+remote: error: hook declined to update refs/heads/tofail
 remote: STDOUT post-receive
 remote: STDERR post-receive
 remote: STDOUT post-update
@@ -131,7 +132,7 @@ remote: STDERR post-update
 EOF
 test_expect_success 'send-pack stderr contains hook messages' '
 	grep ^remote: send.err | sed "s/ *\$//" >actual &&
-	test_cmp - actual <expect
+	test_cmp expect actual
 '
 
 test_done
-- 
1.7.0.rc2.170.gbc565

-- 
Shawn.

^ permalink raw reply related

* Re: [PATCH] Fix signal handler
From: Jeff King @ 2010-02-10 17:35 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Markus Elfring, git
In-Reply-To: <20100210171406.GE2747@spearce.org>

On Wed, Feb 10, 2010 at 09:14:06AM -0800, Shawn O. Pearce wrote:

> > +	early_output_function = &log_show_early;
> ...
> > -volatile show_early_output_fn_t show_early_output;
> > +sig_atomic_t show_early_output = 0;
> > +show_early_output_fn_t early_output_function = NULL;
> ...
> > +		if (show_early_output) {
> > +			(*early_output_function)(revs, newlist);
> > +			show_early_output = 0;
> > +		}
> 
> The function pointer isn't necessary.  AFAIK its only called in
> this one call site.  So you can make a direct reference to the
> log_show_early function.

I disagree. The original intent was to decrease coupling between the
library-like revision walker and the actual log command. We aren't using
that flexibility now, but I don't see any reason to decrease it
(especially since it is so easy to keep it).

-Peff

^ permalink raw reply

* Re: git-svn taking a long time
From: Andrew Myrick @ 2010-02-10 17:39 UTC (permalink / raw)
  To: David Kågedal; +Cc: Git Mailing List
In-Reply-To: <87bpfxov6w.fsf@krank.kagedal.org>


On Feb 10, 2010, at 9:29 AM, David Kågedal wrote:

> Andrew Myrick <amyrick@apple.com> writes:
> 
>> Give 1.7.0-rc2 a try.  It includes commit 8bff7c5383ed833bd1df9c8d85c00a27af3e5b02, which attempts to persistently cache a lot of the processing that git-svn has to do on subversion's merge tickets, which has improved my fetch times significantly.  
> 
> By "merge tickets", are you talking about the merge functionality that
> appeared in subversion 1.5? We don't use that.

I do mean that.  If you don't use them, then I'm stumped.

> But I had another idea. I pecularity of our subversion repo is that we
> no longer use the foo/trunk branch, but only foo/branches/*. But we did
> once upon a time have a foo/trunk. And since I didn't include a "fetch =
> foo/trunk:refs/remotes/svn/trunk" in my config, it might need to refetch
> that information every time. For instance, the first revision is on
> trunk.
> 
> I'm rerunning the fetch now with the trunk added, so see if it helps.
> 
> And another note is that "git svn fetch --parent" was always quick.

Sounds reasonable.  Good luck!

-Andrew

^ permalink raw reply

* Re: [PATCH] Documentation: quote braces in {upstream} notation
From: Junio C Hamano @ 2010-02-10 18:06 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git
In-Reply-To: <a91746a91d0ad0b38a4e49f1f79a056a2d3eac69.1265819421.git.trast@student.ethz.ch>

Yikes; thanks for spotting this.

^ permalink raw reply

* Re: [RFC PATCHv2 04/10] gitweb: Use Cache::Cache compatibile (get, set) output caching
From: Jakub Narebski @ 2010-02-10 18:22 UTC (permalink / raw)
  To: Petr Baudis
  Cc: git, John 'Warthog9' Hawley,
	John 'Warthog9' Hawley
In-Reply-To: <20100210120257.GP4159@machine.or.cz>

Dnia środa 10. lutego 2010 13:02, Petr Baudis napisał:
> On Wed, Feb 10, 2010 at 12:28:14PM +0100, Jakub Narebski wrote:
>> On Wed, 10 Feb 2010, Petr Baudis wrote:
>>> On Wed, Feb 10, 2010 at 02:12:24AM +0100, Jakub Narebski wrote:

[...]
>>>> So we either would have to live with non-core PerlIO::Util or (pure Perl)
>>>> Capture::Tiny, or do the 'print -> print $out' patch...
>>> 
>>> All the magic methods seem to be troublesome, but in that case I'd
>>> really prefer a level of indirection instead of filehandle - as is,
>>> 'print (...) -> output (...)' ins. of 'print (...) -> print $out (...)'
>>> (or whatever). That should be really flexible and completely
>>> futureproof, and I don't think the level of indirection would incur any
>>> measurable overhead, would it?
>> 
>> First, it is not only 'print (...) -> print $out (...)'; [...]
>> 
>> Second, using "tie" on filehandle (on *STDOUT) can be used also for 
>> just capturing output, not only for "tee"-ing; [...]
>> 
>> Third, as you can see below tie-ing is about 1% slower than using
>> 'output (...)', which in turn is less than 10% slower than explicit
>> filehandle solution i.e. 'print $out (...)'... and is almost twice
>> slower than solution using PerlIO::Util
[...]
>>                Rate tie *STDOUT      output print \$out      perlio
>> tie *STDOUT 27636/s          --         -1%         -9%        -45%
>> output      28030/s          1%          --         -8%        -44%
>> print \$out 30319/s         10%          8%          --        -39%
>> perlio      49967/s         81%         78%         65%          --
>> need
 
> Ok, on my machine it's similar:
> 
>                 Rate      output tie *STDOUT print \$out
> output      150962/s          --         -1%         -7%
> tie *STDOUT 152769/s          1%          --         -6%
> print \$out 162604/s          8%          6%          --

I wonder why in my case the 'output (...)' solution was faster than tie,
and you have tie faster than 'output'... but I guess 1% is the noise
level.
 
> is roughly consistent image coming out of it.
> 
> I guess the time spent here is generally negligible in gitweb anyway...
> I suggested using output() because I think hacking it would be _very_
> _slightly_ easier than tied filehandle, but you are right that doing
> that is also really easy; having the possibility to use PerlIO::Util if
> available would be non-essentially nice, but requiring it by stock
> gitweb is not reasonable, especially seeing that it's not packaged even
> for Debian. ;-)

Well, the idea was to use PerlIO::Util if possible, checking it via

  out $use_perlio_layers = eval { require PerlIO::Util; 1 };

and fallback to generic mechanism if it is not present.  Only the
generic mechanism would have to be changed from manipulating *STDOUT
(*STDOUT = $data_fh etc.) to tied filehandle.

What we need to be careful about is ':utf8' vs ':raw' mode (IO layer).
In the PerlIO layers solution, and in 'print <sth> -> print $out <sth>'
solution where $out = $data_fh, and $data_fh was opened to in-memory
file, the data saved in variable is already converted, already passed
via 'utf8' layer, and is saved as bytes.  And if we use binary mode,
it is passed unchanged, and is also saved as bytes.  Therefore we can
save to cache file in ':raw' more, and read from cache file in ':raw'
mode, and that is why we don't need separate files for text and for
binary output.

PRINT method in class tied to filehandle gets _untransformed_ argument,
so we have to use utf8::encode($str) if in ':utf8' mode, and either
use PerlIO::get_layers on *STDOUT, or provide BINMODE method in tied
class to watch for mode changes.  (Note that e.g. for snapshots we 
print HTTP headers in ':utf8' mode, and the snapshot itself in ':raw'
i.e. binary mode.).

But all that is doable, and not that much work.  Well, perhaps more
than in the case of 'print -> print $out' etc., and opening in-memory
file via

  open $data_fh, '>', \$data;

but not that more, and we don't need extra global variable $out.  But
no large gitweb patch, and no worry about somebody accidentally using
'print <sth>;' or 'printf <fmt>, <sth>;' instead of respectively
'print $out <sth>;' and 'printf $out <fmt>, <sth>;'.


As to how I installed PerlIO::Util for myself (this might be interesting
to other people): in short, I use local::lib bootstrapping and cpan 
client.  I could from start install some Perl modules from CPAN locally
using 'cpan' client (included in perl RPM).  I have asked on #perl 
channel on FreeNode what to do, and they recommended local::lib.  After
following the bootstapping technique described in local::lib manpage
(see e.g. http://p3rl.org/local::lib) installing PerlIO::Util in
~/perl5 is as simple as 'cpan -i PerlIO::Util' (or using 'cpan' client
interactively).

You can always put
  use lib '/path/to/perl5/lib';
in your $GITWEB_CONFIG file.

Perhaps adding something like "use lib __DIR__.'/lib';" somewhere near
beginning of file (where __DIR__ is appropriate expression that expands
to directory the gitweb.cgi/gitweb.perl is in) to gitweb would be a good
idea?  Then you would be able to make __DIR__/lib symlink to local Perl
modules, or put extra modules by hand under __DIR__/lib.
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] blame: allow -L n,m to have an m bigger than the file's  line count
From: Junio C Hamano @ 2010-02-10 18:58 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Stephen Boyd, git
In-Reply-To: <76718491002100537h521fcc26gb267ed7cd2b8db6f@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> On Wed, Feb 10, 2010 at 2:27 AM, Stephen Boyd <bebarino@gmail.com> wrote:
>> and get what I want but that isn't very discoverable. If the range is
>> greater than the number of lines just truncate the range to go up to
>> the end of the file.
>
> I agree this is the right thing to do. I'm working on a patch to
> support matching multiple times when given a regex range and made just
> that change as well. :-)

I would mildly suggest against going in that direction.

This is merely "mildly", because truncating 99999 in "-L200,99999" to the
number of lines in the target blob would _not_ hurt.  But it is an ugly
hack.  I would be Ok with coding that special case, but I do not want to
see it advertised, especially if we are making "omission defaults to the
end" the documented way to explicitly say "I don't care to count, just do
it til the end".

While we are talking about touching the vicinity, I think we should
tighten the -L s,e parsing rules a bit further.

There is an undocumented code that swaps start and end if the given end is
smaller than the start.  This triggers even when "-L280,300" is mis-typed
as "-L280,30".  I was bitten by this more than once---when the input does
not make sense, we should actively error out, instead of doing a wrong
thing.  I suspect that I coded it that way _only_ to support this pattern:

	-L'/^#endif \/\* !WINDOWS \/\*/,-30'

i.e. "blame 30 lines before the '#endif' line".  But the code also
internally turns "-L50,-20" into "-L 50,30" and then swaps them to may
make it "-L30,50"; this was merely an unintended side effect.

I do want to see -L'/regexp/,-offset' keep working, I do not mind if we
keep taking "-L50,-20" as an unintuitive way to spell "-L30,50", or reject
"-L50,-20" as a nonsense.  But I do want to see us reject "-L280,30" as a
typo.

As to use of more than one -L option, especially when the start (or end
for that matter) is specified with an regexp, I am of two minds.

When annotating the body of two functions, frotz and nitfol, I might
expect this to work:

    -L'/^int frotz(/,/^}/'  -L'/^int nitfol(/,/^}/'

regardless of the order these functions appear in the blob (i.e. nitfol
may be defined first).  This requires that parsing of "regexp" needs to
reset to the beginning of blob for each -L option (iow, multiple -L are
parsed independently from each other).

But at the same time, if I am actually looking at the blob contents in one
terminal while spelling the blame command line in another, it would be
nicer if the multiple -L looked for patterns incrementally.  I may
appreciate if I can write the above command line as:

    -L'/^int frotz(/,/^}/'  -L'/nitfol/,/^}/'

when I can see in my "less" of the blob contents in the other terminal
that the first line that has string "nitfol" after the end of the
definition of "frotz" is the beginning of function "nitfol".

Another thing we _might_ want to consider doing is something like:

    -L'*/^#ifdef WINDOWS/,/^#endif \/\* WINDOWS \/\*/'

to tell it "I don't care to count how many WINDOWS ifdef blocks there are;
grab all of them".

Regardless of how parsing of multiple -L goes, you need to be careful to
sort the resulting line ranges and possibly coalesce them when there are
overlaps (e.g. "-L12,+7 -L10,+5" should become "-L10,17").  And be careful
about refcounting of origin.  You'll be making multiple blame_ent and
queuing them to the scoreboard when starting, all pointing at the blame
target blob; the origin blob needs to start with the right number of
references to keep origin_decref() discarding it.

^ permalink raw reply

* Re: [PATCH] Documentation: reword --thin description
From: Junio C Hamano @ 2010-02-10 18:59 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Stephen Boyd, git
In-Reply-To: <alpine.LFD.2.00.1002101037300.1681@xanadu.home>

Nicolas Pitre <nico@fluxnic.net> writes:

> ....  But --thin turned 
> out to be re-enabled by default for pushes by mistake on a few occasions 
> when the affected code has been reworked.  No idea what state it is now, 
> and I don't think that makes such a difference on the server in the end.

Hmmmm... it's doubly bad that the maintainer does not recall these
few occasions that mistakes happened.

^ permalink raw reply

* Re: [PATCH] git-archive documentation: .gitattributes must be committed
From: René Scharfe @ 2010-02-10 19:07 UTC (permalink / raw)
  To: Francois Marier; +Cc: git
In-Reply-To: <1265770284-14830-1-git-send-email-fmarier@gmail.com>

Am 10.02.2010 03:51, schrieb Francois Marier:
> Add a note to the documentation stating that the .gitattributes
> file must be present (i.e. committed) in the named tree that is
> exported.
> 
> This can be a bit confusing because it's different from .gitignore
> which takes effect as soon as the file is created.
> 
> Signed-off-by: Francois Marier <fmarier@gmail.com>
> ---
>  Documentation/git-archive.txt |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
> index 799c8b6..fcd681d 100644
> --- a/Documentation/git-archive.txt
> +++ b/Documentation/git-archive.txt
> @@ -112,6 +112,9 @@ export-subst::
>  	expand several placeholders when adding this file to an archive.
>  	See linkgit:gitattributes[5] for details.
>  
> +The .gitattributes file must be present in the named tree for it to take
> +effect. Uncommitted attributes will not be considered in exports.
> +
>  EXAMPLES
>  --------
>  git archive --format=tar --prefix=junk/ HEAD | (cd /var/tmp/ && tar xf -)::

Yeah, the description of --worktree-attributes is a bit terse.  The
lines you add make it appear almost as if this switch doesn't exist,
though; perhaps add a "unless --worktree-attributes is given" or similar
to one of the new sentences?

René

^ permalink raw reply

* Re: Suggestion on git-push --porcelain
From: Junio C Hamano @ 2010-02-10 19:14 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: Git Mailing List, Larry D'Anna, Jeff King
In-Reply-To: <be6fef0d1002100318v902689xc937cd332ac752b3@mail.gmail.com>

Tay Ray Chuan <rctay89@gmail.com> writes:

> I sense a chicken-and-egg situation here.
>
> Printing a "To: <destination>" to stdout is a correction as far as the
> current non-prefixed output scheme is concerned.
>
> If a prefixed scheme (or some other output scheme) is adopted, then
> where, or whether, the "To: <destination>" is printed, is not relevant
> to porcelain script writers - they just get ignored.

Sorry.  I don't get this.  Are you saying that you will write "To" without
the prefix in order to make older scripts ignore it?  What then would newer
scripts that want to use the information do?  Pay attention to lines that
do not start with your prefix?  Then what good does it do to introduce
such a prefix to the output?

I think what you need is not a prefix or any syntax but a rule that says
"ignore things that you do not understand" and possibly a way for the
output stream to say "if you do not understand this, you are too old to
correctly process this stream---please abort, because ignoring this will
make you produce an incorrect result".

^ permalink raw reply

* Re: [PATCH] git-archive documentation: .gitattributes must be committed
From: Junio C Hamano @ 2010-02-10 19:27 UTC (permalink / raw)
  To: René Scharfe; +Cc: Francois Marier, git
In-Reply-To: <4B7303FC.6070701@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

>> +The .gitattributes file must be present in the named tree for it to take
>> +effect. Uncommitted attributes will not be considered in exports.
>> +
>>  EXAMPLES
>>  --------
>>  git archive --format=tar --prefix=junk/ HEAD | (cd /var/tmp/ && tar xf -)::
>
> Yeah, the description of --worktree-attributes is a bit terse.  The
> lines you add make it appear almost as if this switch doesn't exist,
> though; perhaps add a "unless --worktree-attributes is given" or similar
> to one of the new sentences?

My impression has always been that people use attributes with archive more
often to _tweak_ how the archive is produced after the fact, and they do
so by modifying checked out .gitattributes (or $GIT_DIR/info/attributes)
than allowing a possibly stale .gitattributes file etched in stone^Wtree
being archived.  So in that sense, probably --worktree-attributes should
have been the default.

By the way, if you commit the attributes file, then commit the removal of
that attributes file, would that removed attributes file take effect when
you archive HEAD^ (which still had the attribues file)?  That's how I read
what the added description claims, but I somehow suspect that is not what
actually happens.

^ permalink raw reply

* Re: [PATCH] git-archive documentation: .gitattributes must be committed
From: Junio C Hamano @ 2010-02-10 19:33 UTC (permalink / raw)
  To: René Scharfe; +Cc: Francois Marier, git
In-Reply-To: <7v1vgsao21.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> By the way, if you commit the attributes file, then commit the removal of
> that attributes file, would that removed attributes file take effect when
> you archive HEAD^ (which still had the attribues file)?  That's how I read
> what the added description claims, but I somehow suspect that is not what
> actually happens.

Scratch this part, please.  You do read the tree into the index to make
it happen.

^ permalink raw reply

* Re: [PATCH] blame: allow -L n,m to have an m bigger than the file's  line count
From: Jay Soffian @ 2010-02-10 19:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stephen Boyd, git
In-Reply-To: <7vwrykapfp.fsf@alter.siamese.dyndns.org>

On Wed, Feb 10, 2010 at 1:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> While we are talking about touching the vicinity, I think we should
> tighten the -L s,e parsing rules a bit further.
>
> There is an undocumented code that swaps start and end if the given end is
> smaller than the start.  This triggers even when "-L280,300" is mis-typed
> as "-L280,30".  I was bitten by this more than once---when the input does
> not make sense, we should actively error out, instead of doing a wrong
> thing.  I suspect that I coded it that way _only_ to support this pattern:
>
>        -L'/^#endif \/\* !WINDOWS \/\*/,-30'
>
> i.e. "blame 30 lines before the '#endif' line".  But the code also
> internally turns "-L50,-20" into "-L 50,30" and then swaps them to may
> make it "-L30,50"; this was merely an unintended side effect.

I was curious what sed does. At least on my system, sed -n -e '10,1p'
prints just line 1. Seems a bit odd. sed -n -e '1,10000p' just prints
to the end, and doesn't error out if there are less than 10k lines.

> I do want to see -L'/regexp/,-offset' keep working, I do not mind if we
> keep taking "-L50,-20" as an unintuitive way to spell "-L30,50", or reject
> "-L50,-20" as a nonsense.  But I do want to see us reject "-L280,30" as a
> typo.
>
> As to use of more than one -L option, especially when the start (or end
> for that matter) is specified with an regexp, I am of two minds.

Actually, I was not planning on supporting multiple -L options, but rather...

> When annotating the body of two functions, frotz and nitfol, I might
> expect this to work:
>
>    -L'/^int frotz(/,/^}/'  -L'/^int nitfol(/,/^}/'
>
> regardless of the order these functions appear in the blob (i.e. nitfol
> may be defined first).  This requires that parsing of "regexp" needs to
> reset to the beginning of blob for each -L option (iow, multiple -L are
> parsed independently from each other).
>
> But at the same time, if I am actually looking at the blob contents in one
> terminal while spelling the blame command line in another, it would be
> nicer if the multiple -L looked for patterns incrementally.  I may
> appreciate if I can write the above command line as:
>
>    -L'/^int frotz(/,/^}/'  -L'/nitfol/,/^}/'
>
> when I can see in my "less" of the blob contents in the other terminal
> that the first line that has string "nitfol" after the end of the
> definition of "frotz" is the beginning of function "nitfol".
>
> Another thing we _might_ want to consider doing is something like:
>
>    -L'*/^#ifdef WINDOWS/,/^#endif \/\* WINDOWS \/\*/'
>
> to tell it "I don't care to count how many WINDOWS ifdef blocks there are;
> grab all of them".

That was my aim, but the syntax I'd settled on was to use
-L/pattern/..END where END is either a numerical argument or another
pattern. IOW, ".." instead of ",".

> Regardless of how parsing of multiple -L goes, you need to be careful to
> sort the resulting line ranges and possibly coalesce them when there are
> overlaps (e.g. "-L12,+7 -L10,+5" should become "-L10,17").  And be careful
> about refcounting of origin.  You'll be making multiple blame_ent and
> queuing them to the scoreboard when starting, all pointing at the blame
> target blob; the origin blob needs to start with the right number of
> references to keep origin_decref() discarding it.

Understood.

j.

^ permalink raw reply

* Re: RFC: git sync
From: Larry D'Anna @ 2010-02-10 19:47 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <m3wryl8lkp.fsf@localhost.localdomain>

* Jakub Narebski (jnareb@gmail.com) [100210 04:53]:
> Why not use the recpie from GitFaq:
> "How would I use "git push" to sync out of a host that I cannot pull from?"
> http://git.wiki.kernel.org/index.php/GitFaq#How_would_I_use_.22git_push.22_to_sync_out_of_a_host_that_I_cannot_pull_from.3F

Because that has nothing whatsoever to do with the problem I'm trying to solve.

I'm trying to make a common task more automated and convenient.  It's not like I
don't know how to perform it manually.

      --larry

^ permalink raw reply

* Re: [PATCH] blame: allow -L n,m to have an m bigger than the file's  line count
From: Junio C Hamano @ 2010-02-10 19:47 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Junio C Hamano, Stephen Boyd, git
In-Reply-To: <76718491002101139m4061fb90qcee7d34fca9f242f@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

>> As to use of more than one -L option, especially when the start (or end
>> for that matter) is specified with an regexp, I am of two minds.
>
> Actually, I was not planning on supporting multiple -L options, but rather...

That is extremely sad.

^ permalink raw reply

* Re: [PATCH] blame: allow -L n,m to have an m bigger than the file's  line count
From: Jay Soffian @ 2010-02-10 19:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stephen Boyd, git
In-Reply-To: <7v8wb098kv.fsf@alter.siamese.dyndns.org>

On Wed, Feb 10, 2010 at 2:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> That is extremely sad.

For you, I'll see what I can do. I don't like the maintainer to be grumpy. :-)

j.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox