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

Please support Windows. #1154

Open
mrolle45 opened this issue Mar 20, 2022 · 9 comments
Open

Please support Windows. #1154

mrolle45 opened this issue Mar 20, 2022 · 9 comments
Labels
cat: infrastructure supported platforms, dependency detection, etc. help wanted

Comments

@mrolle45
Copy link

My tale of woe
I have a Windows PC, and I wanted to run pytype on my code. I had to install WSL and then load several needed packages. Not only was this a big pain, but when pytype gave me some bad results, I tried debugging it.
I'm not familiar with the latest IDEs on linux, nor did I want to spend the effort of learning to debug Python code visually (and load even more packages).
So I resorted to inserting lots of print() calls in the code until I figured out what the problem was.
About two days spent on this effort.
I managed to get good pyi outputs for most, but not all, of my source code. I then tried running merge_pyi on the good ones, only to find out that merge_pyi crashed.
On the hunch that at least merge_pyi didn't do anything weird, I installed pytype on my Windows Python, and used Visual Studio to debug it. It worked without problems. I was able to find and fix the bug, and file an issue about it. See #1153.

The pitch
I presume that there can't be too much involved in getting the entire pytype package to run correctly on Windows. After all, Python is mostly portable.
It would be a great assistance to Windows Python users if they could run pytype directly.
Thank you.

@martindemello
Copy link
Contributor

we would definitely love to have better support for windows, the problem is that there are only three pytype devs, and we all use linux. we are very open to pull requests for windows and macos support, but we are unable to do the work ourselves.

@martindemello martindemello added the cat: infrastructure supported platforms, dependency detection, etc. label Mar 20, 2022
@IIHERO4
Copy link

IIHERO4 commented Mar 21, 2022

we would definitely love to have better support for windows, the problem is that there are only three pytype devs, and we all use linux. we are very open to pull requests for windows and macos support, but we are unable to do the work ourselves.

Hey, i was just wondering what would be necessary in order to support windows.

@martindemello
Copy link
Contributor

@IIHERO4 just an iterative process of running pytype, finding issues, investigating and fixing them - i'm afraid we don't have a proper roadmap because we don't even know what all the issues are.

the current blocker is #90 - it's possible that everything will just work after that is fixed, but it's also possible that other issues are hiding behind it.

@HexDecimal
Copy link

The designated initializers like the ones here seem to be non-standard and break MSVC.

PyTypeObject PyProgram = {
PyVarObject_HEAD_INIT(&PyType_Type, 0) tp_name : "Program",
tp_basicsize : sizeof(PyProgramObj),
tp_itemsize : 0,
tp_dealloc : ProgramDealloc,
#if PY_VERSION_HEX >= 0x03080000 // 3.8
tp_vectorcall_offset: -1,
# else
tp_print : nullptr,
# endif
tp_getattr : nullptr,
tp_setattr : nullptr,
#if PY_VERSION_HEX >= 0x03050000 // 3.5
tp_as_async : nullptr,
#else
tp_compare : nullptr,
#endif
tp_repr : nullptr,
tp_as_number : nullptr,
tp_as_sequence : nullptr,
tp_as_mapping : nullptr,
tp_hash : nullptr,
tp_call : nullptr,
tp_str : nullptr,
tp_getattro : ProgramGetAttro,
tp_setattro : ProgramSetAttro,
tp_as_buffer : nullptr,
tp_flags : 0,
tp_doc : program_doc,
tp_traverse : nullptr,
tp_clear : nullptr,
tp_richcompare : nullptr,
tp_weaklistoffset : 0,
tp_iter : nullptr,
tp_iternext : nullptr,
tp_methods : program_methods,
tp_members : nullptr,
tp_getset : nullptr,
tp_base : nullptr,
tp_dict : nullptr,
tp_descr_get : nullptr,
tp_descr_set : nullptr,
tp_dictoffset : 0,
tp_init : nullptr,
tp_alloc : nullptr,
tp_new : ProgramNew,
};

Even if you used C++20 designated initializers PyVarObject_HEAD_INIT would cause non-designated initializers to be mixed in and break MSVC. I solved this issue by switching from Python static types to heap types rather than leaving a lot of attributes without names.

The CMake script doesn't set a C++ standard so MSVC defaults to one that's too early. I added set(CMAKE_CXX_STANDARD 17) to the top script but there might be better options. On Windows I was able to run both pip install -e . and python build_scripts/build.py after this change and the one above.

The issues with #90 still exist. pytype generates absolute paths with Windows drive names C:\ in them and Ninja thinks the colon is part of the build command instead of part of a path. The Ninja build file will run if you change some of the paths to be relative. I think only the paths on the build/check lines are having issues. I don't know how you're normally supposed to escape the drive path.

martindemello added a commit that referenced this issue Jul 19, 2022
NOTE: Due to some complicated merge issues the PR commit history got messed up;
here is the original commit message, with changes distributed over several
commits in the log:

I tried to add windows support for pytype. It seems to work now. The package installed with `pip install -e .` works fine now. At least my needs has been satisfied.

* Replace delegate initializers with ordinary ones.
* Update pybind11 to v2.9.0. Previous version of pybind11 causes compiler errors on msvc ( pybind/pybind11#3477).
* Add `tools/path_tools.py`. It provides functions with the same signatures as thoes defined in `os.path` and `glob`, but they replace the `\` in return value with `/` if pytype is running on Windows.
* Add `tools/tempfile.py`. Files created by NamedTemporaryFile cannot be opened twice in the same process on Windows if by default. `tools/tempfile.py` provides an alternative `NamedTemporaryFile ` implementation for Windows. See https://stackoverflow.com/questions/23212435/permission-denied-to-write-to-my-temporary-file for details.
* Edit several `CMakeLists.txt`s

I ran `pip install -e . && python build_scripts/run_tests.py` to test it. It passed every test cases before following text showed in the command line. It seems there are some problems with the build system. (But building with `pip install -e .` works fine.)
```
>>> Found Ninja target failures (includes test failures):
    - pytype/typegraph/cfg.so pytype/typegraph/cfg.lib
>>> FAILED: Ninja command 'ninja -k 100000 test_all'.
>>>         Run it in the 'out' directory to reproduce.
>>>         Full Ninja output is available in 'C:\Users\wweih\Documents\pytype-modified\pytype\out\ninja.log'.
>>>         Failing test modules (if any) will be reported below.
```
ninja.log

* I am not expertize in building systems. Are there any advice about the above issue?
* Which tool should I use in order to format the source code?
* `/` and `\` are both valid seperators on Windows. It caused a lot of problems for pytype. I tried to solve this by replacing all `\`  with `/` on windows. Is this an acceptable solution?
* Is there any automated test pipline to test my commits on Linux and macOS? Or should I setup an Linux system to test it on myself?

Addresses #1154
@Avasam
Copy link

Avasam commented Jan 6, 2023

#727 (comment)
Hi, I'm trying to look into this MSVC syntax error, but for the life of me I can't figure out how to use /std:c++20 instead of /std:c++14 when doing pip install . -e so I can try some things. (namely using standard designated initializers)

@rchen152
Copy link
Contributor

Looks like we have std=c++11 specified here:

add_compile_options("-std=c++11")

and cxx_std=11 here:
cxx_std=11,

Try changing one or both of those?

@hhoppe
Copy link

hhoppe commented Feb 8, 2024

I've been using pytype in WSL (Windows Subsystem for Linux) for a while.
I just tried Windows-native Python and was amazed that just about everything I have ever used works, e.g.:
pip install -U jupyter jupyterlab jupyterlab-execute-time jupytext autopep8 flake8 mypy pyink pylint pytest pdoc matplotlib numba numpy scipy scikit-image tqdm Pillow plotly keras tensorboard tensorflow-estimator torch 'jax[cpu]' opencv-python-headless graphviz kaleido networkx more-itertools sympy nbformat requests requests_oauthlib configparser z3-solver ipywidgets 'pyvista[jupyter]' mediapy resampler

The only packages that failed were: tensorflow and pytype.
Just saying... :-)

wyattscarpenter added a commit to wyattscarpenter/pytype that referenced this issue Aug 11, 2024
this involves: 1. using the c++-standard named struct params like @HexDecimal suggested in google#1154 (comment) ; 2. changing  cxx_std=11 to 20 like @rchen152 suggested in google#1154 (comment)
@wyattscarpenter
Copy link
Contributor

wyattscarpenter commented Aug 14, 2024

Once PR #1734 is incorporated into the project (a PR that incorporates the (very astute!) suggestions found earlier in this discussion), then the few remaining errors will be, it seems to me, idiosyncratic ones possibly relating to import info, like

      self.assertEqual(0, ret)
  AssertionError: 0 != 1
    ⋮
   FAILED: pytype/io_test.log D:/a/pytype/pytype/out/pytype/io_test.log
  C:\Windows\system32\cmd.exe /C "cd /D D:\a\pytype\pytype\out\pytype && C:\Python\Lib\site-packages\cmake\data\bin\cmake.exe -E env TYPESHED_HOME=D:/a/pytype/pytype/typeshed C:/Python/python.exe -B D:/a/pytype/pytype/build_scripts/test_module.py pytype.io_test -o D:/a/pytype/pytype/out/pytype/io_test.log -P D:/a/pytype/pytype/out -s -p"
  C:\Users\runneradmin\AppData\Local\Temp\tmp2coltc1x\m.py:1:1: error: in <module>: Can't find module 'common.foo'. [import-error]
  
  from common import foo; print(foo.x)~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

in case anyone wants to dig into that.

Edit: the PR has now been incorporated. (Not a direct merge, so it says "closed" instead of "merged" on github, but the changes made it in, which is the important part.) You can now install Pytype on windows! Although it might have some bugs. Or maybe it works perfectly, who knows 🤷.

copybara-service bot pushed a commit that referenced this issue Aug 14, 2024
In order to support windows, we need to use the c++-standard named struct params like @HexDecimal suggested in #1154 (comment) and change cxx_std=11 to 20 like @rchen152 suggested in #1154 (comment).

There may be more changes required. In particular, I am getting *one* test failing in the Tests step[^1] of the CI:
```
    File "D:\a/pytype/pytype/out\pytype\io_test.py", line 169, in test_unused_imports_info_files
      self.assertEqual(0, ret)
  AssertionError: 0 != 1
    ⋮
   FAILED: pytype/io_test.log D:/a/pytype/pytype/out/pytype/io_test.log
  C:\Windows\system32\cmd.exe /C "cd /D D:\a\pytype\pytype\out\pytype && C:\Python\Lib\site-packages\cmake\data\bin\cmake.exe -E env TYPESHED_HOME=D:/a/pytype/pytype/typeshed C:/Python/python.exe -B D:/a/pytype/pytype/build_scripts/test_module.py pytype.io_test -o D:/a/pytype/pytype/out/pytype/io_test.log -P D:/a/pytype/pytype/out -s -p"
  C:\Users\runneradmin\AppData\Local\Temp\tmp2coltc1x\m.py:1:1: error: in <module>: Can't find module 'common.foo'. [import-error]

  from common import foo; print(foo.x)~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ```
 I don't know why. But this is still an incremental improvement in Windows support for pytype.

 [^1]: Additionally, in my [repo](https://github.com/wyattscarpenter/pytype/actions/runs/10342876412/jo[]#step:6:289), I've changed the CI script to run all of the checks and not early-out; in this, the Windows version now passes the Extensions Tests and gets some error in Type Check.

PiperOrigin-RevId: 662898874
@oprypin
Copy link
Member

oprypin commented Sep 18, 2024

PyType should work on Windows now. Installing it is rather painful because there are no prebuilt wheels for Windows. Still, if all prerequisites are met, a normal pip install pytype should work. If someone wants to try adding wheel building to the CI and then also confirm that those wheels actually work on Windows, that would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat: infrastructure supported platforms, dependency detection, etc. help wanted
Projects
None yet
Development

No branches or pull requests

9 participants