Skip to content

Add JSU Compiler with JMC Backend for C, Python, Java, JS and Perl.#180

Merged
jviotti merged 24 commits intosourcemeta-research:mainfrom
zx80:jsuc
Apr 15, 2026
Merged

Add JSU Compiler with JMC Backend for C, Python, Java, JS and Perl.#180
jviotti merged 24 commits intosourcemeta-research:mainfrom
zx80:jsuc

Conversation

@zx80
Copy link
Copy Markdown
Contributor

@zx80 zx80 commented Mar 4, 2026

This uses the JSU compiler which uses JMC as a backend for Python, C, Java, JS and Perl.

@jviotti
Copy link
Copy Markdown
Member

jviotti commented Mar 5, 2026

Try to merge main back in. The Blaze build should be fine now

Comment thread implementations/jsu-c/generate-and-run.sh Outdated
Comment thread implementations/jsu-c/generate-and-run.sh Outdated
Comment thread implementations/jsu-c/generate-and-run.sh Outdated
Comment thread implementations/jsu-c/generate-and-run.sh Outdated
Comment thread implementations/jsu-py/version.sh Outdated
Comment thread implementations/jsu-c/README.md
Comment thread implementations/jsu-c/README.md Outdated
@jviotti
Copy link
Copy Markdown
Member

jviotti commented Mar 12, 2026 via email

@jviotti
Copy link
Copy Markdown
Member

jviotti commented Mar 13, 2026

@michaelmior You might want to review this too in the mean-time!

@zx80 zx80 requested a review from jviotti March 14, 2026 14:24
@zx80
Copy link
Copy Markdown
Contributor Author

zx80 commented Mar 14, 2026

Hmmm, the aggregate tasks fails early on:

Error: An error occurred trying to start process '/usr/bin/bash' with working directory '/home/runner/work/jsonschema-benchmark/jsonschema-benchmark'. Argument list too long

I guess shell limits are reached after adding 5 implementations, some re-engineering will be needed.
After some investigation, it seems that the culprit is:

 printf "${OUTPUTS}" | awk 'NR==1 || !/^implementation,/' > dist/report.csv

The OUTPUTS environment variable contains the whole CSV file contents, which must be over 128 kB…
This should probably be turned into artifacts instead.

@zx80
Copy link
Copy Markdown
Contributor Author

zx80 commented Mar 14, 2026

I succeeded in updating the CI to use artifact uploads instead of output transfers so as to avoid failing on the maximum size of an environment variable.

Now the aggregate tasks fails on missing crendentials when uploading the results, which is the usual.

Comment thread implementations/jsu-c/benchmark.sh Outdated
@jviotti
Copy link
Copy Markdown
Member

jviotti commented Mar 17, 2026

I left a comment regarding pipes but otherwise looks good. @michaelmior can you also do a pass on this, as you wrote the vast majority of the surrounding scripts for measuring, etc?

@zx80 zx80 changed the title Add JSU Compiler with JMC Backend for C, Python, JS and Perl. Add JSU Compiler with JMC Backend for C, Python, Java, JS and Perl. Mar 24, 2026
@zx80
Copy link
Copy Markdown
Contributor Author

zx80 commented Mar 24, 2026

Hello Juan,

I'm unsure about what I can do to help this PR move forward.

FYI, in complement to JSU Python backend variant and thanks to Julian, the JSU C, Java, JS and Perl backend variants have been integrated (C commit and Java/JS/Perl commit) to bowtie, thus should be listed on the next report update.

The August PR (a resubmission of this May PR that I wrongly thought had been merged) about listing these tools on the JSON Schema website seems as stuck as ever.

Comment thread .github/workflows/ci.yml Outdated
Comment thread implementations/jsu-java/Dockerfile Outdated
Comment thread implementations/jsu-java/Dockerfile
Comment thread implementations/jsu-pl/Dockerfile Outdated
Comment thread implementations/jsu-c/Dockerfile
Comment thread implementations/jsu-js/Dockerfile Outdated
Comment thread implementations/jsu-pl/Dockerfile
Comment thread implementations/jsu-py/Dockerfile
Comment thread implementations/jsu-py/Dockerfile Outdated
Comment thread Makefile Outdated
@michaelmior
Copy link
Copy Markdown
Contributor

Overall I think this is looking good although there are several minor things that should be addressed.

@zx80
Copy link
Copy Markdown
Contributor Author

zx80 commented Mar 24, 2026

Overall I think this is looking good although there are several minor things that should be addressed.

I think that I've addressed everything in the above push.

@zx80 zx80 requested review from jviotti and michaelmior March 25, 2026 08:37
Copy link
Copy Markdown
Member

@jviotti jviotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ready!

@zx80
Copy link
Copy Markdown
Contributor Author

zx80 commented Apr 2, 2026

I think it's ready!

Great! Thanks! When do you think it may be merged?

@jviotti
Copy link
Copy Markdown
Member

jviotti commented Apr 2, 2026

@zx80 Its pending @michaelmior 's re-review. I can't merge given the requested changes.

Screenshot 2026-04-02 at 07 16 05

@zx80
Copy link
Copy Markdown
Contributor Author

zx80 commented Apr 2, 2026

Ok!

@jviotti
Copy link
Copy Markdown
Member

jviotti commented Apr 10, 2026

Ping @michaelmior

@michaelmior michaelmior dismissed their stale review April 15, 2026 00:27

Changes addressed

@michaelmior
Copy link
Copy Markdown
Contributor

Sorry for the delay. I resolved my review so feel free to merge! @jviotti note that as an admin, you should be able to dismiss my review yourself to meet the merge requirements.

@jviotti
Copy link
Copy Markdown
Member

jviotti commented Apr 15, 2026

@michaelmior Ah, looks like it was not the case, but maybe some repo settings we could tweak. As per my screenshot above, I could not merge indeed.

@jviotti jviotti merged commit 70764bf into sourcemeta-research:main Apr 15, 2026
19 of 20 checks passed
@michaelmior
Copy link
Copy Markdown
Contributor

@jviotti Yes, I understand you can't directly bypass merging, but I think maybe you could have dismissed my review which would then have allowed you to merge. Anyway, it's done now :) Thanks @zx80!

@zx80
Copy link
Copy Markdown
Contributor Author

zx80 commented Apr 15, 2026

Thanks for the reviews and the merge!

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 this pull request may close these issues.

3 participants