conda-forge / pdal-feedstock

A conda-smithy repository for pdal.
BSD 3-Clause "New" or "Revised" License
3 stars 19 forks source link

Handle paths with spaces in activate.bat #184

Closed j9ac9k closed 2 years ago

j9ac9k commented 2 years ago

pdal-activate.bat does not handle the use-case where there are spaces in %CONDA_PREFIX%

activate.bat in its current form does not handle the case where there is a space in %CONDA_PREFIX%, and thus it attempts to set %PDAL_DRIVER_PATH% to a non-existent directory.

This commit allows for the proper escaping of spaces in the variable representing the path.

Checklist

This issue seems to be most common in cases where a user has a space in their windows username, and conda is installed to the user profile in %USERPROFILE%/miniconda

This issue can be replicated as follows:

mkdir "%USERPROFILE%\some space"
conda create -p "%USERPROFILE%\some space" -y
conda activate "%USERPROFILE%\some space"
conda install -c conda-forge pdal -y
...
Proceed ([y]/n)? y

Preparing transaction: done
Verifying transaction: done
Executing transaction: done
The system cannot find the path specified.

C:\Users\ogi\some space>set "GDAL_DRIVER_PATH="
The system cannot find the path specified.

C:\Users\ogi\some space>set "PDAL_DRIVER_PATH="

C:\Users\ogi\some space>rem proj-data is installed because its license was copied over

GDAL feedstock had a similar issue which was fixed here: https://github.com/conda-forge/gdal-feedstock/pull/637

It should be noted that in some cases conda prevents you from creating an environment in a directory with spaces, and in some cases it provides warnings; but for user-level installs where the user-name has a space, it can be difficult to work-around that.

conda-forge-linter commented 2 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

j9ac9k commented 2 years ago

Converted to draft as to not merge in the current state.

j9ac9k commented 2 years ago

Updated PR, should be ready for review/merging now.