Skip to content
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

Use a relative path in Makefile to avoid corset executable conflicts #230

Closed
gusiri opened this issue Jun 4, 2024 · 2 comments
Closed

Comments

@gusiri
Copy link
Contributor

gusiri commented Jun 4, 2024

Current issue:

I found that using CORSET ?= corset and ${CORSET} in the Makefile can be problematic. It relies on the existence of an executable named "corset" in the current working directory, which can lead to conflicts with older versions or other executables with the same name (e.g., old executable generated by cargo install --path.).

This fixes issue #143
which might be caused by ${CORSET} looking at another executable instead of ../corset/target/release/corset


Proposed solution:

Let's change this to a specific path ../corset/target/release/corset, similar to how it's done in prover/Makefile. This ensures the Makefile always uses the intended executable from a clear location.

For example, this will simply fix problems caused by corset executable conflicts

# instead of `CORSET ?= corset` and `${CORSET}`

define.go: ${ZKEVM_MODULES}
	../corset/target/release/corset wizard-iop -vv -P define -o $@ ${ZKEVM_MODULES}

zkevm.bin: ${ZKEVM_MODULES}
	../corset/target/release/corset compile -vv -o $@ ${ZKEVM_MODULES}

@gusiri gusiri self-assigned this Jun 4, 2024
@gusiri
Copy link
Contributor Author

gusiri commented Jun 4, 2024

CI build fails

CORSET=$HOME/corset make -B zkevm.bin
shell: /usr/bin/bash -e {0}
make: ../corset/target/release/corset: No such file or directory
make: *** [Makefile:169: zkevm.bin] Error 127

@gusiri gusiri changed the title Use a specific path in Makefile to avoid corset executable conflicts Use a relative path in Makefile to avoid corset executable conflicts Jun 4, 2024
@gusiri
Copy link
Contributor Author

gusiri commented Jun 5, 2024

Closing as Corset can be located and tested at different relative locations (e.g, CI build docker)

@gusiri gusiri closed this as completed Jun 5, 2024
@gusiri gusiri removed their assignment Jun 6, 2024
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

When branches are created from issues, their pull requests are automatically linked.

1 participant