Skip to content

Conversation

@RaoulSchaffranek
Copy link
Member

Foundry allows overriding settings inside the foundry.toml file by defining environment variables.
See: https://getfoundry.sh/config/reference/overview#environment-variables

This PR mimics the behavior in Kontrol.

Use case: I want to add the ability to run Kontrol from Simbolik. Since Simbolik frequently recompiles the codebase for different purposes (e.g. linting/testing/debugging) it's necessary to have different out folders for each case to avoid unexpected behaviors. For example, trying to debug a code that was compiled for linting will fail because the compilation artifacts lack essential debugging information. Additionally, recompiling for debugging shouldn't delete all Kontrol proofs.

The changes introduced in this PR, allow Simbolik to control the out folder via an environment variable. The idea is to have different out folders for each purpose, namely out/simbolik, out/lint, out/test, out/coverage, and out/kontrol. The same principle is also applied to cache-folders.

While this could be handled via different profiles inside the foundry.toml, we cannot rely on the user to make these adjustments.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds support for environment variables to override settings from foundry.toml, mimicking Foundry's behavior where configuration values can be overridden using FOUNDRY_* prefixed environment variables. The primary motivation is to allow external tools like Simbolik to control the output directory without requiring users to modify their foundry.toml files.

Changes:

  • Introduced a new config() method in the Foundry class to check environment variables before falling back to foundry.toml profile settings
  • Updated properties (out, build_info, test_path, all_tests) to use the new config() method instead of directly accessing the profile
  • Modified exec_load_state to use the new configuration system
  • Updated integration tests to use the new test_path property

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/kontrol/foundry.py Added config() method and test_path property; updated out, build_info, and all_tests to use config()
src/kontrol/main.py Modified exec_load_state to use config() method for determining output directory
src/tests/integration/test_foundry_prove.py Updated tests to use foundry.test_path instead of manually constructing the path
Comments suppressed due to low confidence (2)

src/tests/integration/test_foundry_prove.py:800

  • The test will fail because foundry.test_path returns foundry_root_dir / 'test' (based on the foundry.toml config test = 'test'), but line 799-800 expect the generated files to be in foundry_root_dir / 'src'. The files will be generated in the 'test' directory but read from the 'src' directory, causing a FileNotFoundError. Either change output_dir=foundry.test_path to output_dir=foundry._root / 'src' to match the expected file locations, or remove the conflicting 'output_dir_name': 'src' from options (which is currently unused anyway).
        output_dir=foundry.test_path,
    )

    generated_main_file = foundry_root_dir / 'src' / 'LoadStateDiff.sol'
    generated_code_file = foundry_root_dir / 'src' / 'LoadStateDiffCode.sol'

src/tests/integration/test_foundry_prove.py:842

  • The test will fail because foundry.test_path returns foundry_root_dir / 'test' (based on the foundry.toml config test = 'test'), but lines 841-842 expect the generated files to be in foundry_root_dir / 'src'. The files will be generated in the 'test' directory but read from the 'src' directory, causing a FileNotFoundError. Either change output_dir=foundry.test_path to output_dir=foundry._root / 'src' to match the expected file locations, or remove the conflicting 'output_dir_name': 'src' from options (which is currently unused anyway).
        output_dir=foundry.test_path,
    )

    generated_main_file = foundry_root_dir / 'src' / 'LoadStateDump.sol'
    generated_code_file = foundry_root_dir / 'src' / 'LoadStateDumpCode.sol'

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.

2 participants