diff --git a/swh/loader/svn/replay.py b/swh/loader/svn/replay.py --- a/swh/loader/svn/replay.py +++ b/swh/loader/svn/replay.py @@ -508,22 +508,35 @@ ) self.externals = defaultdict(list) if value is not None: - # externals are set on that directory path, parse and store them - # for later processing in the close method - for external in value.split("\n"): - external = external.rstrip("\r") - # skip empty line or comment - if not external or external.startswith("#"): - continue - ( - path, - external_url, - revision, - relative_url, - ) = parse_external_definition( - external, os.fsdecode(self.path), self.svnrepo.origin_url + try: + # externals are set on that directory path, parse and store them + # for later processing in the close method + for external in value.split("\n"): + external = external.rstrip("\r") + # skip empty line or comment + if not external or external.startswith("#"): + continue + ( + path, + external_url, + revision, + relative_url, + ) = parse_external_definition( + external, os.fsdecode(self.path), self.svnrepo.origin_url + ) + self.externals[path].append( + (external_url, revision, relative_url) + ) + except ValueError: + logger.debug( + "Failed to parse external: %s\n" + "Externals defined on path %s will not be processed", + external, + self.path, ) - self.externals[path].append((external_url, revision, relative_url)) + # as the official subversion client, do not process externals in case + # of parsing error + self.externals = {} if not self.externals: # externals might have been unset on that directory path, diff --git a/swh/loader/svn/tests/test_externals.py b/swh/loader/svn/tests/test_externals.py --- a/swh/loader/svn/tests/test_externals.py +++ b/swh/loader/svn/tests/test_externals.py @@ -1482,3 +1482,94 @@ type="svn", ) check_snapshot(loader.snapshot, loader.storage) + + +def test_loader_with_externals_parsing_error( + swh_storage, repo_url, external_repo_url, tmp_path +): + # first commit on external + add_commit( + external_repo_url, + "Create code directory", + [ + CommitChange( + change_type=CommitChangeType.AddOrUpdate, + path="code/", + ), + ], + ) + + # second commit on external + add_commit( + external_repo_url, + "Create code/foo.sh file", + [ + CommitChange( + change_type=CommitChangeType.AddOrUpdate, + path="code/foo.sh", + properties={"svn:executable": "*"}, + data=b"#!/bin/bash\necho foo", + ), + ], + ) + + # first commit + add_commit( + repo_url, + "Create trunk directory.", + [ + CommitChange( + change_type=CommitChangeType.AddOrUpdate, + path="trunk/", + ), + ], + ) + + # second commit + add_commit( + repo_url, + "Set external on trunk directory that will result in a parsing error.", + [ + CommitChange( + change_type=CommitChangeType.AddOrUpdate, + path="trunk/", + properties={ + "svn:externals": ( + f"-r2{svn_urljoin(external_repo_url, 'code/foo.sh')} foo.sh" + ) + }, + ), + ], + ) + + # third commit + add_commit( + repo_url, + "Fix external definition on trunk directory.", + [ + CommitChange( + change_type=CommitChangeType.AddOrUpdate, + path="trunk/", + properties={ + "svn:externals": ( + f"-r2 {svn_urljoin(external_repo_url, 'code/foo.sh')} foo.sh" + ) + }, + ), + ], + ) + + loader = SvnLoader( + swh_storage, + repo_url, + temp_directory=tmp_path, + check_revision=1, + ) + assert loader.load() == {"status": "eventful"} + assert_last_visit_matches( + loader.storage, + repo_url, + status="full", + type="svn", + ) + check_snapshot(loader.snapshot, loader.storage)