Skip to content

Conversation

jezdez
Copy link
Member

@jezdez jezdez commented May 9, 2025

Description

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@jezdez jezdez requested a review from a team as a code owner May 9, 2025 13:20
@github-project-automation github-project-automation bot moved this to 🆕 New in 🔎 Review May 9, 2025
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label May 9, 2025
@jezdez jezdez moved this from 🆕 New to 🏗️ In Progress in 🔎 Review May 9, 2025
Copy link

codspeed-hq bot commented May 9, 2025

CodSpeed Performance Report

Merging #14820 will not alter performance

Comparing explicit-env-spec (1d301b4) with main (075c8ea)

Summary

✅ 21 untouched benchmarks

@jjhelmus
Copy link
Contributor

jjhelmus commented May 9, 2025

Creating an environment using this plugin requires a solve. A solve is not required when using conda create --file explicit.txt. Should this be documented or mentioned to users?

Additionally CEP-23 states:

When an explicit input file is processed, the conda client SHOULD NOT invoke a solver.

@jjhelmus
Copy link
Contributor

jjhelmus commented May 9, 2025

Testing this out it looks like all explicit files are detected as being handled by the requirements.txt plugin as well.

❯ cat explicit_single.txt 
@EXPLICIT
https://conda.anaconda.org/conda-forge/osx-64/libcurl-7.69.1-hc0b9707_0.tar.bz2

❯ conda env create --prefix blah --file explicit_single.txt

PluginError: Too many plugins found that can handle the environment file 'explicit_single.txt':

explicit, requirements.txt

Please make sure that you don't have any overlapping plugins installed.

@jezdez
Copy link
Member Author

jezdez commented May 9, 2025

Yeah, something is clearly wrong, will fix!

@jezdez jezdez self-assigned this May 9, 2025
@jezdez jezdez changed the title Implement explicit environment spec file support based on CEP-23. Implement explicit environment spec file support based on CEP 23. May 9, 2025
@jezdez jezdez marked this pull request as draft May 9, 2025 22:09
Comment on lines 86 to 88
# Handle explicit environments separately per CEP-23 requirements
if isinstance(env, ExplicitEnvironment):
return _install_explicit_environment(prefix, specs, env)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a special case but one that will come up for any plugins that reads a definition for a locked environment (including pixi.lock or conda-lock.yml files). In these cases a solve is unnecessary, should not be performed and in some cases (e.g. packages outside of a channel are referenced) may not be possible. I think providing a path for plugins to express this information would be useful but likely requires an expansion of the Environment class.

For pixi.lock and conda-lock.yml, parsing can return a set of PackageRecord instances for the conda requirements, where as requirement and environment.yaml files would return a set of MatchSpec instances.

Copy link
Member Author

@jezdez jezdez May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have this information being a metadata in the CondaEnvironmentSpecifier class?

@dataclass
class CondaEnvironmentSpecifier:
    """
    Return type to use when defining a conda env spec plugin hook.

    For details on how this is used, see
    :meth:`~conda.plugins.hookspec.CondaSpecs.conda_environment_specifiers`.

    :param name: name of the spec (e.g., ``environment_yaml``)
    :param environment_spec: EnvironmentSpecBase subclass handler
    :param requires_solve: Whether or not the plugin requires a solve
    """

    name: str
    environment_spec: type[EnvironmentSpecBase]
    requires_solve: bool

and then

@hookimpl
def conda_environment_specifiers():
    yield CondaEnvironmentSpecifier(
        name="explicit",
        environment_spec=ExplicitRequirementsSpec,
        requires_solve=False,
    )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option would be to attaching this metadata to the Environment class itself which is returned from EnvironmentSpecBase.environment method.

Either option is reasonable and will allow for plugins for various lock formats.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I think given that we may have an evolved environment class in the future, it might make sense to have it there, instead of the plugin interface. Thanks for the idea!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, some time has passed, I'm not sure if we should do this in this PR given @soapy1's work on the Environment class. Would you be okay to punt on this @jjhelmus @soapy1?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need to get this feature ASAP? If not I'd prefer to wait until the Environment class is merged. Once that work is done this can be re-worked to use that class.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think we've been stalling on the features that were already well-defined, while the environment class refactor has been ballooning already. This branch here works now, and is a good snapshot in time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaimergp that's the similar conversation you had above, I'll change to the boolean in the environment class

Copy link
Contributor

@soapy1 soapy1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! This works when I (pull in the changes from #14725 and) try to reproduce the issue described in #14725 (comment).

@jezdez
Copy link
Member Author

jezdez commented May 13, 2025

Neat! This works when I (pull in the changes from #14725 and) try to reproduce the issue described in #14725 (comment).

I wonder if we should merge those PRs together?

@soapy1
Copy link
Contributor

soapy1 commented May 13, 2025

I wonder if we should merge those PRs together?

For me, its ok if they are seperate since they implementation doesn't overlap and they have 2 distinct goals. But, if you think it would be helpful to merge them, that sounds fine as well.

@jezdez jezdez marked this pull request as ready for review May 13, 2025 23:38
@jezdez jezdez requested review from jjhelmus, soapy1 and jaimergp and removed request for soapy1 May 28, 2025 16:18
@soapy1
Copy link
Contributor

soapy1 commented May 28, 2025

In general, I think this looks good to me and seems to work in all the ways I expect it to. I want to call out 2 other things happening in parallel here:

  1. once the environment model stuff get's merged we'll want to change the environment spec plugins to be using that environment opposed to the conda.env.env.Environment. I think this change is fully compatible with what is happening in this PR. But merging this first will add to the work required to move over to that desired final state of all plugins using the new environment model (I think this is an ok situation to be in).

  2. @jjhelmus's pr for implementing installing from a lockfile proposes another way of installing explicit packages. Is this the direction we want to go in for installing packages directly? If so, is this PR compatible with being able to move over to this new method?

jezdez and others added 19 commits June 24, 2025 18:15
Co-authored-by: Mahe Iram Khan <65779580+ForgottenProgramme@users.noreply.github.com>
Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
- Introduced `is_url` and `expand` functions for improved URL normalization and path expansion.
- Updated logic to handle file paths and URLs consistently, ensuring compatibility with existing specifications.
- Added validation for URL format using regex, maintaining backward compatibility while improving parsing accuracy.
- Removed the ExplicitEnvironment class and integrated its functionality into the Environment class.
- Updated the Dependencies class to include an explicit marker for environments created from @explicit files.
- Adjusted the conda installer and requirements specifications to work with the new Environment structure.
- Modified tests to reflect changes in the environment handling, ensuring compatibility with explicit specifications.
…cations

- Replaced the ExplicitRequirementsSpec with a unified RequirementsSpec that handles both regular and explicit requirements files.
- Updated the reading logic to utilize a generator for improved efficiency and clarity.
- Adjusted tests to reflect the new unified handling of requirements, ensuring compatibility with explicit specifications.
- Removed deprecated functions and streamlined the codebase for better maintainability.
- Integrated explicit environment handling directly into the install function, removing the need for a separate _install_explicit_environment function.
- Enhanced logging to provide detailed feedback on the source of package specifications used during installation.
- Improved the logic for determining package specs by prioritizing original files, provided specs, and environment dependencies.
- Ensured compatibility with the latest requirements handling and streamlined the installation process for explicit environments.
- Removed the _execute_transaction function and integrated its logic directly into the install function.
- Enhanced the transaction handling by checking for 'nothing to do' before executing.
- Streamlined the process of downloading, extracting, and executing transactions, improving clarity and maintainability.
Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
…ng for better performance and readability.

- Removed unnecessary comments and improved the `yield_lines` function in `read.py` for cleaner code.
- Introduced `test_explicit_marker_in_middle` to verify functionality of the @explicit marker when placed in the middle of the dependency list.
- Added `test_explicit_marker_at_end` to ensure the @explicit marker works correctly when positioned at the end of the dependencies.
- Updated the `explicit` function to track user-requested specs separately from all packages, preventing dependencies from polluting the user's request history.
- Added type hints for parameters in `explicit`, `_solve`, `dry_run`, and `install` functions to improve code clarity and type checking.
- Modified test cases to ensure correct handling of the `@EXPLICIT` marker in various positions within the dependency list.
- Removed redundant fixture and consolidated tests for explicit environments.
- Added new tests to verify the detection of explicit dependencies and the handling of the @explicit marker in various positions within the dependency list.
- Improved type hinting for test functions and ensured consistent assertions for environment properties.
- Updated assertions in `test_explicit_file_spec_is_registered` to improve readability and ensure correct handling of dependencies.
- Replaced direct access to `env.dependencies["conda"]` with a more general approach using `env.dependencies.raw`, enhancing flexibility in dependency checks.
@jezdez jezdez force-pushed the explicit-env-spec branch from 33b11f7 to 58fb7dc Compare June 24, 2025 16:20
@jezdez jezdez mentioned this pull request Jun 24, 2025
3 tasks
@jezdez jezdez requested a review from soapy1 June 24, 2025 16:22
jezdez added 2 commits June 24, 2025 22:13
…ested specs are recorded, preventing dependencies from affecting the user's request history.

Also enhanced the handling of requested specs by using `MatchSpec` for better specification matching.
@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Approved in 🔎 Review Jun 25, 2025
@jezdez jezdez merged commit 8179ddb into main Jun 25, 2025
75 checks passed
@jezdez jezdez deleted the explicit-env-spec branch June 25, 2025 13:25
@github-project-automation github-project-automation bot moved this from ✅ Approved to 🏁 Done in 🔎 Review Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add environment spec plugin for explicit input file types Add environment specifier plugin support for explicit input files (CEP23)
6 participants