Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

feat(abigen): emit cargo build script directives #2449

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

DaniPopes
Copy link
Collaborator

Motivation

... and an include_bytes! when not in build scripts. Signals Cargo to rebuild if the source files change.

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

nice, smol nit re error message otherwise lgtm

let contents = fs::read_to_string(&path).wrap_err("Could not read file")?;

Self::new(name, contents)
let path = path.as_ref().to_str().ok_or_else(|| eyre::eyre!("invalid path"))?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: error should include the path

/// Specify whether to print [cargo build script directives][ref] if the source is a path. By
/// default, this is true only when executing inside of a build script.
///
/// [ref]: https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script
Copy link
Collaborator

Choose a reason for hiding this comment

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

I need to adapt this href style

@@ -368,6 +396,11 @@ impl ContractBindings {
}
}

/// Returns whether the current executable is a cargo build script.
fn in_build_script() -> bool {
std::env::var("CARGO_MAKEFLAGS").is_ok()
Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

honestly i just went over all of the env vars, and this one was the only one that is set only on build scripts, per docs. Not 100% sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TARGET should be fine as well.

@DaniPopes DaniPopes force-pushed the feat/incremental-abigen branch 2 times, most recently from 89d392b to f6ce0d8 Compare May 31, 2023 22:10
@DaniPopes DaniPopes merged commit 401a47d into gakonst:master Jun 1, 2023
@DaniPopes DaniPopes deleted the feat/incremental-abigen branch June 1, 2023 06:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants