rescript-compiler icon indicating copy to clipboard operation
rescript-compiler copied to clipboard

Escape character `/` doesn't work in rescript 10 in `%raw` templates

Open JasoonS opened this issue 2 years ago • 3 comments

Please see more details here: https://forum.rescript-lang.org/t/string-interpolations-in-raw/871/8?u=jasoons

Example:

%%raw(`
// look ma, regular JavaScript!
var message = \`\${"hello"}\`;
function greet(m) {
  console.log(m)
}
`)

In rescript 9 this produces:

// look ma, regular JavaScript!
var message = `${"hello"}`;
function greet(m) {
  console.log(m)
}

But in rescript >= 10 it produces:

// look ma, regular JavaScript!
var message = \`\${"hello"}\`;
function greet(m) {
  console.log(m)
}

You can reproduce this by adjusting the rescript version in the playground settings: https://rescript-lang.org/try?code=KTBOEMHcAoAMCgD0iAEAbA9hg1igtuADQqgCmA5gK5rigoBS4AbuAMoDGoAlgA4AuAQngs6eUgGdx4cqRQBeFAB1YigCQBvAEQALUmkyaAvsoDc8AGaUAduz5cMVlOTKk+0PAEoU6+ChTsHcQw0UgA6THJ3D3hDeFgPIA

JasoonS avatar May 02 '23 12:05 JasoonS

The same problem in Linux with the same onnx and onnxruntime packages versions.

python -m nuitka --version

2.3.10
Commercial: None
Python: 3.12.4 (main, Jun  7 2024, 06:33:07) [GCC 14.1.1 20240522]
Flavor: Arch Python
Executable: /usr/bin/python
OS: Linux
Arch: x86_64
Distribution: Arch None
Version C compiler: /usr/bin/gcc (gcc 14.1.1).

Binary program run:

Traceback (most recent call last):
  File "/home/*/1.py", line 1, in <module>
    import onnx
    
  File "/usr/lib/python3.12/site-packages/onnx/__init__.py", line 78, in <module onnx>
    from onnx.external_data_helper import (
    
  File "/usr/lib/python3.12/site-packages/onnx/external_data_helper.py", line 11, in <module onnx.external_data_helper>
    import onnx.onnx_cpp2py_export.checker as c_checker
    
ModuleNotFoundError: No module named 'onnx.onnx_cpp2py_export.checker'; 'onnx.onnx_cpp2py_export' is not a package

ioctl-user avatar Jun 27 '24 17:06 ioctl-user

this is only reproducible in python 3.12, 3.11 and below work fine

KRRT7 avatar Jun 28 '24 04:06 KRRT7

This is a package extension module, for which a bunch of work has been done for 2.4, not sure but maybe develop does it already.

python3.10 bin/nuitka --edit=onnx.onnx_cpp2py_export
Nuitka-Tools: Found 'onnx.onnx_cpp2py_export' as
Nuitka-Tools: 'C:\Python310_64\lib\site-packages\onnx\onnx_cpp2py_export.cp310-win_amd64.pyd'

I wouldn't know to explain Python version differences from it though. So, for 3.11 it worked with my current factory state, so I suppose develop works now too. Seems, there is really a need to release 2.4 as soon as possible, since this seems to affect quite a few packages. But this should never have worked and I am sure onnx worked before. Maybe 2.3 the added partial support breaks this too, although hotfixes were supposed to resolve that.

kayhayen avatar Jun 28 '24 10:06 kayhayen

So, main branch worked for me too, that would be 2.3.10 with 3.11 now too. I didn't need onnxruntime package, I hope that's not the difference?

kayhayen avatar Jun 28 '24 10:06 kayhayen

Well, with onnxruntime installed, indeed onnx import fails on main branch, so that's surprising, but true.

kayhayen avatar Jun 28 '24 10:06 kayhayen

So, onnx.onnx_cpp2py_export is not recognized as a package. And it's not, there is a directory of the same name with only .pyi files and there has to be some of incompatibility of our loader. It's clear, it doesn't set the values required to be imported from, so what's going on there, not sure.

kayhayen avatar Jun 28 '24 10:06 kayhayen

The duplicate labels seems unwarranted, my suspect is that DLLs or data files are missing for onnxruntime, so I deleted all data files of onnx in a virtualenv, leaving only Python files and that extension module, and Python still works, now checking onnxruntime in the same way.

kayhayen avatar Jun 28 '24 13:06 kayhayen

I think, there is only two one DLL and one extension module left now. I am very confused right now.

kayhayen avatar Jun 28 '24 13:06 kayhayen

So, I added a bit of code around the crash

print("onnx.onnx_cpp2py_export" in sys.modules)
print("onnx.onnx_cpp2py_export.checker" in sys.modules)
import onnx.onnx_cpp2py_export
print("onnx.onnx_cpp2py_export" in sys.modules)
print("onnx.onnx_cpp2py_export.checker" in sys.modules)
print(dir(onnx.onnx_cpp2py_export))
print(onnx.onnx_cpp2py_export.__spec__)

and it turns out that the module is already imported at the time, the from import happens, and checker is in sys.modules with Python, but not with Nuitka, but it's in the dir() and its even a module object, so it's not like creating the extension module went wrong.

It seems I need to seriously look at what could be causing a different in extension module loading here, which is of course our own loader. I don't see any inputs used, with all files deleted. And I would assume, that checker is manually made, so what could trigger that.

kayhayen avatar Jun 28 '24 13:06 kayhayen

So, I found the spot that causes it to be already imported, a top level from import in onnx did this, but now I made the onnx package essentially only the above code and the only difference is that when Nuitka imports it, there is nothing added for the checker module to sys.modules which is supposedly done by the extension module loaded there with the import.

kayhayen avatar Jun 28 '24 13:06 kayhayen

So, I went one step further and deleted the meta data.

kayhayen avatar Jun 28 '24 14:06 kayhayen

Ok, went I diffed the imports, I noticed that the checker is actually named onnx.onnx_cpp2py_export.checker in Python and not so in Nuitka, there it's called onnx_cpp2py_export.checker so it's not in the namespace.

Now maybe, this is not enough:

// For newer Python, this API was moved to global state
#if PYTHON_VERSION >= 0x3c0
#define _Py_PackageContext (_PyRuntime.imports.pkgcontext)
#endif

or not effective in this case. But since @KRRT7 said it works also not for 3.11, that might be untrue. But who knows, maybe stuff happening during LoadLibrary or what not.

kayhayen avatar Jun 28 '24 14:06 kayhayen

I seem to not reproduce the issue I am seeing with 3.11, so this is 3.12 specific.

The extension module is a single phase module, i.e. loads immediately, and doesn't pick up the package name its supposed to live in for Nuitka as result. I don't know what its using to determine the package. I need to consult 3.12 source code, maybe my define does the wrong thing these days.

kayhayen avatar Jun 28 '24 16:06 kayhayen

So, turns out, this cannot be solved. The value Nuitka needs to set for extension modules to know what package they load into, was not moved to the runtime, there it's actually unused, but instead to a thread local private variable, we have no access to. We can make configuration for post load code, that cleans it up for this package, but a lot of packages will be affected.

Instead, what we will need to do is to make use of a fork of CPython 3.12 with changes that allow this extension module to be loaded by Nuitka. This will be based off conda-forge Python, and lead to Nuitka having to download it and use it instead of the standard Python. There will be a bigger delay until this is resolved. I will be doing it for 2.5 release I hope, but 2.4 will go out without a fix for this.

Using Nuitka will self-built Python will in the future also require basing off this Nuitka-standalone-backend repo of ours. The good thing is that it probably will be easy to make it available with conda as a separate source.

Already with 3.11 it was unclear if it can be made compatible, and 3.12 finally broke it. Was maybe bound to happen.

For now, I need to ask people who use standalone to use 3.11 with Nuitka.

kayhayen avatar Jul 08 '24 06:07 kayhayen

For now, I need to ask people who use standalone to use 3.11 with Nuitka.

@kayhayen, will it not work on 3.13 either?

Pro100rus32 avatar Nov 18 '24 15:11 Pro100rus32

I didn't check if it can be any better there yet.

kayhayen avatar Nov 18 '24 15:11 kayhayen

So, I checked, and unsurprisingly it's not better. Would have been cool, but I am pretty set on the idea of a fork being necessary and useful anyway, so no big harm long term.

kayhayen avatar Nov 20 '24 10:11 kayhayen

So, I checked, and unsurprisingly it's not better. Would have been cool, but I am pretty set on the idea of a fork being necessary and useful anyway, so no big harm long term.

@kayhayen, Looking forward to the fix this : )

Pro100rus32 avatar Nov 20 '24 21:11 Pro100rus32

We could make a fix, with a proxy module added beforehand maybe. Can somebody try and make one of these things that forward getattr and maybe setattr and put it as c and redirect to the module actually created.

Given a Python prototype code that's working, populating sys.modules[onnx.onnx_cpp2py_export.checker] and maybe others with that, pointing them to sys.modules[onnx_cpp2py_export.checker might just do the trick.

kayhayen avatar Nov 21 '24 09:11 kayhayen

I came across another package that has that issue, and actually came up with a workaround, so the next hotfix will have this. I am putting factory tag a bit early here, it's coming during the day as part of 2.6.3 hopefully anyway.

kayhayen avatar Feb 03 '25 10:02 kayhayen

This is the kind of config needed

- module-name: 'onnx.onnx_cpp2py_export' # checksum: 6d25b34c
  implicit-imports:
    - post-import-code:
        - |
          import sys
          onnx_cpp2py_export = sys.modules["onnx.onnx_cpp2py_export"]
          onnx_cpp2py_export.__path__ = []

          for sub_module_name in [key for key in sys.modules.keys() if key.startswith("onnx_cpp2py_export.")]:
            sys.modules["onnx."+sub_module_name] = sys.modules.pop(sub_module_name)
      when: 'python312_or_higher and not static_libpython'

It will probably become an import-hack config for 2.7 release, since this is very generic, and works for mediapipe as well, and I am currently looking at paddle which also was rejecting for those 3.12 configurations without static libpython, which sadly includes all of Windows.

kayhayen avatar Feb 03 '25 10:02 kayhayen

This is part of the current hotfix.

kayhayen avatar Feb 04 '25 17:02 kayhayen