Re: [PATCH 1/5] classes/base: Fix fetch task dependency

[Alexander Smirnov <asmirnov@ilbers.de>]

On 08/29/2017 10:43 AM, Henning Schild wrote:
> Am Tue, 29 Aug 2017 09:27:59 +0300
> schrieb Alexander Smirnov <asmirnov@ilbers.de>:
> 
>> On 08/28/2017 05:57 PM, Henning Schild wrote:
>>> Am Sun, 27 Aug 2017 17:38:36 +0300
>>> schrieb Alexander Smirnov <asmirnov@ilbers.de>:
>>>    
>>>> Fetch task should be run before unpack, not before build.
>>>>
>>>> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
>>>> ---
>>>>    meta/classes/dpkg.bbclass | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
>>>> index 23d5e6c..360a95c 100644
>>>> --- a/meta/classes/dpkg.bbclass
>>>> +++ b/meta/classes/dpkg.bbclass
>>>> @@ -25,7 +25,7 @@ python do_fetch() {
>>>>            raise bb.build.FuncFailed(e)
>>>>    }
>>>>    
>>>> -addtask fetch before do_build
>>>> +addtask fetch before do_unpack
>>>>    
>>>>    do_unpack[dirs] = "${BUILDROOT}"
>>>>    do_unpack[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>>
>>> A few lines below that you find:
>>>    
>>>> addtask unpack after do_fetch before do_build
>>>
>>> With this patch you would have the order expressed twice, which is
>>> not a good idea. This patches does not change anything, it just
>>> introduces redundant information.
>>
>> This patch is not about technical issues, it just improves code
>> readability and explicitly defines the order of tasks. If task fetch
>> should go before unpack, it would be more evident do define it in
>> this way.
> 
> The order is already explicit. 

The order is not actually explicit, the line:

   addtask fetch before do_build

doesn't provide information that fetch task stays before unpack. You 
need to reference another line in code to get this information.

> Writing it down twice is redundant. And
> redundancy means that future changes would have to keep both
> expressions in sync. > In my point of view it harms readability because one has to still read
> both expressions and make sure to what result they will add up.

Instead, with such definition one doesn't need to read both lines, each 
line provides self-contained information about order of tasks in pipeline.

Also do_fetch task will go to base class because it's generic.

Alex