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

bootstrap-conda: Refactor, generate versioned environment files #36405

Merged
merged 10 commits into from
Oct 14, 2023

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Oct 5, 2023

Split out from

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe mkoeppe self-assigned this Oct 5, 2023
@mkoeppe mkoeppe force-pushed the bootstrap_conda_refactor branch 2 times, most recently from 9ae39bf to 939aa1b Compare October 5, 2023 01:24
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2023

@tobiasdiez This is a cleaned up version of parts of #35593, which is independent from other changes that are still in the pipeline. This PR should be neutral on the CI workflows, i.e., neither fixing nor breaking something. Please take a look

@tobiasdiez tobiasdiez added the s: run conda ci Run the conda workflow on this PR. label Oct 5, 2023
@tobiasdiez
Copy link
Contributor

Thanks. But there is now a pip section, which makes the build fail. Can you please remove these additional packages again from the environment file?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2023

Ah, I see. I'll back this change (creating the pip section not only for environment-optional and environment-dev, but also for environment) out from here until the package lists are fixed.

Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2023

Thanks!

@vbraun
Copy link
Member

vbraun commented Oct 8, 2023

merge conflict

Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Just noticed that the environment files contain also the declaration - python (coming from _prereq), which overwrites the other python version declaration.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 9, 2023

Along similar lines, should we include the Python version in the default environment name written to the file?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 9, 2023

the environment files contain also the declaration - python (coming from _prereq), which overwrites the other python version declaration.

Care to elaborate why you think it would overwrite it? I don't think it does.

@tobiasdiez
Copy link
Contributor

the environment files contain also the declaration - python (coming from _prereq), which overwrites the other python version declaration.

Care to elaborate why you think it would overwrite it? I don't think it does.

I tried it out and it was not working (that's how I discovered it in the first place).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 9, 2023

I also tried it out and it worked, so some details are needed.

@tobiasdiez
Copy link
Contributor

Might be a version mismatch, I don't know. Either way, I don't think its a good idea to have two python specifiers in the file.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 9, 2023

Well ok, here you go.

@tobiasdiez
Copy link
Contributor

Thanks!

Are the environment-dev files working for you? I get errors related to sagenb, i.e. it only works without the pip dependencies.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 10, 2023

I haven't tried. This PR, as requested, is focused narrowly on what is promised in its title. Other fixes are on other PRs.

@tobiasdiez
Copy link
Contributor

On develop the pip section is empty, with your changes here it is populated and breaks things.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 10, 2023

OK, here's a temporary workaround.

Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Thanks

@github-actions
Copy link

Documentation preview for this PR (built with commit 73ab316; changes) is ready! 🎉

@vbraun vbraun merged commit 6f44770 into sagemath:develop Oct 14, 2023
20 of 28 checks passed
@mkoeppe mkoeppe added this to the sage-10.2 milestone Oct 14, 2023
@mkoeppe mkoeppe deleted the bootstrap_conda_refactor branch October 17, 2023 18:01
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 27, 2023
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

The conda workflow is broken since a few months due to various issues
that arise from some packages being updated in conda-forge and sage not
being quite compatible with the new version. For example, currently it
fails as Cython 3.0.3 is used.

To prevent such issues happening in the feature, we use [conda-
lock](https://conda.github.io/conda-lock/) to create a lock file for
each os + python combination, from which one can then create a
reproducible conda env. These lock files are created using commands of
the form:
```bash
conda-lock --channel conda-forge --kind env --platform linux-64
--platform osx-64 --file ./src/environment-dev-3.10.yml --filename-
template "src/environment-dev-3.10-{platform}"
```


Follow-up: Add a github workflow that updates these lock files
automatically, similar to:
- https://github.com/ibis-
project/ibis/blob/master/.github/workflows/conda-lock.yml
- https://github.com/svopper/munaiah-
analyser/blob/main/out/SciTools/iris/.github_workflows_refresh-
lockfiles.yml
- https://github.com/chipsalliance/fpga-interchange-
tests/blob/main/.github/workflows/update_conda_lock.yml
- https://github.com/ESMValGroup/ESMValTool/blob/main/.github/workflows/
create-condalock-file.yml

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

- sagemath#36405: to get env files for each
python version
- sagemath#36767

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35986
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik, Isuru Fernando, Matthias Köppe, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 2, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

The conda workflow is broken since a few months due to various issues
that arise from some packages being updated in conda-forge and sage not
being quite compatible with the new version. For example, currently it
fails as Cython 3.0.3 is used.

To prevent such issues happening in the feature, we use [conda-
lock](https://conda.github.io/conda-lock/) to create a lock file for
each os + python combination, from which one can then create a
reproducible conda env. These lock files are created using commands of
the form:
```bash
conda-lock --channel conda-forge --kind env --platform linux-64
--platform osx-64 --file ./src/environment-dev-3.10.yml --filename-
template "src/environment-dev-3.10-{platform}"
```


Follow-up: Add a github workflow that updates these lock files
automatically, similar to:
- https://github.com/ibis-
project/ibis/blob/master/.github/workflows/conda-lock.yml
- https://github.com/svopper/munaiah-
analyser/blob/main/out/SciTools/iris/.github_workflows_refresh-
lockfiles.yml
- https://github.com/chipsalliance/fpga-interchange-
tests/blob/main/.github/workflows/update_conda_lock.yml
- https://github.com/ESMValGroup/ESMValTool/blob/main/.github/workflows/
create-condalock-file.yml

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

- sagemath#36405: to get env files for each
python version
- sagemath#36767

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35986
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik, Isuru Fernando, Matthias Köppe, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 5, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

The conda workflow is broken since a few months due to various issues
that arise from some packages being updated in conda-forge and sage not
being quite compatible with the new version. For example, currently it
fails as Cython 3.0.3 is used.

To prevent such issues happening in the feature, we use [conda-
lock](https://conda.github.io/conda-lock/) to create a lock file for
each os + python combination, from which one can then create a
reproducible conda env. These lock files are created using commands of
the form:
```bash
conda-lock --channel conda-forge --kind env --platform linux-64
--platform osx-64 --file ./src/environment-dev-3.10.yml --filename-
template "src/environment-dev-3.10-{platform}"
```


Follow-up: Add a github workflow that updates these lock files
automatically, similar to:
- https://github.com/ibis-
project/ibis/blob/master/.github/workflows/conda-lock.yml
- https://github.com/svopper/munaiah-
analyser/blob/main/out/SciTools/iris/.github_workflows_refresh-
lockfiles.yml
- https://github.com/chipsalliance/fpga-interchange-
tests/blob/main/.github/workflows/update_conda_lock.yml
- https://github.com/ESMValGroup/ESMValTool/blob/main/.github/workflows/
create-condalock-file.yml

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

- sagemath#36405: to get env files for each
python version
- sagemath#36767

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35986
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik, Isuru Fernando, Matthias Köppe, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 13, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

The conda workflow is broken since a few months due to various issues
that arise from some packages being updated in conda-forge and sage not
being quite compatible with the new version. For example, currently it
fails as Cython 3.0.3 is used.

To prevent such issues happening in the feature, we use [conda-
lock](https://conda.github.io/conda-lock/) to create a lock file for
each os + python combination, from which one can then create a
reproducible conda env. These lock files are created using commands of
the form:
```bash
conda-lock --channel conda-forge --kind env --platform linux-64
--platform osx-64 --file ./src/environment-dev-3.10.yml --filename-
template "src/environment-dev-3.10-{platform}"
```


Follow-up: Add a github workflow that updates these lock files
automatically, similar to:
- https://github.com/ibis-
project/ibis/blob/master/.github/workflows/conda-lock.yml
- https://github.com/svopper/munaiah-
analyser/blob/main/out/SciTools/iris/.github_workflows_refresh-
lockfiles.yml
- https://github.com/chipsalliance/fpga-interchange-
tests/blob/main/.github/workflows/update_conda_lock.yml
- https://github.com/ESMValGroup/ESMValTool/blob/main/.github/workflows/
create-condalock-file.yml

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

- sagemath#36405: to get env files for each
python version
- sagemath#36767

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35986
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik, Isuru Fernando, Matthias Köppe, Tobias Diez
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: scripts s: run conda ci Run the conda workflow on this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants