[yocto] [PATCH 4/4] update_layer.py: move layer validation to update.py (Performance improve)
Robert Yang
liezhi.yang at windriver.com
Sun Apr 22 21:23:46 PDT 2018
Yes, you're right, I updated the two comments in the repo:
+ if layerbranch:
+ if layerbranch.actual_branch:
+ branchname = layerbranch.actual_branch
+ branchdesc = "%s (%s)" % (branch, branchname)
+ else:
+ # LayerBranch doesn't exist for this branch, create it
temporarily
+ # (we won't save this - update_layer.py will do the
actual creation
+ # if it gets called).
+ newbranch = True
+ layerbranch = LayerBranch()
+ layerbranch.layer = layer
...
+ if layerbranch.vcs_last_rev == topcommit.hexsha and not
update.reload:
+ logger.info("Layer %s is already up-to-date for branch
%s" % (layer.name, branchdesc))
+ collections.add((layerbranch.collection,
layerbranch.version))
+ continue
+ else:
+ # Check out appropriate branch
+ if not options.nocheckout:
+ utils.checkout_layer_branch(layerbranch, repodir,
logger=logger)
+ layerdir = os.path.join(repodir, layerbranch.vcs_subdir)
+ if layerbranch.vcs_subdir and not os.path.exists(layerdir):
+ print_subdir_error(newbranch, layer.name,
layerbranch.vcs_subdir, branchdesc)
+ continue
+
+ if not os.path.exists(os.path.join(layerdir,
'conf/layer.conf')):
+ logger.error("conf/layer.conf not found for layer
%s - is subdirectory set correctly?" % layer.name)
+ continue
// Robert
On 04/23/2018 11:35 AM, Paul Eggleton wrote:
> On Monday, 23 April 2018 3:06:43 PM NZST Robert Yang wrote:
>> On 04/23/2018 09:55 AM, Paul Eggleton wrote:
>>> It seems that you are missing a call somewhere to layerbranch.save() later
>>> on, guarded with "if newbranch:". This was easy to miss though when copying
>>> over from update_layer.py seeing as we were relying upon saving it later on
>>> there (and that is still needed).
>>
>> We don't need it to save the new branch here, we just want to know whether it is
>> a new branch or not, the logic is:
>>
>> if newbranch and "the branch doesn't exist" in the repo:
>> continue
>> else: # "the branch exits in the repo"
>> update_layer.py
>>
>> So we don't have to save the new branch here, the update_layer.py will save the
>> new branch when needed, this can reducing a lot of calls to update_layer, which
>> can save a lot of time.
>
> OK - I had missed that you hadn't removed the code from
> update_layer.py that does the actual creation, so the logic is fine.
> I think however we need to adjust the "LayerBranch doesn't exist
> for this branch, create it" comment here though, something like
> "LayerBranch doesn't exist for this branch, create it temporarily
> (we won't save this - update_layer.py will do the actual creation
> if it gets called)."
>
>>>> + if layerbranch.vcs_last_rev == topcommit.hexsha and not update.reload:
>>>> + logger.info("Layer %s is already up-to-date for branch %s" % (layer.name, branchdesc))
>>>> + collections.add((layerbranch.collection, layerbranch.version))
>>>> + continue
>>>> +
>>>> + if layerbranch.vcs_last_rev != topcommit.hexsha or update.reload:
>>>> + # Check out appropriate branch
>>>> + if not options.nocheckout:
>>>> + utils.checkout_layer_branch(layerbranch, repodir, logger=logger)
>>>
>>> Isn't "if layerbranch.vcs_last_rev != topcommit.hexsha or update.reload:"
>>> superfluous here? We're skipping to the next layer if that wasn't true via
>>> "continue" above, so it should always evaluate to true.
>>
>> Let's ignore the update.reload here, then the logic will be clear:
>>
>> if layerbranch.vcs_last_rev == topcommit.hexsha:
>> continue
>> else:
>> checkout_layer_branch()
>>
>> So we can't ignore the else block here, the "if" block doesn't cover the "else"
>> block. And If update.reload, then we can't continue, so the first "if" is:
>> "if layerbranch.vcs_last_rev == topcommit.hexsha and not update.reload"
>>
>> And for the second "if", we need checkout the branch when:
>> "layerbranch.vcs_last_rev != topcommit.hexsha or update.reload"
>>
>> And check whether there is a conf/layer.layer in the repo, we have a lot of
>> layers which are only for specific releases, for example, a layer is for morty
>> branch only, then we cleanup its master branch, just leave a README
>> (no conf/layer.conf) file on master branch which says this layer is for morty
>> only, check conf/layer.conf in update.py rather than in update_layer.py can
>> save a lot of time, too.
>
> Right, I understand the logic, what I'm saying is that the two if statements
> are opposites, and the first one results in a "continue" if true, so the second one
> can logically never evaluate to false, thus there's no point doing the check - just
> put the code inside the block immediately afterwards, i.e.:
>
> if layerbranch.vcs_last_rev == topcommit.hexsha and not update.reload:
> logger.info("Layer %s is already up-to-date for branch %s" % (layer.name, branchdesc))
> collections.add((layerbranch.collection, layerbranch.version))
> continue
>
> # Check out appropriate branch
> if not options.nocheckout:
> utils.checkout_layer_branch(layerbranch, repodir, logger=logger)
> ...
>
> If you wanted to make it even more clear you could have it in an "else" block
> instead, but that's not strictly necessary.
>
> Cheers,
> Paul
>
More information about the yocto
mailing list