RDFLib / pySHACL

A Python validator for SHACL
Apache License 2.0
241 stars 63 forks source link

CI fails on pyduktape #202

Closed aucampia closed 10 months ago

aucampia commented 10 months ago

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 commented 10 months ago

Upstream issue: https://github.com/phith0n/pyduktape2/issues/14 and fix https://github.com/phith0n/pyduktape2/pull/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 commented 10 months ago

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 commented 10 months ago

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

aucampia commented 10 months ago

@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 commented 10 months ago

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 (https://github.com/phith0n/pyduktape2/issues/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.