Bug 10619

Summary:	Refactor wic codebase
Product:	[Build System, Metadata & Runtime] OE-Core	Reporter:	Ed Bartosh <eduard.bartosh>
Component:	Scripts and Tools	Assignee:	Ed Bartosh <eduard.bartosh>
Status:	RESOLVED FIXED	QA Contact:
Severity:	enhancement
Priority:	Medium	CC:	bluelightning, ross.burton
Version:	unspecified
Target Milestone:	2.3 M3
Hardware:	x86
OS:	Multiple
Whiteboard:	LET
OS type for building Yocto:	---	Type of Regression:	---
Verified:		Documentation change:	No (bug/feature does not impact docs)

Description Ed Bartosh 2016-11-08 09:55:49 UTC

Currently wic code is quite hard to follow due to its complicated class inheritance and data models. It makes sense to refactor it to make it more maintainable and easier to change.

Possible areas of refactoring:
- get rid of msger logging in favor of logging Python module
- flatten directory structure. Directory paths similar to ./scripts/lib/wic/utils/oe/ with only one file misc.py inside it should be simplified.
- get rid of two different partition structures in direct and partitionedfs modules
- flatten class hierarchy. Currently it has a lot more levels of abstractions than needed for this simple tool
- get rid of two underscore-prefixed attributes and methods
- clean up the codebase. Currently pylint shows hundreds of warnings for it
...

Comment 1 Ed Bartosh 2016-12-16 21:18:03 UTC

one more point from bug 10816: runner/exec code duplicates the other wrapping of subprocess done elsewhere in bitbake and oe-core

Comment 2 Christopher Larson 2016-12-16 21:19:09 UTC

This would be nice, trying to trace the flow for that other recent issue took longer than it might have :) It felt a bit more like a c++ or java codebase -- more OOP than is usually seen in pythonic code, which tends to be more pragmatic and multi-paradigm.

Comment 3 Ed Bartosh 2017-01-23 22:57:12 UTC

one more area to lock at is configuration management. Code in conf.py and empty configuration file in config/wic.conf are very confusing. I think they should be removed. If we need this functionality we can implement it in much cleaner way.

Comment 4 Ed Bartosh 2017-02-01 14:38:55 UTC

First set of changes has been sent for review: http://lists.openembedded.org/pipermail/openembedded-core/2017-February/132117.html

Comment 5 Ed Bartosh 2017-02-03 21:27:43 UTC

flatten image class hierarchy: http://lists.openembedded.org/pipermail/openembedded-core/2017-February/132246.html

Comment 6 Ed Bartosh 2017-02-10 15:12:07 UTC

Simplified interfaces of DirectPlugin and underlying
set of APIs located in partitionedfs module. Removed one of
two different structures for partitioning info:

http://lists.openembedded.org/pipermail/openembedded-core/2017-February/132430.html

Comment 7 Ed Bartosh 2017-02-10 15:30:42 UTC

Moved partition operations to PartitionedImage:
http://lists.openembedded.org/pipermail/openembedded-core/2017-February/132558.html

Comment 8 Ed Bartosh 2017-02-15 09:04:33 UTC

Removed msger in favor of logging. Improved error handling:
http://lists.openembedded.org/pipermail/openembedded-core/2017-February/132782.html

Comment 9 Ed Bartosh 2017-02-16 15:58:55 UTC

Refactored plugin system: http://lists.openembedded.org/pipermail/openembedded-core/2017-February/132854.html

Comment 10 Ed Bartosh 2017-02-16 16:02:23 UTC

I'm going to resolve this bug when all above mentioned patchset are merged.

However, there is still some work left on this area

- make use of ArgumentParser: bug #9636
- refactor engine.py and help.py
- replace plugin code that configures bootloaders with using configuration
  files (--configfile .wks option)
- refactor source plugins
- get rid of custom runner module in favor of using subprocess
- move the rest of the code from utils/ subdirectory to lib/wic/utils.py

I'm going to address this when I have time and put it here as a reminder and in a hope that other people want to join me with this.

Comment 11 Ed Bartosh 2017-03-06 11:38:14 UTC

all patches have been merged to poky master