Improve foundry compilation #488
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smonicas
requested review from
montyly,
0xalpharush and
bohendo
as code owners
montyly
reviewed
Sep 15, 2023
There was a problem hiding this comment.
I am wondering if we should invert the logic for --foundry-compile-all and do something like:
- If there is a foundry project detected, and the target is a
. solfile, then do the auto compile - Have the user use
compile-force-framework solcif they dont want to do that
Because most likely the direct solc call will not work for most of the user anyway in a foundry project
What do you think?
| @@ -154,6 +176,19 @@ def is_dependency(self, path: str) -> bool: | |||
| """ | |||
| return False | |||
|
|
|||
| @staticmethod | |||
There was a problem hiding this comment.
What if we dont make it abstract, to avoid having a code duplication for all platforms, to just return None?
- Remove the auto flag, crytic-compile will directly use the auto feature if its on .sol file + the working director is a known platform - Remove abstract from config to reduce code footprint
|
I made the proposed improvements (remove the auto flag, and make |
montyly
approved these changes
Oct 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a new option
--auto-compilewhich can be used to compile a single file. At the moment it's implemented only for foundry. It will try to get information such as solc version to use, remappings... and pass them to solc. Example we want to run slither on a single file we can do from the root directoryslither src/MyFile.sol --auto-compile.Automatically skip compiling tests and scripts, to compile everything use
--foundry-compile-all.