Skip to content

Commit

Permalink
Fix broken checkout behavior introduced by PR ESCOMP#172.
Browse files Browse the repository at this point in the history
Finding locally installed optional components was broken both because it did not correctly inspect the status of those components, and because it added components-to-checkout by path instead of by name.

Printing status updates also a bit wonky because 'print_progress' was not correctly passed down.

Slight update to testing docs:  no need to run python 2 and 3 separately, and system tests now exist.
  • Loading branch information
johnpaulalex committed Jan 9, 2023
1 parent 27909e2 commit 5665d61
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 35 deletions.
4 changes: 3 additions & 1 deletion manic/checkout.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,10 @@ def main(args):

for comp in args.components:
if comp not in ext_description.keys():
# Note we can't print out the list of found externals because
# they were filtered in create_externals_description above.
fatal_error(
"No component {} found in {}".format(
"No component {} found in {}.".format(
comp, args.externals))

source_tree = SourceTree(root_dir, ext_description, svn_ignore_ancestry=args.svn_ignore_ancestry)
Expand Down
41 changes: 24 additions & 17 deletions manic/sourcetree.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
0;10;1c
FIXME(bja, 2017-11) External and SourceTree have a circular dependancy!
"""

Expand Down Expand Up @@ -98,12 +98,14 @@ def get_local_path(self):
"""
return self._local_path

def status(self, force=False):
def status(self, force=False, print_progress=False):
"""
Returns status of this component and all subcomponents (if available).
Returns status of this component and all subcomponents.
Returns a dict mapping our local path to an ExternalStatus dict. Any
subcomponents will have their own top-level key.
Returns a dict mapping our local path (not component name!) to an
ExternalStatus dict. Any subcomponents will have their own top-level
path keys. Note the return value includes entries for this and all
subcomponents regardless of whether they are locally installed or not.
Side-effect: If self._stat is empty or force is True, calculates _stat.
"""
Expand Down Expand Up @@ -151,7 +153,7 @@ def status(self, force=False):
# 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, force=force)
subcomponent_stats = self._externals_sourcetree.status(self._local_path, force=force, print_progress=print_progress)
os.chdir(cwd)

# Merge our status + subcomponent statuses into one return dict keyed
Expand Down Expand Up @@ -311,19 +313,22 @@ 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.
Note that all components that are checked out locally, whether required or
optional, ar included in the returned status.
"""
Notes about the returned dictionary:
* It is keyed by local path (e.g. 'components/mom'), not by
component name (e.g. 'mom').
* It contains top-level keys for all traversed components, whether
discovered by recursion or top-level.
* It contains entries for all components regardless of whether they
are locally installed or not, or required or optional.
x """
load_comps = self._all_components.keys()

summary = {} # Holds merged statuses from all components.
for comp in load_comps:
if print_progress:
printlog('{0}, '.format(comp), end='')
stat = self._all_components[comp].status(force=force)
stat = self._all_components[comp].status(force=force,
print_progress=print_progress)

# Returned status dictionary is keyed by local path; prepend
# relative_path_base if not already there.
Expand All @@ -342,14 +347,16 @@ def status(self, relative_path_base=LOCAL_PATH_INDICATOR,

def _find_installed_optional_components(self):
"""Returns a list of installed optional component names, if any."""
installed_comps = set()
installed_comps = []
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)
path_to_stat = ext.status()
if any(stat.sync_state != ExternalStatus.EMPTY
for stat in path_to_stat.values()):
installed_comps.append(comp_name)
return installed_comps

def checkout(self, verbosity, load_all, load_comp=None):
"""
Expand Down
20 changes: 3 additions & 17 deletions test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,18 @@ Development environments should be setup for python2 and python3:

## Unit tests

Tests should be run for both python2 and python3. It is recommended
that you have seperate terminal windows open python2 and python3
testing to avoid errors activating and deactivating environments.

```SH
cd checkout_externals/test
. env_python2/bin/activate
make utest
deactivate
```

## System tests

```SH
cd checkout_externals/test
. env_python2/bin/activate
make utest
deactivate
make stest
```

## System tests

Not yet implemented.

## Static analysis

checkout_externals is difficult to test thoroughly because it relies
Expand All @@ -51,9 +41,7 @@ regularly for automatic code formatting and linting.

```SH
cd checkout_externals/test
. env_python2/bin/activate
make lint
deactivate
```

The canonical formatting for the code is whatever autopep8
Expand All @@ -68,10 +56,8 @@ coverage, run the code coverage tool:

```SH
cd checkout_externals/test
. env_python2/bin/activate
make coverage
open -a Firefox.app htmlcov/index.html
deactivate
```


0 comments on commit 5665d61

Please sign in to comment.