Re: [PATCH v2 3/3] devtool: provide support for devtool menuconfig command.

[Paul Eggleton <paul.eggleton@linux.intel.com>]

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).

> +    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