Issue during import of portalocker on windows

[parmeet]

🐛 Describe the bug

Currently TorchText CI is broken on windows due to following error:

ImportError: DLL load failed while importing win32file: The specified module could not be found

The error occurred during import of portalocker.

cc: @vitaly-fedyunin

Versions

Latest from main

[parmeet]

I wonder if torchdata CI on windows is also failing due to this?

[ejguan]

I haven't seen such Error in TorchData CI. The tests running on the latest main branch: https://github.com/pytorch/data/actions/runs/2359941609

Could this part be the problem? https://github.com/WoLpH/portalocker/blob/00cffcb1831208691d3bf6850df4768989a0bd4c/setup.py#L133
I would assume circle CI windows machines are too old to be upgraded to pywin32>226

[parmeet]

Ahh I see! Thanks @ejguan for providing additional context. I am not too familiar with the CI machines on windows. I wonder if @atalman, @mthrok you can provide some suggestions here?

[mthrok]

Couple of observations from quick skim

1. pywin32 is not a dependency of domains libraries, so it kinda explains why.
2. Probably not related, but the binary distributions of PyTorch core and domain libraries are, IIUC, compiled with VS 2019, while TorchData seems to be compiled with VS 2020.

The point 1. suggests that torchtext has to add pywin32 as hard dependency as well. Was this reviewed somewhere?

[ejguan]

@mthrok Thanks for digging into this.

1. pywin32 is not a dependency of domains libraries, so it kinda explains why.

It's weird to me because portalocker is introduced in TorchData however we don't see any failure on CI related to portalocker.

Was this reviewed somewhere?

If you mean the PR introducing portalocker, it's here #409

[mthrok]

@mthrok Thanks for digging into this.

1. pywin32 is not a dependency of domains libraries, so it kinda explains why.

It's weird to me because portalocker is introduced in TorchData however we don't see any failure on CI related to portalocker.

In #409 I do not see a change in CI or packaging script. For the downstream projects, who consume torchdata's binary distribution, the additional dependency should be written probably in setup.py.

Was this reviewed somewhere?

If you mean the PR introducing portalocker, it's here #409

I meant the decision / impact of adding a dependency reviewed? TorchData being upstream of domain libraries have to be careful adding a new dependency as the number of downstream projects are large. And when it's done, it has to be done properly, so that it is transparent, and won't create a conflict with major downstream projects.

[parmeet]

1. pywin32 is not a dependency of domains libraries, so it kinda explains why.

Thanks @mthrok for the investigation. To test the hypothesis, I added the dependency for testing, but seems the error persist. https://app.circleci.com/pipelines/github/pytorch/text/5584/workflows/003de168-e3ed-4941-be1a-3017aba3b446/jobs/189114

The point 1. suggests that torchtext has to add pywin32 as hard dependency as well. Was this reviewed somewhere?

If pywin32 is indeed the issue, I would expect the dependency to be added for TorchData, not downstream libraries right?

Edit: Of-course it does mean indirectly domain libraries depend pywin32, what I actually meant above is in terms of where the install dependencies go :).

[ejguan]

To test the hypothesis, I added the dependency for testing, but seems the error persist. https://app.circleci.com/pipelines/github/pytorch/text/5584/workflows/003de168-e3ed-4941-be1a-3017aba3b446/jobs/189114

I do see pywin32 with the right version installed. And, it's installed based on the dependency graph where torchdata depends on portalocker, and portalocker depends on pywin32.

[ejguan]

I have seen the same issue by searching on the website: https://lifesaver.codes/answer/dll-load-failed-while-importing-win32file-the-specified-module-could-not-be-found-17986

[mthrok]

I thought that the change introduced in Python 3.8 which makes the DLL loading mechanism secure caused this
https://docs.python.org/3/whatsnew/3.8.html#ctypes

However, torchdata's test seems to be passing on Python 3.8. I do not have a clue at the moment.

[ejguan]

It seems the root cause is mhammond/pywin32#1431
However, the fix is introduced in pywin32==226. And, since the pywin32 is installed in CircleCI with version 304, I think the issue should be fixed.

However, torchdata's test seems to be passing on Python 3.8.

That's why I suspect something wrong with CI machines.

[ejguan]

@parmeet
When you have time, could you please try to test if this is going to work by running python3 .\env\Scripts\pywin32_postinstall.py -install before testing?
I think you can download this file from https://github.com/mhammond/pywin32/blob/main/pywin32_postinstall.py

[parmeet]

cc: @Nayef211

[Nayef211]

@parmeet When you have time, could you please try to test if this is going to work by running python3 .\env\Scripts\pywin32_postinstall.py -install before testing? I think you can download this file from https://github.com/mhammond/pywin32/blob/main/pywin32_postinstall.py

@ejguan how do you suggest doing this testing/debugging if we don't have access to a windows system?