diff --git a/swh/loader/package/opam/loader.py b/swh/loader/package/opam/loader.py --- a/swh/loader/package/opam/loader.py +++ b/swh/loader/package/opam/loader.py @@ -64,6 +64,7 @@ Then we just ask the opam binary to give us the list of all versions of the given package. For each version, we ask the opam binary to give us the url to the tarball to archive. + """ visit_type = "opam" @@ -77,6 +78,7 @@ opam_url: str, opam_package: str, max_content_size: Optional[int] = None, + opam_maintenance: bool = True, ): super().__init__(storage=storage, url=url, max_content_size=max_content_size) @@ -84,6 +86,7 @@ self.opam_instance = opam_instance self.opam_url = opam_url self.opam_package = opam_package + self.opam_maintenance = opam_maintenance def get_package_dir(self) -> str: return ( @@ -128,33 +131,36 @@ return versions def get_versions(self) -> List[str]: - """First initialize the opam root directory if needed then start listing the package - versions. + """First initialize the opam root directory if needed then start listing the + package versions. Raises: ValueError in case the lister is not able to determine the list of versions or if the opam root directory is invalid. """ - if not os.path.isdir(self.opam_root): - if os.path.isfile(self.opam_root): + if not self.opam_maintenance: # standard loader, no need to maintain opam root + if ( + os.path.isfile(self.opam_root) + or not os.path.isdir(self.opam_root) + or not os.path.isfile(os.path.join(self.opam_root, "config")) + ): + # so if not correctly setup, raise immediately raise ValueError("invalid opam root") - else: - call( - [ - "opam", - "init", - "--reinit", - "--bare", - "--no-setup", - "--root", - self.opam_root, - self.opam_instance, - self.opam_url, - ] - ) - elif not os.path.isfile(os.path.join(self.opam_root, "config")): - raise ValueError("invalid opam root") + else: # for standalone loader (e.g docker), allow loader to do init work + call( + [ + "opam", + "init", + "--reinit", + "--bare", + "--no-setup", + "--root", + self.opam_root, + self.opam_instance, + self.opam_url, + ] + ) return self._compute_versions() diff --git a/swh/loader/package/opam/tests/test_opam.py b/swh/loader/package/opam/tests/test_opam.py --- a/swh/loader/package/opam/tests/test_opam.py +++ b/swh/loader/package/opam/tests/test_opam.py @@ -12,6 +12,31 @@ from swh.model.model import Person, Snapshot, SnapshotBranch, TargetType +def test_opam_loader_no_opam_repository_fails(swh_storage, tmpdir, datadir): + """Running opam loader without a prepared opam repository fails""" + opam_url = f"file://{datadir}/fake_opam_repo" + opam_root = tmpdir + opam_instance = "loadertest" + opam_package = "agrid" + url = f"opam+{opam_url}/packages/{opam_package}" + + loader = OpamLoader( + swh_storage, + url, + opam_root, + opam_instance, + opam_url, + opam_package, + opam_maintenance=False, # No opam root directory init directory from loader + ) + + # So if it's not taken care of externally, the loading fails That's the expected use + # for the production workers (whose maintenance will be external). + actual_load_status = loader.load() + + assert actual_load_status == {"status": "failed"} + + def test_opam_loader_one_version(tmpdir, requests_mock_datadir, datadir, swh_storage): opam_url = f"file://{datadir}/fake_opam_repo"