Bug 12862

Summary: do_package_setscene is unwillingly skipped
Product: [Build System, Metadata & Runtime] OE-Core Reporter: Dmitry <d-bazhenov>
Component: coreAssignee: Richard Purdie <richard.purdie>
Status: NEW --- QA Contact:
Severity: normal    
Priority: Low CC: mark.asselstine, meta.mr.watcher, meta.watcher, randy.macleod, richard.purdie
Version: 2.5.1   
Target Milestone: Future   
Hardware: x86   
OS: Multiple   
Whiteboard:
OS type for building Yocto: --- Type of Regression: New (Never tested)
Verified: Documentation change: No (bug/feature does not impact docs)
Attachments:
Description Flags
bug-fix none

Description Dmitry 2018-07-25 13:29:35 UTC
Created attachment 4350 [details]
bug-fix

Hi everyone,

do_package_setscene task can be unwillingly skipped in some situations.
One such situation is to restore recipe to a previous state for which the recipe has built results stored in the shared state.
    
When do_package_setscene is skipped, the recipe's packages-split directory is not updated accordingly to the stored state and its contents become out of sync.

How to reproduce:
1. Make a recipe which installs files 'a' and 'b' and produces a package 'ab' which would contain both files.
2. Build the recipe.
3. Change the recipe to produce two packages 'a' and 'b' each containing a single file.
4. Build the recipe.
5. Restore original recipe (as in #1)
6. Build the recipe
7. Check ${workdir}/packages-split directory. There will still be two sub-directories for packages 'a' and 'b'.
    
I was able to fix this bug by making a direct reference from do_build to do_package. This made do_package_setscene task 'unskippable' from the bitbake point of view.

The attached patch fixes the bug. Please review.

Best regards,
Dmitry Bazhenov
Comment 1 Richard Purdie 2018-07-25 22:11:56 UTC
I'm not sure this is a bug since if anything actually depends on do_package, it would restore the correct artifact from sstate before allowing anything depending upon it to run.

What problem is this causing other than some slightly stale data in a work directory? I suspect your patch will break assumptions made by rm_work.bbclass that the package task is removable and it will also cause builds from sstate to install much more data than they need to.
Comment 2 Dmitry 2018-07-26 04:37:38 UTC
Hi Richard,

As I understand package tasks are removed in rm_work.bbclass entirely.
When removing, bitbake updates dependencies for the remaining tasks, so no harm is made with adding a direct reference in this regard.

As for populating packages-split directory via do_package_setscene. It is the only task which is not run while do_packagedata_setscene and do_package_write_rpm are actually run restoring both the recipe-specific files and project-wise build artifacts. And the two latter tasks have more files to restore than do_package_setscene, so you shouldn't even notice much change. Especially because the typical usage scenario for this is when you restore a state of a single recipe or a group of recipes, but not the whole distribution.

The stale data in the packages-split directory is misleading in understanding if everything is okay with the build, because there are discrepancies between split package data and generated packages.

For example I can't be sure the configuration option which I introduced for my recipe to change the package generation (bundled package or a set of packages) works okay seeing that the packages-split directory contents do not coordinate with the option value.

So even though the issue does not affect the finally built packages I still believe it is a bug.

Regards,
Dmitry
Comment 3 Richard Purdie 2018-07-26 14:49:21 UTC
We very nearly closed this as WONTFIX, instead I've left it as a low future, unassigned. The proposed fix will break rm_work and cause lots of extra sstate to be downloaded and unpacked even when it isn't used.

I do agree there is a usability issue here. I'm not sure how we'd fix it though.
Comment 4 Mark 2020-02-22 18:36:11 UTC
(In reply to comment #3)
> We very nearly closed this as WONTFIX, instead I've left it as a low future,
> unassigned. The proposed fix will break rm_work and cause lots of extra
> sstate to be downloaded and unpacked even when it isn't used.
> 
> I do agree there is a usability issue here. I'm not sure how we'd fix it
> though.

Rather than the approach of attempting to make the packages-split be properly sync'd with the state of the world to solve this, would it not be equally valid to assume it may be out of sync and thus just remove it.

Thus, would removing the packages-split directory when SSTATETASKS doesn't include do_package_setscene be an option?
Comment 5 Richard Purdie 2020-02-23 11:15:15 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > We very nearly closed this as WONTFIX, instead I've left it as a low future,
> > unassigned. The proposed fix will break rm_work and cause lots of extra
> > sstate to be downloaded and unpacked even when it isn't used.
> > 
> > I do agree there is a usability issue here. I'm not sure how we'd fix it
> > though.
> 
> Rather than the approach of attempting to make the packages-split be
> properly sync'd with the state of the world to solve this, would it not be
> equally valid to assume it may be out of sync and thus just remove it.

Yes, removing it would be better. The question is when to remove it and how to know when to do that.
 
> Thus, would removing the packages-split directory when SSTATETASKS doesn't
> include do_package_setscene be an option?

SSTATETASKS is a fixed value, it doesn't dynamically change depending on system state unfortunately.

My big worry with this bug is making things even more complex and fragile with the solution being worse than the original problem :(.

There is code already there to remove conflicting/skipped recipes. We probably need to extend that to include sstate tasks which no longer match their hashes.