-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix self-hosted test failures #662
Comments
python.2 fixed in 0d32585 |
Added common-lisp. Now that the underlying common-lisp issue with hash-maps is fixed, it reveals a new self-hosted failure (step4 issue with how nil is handled). |
More recent CI run (with awk and python.2 fixed), is here: https://github.com/kanaka/mal/actions/runs/10393189805/job/28780104927 Some of the failures have some common patterns:
Other failure summaries:
|
@asarhaddon @wasamasa @dubek Anybody want to help me track down some fun self-hosted failures? |
c.2, cpp, dart, and swift5 fixed in 7b23c7e |
java and kotlin fixed in b31180c (last position of do was being EVAL'd twice). |
common-lisp fixed in 40aca13 |
Powershell fixed in 78000d6. However, we probably want to mark powershell as NO_SELF_HOST because the self-hosted tests take 26 minutes to run in GHA. The next longest pole in the tent is rpython which takes less than 10 minutes for everything (most of it is build time). |
I looked at debug logs from jq running |
Okay, found it: jq chokes on
Need to add a tests for |
But then I run into a new issue in mal step3:
more fun with jq awaits... |
ps/postscript fixed in a160866 Missing a "pop" in the |
I have a fix for the sml mlton self-host issue: 4719e58 However, I haven't merged it yet because the test I added to catch the issue in non-self-hosted mode revealed a problem in xslt:
For some reason, the |
Do not expect much from me in the xslt field. A few years ago, I have given up merging eval and evalast. |
For xslt, there is apparently a way to set the recursion depth in saxon via |
xslt may never be able to do this but we would definitely like jq, nasm, and possibly vbs to be fixed for self-hosted tests. Ticket tracking fixes: #662
xslt may never be able to do this but we would definitely like jq, nasm, and possibly vbs to be fixed for self-hosted tests. Ticket tracking fixes: #662
Another jq update:
So |
I just added a commit that enables self-hosted mode by default and added NO_SELF_HOST=1 to |
I've added erlang and vbs to the list since those currently have NO_SELF_HOST=1 but the former ought to work and the latter is reported to work (but there is something wrong with the invocation it appears).
|
That is weird. That will be a good one to add to the tests once you figure it out. |
Erlang has a memory leak (even in non-self-hosted mode). Seems like memory is never reclaimed:
|
Next chapter on jq: This patch (whitespace changes ignored) fixes one bug: diff --git a/impls/jq/utils.jq b/impls/jq/utils.jq
index 7b0876b7..0ad1ec81 100644
--- a/impls/jq/utils.jq
+++ b/impls/jq/utils.jq
@@ -1,6 +1,6 @@
def _debug(ex):
. as $top
- | ex
+ | (ex + "\n")
| debug
| $top;
@@ -45,6 +45,9 @@ def find_free_references(keys):
| if .kind == "symbol" then
if keys | contains([$dot.value]) then [] else [$dot.value] end
else if "list" == $dot.kind then
+ if $dot.value|length == 0 then
+ []
+ else
# if - scan args
# def! - scan body
# let* - add keys sequentially, scan body
@@ -69,6 +72,7 @@ def find_free_references(keys):
else
[ $dot.values[1:][] | _refs ]
end
+ end
else if "vector" == $dot.kind then
($dot.value | map(_refs) | reduce .[] as $x ([]; . + $x))
else if "hashmap" == $dot.kind then And now running mal step2 no longer exits, but the mal step2 tests fail:
So there's another thing lurking there. |
Silly me, my change with the |
OK first step towards jq is in PR #668. Then I discovered another issue in jq: if we introduce this test:
Then on the last line instead of 13 we get:
I assume that But even after changing the order of functions in |
@dubek yeah, I did a little playing around and I think the problem is that jq envs are not actually mutable. They are more like Clojure maps in that a copy is created when a value is added or remove and the new version is the one that is used. This actually is a legitimate way of treating environments but it's not the intended path for mal. I think we've essentially been relying on self-hosted tests to test this mutable property of mal envs. So we probably need a test like this in step4 tests:
The above will fail in jq. The final test will return 12 instead of 2000. But I'm also confident that there is a way to solve this in jq because the jq implementation supports atom types which definitely seem to be mutable in jq. It probably won't be easy to convert env to work like atoms (the handling of atom types doesn't appear to be confined to the atom related functions unfortunately). Maybe you can find an easier workaround that doesn't require a full rewrite of env handling. But if not, we'll probably need to understand fully how the jq implementation is doing atom types and use that mechanism for env handling. 🏋️ |
The above comment looks as if it could close #645 |
I just read through this issue (looking at jq in particular), here are some comments:
Yes, jq does not have mutable state at all, jqmal tries to hide this by late-binding free variables in functions, but modifying an already-bound variable is not really possible in general without a lot of pain-
Atoms have unique identities, it's relatively simple to do the same with bindings (stamp the env entry with an identifier and do a little dance where looking up is equivalent to: def env_get2($name; $current_env; $fallback_env):
$name | env_get($current_env) as $v | $v.identity | env_get_by_id($fallback_env) // $v.entry That said, I don't think the guide actually requires this name-binding behaviour? |
Add io self-hosted failue in step 0: https://github.com/kanaka/mal/actions/runs/11524734437/job/32085546455?pr=699 |
It's been a while since I've done a comprehensive self-host test run. I've add a input variable to workflow so that self-hosted mode can be added for manually triggered workflows. The CI run is at: https://github.com/kanaka/mal/actions/runs/10309343464/job/28538806985
Some of these implementations might just need a
NO_SELF_HOST
setting (such asxslt
), but a lot of the failures look like bugs rather than timeouts or memory exhaustion.Once all these have been fixed or
NO_SELF_HOST
is added, we should make self-hosted test step enabled by default (it only adds a few minutes overall to the full CI workflow).UPDATE: self-host mode has been made the default for workflows and the remaining broken implementations already had or have been updated with NO_SELF_HOST (erlang, jq, nasm, xslt, vbs).
Here is the the full list of self-hosted failures with very brief statement of the issue (see comments for more details):
awk
c.2
cpp
common-lisp
dart
erlang
- memory leak (e.g.(sumdown 40000)
)io
java
jq
- empty lists inside functions/closures are failingkotlin
nasm
- Needs more heap. With increased heap second quasiquote call failsps
powershell
python.2
sml
vala
vbs
(vbscript) - startup/invocation errorxslt
- memory/recursion limitswift5
The text was updated successfully, but these errors were encountered: