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

Make source.hash non-optional, remove metadata-only #104

Merged
merged 1 commit into from
Nov 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions metadata/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
//! let language = SourceLanguage::new(Language::Ink, Version::new(2, 1, 0));
//! let compiler = SourceCompiler::new(Compiler::RustC, Version::parse("1.46.0-nightly").unwrap());
//! let wasm = SourceWasm::new(vec![0u8]);
//! let source = Source::new(Some(wasm), Some(CodeHash([0u8; 32])), language, compiler);
//! let source = Source::new(Some(wasm), CodeHash([0u8; 32]), language, compiler);
//! let contract = Contract::builder()
//! .name("incrementer".to_string())
//! .version(Version::new(2, 1, 0))
Expand Down Expand Up @@ -114,8 +114,7 @@ impl Serialize for CodeHash {

#[derive(Clone, Debug, Serialize)]
pub struct Source {
#[serde(skip_serializing_if = "Option::is_none")]
hash: Option<CodeHash>,
hash: CodeHash,
language: SourceLanguage,
compiler: SourceCompiler,
#[serde(skip_serializing_if = "Option::is_none")]
Expand All @@ -126,7 +125,7 @@ impl Source {
/// Constructs a new InkProjectSource.
pub fn new(
wasm: Option<SourceWasm>,
hash: Option<CodeHash>,
hash: CodeHash,
language: SourceLanguage,
compiler: SourceCompiler,
) -> Self {
Expand Down Expand Up @@ -522,7 +521,7 @@ mod tests {
let compiler =
SourceCompiler::new(Compiler::RustC, Version::parse("1.46.0-nightly").unwrap());
let wasm = SourceWasm::new(vec![0u8, 1u8, 2u8]);
let source = Source::new(Some(wasm), Some(CodeHash([0u8; 32])), language, compiler);
let source = Source::new(Some(wasm), CodeHash([0u8; 32]), language, compiler);
let contract = Contract::builder()
.name("incrementer".to_string())
.version(Version::new(2, 1, 0))
Expand Down Expand Up @@ -604,7 +603,7 @@ mod tests {
let language = SourceLanguage::new(Language::Ink, Version::new(2, 1, 0));
let compiler =
SourceCompiler::new(Compiler::RustC, Version::parse("1.46.0-nightly").unwrap());
let source = Source::new(None, Some(CodeHash([0u8; 32])), language, compiler);
let source = Source::new(None, CodeHash([0u8; 32]), language, compiler);
let contract = Contract::builder()
.name("incrementer".to_string())
.version(Version::new(2, 1, 0))
Expand Down
20 changes: 5 additions & 15 deletions src/cmd/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,9 @@ impl GenerateMetadataCommand {

let generate_metadata = |manifest_path: &ManifestPath| -> Result<()> {
let mut current_progress = 4;
let curr_step = match self.build_artifact {
GenerateArtifacts::MetadataOnly => 1,
_ => current_progress,
};
println!(
" {} {}",
format!("[{}/{}]", curr_step, self.build_artifact.steps()).bold(),
format!("[{}/{}]", current_progress, self.build_artifact.steps()).bold(),
"Generating metadata".bright_green().bold()
);
let target_dir_arg = format!("--target-dir={}", target_directory.to_string_lossy());
Expand Down Expand Up @@ -167,13 +163,7 @@ impl GenerateMetadataCommand {
.transpose()?;
let homepage = self.crate_metadata.homepage.clone();
let license = contract_package.license.clone();
let (dest_wasm, hash, optimization_result) =
if self.build_artifact != GenerateArtifacts::MetadataOnly {
let (wasm, hash, optimization) = self.wasm_hash()?;
(Some(wasm), Some(hash), Some(optimization))
} else {
(None, None, None)
};
let (dest_wasm, hash, optimization_result) = self.wasm_hash()?;
let source = {
let lang = SourceLanguage::new(Language::Ink, ink_version.clone());
let compiler = SourceCompiler::new(Compiler::RustC, rust_version);
Expand All @@ -182,7 +172,7 @@ impl GenerateMetadataCommand {
// The Wasm which we read must have the same hash as `source.hash`
debug_assert!({
let expected = blake2_hash(wasm.as_slice());
Some(expected) == hash
expected == hash
});
Some(SourceWasm::new(wasm))
} else {
Expand Down Expand Up @@ -226,11 +216,11 @@ impl GenerateMetadataCommand {
let user = self.crate_metadata.user.clone().map(User::new);

Ok(ExtendedMetadataResult {
dest_wasm,
dest_wasm: Some(dest_wasm),
Comment on lines -229 to +219
Copy link
Contributor

Choose a reason for hiding this comment

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

Useful to be an Option?

Copy link
Collaborator Author

@cmichi cmichi Nov 10, 2020

Choose a reason for hiding this comment

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

So this is mostly used for cargo contract check (and previously metadata-only). check must never result in a dest_wasm in the target directory. So for the GenerationResult object dest_wasm was made an Option. There is also an assert for is_none() in the cargo contract check logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay cool, thanks!

source,
contract,
user,
optimization_result,
optimization_result: Some(optimization_result),
})
}

Expand Down
48 changes: 8 additions & 40 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,6 @@ pub enum GenerateArtifacts {
/// Only the Wasm is created, generation of metadata and a bundled `<name>.contract` file is skipped
#[structopt(name = "code-only")]
CodeOnly,
/// Only the Wasm and the metadata are generated, no bundled `<name>.contract` file is created
#[structopt(name = "metadata-only")]
MetadataOnly,
}

impl GenerateArtifacts {
Expand All @@ -173,24 +170,10 @@ impl GenerateArtifacts {
match self {
GenerateArtifacts::All => 5,
GenerateArtifacts::CodeOnly => 3,
GenerateArtifacts::MetadataOnly => 1,
}
}

pub fn display(&self, result: &GenerationResult) -> String {
if self == &GenerateArtifacts::MetadataOnly {
return format!(
"\nYour contract's metadata is ready. You can find it here:\n{}",
result
.dest_metadata
.as_ref()
.expect("metadata path must exist")
.display()
.to_string()
.bold()
);
}

let optimization = GenerationResult::display_optimization(result);
let size_diff = format!(
"\nOriginal wasm size: {}, Optimized: {}\n\n",
Expand Down Expand Up @@ -249,7 +232,6 @@ impl std::str::FromStr for GenerateArtifacts {
match artifact {
"all" => Ok(GenerateArtifacts::All),
"code-only" => Ok(GenerateArtifacts::CodeOnly),
"metadata-only" => Ok(GenerateArtifacts::MetadataOnly),
_ => Err("Could not parse build artifact".to_string()),
}
}
Expand Down Expand Up @@ -318,18 +300,13 @@ enum Command {
/// Which build artifacts to generate.
///
/// - `all`: Generate the Wasm, the metadata and a bundled `<name>.contract` file.
/// The metadata file includes the Wasm hash.
///
/// - `code-only`: Only the Wasm is created, generation of metadata and a bundled
/// `<name>.contract` file is skipped.
///
/// - `metadata-only`: Only the metadata iis generated, neither the bundled
/// `<name>.contract`, nor the Wasm file are created. The resulting metadata
/// does not contain the Wasm hash.
#[structopt(
long = "generate",
default_value = "all",
value_name = "all | code-only | metadata-only",
value_name = "all | code-only",
verbatim_doc_comment
)]
build_artifact: GenerateArtifacts,
Expand Down Expand Up @@ -421,22 +398,13 @@ fn exec(cmd: Command) -> Result<String> {
unstable_options,
} => {
let manifest_path = ManifestPath::try_from(manifest_path.as_ref())?;
let result = if build_artifact == &GenerateArtifacts::MetadataOnly {
cmd::metadata::execute(
&manifest_path,
verbosity.try_into()?,
*build_artifact,
unstable_options.try_into()?,
)?
} else {
cmd::build::execute(
&manifest_path,
verbosity.try_into()?,
true,
*build_artifact,
unstable_options.try_into()?,
)?
};
let result = cmd::build::execute(
&manifest_path,
verbosity.try_into()?,
true,
*build_artifact,
unstable_options.try_into()?,
)?;

Ok(build_artifact.display(&result))
}
Expand Down