git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [BUG] Cannot push some grafted branches
@ 2012-12-11 14:39 Yann Dirson
  2012-12-11 18:15 ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Yann Dirson @ 2012-12-11 14:39 UTC (permalink / raw)
  To: git list

There seems to be some bad interactions between git-push and grafts.
The problem seems to occur when a commit that exists in the remote
repo is subject to a graft in the local repo, and we try to push one
of the fake parents.

The problem was first seen on 1.7.12.3 in a private repo, and I could
reproduce it using 1.8.1.rc0, as shown below.  1.7.10.4 seems even
more affected, with something looking like a memory corruption issue.

Here is the test:

$ git clone git.git git-test
Cloning into 'git-test'...
done.
Checking out files: 100% (2518/2518), done.
$ cd git-test/
git-test$ git co maint
Branch maint set up to track remote branch maint from origin.
Switched to a new branch 'maint'
git-test$ echo >> README 
git-test$ git commit -a -m "test"
[maint 0708279] test
 1 file changed, 1 insertion(+)
git-test$ echo $(git rev-parse origin/master; git rev-parse origin/master^; git rev-parse HEAD) > .git/info/grafts

git-test$ git version
git version 1.8.1.rc0
git-test$ git push origin maint
Total 0 (delta 0), reused 0 (delta 0)
fatal: bad object 0708279e168b52003234dd23601796b3b12e278b
fatal: bad object 0708279e168b52003234dd23601796b3b12e278b
To /home/localadm/softs/git.git
 ! [remote rejected] maint -> maint (missing necessary objects)
error: failed to push some refs to '/home/localadm/softs/git.git'


$ git version
git version 1.7.10.4

git-test$ git push origin maint
Total 0 (delta 0), reused 0 (delta 0)
fatal: bad object 0708279e168b52003234dd23601796b3b12e278b
fatal: bad object 0708279e168b52003234dd23601796b3b12e278b
Auto packing the repository for optimum performance.
fatal: protocol error: bad line length character: Remo
error: error in sideband demultiplexer
error: ૏        >S��ŋJ�jB�;�x'��R died of signal 13
To /home/localadm/softs/git.git
 ! [remote rejected] maint -> maint (missing necessary objects)
error: failed to push some refs to '/home/localadm/softs/git.git'

-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-11 14:39 [BUG] Cannot push some grafted branches Yann Dirson
@ 2012-12-11 18:15 ` Junio C Hamano
  2012-12-12  8:44   ` Yann Dirson
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2012-12-11 18:15 UTC (permalink / raw)
  To: Yann Dirson; +Cc: git list

Yann Dirson <dirson@bertin.fr> writes:

> There seems to be some bad interactions between git-push and grafts.
> The problem seems to occur when a commit that exists in the remote
> repo is subject to a graft in the local repo, and we try to push one
> of the fake parents.

History tweaking by grafts is only visible inside your local
repository and objects are not rewritten, and grafts are not
transferred across repositories.  They were invented to be used as a
stop-gap measure until you filter-branch the history before
publishing (or if you do not publish, then you can keep using your
local grafts).

Isn't this well known?  Perhaps we would need to document it better.

What you can do is to use "replace" instead and publish the replace
refs, I think.  Object transfer will then follow the true parenthood
connectivity and people who choose to use the same replacement as
you do can fetch the replace ref from you (this will grab objects
necessary to complete the alternative history) and install it.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-11 18:15 ` Junio C Hamano
@ 2012-12-12  8:44   ` Yann Dirson
  2012-12-12 10:54     ` Yann Dirson
  2012-12-12 19:57     ` Junio C Hamano
  0 siblings, 2 replies; 29+ messages in thread
From: Yann Dirson @ 2012-12-12  8:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git list

On Tue, 11 Dec 2012 10:15:23 -0800
Junio C Hamano <gitster@pobox.com> wrote:

> Yann Dirson <dirson@bertin.fr> writes:
> 
> > There seems to be some bad interactions between git-push and grafts.
> > The problem seems to occur when a commit that exists in the remote
> > repo is subject to a graft in the local repo, and we try to push one
> > of the fake parents.
> 
> History tweaking by grafts is only visible inside your local
> repository and objects are not rewritten, and grafts are not
> transferred across repositories.  They were invented to be used as a
> stop-gap measure until you filter-branch the history before
> publishing (or if you do not publish, then you can keep using your
> local grafts).
> 
> Isn't this well known?  Perhaps we would need to document it better.

I am well aware of that, and did not intend to push any grafted commit.
I am attempting to push a well-formed commit, which happens to be used as
a grafted commit's fake parent, and my interpretation is that git reacts
as if it was considering that the remote already had that commit, possibly
because it would not ignore grafts when deciding which commits are already
known to the remote.

> What you can do is to use "replace" instead and publish the replace
> refs, I think.  Object transfer will then follow the true parenthood
> connectivity and people who choose to use the same replacement as
> you do can fetch the replace ref from you (this will grab objects
> necessary to complete the alternative history) and install it.

I am only using grafts as a temporary and lightweight drafting area,
before setting the results in stone - although in my case it will be
with filter-branch rather than replace, but the idea is the same.  I just
got bitten when attempting to push a valid branch while the grafts were in
effect, when in fact they should have had no influence at all.

In fact, I even looked for a way to specify an alternate (or supplementary)
grafts file for this drafting work, so only well-controlled git invocations
would see them, whereas the others would just ignore them, and could not find
any - nor could I identify an existing way of disabling the use of grafts by
other means than moving it out of the way.  In this respect, they seem to be
lacking a few features, when compared to "replace" refs, but they have different
uses, and just using the latter as a drafting area is just not adequate.

I thought about adding support for a GIT_GRAFTS_FILE envvar, which would
default to $GITDIR/info/grafts, or maybe with a more general addition of a
GIT_EXTRA_GRAFT_FILES envvar, but I'm not sure the latter would be that useful.

-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-12  8:44   ` Yann Dirson
@ 2012-12-12 10:54     ` Yann Dirson
  2012-12-12 19:57     ` Junio C Hamano
  1 sibling, 0 replies; 29+ messages in thread
From: Yann Dirson @ 2012-12-12 10:54 UTC (permalink / raw)
  To: Yann Dirson; +Cc: Junio C Hamano, git list

On Wed, 12 Dec 2012 09:44:32 +0100 Yann Dirson <dirson@bertin.fr> wrote:
> In fact, I even looked for a way to specify an alternate (or supplementary)
> grafts file for this drafting work, so only well-controlled git invocations
> would see them, whereas the others would just ignore them, and could not find
> any - nor could I identify an existing way of disabling the use of grafts by
> other means than moving it out of the way.  In this respect, they seem to be
> lacking a few features, when compared to "replace" refs, but they have different
> uses, and just using the latter as a drafting area is just not adequate.
> 
> I thought about adding support for a GIT_GRAFTS_FILE envvar, which would
> default to $GITDIR/info/grafts, or maybe with a more general addition of a
> GIT_EXTRA_GRAFT_FILES envvar, but I'm not sure the latter would be that useful.

My bad on this point: there *is* a GIT_GRAFT_FILE envvar, it is just undocumented.
In fact it is not the only one:

git.git$ for v in $(git grep define.*_ENVIRONMENT master -- cache.h | cut -d'"' -f2|grep ^GIT_); do git grep -q $v master -- Documentation || echo "missing $v"; done
missing GIT_GRAFT_FILE
missing GIT_CONFIG_PARAMETERS


-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-12  8:44   ` Yann Dirson
  2012-12-12 10:54     ` Yann Dirson
@ 2012-12-12 19:57     ` Junio C Hamano
  2012-12-17  7:52       ` Yann Dirson
  2012-12-17  8:43       ` Thomas Rast
  1 sibling, 2 replies; 29+ messages in thread
From: Junio C Hamano @ 2012-12-12 19:57 UTC (permalink / raw)
  To: Yann Dirson; +Cc: git list

Yann Dirson <dirson@bertin.fr> writes:

> ....  In this respect, they seem to be
> lacking a few features, when compared to "replace" refs, but they have different
> uses, ...

Not reallyl; grafts were old hack whose use is still supported with
its original limitations; replace is meant to replace all uses of
grafts while removing grafts' largest warts.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-12 19:57     ` Junio C Hamano
@ 2012-12-17  7:52       ` Yann Dirson
  2012-12-17  8:56         ` Junio C Hamano
  2012-12-17  8:43       ` Thomas Rast
  1 sibling, 1 reply; 29+ messages in thread
From: Yann Dirson @ 2012-12-17  7:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git list

On Wed, 12 Dec 2012 11:57:47 -0800
Junio C Hamano <gitster@pobox.com> wrote:

> Yann Dirson <dirson@bertin.fr> writes:
> 
> > ....  In this respect, they seem to be
> > lacking a few features, when compared to "replace" refs, but they have different
> > uses, ...
> 
> Not reallyl; grafts were old hack whose use is still supported with
> its original limitations; replace is meant to replace all uses of
> grafts while removing grafts' largest warts.

OK, I'll take this into account.

But this situation should probably be make more clear in the docs.  Currently,
gitrepository-layout.txt describes refs/replace/ (and shallow) by reference to grafts,
and those are not marked as discouraged-use or anything.

And we may still want the bug fixed, or would we just list it as a known bug ?
At least it does not seem to occur with "replace" refs:

git-test$ rm .git/info/grafts 
git-test$ echo "fake merge" | git commit-tree master^{tree} -p master^ -p maint
b821b2aa00973a47936d7cd25c9a5978b1c839c6
git-test$ git replace master b821b2aa00973a47936d7cd25c9a5978b1c839c6
git-test$ git push origin maint
...
   50b03b0..79211fe  maint -> maint

-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-12 19:57     ` Junio C Hamano
  2012-12-17  7:52       ` Yann Dirson
@ 2012-12-17  8:43       ` Thomas Rast
  2012-12-17 10:40         ` Yann Dirson
  1 sibling, 1 reply; 29+ messages in thread
From: Thomas Rast @ 2012-12-17  8:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Yann Dirson, git list

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

> Yann Dirson <dirson@bertin.fr> writes:
>
>> ....  In this respect, they seem to be
>> lacking a few features, when compared to "replace" refs, but they have different
>> uses, ...
>
> Not reallyl; grafts were old hack whose use is still supported with
> its original limitations; replace is meant to replace all uses of
> grafts while removing grafts' largest warts.

I suppose there's the additional issue that grafts are much easier to
use than replacements if you really only want to replace some parent
lists.  With replace you need to handcraft the replacement commits, and
git-replace(1) unhelpfully does not say this, much less gives an example
how to do it.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-17  7:52       ` Yann Dirson
@ 2012-12-17  8:56         ` Junio C Hamano
  2012-12-17 10:30           ` Yann Dirson
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2012-12-17  8:56 UTC (permalink / raw)
  To: Yann Dirson; +Cc: git list

Yann Dirson <dirson@bertin.fr> writes:

> And we may still want the bug fixed, or would we just list it as a known bug ?
> At least it does not seem to occur with "replace" refs:

The "replace" was designed to "fix" known limitation of grafts,
which is _inherent_ to it; the graft information was designed _not_
to be shared across repositories.  The fix was done by by using a
different mechanism to allow propagating the information across
repositories.

So there is nothing further to fix, except that there is a documentation
bug you can fix if you didn't find it documented.

Thanks.

>
> git-test$ rm .git/info/grafts 
> git-test$ echo "fake merge" | git commit-tree master^{tree} -p master^ -p maint
> b821b2aa00973a47936d7cd25c9a5978b1c839c6
> git-test$ git replace master b821b2aa00973a47936d7cd25c9a5978b1c839c6
> git-test$ git push origin maint
> ...
>    50b03b0..79211fe  maint -> maint

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-17  8:56         ` Junio C Hamano
@ 2012-12-17 10:30           ` Yann Dirson
  0 siblings, 0 replies; 29+ messages in thread
From: Yann Dirson @ 2012-12-17 10:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git list

On Mon, 17 Dec 2012 00:56:06 -0800
Junio C Hamano <gitster@pobox.com> wrote:

> Yann Dirson <dirson@bertin.fr> writes:
> 
> > And we may still want the bug fixed, or would we just list it as a known bug ?
> > At least it does not seem to occur with "replace" refs:
> 
> The "replace" was designed to "fix" known limitation of grafts,
> which is _inherent_ to it; the graft information was designed _not_
> to be shared across repositories.  The fix was done by by using a
> different mechanism to allow propagating the information across
> repositories.

I see.  But from what I observed (without looking at the source), it looks like
when determining which commits are to be pushed, the grafts file is not "neutralized"
as it should.

> So there is nothing further to fix, except that there is a documentation
> bug you can fix if you didn't find it documented.

Will do.

> Thanks.
> 
> >
> > git-test$ rm .git/info/grafts 
> > git-test$ echo "fake merge" | git commit-tree master^{tree} -p master^ -p maint
> > b821b2aa00973a47936d7cd25c9a5978b1c839c6
> > git-test$ git replace master b821b2aa00973a47936d7cd25c9a5978b1c839c6
> > git-test$ git push origin maint
> > ...
> >    50b03b0..79211fe  maint -> maint


-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-17  8:43       ` Thomas Rast
@ 2012-12-17 10:40         ` Yann Dirson
  2012-12-17 13:43           ` Christian Couder
  0 siblings, 1 reply; 29+ messages in thread
From: Yann Dirson @ 2012-12-17 10:40 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Junio C Hamano, git list

On Mon, 17 Dec 2012 09:43:53 +0100
Thomas Rast <trast@student.ethz.ch> wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Yann Dirson <dirson@bertin.fr> writes:
> >
> >> ....  In this respect, they seem to be
> >> lacking a few features, when compared to "replace" refs, but they have different
> >> uses, ...
> >
> > Not reallyl; grafts were old hack whose use is still supported with
> > its original limitations; replace is meant to replace all uses of
> > grafts while removing grafts' largest warts.
> 
> I suppose there's the additional issue that grafts are much easier to
> use than replacements if you really only want to replace some parent
> lists.  With replace you need to handcraft the replacement commits, and
> git-replace(1) unhelpfully does not say this, much less gives an example
> how to do it.
> 

Right, replace refs can surely be made easier to use.  The requirement to craft a
new commit manually is a major step back in ease of use.

Maybe something like "git replace -p <orig-commit> <parent>..." to just provide a simple
API to the exact graft functionnality would be good.  But it would be commit-specific, whereas
replace refs are indeed more generic, and, one could want to rewrite any other part of the commit,
so we could prefer a more general mechanism.

Something that could be useful in this respect, would be an --amend like option to git-commit, like
"git commit --replace".  But unfortunately it does not allow to change parents, and it has the
drawback of requiring that HEAD points to the commit to be replaced.

So maybe, if there are no other idea, a simple "git graft" command that would wrap "git replace",
would fill the gap.

-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-17 10:40         ` Yann Dirson
@ 2012-12-17 13:43           ` Christian Couder
  2012-12-17 14:02             ` Yann Dirson
  2012-12-17 20:03             ` Andreas Schwab
  0 siblings, 2 replies; 29+ messages in thread
From: Christian Couder @ 2012-12-17 13:43 UTC (permalink / raw)
  To: Yann Dirson; +Cc: Thomas Rast, Junio C Hamano, git list

Hi Yann,

On Mon, Dec 17, 2012 at 11:40 AM, Yann Dirson <dirson@bertin.fr> wrote:
> On Mon, 17 Dec 2012 09:43:53 +0100
> Thomas Rast <trast@student.ethz.ch> wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>
>> I suppose there's the additional issue that grafts are much easier to
>> use than replacements if you really only want to replace some parent
>> lists.  With replace you need to handcraft the replacement commits, and
>> git-replace(1) unhelpfully does not say this, much less gives an example
>> how to do it.
>>
>
> Right, replace refs can surely be made easier to use.  The requirement to craft a
> new commit manually is a major step back in ease of use.

Yeah, at one point I wanted to have a command that created to craft a
new commit based on an existing one.
Perhaps it could be useful when using filter-branch or perhaps it
could reuse some filter-branch code.

> Maybe something like "git replace -p <orig-commit> <parent>..." to just provide a simple
> API to the exact graft functionnality would be good.  But it would be commit-specific, whereas
> replace refs are indeed more generic, and, one could want to rewrite any other part of the commit,
> so we could prefer a more general mechanism.

Yeah I wondered at one point if something like the following would do:

git replace --parent <parent1> --parent <parent2> --author <author>
--commiter <commiter> ... <orig-commit>

> Something that could be useful in this respect, would be an --amend like option to git-commit, like
> "git commit --replace".  But unfortunately it does not allow to change parents, and it has the
> drawback of requiring that HEAD points to the commit to be replaced.
>
> So maybe, if there are no other idea, a simple "git graft" command that would wrap "git replace",
> would fill the gap.

It would not be straightforward to call it "graft" if it uses git replace.

Best,
Christian.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-17 13:43           ` Christian Couder
@ 2012-12-17 14:02             ` Yann Dirson
  2012-12-17 20:03             ` Andreas Schwab
  1 sibling, 0 replies; 29+ messages in thread
From: Yann Dirson @ 2012-12-17 14:02 UTC (permalink / raw)
  To: Christian Couder; +Cc: Thomas Rast, Junio C Hamano, git list

On Mon, 17 Dec 2012 14:43:59 +0100
Christian Couder <christian.couder@gmail.com> wrote:

> Hi Yann,
> 
> On Mon, Dec 17, 2012 at 11:40 AM, Yann Dirson <dirson@bertin.fr> wrote:
> > On Mon, 17 Dec 2012 09:43:53 +0100
> > Thomas Rast <trast@student.ethz.ch> wrote:
> >
> >> Junio C Hamano <gitster@pobox.com> writes:
> >>
> >>
> >> I suppose there's the additional issue that grafts are much easier to
> >> use than replacements if you really only want to replace some parent
> >> lists.  With replace you need to handcraft the replacement commits, and
> >> git-replace(1) unhelpfully does not say this, much less gives an example
> >> how to do it.
> >>
> >
> > Right, replace refs can surely be made easier to use.  The requirement to craft a
> > new commit manually is a major step back in ease of use.
> 
> Yeah, at one point I wanted to have a command that created to craft a
> new commit based on an existing one.
> Perhaps it could be useful when using filter-branch or perhaps it
> could reuse some filter-branch code.
> 
> > Maybe something like "git replace -p <orig-commit> <parent>..." to just provide a simple
> > API to the exact graft functionnality would be good.  But it would be commit-specific, whereas
> > replace refs are indeed more generic, and, one could want to rewrite any other part of the commit,
> > so we could prefer a more general mechanism.
> 
> Yeah I wondered at one point if something like the following would do:
> 
> git replace --parent <parent1> --parent <parent2> --author <author>
> --commiter <commiter> ... <orig-commit>

Yes, modification flags, that would only be allowed when the objects are commits, 
and would cause creation of a replace commit that's <orig-commit> plus modifications.
We could then reuse the relevant options from git-commit, and add the missing --parent.

But wouldn't it stretch git-replace too much, to add commit-specific behaviour there ?

> > Something that could be useful in this respect, would be an --amend like option to git-commit, like
> > "git commit --replace".  But unfortunately it does not allow to change parents, and it has the
> > drawback of requiring that HEAD points to the commit to be replaced.
> >
> > So maybe, if there are no other idea, a simple "git graft" command that would wrap "git replace",
> > would fill the gap.
> 
> It would not be straightforward to call it "graft" if it uses git replace.

Well, "git replace" would just be the "implementation detail".  The idea would be to keep
the concept of a "graft", and just change its implementation.  If we care (and we surely
do not, it's just a thought experiment ;), we could even provide, for pre-replace gits, a
"git graft" implementation that would manipulate info/grafts, together with a docpatch
saying that direct manipulation of info/grafts is deprecated.

-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-17 13:43           ` Christian Couder
  2012-12-17 14:02             ` Yann Dirson
@ 2012-12-17 20:03             ` Andreas Schwab
  2012-12-17 21:14               ` Junio C Hamano
  1 sibling, 1 reply; 29+ messages in thread
From: Andreas Schwab @ 2012-12-17 20:03 UTC (permalink / raw)
  To: Christian Couder; +Cc: Yann Dirson, Thomas Rast, Junio C Hamano, git list

Christian Couder <christian.couder@gmail.com> writes:

> Yeah, at one point I wanted to have a command that created to craft a
> new commit based on an existing one.

This isn't hard to do, you only have to resort to plumbing:

$ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w
bb45cc6356eac6c7fa432965090045306dab7026

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-17 20:03             ` Andreas Schwab
@ 2012-12-17 21:14               ` Junio C Hamano
  2012-12-18 11:00                 ` Yann Dirson
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2012-12-17 21:14 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Christian Couder, Yann Dirson, Thomas Rast, git list

Andreas Schwab <schwab@linux-m68k.org> writes:

> Christian Couder <christian.couder@gmail.com> writes:
>
>> Yeah, at one point I wanted to have a command that created to craft a
>> new commit based on an existing one.
>
> This isn't hard to do, you only have to resort to plumbing:
>
> $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w
> bb45cc6356eac6c7fa432965090045306dab7026

Good.  I do not think an extra special-purpose command is welcome
here.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-17 21:14               ` Junio C Hamano
@ 2012-12-18 11:00                 ` Yann Dirson
  2012-12-18 12:03                   ` Johannes Sixt
  2012-12-18 16:09                   ` Junio C Hamano
  0 siblings, 2 replies; 29+ messages in thread
From: Yann Dirson @ 2012-12-18 11:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Schwab, Christian Couder, Thomas Rast, git list

On Mon, 17 Dec 2012 13:14:56 -0800
Junio C Hamano <gitster@pobox.com> wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
> 
> > Christian Couder <christian.couder@gmail.com> writes:
> >
> >> Yeah, at one point I wanted to have a command that created to craft a
> >> new commit based on an existing one.
> >
> > This isn't hard to do, you only have to resort to plumbing:
> >
> > $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w
> > bb45cc6356eac6c7fa432965090045306dab7026
> 
> Good.  I do not think an extra special-purpose command is welcome
> here.

Well, I'm not sure this is intuitive enough to be useful to the average user :)
Adding git-rev-parse calls for convenience, and calling git-replace, would make it
a more complete recipe, and we could suggest that as an alias in the collection that's
in the wiki (which is not even linked any more from git-scm.com btw), but imho that
would be hiding valuable information in a dark corner.

Anyway, in this form it will only replace a parent with another, whereas a full
graft replacement should allow to write a different number of new parents instead.
That is, instead of this simple sed, something like:

(NEWPARENTS='parent xxx\nparent yyy\nparent zzz\n; git cat-file commit master | perl -ne 'BEGIN { $state=0 }; if ($state eq 0) { if (/^parent/) { $state=1 } else { print } } elsif ($state eq 1) { if (/^author/) { print "'"$NEWPARENTS"'"; print; $state=2 } } else { print }')

Well, a short bash script should be more readable and possibly faster, but that's the
idea.  Such a script could be a candidate for contrib ?
-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-18 11:00                 ` Yann Dirson
@ 2012-12-18 12:03                   ` Johannes Sixt
  2012-12-18 12:49                     ` Thomas Rast
  2012-12-18 16:09                   ` Junio C Hamano
  1 sibling, 1 reply; 29+ messages in thread
From: Johannes Sixt @ 2012-12-18 12:03 UTC (permalink / raw)
  To: Yann Dirson
  Cc: Junio C Hamano, Andreas Schwab, Christian Couder, Thomas Rast,
	git list

Am 12/18/2012 12:00, schrieb Yann Dirson:
> On Mon, 17 Dec 2012 13:14:56 -0800
> Junio C Hamano <gitster@pobox.com> wrote:
> 
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>
>>> Christian Couder <christian.couder@gmail.com> writes:
>>>
>>>> Yeah, at one point I wanted to have a command that created to craft a
>>>> new commit based on an existing one.
>>>
>>> This isn't hard to do, you only have to resort to plumbing:
>>>
>>> $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w
>>> bb45cc6356eac6c7fa432965090045306dab7026
>>
>> Good.  I do not think an extra special-purpose command is welcome
>> here.
> 
> Well, I'm not sure this is intuitive enough to be useful to the average user :)

When I played with git-replace in the past, I imagined that it could be

   git replace <object> --commit ...commit options...

that would do the trick.

We could implement it with a git-replace--commit helper script that
generates the replacement commit using the ...commit options... (to be
defined what this should be), and git-replace would just pick its output
(the SHA1 of the generated commit) as a substitute for the <replacement>
argument that would have to be given without the --commit option.

-- Hannes

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-18 12:03                   ` Johannes Sixt
@ 2012-12-18 12:49                     ` Thomas Rast
  2012-12-18 13:41                       ` Yann Dirson
  0 siblings, 1 reply; 29+ messages in thread
From: Thomas Rast @ 2012-12-18 12:49 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Yann Dirson, Junio C Hamano, Andreas Schwab, Christian Couder,
	Thomas Rast, git list

Johannes Sixt <j.sixt@viscovery.net> writes:

> Am 12/18/2012 12:00, schrieb Yann Dirson:
>> On Mon, 17 Dec 2012 13:14:56 -0800
>> Junio C Hamano <gitster@pobox.com> wrote:
>> 
>>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>>
>>>> Christian Couder <christian.couder@gmail.com> writes:
>>>>
>>>>> Yeah, at one point I wanted to have a command that created to craft a
>>>>> new commit based on an existing one.
>>>>
>>>> This isn't hard to do, you only have to resort to plumbing:
>>>>
>>>> $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 |
>>>> sed
>>>> s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/
>>>> | git hash-object -t commit --stdin -w
>>>> bb45cc6356eac6c7fa432965090045306dab7026
>>>
>>> Good.  I do not think an extra special-purpose command is welcome
>>> here.
>> 
>> Well, I'm not sure this is intuitive enough to be useful to the average user :)
>
> When I played with git-replace in the past, I imagined that it could be
>
>    git replace <object> --commit ...commit options...
>
> that would do the trick.
>
> We could implement it with a git-replace--commit helper script that
> generates the replacement commit using the ...commit options... (to be
> defined what this should be), and git-replace would just pick its output
> (the SHA1 of the generated commit) as a substitute for the <replacement>
> argument that would have to be given without the --commit option.

I wouldn't even want a script -- we'd end up inventing a complicated
command-line editor for what can simply be done by judicious use of an
actual text editor.  How about something like the following?


 Documentation/git-replace.txt | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git i/Documentation/git-replace.txt w/Documentation/git-replace.txt
index 51131d0..2502118 100644
--- i/Documentation/git-replace.txt
+++ w/Documentation/git-replace.txt
@@ -61,6 +61,27 @@ OPTIONS
 	Typing "git replace" without arguments, also lists all replace
 	refs.
 
+
+EXAMPLE
+-------
+
+Replacements (and before them, grafts) are often used to replace the
+parent list of a commit.  Since commits are stored in a human-readable
+format, you can in fact change any property using the following
+recipe:
+
+------------------------------------------------
+$ git cat-file commit original_commit >tmp
+$ vi tmp
+------------------------------------------------
+In the editor, adjust the commit as needed.  For example, you can edit
+the parent lists by adding/removing lines starting with "parent".
+When done, replace the original commit with the edited one:
+------------------------------------------------
+$ git replace original_commit $(git hash-object -w tmp)
+------------------------------------------------
+
+
 BUGS
 ----
 Comparing blobs or trees that have been replaced with those that


-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-18 12:49                     ` Thomas Rast
@ 2012-12-18 13:41                       ` Yann Dirson
  2012-12-18 14:31                         ` Thomas Rast
  2012-12-18 16:24                         ` Jeff King
  0 siblings, 2 replies; 29+ messages in thread
From: Yann Dirson @ 2012-12-18 13:41 UTC (permalink / raw)
  To: Thomas Rast
  Cc: Johannes Sixt, Junio C Hamano, Andreas Schwab, Christian Couder,
	Thomas Rast, git list

On Tue, 18 Dec 2012 13:49:44 +0100
Thomas Rast <trast@inf.ethz.ch> wrote:

> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
> > Am 12/18/2012 12:00, schrieb Yann Dirson:
> >> On Mon, 17 Dec 2012 13:14:56 -0800
> >> Junio C Hamano <gitster@pobox.com> wrote:
> >> 
> >>> Andreas Schwab <schwab@linux-m68k.org> writes:
> >>>
> >>>> Christian Couder <christian.couder@gmail.com> writes:
> >>>>
> >>>>> Yeah, at one point I wanted to have a command that created to craft a
> >>>>> new commit based on an existing one.
> >>>>
> >>>> This isn't hard to do, you only have to resort to plumbing:
> >>>>
> >>>> $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 |
> >>>> sed
> >>>> s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/
> >>>> | git hash-object -t commit --stdin -w
> >>>> bb45cc6356eac6c7fa432965090045306dab7026
> >>>
> >>> Good.  I do not think an extra special-purpose command is welcome
> >>> here.
> >> 
> >> Well, I'm not sure this is intuitive enough to be useful to the average user :)
> >
> > When I played with git-replace in the past, I imagined that it could be
> >
> >    git replace <object> --commit ...commit options...
> >
> > that would do the trick.
> >
> > We could implement it with a git-replace--commit helper script that
> > generates the replacement commit using the ...commit options... (to be
> > defined what this should be), and git-replace would just pick its output
> > (the SHA1 of the generated commit) as a substitute for the <replacement>
> > argument that would have to be given without the --commit option.
> 
> I wouldn't even want a script -- we'd end up inventing a complicated
> command-line editor for what can simply be done by judicious use of an
> actual text editor.  How about something like the following?

Well, while it does the job, it is still hardly as straightforward as the
old "vi .git/info/grafts", or as a single easily-remembered commandline.

I was again thinking the only commandline stuff that does not exist currently in
git-commit is specifying parents.  One possiblity would be to add such an
option to git-commit, together with a --replace flag that would cause the
new commit to attached a replace ref (not completely unlike --append, in that
we're doing some non-default action instead of just adding the changes to a
new commit).

But well, I don't think we would want to add to git-commit the ability of playing
with something else than what's in the index/worktree.  Abstracting the commit
commandline to make it reusable by a git-replace--commit and possibly other tools
that may want to rw-manipulate arbitrary commits could make sense ?


> 
>  Documentation/git-replace.txt | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git i/Documentation/git-replace.txt w/Documentation/git-replace.txt
> index 51131d0..2502118 100644
> --- i/Documentation/git-replace.txt
> +++ w/Documentation/git-replace.txt
> @@ -61,6 +61,27 @@ OPTIONS
>  	Typing "git replace" without arguments, also lists all replace
>  	refs.
>  
> +
> +EXAMPLE
> +-------
> +
> +Replacements (and before them, grafts) are often used to replace the
> +parent list of a commit.  Since commits are stored in a human-readable
> +format, you can in fact change any property using the following
> +recipe:
> +
> +------------------------------------------------
> +$ git cat-file commit original_commit >tmp
> +$ vi tmp
> +------------------------------------------------
> +In the editor, adjust the commit as needed.  For example, you can edit
> +the parent lists by adding/removing lines starting with "parent".
> +When done, replace the original commit with the edited one:
> +------------------------------------------------
> +$ git replace original_commit $(git hash-object -w tmp)

You probably meant "-t commit" - a sign that it's not so trivial to forge ?

> +------------------------------------------------
> +
> +
>  BUGS
>  ----
>  Comparing blobs or trees that have been replaced with those that
> 
> 


-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-18 13:41                       ` Yann Dirson
@ 2012-12-18 14:31                         ` Thomas Rast
  2012-12-18 16:24                         ` Jeff King
  1 sibling, 0 replies; 29+ messages in thread
From: Thomas Rast @ 2012-12-18 14:31 UTC (permalink / raw)
  To: Yann Dirson
  Cc: Johannes Sixt, Junio C Hamano, Andreas Schwab, Christian Couder,
	git list

Yann Dirson <dirson@bertin.fr> writes:

>> +EXAMPLE
>> +-------
>> +
>> +Replacements (and before them, grafts) are often used to replace the
>> +parent list of a commit.  Since commits are stored in a human-readable
>> +format, you can in fact change any property using the following
>> +recipe:
>> +
>> +------------------------------------------------
>> +$ git cat-file commit original_commit >tmp
>> +$ vi tmp
>> +------------------------------------------------
>> +In the editor, adjust the commit as needed.  For example, you can edit
>> +the parent lists by adding/removing lines starting with "parent".
>> +When done, replace the original commit with the edited one:
>> +------------------------------------------------
>> +$ git replace original_commit $(git hash-object -w tmp)
>
> You probably meant "-t commit" - a sign that it's not so trivial to forge ?

Mostly a sign that despite my testing efforts, I still fail at
cut&paste...

But yes, it absolutely needs -t commit.  Otherwise the commit would be
replaced by a blob, and confusion ensues.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-18 11:00                 ` Yann Dirson
  2012-12-18 12:03                   ` Johannes Sixt
@ 2012-12-18 16:09                   ` Junio C Hamano
  2012-12-19  8:29                     ` Yann Dirson
  2012-12-19 13:12                     ` Thomas Rast
  1 sibling, 2 replies; 29+ messages in thread
From: Junio C Hamano @ 2012-12-18 16:09 UTC (permalink / raw)
  To: Yann Dirson; +Cc: Andreas Schwab, Christian Couder, Thomas Rast, git list

Yann Dirson <dirson@bertin.fr> writes:

> On Mon, 17 Dec 2012 13:14:56 -0800
> Junio C Hamano <gitster@pobox.com> wrote:
>
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>> 
>> > Christian Couder <christian.couder@gmail.com> writes:
>> >
>> >> Yeah, at one point I wanted to have a command that created to craft a
>> >> new commit based on an existing one.
>> >
>> > This isn't hard to do, you only have to resort to plumbing:
>> >
>> > $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w
>> > bb45cc6356eac6c7fa432965090045306dab7026
>> 
>> Good.  I do not think an extra special-purpose command is welcome
>> here.
>
> Well, I'm not sure this is intuitive enough to be useful to the average user :)

I do not understand why you even want to go in the harder route in
the first place, only to complicate things?

All you want to do is to craft a commit object that records a
specific tree shape, has a set of parents you want, and has the log
information you want.  Once you have the commit, you can replace an
unwanted commit with it.

    ----A----B----o---- ....

           X----Y----Z---- ....

Suppose you want to pretend that X is a child of A, even though it
is not in the real life.  So you want to create a commit that 

    - has the same tree as X;
    - has A as its parent; and
    - records log and authorship of X.

and then use "git replace" to replace X, right?  How about doing it
this way?

    $ git checkout X^0 ;# detach
    $ git reset --soft A
    $ git commit -C X

The first gives you the index and the working tree that is the same
as X, the second moves HEAD while keeping the index and the working
tree so that the commit you create will be a child of A, and the
last makes that commit with the metainformation from X [*1*].  If
you want, you can even tweak the contents of the tree before making
the commit in the final step, or tweak the log message during the
final step.

Then you can take the resulting commit and replace X with it, no?

Alternatively, you can do:

    $ git checkout X^0 ;# detach
    $ git reset --soft B
    $ git commit --amend -C X

that is, find an existing commit B that has the desired set of
parents, and amend it with the same tree and the metainformation as
X.  This would even work when you want to come up with a commit that
replaces a merge.  For example, if you want to pretend that B were a
merge between A and X in the above topology, you could

    $ git checkout -b temp A
    $ git merge -s ours X ;# the recorded tree does not matter
    $ git checkout B^0 ;# detach
    $ git reset --soft temp
    $ git commit --amend -c B

which would create one merge that has the desired set of parents
(i.e. A and X) in the first two steps on temp branch, prepares the
index and the working tree to match the tree of B, and with that
tree and the metainformation from B, amends that merge.  The
resulting commit will be a merge between A and X that has the tree
of B and metainformation of B (with a chance to edit it further, as
I used -c there).

Is this not intuitive enough?


[Footnote]

*1* If you are not tweaking the tree contents, you can do this
all in the index without affecting the working tree, e.g.

    $ git checkout HEAD^0 ;# totally random state unrelated to X nor A
    $ git read-tree X ;# just update the index to match tree of X
    $ git reset --soft A ;# next commit will be child of A
    $ git commit -C X ;# and with metainformation from X

After you are done, you can "read-tree $branch" followed by
"checkout $branch" to come back to where you were.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-18 13:41                       ` Yann Dirson
  2012-12-18 14:31                         ` Thomas Rast
@ 2012-12-18 16:24                         ` Jeff King
  2012-12-19  7:13                           ` Johannes Sixt
  1 sibling, 1 reply; 29+ messages in thread
From: Jeff King @ 2012-12-18 16:24 UTC (permalink / raw)
  To: Yann Dirson
  Cc: Thomas Rast, Johannes Sixt, Junio C Hamano, Andreas Schwab,
	Christian Couder, Thomas Rast, git list

On Tue, Dec 18, 2012 at 02:41:57PM +0100, Yann Dirson wrote:

> > I wouldn't even want a script -- we'd end up inventing a complicated
> > command-line editor for what can simply be done by judicious use of an
> > actual text editor.  How about something like the following?
> 
> Well, while it does the job, it is still hardly as straightforward as the
> old "vi .git/info/grafts", or as a single easily-remembered commandline.

I wouldn't discount coming up with something based around "git commit"
that might be easier to use for specific instances, but it does seem
like an obvious feature to "git replace" to encapsulate Thomas's edit
script, which is the most general form.

I am not really interested in pushing this forward myself, but I worked
up this toy that somebody might find interesting (you can "git replace
HEAD~20" to get dumped in an editor). It should probably handle trees,
and it would probably make sense to do per-object-type sanity checks
(e.g., call verify_tag on tags).

diff --git a/builtin/replace.c b/builtin/replace.c
index 398ccd5..90979b6 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -81,6 +81,57 @@ static int delete_replace_ref(const char *name, const char *ref,
 	return 0;
 }
 
+static void edit_buffer(struct strbuf *out, const char *buf, unsigned long len)
+{
+	char tmpfile[PATH_MAX];
+	int fd;
+
+	fd = git_mkstemp(tmpfile, sizeof(tmpfile), "replace.XXXXXX");
+	if (fd < 0)
+		die_errno("unable to create tempfile");
+	if (write_in_full(fd, buf, len) < 0)
+		die_errno("unable to write to tempfile");
+	if (launch_editor(tmpfile, out, NULL) < 0)
+		die_errno("unable to run editor");
+
+	close(fd);
+	unlink_or_warn(tmpfile);
+}
+
+static void edit_object(unsigned char old[20], unsigned char new[20])
+{
+	enum object_type type;
+	unsigned long size;
+	char *old_buf;
+	struct strbuf new_buf = STRBUF_INIT;
+
+	old_buf = read_sha1_file_extended(old, &type, &size, 0);
+	if (!old_buf)
+		die("unable to read object '%s'", sha1_to_hex(old));
+
+	switch (type) {
+	case OBJ_COMMIT:
+	case OBJ_TAG:
+	case OBJ_BLOB:
+		/* These are OK to edit literally. */
+		edit_buffer(&new_buf, old_buf, size);
+		break;
+	case OBJ_TREE:
+		/*
+		 * XXX we'd probably want to massage this into ls-tree format,
+		 * and then read the result back via mktree.
+		 */
+		die("editing tree objects is not yet supported");
+	default:
+		die("unknown object type for %s", sha1_to_hex(old));
+	}
+
+	if (write_sha1_file(new_buf.buf, new_buf.len, typename(type), new) < 0)
+		die("unable to write replacement object");
+	free(old_buf);
+	strbuf_release(&new_buf);
+}
+
 static int replace_object(const char *object_ref, const char *replace_ref,
 			  int force)
 {
@@ -90,7 +141,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
 
 	if (get_sha1(object_ref, object))
 		die("Failed to resolve '%s' as a valid ref.", object_ref);
-	if (get_sha1(replace_ref, repl))
+	if (replace_ref && get_sha1(replace_ref, repl))
 		die("Failed to resolve '%s' as a valid ref.", replace_ref);
 
 	if (snprintf(ref, sizeof(ref),
@@ -105,6 +156,9 @@ static int replace_object(const char *object_ref, const char *replace_ref,
 	else if (!force)
 		die("replace ref '%s' already exists", ref);
 
+	if (!replace_ref)
+		edit_object(object, repl);
+
 	lock = lock_any_ref_for_update(ref, prev, 0);
 	if (!lock)
 		die("%s: cannot lock the ref", ref);
@@ -144,7 +198,7 @@ int cmd_replace(int argc, const char **argv, const char *prefix)
 
 	/* Replace object */
 	if (!list && argc) {
-		if (argc != 2)
+		if (argc < 1 || argc > 2)
 			usage_msg_opt("bad number of arguments",
 				      git_replace_usage, options);
 		return replace_object(argv[0], argv[1], force);

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-18 16:24                         ` Jeff King
@ 2012-12-19  7:13                           ` Johannes Sixt
  2012-12-19 13:06                             ` Jeff King
  0 siblings, 1 reply; 29+ messages in thread
From: Johannes Sixt @ 2012-12-19  7:13 UTC (permalink / raw)
  To: Jeff King
  Cc: Yann Dirson, Thomas Rast, Junio C Hamano, Andreas Schwab,
	Christian Couder, Thomas Rast, git list

Am 12/18/2012 17:24, schrieb Jeff King:
> I am not really interested in pushing this forward myself, but I worked
> up this toy that somebody might find interesting (you can "git replace
> HEAD~20" to get dumped in an editor). It should probably handle trees,
> and it would probably make sense to do per-object-type sanity checks
> (e.g., call verify_tag on tags).

I know it's just a throw-away patch, but I would discourage to go this
route without also adding all the sanity checks. Otherwise, it will have
just created a porcelain command that can generate a commit object with
any content you want!

-- Hannes

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-18 16:09                   ` Junio C Hamano
@ 2012-12-19  8:29                     ` Yann Dirson
  2012-12-19 13:12                     ` Thomas Rast
  1 sibling, 0 replies; 29+ messages in thread
From: Yann Dirson @ 2012-12-19  8:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Schwab, Christian Couder, Thomas Rast, git list

On Tue, 18 Dec 2012 08:09:35 -0800
Junio C Hamano <gitster@pobox.com> wrote:

> Yann Dirson <dirson@bertin.fr> writes:
> 
> > On Mon, 17 Dec 2012 13:14:56 -0800
> > Junio C Hamano <gitster@pobox.com> wrote:
> >
> >> Andreas Schwab <schwab@linux-m68k.org> writes:
> >> 
> >> > Christian Couder <christian.couder@gmail.com> writes:
> >> >
> >> >> Yeah, at one point I wanted to have a command that created to craft a
> >> >> new commit based on an existing one.
> >> >
> >> > This isn't hard to do, you only have to resort to plumbing:
> >> >
> >> > $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w
> >> > bb45cc6356eac6c7fa432965090045306dab7026
> >> 
> >> Good.  I do not think an extra special-purpose command is welcome
> >> here.
> >
> > Well, I'm not sure this is intuitive enough to be useful to the average user :)
> 
> I do not understand why you even want to go in the harder route in
> the first place, only to complicate things?

Although the approach you propose is elegant, it still looks like one
could not leave the worktree untouched in the case of creating a merge replace,
which the "just forge an arbitrary commit" approach handles easily.

It seems the latter would also be more powerful, in that you can create new commits with an
arbitrary number of parents, even when merge-octopus would simply refuse to help;
and it is has no special case for creating merges.

> Is this not intuitive enough?

I would say it is a nice read that can help an advanced user to earn
some XP - but well, replace refs are also meant for somewhat advanced users :)

-- 
Yann Dirson - Bertin Technologies

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-19  7:13                           ` Johannes Sixt
@ 2012-12-19 13:06                             ` Jeff King
  0 siblings, 0 replies; 29+ messages in thread
From: Jeff King @ 2012-12-19 13:06 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Yann Dirson, Thomas Rast, Junio C Hamano, Andreas Schwab,
	Christian Couder, Thomas Rast, git list

On Wed, Dec 19, 2012 at 08:13:21AM +0100, Johannes Sixt wrote:

> Am 12/18/2012 17:24, schrieb Jeff King:
> > I am not really interested in pushing this forward myself, but I worked
> > up this toy that somebody might find interesting (you can "git replace
> > HEAD~20" to get dumped in an editor). It should probably handle trees,
> > and it would probably make sense to do per-object-type sanity checks
> > (e.g., call verify_tag on tags).
> 
> I know it's just a throw-away patch, but I would discourage to go this
> route without also adding all the sanity checks. Otherwise, it will have
> just created a porcelain command that can generate a commit object with
> any content you want!

I think I agree with you that it would not be worth doing without sanity
checks. I am not sure if your "any content you want" statement means
"bad people can easily make bogus objects" or "it is too easy to make
arbitrary mistakes, putting your repo in a bogus state".

I would agree that the latter is compelling, but not the former.  You
can already easily generate a commit with any content you want via
"hash-object -t commit", and I have frequently done this while testing
corner cases of fsck, how git behaves when given buggy data, etc. So to
me it is not about preventing intentional abuse, but about not promoting
a feature that makes it too easy to screw up.

-Peff

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-18 16:09                   ` Junio C Hamano
  2012-12-19  8:29                     ` Yann Dirson
@ 2012-12-19 13:12                     ` Thomas Rast
  2012-12-19 20:07                       ` Junio C Hamano
  1 sibling, 1 reply; 29+ messages in thread
From: Thomas Rast @ 2012-12-19 13:12 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Yann Dirson, Andreas Schwab, Christian Couder, Thomas Rast,
	git list, Jeff King

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

> I do not understand why you even want to go in the harder route in
> the first place, only to complicate things?
>
> All you want to do is to craft a commit object that records a
> specific tree shape, has a set of parents you want, and has the log
> information you want.  Once you have the commit, you can replace an
> unwanted commit with it.
[...]
>     $ git checkout X^0 ;# detach
>     $ git reset --soft A
>     $ git commit -C X
[...]
> Is this not intuitive enough?

I still wouldn't recommend this approach in git-replace(1) for several
reasons:

* It does not generalize in any direction.  For each field you may want
  to change, you have to know a _specific_ way of getting just the
  commit you want.

* More to the point of replacing the parent lists, while the above might
  be expected of a slightly advanced git user, you get into deep magic
  the second you want to fake a merge commit with an arbitrary
  combination of parents.  (No, you don't need to tell me how.  I'm just
  saying that fooling with either MERGE_HEAD or read-tree is not for
  mere mortals.)

* The above potentially introduces clock skew into the repository, which
  can trigger bugs (like rev-list accidentally missing out on some side
  arm!) until we get around to implementing and using generation
  numbers.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-19 13:12                     ` Thomas Rast
@ 2012-12-19 20:07                       ` Junio C Hamano
  2012-12-21 12:47                         ` Michael J Gruber
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2012-12-19 20:07 UTC (permalink / raw)
  To: Thomas Rast
  Cc: Yann Dirson, Andreas Schwab, Christian Couder, git list,
	Jeff King

Thomas Rast <trast@student.ethz.ch> writes:

> I still wouldn't recommend this approach in git-replace(1) for several
> reasons:
>
> * It does not generalize in any direction.  For each field you may want
>   to change, you have to know a _specific_ way of getting just the
>   commit you want.
>
> * More to the point of replacing the parent lists, while the above might
>   be expected of a slightly advanced git user, you get into deep magic
>   the second you want to fake a merge commit with an arbitrary
>   combination of parents.  (No, you don't need to tell me how.  I'm just
>   saying that fooling with either MERGE_HEAD or read-tree is not for
>   mere mortals.)

I do not buy either of the above.  When you are replacing one with
something else, you ought to know what that something else is and
how to create it.  Editing a text file with an editor to replace
40-hex object names with another is not a more intuitive way for end
users, either (in other words, you are seeing this from the point of
view of somebody who *knows* the internal representation of Git
objects too much).

> * The above potentially introduces clock skew into the repository, which
>   can trigger bugs (like rev-list accidentally missing out on some side
>   arm!) until we get around to implementing and using generation
>   numbers.

That is an irrelevant point when comparing the "go down to bare
metal replacing the object representation" vs "use the usual Git
tools the end users are already familiar with" approaches.  You will
encounter the issue you are raising if you make a newer commit a
parent of an existing child with an older commit timestamp, no
matter how you do the grafting.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-19 20:07                       ` Junio C Hamano
@ 2012-12-21 12:47                         ` Michael J Gruber
  2012-12-21 16:58                           ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Michael J Gruber @ 2012-12-21 12:47 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Thomas Rast, Yann Dirson, Andreas Schwab, Christian Couder,
	git list, Jeff King

While replace refs are much more general than grafts, it seems the two
main uses are:

- grafts (change the recorded parents for a commit)
- svn cleanup (convert tagging commits into tag objects)

The latter one being quite a special case already.

The script below has helped me move from grafts to replace objects.
While not being super clean, something like it may be fit for contrib.

I think we ought to help John Doe get along with parents, while we can
safely leave most more advanced operations to people who know how to
edit a raw object file. Putting that facility into "git-commit" seems to
be too encouraging, though - people would use replace when they should
use amend or rebase-i. I'd prefer a special git-replace mode (be it
"--graft" or "--graft-commit") which does just what my script does. We
could add things like "--commit-tag" later, a full blown
"object-factory" seems like overkill.

Michael

--->%---

#!/bin/sh

die () {
	echo "$@"
	rm -f "$commitfile"
 	exit 1
}

warn () {
	echo "$@"
}

test $# -gt 0 || die "Usage: $0 <commit> [<parent>]*"

for commit
do
	git rev-parse --verify -q "$commit" >/dev/null || die "Cannot parse
$commit."
	test x$(git cat-file -t $commit) == "xcommit" || die "$commit is no
commit."
done

commit="$1"
shift

commitfile=$(mktemp)

git cat-file commit "$commit" | while read a b
do
	if test "$a" != "parent"
	then
		echo $a $b
	fi
	if test "$a" == "tree"
	then
		for parent
		do
			echo "parent $(git rev-parse $parent)"
		done
	fi
done >$commitfile
hash=$(git hash-object -t commit -w "$commitfile") || die "Cannot create
commit object."
git replace "$commit" $hash
rm -f $commitfile

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-21 12:47                         ` Michael J Gruber
@ 2012-12-21 16:58                           ` Junio C Hamano
  2012-12-22 16:38                             ` Michael J Gruber
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2012-12-21 16:58 UTC (permalink / raw)
  To: Michael J Gruber
  Cc: Thomas Rast, Yann Dirson, Andreas Schwab, Christian Couder,
	git list, Jeff King

Michael J Gruber <git@drmicha.warpmail.net> writes:

> While replace refs are much more general than grafts, it seems the two
> main uses are:
>
> - grafts (change the recorded parents for a commit)
> - svn cleanup (convert tagging commits into tag objects)
>
> The latter one being quite a special case already.
>
> The script below has helped me move from grafts to replace objects.
> While not being super clean, something like it may be fit for contrib.
>
> I think we ought to help John Doe get along with parents, while we can
> safely leave most more advanced operations to people who know how to
> edit a raw object file. Putting that facility into "git-commit" seems to
> be too encouraging, though - people would use replace when they should
> use amend or rebase-i. I'd prefer a special git-replace mode (be it
> "--graft" or "--graft-commit") which does just what my script does. We
> could add things like "--commit-tag" later, a full blown
> "object-factory" seems like overkill.
>
> Michael
>
> --->%---
>
> #!/bin/sh
>
> die () {
> 	echo "$@"
> 	rm -f "$commitfile"
>  	exit 1
> }
>
> warn () {
> 	echo "$@"
> }
>
> test $# -gt 0 || die "Usage: $0 <commit> [<parent>]*"
>
> for commit
> do
> 	git rev-parse --verify -q "$commit" >/dev/null || die "Cannot parse
> $commit."
> 	test x$(git cat-file -t $commit) == "xcommit" || die "$commit is no
> commit."

s/==/=/ or you have to say #!/bin/bash on the first line, I think.
Appears multiple times throughout this script.


> done
>
> commit="$1"
> shift
>
> commitfile=$(mktemp)
>
> git cat-file commit "$commit" | while read a b
> do
> 	if test "$a" != "parent"
> 	then
> 		echo $a $b

You are losing information on non-header lines by reading without
"-r" in the above, and also multi-line headers (e.g. mergetag),
aren't you?

> 	fi
> 	if test "$a" == "tree"
> 	then
> 		for parent
> 		do
> 			echo "parent $(git rev-parse $parent)"
> 		done
> 	fi
> done >$commitfile
> hash=$(git hash-object -t commit -w "$commitfile") || die "Cannot create
> commit object."
> git replace "$commit" $hash
> rm -f $commitfile

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [BUG] Cannot push some grafted branches
  2012-12-21 16:58                           ` Junio C Hamano
@ 2012-12-22 16:38                             ` Michael J Gruber
  0 siblings, 0 replies; 29+ messages in thread
From: Michael J Gruber @ 2012-12-22 16:38 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Thomas Rast, Yann Dirson, Andreas Schwab, Christian Couder,
	git list, Jeff King

Junio C Hamano venit, vidit, dixit 21.12.2012 17:58:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> While replace refs are much more general than grafts, it seems the two
>> main uses are:
>>
>> - grafts (change the recorded parents for a commit)
>> - svn cleanup (convert tagging commits into tag objects)
>>
>> The latter one being quite a special case already.
>>
>> The script below has helped me move from grafts to replace objects.
>> While not being super clean, something like it may be fit for contrib.
>>
>> I think we ought to help John Doe get along with parents, while we can
>> safely leave most more advanced operations to people who know how to
>> edit a raw object file. Putting that facility into "git-commit" seems to
>> be too encouraging, though - people would use replace when they should
>> use amend or rebase-i. I'd prefer a special git-replace mode (be it
>> "--graft" or "--graft-commit") which does just what my script does. We
>> could add things like "--commit-tag" later, a full blown
>> "object-factory" seems like overkill.
>>
>> Michael
>>
>> --->%---
>>
>> #!/bin/sh
>>
>> die () {
>> 	echo "$@"
>> 	rm -f "$commitfile"
>>  	exit 1
>> }
>>
>> warn () {
>> 	echo "$@"
>> }
>>
>> test $# -gt 0 || die "Usage: $0 <commit> [<parent>]*"
>>
>> for commit
>> do
>> 	git rev-parse --verify -q "$commit" >/dev/null || die "Cannot parse
>> $commit."
>> 	test x$(git cat-file -t $commit) == "xcommit" || die "$commit is no
>> commit."
> 
> s/==/=/ or you have to say #!/bin/bash on the first line, I think.
> Appears multiple times throughout this script.
> 
> 
>> done
>>
>> commit="$1"
>> shift
>>
>> commitfile=$(mktemp)
>>
>> git cat-file commit "$commit" | while read a b
>> do
>> 	if test "$a" != "parent"
>> 	then
>> 		echo $a $b
> 
> You are losing information on non-header lines by reading without
> "-r" in the above, and also multi-line headers (e.g. mergetag),
> aren't you?
>

Oh yes, it has bashisms and imperfections. It's not a submitted patch,
not even RFC. It's meant to show the git-replace mode that many users
could benefit from: works for commits only and replaces the parent list,
but takes any rev arguments as the new parents, rather than forcing the
user to specify a full sha1.

>> 	fi
>> 	if test "$a" == "tree"
>> 	then
>> 		for parent
>> 		do
>> 			echo "parent $(git rev-parse $parent)"
>> 		done
>> 	fi
>> done >$commitfile
>> hash=$(git hash-object -t commit -w "$commitfile") || die "Cannot create
>> commit object."
>> git replace "$commit" $hash
>> rm -f $commitfile

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2012-12-22 16:39 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-11 14:39 [BUG] Cannot push some grafted branches Yann Dirson
2012-12-11 18:15 ` Junio C Hamano
2012-12-12  8:44   ` Yann Dirson
2012-12-12 10:54     ` Yann Dirson
2012-12-12 19:57     ` Junio C Hamano
2012-12-17  7:52       ` Yann Dirson
2012-12-17  8:56         ` Junio C Hamano
2012-12-17 10:30           ` Yann Dirson
2012-12-17  8:43       ` Thomas Rast
2012-12-17 10:40         ` Yann Dirson
2012-12-17 13:43           ` Christian Couder
2012-12-17 14:02             ` Yann Dirson
2012-12-17 20:03             ` Andreas Schwab
2012-12-17 21:14               ` Junio C Hamano
2012-12-18 11:00                 ` Yann Dirson
2012-12-18 12:03                   ` Johannes Sixt
2012-12-18 12:49                     ` Thomas Rast
2012-12-18 13:41                       ` Yann Dirson
2012-12-18 14:31                         ` Thomas Rast
2012-12-18 16:24                         ` Jeff King
2012-12-19  7:13                           ` Johannes Sixt
2012-12-19 13:06                             ` Jeff King
2012-12-18 16:09                   ` Junio C Hamano
2012-12-19  8:29                     ` Yann Dirson
2012-12-19 13:12                     ` Thomas Rast
2012-12-19 20:07                       ` Junio C Hamano
2012-12-21 12:47                         ` Michael J Gruber
2012-12-21 16:58                           ` Junio C Hamano
2012-12-22 16:38                             ` Michael J Gruber

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).