From adbd71557bc8130724f33a1046feea2704f956cf Mon Sep 17 00:00:00 2001 From: John Alex Date: Wed, 4 Jan 2023 17:05:21 +0000 Subject: [PATCH] On checkout, refresh locally installed optional packages regardless of whether -o is passed in. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implementation detail: this involves checking external’s status. But since Sourcetree.status() and therefore _External.status() is already called at the very start of checkout_externals, allow it to be efficiently called twice: Change _External.status() to skip doing work if it was run before. Make it optional to print out the external names as it does its work. Also: sourcetree.py: no longer assumes status() is called before checkout() - checks self._stat and calls status() if needed. checkout.py: move enormous message into its own helper function --- manic/checkout.py | 74 +++++++++---------- manic/externals_description.py | 9 +++ manic/sourcetree.py | 131 +++++++++++++++++++-------------- 3 files changed, 120 insertions(+), 94 deletions(-) diff --git a/manic/checkout.py b/manic/checkout.py index a581f8787..ee39f9e35 100755 --- a/manic/checkout.py +++ b/manic/checkout.py @@ -338,7 +338,34 @@ def commandline_arguments(args=None): options = parser.parse_args() return options - +def _dirty_local_repo_msg(program_name, config_file): + return """The external repositories labeled with 'M' above are not in a clean state. +The following are four options for how to proceed: +(1) Go into each external that is not in a clean state and issue either a 'git status' or + an 'svn status' command (depending on whether the external is managed by git or + svn). Either revert or commit your changes so that all externals are in a clean + state. (To revert changes in git, follow the instructions given when you run 'git + status'.) (Note, though, that it is okay to have untracked files in your working + directory.) Then rerun {program_name}. +(2) Alternatively, you do not have to rely on {program_name}. Instead, you can manually + update out-of-sync externals (labeled with 's' above) as described in the + configuration file {config_file}. (For example, run 'git fetch' and 'git checkout' + commands to checkout the appropriate tags for each external, as given in + {config_file}.) +(3) You can also use {program_name} to manage most, but not all externals: You can specify + one or more externals to ignore using the '-x' or '--exclude' argument to + {program_name}. Excluding externals labeled with 'M' will allow {program_name} to + update the other, non-excluded externals. +(4) As a last resort, if you are confident that there is no work that needs to be saved + from a given external, you can remove that external (via "rm -rf [directory]") and + then rerun the {program_name} tool. This option is mainly useful as a workaround for + issues with this tool (such as https://github.com/ESMCI/manage_externals/issues/157). +The external repositories labeled with '?' above are not under version +control using the expected protocol. If you are sure you want to switch +protocols, and you don't have any work you need to save from this +directory, then run "rm -rf [directory]" before rerunning the +{program_name} tool. +""".format(program_name=program_name, config_file=config_file) # --------------------------------------------------------------------- # # main @@ -380,8 +407,12 @@ def main(args): comp, args.externals)) source_tree = SourceTree(root_dir, external, svn_ignore_ancestry=args.svn_ignore_ancestry) - printlog('Checking status of externals: ', end='') - tree_status = source_tree.status() + if args.components: + components_str = 'specified components' + else: + components_str = 'required & optional components' + printlog('Checking local status of ' + components_str + ': ', end='') + tree_status = source_tree.status(print_progress=True) printlog('') if args.status: @@ -396,43 +427,8 @@ def main(args): for comp in sorted(tree_status): tree_status[comp].log_status_message(args.verbose) # exit gracefully - msg = """The external repositories labeled with 'M' above are not in a clean state. - -The following are four options for how to proceed: - -(1) Go into each external that is not in a clean state and issue either a 'git status' or - an 'svn status' command (depending on whether the external is managed by git or - svn). Either revert or commit your changes so that all externals are in a clean - state. (To revert changes in git, follow the instructions given when you run 'git - status'.) (Note, though, that it is okay to have untracked files in your working - directory.) Then rerun {program_name}. - -(2) Alternatively, you do not have to rely on {program_name}. Instead, you can manually - update out-of-sync externals (labeled with 's' above) as described in the - configuration file {config_file}. (For example, run 'git fetch' and 'git checkout' - commands to checkout the appropriate tags for each external, as given in - {config_file}.) - -(3) You can also use {program_name} to manage most, but not all externals: You can specify - one or more externals to ignore using the '-x' or '--exclude' argument to - {program_name}. Excluding externals labeled with 'M' will allow {program_name} to - update the other, non-excluded externals. - -(4) As a last resort, if you are confident that there is no work that needs to be saved - from a given external, you can remove that external (via "rm -rf [directory]") and - then rerun the {program_name} tool. This option is mainly useful as a workaround for - issues with this tool (such as https://github.com/ESMCI/manage_externals/issues/157). - - -The external repositories labeled with '?' above are not under version -control using the expected protocol. If you are sure you want to switch -protocols, and you don't have any work you need to save from this -directory, then run "rm -rf [directory]" before rerunning the -{program_name} tool. -""".format(program_name=program_name, config_file=args.externals) - printlog('-' * 70) - printlog(msg) + printlog(_dirty_local_repo_msg(program_name, args.externals)) printlog('-' * 70) else: if not args.components: diff --git a/manic/externals_description.py b/manic/externals_description.py index 4b08bc42a..9449bbf72 100644 --- a/manic/externals_description.py +++ b/manic/externals_description.py @@ -281,6 +281,10 @@ def read_gitmodules_file(root_dir, file_name): def create_externals_description( model_data, model_format='cfg', components=None, exclude=None, parent_repo=None): """Create the a externals description object from the provided data + + components: list of component names to include, None to include all. If a + name isn't found, it is silently omitted from the return value. + exclude: list of component names to skip. """ externals_description = None if model_format == 'dict': @@ -763,6 +767,8 @@ def __init__(self, model_data, components=None, exclude=None, parent_repo=None): """Convert the config data into a standardized dict that can be used to construct the source objects + components: list of component names to include, None to include all. + exclude: list of component names to skip. """ ExternalsDescription.__init__(self, parent_repo=parent_repo) self._schema_major = 1 @@ -786,6 +792,9 @@ def _remove_metadata(model_data): def _parse_cfg(self, cfg_data, components=None, exclude=None): """Parse a config_parser object into a externals description. + + components: list of component names to include, None to include all. + exclude: list of component names to skip. """ def list_to_dict(input_list, convert_to_lower_case=True): """Convert a list of key-value pairs into a dictionary. diff --git a/manic/sourcetree.py b/manic/sourcetree.py index eafd43e65..1e66623a2 100644 --- a/manic/sourcetree.py +++ b/manic/sourcetree.py @@ -47,7 +47,7 @@ def __init__(self, root_dir, name, ext_description, svn_ignore_ancestry): self._externals_path = EMPTY_STR # Can also be "none" self._externals_sourcetree = None - self._stat = ExternalStatus() # Populated in status() + self._stat = None # Populated in status() self._sparse = None # Parse the sub-elements @@ -98,47 +98,51 @@ def get_local_path(self): """ return self._local_path - def status(self): + def status(self, force=False): """ Returns status of this component and all subcomponents (if available). Returns a dict mapping our local path to an ExternalStatus dict. Any subcomponents will have their own top-level key. - Populates self._stat as a side effect. + Side-effect: If self._stat is empty or force is True, calculates _stat. """ - self._stat.path = self.get_local_path() - if not self._required: - self._stat.source_type = ExternalStatus.OPTIONAL - elif self._local_path == LOCAL_PATH_INDICATOR: - # LOCAL_PATH_INDICATOR, '.' paths, are standalone - # component directories that are not managed by - # checkout_externals. - self._stat.source_type = ExternalStatus.STANDALONE - else: - # managed by checkout_externals - self._stat.source_type = ExternalStatus.MANAGED + calc_stat = force or not self._stat + + if calc_stat: + self._stat = ExternalStatus() + self._stat.path = self.get_local_path() + if not self._required: + self._stat.source_type = ExternalStatus.OPTIONAL + elif self._local_path == LOCAL_PATH_INDICATOR: + # LOCAL_PATH_INDICATOR, '.' paths, are standalone + # component directories that are not managed by + # checkout_externals. + self._stat.source_type = ExternalStatus.STANDALONE + else: + # managed by checkout_externals + self._stat.source_type = ExternalStatus.MANAGED subcomponent_stats = {} - if not os.path.exists(self._repo_dir_path): - # No local repository. - self._stat.sync_state = ExternalStatus.EMPTY - msg = ('status check: repository directory for "{0}" does not ' - 'exist.'.format(self._name)) - logging.info(msg) - self._stat.current_version = 'not checked out' - # NOTE(bja, 2018-01) directory doesn't exist, so we cannot - # use repo to determine the expected version. We just take - # a best-guess based on the assumption that only tag or - # branch should be set, but not both. - if not self._repo: - self._stat.expected_version = 'unknown' - else: - self._stat.expected_version = self._repo.tag() + self._repo.branch() + if calc_stat: + # No local repository. + self._stat.sync_state = ExternalStatus.EMPTY + msg = ('status check: repository directory for "{0}" does not ' + 'exist.'.format(self._name)) + logging.info(msg) + self._stat.current_version = 'not checked out' + # NOTE(bja, 2018-01) directory doesn't exist, so we cannot + # use repo to determine the expected version. We just take + # a best-guess based on the assumption that only tag or + # branch should be set, but not both. + if not self._repo: + self._stat.expected_version = 'unknown' + else: + self._stat.expected_version = self._repo.tag() + self._repo.branch() else: # Merge local repository state (e.g. clean/dirty) into self._stat. - if self._repo: + if calc_stat and self._repo: self._repo.status(self._stat, self._repo_dir_path) # Status of subcomponents, if any. @@ -147,7 +151,7 @@ def status(self): # SourceTree.status() expects to be called from the correct # root directory. os.chdir(self._repo_dir_path) - subcomponent_stats = self._externals_sourcetree.status(self._local_path) + subcomponent_stats = self._externals_sourcetree.status(self._local_path, force=force) os.chdir(cwd) # Merge our status + subcomponent statuses into one return dict keyed @@ -171,12 +175,9 @@ def checkout(self, verbosity, load_all): correct URL, correct branch or tag), and possibly update the external. If the repo destination directory does not exist, checkout the correct branch or tag. - If load_all is True, also load all of the the externals sub-externals. + load_all is currently ignored. See checkout_externals() to check out sub-externals. """ - if load_all: - pass # Make sure we are in correct location - if not os.path.exists(self._repo_dir_path): # repository directory doesn't exist. Need to check it # out, and for that we need the base_dir_path to exist @@ -188,6 +189,10 @@ def checkout(self, verbosity, load_all): self._base_dir_path) fatal_error(msg) + if not self._stat: + self.status() + assert self._stat + if self._stat.source_type != ExternalStatus.STANDALONE: if verbosity >= VERBOSITY_VERBOSE: # NOTE(bja, 2018-01) probably do not want to pass @@ -209,7 +214,9 @@ def checkout(self, verbosity, load_all): checkout_verbosity, self.clone_recursive()) def checkout_externals(self, verbosity, load_all): - """Checkout the sub-externals for this object + """Checkout the sub-externals for this component, if any. + + if load_all is True, also recurse into sub-sub-externals and so on. """ if self.load_externals(): if self._externals_sourcetree: @@ -224,7 +231,7 @@ def checkout_externals(self, verbosity, load_all): self._externals_sourcetree.checkout(verbosity, load_all) def load_externals(self): - 'Return True iff an externals file should be loaded' + 'Return True iff an externals file exists (and therefore should be loaded)' load_ex = False if os.path.exists(self._repo_dir_path): if self._externals_path: @@ -236,7 +243,7 @@ def load_externals(self): def clone_recursive(self): 'Return True iff any .gitmodules files should be processed' - # Try recursive unless there is an externals entry + # Try recursive .gitmodules unless there is an externals entry recursive = not self._externals_path return recursive @@ -300,26 +307,23 @@ def __init__(self, root_dir, ext_description, svn_ignore_ancestry=False): if ext_description[comp][ExternalsDescription.REQUIRED]: self._required_compnames.append(comp) - def status(self, relative_path_base=LOCAL_PATH_INDICATOR): + def status(self, relative_path_base=LOCAL_PATH_INDICATOR, + force=False, print_progress=False): """Return a dictionary of local path->ExternalStatus. Note that all traversed components, whether recursive or top-level, have a top-level key in the returned dictionary. - FIXME(bja, 2017-10) what do we do about situations where the - user checked out the optional components, but didn't add - optional for running status? What do we do where the user - didn't add optional to the checkout but did add it to the - status. -- For now, we run status on all components, and try - to do the right thing based on the results.... - + Note that all components that are checked out locally, whether required or + optional, ar included in the returned status. """ load_comps = self._all_components.keys() summary = {} # Holds merged statuses from all components. for comp in load_comps: - printlog('{0}, '.format(comp), end='') - stat = self._all_components[comp].status() + if print_progress: + printlog('{0}, '.format(comp), end='') + stat = self._all_components[comp].status(force=force) # Returned status dictionary is keyed by local path; prepend # relative_path_base if not already there. @@ -336,6 +340,17 @@ def status(self, relative_path_base=LOCAL_PATH_INDICATOR): return summary + def _find_installed_optional_components(self): + """Returns a list of installed optional component names, if any.""" + installed_comps = set() + for comp_name, ext in self._all_components.items(): + if comp_name in self._required_compnames: + continue + # Note that in practice we expect this status to be cached. + stat = ext.status() + installed_comps.update(stat.keys()) + return list(installed_comps) + def checkout(self, verbosity, load_all, load_comp=None): """ Checkout or update indicated components into the the configured @@ -343,19 +358,25 @@ def checkout(self, verbosity, load_all, load_comp=None): If load_all is True, recursively checkout all externals. If load_all is False, load_comp is an optional set of components to load. - If load_all is True and load_comp is None, only load the required externals. + If load_all is False and load_comp is None, only checkout the required external (plus any optionals that are already checked out) + For all 3 cases, sub-externals are also recursively checked out. """ - if verbosity >= VERBOSITY_VERBOSE: - printlog('Checking out externals: ') - else: - printlog('Checking out externals: ', end='') - if load_all: tmp_comps = self._all_components.keys() elif load_comp is not None: tmp_comps = [load_comp] else: - tmp_comps = self._required_compnames + local_optional_compnames = self._find_installed_optional_components() + tmp_comps = self._required_compnames + local_optional_compnames + if local_optional_compnames: + printlog('Found locally installed optional components: ' + + ', '.join(local_optional_compnames)) + + if verbosity >= VERBOSITY_VERBOSE: + printlog('Checking out externals: ') + else: + printlog('Checking out externals: ', end='') + # Sort by path so that if paths are nested the # parent repo is checked out first. load_comps = sorted(tmp_comps, key=lambda comp: self._all_components[comp].get_local_path())