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 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 the

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

2014-03-19 Thread Junio C Hamano
Jeff King 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 layer

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"

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 wrote: > Michael Haggerty 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 troubl

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

2014-03-06 Thread Philip Oakley
From: "Michael Haggerty" On 03/07/2014 12:01 AM, Philip Oakley wrote: From: "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 fo

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

2014-03-06 Thread Junio C Hamano
Michael Haggerty 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. Putting this fu

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" >> 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

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

2014-03-06 Thread Philip Oakley
From: "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

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 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 furt

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

2014-03-06 Thread Junio C Hamano
Jeff King 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 the repl

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 d

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 t

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 Christian Couder
On Thu, Mar 6, 2014 at 9:42 AM, Michael Haggerty wrote: > On 03/05/2014 08:18 PM, Junio C Hamano wrote: >> Jeff King 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 lit

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 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 wit

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

2014-03-05 Thread Junio C Hamano
Jeff King 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 is

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 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

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

2014-03-05 Thread Junio C Hamano
Jeff King 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 discour

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 messa

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

2014-03-05 Thread Junio C Hamano
Jeff King 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 is coming

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 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

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

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 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 wrote: >> > diff --git a/commit.c b/commit.c >> > index 6bf4fe0..886dbfe 100644 >> > --- a/commit.c >> > +++ b/commit.c >> > @@ -114,6

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. > > "Doi

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 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 cha

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 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 **commit_graft; >

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

2014-03-04 Thread Junio C Hamano
Jeff King 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 objects t