[poky] SRC checksum checking broken

[Gary Thomas]

On 2012-04-17 09:44, Richard Purdie wrote:
> On Tue, 2012-04-17 at 09:21 -0600, Gary Thomas wrote:
>> On 2012-04-17 09:01, Richard Purdie wrote:
>>> On Tue, 2012-04-17 at 06:17 -0600, Gary Thomas wrote:
>>>> On 2012-04-10 19:36, Gary Thomas wrote:
>>>>> On 2012-04-10 18:14, Gary Thomas wrote:
>>>>>> On 2012-04-10 17:27, Richard Purdie wrote:
>>>>>>> On Tue, 2012-04-10 at 14:10 -0600, Gary Thomas wrote:
>>>>>>>> I'm not sure how, but this revision broke the checking of checksums:
>>>>>>>>
>>>>>>>> commit 3d69d9462d550ce4e00e14768cc616bc9ad7e8a5
>>>>>>>> Author: Richard Purdie<richard.purdie at linuxfoundation.org>
>>>>>>>> Date: Fri Mar 23 14:52:23 2012 +0000
>>>>>>>>
>>>>>>>> bitbake/fetch2: Ensure mirror tarballs have .done stamps so we don't redownload and corrupt them
>>>>>>>>
>>>>>>>> [YOCTO #2154]
>>>>>>>>
>>>>>>>> (Bitbake rev: 521d6b14151d3bf934b9597557f7ae46e50a3d7a)
>>>>>>>>
>>>>>>>> Signed-off-by: Richard Purdie<richard.purdie at linuxfoundation.org>
>>>>>>>>
>>>>>>>> The attached log shows this failure - I've modified the busybox_1.19.4.bb
>>>>>>>> recipe to have incorrect checksums. With revision 67c83a8, the failure
>>>>>>>> is correctly identified. With revision 3d69d94, it is not.
>>>>>>>>
>>>>>>>> The error happens with direct downloads (using DL_DIR) and also
>>>>>>>> with a mirror setup (using own-mirrors).
>>>>>>>>
>>>>>>>> I've reviewed the change and I don't see how/why this happens.
>>>>>>>
>>>>>>> There was a patch just merged for something that sounds very similar. I
>>>>>>> was going to give you a link and realised something has gone wrong with
>>>>>>> the commit:
>>>>>>>
>>>>>>> http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=a46aafb8b27bd86d046385a569deee6b4c9b8bb6
>>>>>>>
>>>>>>> as its been squashed with a subsequent commit I had to resolve a
>>>>>>> conflict with.
>>>>>>>
>>>>>>> Regardless, could you see whether that change solves your problem
>>>>>>> please?
>>>>>>
>>>>>> No change, sorry.
>>>>>>
>>>>>
>>>>> I think the problem is this part of the change:
>>>>> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
>>>>> index 7524fa9..e5c4b34 100644
>>>>> --- a/bitbake/lib/bb/fetch2/__init__.py
>>>>> +++ b/bitbake/lib/bb/fetch2/__init__.py
>>>>> @@ -476,6 +476,7 @@ def try_mirrors(d, origud, mirrors, check = False):
>>>>>
>>>>> if not os.path.exists(ud.donestamp) or ud.method.need_update(newuri, ud, ld):
>>>>> ud.method.download(newuri, ud, ld)
>>>>> + open(ud.donestamp, 'w').close()
>>>>> if hasattr(ud.method,"build_mirror_data"):
>>>>> ud.method.build_mirror_data(newuri, ud, ld)
>>>>>
>>>>> I think the call to open() that creates the .done stamp file should really
>>>>> be a call to the update_stamp() function. This is the only place checksums
>>>>> are being checked (as far as I can tell) and the fact that the .done file
>>>>> has already been created is getting in the way. I'd suggest a patch, but I don't
>>>>> understand enough of the context to figure out the parameters to call update_stamp() with
>>>>> [parameters named u, ud&   d don't help me much]
>>>>
>>>> This is still broken, both in master (04d6aa1) and 1.2_M4.rc4 (4d9f4d6)
>>>> Filed as https://bugzilla.yoctoproject.org/show_bug.cgi?id=2311
>>>
>>> Does this help?:
>>>
>>> bitbake/fetch2: Ensure directly mirrored files have their checksum validated
>>>
>>> Signed-off-by: Richard Purdie<richard.purdie at linuxfoundation.org>
>>> ---
>>> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
>>> index 414cc2b..e36d3ec 100644
>>> --- a/bitbake/lib/bb/fetch2/__init__.py
>>> +++ b/bitbake/lib/bb/fetch2/__init__.py
>>> @@ -477,7 +477,10 @@ def try_mirrors(d, origud, mirrors, check = False):
>>>                if not os.path.exists(ud.donestamp) or ud.method.need_update(newuri, ud, ld):
>>>                    ud.method.download(newuri, ud, ld)
>>>                    if os.path.exists(ud.localpath):
>>> -                    open(ud.donestamp, 'w').close()
>>> +                    if ud.localpath == origud.localpath:
>>> +                        update_stamp(newuri, origud, ld)
>>> +                    else:
>>> +                        open(ud.donestamp, 'w').close()
>>>                        if hasattr(ud.method,"build_mirror_data"):
>>>                            ud.method.build_mirror_data(newuri, ud, ld)
>>
>> It does fix the case when using DL_DIR, but own-mirrors is still broken.
>> Note: both used to work correctly.
>
> Can you give more details of the configuration please? Is this using
> file:// urls?

Here's what I use.  I've not tried any other transport method:
   SOURCE_MIRROR_URL ?= "file:///work/misc/Poky/sources/"
   INHERIT += "own-mirrors"

>
> For example in the bug you said you did "-c cleansstate" and I'd not
> expect that to recheck checksums, you'd need to use a "cleanall" for
> that.

I manually removed the ${BUILD}/downloads/busybox* - tarballs and stamps
to force it.  I didn't think to use cleanall.

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------