From 74427efa41d4a60aaca369d88748659b0dff24fc Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Fri, 13 Sep 2024 09:27:59 +0200 Subject: [PATCH 01/10] Add an error message to render command when lockfile doesn't exist --- conda_lock/conda_lock.py | 1 + 1 file changed, 1 insertion(+) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index f1643261..068e1d96 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -1619,6 +1619,7 @@ def render( # bail out if we do not encounter the lockfile lock_file = pathlib.Path(lock_file) if not lock_file.exists(): + print(f"ERROR: Lockfile {lock_file} does not exist.\n\n", file=sys.stderr) print(ctx.get_help()) sys.exit(1) From 1e9169e1aadbdc417d99697310f9bb3ddf3a1798 Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Fri, 13 Sep 2024 01:17:55 +0200 Subject: [PATCH 02/10] Add an error message when no source files are provided I am sometimes confused why it just prints the help message. --- conda_lock/conda_lock.py | 1 + 1 file changed, 1 insertion(+) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index 068e1d96..32f46cec 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -1351,6 +1351,7 @@ def lock( if f.exists(): break else: + logger.error("No source files provided.") print(ctx.get_help()) sys.exit(1) From c5f56d302d13134fdaa04379370002d0ac2197a6 Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Thu, 12 Sep 2024 20:08:44 +0200 Subject: [PATCH 03/10] Refactor run_lock to reconstruct environment files in helper --- conda_lock/conda_lock.py | 70 +++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index 32f46cec..3382ab47 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -1052,6 +1052,45 @@ def _detect_lockfile_kind(path: pathlib.Path) -> TKindAll: ) +def reconstruct_environment_files_from_lockfile( + lockfile_path: pathlib.Path, +) -> List[pathlib.Path]: + if lockfile_path.exists(): + lock_content = parse_conda_lock_file(lockfile_path) + # reconstruct native paths + locked_environment_files = [ + ( + pathlib.Path(p) + # absolute paths could be locked for both flavours + if pathlib.PurePosixPath(p).is_absolute() + or pathlib.PureWindowsPath(p).is_absolute() + else pathlib.Path( + pathlib.PurePosixPath(lockfile_path).parent + / pathlib.PurePosixPath(p) + ) + ) + for p in lock_content.metadata.sources + ] + if all(p.exists() for p in locked_environment_files): + environment_files = locked_environment_files + else: + missing = [p for p in locked_environment_files if not p.exists()] + environment_files = DEFAULT_FILES.copy() + print( + f"{lockfile_path} was created from {[str(p) for p in locked_environment_files]}," + f" but some files ({[str(p) for p in missing]}) do not exist. Falling back to" + f" {[str(p) for p in environment_files]}.", + file=sys.stderr, + ) + else: + long_ext_file = pathlib.Path("environment.yaml") + if long_ext_file.exists() and not DEFAULT_FILES[0].exists(): + environment_files = [long_ext_file] + else: + environment_files = DEFAULT_FILES.copy() + return environment_files + + def run_lock( environment_files: List[pathlib.Path], *, @@ -1075,36 +1114,7 @@ def run_lock( strip_auth: bool = False, ) -> None: if environment_files == DEFAULT_FILES: - if lockfile_path.exists(): - lock_content = parse_conda_lock_file(lockfile_path) - # reconstruct native paths - locked_environment_files = [ - ( - pathlib.Path(p) - # absolute paths could be locked for both flavours - if pathlib.PurePosixPath(p).is_absolute() - or pathlib.PureWindowsPath(p).is_absolute() - else pathlib.Path( - pathlib.PurePosixPath(lockfile_path).parent - / pathlib.PurePosixPath(p) - ) - ) - for p in lock_content.metadata.sources - ] - if all(p.exists() for p in locked_environment_files): - environment_files = locked_environment_files - else: - missing = [p for p in locked_environment_files if not p.exists()] - print( - f"{lockfile_path} was created from {[str(p) for p in locked_environment_files]}," - f" but some files ({[str(p) for p in missing]}) do not exist. Falling back to" - f" {[str(p) for p in environment_files]}.", - file=sys.stderr, - ) - else: - long_ext_file = pathlib.Path("environment.yaml") - if long_ext_file.exists() and not environment_files[0].exists(): - environment_files = [long_ext_file] + environment_files = reconstruct_environment_files_from_lockfile(lockfile_path) _conda_exe = determine_conda_executable( conda_exe, mamba=mamba, micromamba=micromamba From 96946facbbfdd0490c928bca0a57b25f92e798a0 Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Fri, 13 Sep 2024 00:09:10 +0200 Subject: [PATCH 04/10] Fix some type hints in lock() --- conda_lock/conda_lock.py | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index 3382ab47..ee7f77e8 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -260,7 +260,7 @@ def make_lock_files( # noqa: C901 platform_overrides: Optional[Sequence[str]] = None, channel_overrides: Optional[Sequence[str]] = None, virtual_package_spec: Optional[pathlib.Path] = None, - update: Optional[List[str]] = None, + update: Optional[Sequence[str]] = None, include_dev_dependencies: bool = True, filename_template: Optional[str] = None, filter_categories: bool = False, @@ -1095,7 +1095,7 @@ def run_lock( environment_files: List[pathlib.Path], *, conda_exe: Optional[PathLike], - platforms: Optional[List[str]] = None, + platforms: Optional[Sequence[str]] = None, mamba: bool = False, micromamba: bool = False, include_dev_dependencies: bool = True, @@ -1107,7 +1107,7 @@ def run_lock( extras: Optional[AbstractSet[str]] = None, virtual_package_spec: Optional[pathlib.Path] = None, with_cuda: Optional[str] = None, - update: Optional[List[str]] = None, + update: Optional[Sequence[str]] = None, filter_categories: bool = False, metadata_choices: AbstractSet[MetadataOption] = frozenset(), metadata_yamls: Sequence[pathlib.Path] = (), @@ -1308,25 +1308,25 @@ def lock( conda: Optional[str], mamba: bool, micromamba: bool, - platform: List[str], - channel_overrides: List[str], + platform: Sequence[str], + channel_overrides: Sequence[str], dev_dependencies: bool, - files: List[pathlib.Path], - kind: List[Union[Literal["lock"], Literal["env"], Literal["explicit"]]], + files: Sequence[PathLike], + kind: Sequence[Union[Literal["lock"], Literal["env"], Literal["explicit"]]], filename_template: str, lockfile: PathLike, strip_auth: bool, - extras: List[str], + extras: Sequence[str], filter_categories: bool, check_input_hash: bool, log_level: TLogLevel, pdb: bool, - virtual_package_spec: Optional[pathlib.Path], + virtual_package_spec: Optional[PathLike], pypi_to_conda_lookup_file: Optional[str], with_cuda: Optional[str] = None, - update: Optional[List[str]] = None, + update: Optional[Sequence[str]] = None, metadata_choices: Sequence[str] = (), - metadata_yamls: Sequence[pathlib.Path] = (), + metadata_yamls: Sequence[PathLike] = (), ) -> None: """Generate fully reproducible lock files for conda environments. @@ -1351,11 +1351,9 @@ def lock( metadata_enum_choices = set(MetadataOption(md) for md in metadata_choices) - metadata_yamls = [pathlib.Path(path) for path in metadata_yamls] - # bail out if we do not encounter the default file if no files were passed if ctx.get_parameter_source("files") == click.core.ParameterSource.DEFAULT: # type: ignore - candidates = list(files) + candidates = DEFAULT_FILES.copy() candidates += [f.with_name(f.name.replace(".yml", ".yaml")) for f in candidates] for f in candidates: if f.exists(): @@ -1368,7 +1366,7 @@ def lock( if pdb: sys.excepthook = _handle_exception_post_mortem - if not virtual_package_spec: + if virtual_package_spec is None: candidates = [ pathlib.Path("virtual-packages.yml"), pathlib.Path("virtual-packages.yaml"), @@ -1381,11 +1379,10 @@ def lock( else: virtual_package_spec = pathlib.Path(virtual_package_spec) - files = [pathlib.Path(file) for file in files] extras_ = set(extras) lock_func = partial( run_lock, - environment_files=files, + environment_files=[pathlib.Path(file) for file in files], conda_exe=conda, platforms=platform, mamba=mamba, @@ -1400,7 +1397,7 @@ def lock( update=update, filter_categories=filter_categories, metadata_choices=metadata_enum_choices, - metadata_yamls=metadata_yamls, + metadata_yamls=[pathlib.Path(path) for path in metadata_yamls], strip_auth=strip_auth, ) if strip_auth: From d1eea533b7d261becbd004065da0bb8a5781ce15 Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Fri, 13 Sep 2024 00:47:50 +0200 Subject: [PATCH 05/10] Track whether lockfile path is unspecified an emit a warning --- conda_lock/conda_lock.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index ee7f77e8..62ffaf3c 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -256,7 +256,7 @@ def make_lock_files( # noqa: C901 conda: PathLike, src_files: List[pathlib.Path], kinds: Sequence[TKindAll], - lockfile_path: pathlib.Path = pathlib.Path(DEFAULT_LOCKFILE_NAME), + lockfile_path: Optional[pathlib.Path] = None, platform_overrides: Optional[Sequence[str]] = None, channel_overrides: Optional[Sequence[str]] = None, virtual_package_spec: Optional[pathlib.Path] = None, @@ -328,6 +328,8 @@ def make_lock_files( # noqa: C901 # Load existing lockfile if it exists original_lock_content: Optional[Lockfile] = None + if lockfile_path is None: + lockfile_path = pathlib.Path(DEFAULT_LOCKFILE_NAME) if lockfile_path.exists(): try: original_lock_content = parse_conda_lock_file(lockfile_path) @@ -1053,8 +1055,11 @@ def _detect_lockfile_kind(path: pathlib.Path) -> TKindAll: def reconstruct_environment_files_from_lockfile( - lockfile_path: pathlib.Path, + lockfile_path: Optional[pathlib.Path], ) -> List[pathlib.Path]: + is_lockfile_specified = lockfile_path is not None + if lockfile_path is None: + lockfile_path = pathlib.Path(DEFAULT_LOCKFILE_NAME) if lockfile_path.exists(): lock_content = parse_conda_lock_file(lockfile_path) # reconstruct native paths @@ -1073,6 +1078,12 @@ def reconstruct_environment_files_from_lockfile( ] if all(p.exists() for p in locked_environment_files): environment_files = locked_environment_files + if not is_lockfile_specified: + logger.warning( + f"Using source files {[str(p) for p in locked_environment_files]} " + f"from implicitly-specified {lockfile_path} to create the " + f"environment." + ) else: missing = [p for p in locked_environment_files if not p.exists()] environment_files = DEFAULT_FILES.copy() @@ -1102,7 +1113,7 @@ def run_lock( channel_overrides: Optional[Sequence[str]] = None, filename_template: Optional[str] = None, kinds: Optional[Sequence[TKindAll]] = None, - lockfile_path: pathlib.Path = pathlib.Path(DEFAULT_LOCKFILE_NAME), + lockfile_path: Optional[pathlib.Path] = None, check_input_hash: bool = False, extras: Optional[AbstractSet[str]] = None, virtual_package_spec: Optional[pathlib.Path] = None, @@ -1214,7 +1225,7 @@ def main() -> None: ) @click.option( "--lockfile", - default=DEFAULT_LOCKFILE_NAME, + default=None, help="Path to a conda-lock.yml to create or update", ) @click.option( @@ -1314,7 +1325,7 @@ def lock( files: Sequence[PathLike], kind: Sequence[Union[Literal["lock"], Literal["env"], Literal["explicit"]]], filename_template: str, - lockfile: PathLike, + lockfile: Optional[PathLike], strip_auth: bool, extras: Sequence[str], filter_categories: bool, @@ -1390,7 +1401,7 @@ def lock( include_dev_dependencies=dev_dependencies, channel_overrides=channel_overrides, kinds=kind, - lockfile_path=pathlib.Path(lockfile), + lockfile_path=None if lockfile is None else pathlib.Path(lockfile), extras=extras_, virtual_package_spec=virtual_package_spec, with_cuda=with_cuda, From 0ca62aed9016385d641d37d0c5106ae516f618ab Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Fri, 13 Sep 2024 01:06:12 +0200 Subject: [PATCH 06/10] Narrow update for mypy --- conda_lock/conda_lock.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index 62ffaf3c..272cf410 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -358,6 +358,9 @@ def make_lock_files( # noqa: C901 platforms_already_locked: List[str] = [] if original_lock_content is not None: platforms_already_locked = list(original_lock_content.metadata.platforms) + if update is not None: + # Narrow `update` sequence to list for mypy + update = list(update) update_spec = UpdateSpecification( locked=original_lock_content.package, update=update ) From b471f7f50a2f7f823c9e1fc04f8bbaf6ecab97ff Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Fri, 13 Sep 2024 15:37:01 +0200 Subject: [PATCH 07/10] Don't immediately set source files to default Leave it as an empty list, at least in the beginning. --- conda_lock/conda_lock.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index 272cf410..16590b4c 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -1208,7 +1208,6 @@ def main() -> None: "-f", "--file", "files", - default=DEFAULT_FILES, type=click.Path(), multiple=True, help="path to a conda environment specification(s)", @@ -1366,7 +1365,7 @@ def lock( metadata_enum_choices = set(MetadataOption(md) for md in metadata_choices) # bail out if we do not encounter the default file if no files were passed - if ctx.get_parameter_source("files") == click.core.ParameterSource.DEFAULT: # type: ignore + if len(files) == 0: candidates = DEFAULT_FILES.copy() candidates += [f.with_name(f.name.replace(".yml", ".yaml")) for f in candidates] for f in candidates: @@ -1376,6 +1375,7 @@ def lock( logger.error("No source files provided.") print(ctx.get_help()) sys.exit(1) + files = DEFAULT_FILES.copy() if pdb: sys.excepthook = _handle_exception_post_mortem From d404f2170e8204e0420d23a3d6e763a3e10d2f68 Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Fri, 13 Sep 2024 15:45:33 +0200 Subject: [PATCH 08/10] Propagate empty source list a little bit further --- conda_lock/conda_lock.py | 10 +++++++--- tests/test_conda_lock.py | 7 ++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index 16590b4c..d1ffdec3 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -1060,6 +1060,10 @@ def _detect_lockfile_kind(path: pathlib.Path) -> TKindAll: def reconstruct_environment_files_from_lockfile( lockfile_path: Optional[pathlib.Path], ) -> List[pathlib.Path]: + """No sources were specified on the CLI, so try to read them from the lockfile. + + If none are found, then fall back to the default files. + """ is_lockfile_specified = lockfile_path is not None if lockfile_path is None: lockfile_path = pathlib.Path(DEFAULT_LOCKFILE_NAME) @@ -1127,7 +1131,7 @@ def run_lock( metadata_yamls: Sequence[pathlib.Path] = (), strip_auth: bool = False, ) -> None: - if environment_files == DEFAULT_FILES: + if len(environment_files) == 0: environment_files = reconstruct_environment_files_from_lockfile(lockfile_path) _conda_exe = determine_conda_executable( @@ -1375,7 +1379,7 @@ def lock( logger.error("No source files provided.") print(ctx.get_help()) sys.exit(1) - files = DEFAULT_FILES.copy() + environment_files = [pathlib.Path(file) for file in files] if pdb: sys.excepthook = _handle_exception_post_mortem @@ -1396,7 +1400,7 @@ def lock( extras_ = set(extras) lock_func = partial( run_lock, - environment_files=[pathlib.Path(file) for file in files], + environment_files=environment_files, conda_exe=conda, platforms=platform, mamba=mamba, diff --git a/tests/test_conda_lock.py b/tests/test_conda_lock.py index 17a92b97..858aef4c 100644 --- a/tests/test_conda_lock.py +++ b/tests/test_conda_lock.py @@ -30,7 +30,6 @@ from conda_lock import __version__, pypi_solver from conda_lock.conda_lock import ( - DEFAULT_FILES, DEFAULT_LOCKFILE_NAME, _add_auth_to_line, _add_auth_to_lockfile, @@ -1446,7 +1445,7 @@ def test_run_lock_with_locked_environment_files( run_lock([pre_environment], conda_exe="mamba") make_lock_files = MagicMock() monkeypatch.setattr("conda_lock.conda_lock.make_lock_files", make_lock_files) - run_lock(DEFAULT_FILES, conda_exe=conda_exe, update=["pydantic"]) + run_lock([], conda_exe=conda_exe, update=["pydantic"]) src_files = make_lock_files.call_args.kwargs["src_files"] assert [p.resolve() for p in src_files] == [ @@ -1473,9 +1472,7 @@ def test_run_lock_relative_source_path( assert Path(locked_environment) == Path("../sources/environment.yaml") make_lock_files = MagicMock() monkeypatch.setattr("conda_lock.conda_lock.make_lock_files", make_lock_files) - run_lock( - DEFAULT_FILES, lockfile_path=lockfile, conda_exe=conda_exe, update=["pydantic"] - ) + run_lock([], lockfile_path=lockfile, conda_exe=conda_exe, update=["pydantic"]) src_files = make_lock_files.call_args.kwargs["src_files"] assert [p.resolve() for p in src_files] == [environment.resolve()] From 3626064cfe521b4d7951827e22e2558174a2fca9 Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Fri, 13 Sep 2024 15:50:28 +0200 Subject: [PATCH 09/10] Consolidate logic for handling no specified source files --- conda_lock/conda_lock.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index d1ffdec3..453b4b36 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -1057,13 +1057,23 @@ def _detect_lockfile_kind(path: pathlib.Path) -> TKindAll: ) -def reconstruct_environment_files_from_lockfile( +def handle_no_specified_source_files( lockfile_path: Optional[pathlib.Path], ) -> List[pathlib.Path]: """No sources were specified on the CLI, so try to read them from the lockfile. If none are found, then fall back to the default files. """ + # bail out if we do not encounter the default file if no files were passed + candidates = DEFAULT_FILES.copy() + candidates += [f.with_name(f.name.replace(".yml", ".yaml")) for f in candidates] + for f in candidates: + if f.exists(): + break + else: + logger.error("No source files provided.") + sys.exit(1) + is_lockfile_specified = lockfile_path is not None if lockfile_path is None: lockfile_path = pathlib.Path(DEFAULT_LOCKFILE_NAME) @@ -1132,7 +1142,7 @@ def run_lock( strip_auth: bool = False, ) -> None: if len(environment_files) == 0: - environment_files = reconstruct_environment_files_from_lockfile(lockfile_path) + environment_files = handle_no_specified_source_files(lockfile_path) _conda_exe = determine_conda_executable( conda_exe, mamba=mamba, micromamba=micromamba @@ -1368,17 +1378,6 @@ def lock( metadata_enum_choices = set(MetadataOption(md) for md in metadata_choices) - # bail out if we do not encounter the default file if no files were passed - if len(files) == 0: - candidates = DEFAULT_FILES.copy() - candidates += [f.with_name(f.name.replace(".yml", ".yaml")) for f in candidates] - for f in candidates: - if f.exists(): - break - else: - logger.error("No source files provided.") - print(ctx.get_help()) - sys.exit(1) environment_files = [pathlib.Path(file) for file in files] if pdb: From 69a98201840cda388945abc835eeffcd22547699 Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Fri, 13 Sep 2024 16:03:09 +0200 Subject: [PATCH 10/10] Clean up logic for handling no specified source files --- conda_lock/conda_lock.py | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/conda_lock/conda_lock.py b/conda_lock/conda_lock.py index 453b4b36..f9edd810 100644 --- a/conda_lock/conda_lock.py +++ b/conda_lock/conda_lock.py @@ -83,7 +83,7 @@ logger = logging.getLogger(__name__) -DEFAULT_FILES = [pathlib.Path("environment.yml")] +DEFAULT_FILES = [pathlib.Path("environment.yml"), pathlib.Path("environment.yaml")] # Captures basic auth credentials, if they exists, in the third capture group. AUTH_PATTERN = re.compile(r"^(# pip .* @ )?(https?:\/\/)(.*:.*@)?(.*)") @@ -1064,17 +1064,6 @@ def handle_no_specified_source_files( If none are found, then fall back to the default files. """ - # bail out if we do not encounter the default file if no files were passed - candidates = DEFAULT_FILES.copy() - candidates += [f.with_name(f.name.replace(".yml", ".yaml")) for f in candidates] - for f in candidates: - if f.exists(): - break - else: - logger.error("No source files provided.") - sys.exit(1) - - is_lockfile_specified = lockfile_path is not None if lockfile_path is None: lockfile_path = pathlib.Path(DEFAULT_LOCKFILE_NAME) if lockfile_path.exists(): @@ -1095,12 +1084,10 @@ def handle_no_specified_source_files( ] if all(p.exists() for p in locked_environment_files): environment_files = locked_environment_files - if not is_lockfile_specified: - logger.warning( - f"Using source files {[str(p) for p in locked_environment_files]} " - f"from implicitly-specified {lockfile_path} to create the " - f"environment." - ) + logger.warning( + f"Using source files {[str(p) for p in locked_environment_files]} " + f"from {lockfile_path} to create the environment." + ) else: missing = [p for p in locked_environment_files if not p.exists()] environment_files = DEFAULT_FILES.copy() @@ -1111,11 +1098,16 @@ def handle_no_specified_source_files( file=sys.stderr, ) else: - long_ext_file = pathlib.Path("environment.yaml") - if long_ext_file.exists() and not DEFAULT_FILES[0].exists(): - environment_files = [long_ext_file] - else: - environment_files = DEFAULT_FILES.copy() + # No lockfile provided, so fall back to the default files + environment_files = [f for f in DEFAULT_FILES if f.exists()] + if len(environment_files) == 0: + logger.error( + "No source files provided and no default files found. Exiting." + ) + sys.exit(1) + elif len(environment_files) > 1: + logger.error(f"Multiple default files found: {environment_files}. Exiting.") + sys.exit(1) return environment_files