[OE-core] [OE-Core][PATCH v2 3/3] devtool: provide support for devtool menuconfig command.

[Chandana Kalluri]

-----Original Message-----
From: Paul Eggleton <paul.eggleton at linux.intel.com> 
Sent: Monday, March 4, 2019 3:46 PM
To: Chandana Kalluri <ckalluri at xilinx.com>
Cc: openembedded-core at lists.openembedded.org
Subject: Re: [OE-core] [OE-Core][PATCH v2 3/3] devtool: provide support for devtool menuconfig command.

On Saturday, 26 January 2019 8:53:02 AM NZDT Sai Hari Chandana Kalluri wrote:
> +def menuconfig(args, config, basepath, workspace):
> +    """Entry point for the devtool 'menuconfig' subcommand"""
> +
> +    rd = "" 
> +    kconfigpath = ""
> +    pn_src = ""
> +    localfilesdir = ""
> +    workspace_dir = ""
> +    tinfoil = setup_tinfoil(basepath=basepath)
> +    try:
> +      rd = parse_recipe(config, tinfoil, args.component, appends=True, filter_workspace=False)
> +      if not rd:
> +         return 1
> +
> +      pn =  rd.getVar('PN', True)
> +      if pn not in workspace:
> +         raise DevtoolError("Run devtool modify before calling menuconfig for %s" %pn)

Strictly speaking we have check_workspace_recipe() for this. It could be that we should extend the message printed by that to be a bit more friendly and suggest devtool add/modify/upgrade first, but I'd like to be consistent.

>...
> +      #add check to see if oe_local_files exists or not
> +      localfilesdir = os.path.join(pn_src,'oe-local-files') 
> +      if not os.path.exists(localfilesdir):
> +          bb.utils.mkdirhier(localfilesdir)
> +          #Add gitignore to ensure source tree is clean
> +          gitignorefile = os.path.join(localfilesdir,'.gitignore')
> +          with open(gitignorefile, 'w') as f:
> +                  f.write('# Ignore local files, by default. Remove this file if you want to commit the directory to Git\n')
> +                  f.write('*\n')

We should already be handling this in devtool-source.bbclass, but I'm guessing the hardlinking you're adding bypasses that, so we likely need to handle that separately during devtool modify rather than here (preferably without duplicating code).

This check for oe_local_files is specific for all other packages except linux-yocto. This is needed if the package uses make menuconfig to generate cfg and for devtool finish to pick up the cfg fragment. 
For linux-yocto, after the hardlinking is done, the check for oe-local-files is taken care for within devtool modify( this is in PATCH 1/3 ). 

The above check for oe-local-files for all packages could be handled within devtool modify and can be done in two ways:
1. Either all packages(even the ones that do not run menuconfig command) have an empty oe-local-files dir as a part of workspace/source OR
2. have a check within modify to see if it runs menuconfig task and create the oe-local-files. 

What would you recommend ?

> +    parser_menuconfig = subparsers.add_parser('menuconfig',help='allows altering the system component configuration', description='launches the make menuconfig command, allows user to make changes to configuration. creates a config fragment', group='advanced') 
> +    parser_menuconfig.add_argument('component', help='compenent to alter config')

To be consistent with other subcommands and a bit more readable this should be something like:

    parser_menuconfig = subparsers.add_parser('menuconfig', help='Alter build-time configuration for a recipe', description='Launches the "make menuconfig" command (for recipes where do_menuconfig is available), allowing you to make changes to the build-time configuration. Creates a config fragment corresponding to changes made.', group='advanced') 
    parser_menuconfig.add_argument('recipename', help='Recipe to reconfigure')

Additionally, I would really like to see some oe-selftest tests for this along with the changes.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre