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

CI fails on pyduktape #202

Closed
aucampia opened this issue Sep 3, 2023 · 5 comments · Fixed by #203
Closed

CI fails on pyduktape #202

aucampia opened this issue Sep 3, 2023 · 5 comments · Fixed by #203

Comments

@aucampia
Copy link
Member

aucampia commented Sep 3, 2023

The CI is failing on:

https://drone.rdflib.ashs.dev/RDFLib/pySHACL/146/2/2

	  Error compiling Cython file:
17s
126	  ------------------------------------------------------------
17s
127	  ...
17s
128	          duk_print_alert_init(self.ctx, 0)
17s
129	          self._setup_module_search_function()
17s
130	  
17s
131	      cdef void _setup_module_search_function(self):
17s
132	          duk_get_global_string(self.ctx, 'Duktape')
17s
133	          duk_push_c_function(self.ctx, module_search, 1)
17s
134	                                        ^
17s
135	  ------------------------------------------------------------
17s
136	  
17s
137	  pyduktape2.pyx:167:38: Cannot assign type 'duk_ret_t (duk_context *) except? -1' to 'duk_c_function'. Exception values are incompatible. Suggest adding 'noexcept' to type 'duk_ret_t (duk_context *) except? -1'.
17s
138	  
17s
139	  Error compiling Cython file:
17s
140	  ------------------------------------------------------------
17s
141	  ...
17s
142	  
17s
143	  
17s
144	  cdef duk_ret_t safe_new(duk_context *ctx, int nargs):
17s
145	      # [ constructor arg1 arg2 ... argn nargs ]
17s
146	      duk_push_int(ctx, nargs)
17s
147	      return duk_safe_call(ctx, call_new, NULL, nargs + 2, 1)
17s
148	                                ^
@aucampia
Copy link
Member Author

aucampia commented Sep 3, 2023

Upstream issue: phith0n/pyduktape2#14 and fix phith0n/pyduktape2#15.

On a side note, is there something https://github.com/phith0n/pyduktape2/ provides that https://pypi.org/project/dukpy/ does not? As the second seems more popular.

@aucampia
Copy link
Member Author

aucampia commented Sep 3, 2023

I don't think there is a workaround because pyduktape2 does not publish a wheel, and the failure is during wheel building from the pyduktape2 sdist and poetry does not have an option that can be used to restrict the version of cython that is used.

@aucampia
Copy link
Member Author

aucampia commented Sep 3, 2023

Another JavaScript library for python: https://github.com/Distributive-Network/PythonMonkey#pythonmonkey

@aucampia
Copy link
Member Author

aucampia commented Sep 3, 2023

@ashleysommer not sure what you think is the best thing to do here, happy to help out depending on your choice. Having the CI and make test work is probably the most important for me, so options I can see:

@ashleysommer
Copy link
Collaborator

ashleysommer commented Sep 3, 2023

Hi @aucampia
Yep, I'm aware of this issue and I have been working on a fix for it within PySHACL.

In the meantime, I believe it is okay to simply make testing with pyduktape2 optional (in the same way berkeleydb is for RDFLib) as per your suggestion.

On a side note, is there something https://github.com/phith0n/pyduktape2/ provides that https://pypi.org/project/dukpy/ does not? As the second seems more popular.

Back in 2019 when I began implementation of the SHACL-JS feature set, the only options I could find were stefano/pyduktape, that didn't support Python3, or the fork phith0n/pyduktape2 that was more up to date and did support Python3. If DukPy was around at the time, it was not popular enough to come up when searching PyPi, Github or Google, for lightweight JS engine wrappers for python. Just looking at it now, it seems to be a very feature-filled package with JSX transpilation, LESS processing, etc, that we don't need in PySHACL.

I do remember PythonMonkey was around, but it is a full implementation of the Mozilla SpiderMonkey engine, that is very heavy, it requires LLVM and Rust toolchains to compile, and is overkill for the simple scripting required in PySHACL.

Wait for upstream fixes (phith0n/pyduktape2#14), but I suspect this will take quite a long time.

I'm very happy to stick with pyduktape2, in the past I have found the maintainer to be very responsive and will endeavor to fix issues as they arise. As an example, only three hours after you filed your issue, your fix was merged a new release has been pushed our for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants