Re: [edk2-staging/EdkRepo] [PATCH] EdkRepo: Update Create-Pin to be support multiple manifest repositories

["Nate DeSimone" <nathaniel.l.desimone@intel.com>]

Hi Ashley,

Feedback inline below.

Thanks,
Nate

> -----Original Message-----
> From: Desimone, Ashley E <ashley.e.desimone@intel.com>
> Sent: Wednesday, May 6, 2020 3:23 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja
> <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: [edk2-staging/EdkRepo] [PATCH] EdkRepo: Update Create-Pin to be
> support multiple manifest repositories
> 
> Update the create pin command to select and interact with only the
> workspaces source manifest repository.
> 
> Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Puja Pandya <puja.pandya@intel.com>
> Cc: Erik Bjorge <erik.c.bjorge@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  edkrepo/commands/create_pin_command.py | 26
> ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/edkrepo/commands/create_pin_command.py
> b/edkrepo/commands/create_pin_command.py
> index f8618e5..5b77649 100644
> --- a/edkrepo/commands/create_pin_command.py
> +++ b/edkrepo/commands/create_pin_command.py
> @@ -12,14 +12,16 @@ from collections import namedtuple
> 
>  from git import Repo
> 
> -from edkrepo.commands.edkrepo_command import EdkrepoCommand
> +from edkrepo.commands.edkrepo_command import EdkrepoCommand,
> +SourceManifestRepoArgument
>  import edkrepo.commands.arguments.create_pin_args as arguments -from
> edkrepo.common.common_repo_functions import
> pull_latest_manifest_repo  from edkrepo.common.edkrepo_exception
> import EdkrepoManifestInvalidException,
> EdkrepoInvalidParametersException  from
> edkrepo.common.edkrepo_exception import
> EdkrepoWorkspaceCorruptException  from edkrepo.common.humble import
> WRITING_PIN_FILE, GENERATING_PIN_DATA, GENERATING_REPO_DATA,
> BRANCH, COMMIT  from edkrepo.common.humble import
> COMMIT_MESSAGE, PIN_PATH_NOT_PRESENT, PIN_FILE_ALREADY_EXISTS,
> PATH_AND_FILEPATH_USED  from edkrepo.common.humble import
> MISSING_REPO
> +from
> edkrepo.common.workspace_maintenance.manifest_repos_maintenance
> +import find_project_in_all_indices from
> +edkrepo.common.workspace_maintenance.manifest_repos_maintenance
> import
> +pull_workspace_manifest_repo from
> +edkrepo.common.workspace_maintenance.humble.manifest_repos_main
> tenance_
> +humble import PROJ_NOT_IN_REPO,
> SOURCE_MANIFEST_REPO_NOT_FOUND
>  from edkrepo.config.config_factory import get_workspace_manifest,
> get_workspace_path  from edkrepo_manifest_parser.edk_manifest import
> ManifestXml
> 
> @@ -49,6 +51,7 @@ class CreatePinCommand(EdkrepoCommand):
>                       'positional': False,
>                       'required': False,
>                       'help-text': arguments.PUSH_HELP})
> +        args.append(SourceManifestRepoArgument)
>          return metadata
> 
>      def run_command(self, args, config):
> @@ -56,14 +59,25 @@ class CreatePinCommand(EdkrepoCommand):
>          if args.push and os.path.dirname(args.PinFileName):
>              raise
> EdkrepoInvalidParametersException(PATH_AND_FILEPATH_USED)
> 
> -        pull_latest_manifest_repo(args, config)
>          workspace_path = get_workspace_path()
>          manifest = get_workspace_manifest()
> 
> +        manifest_repo, cfg, manifest_repo_path =
> find_project_in_all_indices(manifest.project_info.codename,
> +                                                                                     config['cfg_file'],
> +                                                                                     config['user_cfg_file'],
> +
> PROJ_NOT_IN_REPO.format(manifest.project_info.codename),
> +
> SOURCE_MANIFEST_REPO_NOT_FOUND.format(manifest.project_info.cod
> ename),
> +                                                                                     args.source_manifest_repo)
> +        pull_workspace_manifest_repo(manifest, config['cfg_file'],
> + config['user_cfg_file'], args.source_manifest_repo, False)
> +
> +        if 'user' not in cfg:
> +            manifest_repo_path =
> config['cfg_file'].manifest_repo_abs_path(manifest_repo)
> +        else:
> +            manifest_repo_path =
> + config['user_cfg_file'].manifest_repo_abs_path(manifest_repo)

Please wrap this manifest repo update + manifest repo path retrieval code with "if args.push". We only need to mess around with the manifest repo if we are actually going to push the pin file. If we are not pushing it, then don't bother. There are a bunch of corner cases that could happen here like HTTP/SSH connections not working, loose manifest files, etc. that don't need to work just to create the new XML file. This will also have the side benefit of significantly increasing the performance of this command if the push flag is not given.

>          # If the push flag is enabled use general_config.pin_path to determine
> global manifest relative location to save
>          # pin file to.
>          if args.push and manifest.general_config.pin_path is not None:
> -            pin_dir =
> os.path.join(config['cfg_file'].manifest_repo_abs_local_path,
> os.path.normpath(manifest.general_config.pin_path))
> +            pin_dir = os.path.join(manifest_repo_path,
> + os.path.normpath(manifest.general_config.pin_path))
>              pin_file_name = os.path.join(pin_dir, args.PinFileName)
>          elif args.push and manifest.general_config.pin_path is None:
>              raise EdkrepoManifestInvalidException(PIN_PATH_NOT_PRESENT)
> @@ -104,7 +118,7 @@ class CreatePinCommand(EdkrepoCommand):
> 
>          # commit and push the pin file
>          if args.push:
> -            manifest_repo =
> Repo(config['cfg_file'].manifest_repo_abs_local_path)
> +            manifest_repo = Repo(manifest_repo_path)
>              # Create a local branch with the same name as the pin file arg and
> check it out before attempting the push
>              # to master
>              master_branch = manifest_repo.active_branch @@ -124,4 +138,4 @@
> class CreatePinCommand(EdkrepoCommand):
>                  manifest_repo.git.push('origin', 'HEAD:master')
>              finally:
>                  manifest_repo.heads[master_branch.name].checkout()
> -                manifest_repo.delete_head(local_branch, '-D')
> \ No newline at end of file
> +                manifest_repo.delete_head(local_branch, '-D')
> --
> 2.16.2.windows.1