Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Some modules are not successfully loaded by 'import pythonmonkey' on windows #445

Open
jacalata opened this issue Oct 2, 2024 · 7 comments

Comments

@jacalata
Copy link

jacalata commented Oct 2, 2024

Issue type

Bug

How did you install PythonMonkey?

Installed from pip

OS platform and distribution

Windows 11

Python version (python --version)

3.12

PythonMonkey version (pip show pythonmonkey)

1.0.0

Bug Description

I debugged through this and eventually found that in ctx-module.js::219 there is a regex check for whether the file path is absolute
Unfortunately on Windows, drive letters may be lowercase, and this was not caught by the regex.

existing code:
if (moduleIdentifier[0] === '/' || moduleIdentifier.match(/^[A-Z]:[/\]/)) // absolute paths
my local fix:
if (moduleIdentifier[0] === '/' || moduleIdentifier.match(/^[A-Za-z]:[/\]/)) // absolute paths

Standalone code to reproduce the issue

import pythonmonkey as pm

Relevant log output or backtrace

PS C:\dev\tsc>  c:; cd 'c:\dev\tsc'; & 'c:\Users\jac.fitzgerald\AppData\Local\Programs\Python\Python312\python.exe' 'c:\Users\jac.fitzgerald\.vscode\extensions\ms-python.debugpy-2024.10.0-win32-x64\bundled\libs\debugpy\adapter/../..\debugpy\launcher' '53945' '--' 'C:\dev\tsc\samples\call-embedding-lib.py' 
Traceback (most recent call last):
  File "C:\dev\tsc\samples\call-embedding-lib.py", line 2, in <module>
    import pythonmonkey as pm
  File "c:\Users\jac.fitzgerald\AppData\Local\Programs\Python\Python312\Lib\site-packages\pythonmonkey\__init__.py", line 17, in <module>
    require("timers")
  File "c:\Users\jac.fitzgerald\AppData\Local\Programs\Python\Python312\Lib\site-packages\pythonmonkey\require.py", line 429, in require
    return createRequire(filename)(moduleIdentifier)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pythonmonkey.SpiderMonkeyError: Error in file c:\Users\jac.fitzgerald\AppData\Local\Programs\Python\Python312\Lib\site-packages\pythonmonkey\node_modules/ctx-module/ctx-module.js, on line 239, column 21:
Error: module not found -- searched   ('c:/Users/jac.fitzgerald/AppData/Local/Programs/Python/Python312/Lib/site-packages/pythonmonkey/node_modules/core-js/stable/dom-exception') from c:/Users/jac.fitzgerald/AppData/Local/Programs/Python/Python312/Lib/site-packages/pythonmonkey/node_modules/core-js/actual/dom-exception/index.js
Stack Trace:
  ctxRequire@c:\Users\jac.fitzgerald\AppData\Local\Programs\Python\Python312\Lib\site-packages\pythonmonkey\node_modules/ctx-module/ctx-module.js:122:7

Additional info if applicable

No response

What branch of PythonMonkey were you developing on? (If applicable)

No response

wiwichips added a commit to wiwichips/ctx-module that referenced this issue Oct 3, 2024
Check for Windows drive letters only checked for CAPS but Windows fs is case insensitive

Thank you to jacalata Distributive-Network/PythonMonkey#445

Co-authored-by: Jac <jacalata@users.noreply.github.com>
@wiwichips
Copy link
Collaborator

Awesome bug report @jacalata! I filed a PR on ctx-module wesgarland/ctx-module#9 and added you as a co-author

I'll keep this issue open while this remains an issue on PythonMonkey

@Nirat888
Copy link

Nirat888 commented Dec 28, 2024

How to install pythonmonkey with this commit applied?

@wiwichips
Copy link
Collaborator

@wesgarland @Xmader would you mind reviewing this wesgarland/ctx-module#9 when you have the time?

@mxrch
Copy link

mxrch commented Feb 6, 2025

@wiwichips I reviewed this and it works perfectly on Windows :) imho you can merge it

@wiwichips
Copy link
Collaborator

@wesgarland @Xmader -- Please check out my PR on ctx-module (wesgarland/ctx-module#9) it may possibly affect Bifrost2 on Windows

@Xmader
Copy link
Member

Xmader commented Feb 6, 2025

Thanks @wiwichips and @mxrch! I believe only @wesgarland can merge it.

@wiwichips
Copy link
Collaborator

@Xmader would you mind asking Wes to merge it, after you review it. It may possibly affect Bifrost2 on Windows

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

5 participants