[bitbake-devel] [PATCH V2] fetch2/gitsm: fix config file concurrent update race

[Mark Hatle]

On 1/26/19 7:50 AM, Stefan Agner wrote:
> On 26.01.2019 14:35, Richard Purdie wrote:
>> On Sat, 2019-01-26 at 13:07 +0100, Patrick Vacek wrote:
>>> On Sat, Jan 26, 2019 at 11:15 AM Stefan Agner <stefan at agner.ch>
>>> wrote:
>>>>> So it seems to be that something in the recipe was expecting
>>>> semi-broken
>>>>> behavior.. and its actually a recipe problem.
>>>>
>>>> Agreed, in this case we need to fix the recipe.
>>>
>>> Sorry, I must be missing something. What might be wrong with the
>>> recipe? Is there more that needs to be specified than a SRCREV and a
>>> branch? As far as I understand git, it keeps track of the submodules,
>>> so I don't know what other information there would be to provide. The
>>> reason this particular case is difficult is because there are
>>> recursive submodules. When we build aktualizr, we have to be careful
>>> to specify that. See
>>> https://github.com/advancedtelematic/aktualizr#building:
>>>
>>> git submodule update --init --recursive
>>
>> The problem is the checkout from the old version of the fetcher was
>> broken, it incorrectly checked out that SRCREV. The recipe was written
>> to build that broken checkout.
>>
>> We've fixed the fetcher and Mark is saying that it now checks out the
>> correct thing (as compared with a git checkout outside of the build
>> system, independent of the fetcher).
>>
>> The recipe assumed the old broken code so now fails to build. The
>> recipe therefore needs to change to adapt to the now correct source
>> code.
> 
> I was not able to reproduce Marks findings.
> 
> In particular:
> 
>> For instance, the submodule partial/extern/isotp-c:
>>
>> In the current version is checked out on
>> 4c5904bff3ed83ec471c8586b5faed62898d0224.  (Which matches the manual git behavior.)
>>
>> While the gitsm fetcher in the older system checkedout commit
>> 614fe48fe55aeea98b49804b7e31ef2d304f62de.  Which is incorrect.
> 
> That wasn't the case for me, the old code checked out 4c5904bf just
> fine.
> 
> However, I think Ming Liu's recent findings point out the culprit.
> 
> The submodule isotp-c requires a submodule on its own, and the new code
> seems not to unpack that.

i see what you are saying, but for some reason the version of git on my machine
(using the steps I mentioned in the original email) for some reason did not pull
down the recursive git submodule, which would explain why my diff didn't find
it.  (It's pretty clear to me that even within git itself, some submodule
operations have been broken in various versions in the past.. my guess is that
I'm hitting one of those errant behaviors locally.)

If I manually go to the isotp-c directory and run git init ; git submodule
update it -then- downloads it.  (So apparently the --recursive argument isn't in
some versions.  My system has 1.8.3.1 on it BTW.)

Looking at this quickly, I suspect I know what the problem is:

        if not ud.bareclone and self.process_submodules(ud, ud.destdir,
unpack_submodules, d):

if ud.bareclone is set, then python will optimize and not even run the
process_submodules which is what does the unpack.  This is set for recursive
submodule parsing.

I'm getting ready to board a plane in a few hours.  So it'll be a day or so
before I can get back to this.  I think I have a fix, see:

http://git.openembedded.org/bitbake-contrib/commit/?h=mgh/gitsm&id=ce5bb0056e6c510855811a26afb95073daf90f2a

If you see this and can test it... I can formally send it for review if it's
resolved the issues when I get to my destination.

--Mark