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
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion constructor/header.sh
Original file line number Diff line number Diff line change
Expand Up @@ -461,11 +461,15 @@ mkdir -p "$TMP"
# We need to specify CONDA_SOLVER=classic for conda-standalone
# to work around this bug in conda-libmamba-solver:
# https://github.com/conda/conda-libmamba-solver/issues/480
# micromamba needs an existing pkgs_dir to operate even offline,
# but we haven't created $PREFIX/pkgs yet... give it a temp location
# shellcheck disable=SC2050
if [ "__VIRTUAL_SPECS__" != "" ]; then
echo 'Checking virtual specs compatibility: __VIRTUAL_SPECS__'
CONDA_QUIET="$BATCH" \
CONDA_SOLVER="classic" \
"$CONDA_EXEC" create --dry-run --prefix "$PREFIX" --offline __VIRTUAL_SPECS__
CONDA_PKGS_DIRS="$(mktemp -d)" \
"$CONDA_EXEC" create --dry-run --prefix "$PREFIX/envs/_virtual_specs_checks" --offline __VIRTUAL_SPECS__
fi

# Create $PREFIX/.nonadmin if the installation didn't require superuser permissions
Expand Down
2 changes: 1 addition & 1 deletion constructor/nsis/main.nsi.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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.

push 'Failed to check virtual specs: @VIRTUAL_SPECS@'
push 'WithLog'
call AbortRetryNSExecWait
Expand Down
7 changes: 5 additions & 2 deletions constructor/osx/prepare_installation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,14 @@ touch "$PREFIX/conda-meta/history"
# We need to specify CONDA_SOLVER=classic for conda-standalone
# to work around this bug in conda-libmamba-solver:
# https://github.com/conda/conda-libmamba-solver/issues/480
# micromamba needs an existing pkgs_dir to operate even offline,
# but we haven't created $PREFIX/pkgs yet... do it in a temporary location
# shellcheck disable=SC2050
if [ "__VIRTUAL_SPECS__" != "" ]; then
CONDA_QUIET="$BATCH" \
notify 'Checking virtual specs compatibility: __VIRTUAL_SPECS__'
CONDA_SOLVER="classic" \
"$CONDA_EXEC" create --dry-run --prefix "$PREFIX" --offline __VIRTUAL_SPECS__
CONDA_PKGS_DIRS="$(mktemp -d)" \
"$CONDA_EXEC" create --dry-run --prefix "$PREFIX/envs/_virtual_specs_checks" --offline __VIRTUAL_SPECS__
fi

# Create $PREFIX/.nonadmin if the installation didn't require superuser permissions
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: virtual_specs
name: virtual_specs_failed

version: 0.0.1

Expand Down
22 changes: 22 additions & 0 deletions examples/virtual_specs_ok/construct.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: virtual_specs_ok

version: 0.0.1

keep_pkgs: True

channels:
- conda-forge

specs:
- ca-certificates

virtual_specs:
- __osx>=10.9 # [osx]
- __glibc>=2.12 # [linux]
- __win>=0 # [win]

initialize_by_default: false
register_python: false
check_path_spaces: false
check_path_length: false
installer_type: all
19 changes: 19 additions & 0 deletions news/843-conda-pkgs-dirs-virtual-specs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
### Enhancements

* <news item>

### Bug fixes

* Fix a couple issues in the `virtual_specs` feature (set `CONDA_PKGS_DIRS` for `micromamba`, do not rely `$BATCH` for PKG, update the Windows example). (#843)
marcoesters marked this conversation as resolved.
Show resolved Hide resolved

### Deprecations

* <news item>

### Docs

* <news item>

### Other

* <news item>
17 changes: 15 additions & 2 deletions tests/test_examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -693,8 +693,8 @@ def test_cross_osx_building(tmp_path):
)


def test_virtual_specs(tmp_path, request):
input_path = _example_path("virtual_specs")
def test_virtual_specs_failed(tmp_path, request):
input_path = _example_path("virtual_specs_failed")
for installer, install_dir in create_installer(input_path, tmp_path):
process = _run_installer(
input_path,
Expand All @@ -720,3 +720,16 @@ def test_virtual_specs(tmp_path, request):
msg = "Installer requires"
assert process.returncode != 0
assert msg in process.stdout + process.stderr


def test_virtual_specs_ok(tmp_path, request):
input_path = _example_path("virtual_specs_ok")
for installer, install_dir in create_installer(input_path, tmp_path):
_run_installer(
input_path,
installer,
install_dir,
request=request,
check_subprocess=True,
uninstall=True,
)
Loading