Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/9] gitweb: change die_error to take "extra" argument for extended die information
From: Jakub Narebski @ 2010-01-15 22:40 UTC (permalink / raw)
  To: John 'Warthog9' Hawley; +Cc: git
In-Reply-To: <1263432185-21334-3-git-send-email-warthog9@eaglescrag.net>

"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:

Don't you want kernel.org address also for authorship?  Also commit
summary can be written in shorter way, see proposal below.

From: "John 'Warthog9' Hawley" <warthog9@kernel.org>
Subject: gitweb: Allow for longer error explanation in die_error()

> This is a small change that just adds a 3rd, optional, parameter to die_error
> that allows for extended error information to be output along with what the
> error was.

Singed-off-by: "John 'Warthog9' Hawley" <warthog9@kernel.org>
> ---
>  gitweb/gitweb.perl |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0a07d3a..8298de5 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3391,6 +3391,7 @@ sub git_footer_html {
>  sub die_error {
>  	my $status = shift || 500;
>  	my $error = shift || "Internal server error";
> +	my $extra = shift;
>  
>  	my %http_responses = (
>  		400 => '400 Bad Request',
> @@ -3405,8 +3406,13 @@ sub die_error {
>  <br /><br />
>  $status - $error
>  <br />
> -</div>
>  EOF
> +	if (defined $extra) {
> +		print "<hr />\n" .
> +			"$extra\n";

Following gitweb whitespace convention (tab for indent, space for
align), it should be:

 +		print "<hr />\n" .
 +		      "$extra\n";

(6 x space in place of last tab).

> +	}
> +	print "</div>\n";
> +
>  	git_footer_html();
>  	exit;
>  }
> -- 
> 1.6.5.2
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH v2 09/14] daemon: use run-command api for async serving
From: Johannes Sixt @ 2010-01-15 22:42 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: msysgit, git, Erik Faye-Lund
In-Reply-To: <1263591033-4992-10-git-send-email-kusmabite@gmail.com>

On Freitag, 15. Januar 2010, Erik Faye-Lund wrote:
> fork() is only available on POSIX, so to support git-daemon
> on Windows we have to use something else. Conveniently
> enough, we have an API for async operation already.

I had a huh?-moment when I read this statement. This patch does not use what 
we call 'async', but start_command().

-- Hannes

^ permalink raw reply

* [PATCH v2] Add push --set-upstream
From: Ilari Liusvaara @ 2010-01-15 22:47 UTC (permalink / raw)
  To: git

Frequent complaint is lack of easy way to set up upstream (tracking)
references for git pull to work as part of push command. So add switch
--set-upstream (-u) to do just that.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
Changes from v1:
- Handle 'git push -u <remote> HEAD' correctly.
- Add testsuite (thanks Peff), with some additional tests to test delete.
- Modify documentation for push -u (thanks Matthieu Moy).

 Documentation/git-push.txt |    9 +++++-
 builtin-push.c             |    1 +
 t/t5523-push-upstream.sh   |   64 ++++++++++++++++++++++++++++++++++++++++++++
 transport.c                |   49 +++++++++++++++++++++++++++++++++
 transport.h                |    1 +
 5 files changed, 123 insertions(+), 1 deletions(-)
 create mode 100755 t/t5523-push-upstream.sh

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index e3eb1e8..2a5394b 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git push' [--all | --mirror | --tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
-	   [--repo=<repository>] [-f | --force] [-v | --verbose]
+	   [--repo=<repository>] [-f | --force] [-v | --verbose] [-u | --set-upstream]
 	   [<repository> <refspec>...]
 
 DESCRIPTION
@@ -122,6 +122,13 @@ nor in any Push line of the corresponding remotes file---see below).
 	the name "origin" is used. For this latter case, this option
 	can be used to override the name "origin". In other words,
 	the difference between these two commands
+
+-u::
+--set-upstream::
+	For every branch that is up to date or successfully pushed, add
+	upstream (tracking) reference, used by argument-less
+	linkgit:git-pull[1] and other commands. For more information,
+	see 'branch.<name>.merge' in linkgit:git-config[1].
 +
 --------------------------
 git push public         #1
diff --git a/builtin-push.c b/builtin-push.c
index 28a26e7..75ddaf4 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -218,6 +218,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
+		OPT_BIT('u', "set-upstream", &flags, "Set upstream for git pull", TRANSPORT_PUSH_SET_UPSTREAM),
 		OPT_END()
 	};
 
diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
new file mode 100755
index 0000000..e098d37
--- /dev/null
+++ b/t/t5523-push-upstream.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+
+test_description='push with --set-upstream'
+. ./test-lib.sh
+
+test_expect_success 'setup bare parent' '
+	git init --bare parent &&
+	git remote add upstream parent
+'
+
+test_expect_success 'setup local commit' '
+	echo content >file &&
+	git add file &&
+	git commit -m one
+'
+
+check_config() {
+	(echo $2; echo $3) >expect.$1
+	(git config branch.$1.remote
+	 git config branch.$1.merge) >actual.$1
+	test_cmp expect.$1 actual.$1
+}
+
+test_expect_success 'push -u master:master' '
+	git push -u upstream master:master &&
+	check_config master upstream refs/heads/master
+'
+
+test_expect_success 'push -u master:other' '
+	git push -u upstream master:other &&
+	check_config master upstream refs/heads/other
+'
+
+test_expect_success 'push -u master2:master2' '
+	git branch master2 &&
+	git push -u upstream master2:master2 &&
+	check_config master2 upstream refs/heads/master2
+'
+
+test_expect_success 'push -u master2:other2' '
+	git push -u upstream master2:other2 &&
+	check_config master2 upstream refs/heads/other2
+'
+
+test_expect_success 'push -u :master2' '
+	git push -u upstream :master2 &&
+	check_config master2 upstream refs/heads/other2
+'
+
+test_expect_success 'push -u --all' '
+	git branch all1 &&
+	git branch all2 &&
+	git push -u --all &&
+	check_config all1 upstream refs/heads/all1 &&
+	check_config all2 upstream refs/heads/all2
+'
+
+test_expect_success 'push -u HEAD' '
+	git checkout -b headbranch &&
+	git push -u upstream HEAD &&
+	check_config headbranch upstream refs/heads/headbranch
+'
+
+test_done
diff --git a/transport.c b/transport.c
index b5332c0..e5b462b 100644
--- a/transport.c
+++ b/transport.c
@@ -8,6 +8,7 @@
 #include "bundle.h"
 #include "dir.h"
 #include "refs.h"
+#include "branch.h"
 
 /* rsync support */
 
@@ -135,6 +136,47 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
 	}
 }
 
+static void set_upstreams(struct transport *trans, struct ref *refs)
+{
+	struct ref *i;
+	for (i = refs; i; i = i->next) {
+		const char *localname;
+		const char *tmp;
+		const char *remotename;
+		unsigned char sha[20];
+		int flag = 0;
+		/*
+		 * Check suitability for tracking. Must be successful /
+		 * alreay up-to-date ref create/modify (not delete).
+		 */
+		if (i->status != REF_STATUS_OK &&
+			i->status != REF_STATUS_UPTODATE)
+			continue;
+		if (!i->peer_ref)
+			continue;
+		if (!i->new_sha1 || is_null_sha1(i->new_sha1))
+			continue;
+
+		/* Chase symbolic refs (mainly for HEAD). */
+		localname = i->peer_ref->name;
+		remotename = i->name;
+		tmp = resolve_ref(localname, sha, 1, &flag);
+		if (tmp && flag & REF_ISSYMREF &&
+			!prefixcmp(tmp, "refs/heads/"))
+			localname = tmp;
+
+		/* Both source and destination must be local branches. */
+		if (!localname || prefixcmp(localname, "refs/heads/"))
+			continue;
+		if (!remotename || prefixcmp(remotename, "refs/heads/"))
+			continue;
+
+		install_branch_config(BRANCH_CONFIG_VERBOSE,
+			localname + 11, trans->remote->name,
+			remotename);
+	}
+}
+
 static const char *rsync_url(const char *url)
 {
 	return prefixcmp(url, "rsync://") ? skip_prefix(url, "rsync:") : url;
@@ -974,6 +1016,10 @@ int transport_push(struct transport *transport,
 	verify_remote_names(refspec_nr, refspec);
 
 	if (transport->push) {
+		/* Maybe FIXME. But no important transport uses this case. */
+		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
+			die("This transport does not support using --set-upstream");
+
 		return transport->push(transport, refspec_nr, refspec, flags);
 	} else if (transport->push_refs) {
 		struct ref *remote_refs =
@@ -1002,6 +1048,9 @@ int transport_push(struct transport *transport,
 					verbose | porcelain, porcelain,
 					nonfastforward);
 
+		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
+			set_upstreams(transport, remote_refs);
+
 		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
 			struct ref *ref;
 			for (ref = remote_refs; ref; ref = ref->next)
diff --git a/transport.h b/transport.h
index 97ba251..c4314dd 100644
--- a/transport.h
+++ b/transport.h
@@ -91,6 +91,7 @@ struct transport {
 #define TRANSPORT_PUSH_VERBOSE 16
 #define TRANSPORT_PUSH_PORCELAIN 32
 #define TRANSPORT_PUSH_QUIET 64
+#define TRANSPORT_PUSH_SET_UPSTREAM 128
 
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);
-- 
1.6.6.102.gd6f8f.dirty

^ permalink raw reply related

* Re: [PATCH v2 13/14] daemon: use select() instead of poll()
From: Johannes Sixt @ 2010-01-15 22:49 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: msysgit, git, Erik Faye-Lund
In-Reply-To: <1263591033-4992-14-git-send-email-kusmabite@gmail.com>

On Freitag, 15. Januar 2010, Erik Faye-Lund wrote:
> +#undef FD_SET
> +#define FD_SET(fd, set) do { \
> +	((fd_set*)(set))->fd_array[((fd_set *)(set))->fd_count++] =
> _get_osfhandle(fd); \ +	} while(0)
> +#undef FD_ISSET
> +#define FD_ISSET(fd, set) __WSAFDIsSet(_get_osfhandle(fd), (fd_set
> *)(set)) +

I'm worried about the internals that you have to use here. Isn't it possible 
save the original macro text and use it in the new definition, like (this is 
for exposition only):

#define ORIG_FD_SET(fd, set) FD_SET(fd, set)
#undef FD_SET
#define FD_SET(fd, set) ORIG_FD_SET(_get_osfhandle(fd), set)

Another approach would be to extend the poll emulation such that it uses 
select if all FDs to wait for are sockets, and I think this would be the case 
in this application.

-- Hannes

^ permalink raw reply

* Re: [PATCH v2 00/14] daemon-win32
From: Erik Faye-Lund @ 2010-01-15 22:51 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: msysgit, git
In-Reply-To: <201001152327.23189.j6t@kdbg.org>

On Fri, Jan 15, 2010 at 11:27 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> A very nicely done series. Thank you very much!
>
> On Freitag, 15. Januar 2010, Erik Faye-Lund wrote:
>> Here's the long overdue v2 of my daemon-win32 attempt. A lot
>> has happened since v1. Most importantly, I abandoned using
>> the async API to replace fork(), and went for explicitly
>> spawning child process that handle the connection.
>
> IOW, you run git-daemon recursively in inetd mode (almost). Let's see what
> people say about this approach.
>
> -- Hannes
>

Yes. Or, a subset of the inetd-mode.

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: [PATCH v2 06/14] mingw: use real pid
From: Erik Faye-Lund @ 2010-01-15 22:53 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: msysgit, git
In-Reply-To: <201001152330.06083.j6t@kdbg.org>

On Fri, Jan 15, 2010 at 11:30 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> On Freitag, 15. Januar 2010, Erik Faye-Lund wrote:
>> @@ -729,7 +729,7 @@ static pid_t mingw_spawnve(const char *cmd, const char
>> **argv, char **env, return -1;
>>       }
>>       CloseHandle(pi.hThread);
>> -     return (pid_t)pi.hProcess;
>> +     return (pid_t)pi.dwProcessId;
>>  }
>
> You are not using the pi.hProcess anymore, so you must close it.
>

No. If I do, the pid becomes invalid after the process is finished,
and waitpid won't work. I couldn't find anywhere were we actually were
closing the handle, even after it was finished. So I don't think we
leak any more than we already did (for non-daemon purposes).

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: [msysGit] [PATCH v2 02/14] mingw: implement syslog
From: Janos Laube @ 2010-01-15 22:57 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: msysgit, git, j6t, Mike Pape, Erik Faye-Lund
In-Reply-To: <1263591033-4992-3-git-send-email-kusmabite@gmail.com>

> +static HANDLE ms_eventlog;
> +
> +void openlog(const char *ident, int logopt, int facility)
> +{
> +       if (ms_eventlog)
> +               return;
> +       ms_eventlog = RegisterEventSourceA(NULL, ident);
> +}

maybe make ms_eventlog thread local?
for example:

static __thread HANDLE ms_eventlog;

this would break compilation with msvc tho.

janos

^ permalink raw reply

* Re: [msysGit] [PATCH v2 02/14] mingw: implement syslog
From: Erik Faye-Lund @ 2010-01-15 23:01 UTC (permalink / raw)
  To: Janos Laube; +Cc: msysgit, git, j6t, Mike Pape
In-Reply-To: <9d6091531001151457v4c446b61k40c93f7c6180683d@mail.gmail.com>

On Fri, Jan 15, 2010 at 11:57 PM, Janos Laube <janos.dev@gmail.com> wrote:
>> +static HANDLE ms_eventlog;
>> +
>> +void openlog(const char *ident, int logopt, int facility)
>> +{
>> +       if (ms_eventlog)
>> +               return;
>> +       ms_eventlog = RegisterEventSourceA(NULL, ident);
>> +}
>
> maybe make ms_eventlog thread local?
> for example:
>
> static __thread HANDLE ms_eventlog;
>
> this would break compilation with msvc tho.
>
> janos
>

Since the code that use it isn't multi-threaded, I fail to see the
point. In fact even if it were, I'm not sure I see the big point...
especially since the "__thread"-keyword isn't used (AFAICT) at all in
the git source code so far.

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: git clone against firewall
From: Andreas Krey @ 2010-01-15 23:03 UTC (permalink / raw)
  To: Sebastian Pipping; +Cc: Andreas Schwab, Junio C Hamano, git
In-Reply-To: <4B50C311.2040305@hartwork.org>

On Fri, 15 Jan 2010 20:33:37 +0000, Sebastian Pipping wrote:
...
> > Not much that
> > can be done about that.
> 
> My problem actually is not the dropping but that git doesn't stop
> trying.  I actually want it to fail.

It does. After the usual network timeout, which unfortunately is
a few minutes, and thus not really useful for trying several
machines:

  74.43.91.xxx[0: 74.43.91.xxx]: errno=Connection timed out
  fatal: unable to connect a socket (Connection timed out)

Andreas

^ permalink raw reply

* Re: [PATCH v2 13/14] daemon: use select() instead of poll()
From: Erik Faye-Lund @ 2010-01-15 23:08 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: msysgit, git
In-Reply-To: <201001152349.06418.j6t@kdbg.org>

On Fri, Jan 15, 2010 at 11:49 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> On Freitag, 15. Januar 2010, Erik Faye-Lund wrote:
>> +#undef FD_SET
>> +#define FD_SET(fd, set) do { \
>> +     ((fd_set*)(set))->fd_array[((fd_set *)(set))->fd_count++] =
>> _get_osfhandle(fd); \ +       } while(0)
>> +#undef FD_ISSET
>> +#define FD_ISSET(fd, set) __WSAFDIsSet(_get_osfhandle(fd), (fd_set
>> *)(set)) +
>
> I'm worried about the internals that you have to use here. Isn't it possible
> save the original macro text and use it in the new definition, like (this is
> for exposition only):
>
> #define ORIG_FD_SET(fd, set) FD_SET(fd, set)
> #undef FD_SET
> #define FD_SET(fd, set) ORIG_FD_SET(_get_osfhandle(fd), set)
>

Redefining it is indeed fishy - I guess I should also have noted that
I even stripped the code down slightly (compared to the original).

I'm no preprocessor wizard, but I'll give it a stab.

> Another approach would be to extend the poll emulation such that it uses
> select if all FDs to wait for are sockets, and I think this would be the case
> in this application.
>

The problem with that is differentiating between pipes and sockets.
GetFileType() returns FILE_TYPE_PIPE for sockets (ugh). I did find
some code in gnulib that used WSAEnumNetworkEvents() to differentiate
between them, but I find this quite hacky.

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: [msysGit] [PATCH v2 02/14] mingw: implement syslog
From: Janos Laube @ 2010-01-15 23:09 UTC (permalink / raw)
  To: kusmabite; +Cc: msysgit, git, j6t, Mike Pape
In-Reply-To: <40aa078e1001151501s462802ffua3aec600ed38f516@mail.gmail.com>

> Since the code that use it isn't multi-threaded, I fail to see the
> point. In fact even if it were, I'm not sure I see the big point...
> especially since the "__thread"-keyword isn't used (AFAICT) at all in
> the git source code so far.

that's why i have put a question mark behind my sentence. it was just
an idea :-). it would allow different threads to be an own event
source. but yes, i wasn't sure how much git makes use of threads. if
it doesn't, it does not make much sense at the moment, indeed.

janos

^ permalink raw reply

* Re: [RFC] Git Wiki Move
From: J.H. @ 2010-01-15 23:18 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Petr Baudis, git
In-Reply-To: <4B4F68E8.5050809@eaglescrag.net>

Quick update - I think I've got the vast majority of the obvious and
simple to correct problems fixed at http://git.wiki.kernel.org anyone
want to run through and see if there's anything else that would be
considered a show stopper?

I do know that there's some table formatting stuff that I need to look
into, but there aren't that many tables on the wiki and those can likely
be cleaned by hand now.

- John 'Warthog9' Hawley

^ permalink raw reply

* Re: [RFC] Git Wiki Move
From: Sverre Rabbelier @ 2010-01-15 23:21 UTC (permalink / raw)
  To: J.H.; +Cc: Matthieu Moy, Petr Baudis, git
In-Reply-To: <4B50F7DB.7020204@eaglescrag.net>

Heya,

On Sat, Jan 16, 2010 at 00:18, J.H. <warthog19@eaglescrag.net> wrote:
> Quick update - I think I've got the vast majority of the obvious and
> simple to correct problems fixed at http://git.wiki.kernel.org anyone
> want to run through and see if there's anything else that would be
> considered a show stopper?

I'd say it's pretty embarassing if our FAQ [0] is broken. Don't have
the time to fix it manually atm though :(.

[0] http://git.wiki.kernel.org/index.php/GitFaq

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH v2 13/14] daemon: use select() instead of poll()
From: Erik Faye-Lund @ 2010-01-15 23:23 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: msysgit, git
In-Reply-To: <40aa078e1001151508j208fa50boc5565a3be6bef893@mail.gmail.com>

On Sat, Jan 16, 2010 at 12:08 AM, Erik Faye-Lund
<kusmabite@googlemail.com> wrote:
> On Fri, Jan 15, 2010 at 11:49 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>> On Freitag, 15. Januar 2010, Erik Faye-Lund wrote:
>>> +#undef FD_SET
>>> +#define FD_SET(fd, set) do { \
>>> +     ((fd_set*)(set))->fd_array[((fd_set *)(set))->fd_count++] =
>>> _get_osfhandle(fd); \ +       } while(0)
>>> +#undef FD_ISSET
>>> +#define FD_ISSET(fd, set) __WSAFDIsSet(_get_osfhandle(fd), (fd_set
>>> *)(set)) +
>>
>> I'm worried about the internals that you have to use here. Isn't it possible
>> save the original macro text and use it in the new definition, like (this is
>> for exposition only):
>>
>> #define ORIG_FD_SET(fd, set) FD_SET(fd, set)
>> #undef FD_SET
>> #define FD_SET(fd, set) ORIG_FD_SET(_get_osfhandle(fd), set)
>>
>
> Redefining it is indeed fishy - I guess I should also have noted that
> I even stripped the code down slightly (compared to the original).
>
> I'm no preprocessor wizard, but I'll give it a stab.
>

The following worked for me:

diff --git a/compat/mingw.h b/compat/mingw.h
index e515726..ea15967 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -269,10 +269,13 @@ int mingw_accept(int sockfd, struct sockaddr
*sa, socklen_t *sz);
 int mingw_rename(const char*, const char*);
 #define rename mingw_rename

+static inline void mingw_fd_set(int fd, fd_set *set)
+{
+	FD_SET(_get_osfhandle(fd), set);
+}
 #undef FD_SET
-#define FD_SET(fd, set) do { \
-	((fd_set*)(set))->fd_array[((fd_set *)(set))->fd_count++] =
_get_osfhandle(fd); \
-	} while(0)
+#define FD_SET(a,b) mingw_fd_set(a,b)
+
 #undef FD_ISSET
 #define FD_ISSET(fd, set) __WSAFDIsSet(_get_osfhandle(fd), (fd_set *)(set))


-- 
Erik "kusma" Faye-Lund

^ permalink raw reply related

* Re: [PATCH 3/9] gitweb: Add option to force version match
From: Jakub Narebski @ 2010-01-15 23:36 UTC (permalink / raw)
  To: John 'Warthog9' Hawley; +Cc: git
In-Reply-To: <1263432185-21334-4-git-send-email-warthog9@eaglescrag.net>

"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:

> This adds $git_versions_must_match variable, which is set to true,
> checks that we are running on the same version of git that we
> shipped with, and if not throw '500 Internal Server Error' error.
> What is checked is the version of gitweb (embedded in building
> gitweb.cgi), against version of runtime git binary used.
> 
> Gitweb can usually run with a mismatched git install.  This is more
> here to give an obvious warning as to whats going on vs. silently
> failing.
> 
> By default this feature is turned on.

If this feature is turned on, then I would prefer for gitweb tests to
have it explicitly turned off, so I don't need to rebuild git to test
gitweb.  

Also it would be nice to have some tests for this new feature.

See patch below (proposed to be squashed with this one).


P.S. t is there where I have noticed the issue with undefined $action
in git_footer_html(), mentioned in reply to patch 1/9.
 
> Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---

Here could be information about differences from previous versions of
this patch.

>  gitweb/README      |    3 +++
>  gitweb/gitweb.perl |   23 +++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/README b/gitweb/README
> index 6c2c8e1..03151d2 100644
> --- a/gitweb/README
> +++ b/gitweb/README
> @@ -233,6 +233,9 @@ not include variables usually directly set during build):
>     If server load exceed this value then return "503 Service Unavaliable" error.
>     Server load is taken to be 0 if gitweb cannot determine its value.  Set it to
>     undefined value to turn it off.  The default is 300.
> + * $git_versions_must_match
> +   If set, gitweb fails with 500 Internal Server Error if the version of gitweb
> +   doesn't match version of git binary.  The default is true.
>  
>  
>  Projects list file format
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8298de5..b41bc33 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -221,6 +221,9 @@ our %avatar_size = (
>  	'double'  => 32
>  );
>  
> +# If it is true, exit if gitweb version and git binary version don't match
> +our $git_versions_must_match = 1;
> +
>  # Used to set the maximum load that we will still respond to gitweb queries.
>  # If server load exceed this value then return "503 server busy" error.
>  # If gitweb cannot determined server load, it is taken to be 0.
> @@ -587,6 +590,26 @@ if (defined $maxload && get_loadavg() > $maxload) {
>  our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
>  $number_of_git_cmds++;
>  
> +# Throw an error if git versions does not match, if $git_versions_must_match is true.
> +if ($git_versions_must_match &&
> +    $git_version ne $version) {
> +	my $admin_contact =
> +		defined $ENV{'SERVER_ADMIN'} ? ", $ENV{'SERVER_ADMIN'}," : '';
> +	my $err_msg = <<EOT;
> +<h1 align="center">*** Warning ***</h1>
> +<p>
> +This version of gitweb was compiled for <b>@{[esc_html($version)]}</b>,
> +however git version <b>@{[esc_html($git_version)]}</b> was found on server,
> +and administrator requested strict version checking.
> +</p>
> +<p>
> +Please contact the server administrator${admin_contact} to either configure
> +gitweb to allow mismatched versions, or update git or gitweb installation.
> +</p>
> +EOT
> +	die_error(500, 'Internal server error', $err_msg);
> +}
> +
>  $projects_list ||= $projectroot;
>  
>  # ======================================================================
diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 76d8b7b..d9ffc90
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -25,6 +25,7 @@ our \$favicon = 'file:///$TEST_DIRECTORY/../gitweb/git-favicon.png';
 our \$projects_list = '';
 our \$export_ok = '';
 our \$strict_export = '';
+our \$git_versions_must_match = 0;
 
 EOF
 
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index 0688a57..721900e 100755
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -113,5 +113,31 @@ test_expect_success 'snapshots: bad object id' '
 '
 test_debug 'cat gitweb.output'
 
+# ======================================================================
+# check $git_versions_must_match feature
+# should be last section, just in case
+cp -f gitweb_config.perl gitweb_config.perl.bak
+echo 'our $git_versions_must_match = 1;' >>gitweb_config.perl
+
+cat <<\EOF >>gitweb_config.perl
+our $version = "current";
+EOF
+test_expect_success 'force version match: no match' '
+	gitweb_run "p=.git" &&
+	grep "500 - Internal Server Error" gitweb.output
+'
+test_debug 'cat gitweb.headers'
+
+cat <<\EOF >>gitweb_config.perl
+# must be kept in sync with code in gitweb/gitweb.perl
+our $version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
+EOF
+test_expect_success 'force version match: match' '
+	gitweb_run "p=.git" &&
+	grep "Status: 200 OK" gitweb.output
+'
+test_debug 'cat gitweb.headers'
+
+mv -f gitweb_config.perl.bak gitweb_config.perl
 
 test_done

> -- 
> 1.6.5.2
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply related

* Re: [PATCH v2] Add push --set-upstream
From: Junio C Hamano @ 2010-01-15 23:40 UTC (permalink / raw)
  To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1263595630-18962-1-git-send-email-ilari.liusvaara@elisanet.fi>

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:

> @@ -974,6 +1016,10 @@ int transport_push(struct transport *transport,
>  	verify_remote_names(refspec_nr, refspec);
>  
>  	if (transport->push) {
> +		/* Maybe FIXME. But no important transport uses this case. */
> +		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
> +			die("This transport does not support using --set-upstream");
> +

Would it be better to just warn() and continue instead of dying?  I think
it can be argued both ways, and I personally think die() is better by
making it more visible that the user does not have the config s/he wanted
to add, but I am pointing it out just in case somebody thinks of a better
solution (of course, doing an extra ls-remote and doing the configuration
is such a "better solution" but that is not what I mean---I am not that
greedy).

> @@ -1002,6 +1048,9 @@ int transport_push(struct transport *transport,
>  					verbose | porcelain, porcelain,
>  					nonfastforward);
>  
> +		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
> +			set_upstreams(transport, remote_refs);
> +
>  		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
>  			struct ref *ref;
>  			for (ref = remote_refs; ref; ref = ref->next)

Shouldn't this honor TRANSPORT_PUSH_DRY_RUN?  IOW, when should it touch
the configuration if you do this sequence?

	# I am paranoid and want to check what happens first
	git push -n --track there this
        # Ok let's do it for real.
        git push    --track there this

^ permalink raw reply

* Re: [PATCH 7/9] gitweb: cleanup error message produced by undefined $site_header
From: Jakub Narebski @ 2010-01-15 23:49 UTC (permalink / raw)
  To: John 'Warthog9' Hawley; +Cc: git
In-Reply-To: <1263432185-21334-8-git-send-email-warthog9@eaglescrag.net>

"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:

I have modified commit message a bit

> If $site_header is not defined you get extraneous errors in the web
> logs:
> 
> [Wed Jan 13 16:55:42 2010] [error] [client ::1] [Wed Jan 13 16:55:42 2010]
>  gitweb.cgi: Use of uninitialized value $site_header in -f at .../gitweb.cgi line 3287.,
>  referer: http://git/gitweb.cgi
> 
> for example (line wrapped for better readibility).  This commit
> ensures that the variable is defined before trying to use it.

  Ordinarily build procedure ensures that $site_header is defined (but
  empty, therefore false-ish), so this issue might happen only because
  of errors in gitweb config file.  Nevetheless it is better to code
  defensively.

  Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 07fdeb5..c4a177d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3277,7 +3277,7 @@ EOF
>  	print "</head>\n" .
>  	      "<body>\n";
>  
> -	if (-f $site_header) {
> +	if ($site_header && -f $site_header) {

It should be, I think

+	if (defined $site_header && -f $site_header) {

although I guess that nobody would name $site_header file "0".

>  		insert_file($site_header);
>  	}
>  
> -- 

More important is the fact that it is not the only variable holding
file name, that is not checked that it is defined before use.  You
should do the same change also for $site_footer and $home_text, and
die_error(500, "Gitweb misconfigured") or just die_error(500) if
$projects_list is not defined.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [RFC] Git Wiki Move
From: Junio C Hamano @ 2010-01-15 23:50 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: J.H., Matthieu Moy, Petr Baudis, git
In-Reply-To: <fabb9a1e1001151521s1837b3d5o2a35cb5bb35c8038@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Sat, Jan 16, 2010 at 00:18, J.H. <warthog19@eaglescrag.net> wrote:
>> Quick update - I think I've got the vast majority of the obvious and
>> simple to correct problems fixed at http://git.wiki.kernel.org anyone
>> want to run through and see if there's anything else that would be
>> considered a show stopper?
>
> I'd say it's pretty embarassing if our FAQ [0] is broken.

Many "<<GitLink(foobar}}" in the FAQ page do seem like result of
mechanical misconversion.

John, thanks for doing this.  If people fix things up manually, can they
expect their fixes will be kept from now on, iow, the changes will not be
overwritten by "Ok, I found a much better mechanical conversion tool and
updated with the latest snapshot from the original wiki" in the future?

^ permalink raw reply

* Re: [PATCH v2] Add push --set-upstream
From: Junio C Hamano @ 2010-01-15 23:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ilari Liusvaara, git
In-Reply-To: <7vzl4frl7i.fsf@alter.siamese.dyndns.org>

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

>         # Ok let's do it for real.
>         git push    --track there this

Ugh; s/--track/--set-upstream/, of course.

^ permalink raw reply

* Re: [RFC] Git Wiki Move
From: J.H. @ 2010-01-16  0:02 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Matthieu Moy, Petr Baudis, git
In-Reply-To: <fabb9a1e1001151521s1837b3d5o2a35cb5bb35c8038@mail.gmail.com>

On 01/15/2010 03:21 PM, Sverre Rabbelier wrote:
> Heya,
> 
> On Sat, Jan 16, 2010 at 00:18, J.H. <warthog19@eaglescrag.net> wrote:
>> Quick update - I think I've got the vast majority of the obvious and
>> simple to correct problems fixed at http://git.wiki.kernel.org anyone
>> want to run through and see if there's anything else that would be
>> considered a show stopper?
> 
> I'd say it's pretty embarassing if our FAQ [0] is broken. Don't have
> the time to fix it manually atm though :(.
> 
> [0] http://git.wiki.kernel.org/index.php/GitFaq

If you can be more specific I can go through and fix it...  There's some
extraneous bits on the page that I haven't figured out what they were
for originally but most everything on the page seems to work fine for me...

Like I said I'm not claiming it's completely 100% accurate in the
transition, but for a page that's 144K in size it looks like it's
predominantly correct.

- John 'Warthog9' Hawley

^ permalink raw reply

* Re: [PATCH v2] Add push --set-upstream
From: Nanako Shiraishi @ 2010-01-16  0:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ilari Liusvaara, git
In-Reply-To: <7vfx66sz5p.fsf@alter.siamese.dyndns.org>

Quoting Junio C Hamano <gitster@pobox.com>

> Junio C Hamano <gitster@pobox.com> writes:
>
>>         # Ok let's do it for real.
>>         git push    --track there this
>
> Ugh; s/--track/--set-upstream/, of course.

How can I use this to say I want to use 'pull --rebase'?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: [RFC] Git Wiki Move
From: J.H. @ 2010-01-16  0:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sverre Rabbelier, Matthieu Moy, Petr Baudis, git
In-Reply-To: <7vska7rkr0.fsf@alter.siamese.dyndns.org>

On 01/15/2010 03:50 PM, Junio C Hamano wrote:
> Sverre Rabbelier <srabbelier@gmail.com> writes:
> 
>> On Sat, Jan 16, 2010 at 00:18, J.H. <warthog19@eaglescrag.net> wrote:
>>> Quick update - I think I've got the vast majority of the obvious and
>>> simple to correct problems fixed at http://git.wiki.kernel.org anyone
>>> want to run through and see if there's anything else that would be
>>> considered a show stopper?
>>
>> I'd say it's pretty embarassing if our FAQ [0] is broken.
> 
> Many "<<GitLink(foobar}}" in the FAQ page do seem like result of
> mechanical misconversion.
> 
> John, thanks for doing this.  If people fix things up manually, can they
> expect their fixes will be kept from now on, iow, the changes will not be
> overwritten by "Ok, I found a much better mechanical conversion tool and
> updated with the latest snapshot from the original wiki" in the future?

I have no intention of re-converting the data, so yes fixups will not be
squashed.  I've got a small bot script right now that if there's a
change that needs to happen pretty universally across the entirety wiki
it can be run (it's written in perl, reads in the page text, runs the
conversion and re-uploads it), so if there's something that's
particularly repetitive people can get me some code that would fix the
problem.

So again short answer: I'm at the point where I'd rather clean-up what
we've got then try and re-import the data.

- John 'Warthog9' Hawley

^ permalink raw reply

* Re: [PATCH v2] Add push --set-upstream
From: Junio C Hamano @ 2010-01-16  0:06 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Ilari Liusvaara, git
In-Reply-To: <20100116090321.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting Junio C Hamano <gitster@pobox.com>
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>>         # Ok let's do it for real.
>>>         git push    --track there this
>>
>> Ugh; s/--track/--set-upstream/, of course.
>
> How can I use this to say I want to use 'pull --rebase'?

I dunno; "git push --set-upstream=rebase", perhaps?

^ permalink raw reply

* Re: [RFC] Git Wiki Move
From: Sverre Rabbelier @ 2010-01-16  0:08 UTC (permalink / raw)
  To: J.H.; +Cc: Matthieu Moy, Petr Baudis, git
In-Reply-To: <4B510217.8060200@eaglescrag.net>

Heya,

On Sat, Jan 16, 2010 at 01:02, J.H. <warthog19@eaglescrag.net> wrote:
> If you can be more specific I can go through and fix it...

>From the original faq:

<<GitLink(git-name, Because Linus is an egotistical git)>>

Is translated to

<<[[GitLink]](git-name, Because Linus is an egotistical git}}

But should instead be turned into a comment (from the source of the faq):

<!-- GitLink[git-name] Because Linus is an egotistical git -->

> There's some
> extraneous bits on the page that I haven't figured out what they were
> for originally but most everything on the page seems to work fine for me...

We're probably referring to the same then :).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [RFC] Git Wiki Move
From: Junio C Hamano @ 2010-01-16  0:12 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: J.H., Matthieu Moy, Petr Baudis, git
In-Reply-To: <fabb9a1e1001151608k6911bcf8p854d97c2f2d46264@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> From the original faq:
>
> <<GitLink(git-name, Because Linus is an egotistical git)>>
>
> Is translated to
>
> <<[[GitLink]](git-name, Because Linus is an egotistical git}}
>
> But should instead be turned into a comment (from the source of the faq):
>
> <!-- GitLink[git-name] Because Linus is an egotistical git -->
>
>> There's some
>> extraneous bits on the page that I haven't figured out what they were
>> for originally but most everything on the page seems to work fine for me...
>
> We're probably referring to the same then :).

IIRC, they are read by gitbot listening to #git channel at freenode, so
that people can say "faq git-name" and such to summon a response.

^ 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