[BUG] apply_changeset / generate_changeset use wrong base

[John A Meinel]

Aaron Bentley wrote:
> John A Meinel wrote:
> 
>>>Aaron Bentley wrote:
>>>
>>>
>>>>I take it back.  Using the common ancestor as the changeset base will
>>>>show the differences most clearly.
> 
> 
>>>I think using the common ancestor will show what I have done. While
>>>using the last commit will show what it will do to your tree.
> 
> 
> No, using the common ancestor shows (roughly) what it would do to my
> tree (aside from conflicts and parallel changes).  And the point of a
> merge is to do to my tree what you did to yours.  So the common ancestor
> should do both.
> 
> Using the last commit would show the differences between your tree and
> mine, both from what you'd done, and from my changes.

Sure. I think the default should be the common ancestor.

> 
> 
>>>There is a case for both.
>>>However, once you have "bzr apply-changeset <cset>" you can just do "bzr
>>>diff | vim -" to see how the changes apply to your tree.
>>>
>>>You could always use a throwaway branch to make sure you don't mess up
>>>the important branch.
> 
> 
> Yeah.  Especially if apply-changeset refuses to apply on a modified tree
> (by default).  I'm just thinking of what would be the best default for
> mailed revisions.

As far as I know, it does, though it might do that check a little bit 
late (after pulling in the revisions).

> 
> Btw, I am running into all kinds of problems because
> 1. bzr's revision hash includes parent hashes
> 2. bzr upgrade doesn't upgrade all revisions
> 3. different trees are missing different revisions.
> 
> For example, I can't create a changeset in integration and apply it to
> bzr.24.
> 

Because one has been upgraded and one hasn't? In general if one tree has 
a different hash for a revision entry than the other, that should be a 
problem to start with.

The whole point of having a revision is that they should all be the 
same. I can tell you where the checks are, and you can possibly have 
them report a warning instead of an error (when installing revisions, if 
one is already present, it checks the hash to make sure everything is 
kosher).

If we aren't planning on using a hash to identify a parent, then the 
only thing that should be hashed is the current state.
However, you still have the problem that if one tree computes it's 
hashes different than the other tree, you are going to have problems.

Code *should* be looking at hashes, otherwise what are they there for? 
I've already discussed whether parents hashes should be present and/or 
included in the revision hash, I'm punting to there for the specifics, 
but just defending that if we have hashes we need to use them.

> 
>>>So did we ever work out what the new syntax was going to look like? Are
>>>we switching to using the semi-colon syntax?
> 
> 
> I don't think so, but now that we can specify revisions that aren't in
> the revision history, I think we can pile anything we want onto the -r
> syntax.

Except we were discussing how to do it for a web interface, which would 
not have a "-r" be a possibility.

> 
> I've switched to using common_ancestor, but now test cases are failing,
> and I'm not sure what to do:
> 
> ======================================================================
> ERROR: test_changeset (bzrlib.plugin.bzr-changeset.testchangeset.CSetTester)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File
> "/home/abentley/.bzr.conf/plugins/bzr-changeset/testchangeset.py", line
> 419, in test_changeset
>     cset = self.get_valid_cset(None, 'a at cset-0-1')
>   File
> "/home/abentley/.bzr.conf/plugins/bzr-changeset/testchangeset.py", line
> 315, in get_valid_cset
>     auto_commit=auto_commit, checkout_dir=checkout_dir)
>   File
> "/home/abentley/.bzr.conf/plugins/bzr-changeset/testchangeset.py", line
> 355, in valid_apply_changeset
>     auto_committed = _apply_cset(to_branch, cset, auto_commit=auto_commit)
>   File
> "/home/abentley/.bzr.conf/plugins/bzr-changeset/apply_changeset.py",
> line 114, in _apply_cset
>     base = common_ancestor(branch.last_patch(), cset_info.target, branch)
>   File "/home/abentley/bzr.24/bzrlib/revision.py", line 274, in
> common_ancestor
>     raise bzrlib.errors.NoCommonAncestor(revision_a, revision_b)
> NoCommonAncestor: Revisions have no common ancestor: None a at cset-0-1.

The specific error here looks like one branch has never had a commit, 
versus the other branch.
I thought I was testing doing a complete changeset from "null:->last_patch".

If you look at the request, it is "get_valid_cset(None, 'a at cset-0-1')"
So maybe it should just be changed to "get_valid_cset('null:', 
'a at cset-0-1')"

There needs to be some way to get the complete changeset of the entire 
history.

> 
> common_ancestor is supposed to throw an exception when two trees don't
> have a common ancestor (aside from null:).  This is because it's more
> likely to be a mistake than anything else.  If the user truly wants to
> combine two unrelated trees, (i.e. graft one tree onto another) they can
> do bzr merge -r0..-1.
> 
> I think the same applies here, so that would mean we should fix the test
> cases, not the code.

I think you misunderstood what the test case was trying to do. They were 
related branches, in that it was a branch against 'null:'.

John
=:->

> 
> What do you think?
> 
> Aaron
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 253 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20050927/05481dd3/attachment.pgp