[meta-intel] [PATCH 21/31] machinesetuptool: a new recipe for setup of a machine

[Kamble, Nitin A]

On 9/19/14, 5:35 PM, "Darren Hart" <dvhart at linux.intel.com> wrote:

>On Thu, Sep 18, 2014 at 05:35:41PM -0700, nitin.a.kamble at intel.com wrote:
>> From: Nitin A Kamble <nitin.a.kamble at intel.com>
>> 
>> This recipe adds ability to setup a BSP image for a specific machine or
>> platform at the boot time. The base recipe does not provide any machine
>> configuration files, and the required machine configuration files are
>> to be provided in the BSP layers.
>> 
>> This recipe is currently split in 2 files for ease of future migration
>>of
>> the base recipe to the oecore layer.
>> 
>> Signed-off-by: Nitin A Kamble <nitin.a.kamble at intel.com>
>> ---
>>  .../machinesetuptool/machinesetuptool_git.bb       | 49
>>++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>  create mode 100644
>>common/recipes-bsp/machinesetuptool/machinesetuptool_git.bb
>> 
>> diff --git 
>>a/common/recipes-bsp/machinesetuptool/machinesetuptool_git.bb
>>b/common/recipes-bsp/machinesetuptool/machinesetuptool_git.bb
>> new file mode 100644
>> index 0000000..0dfe242
>> --- /dev/null
>> +++ b/common/recipes-bsp/machinesetuptool/machinesetuptool_git.bb
>> @@ -0,0 +1,49 @@
>> +SUMMARY = "Daemon to setup an image for a specific machine at boot
>>time."
>> +SECTION = "base"
>> +LICENSE = "GPLv3"
>> +LIC_FILES_CHKSUM =
>>"file://COPYING;md5=d32239bcb673463ab874e80d47fae504"
>> +RDEPENDS_${PN} = "sysvinit sed"
>> +
>> +PV = "1.0+git${SRCPV}"
>> +
>> +SRCREV = "4cb28ca5de3385f6e16a1e3f69b1a8a79b75ace4"
>> +
>> +SRC_URI = "git://git.yoctoproject.org/machinesetuptool.git"
>> +
>> +S = "${WORKDIR}/git"
>> +
>> +PACKAGE_ARCH = "${MACHINE_ARCH}"
>> +
>> +inherit autotools pkgconfig update-rc.d
>> +
>> +INITSCRIPT_NAME = "machinesetuptool"
>> +INITSCRIPT_PARAMS = "start 00 S . stop 20 0 1 6 ."
>
>Are these deliberate, or possibly copied from something else?

Both. This are coming from zaurusd recipe, and I customized it further.

>
>> +
>> +RRECOMMENDS_${PN} += "kernel-module-uinput"
>> +
>
>Why does the machinesetuptool recommend a specific kernel module?

This also came from the zaurusd, and I believe it¹s purpose to ensure the
ability to get user input at the boot time.

>
>> +python __anonymous () {
>> +    src_uri = d.getVar('SRC_URI', True)
>> +    machine_config_files = (d.getVar('MACHINE_CONFIG_FILES', True) or
>>"")
>> +    for file in machine_config_files.split():
>> +         src_uri += " file://" + file
>> +    d.setVar('SRC_URI', src_uri)
>> +}
>
>Hrm, does this dynamic SRC_URI creation play nice with all the bitbake
>hashing
>of recipes and such?

It is working fine in my testing. Also I don¹t see why it would cause any
problems with bitbake hashes to determine the recipe has changed or not.

>
>> +
>> +do_install_append() {
>> +	{
>> +	echo SUPPORTED_MACHINES=\"${SUPPORTED_MACHINES}\"
>> +	echo DEFAULT_MACHINE_SELECTION=${DEFAULT_MACHINE_SELECTION}
>
>One quoted, one not.. should probably be consistent...

The quoted can have space in the variable definition. Other does not have
space. But it will not hurt putting quotes there.
>
>> +	} > ${D}/${sysconfdir}/${BPN}/defaults
>
>I'd suggest a HERE document here.
>
>http://tldp.org/LDP/abs/html/here-docs.html
>
>(cat << EOF
>SUPPORTED_MACHINE="${SUPPORTED_MACHINES"
>DEFAULT_MACHINE_SELECTION="${DEFAULT_MACHINE_SELECTION}"
>EOF
>) > ${D}/${sysconfdir}/${BPN}/defaults
>
>Tested with bash and dash
Good. Was there any functional issue with the previous approach? Or the
change is mainly for the style reasons.

>
>> +
>> +	for file in ${MACHINE_CONFIG_FILES}
>> +	do
>> +		install -m 0644 ${S}/../${file} ${D}/${sysconfdir}/${BPN}/config/
>> +	done
>> +}
>> +
>> +# following variables are initialized to empty values now.
>> +# These need to be populated with the desired machine configurations
>> +# for each BSP in it's own layer.
>> +MACHINE_CONFIG_FILES = ""
>> +SUPPORTED_MACHINES = ""
>> +DEFAULT_MACHINE_SELECTION = "none"
>
>I'd move these above where they are used, and I'd assign them with ?=
>instead of
>=.

Makes sense.

>
>Before including this feature, I would also like to review the
>machinesetuptool
>itself.

http://git.yoctoproject.org/cgit/cgit.cgi/machinesetuptool/commit/?id=4cb28
ca5de3385f6e16a1e3f69b1a8a79b75ace4


Another question is shall the emenlow machine be named here as
"emenlow-noemgd" or it can be "emenlow², because there is no legacy to
consider in this space.


Thanks for the review Darren,
Nitin

>--
>Darren Hart
>Intel Open Source Technology Center