Cannot commit directly to a stacked branch

Bug #375013 reported by John A Meinel
120
This bug affects 27 people
Affects Status Importance Assigned to Milestone
Bazaar
Fix Released
High
John A Meinel
Launchpad itself
Fix Released
High
Jeroen T. Vermeulen
bzr-builder
Fix Released
High
Jelmer Vernooij

Bug Description

Symptoms
--------

bzr commit reports that commit to a stacked branch won't work, and to see this bug.

Workarounds
----------

Make a checkout of the branch and commit to that:
$ bzr checkout $BRANCH $MYCHECKOUT
$ cd $MYCHECKOUT
$ bzr commit

Alternatively, reconfigure that branch to be a full branch, rather than stacked:
$ bzr reconfigure --unstacked $BRANCH

Cause
-----

The internal module that does commits was assuming that a commit on top of an existing repository would always permit a delta to be created by the repository. Stacked repositories don't have that guarantee. While we have an API that will allow a stacked repository to ask for more data, commit did not use that API, and making it use it is non-trivial. Until we fix commit to use that API in some fashion, we've disabled commit in situations where subsequent operations will fail.

Related branches

John A Meinel (jameinel)
Changed in bzr:
importance: Undecided → Medium
status: New → Triaged
tags: added: stacking
Revision history for this message
Andrew Bennetts (spiv) wrote :

This bug (or at least I think it is this bug) causes test_branch_from_trivial_stacked_branch_streaming_acceptance in bzrlib/tests/blackbox/test_branch.py to fail if you change that test to use a 2a format branch:

[...]
  File "/home/andrew/warthogs/bzr/blackbox-with-2a/bzrlib/repofmt/groupcompress_repo.py", line 984, in _get_parent_id_basename_to_file_id_pages
    self._chk_p_id_roots, uninteresting_pid_root_keys):
  File "/home/andrew/warthogs/bzr/blackbox-with-2a/bzrlib/chk_map.py", line 1608, in process
    for record, items in self._process_queues():
  File "/home/andrew/warthogs/bzr/blackbox-with-2a/bzrlib/chk_map.py", line 1578, in _flush_new_queue
    for record, _, p_refs, items in self._read_nodes_from_store(refs):
  File "/home/andrew/warthogs/bzr/blackbox-with-2a/bzrlib/chk_map.py", line 1461, in _read_nodes_from_store
    raise errors.NoSuchRevision(self._store, record.key)
NoSuchRevision: <bzrlib.groupcompress.GroupCompressVersionedFiles object at 0x9db090c> has no revision ('sha1:a4526a70993d853ef740978187ef5479fb179fe9',)

If we change the default format to 2a, as we are planning to do quite soon, then we'll have to do something about this test failure.

Revision history for this message
Andrew Bennetts (spiv) wrote :

Setting 2.0 as the milestone given that 2.0 will have 2a as the default format, which will cause at least one test to fail due to this bug.

Changed in bzr:
milestone: none → 2.0
Changed in bzr:
importance: Medium → Critical
Revision history for this message
Robert Collins (lifeless) wrote :

Making CommitBuilder.commit() get the result of an implicit (or heck, even explicit) StreamSink.insert_stream() would be ideal.

Need to fixup the return code support from/for insert_stream on the underlying versioned files, but I've wanted to do that for a while.

Revision history for this message
Robert Collins (lifeless) wrote :

        tree2 = t.branch.bzrdir.sprout('feature', stacked=True
            ).open_workingtree()
        tree2.commit('feature change')

This definitely adds just one inventory to the inventories VF in tree2.branch.repository.
open questions:
 - why doesn't this break on 1.9?

Revision history for this message
Robert Collins (lifeless) wrote :

Changing the format to 1.9-rich-root makes it fail.

It works on 1.9 because there are _no_ versioned records being copied by the test.

I'm going to change the test to not depend on committing directly to a stacked branch, separating out 'change default formats' from 'test branching from a stacked branch'.

Revision history for this message
Robert Collins (lifeless) wrote :

So, I've put a branch up for review. It disables the broken code path at the repository layer. The test could be ade more sophisticated - e.g. by checking the inventories of all parents are present, but it would make it very hard to explain to users. I ran a full test run with the patch and fixed the tests that failed as a result. I think we can safely untag this from 2.0

summary: - Commit doesn't honor stacking invariants
+ lightweight checkout commit to a stacked branch does not work
tags: added: commit
description: updated
Changed in bzr:
importance: Critical → High
milestone: 2.0 → none
Revision history for this message
Martin Pool (mbp) wrote : Re: lightweight checkout commit to a stacked branch does not work

The branch was apparently https://code.edge.launchpad.net/~lifeless/bzr/bug-375013/+merge/9967 therefore this now seems to be fixed?

Changed in bzr:
milestone: none → 2.0
status: Triaged → Fix Released
Revision history for this message
Robert Collins (lifeless) wrote : Re: [Bug 375013] Re: lightweight checkout commit to a stacked branch does not work

Well, we've put a stop gap in place to prevent corruption.

It is something we should support though. Which is why I had it set to
triaged, and not targeted to anything.

-Rob

Changed in bzr:
status: Fix Released → Triaged
milestone: 2.0 → none
description: updated
Revision history for this message
Aaron Bentley (abentley) wrote : Re: Commit to a stacked branch does not work

Changed title because this appears to affect all commits to stacked branches, not just when lightweight checkouts are involved.

summary: - lightweight checkout commit to a stacked branch does not work
+ Commit to a stacked branch does not work
Revision history for this message
Robert Collins (lifeless) wrote : Re: [Bug 375013] Re: Commit to a stacked branch does not work

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Aaron Bentley wrote:
> Changed title because this appears to affect all commits to stacked
> branches, not just when lightweight checkouts are involved.
>
> ** Summary changed:
>
> - lightweight checkout commit to a stacked branch does not work
> + Commit to a stacked branch does not work
>

I don't think this summary is as useful as it can be, because 'bound
branches' can commit fine to stacked branches. I was hoping in my
previous summary to steer a fine line - can you think of some better
phrasing to indicate 'cannot commit without a bound branch', or something?

- -Rob
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrDW7cACgkQ42zgmrPGrq50EwCgzSLjbKOoU8JlLAqSv0wRNzjT
vVIAn3b/935j0ne2nOpn4DHgHxSFeDE5
=XKY+
-----END PGP SIGNATURE-----

Revision history for this message
Данило Шеган (danilo) wrote : Re: Commit to a stacked branch does not work

We are hitting this problem in Launchpad Translations (bug 439248) as well. If we introduce the workaround, we'll probably add a 'rosetta' bug task here so we don't forget to remove it once the bug is fixed.

Revision history for this message
Aaron Bentley (abentley) wrote : Re: [Bug 375013] Re: Commit to a stacked branch does not work

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> Aaron Bentley wrote:

>> - lightweight checkout commit to a stacked branch does not work
>> + Commit to a stacked branch does not work
>
>
> I don't think this summary is as useful as it can be, because 'bound
> branches' can commit fine to stacked branches. I was hoping in my
> previous summary to steer a fine line

I found it misleading, because it implied that commits where the tree
and stacked branch were co-located would work, and they do not. It
implied a behavioural difference based on whether the branch and tree
were co-located, and those are very rare. It implied that
TreeTransform.commit would work, and it does not.

> - can you think of some better
> phrasing to indicate 'cannot commit without a bound branch', or something?

The only reason why commit via a bound branch works is because it's
pushing, not committing, to the stacked branch. Perhaps "cannot commit
directly to a stacked branch"?

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrKTCwACgkQ0F+nu1YWqI21/QCfaoPwb3xHcTyv5HRNzNMxDC/w
WggAnAkSfW/YjtiGfFL8vOy1rIGnz4TS
=5JVy
-----END PGP SIGNATURE-----

Changed in rosetta:
status: New → Triaged
importance: Undecided → High
summary: - Commit to a stacked branch does not work
+ Cannot commit directly to a stacked branch
Revision history for this message
John Szakmeister (jszakmeister) wrote :

 BTW, the error message should probably be adjusted. At the moment, it's still complaining about committing from a lightweight checkout even though that's not how I have this set up. I was trying to commit to a branch that was stacked against a regular branch. Is there any timeline on when this is going to be fixed?

Revision history for this message
David I (david-ingamells) wrote :

I perceive this bug as a show stopper that is presently preventing me upgrading our team to the LTS 2.0 version.

Since the work flow we follow uses 'branch --stacked', the sticking plaster that has been applied now breaks this flow. Neither of the proposed workarounds are attractive as they both impose extra performance/disk overhead. BTW we have not experienced a single problem with this flow using bzr 1.7.

How do you think l my users would react if I upgrade the scripting to workaround this bug and released 2.0 with an announcement:

Upgrading to the new 2.0 version of bzr - which has as an added feature that commits will be about 3 -10 times slower AND use lots of extra disk space?

Like John S, I would like to see this bug fixed - at least assign it to somebody who can fix it. Presently the only thing most commentary here seems to care about is the exact wording of the bug title and the message the sticking plaster code produces. After all, if the bug was fixed nobody would give a damn what the message used to say.

Revision history for this message
Joke de Buhr (joke) wrote :

At least add a big line to the documentation (http://doc.bazaar.canonical.com/bzr.2.0/en/user-guide/stacked.html) that the feature isn't working.

Martin Pool (mbp)
Changed in bzr:
status: Triaged → Confirmed
Revision history for this message
Alok (alokgovil) wrote :

I only vaguely understand the issue... but nevertheless:

Would it help if a stacked branch holds the last commit from the stacked-on branch whenever it was created or merged last? This will also allow some of the operations like diff on the stacked branch to be local. If my understanding is correct, I do understand that this is somewhat a change in the very definition of a stacked branch but this does not seem to be an issue to me. Extra space required would be justified for me if network load is reduced.

Revision history for this message
Robert Collins (lifeless) wrote : Re: [Bug 375013] Re: Cannot commit directly to a stacked branch

Sadly no, that wouldn't help. We have to guarantee the entire adjacent
inventories are present in order to be able to generate a delta from
the repo.

Tim Penhey (thumper)
tags: added: launchpad
John A Meinel (jameinel)
Changed in bzr:
milestone: none → 2.3b4
assignee: nobody → John A Meinel (jameinel)
milestone: 2.3b4 → none
Vincent Ladeuil (vila)
Changed in bzr:
status: Confirmed → In Progress
milestone: none → 2.3b5
Revision history for this message
John A Meinel (jameinel) wrote :

Fixed for 2a formats in bzr.dev 5599

Changed in bzr:
status: In Progress → Fix Released
Jelmer Vernooij (jelmer)
Changed in bzr-builder:
status: New → In Progress
importance: Undecided → High
assignee: nobody → Jelmer Vernooij (jelmer)
Jelmer Vernooij (jelmer)
Changed in bzr-builder:
status: In Progress → Fix Committed
Jelmer Vernooij (jelmer)
tags: added: rodeo2011
Jelmer Vernooij (jelmer)
tags: added: verified
Jelmer Vernooij (jelmer)
tags: removed: verified
Jelmer Vernooij (jelmer)
Changed in bzr-builder:
milestone: none → 0.7
Jelmer Vernooij (jelmer)
Changed in bzr-builder:
status: Fix Committed → Fix Released
Changed in launchpad:
status: Triaged → In Progress
assignee: nobody → Jeroen T. Vermeulen (jtv)
Revision history for this message
David I (david-ingamells) wrote :

On 14/10/11 10:23, Jeroen T. Vermeulen wrote:
> ** Changed in: launchpad
> Status: Triaged => In Progress
>
> ** Changed in: launchpad
> Assignee: (unassigned) => Jeroen T. Vermeulen (jtv)
>

Jeroen,
If you are working on this bug for launchpad, be aware that bug 718723
also affects the issue. It is marked as critical for launchpad and it is
not yet solved in bazaar itself - nor looks to be in the near future :'( .

--

*David*

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

David I: thanks for the warning—tragically I have not the smallest clue what bug 718723 even means! The title reads like an unknown language to me.

The only Translations change that was needed (already in review, and it passed tests on a stacked branch) affects use of Launchpad's DirectBranchCommit class on a stacked branch. It's a lightweight way to commit changes to selected files to a branch, and as far as I can tell only just similar enough to a lightweight checkout to be caught out by this bug here.

Revision history for this message
Launchpad QA Bot (lpqabot) wrote :
tags: added: qa-needstesting
Changed in launchpad:
status: In Progress → Fix Committed
tags: added: qa-ok
removed: qa-needstesting
Revision history for this message
Robert Collins (lifeless) wrote :

The referenced bug is for bundles, which are not involved in the translations code-path.

William Grant (wgrant)
Changed in launchpad:
status: Fix Committed → Fix Released
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Thanks Robert for confirming that the fix isn't inviting another known bug. Thanks William for arranging the rollout.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.