-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
forge: configurable build-info path #2223
Conversation
cli/src/cmd/forge/build/core.rs
Outdated
pub build_info: bool, | ||
|
||
#[clap( | ||
help_heading = "PROJECT OPTIONS", | ||
help = "Output path to directory that build info files will be written to.", | ||
long | ||
)] | ||
#[serde(skip_serializing_if = "Option::is_none")] | ||
pub build_info_path: Option<PathBuf>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a way to unify this via
build_info: Option<Option<PathBuf>>
which works as flag --build-info
but also accepts a path: --build-info <path>
but perhaps it's better to have both arguments? if so we need a requires = "build-info"
clap attribute
build_info: Option<Option<PathBuf>>
would need manual serialization in the Provider
impl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a fix that adds the requires = "build-info"
. I am open to unify but think that it will add some complexity with the toml + hh plugin. What would toml config look like just turning it on without specifying a path so that the default value is used? Same for configuring via hh, would need to use a union type of bool/string. Also open to leave as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sg
Allows the user to configure the output path of the emitted build-info files. This is useful because tooling that integrates with hardhat expects the `build-info` directory to exist at a particular location and `forge` by default outputs it at a different location. A new cli flag to `forge build` is added: `--build-info-path`. It is implemented as an `Option<PathBuf>` so that `ethers-rs` can handle the default values for it when it is not configured.
d7d4e45
to
fa24dc3
Compare
cargo update ethers, build-info PR is merged |
* forge: configurable build-info path Allows the user to configure the output path of the emitted build-info files. This is useful because tooling that integrates with hardhat expects the `build-info` directory to exist at a particular location and `forge` by default outputs it at a different location. A new cli flag to `forge build` is added: `--build-info-path`. It is implemented as an `Option<PathBuf>` so that `ethers-rs` can handle the default values for it when it is not configured. * bump ethers Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Motivation
I'd like more flexibility over the output directory of the
BuildInfo
json files so that slither in hardhat mode just works with foundry.Solution
Allows the user to configure the output path of the
emitted build-info files. This is useful because tooling
that integrates with hardhat expects the
build-info
directoryto exist at a particular location and
forge
by default outputsit at a different location.
A new cli flag to
forge build
is added:--build-info-path
.It is implemented as an
Option<PathBuf>
so thatethers-rs
can handle the default values for it when it is not configured.