Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-20 Thread Jeff King
On Wed, Mar 19, 2014 at 03:39:42PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Fri, Mar 07, 2014 at 08:08:37AM +0100, Christian Couder wrote: Be it graft or replace, I do not think we want to invite people to use these mechansims too lightly to locally rewrite

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-19 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Fri, Mar 07, 2014 at 08:08:37AM +0100, Christian Couder wrote: Be it graft or replace, I do not think we want to invite people to use these mechansims too lightly to locally rewrite their history willy-nilly without fixing their mistakes at the object

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-07 Thread Jeff King
On Fri, Mar 07, 2014 at 08:08:37AM +0100, Christian Couder wrote: Be it graft or replace, I do not think we want to invite people to use these mechansims too lightly to locally rewrite their history willy-nilly without fixing their mistakes at the object layer with commit --amend, rebase,

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Michael Haggerty
On 03/05/2014 08:18 PM, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Wed, Mar 05, 2014 at 10:49:24AM -0800, Junio C Hamano wrote: ... the plan, at least in my mind, has always been exactly that: grafts were a nice little attempt but is broken---if you really wanted to muck with

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Christian Couder
On Thu, Mar 6, 2014 at 9:42 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 03/05/2014 08:18 PM, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Wed, Mar 05, 2014 at 10:49:24AM -0800, Junio C Hamano wrote: ... the plan, at least in my mind, has always been exactly that: grafts

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Jeff King
On Thu, Mar 06, 2014 at 09:42:46AM +0100, Michael Haggerty wrote: Replace objects are better than grafts in *almost* every dimension. The exception is that it is dead simple to create grafts, whereas I always have to break open the man pages to remember how to create a replace object that

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Michael Haggerty
On 03/06/2014 04:56 PM, Jeff King wrote: On Thu, Mar 06, 2014 at 09:42:46AM +0100, Michael Haggerty wrote: Replace objects are better than grafts in *almost* every dimension. The exception is that it is dead simple to create grafts, whereas I always have to break open the man pages to

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Jeff King
On Thu, Mar 06, 2014 at 05:41:27PM +0100, Michael Haggerty wrote: We can wrap that in git replace --convert-grafts, but I do not think grafts are so common that there would be a big demand for it. It's probably easier to wrap it than to explain to Windows users what they have to do. How

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Junio C Hamano
Jeff King p...@peff.net writes: I also noticed that the diff engine does not play well with replacements of blobs. When we are diffing the trees, we see that the sha1 for path foo is the same on either side, and do not look further, even though feeding those sha1s to builtin_diff would fetch

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Jeff King
On Thu, Mar 06, 2014 at 11:00:08AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: I also noticed that the diff engine does not play well with replacements of blobs. When we are diffing the trees, we see that the sha1 for path foo is the same on either side, and do not look

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Philip Oakley
From: Jeff King p...@peff.net On Thu, Mar 06, 2014 at 05:41:27PM +0100, Michael Haggerty wrote: We can wrap that in git replace --convert-grafts, but I do not think grafts are so common that there would be a big demand for it. It's probably easier to wrap it than to explain to Windows

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Michael Haggerty
On 03/07/2014 12:01 AM, Philip Oakley wrote: From: Jeff King p...@peff.net On Thu, Mar 06, 2014 at 05:41:27PM +0100, Michael Haggerty wrote: We can wrap that in git replace --convert-grafts, but I do not think grafts are so common that there would be a big demand for it. It's probably

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes: I didn't mean to insult all Windows users in general. I was only referring to the fact that since the default Windows command line is not a POSIX shell, even an experienced Windows user might have trouble figuring out how to execute a shell loop.

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Philip Oakley
From: Michael Haggerty mhag...@alum.mit.edu On 03/07/2014 12:01 AM, Philip Oakley wrote: From: Jeff King p...@peff.net On Thu, Mar 06, 2014 at 05:41:27PM +0100, Michael Haggerty wrote: We can wrap that in git replace --convert-grafts, but I do not think grafts are so common that there

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-06 Thread Christian Couder
On Fri, Mar 7, 2014 at 12:39 AM, Junio C Hamano gits...@pobox.com wrote: Michael Haggerty mhag...@alum.mit.edu writes: I didn't mean to insult all Windows users in general. I was only referring to the fact that since the default Windows command line is not a POSIX shell, even an experienced

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-05 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Tue, Mar 04, 2014 at 12:52:18PM -0800, Junio C Hamano wrote: I do not recall any past discussion on this topic, and searching the archive only shows people echoing what I said above. Is this something we've promised to work in the past? The history lesson

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-05 Thread Jeff King
On Wed, Mar 05, 2014 at 10:49:24AM -0800, Junio C Hamano wrote: Perhaps the right response is grafts are broken, use git-replace instead. But then should we think about deprecating grafts? I am sort of surprised to hear that question, actually ;-) I didn't say that in the message you

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-05 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Wed, Mar 05, 2014 at 10:49:24AM -0800, Junio C Hamano wrote: ... the plan, at least in my mind, has always been exactly that: grafts were a nice little attempt but is broken---if you really wanted to muck with the history without rewriting (which is still

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-05 Thread Jeff King
On Wed, Mar 05, 2014 at 11:18:17AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Wed, Mar 05, 2014 at 10:49:24AM -0800, Junio C Hamano wrote: ... the plan, at least in my mind, has always been exactly that: grafts were a nice little attempt but is broken---if you

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-05 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Wed, Mar 05, 2014 at 11:18:17AM -0800, Junio C Hamano wrote: Given that we discourage grafts strongly and replace less so (but still discourage it), telling the users that biting the bullet and rewriting the history is _the_ permanent solution, I think it

[PATCH] disable grafts during fetch/push/bundle

2014-03-04 Thread Jeff King
When we are creating a pack to send to a remote, we should make sure that we are not respecting grafts or replace refs. Otherwise, we may end up sending a broken pack to the other side that does not contain all objects (either omitting them entirely, or using objects that the other side does not

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-04 Thread Junio C Hamano
Jeff King p...@peff.net writes: When we are creating a pack to send to a remote, we should make sure that we are not respecting grafts or replace refs. Otherwise, we may end up sending a broken pack to the other side that does not contain all objects (either omitting them entirely, or using

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-04 Thread Eric Sunshine
On Tue, Mar 4, 2014 at 12:48 PM, Jeff King p...@peff.net wrote: diff --git a/commit.c b/commit.c index 6bf4fe0..886dbfe 100644 --- a/commit.c +++ b/commit.c @@ -114,6 +114,11 @@ static unsigned long parse_commit_date(const char *buf, const char *tail) static struct commit_graft

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-04 Thread Jeff King
On Tue, Mar 04, 2014 at 06:36:07PM -0500, Eric Sunshine wrote: On Tue, Mar 4, 2014 at 12:48 PM, Jeff King p...@peff.net wrote: diff --git a/commit.c b/commit.c index 6bf4fe0..886dbfe 100644 --- a/commit.c +++ b/commit.c @@ -114,6 +114,11 @@ static unsigned long parse_commit_date(const

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-04 Thread Jeff King
On Tue, Mar 04, 2014 at 12:52:18PM -0800, Junio C Hamano wrote: We already make an attempt to do the right thing in several places by turning off read_replace_refs. However, we missed at least one case (during bundle creation), and we do nothing anywhere to handle grafts. Doing nothing

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-04 Thread Eric Sunshine
On Tue, Mar 4, 2014 at 7:37 PM, Jeff King p...@peff.net wrote: On Tue, Mar 04, 2014 at 06:36:07PM -0500, Eric Sunshine wrote: On Tue, Mar 4, 2014 at 12:48 PM, Jeff King p...@peff.net wrote: diff --git a/commit.c b/commit.c index 6bf4fe0..886dbfe 100644 --- a/commit.c +++ b/commit.c @@

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-04 Thread Jeff King
On Tue, Mar 04, 2014 at 08:00:44PM -0500, Eric Sunshine wrote: +int commit_grafts_loaded(void) +{ + return !!commit_graft_nr; +} Did you mean !!commit_graft ? Shouldn't they produce the same results? Yes they should, but the use of !! seemed to imply that you wanted

Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-04 Thread Eric Sunshine
On Tue, Mar 4, 2014 at 8:05 PM, Jeff King p...@peff.net wrote: On Tue, Mar 04, 2014 at 08:00:44PM -0500, Eric Sunshine wrote: +int commit_grafts_loaded(void) +{ + return !!commit_graft_nr; +} Did you mean !!commit_graft ? Shouldn't they produce the same results? Yes