Stream fetch fails "insert_from_broken_repo" test
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Bazaar |
Fix Released
|
Medium
|
Robert Collins |
Bug Description
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
So in trying to land my PackStreamSource patch, I ran into an issue with
streaming fetch, in general. Namely the test:
bzr selftest -s bt.test_repository test_insert_
Now, #1 this is only a test of a single format, when it really seems
like it should be a per_repo or per_inter_repo test.
Changing from InterPackRepository to PackStreamSource causes the
exception to not be raised, but as near as I can tell, that is true for
*any* StreamSource (at the moment).
The part that the test is triggering is:
1) Insert a text into the repository (fid, rev1c)
2) Build a new revision rev3 which does *not* reference rev1c, but the
*text* (fid, rev3) claims (fid, rev1c) as a parent.
3) During the insert, we call _check_references, which calls
"_get_external_
referenced, but not present in the repository.
With StreamSink.
Namely for missing entries it does:
for prefix, versioned_file in (
... ):
if versioned_file is None:
It then does a single "fetch me whatever is missing in the first pass",
and then fails if there is still anything missing.
The issue breaks down in 2 ways:
1) It is somewhat random whether (fid, rev1c) is going to be a
compression parent. It certainly wouldn't for --2a repos, as it stands
it randomly is because the text sizes are 0, so you always get a delta
chain of length 1. (1 fulltext, 1 delta)
Because (fid, rev1c) ends up being a compression parent, it goes ahead
and fetches the text, and then nothing is missing. (sort of good, in
that it fills in what would otherwise be missing, though that missing
entry is a bad reference.)
2) So to make it fail, I tried increasing the length of the bogus
references, adding (fid, rev3) => (fid, rev1c) => (fid, rev1d)
However, the get_stream_
(fid, rev1c) because that is how we designed it.
Which means that again there are no missing *compression* parents. What
*is* missing is the actual parent text.
Now the check that Packer was doing, would be to actually check
"get_missing_
note that Packer doesn't get used for stacked repos...)
One option would be something that would:
get missing_
and missing_
Except with the way that stacking works with Smart requests, you don't
*have* fallbacks that you can query...
So at this point I feel like I'm out of options. I can't write a runtime
missing parent check that will work with Smart + Stacking, and the
'get_stream_
I've decided to just disable that test for now, and submit this as a bug
report to Launchpad so we don't forget about it. But I would certainly
be interested in any feedback people have about possible solutions.
John
=:->
affects bzr
status triaged
importance medium
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAko
8x8AniXPIjgi4/
=voNX
-----END PGP SIGNATURE-----
Related branches
- John A Meinel: Approve
- Diff: 29 lines
Changed in bzr: | |
status: | Triaged → Fix Released |
assignee: | nobody → Robert Collins (lifeless) |
milestone: | none → 2.0 |
2a requires that we address this now.
AIUI the problem is solved now; that is we can copy successfully from this sort of corruption.
So why not change to test to handle both:
- error
- the text is extractable.
That should cover all bases, and I'm working on that now.