Hi Richard,

Your point is valid, however the test is failing with fetcher_init() being called before the constructor (see v3). Actually fetcher.unpack() and unpacker.unpack() can be dropped from the test because the problem is in the constructor itself.

They have been added there to do test more "complete"

In normal build we have: fetcher_init -> Fetcher() -> download() -> disable_network() -> unpack()
In "short-circuit" we have: fetcher_init [-> (Fetcher() and download() skipped) ] -> disable_network() -> Fetcher() (because it was not initialized before) -> unpack()

without fetcher_init() the test will pass because data store is not cleaned up and cache will be used to determinate latest_revision without calling ls-remote from the exception handler.

Pavel

07.02.2022, 13:43, "Richard Purdie" <richard.purdie@linuxfoundation.org>:

On Mon, 2022-02-07 at 12:53 +0100, Pavel Zhukov wrote:
Sometimes Fetcher is initialized while network is disabled (unpack() is
one of such cases) and Fetcher fails to map non SHA1 SRCREV to SHA1 one.
In such cases git ls-remote cannot be used to determinate latest
revision and git rev-parse can be used instead. This approach doesn't
work with rebaseable repos so exception will occur for such repos.

See [YOCTO #14707] for more details. Note: the issue doesn't occur each
time in unpack() because in normal flow (fetch->unpack in one build)
URI_HEADREVs are cached.

Signed-off-by: Pavel Zhukov <pavel.zhukov@huawei.com>
---

Thanks for looking at this. I like the idea of some tests to demonstrate the
problem so we should start there.

+class GitUnpackOfflineTest(FetcherTest):
+
+ def setUp(self):
+ super(GitUnpackOfflineTest, self).setUp()
+ self.tempdir = self.tempdir
+ self.persistdir = os.path.join(self.tempdir, "persistdata")
+
+ def setData(self):
+ self.recipe_url = "git://github.com/lz4/lz4.git;branch=release;protocol=https;"
+ self.d.setVar('SRCREV', 'v1.9.0')
+ self.d.setVar("PERSISTENT_DIR", self.persistdir)
+ self.d.setVar("DL_DIR", self.dldir)
+
+ @skipIfNoNetwork()
+ def test_unpack_offline(self):
+ self.setData()
+ fetcher = bb.fetch.Fetch([self.recipe_url], self.d)
+ bb.fetch.fetcher_init(self.d)
+ fetcher.download()
+ fetcher.unpack(self.unpackdir)
+
+ ## emulate do_unpack (disable network)
+ self.d.setVar("BB_NO_NETWORK", "1")
+ unpacker = bb.fetch.Fetch([self.recipe_url], self.d)
+ bb.fetch.fetcher_init(self.d)
+ unpacker.unpack(self.unpackdir)
+

I don't think this is a valid test. I think it should only call
bb.fetch.fetcher_init() once and at the start of the test before the Fetch()
call.

If we change it to do that, the test passes.

Going back to your original bug report, the issue could be that
bb.fetch.fetcher_init() is being called in the do_unpack task and "destroying"
the source revs in the worker context.

I suspect but haven't looked into that yet. What we may need to do is transfer
the bb.fetch2.saved_headrevs into the worker context?

Cheers,

Richard