Skip to content

fix: honour already provided source files when compiling - #2193

Merged
ivokub merged 3 commits into
mainfrom
provided-files
Aug 31, 2026
Merged

fix: honour already provided source files when compiling#2193
ivokub merged 3 commits into
mainfrom
provided-files

Conversation

@ivokub

@ivokub ivokub commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

When we do:

compiler.Compile(field.KOALABEAR_16, cfg.GetMaxStaticHeight(), srcFiles...)

we can already provide all the source files. However, in the pre-PR implementation we still searched the file system for the file when looking for files to link even if it was already provided. This prevents for example embedding the arithmetization into a binary (through an archive or fs.FS implementation for example).

This PR fixes the case and makes the source file searcher to skip loading files from file system if it is already provided in a call to compiler.Compile.

Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
@ivokub
ivokub requested review from DavePearce and amkCha and a lite review from Copilot August 27, 2026 07:36
@ivokub ivokub self-assigned this Aug 27, 2026
@ivokub ivokub added the bug Something isn't working label Aug 27, 2026
@CLAassistant

CLAassistant commented Aug 27, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the ZkC compiler’s include-scanning logic to avoid re-reading source files from the host filesystem when those files were already supplied directly to compiler.Compile(...). This aligns compilation behavior with use cases where callers preload sources (e.g., from an archive or other non-filesystem source) and want includes to resolve without additional filesystem access.

Changes:

  • During include scanning, skip filesystem globbing/loading when the included path is already present in knownSourceFiles.
  • Canonicalize the include pattern path before checking/deduping against knownSourceFiles, matching how top-level inputs are keyed.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@DavePearce

Copy link
Copy Markdown
Contributor

This prevents for example embedding the arithmetization into a binary (through an archive or fs.FS implementation for example).

@ivokub This is not actually correct. There is a binary "compiled form" for any set of constraints which provides what you want. Specifically, you can run zkc compile -o constraints.bin main.zkc to get a single bin file. Within the existing arithmetisation, there is a Makefile target which does this here.

That said, I think this PR is fine and I am happy to merge it. I just wanted to make sure you knew that this was the original intent.

@DavePearce DavePearce left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@ivokub
ivokub merged commit 838186e into main Aug 31, 2026
22 checks passed
@ivokub
ivokub deleted the provided-files branch August 31, 2026 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants