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

Add tests for successful virtual_specs checks #843

Merged
merged 11 commits into from
Aug 15, 2024

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Aug 14, 2024

Description

Comes from conda-forge/miniforge#626. micromamba doesn't like something in that dry-run, apparently :)

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?

@jaimergp jaimergp requested a review from a team as a code owner August 14, 2024 15:47
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Aug 14, 2024
@jaimergp
Copy link
Contributor Author

This is weird. This PR makes things at conda-forge/miniforge#626 pass, but it fails here. Previous iterations made things work here, but fail at the miniforge PR. 🤷

@@ -1139,7 +1139,7 @@ Section "Install"
System::Call 'kernel32::SetEnvironmentVariable(t,t)i("CONDA_SOLVER", "classic").r0'
SetDetailsPrint TextOnly
DetailPrint "Checking virtual specs..."
push '"$INSTDIR\_conda.exe" create --dry-run --prefix "$INSTDIR" --offline @VIRTUAL_SPECS@'
push '"$INSTDIR\_conda.exe" create --dry-run --prefix "$INSTDIR\envs\_virtual_specs_checks" --offline @VIRTUAL_SPECS@'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to create a temporary directory here, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Micromamba is not used often on Windows due to the lack of menuinst, so it's not as critical. No clue how to create a tempdir via NSIS either.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I know why we don't need to. We are setting CONDA_PKGS_DIRS here: https://github.com/conda/constructor/blob/main/constructor/nsis/main.nsi.tmpl#L1124

In the sh and pkg installers, that environment variable is set after the checks for virtual specs.

marcoesters
marcoesters previously approved these changes Aug 15, 2024
@marcoesters marcoesters merged commit 6916207 into conda:main Aug 15, 2024
18 checks passed
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.

3 participants