Git development
 help / color / mirror / Atom feed
* Re: [PATCHv2] Add options to specify snapshot file name, prefix
From: Jakub Narebski @ 2011-11-04 16:10 UTC (permalink / raw)
  To: Thomas Guyot-Sionnest; +Cc: git, Jakub Narebski
In-Reply-To: <1320367999-24435-1-git-send-email-dermoth@aei.ca>

Thomas Guyot-Sionnest <dermoth@aei.ca> writes:

> commit b629275 implemented "smart" snapshot names and prefixes. I have
> scripts that used to rely on the old behaviour which allowed in some
> cases to have fixed prefix, and would require modifications to work with
> newer Gitweb.

If scripts use 'wget' or 'curl' you can always change the name of
saved file:

  wget -O <file> ...
  curl -o <file> ...

If downloaded snapshot is compressed tarfile, you can use
--strip-components=1 to strip prefix.
 
> This patch adds two parameters for overriding the snapshot name and
> prefix, sn and sp respectively. For example, to get a snapshot
> named "myproject.[suffix]" with no prefix one can add this query string:
>   ?sn=myproject;sp=

Would you need support for expandable parameters in both (a la
'action' feature)?

[...] 
> @@ -6684,11 +6686,19 @@ sub git_snapshot {
>  	}
>  
>  	my ($name, $prefix) = snapshot_name($project, $hash);
> +	if (defined($input_params{'snapshot_name'})) {
> +		$name = $input_params{'snapshot_name'};
> +	}
> +	if (defined($input_params{'snapshot_prefix'})) {
> +		$prefix = $input_params{'snapshot_prefix'};
> +	} else {
> +		$prefix .= '/';
> +	}
>  	my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
>  	my $cmd = quote_command(
>  		git_cmd(), 'archive',
>  		"--format=$known_snapshot_formats{$format}{'format'}",
> -		"--prefix=$prefix/", $hash);
> +		"--prefix=$prefix", $hash);
>  	if (exists $known_snapshot_formats{$format}{'compressor'}) {
>  		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
>  	}

I wonder if you really want to allow prefix which do not end in '/'
(which would be suprising, isn't it), or just allow empty prefix too.

For example

  @@ -6684,11 +6686,19 @@ sub git_snapshot {
   	}
   
   	my ($name, $prefix) = snapshot_name($project, $hash);
  +	if (defined($input_params{'snapshot_name'})) {
  +		$name = $input_params{'snapshot_name'};
  +	}
  +	if (defined($input_params{'snapshot_prefix'})) {
  +		$prefix = $input_params{'snapshot_prefix'};
  +	}
   	my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
   	my $cmd = quote_command(
   		git_cmd(), 'archive',
   		"--format=$known_snapshot_formats{$format}{'format'}",
  -		"--prefix=$prefix/", $hash);
  +		($prefix eq "" ? () : "--prefix=$prefix"), $hash);
   	if (exists $known_snapshot_formats{$format}{'compressor'}) {
   		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
   	}

-- 
Jakub Narębski

^ permalink raw reply

* Re: [PATCH] http-push: don't always prompt for password
From: Junio C Hamano @ 2011-11-04 16:48 UTC (permalink / raw)
  To: Stefan Naewe; +Cc: git, peff
In-Reply-To: <1320390188-24334-1-git-send-email-stefan.naewe@gmail.com>

Stefan Naewe <stefan.naewe@gmail.com> writes:

> http-push prompts for a password when the URL is set as
> 'https://user@host/repo' even though there is one set
> in ~/.netrc. Pressing ENTER at the password prompt succeeds
> then, but is a annoying and makes it almost useless
> in a shell script, e.g.
>
> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
> ---

Thanks.

With this the only callsite of init_curl_http_auth() becomes the one after
we get the 401 response, and this caller makes sure that user_name is not
NULL.

Do we still want "if (user_name)" inside init_curl_http_auth()?

I tried to rewrite the proposed commit log message to describe the real
issue, and here is what I came up with:

Author: Stefan Naewe <stefan.naewe@gmail.com>
Date:   Fri Nov 4 08:03:08 2011 +0100

    http: don't always prompt for password
    
    When a username is already specified at the beginning of any HTTP
    transaction (e.g. "git push https://user@hosting.example.com/project.git"
    or "git ls-remote https://user@hosting.example.com/project.git"), the code
    interactively asks for a password before calling into the libcurl library.
    It is very likely that the reason why user included the username in the
    URL is because the user knows that it would require authentication to
    access the resource. Asking for the password upfront would save one
    roundtrip to get a 401 response, getting the password and then retrying
    the request. This is a reasonable optimization.
    
    HOWEVER.
    
    This is done even when $HOME/.netrc might have a corresponding entry to
    access the site, or the site does not require authentication to access the
    resource after all. But neither condition can be determined until we call
    into libcurl library (we do not read and parse $HOME/.netrc ourselves). In
    these cases, the user is forced to respond to the password prompt, only to
    give a password that is not used in the HTTP transaction. If the password
    is in $HOME/.netrc, an empty input would later let the libcurl layer to
    pick up the password from there, and if the resource does not require
    authentication, any input would be taken and then discarded without
    getting used. It is wasteful to ask this unused information to the end
    user.
    
    Reduce the confusion by not trying to optimize for this case and always
    incur roundtrip penalty. An alternative might be to document this and keep
    this round-trip optimization as-is.
    
    Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
    Helped-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

What is somewhat troubling is that after analyzing the root cause of the
issue, I am wondering if a more correct fix is to remove the user@ part
from the URL (in other words, document that a URL with an embedded
username will ask for password upfront, and tell the users that if they
have netrc entries or if they are accessing a resource that does not
require authentication, they should omit the username from the URL).

^ permalink raw reply

* Re: [PATCH v3 2/3] http.c: Use timeout suggested by curl instead of fixed 50ms timeout
From: Junio C Hamano @ 2011-11-04 17:13 UTC (permalink / raw)
  To: Mika Fischer; +Cc: git, daniel, peff
In-Reply-To: <1320416367-28843-3-git-send-email-mika.fischer@zoopnet.de>

Mika Fischer <mika.fischer@zoopnet.de> writes:

> Recent versions of curl can suggest a period of time the library user
> should sleep and try again, when curl is blocked on reading or writing
> (or connecting). Use this timeout instead of always sleeping for 50ms.
>
> Signed-off-by: Mika Fischer <mika.fischer@zoopnet.de>

Thanks.

I'm inclined to squash in the following to narrow the scope of
curl_timeout, though.

diff --git a/http.c b/http.c
index 5cb0fb6..924be52 100644
--- a/http.c
+++ b/http.c
@@ -636,9 +636,6 @@ void run_active_slot(struct active_request_slot *slot)
 	fd_set excfds;
 	int max_fd;
 	struct timeval select_timeout;
-#if LIBCURL_VERSION_NUM >= 0x070f04
-	long curl_timeout;
-#endif
 	int finished = 0;
 
 	slot->finished = &finished;
@@ -655,6 +652,7 @@ void run_active_slot(struct active_request_slot *slot)
 
 		if (slot->in_use && !data_received) {
 #if LIBCURL_VERSION_NUM >= 0x070f04
+			long curl_timeout;
 			curl_multi_timeout(curlm, &curl_timeout);
 			if (curl_timeout == 0) {
 				continue;

^ permalink raw reply related

* Re: [PATCH] http-push: don't always prompt for password
From: Jeff King @ 2011-11-04 17:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stefan Naewe, git
In-Reply-To: <7vlirvdeb2.fsf@alter.siamese.dyndns.org>

On Fri, Nov 04, 2011 at 09:48:17AM -0700, Junio C Hamano wrote:

> Stefan Naewe <stefan.naewe@gmail.com> writes:
> 
> > http-push prompts for a password when the URL is set as
> > 'https://user@host/repo' even though there is one set
> > in ~/.netrc. Pressing ENTER at the password prompt succeeds
> > then, but is a annoying and makes it almost useless
> > in a shell script, e.g.
> >
> > Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
> > ---
> 
> Thanks.
> 
> With this the only callsite of init_curl_http_auth() becomes the one after
> we get the 401 response, and this caller makes sure that user_name is not
> NULL.
> 
> Do we still want "if (user_name)" inside init_curl_http_auth()?

Since we now only call init_curl_http_auth when we know we need auth, I
think it would make more sense to just move the user_name asking there,
too, like:

  static void init_curl_http_auth(CURL *result)
  {
          struct strbuf up = STRBUF_INIT;

          if (!user_name)
                  user_name = xstrdup(git_getpass_with_description("Username", description);
          if (!user_pass)
                  user_pass = xstrdup(git_getpass_with_description("Password", description);

          strbuf_addf(&up, "%s:%s", user_name, user_pass);
          curl_easy_setopt(result, CURLOPT_USERPWD, strbuf_detach(&up, NULL));
  }

And then it's easy to swap out the asking for credential_fill() when it
becomes available. But I admit I don't care that much now, as I'll just
end up doing that refactoring later with my credential patches anyway.

> I tried to rewrite the proposed commit log message to describe the real
> issue, and here is what I came up with:

Your description looks accurate to me.

> What is somewhat troubling is that after analyzing the root cause of the
> issue, I am wondering if a more correct fix is to remove the user@ part
> from the URL (in other words, document that a URL with an embedded
> username will ask for password upfront, and tell the users that if they
> have netrc entries or if they are accessing a resource that does not
> require authentication, they should omit the username from the URL).

It's tempting, because the non-netrc case is the common one, and we are
dropping the round-trip avoidance for those people. I'm just not sure
that it's going to be obvious to users that they need to drop the user@
portion from their URL when using netrc. That seems like a bizarre
requirement from the user's POV, even if we do document it.

-Peff

^ permalink raw reply

* Re: [PATCH v3 2/3] http.c: Use timeout suggested by curl instead of fixed 50ms timeout
From: Mika Fischer @ 2011-11-04 17:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, daniel, peff
In-Reply-To: <7vehxndd4q.fsf@alter.siamese.dyndns.org>

On Fri, Nov 4, 2011 at 18:13, Junio C Hamano <gitster@pobox.com> wrote:
> I'm inclined to squash in the following to narrow the scope of
> curl_timeout, though.
>
> diff --git a/http.c b/http.c
> index 5cb0fb6..924be52 100644
> --- a/http.c
> +++ b/http.c
> @@ -636,9 +636,6 @@ void run_active_slot(struct active_request_slot *slot)
>        fd_set excfds;
>        int max_fd;
>        struct timeval select_timeout;
> -#if LIBCURL_VERSION_NUM >= 0x070f04
> -       long curl_timeout;
> -#endif
>        int finished = 0;
>
>        slot->finished = &finished;
> @@ -655,6 +652,7 @@ void run_active_slot(struct active_request_slot *slot)
>
>                if (slot->in_use && !data_received) {
>  #if LIBCURL_VERSION_NUM >= 0x070f04
> +                       long curl_timeout;
>                        curl_multi_timeout(curlm, &curl_timeout);
>                        if (curl_timeout == 0) {
>                                continue;

Ah yes, that's good. I would have done it this way in C++, but I
wasn't sure whether C99 is OK for git.

^ permalink raw reply

* Re: [PATCH v3 2/3] http.c: Use timeout suggested by curl instead of fixed 50ms timeout
From: Jeff King @ 2011-11-04 17:51 UTC (permalink / raw)
  To: Mika Fischer; +Cc: Junio C Hamano, git, daniel
In-Reply-To: <CAOs=hRKxc9SdE_HTnfs+WdnxZEY6yF9MBV_K1FX2=7B7xtj7-w@mail.gmail.com>

On Fri, Nov 04, 2011 at 06:47:44PM +0100, Mika Fischer wrote:

> >                if (slot->in_use && !data_received) {
> >  #if LIBCURL_VERSION_NUM >= 0x070f04
> > +                       long curl_timeout;
> >                        curl_multi_timeout(curlm, &curl_timeout);
> >                        if (curl_timeout == 0) {
> >                                continue;
> 
> Ah yes, that's good. I would have done it this way in C++, but I
> wasn't sure whether C99 is OK for git.

C99 is not OK. But this is not C99, as the conditional opens a new
block.

-Peff

^ permalink raw reply

* Re: [PATCH v3 0/3] Improve use of select in http backend
From: Jeff King @ 2011-11-04 17:53 UTC (permalink / raw)
  To: Mika Fischer; +Cc: git, gitster, daniel
In-Reply-To: <1320416367-28843-1-git-send-email-mika.fischer@zoopnet.de>

On Fri, Nov 04, 2011 at 03:19:24PM +0100, Mika Fischer wrote:

> Mika Fischer (3):
>   http.c: Use curl_multi_fdset to select on curl fds instead of just
>     sleeping
>   http.c: Use timeout suggested by curl instead of fixed 50ms timeout
>   http.c: Rely on select instead of tracking whether data was received

All three patches look good to me. Your 3/3 does most of the cleanup
from the other patch I posted, but we can also do this on top:

-- >8 --
Subject: [PATCH 4/3] http: drop "local" member from request struct

This is a FILE pointer in the case that we are sending our
output to a file. We originally used it to run ftell() to
determine whether data had been written to our file during
our last call to curl. However, as of the last patch, we no
longer care about that flag anymore. All uses of this struct
member are now just book-keeping that can go away.

Signed-off-by: Jeff King <peff@peff.net>
---
 http.c |    6 ------
 http.h |    1 -
 2 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/http.c b/http.c
index 3c6a00b..cfa9b07 100644
--- a/http.c
+++ b/http.c
@@ -535,7 +535,6 @@ struct active_request_slot *get_active_slot(void)
 
 	active_requests++;
 	slot->in_use = 1;
-	slot->local = NULL;
 	slot->results = NULL;
 	slot->finished = NULL;
 	slot->callback_data = NULL;
@@ -829,7 +828,6 @@ static int http_request(const char *url, void *result, int target, int options)
 				headers = curl_slist_append(headers, buf.buf);
 				strbuf_reset(&buf);
 			}
-			slot->local = result;
 		} else
 			curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
 					 fwrite_buffer);
@@ -876,7 +874,6 @@ static int http_request(const char *url, void *result, int target, int options)
 		ret = HTTP_START_FAILED;
 	}
 
-	slot->local = NULL;
 	curl_slist_free_all(headers);
 	strbuf_release(&buf);
 
@@ -1071,7 +1068,6 @@ void release_http_pack_request(struct http_pack_request *preq)
 	if (preq->packfile != NULL) {
 		fclose(preq->packfile);
 		preq->packfile = NULL;
-		preq->slot->local = NULL;
 	}
 	if (preq->range_header != NULL) {
 		curl_slist_free_all(preq->range_header);
@@ -1093,7 +1089,6 @@ int finish_http_pack_request(struct http_pack_request *preq)
 
 	fclose(preq->packfile);
 	preq->packfile = NULL;
-	preq->slot->local = NULL;
 
 	lst = preq->lst;
 	while (*lst != p)
@@ -1162,7 +1157,6 @@ struct http_pack_request *new_http_pack_request(
 	}
 
 	preq->slot = get_active_slot();
-	preq->slot->local = preq->packfile;
 	curl_easy_setopt(preq->slot->curl, CURLOPT_FILE, preq->packfile);
 	curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite);
 	curl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url);
diff --git a/http.h b/http.h
index 71bdf58..ee16069 100644
--- a/http.h
+++ b/http.h
@@ -49,7 +49,6 @@ struct slot_results {
 
 struct active_request_slot {
 	CURL *curl;
-	FILE *local;
 	int in_use;
 	CURLcode curl_result;
 	long http_code;
-- 
1.7.7.2.4.gfd7b9

^ permalink raw reply related

* Re: [PATCH v3 2/3] http.c: Use timeout suggested by curl instead of fixed 50ms timeout
From: Mika Fischer @ 2011-11-04 17:55 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111104175127.GA26118@sigill.intra.peff.net>

On Fri, Nov 4, 2011 at 18:51, Jeff King <peff@peff.net> wrote:
>> Ah yes, that's good. I would have done it this way in C++, but I
>> wasn't sure whether C99 is OK for git.
>
> C99 is not OK. But this is not C99, as the conditional opens a new
> block.

Oh I see, thanks for clarifying!

^ permalink raw reply

* Re: Find the size of git push in pre-receive hook
From: Junio C Hamano @ 2011-11-04 18:01 UTC (permalink / raw)
  To: manigandans; +Cc: git
In-Reply-To: <1320398420796-6962141.post@n2.nabble.com>

manigandans <etc.mani@gmail.com> writes:

> I want to restrict the size of the push on the git remote repository. How
> can I find the size of the push in pre-receive hook?

You don't.

What are you trying to solve?

The thing is, by the time pre-receive-hook is run, the packs have already
been transferred to the destination. Otherwise pre-receive-hook cannot do
its primary task of inspecting the history the push is trying to update
the refs with and allow or deny the ref updates.

Presumably you could run "git rev-list --objects" between the values of
refs before and after the proposed updates, sum up their sizes and reject
the push when the push adds too much data for your liking, and the next gc
will clean things up, but if your goal is to cap the maximum disk quota
including the transient use during the time pre-receive-hook is run, it
would not help.

^ permalink raw reply

* Re: [PATCH v3 2/3] http.c: Use timeout suggested by curl instead of fixed 50ms timeout
From: Junio C Hamano @ 2011-11-04 18:02 UTC (permalink / raw)
  To: Mika Fischer; +Cc: git, daniel, peff
In-Reply-To: <CAOs=hRKxc9SdE_HTnfs+WdnxZEY6yF9MBV_K1FX2=7B7xtj7-w@mail.gmail.com>

Mika Fischer <mika.fischer@zoopnet.de> writes:

>>                if (slot->in_use && !data_received) {
>>  #if LIBCURL_VERSION_NUM >= 0x070f04
>> +                       long curl_timeout;
>>                        curl_multi_timeout(curlm, &curl_timeout);
>>                        if (curl_timeout == 0) {
>>                                continue;
>
> Ah yes, that's good. I would have done it this way in C++, but I
> wasn't sure whether C99 is OK for git.

I do not see anything C99 here.

^ permalink raw reply

* Re: [PATCH] http-push: don't always prompt for password
From: Stefan Naewe @ 2011-11-04 18:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, peff
In-Reply-To: <7vlirvdeb2.fsf@alter.siamese.dyndns.org>

On Fri, Nov 4, 2011 at 5:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Naewe <stefan.naewe@gmail.com> writes:
>
>> http-push prompts for a password when the URL is set as
>> 'https://user@host/repo' even though there is one set
>> in ~/.netrc. Pressing ENTER at the password prompt succeeds
>> then, but is a annoying and makes it almost useless
>> in a shell script, e.g.
>>
>> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
>> ---
>
> Thanks.
>
> With this the only callsite of init_curl_http_auth() becomes the one after
> we get the 401 response, and this caller makes sure that user_name is not
> NULL.
>
> Do we still want "if (user_name)" inside init_curl_http_auth()?

Dunno...
I think what Peff says makes sense.

> I tried to rewrite the proposed commit log message to describe the real
> issue, and here is what I came up with:
>
> [...]

Looks good to me.

> What is somewhat troubling is that after analyzing the root cause of the
> issue, I am wondering if a more correct fix is to remove the user@ part
> from the URL (in other words, document that a URL with an embedded
> username will ask for password upfront, and tell the users that if they
> have netrc entries or if they are accessing a resource that does not
> require authentication, they should omit the username from the URL).

Don't get me wrong, but I really don't care.
I just wanted to have that issue fixed, to get my scripted 'multi
repository pull' working
again.

Regards,
  Stefan
-- 
----------------------------------------------------------------
python -c "print '73746566616e2e6e6165776540676d61696c2e636f6d'.decode('hex')"

^ permalink raw reply

* Re: git-p4: problem with commit 97a21ca50ef8
From: Pete Wyckoff @ 2011-11-04 18:39 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: Michael Wookey, Git Mailing List, Luke Diamand
In-Reply-To: <CAOpHH-W1JO9PLsyp2hQxfr6eyKRr+=pMkaDikV5NcFwF98Miow@mail.gmail.com>

vitor.hda@gmail.com wrote on Thu, 03 Nov 2011 11:04 +0000:
> Hi Michael,
> 
> On Wed, Nov 2, 2011 at 10:42 PM, Michael Wookey <michaelwookey@gmail.com> wrote:
> > I tried your suggested version of git-p4 (at rev 630fb678c46c) and
> > unfortunately, the perforce repository fails to import. Firstly, there
> > was a problem with importing UTF-16 encoded files, secondly the
> > "apple" filetype files are still skipped.
> 
> I had no intention of directing you to try that version. Sorry for
> misleading you on this.
> 
> I just found it interesting that P4's KB contains an article that
> directs users to another version which isn't this one.

We're making contact offline with perforce folk and other git-p4
folk.  I'll update with the results.

I've not run the kb version, but for git's git-p4, Utf-16 was
fixed only recently (55aa571).  I'm going to revert the apple
filetype issue (97a21ca) that Michael found soon, hopefully
before v1.7.8 goes out.

		-- Pete

^ permalink raw reply

* Re: [PATCH 0/4] fsck improvements
From: Junio C Hamano @ 2011-11-04 18:43 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1320421670-518-1-git-send-email-pclouds@gmail.com>

I think patches 1, 3 and 4 all are good ideas from a cursory look.

I am not sure what purpose patch 2 serves, though. When we find a checksum
mismatch for an object in a packstream due to a single-bit error, we would
still be able to salvage other objects in other parts of the pack as long
as we have a good .idx file, and in such a case, wouldn't it be better if
we attempted to find as many corrupt objects that we know we cannot
recover as possible and tell the user about them, so that they can be
skipped during the recovery process?

^ permalink raw reply

* Re: [PATCH] http-push: don't always prompt for password
From: Junio C Hamano @ 2011-11-04 19:06 UTC (permalink / raw)
  To: Jeff King; +Cc: Stefan Naewe, git
In-Reply-To: <20111104174303.GA22568@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Since we now only call init_curl_http_auth when we know we need auth, I
> think it would make more sense to just move the user_name asking there,
> too, like:
>
>   static void init_curl_http_auth(CURL *result)
>   {
>           struct strbuf up = STRBUF_INIT;
>
>           if (!user_name)
>                   user_name = xstrdup(git_getpass_with_description("Username", description);
>           if (!user_pass)
>                   user_pass = xstrdup(git_getpass_with_description("Password", description);
>
>           strbuf_addf(&up, "%s:%s", user_name, user_pass);
>           curl_easy_setopt(result, CURLOPT_USERPWD, strbuf_detach(&up, NULL));
>   }
>
> And then it's easy to swap out the asking for credential_fill() when it
> becomes available. But I admit I don't care that much now, as I'll just
> end up doing that refactoring later with my credential patches anyway.

Yeah, let's not over-churn this part for now as we know it will change
anyway.

Thanks both!

^ permalink raw reply

* How do I get a squashed diff for review
From: Alexander Usov @ 2011-11-04 19:15 UTC (permalink / raw)
  To: git

Hi,

I'm wondering if there is an easy way to get a squashed diff of the
changes done on the feature branch for review.
In the simple cases (where feature branch is linear) there is an
absolutely fantastic way to get a patch for review:
git diff master...feature

However if the feature branch happened to be long-lived and had
mainline merged into it it's not going to work -- the
resulting diff would contain changes from the merge. The way we are
doing things now is to merge master into it
once more and then diff, however this is somewhat cumbersome. Is there
easier way to do it?

And while we are on the topic -- is there a tool for git similar to "bzr qdiff"?
It's a simple graphical diff viewer with 2 nice features -- it shows
complete diff (of multiple files) in a single window and
has a checkbox to switch between diff-only & full-text modes.
I have seen difftool, but it seems to work on per-file basis, and
something like "vi <(git diff ...)" lacks the easy way to
switch into full-text mode.

--
Best regards,
  Alexander.

^ permalink raw reply

* qgrit: graphical git rebase -i helper
From: Peter Oberndorfer @ 2011-11-04 19:38 UTC (permalink / raw)
  To: git; +Cc: Phil Hord, Junio C Hamano

Hi,

i wrote a small tool to support users during git rebase -i.
by presenting commits in a graphical interface.
(usable with git 1.7.8.rc0 and up)

Lacking a better idea i named it "qgrit" :-)
(Qt git rebase --interactive tool)

Notable features:
* Reorder commits by drag drop and up/down buttons
* Select actions from a combo box
* Undo reordering function
* Shows full commit message, not only title line
* Works on mingw (auto detects git path)
* Easy setup as rebase -i editor: run qgrit, click "Install qgrit" button
 (this also detects too old git versions)

For more information, screenshots and source code see:
https://github.com/qgrit/qgrit/wiki

Ideas for improvements, bugreports welcome!

Have fun,
Greetings Peter

^ permalink raw reply

* Re: [PATCH 4/4] fsck: print progress
From: Jeff King @ 2011-11-04 20:14 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1320421670-518-5-git-send-email-pclouds@gmail.com>

On Fri, Nov 04, 2011 at 10:47:50PM +0700, Nguyen Thai Ngoc Duy wrote:

>  static int traverse_reachable(void)
>  {
> +	struct progress *progress = NULL;
> +	unsigned int nr = 0;
>  	int result = 0;
> +	if (show_progress)
> +		progress = start_progress_delay("Checking connectivity", 0, 0, 2);
>  	while (pending.nr) {
>  		struct object_array_entry *entry;
>  		struct object *obj;
> @@ -146,7 +152,9 @@ static int traverse_reachable(void)
>  		entry = pending.objects + --pending.nr;
>  		obj = entry->item;
>  		result |= traverse_one_object(obj);
> +		display_progress(progress, ++nr);
>  	}
> +	stop_progress(&progress);
>  	return !!result;
>  }

Thanks, this is a good place to put a progress meter, too. If you're
feeling like pushing this topic further, "git prune" might be a good
place for a progress meter, too. It does a similar traversal[1] for
reachability, and makes "git gc" appear to hang at the end (we have nice
progress meters for packing, but it takes something like 25 seconds to
run "git prune" at the end, during which we are silent).

-Peff

[1] I wonder why fsck doesn't use mark_reachable from reachable.c.

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Junio C Hamano @ 2011-11-04 20:16 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Shawn Pearce, git, James Bottomley, Jeff Garzik, Andrew Morton,
	linux-ide, LKML
In-Reply-To: <CA+55aFzstE-+NzfSAWMEokB7-rYsZOcZe9Ez-LxPNOKnciJ3UQ@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> So I suspect we should just apply this patch, but I didn't check
> exacty what the failed tests are - except for the first one, that just
> compares against a canned response (and the canned response should
> just be changed).

After applying your patch and running

$ perl -pi -e 'if (/\ttag /) {
    s/754b754407bf032e9a2f9d5a9ad05ca79a6b228f/6c9dec2b923228c9ff994c6cfe4ae16c12408dc5/;
    s/0567da4d5edd2ff4bb292a465ba9e64dcad9536b/c61a82b60967180544e3c19f819ddbd0c9f89899/;
    s/6134ee8f857693b96ff1cc98d3e2fd62b199e5a8/525b7fb068d59950d185a8779dc957c77eed73ba/;
}' t/t5515/fetch.*

to unpeel the three tags used in the test 5515 that used to expect
FETCH_HEAD to have peeled tags to expect tag objects themselves instead,
all tests passes.

> although I suspect it was just a fairly mindless case of "make it a
> commit, because the merge needs the commit" - never mind that the
> merge would peel it anyway.

I am 100% sure the machinery that comes up with the tree (or half-merged
conflicted state) does not mind being fed tags. After all they need to
peel them down to commits for common ancestor discovery, and they need to
further peel them down to trees to perform three-way merges.

However we would need to audit so that we do not accidentally record the
tag object names in the "parent" headers in the merge commits, which is
what I'll be doing next.

^ permalink raw reply

* Re: aliases causing “Permission denied” error in git v1.7
From: Jeff King @ 2011-11-04 20:34 UTC (permalink / raw)
  To: Алексей Данченков
  Cc: git
In-Reply-To: <CALUFZ3n9cpHw3r3rcGriDqvJ+UM83L3Q19m=0YeAy51LBJzosA@mail.gmail.com>

On Fri, Nov 04, 2011 at 12:09:07PM +0400, Алексей Данченков wrote:

> $ git co -b newbranch
> $ git co oldbranch
> 
> results in "fatal: cannot exec 'git-co': Permission denied" error.

Git will try commands in your PATH before aliases. So you are running
into a permissions problem with a "git-co" in your PATH.

Or more likely, there is an inaccessible directory of your PATH (and we
get the same error code from execve whether it is the directory or the
file itself which lacks permission). Try "git foobar" and I suspect you
will see the same thing (it doesn't matter that you have no foobar
alias; we never get past the "try to exec it" stage).

If you are on Linux, try:

  strace -f -e execve git foobar

to see the the problematic entry that is returning EACCES. Or look
through your $PATH manually.

-Peff

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Junio C Hamano @ 2011-11-04 21:12 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Ted Ts'o, Shawn Pearce, git, James Bottomley, Jeff Garzik,
	Andrew Morton, linux-ide, LKML
In-Reply-To: <7v62j1gitn.fsf@alter.siamese.dyndns.org>

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

> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>>   [torvalds@i5 linux]$ git fetch
>> git://github.com/rustyrussell/linux.git
>> rusty@rustcorp.com.au-v3.1-8068-g5087a50
>>   fatal: Couldn't find remote ref rusty@rustcorp.com.au-v3.1-8068-g5087a50
>>
>> oops. Ok, so his tag naming is *really* akward. Whatever.
>
> It is not "Whatever".
>
>  $ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v3.0
>  fatal: Couldn't find remote ref v3.0
>
> I do not think we ever DWIMmed fetch refspecs to prefix refs/tags/, so it
> is not the naming but fetching tags without saying "git fetch tag v3.0"
> (which IIRC was your invention long time ago). 

If we really wanted to go this route, the attached single-liner should be
sufficient for the DWIMmery.

Note that the DWIMmery rules for "git fetch" and local "git rev-parse" are
still different even after this patch.

"git log frotz" can DWIM to "refs/remotes/frotz/HEAD", but in the remote
access context, "git fetch frotz" to fetch what the other side happened to
have fetched from what it calls 'frotz' (which may not have any relation
to what we consider is 'frotz') the last time would not make much sense,
so the fetch rules table does not include "refs/remotes/%.*s/HEAD".

When the user really wants to, "git fetch $there remotes/frotz/HEAD" would
let her do so anyway, so this is not about safety or security; it merely
is about confusion avoidance and discouraging meaningless usage.

Specifically, it is _not_ about ambiguity avoidance. A name that would
become ambiguous if we use the same rules table for both fetch and local
rev-parse would be ambiguous locally at the remote side.

If we really wanted to, we could 

	#define ref_fetch_rules ref_rev_parse_rules
 
in cache.h and remove the array's declaration from cache.h and its
definition from refs.c to really unify the two, but I haven't thought
things through.

-- >8 --
Subject: [PATCH] fetch: allow "git fetch $there v1.0" to fetch a tag

You can already do so with "git fetch $there tags/v1.0" but if it is not
ambiguous there is no reason to force users to type more.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 refs.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/refs.c b/refs.c
index e69ba26..670a7b3 100644
--- a/refs.c
+++ b/refs.c
@@ -1001,6 +1001,7 @@ const char *ref_rev_parse_rules[] = {
 const char *ref_fetch_rules[] = {
 	"%.*s",
 	"refs/%.*s",
+	"refs/tags/%.*s",
 	"refs/heads/%.*s",
 	NULL
 };
-- 
1.7.8.rc0.108.g71b5ec

^ permalink raw reply related

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Junio C Hamano @ 2011-11-04 21:22 UTC (permalink / raw)
  To: git
  Cc: Linus Torvalds, Shawn Pearce, James Bottomley, Jeff Garzik,
	Andrew Morton, linux-ide, LKML
In-Reply-To: <7vlirvbq47.fsf@alter.siamese.dyndns.org>

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

> However we would need to audit so that we do not accidentally record the
> tag object names in the "parent" headers in the merge commits, which is
> what I'll be doing next.

Just reporting my findings.

builtin/merge.c was updated to use want_commit() that uses peel_to_type()
to commit to make sure we do not get fed anything funky, and also uses
struct commit_list to pass around list of parents to be recorded, so we
should be OK in this department.

^ permalink raw reply

* Re: [PATCH] log: allow to specify diff pathspec in addition to prune pathspec
From: Junio C Hamano @ 2011-11-04 22:04 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1320322556-32675-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Pathspec in "git log -p <pathspec>" is used for both commit pruning
> and diff generation. If --full-diff is given, then diff pathspec is
> reset to generate complete diff.
>
> This patch gives more control to diff generation.  The first pathspec in
> "git log -p -- <pathspec> -- <pathspec>" is used as commit pruning
> as usual. The second one is used for diff generation. So --full-diff
> now is essentially "git log -p -- <pathspec> --".

I agree that giving more control to diff generation is a good idea, and
this certainly is better than the previous round that nobody reviewed
before you rerolled this round.

But I have doubts about declaring "--full-diff is equivalent to giving the
'output' pathspec that is empty".

Have you thought about the interaction between this and -M/-C options?

^ permalink raw reply

* Re: [PATCH] log: allow to specify diff pathspec in addition to prune pathspec
From: Junio C Hamano @ 2011-11-04 22:07 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1320322556-32675-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This form requires specifying "--" twice. If a file name happens to be
> "--", it may be misunderstood as the second "--" marker. This is an
> unfortunate consequence for this syntax. Users can still use "./--" or
> similar to workaround this.

This is not a new "problem" (and it is probably not a problem to begin
with).  If you had "--" in the working tree and you wanted to treat it as
a path, you said either one of these:

    $ git log HEAD ./--
    $ git log HEAD -- --

The latter is what your patch breaks, I suspect.

Also it forces existing scripts and programs that knew "everything in this
array is a pathspec" and added "--" before adding the contents of the
array to form a command line to drive "git log" family of commands to be
updated.

^ permalink raw reply

* share object storage for multiple clones of different repositories
From: Gelonida N @ 2011-11-04 23:10 UTC (permalink / raw)
  To: git

Hi,

(Thunderbird frozewhile sending my previous message so here a resend)


I wondered whether it is possible, that all of my git repository clones
share the same object storage whether they are cloned from they same
remote repository or not.

In my case this might save a lot of diskspace and accelerate cloning
as some huge files are part of several repositories' history and as I'd
like to clone the same repository multiple times in order to have
parallel working directories for parallel tests on different versions /
branches / tags

Further this might reduce clone times.



My goal would be to:
- reduce disk space
- reduce clone time, as objects would be taken from the existing shared
  object storage

If possible it would be great if also all new created shared objects
would end up in the new object storage.


If git doesn't support this natively, then would following approach work
for reducing the disk space (clone time would not be reduced though)



SHARED_STORAGE=$HOME/shared_storage
mkdir $SHARED_STORAGE

git clone remotehost1:repo1
cd repo1
rsync -av .git/objects $SHARED_REPO
rm -rf .git/objects
ln -s $SHARED_REPO/objects .git/


git clone remotehost2:repo2
cd repo2
rsync -av .git/objects $SHARED_REPO
rm -rf .git/objects
ln -s $SHARED_REPO/objects .git/

Thanks for any feedback or other suggestions.

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Linus Torvalds @ 2011-11-04 23:10 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Shawn Pearce, James Bottomley, Jeff Garzik, Andrew Morton,
	linux-ide, LKML
In-Reply-To: <7v39e3bn1n.fsf@alter.siamese.dyndns.org>

On Fri, Nov 4, 2011 at 2:22 PM, Junio C Hamano <junio@pobox.com> wrote:
>
> builtin/merge.c was updated to use want_commit() that uses peel_to_type()
> to commit to make sure we do not get fed anything funky, and also uses
> struct commit_list to pass around list of parents to be recorded, so we
> should be OK in this department.

I'm pretty sure people have already done "git merge v3.1" kind of
things using local tags (where no peeling of FETCH_HEAD has been
done). See

    git log --merges --grep 'Merge.*v[23]\.[0-9]'

for a ton of examples of this (and there's something odd going on: we
have "Merge commit .." and "Merge tag ..", and I suspect the latter is
people editing it to be correct by hand, but I dunno).

So this has always worked, methinks.

However - exactly beause git apparently makes it do that "Merge commit
" message, I suspect we've peeled things too early and too much. We've
peeled it so early that once again something thinks it's a commit, not
a tag.

So if anything, I suspect "git merge" not only peels, but peels too
much (or at the wrong point). We should probably peel as late as
possible.

But it's a small detail.

                  Linus

^ 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