Repository: r5py/r5py (language: python)

Issue:
Fiona and OpenJDK don’t get along in Windows runners 
See the entire discussion in pull request https://github.com/r5py/r5py/pull/149.

It seems that Fiona/GDAL/OGR and OpenJDK somehow confuse each other on the GitHub Windows runners.

Could the people with access to ‘bare-metal’ Windows (@HTenkanen , @wklumpen ) please try to following minimum example:

1. Create an environment file `fionatest.yaml` with the following content:

```
name: fionatest

channels:
    - conda-forge

dependencies:
    - python=3.10
    - fiona
```

2. Use mamba to create a new environment using this file: 

```
mamba env create --file fionatest.yaml
```

3. try to import fiona (should be successful)

```
python -c "import fiona"
```

4. install openjdk (in this example using mamba, in the github runners we use the appropriate github action)

```
mamba install -y openjdk=11
```

5. try to import fiona, again (fails!)

```
python -c "import fiona"
```

6. Bonus step 1: uninstall the JDK

```
mamba remove -y openjdk
```

7. Bonus step 2: try to import fiona, again (STILL fails!)

```
python -c "import fiona"
```

Not quite sure how to get out of this, as we need *both* a JDK and fiona. At least I’m my semi-certain that it’s not pytest. I’m fairly sure that it’s somehow connected to environment variables (like %PATH%, e.g.), and that installing the JDK messes them up, apparently both with the GH action and from mamba. If you try the (hopefully reproducible) example, could you compar the environment variables (or at least %PATH%) before and after every step? (e.g., `python -c "import os; print(os.environ['PATH'])"`) Thanks!

_Originally posted by @christophfink in https://github.com/r5py/r5py/issues/149#issuecomment-1218061269_


Discussion / hints:
@christophfink : Will try this out 👍🏻 
@christophfink : Okay, can confirm that exactly this happens with me as well on Windows side. 

The PATH variables after steps 1 and 2 do not seem to differ. However, `"CONDA_EXE"` are different

1: `'CONDA_EXE': 'C:\\Users\\tenks\\miniconda3\\Scripts\\conda.exe'`

2: `'CONDA_EXE': 'C:\\Users\\tenks\\miniconda3\\condabin\\..\\Scripts\\conda.exe'`

Perhaps this might cause issues? At least to me this sounds a promising lead explaining the behavior as the conda executable changes after the installation of openjdk. 


Also as Joris hints here https://github.com/r5py/r5py/pull/149#issuecomment-1218091820,
a difference between environments is that **before** openjdk is installed, there actually isn't an environment variable called `JAVA_HOME` but openjdk adds that variable once it is installed. 👀 
Does JAVA_HOME persist after uninstalling the JDK (it does not for me)
> Does JAVA_HOME persist after uninstalling the JDK (it does not for me)

Yes it does at least on my Windows.
@christophfink : Some additional leads from gdal when trying to import that after installing/removing openjdk:

![image](https://user-images.githubusercontent.com/6659534/185302103-479a6c8c-bc46-469d-b416-e69ef7686470.png)

Unfortunately, after trying to add the environment variable `USE_PATH_FOR_GDAL_PYTHON=YES` did not do the trick at least on my computer. Somehow the openjdk messes around where the dll files are located or where they are searched from. 

This thread could be useful: https://github.com/conda-forge/gdal-feedstock/issues/541

This might be relevant: https://github.com/conda-forge/gdal-feedstock/issues/541#issuecomment-945059726
> @christophfink : Okay, can confirm that exactly this happens with me as well on Windows side.
> 
> The PATH variables after steps 1 and 2 do not seem to differ. However, `"CONDA_EXE"` are different
> 
> 1: `'CONDA_EXE': 'C:\\Users\\tenks\\miniconda3\\Scripts\\conda.exe'`
> 
> 2: `'CONDA_EXE': 'C:\\Users\\tenks\\miniconda3\\condabin\\..\\Scripts\\conda.exe'`
> 
> Perhaps this might cause issues? At least to me this sounds a promising lead explaining the behavior as the conda executable changes after the installation of openjdk.

Just for the record, those paths are the same.
So, it seems, indeed, that GDAL somehow gets the idea that it would be able to use Java, but doesn’t find it (the netcdf.dll is yet another issue):

![Screenshot from 2022-08-18 13-51-53](https://user-images.githubusercontent.com/4998473/185378294-6e931cfb-e999-49ad-8e42-166dc8a5505e.png)

![Screenshot from 2022-08-18 13-52-21](https://user-images.githubusercontent.com/4998473/185378291-1940d809-0a24-4942-894c-5e9b81b50701.png)

I’ve now also compiled file lists before and after (un)installing openjdk, I think I’m finally onto something here :)
It seems OpenJDK overwrites (on installation) and then uninstalls the following DLLs/EXEs: 

- freetype.dll
- kinit.exe
- klist.exe
- zip.dll

```
$ diff files-02-after-fiona-before-jdk-install files-05-after-jdk-uninstall
224d223
< 2022-08-18  01:40 PM                 0 .nonadmin
299c298
<               64 File(s)     23,796,187 bytes
---
>               63 File(s)     23,796,187 bytes
334c333
< 2022-08-18  01:40 PM             4,446 history
---
> 2022-08-18  01:40 PM             4,875 history
396c395
<               91 File(s)     12,259,025 bytes
---
>               91 File(s)     12,259,454 bytes
9582d9580
< 2022-08-16  02:27 AM           648,704 freetype.dll
9712,9713d9709
< 2022-03-15  01:24 PM            27,648 kinit.exe
< 2022-03-15  01:24 PM            27,136 klist.exe
9973d9968
< 2022-08-13  02:38 AM           104,448 zip.dll
9980c9975
<              520 File(s)  1,033,282,064 bytes
---
>              516 File(s)  1,032,474,128 bytes
72441c72436
< 2022-08-18  01:39 PM    <DIR>          cache
---
> 2022-08-18  01:40 PM    <DIR>          cache
101390,101393c101385,101388
< 2022-08-18  01:39 PM    <DIR>          .
< 2022-08-18  01:39 PM    <DIR>          ..
< 2022-08-18  01:19 PM        64,441,526 09cdf8bf.json
< 2022-08-18  01:19 PM        14,461,631 09cdf8bf.solv
---
> 2022-08-18  01:40 PM    <DIR>          .
> 2022-08-18  01:40 PM    <DIR>          ..
> 2022-08-18  01:40 PM        64,442,588 09cdf8bf.json
> 2022-08-18  01:40 PM        14,461,921 09cdf8bf.solv
101396c101391
<                4 File(s)    233,844,536 bytes
---
>                4 File(s)    233,845,888 bytes
185464,185465c185459,185460
<            111155 File(s)  6,968,723,890 bytes
<            31844 Dir(s)  79,186,104,320 bytes free
---
>            111150 File(s)  6,967,917,735 bytes
>            31844 Dir(s)  79,157,850,112 bytes free
```
Okay, sounds like you're on a right track! 👍🏻 Any luck yet, getting things to work?
Not yet, continueing tomorrow mornign
Alright, so it was _not_ the right track, although we did discover an upstream bug there: 

The mamba OpenJDK requires a very particular `zip.dll`, and simply overwrites any existing one. That, in turn, conflicts with GDAL that (rightfully so) expects libzip from the repositories to be installed.

conda-forge’s `libzip`’s zip.dll:
![Screenshot from 2022-08-19 10-47-09](https://user-images.githubusercontent.com/4998473/185570321-ef695304-4e99-4723-ab3b-890e78ed5ea5.png)

the zip.dll installed by openjdk=11
![Screenshot from 2022-08-19 10-45-41](https://user-images.githubusercontent.com/4998473/185570469-b1107dfc-95b3-4d2d-809c-9f8150f64e32.png)

GDAL (and with it fiona) can be fixed by `mamba install --force-reinstall libzip` - however, this, then, breaks the JDK. 

Lessons learnt: don’t get the JDK from conda-forge, but use a system-wide one (which is what we do on the GitHub runners).
That said, this sadly does not fix anything with respect to the original problem with our test runs: inside pytest, `import fiona` continues to fail consistently.
The good news: I can reproduce this exact behaviour in my local Windows VM.
Gathering a bit more information (sorry about spamming this issue):

When running with `python -X faulthandler -m pytest -p no:faulthandler`, we are reported a `Windows fatal exception: code 0xc0000139`, which, according to https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/596a1078-e883-4972-9bbc-49e60bebca55 , means `STATUS_ENTRYPOINT_NOT_FOUND` ({Entry Point Not Found} The procedure entry point %hs could not be located in the dynamic link library %hs.). 

The module this fails with is `fiona.ogrext`, which is a compiled `.pyd` binary. 
Hmm. looks like a very tricky issue. Do you @christophfink see any possible ways forward with this? I think this is a serious conflict between the packages and we should probably raise an issue on the OpenJDK side about this? I think it's quite problematic that the `zip.dll` is simply overwritten during the installation. 
We absolutely should (it would be the way openjdk is packaged for conda-forge). 

However, this is separate from our issue, for which I also don't know any way forward. I would suggest to put this to the side for the immediate time being, and 'simply' allow some tests to be skipped on Windows.
https://github.com/conda-forge/openjdk-feedstock/issues/86
> However, this is separate from our issue, for which I also don't know any way forward. I would suggest to put this to the side for the immediate time being, and 'simply' allow some tests to be skipped on Windows.

I’ll disable testing on Windows entirely, for the time being. Let’s discuss possible ways forward in our call later today.
@christophfink Super! I was just thinking the same, so that we can keep things rolling 👍🏻 

Fix patch:
diff --git a/ci/python_310.yaml b/ci/python_310.yaml
index 16c287ec..f67e9821 100644
--- a/ci/python_310.yaml
+++ b/ci/python_310.yaml
@@ -6,6 +6,7 @@ channels:
 dependencies:
     - python=3.10
     - configargparse
+    - fiona
     - geopandas
     - joblib
     - jpype1
diff --git a/ci/python_310_dev.yaml b/ci/python_310_dev.yaml
index c18d2069..da9c1a6f 100644
--- a/ci/python_310_dev.yaml
+++ b/ci/python_310_dev.yaml
@@ -6,6 +6,7 @@ channels:
 dependencies:
     - python=3.10
     - configargparse
+    - fiona
     - geopandas
     - joblib
     - jpype1
diff --git a/ci/python_38.yaml b/ci/python_38.yaml
index d862dd79..33d7af45 100644
--- a/ci/python_38.yaml
+++ b/ci/python_38.yaml
@@ -6,6 +6,7 @@ channels:
 dependencies:
     - python=3.8
     - configargparse
+    - fiona
     - geopandas
     - joblib
     - jpype1
diff --git a/ci/python_39.yaml b/ci/python_39.yaml
index e68e7874..50f1b333 100644
--- a/ci/python_39.yaml
+++ b/ci/python_39.yaml
@@ -6,6 +6,7 @@ channels:
 dependencies:
     - python=3.9
     - configargparse
+    - fiona
     - geopandas
     - joblib
     - jpype1
diff --git a/ci/r5py_distro.yaml b/ci/r5py_distro.yaml
index cb892db7..91f1a0f0 100644
--- a/ci/r5py_distro.yaml
+++ b/ci/r5py_distro.yaml
@@ -6,6 +6,7 @@ channels:
 dependencies:
     - python=3.10
     - folium
+    - fiona
     - geopandas
     - matplotlib
     - mapclassify


Test patch:
diff --git a/.github/workflows/lint-test-upon-push.yml b/.github/workflows/lint-test-upon-push.yml
index 97b47414..97c06534 100644
--- a/.github/workflows/lint-test-upon-push.yml
+++ b/.github/workflows/lint-test-upon-push.yml
@@ -42,7 +42,7 @@ jobs:
       matrix:
         os: 
           - ubuntu-latest
-          # - windows-latest  # see issue #156
+          - windows-latest
           - macos-latest
         env:
           - ci/python_310.yaml
@@ -70,13 +70,6 @@ jobs:
           miniforge-version: latest
           use-mamba: true
 
-      - name: On Windows, resort JDK/GDAL conflicts
-        if: matrix.os == 'windows-latest'
-        run: mamba install --force-reinstall gdal libgdal libzip freetype
-
-      - name: Test whether fiona can be imported
-        run: python -c "import fiona"
-
       - name: Make sure pytest{,-cov,-lazy-fixture} are installed
         shell: bash -l {0}
         run: mamba install -c conda-forge pytest pytest-cov pytest-lazy-fixture
diff --git a/tests/conftest.py b/tests/conftest.py
index 1ee65304..f68b6ebe 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -9,6 +9,8 @@
 
 import pathlib
 
+# explicitly importing fiona before geopandas fixes issue #156
+import fiona  # noqa: F401
 import geopandas
 import pytest  # noqa: F401