Skip to content

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

Merged
jviotti merged 24 commits into
sourcemeta-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 into
sourcemeta-research:mainfrom
zx80:jsuc

Conversation

@zx80

@zx80 zx80 commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

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

@jviotti

jviotti commented Mar 5, 2026

Copy link
Copy Markdown
Member

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

jviotti commented Mar 12, 2026 via email

Copy link
Copy Markdown
Member

@jviotti

jviotti commented Mar 13, 2026

Copy link
Copy Markdown
Member

@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

zx80 commented Mar 14, 2026

Copy link
Copy Markdown
Contributor Author

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

zx80 commented Mar 14, 2026

Copy link
Copy Markdown
Contributor Author

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

jviotti commented Mar 17, 2026

Copy link
Copy Markdown
Member

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

zx80 commented Mar 24, 2026

Copy link
Copy Markdown
Contributor Author

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

zx80 commented Mar 24, 2026

Copy link
Copy Markdown
Contributor Author

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

@jviotti jviotti left a comment

Copy link
Copy Markdown
Member

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

zx80 commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

I think it's ready!

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

@jviotti

jviotti commented Apr 2, 2026

Copy link
Copy Markdown
Member

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

Screenshot 2026-04-02 at 07 16 05

@zx80

zx80 commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

Ok!

@jviotti

jviotti commented Apr 10, 2026

Copy link
Copy Markdown
Member

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

jviotti commented Apr 15, 2026

Copy link
Copy Markdown
Member

@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

zx80 commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

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