Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Export conda environment.yml #2003

Merged
merged 23 commits into from
Sep 17, 2024

Conversation

abkfenris
Copy link
Contributor

@abkfenris abkfenris commented Sep 7, 2024

Builds upon #1873 to add exporting of environment.yml files.

Since #1873 provides the conda-lock style matrix of exports, I focused on exporting an environment.yml that matches the manifest.

Tests against a bunch of the different example manifests for the various pypi-options, and I tested locally that at least some of those were installable with micromamba.

Replaces #1427

synapticarbors and others added 8 commits August 21, 2024 13:22
Builds upon prefix-dev#1873 to add exporting of `environment.yml` files.

Since prefix-dev#1873 provides the conda-lock style matrix of exports, I focused on exporting an environment.yml that matches the manifest.

Tests against a bunch of the different example manifests for the various pypi-options, and I tested locally that at least some of those were installable with micromamba.

Replaces prefix-dev#1427
@abkfenris
Copy link
Contributor Author

abkfenris commented Sep 7, 2024

Thanks to @synapticarbors for showing how to do things cleaner and test them as well!

Depends on conda/rattler#855 being released It's released, and main was updated to use the new version, so we're good there.

# Conflicts:
#	Cargo.lock
#	Cargo.toml
#	docs/reference/cli.md
#	src/cli/project/export/conda_explicit_spec.rs
#	src/cli/project/export/mod.rs
@abkfenris abkfenris marked this pull request as ready for review September 11, 2024 00:42
@abkfenris
Copy link
Contributor Author

Ehh, maybe not quite yet. Found at least one of the examples that panics.

@abkfenris
Copy link
Contributor Author

All the examples export now.

Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

If you have an idea to test the actual file for mamba compatibility that would be great!

src/cli/project/export/mod.rs Outdated Show resolved Hide resolved
docs/reference/cli.md Outdated Show resolved Hide resolved
docs/reference/cli.md Outdated Show resolved Hide resolved
docs/reference/cli.md Outdated Show resolved Hide resolved
src/cli/project/export/conda_environment.rs Outdated Show resolved Hide resolved
#[clap(arg_required_else_help = false)]
pub struct Args {
/// Explicit path to export the environment to
pub output_path: Option<PathBuf>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of the default behavior, could you make that for pixi project export ces to? As I would like these interfaces to be similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mind if that waits for a separate PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

One question here is the requested design for ces was to export by default a file per platform/env. Here there is a single output so without an output_path it just prints to stdout. For ces what would the behavior be if no output directory is specified given that without a --platform or --env flag it's going to try to write for all combinations of platform and env?

src/cli/project/export/conda_environment.rs Outdated Show resolved Hide resolved
src/cli/project/export/conda_environment.rs Outdated Show resolved Hide resolved
@abkfenris
Copy link
Contributor Author

If you have an idea to test the actual file for mamba compatibility that would be great!

I believe I tested all test outputs but the custom registry with micromamba locally. Are you thinking of automating some tests?

@ruben-arts
Copy link
Contributor

Are you thinking of automating some tests?

That would be the best yes!

@abkfenris
Copy link
Contributor Author

Are you thinking of automating some tests?

That would be the best yes!

Hmm, would the example tests be a reasonable spot to start from? Or is there a better example of orchestrating tests across multiple tools that I should model from?

  • https://github.com/prefix-dev/pixi/blob/main/tests/test_examples.sh
  • # Test all of the examples
    test-examples:
    name: ${{ matrix.arch.name }} - Test Examples
    runs-on: ${{ matrix.arch.os }}
    strategy:
    fail-fast: false
    matrix:
    arch:
    # Linux
    - {
    target: x86_64-unknown-linux-musl,
    os: ubuntu-20.04,
    name: "Linux",
    }
    # MacOS
    - { target: x86_64-apple-darwin, os: macos-13, name: "MacOS-x86" }
    - { target: aarch64-apple-darwin, os: macos-14, name: "MacOS-arm" } # macOS-14 is the ARM chipset
    # Windows
    - {
    target: x86_64-pc-windows-msvc,
    os: windows-latest,
    extension: .exe,
    name: "Windows",
    }
    steps:
    - name: checkout repo
    uses: actions/checkout@v4
    - name: Download binary from build
    uses: actions/download-artifact@v4
    with:
    name: pixi-${{ matrix.arch.target }}${{ matrix.arch.extension }}
    path: pixi_bin
    - name: Debug
    run: |
    pwd
    - name: Setup unix binary, add to github path
    if: matrix.arch.name != 'Windows'
    run: |
    mv pixi_bin/pixi-${{ matrix.arch.target }} pixi_bin/pixi
    chmod a+x pixi_bin/pixi
    echo "$(pwd)/pixi_bin" >> $GITHUB_PATH
    - name: Create Directory and Move Executable
    if: matrix.arch.name == 'Windows' && matrix.arch.target == 'x86_64-pc-windows-msvc'
    run: |
    New-Item -ItemType Directory -Force -Path "D:\.pixi"
    Move-Item -Path "pixi_bin/pixi-${{ matrix.arch.target }}${{ matrix.arch.extension }}" -Destination "D:\.pixi\pixi.exe"
    shell: pwsh
    - name: Add to PATH
    if: matrix.arch.name == 'Windows' && matrix.arch.target == 'x86_64-pc-windows-msvc'
    run: echo "D:\.pixi" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_PATH
    shell: pwsh
    - name: Verify and Use Executable
    if: matrix.arch.name == 'Windows' && matrix.arch.target == 'x86_64-pc-windows-msvc'
    run: |
    echo "Current PATH: $env:PATH"
    pixi --version
    shell: pwsh
    - name: Help
    run: pixi --help
    - name: Info
    run: pixi info
    - name: Install pixi
    run: pixi install -v
    - name: Test examples
    shell: bash
    run: bash tests/test_examples.sh

@abkfenris
Copy link
Contributor Author

Ok, I've got a handful of examples being tested on Linux and Mac.

I currently have Windows commented out as I didn't want to try to change what tests are run by architecture at this point of the evening, and one of the tests is commented out as it has a sdist that requires a custom environment to build.

cd src/cli/project/export/test-data/testenv
pixi project export conda-environment | tee test-env.yml
echo "Creating the export test environment with micromamba"
micromamba create -y -f test-env.yml -n export-test
Copy link
Contributor

Choose a reason for hiding this comment

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

Great that you added this test! I think just running a --dry-run of most of this would be enough we only need to test if it loads the file correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--dry-run doesn't appear to test if the pip dependencies are formatted correctly, and instead tries to activate the environment and hand them off to pip.

❯ bash tests/test_export.sh
Running test_export.sh
Exporting the export test environment
name: default
channels:
- conda-forge
dependencies:
- python >=3.12.5,<4
- pyyaml >=6.0.2,<7
- pip
- pip:
  - rich>=13.8.0, <14

Creating the export test environment with micromamba
conda-forge/osx-arm64                               11.8MB @  14.0MB/s  0.8s
conda-forge/noarch                                  16.4MB @  16.0MB/s  1.0s

Transaction

  Prefix: /Users/akerney/micromamba/envs/export-test

  Updating specs:

   - python[version='>=3.12.5,<4']
   - pyyaml[version='>=6.0.2,<7']
   - pip


  Package              Version  Build               Channel           Size
────────────────────────────────────────────────────────────────────────────
  Install:
────────────────────────────────────────────────────────────────────────────

  + xz                   5.2.6  h57fd34a_0          conda-forge     Cached
  + libexpat             2.6.3  hf9b8971_0          conda-forge     Cached
  + ncurses                6.5  h7bae524_1          conda-forge     Cached
  + yaml                 0.2.5  h3422bc3_2          conda-forge     Cached
  + python_abi            3.12  5_cp312             conda-forge     Cached
  + bzip2                1.0.8  h99b78c6_7          conda-forge     Cached
  + libffi               3.4.2  h3422bc3_5          conda-forge     Cached
  + libzlib              1.3.1  hfb2fe0b_1          conda-forge     Cached
  + ca-certificates  2024.8.30  hf0a4a13_0          conda-forge     Cached
  + readline               8.2  h92ec313_1          conda-forge     Cached
  + tk                  8.6.13  h5083fa2_1          conda-forge     Cached
  + libsqlite           3.46.1  hc14010f_0          conda-forge     Cached
  + openssl              3.3.2  h8359307_0          conda-forge     Cached
  + tzdata               2024a  h8827d51_1          conda-forge     Cached
  + python              3.12.6  h739c21a_0_cpython  conda-forge     Cached
  + wheel               0.44.0  pyhd8ed1ab_0        conda-forge     Cached
  + setuptools          73.0.1  pyhd8ed1ab_0        conda-forge     Cached
  + pip                   24.2  pyh8b19718_1        conda-forge     Cached
  + pyyaml               6.0.2  py312h024a12e_1     conda-forge     Cached

  Summary:

  Install: 19 packages

  Total download: 0 B

────────────────────────────────────────────────────────────────────────────


Dry run. Not executing the transaction.

Installing pip packages: rich>=13.8.0, <14
critical libmamba Cannot activate, prefix does not exist at: '/Users/akerney/micromamba/envs/export-test'
/var/folders/n6/hl07fcc55ys7mbybt01gb8r80000gn/T/mambafD4HVTylLmG: line 5: : command not found
critical libmamba pip failed to install packages

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh lol, that sounds like a bug. I'm a bit conflicted here. We're having a pretty big issue with slow CI and this would blow it up more then I would like. Do you think you could only run it if the export code and these tests are touched?

Copy link
Member

Choose a reason for hiding this comment

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

We can also make it a test that only runs when a certain label is applied?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like it will take an extra job which will check if things have changed, but then export tests could run afterwards. https://stackoverflow.com/a/70711156

Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

Thankyou @abkfenris, this is ready!

@ruben-arts ruben-arts merged commit dc9b79e into prefix-dev:main Sep 17, 2024
44 checks passed
@abkfenris
Copy link
Contributor Author

Thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants