From cabefe3797e6dc9240ea5be815d8021725bcb60b Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Wed, 7 Apr 2021 17:21:38 +0200 Subject: [PATCH] Fix ibc-proto recompilation bug (#802) * Fixed overwritten cosmos files * changelog * Fix typo in proto-compiler README * Fix bug where proto-compiler didn't find SDK repo if already cloned Co-authored-by: Romain Ruetschi --- CHANGELOG.md | 4 ++ proto-compiler/README.md | 8 ++-- proto-compiler/src/cmd/clone.rs | 40 ++++++++++------- proto-compiler/src/cmd/compile.rs | 73 +++++++++++++++++++++++++------ 4 files changed, 94 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd7ee92a5d..386961d0b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,9 @@ - Hermes guide: improved installation guideline ([#672]) - Make fee denom and amount configurable ([#754]) +- [ibc-proto] + - Fix for proto files re-compilation bug ([#801]) + ### BREAKING CHANGES - [ibc] @@ -92,6 +95,7 @@ [#772]: https://github.com/informalsystems/ibc-rs/issues/772 [#770]: https://github.com/informalsystems/ibc-rs/issues/770 [#798]: https://github.com/informalsystems/ibc-rs/issues/798 +[#801]: https://github.com/informalsystems/ibc-rs/issues/801 ## v0.1.1 diff --git a/proto-compiler/README.md b/proto-compiler/README.md index 0ea210c722..967fd3905d 100644 --- a/proto-compiler/README.md +++ b/proto-compiler/README.md @@ -23,7 +23,7 @@ Note: the full commit hash must be specified. Alternatively, one can check out a tag for the Cosmos SDK with the `--sdk-tag` option: ```bash -$ cargo run -- clone --out /tmp/cosmos --sdk-tag v0.42.1 --ibc-commit 333c1f338b2a14a1928a6f8ab64c37123c0e97b6 +$ cargo run -- clone --out /tmp/cosmos --sdk-tag v0.42.1 --ibc-go-commit 333c1f338b2a14a1928a6f8ab64c37123c0e97b6 ``` ### Generate Rust sources from Protobuf definitions @@ -34,6 +34,8 @@ To generate the Rust sources from the Protobuf definitions, and copy them to the $ cargo run -- compile --sdk /tmp/cosmos/sdk --ibc /tmp/cosmos/ibc --out ../proto/src/prost ``` -Additionally, this command will output the commit hash at which the Cosmos SDK is checked out into `$out/COSMOS_SDK_COMMIT`. +Additionally, this command will output the commit hash at which the Cosmos SDK is checked out into `$out/COSMOS_SDK_COMMIT` and +similarly the commit hash for IBC-go is saved into `$out/COSMOS_IBC_VERSION`. -This value is exposed via the `ibc_proto::COSMOS_SDK_VERSION` constant in the `ibc-proto` library. +The two commit values are exposed via the `ibc_proto::COSMOS_SDK_VERSION` and `ibc_proto::COSMOS_IBC_VERSION` +constants in the `ibc-proto` library. diff --git a/proto-compiler/src/cmd/clone.rs b/proto-compiler/src/cmd/clone.rs index 4cfe6e4352..1808a71388 100644 --- a/proto-compiler/src/cmd/clone.rs +++ b/proto-compiler/src/cmd/clone.rs @@ -25,6 +25,9 @@ pub struct CloneCmd { out: PathBuf, } +pub const COSMOS_SDK_URL: &str = "https://github.com/cosmos/cosmos-sdk"; +pub const IBC_GO_URL: &str = "https://github.com/cosmos/ibc-go"; + impl CloneCmd { pub fn validate(&self) { if self.sdk_commit.is_some() && self.sdk_tag.is_some() { @@ -48,27 +51,26 @@ impl CloneCmd { pub fn run(&self) { self.validate(); - let sdk_repo = if self.out.exists() { + let sdk_path = self.sdk_subdir(); + let sdk_repo = if sdk_path.exists() { println!( - "[info ] Found Cosmos SDK or IBC proto source at '{}'", - self.out.display() + "[info ] Found Cosmos SDK source at '{}'", + sdk_path.display() ); - Repository::open(&self.out).unwrap_or_else(|e| { + Repository::open(&sdk_path).unwrap_or_else(|e| { println!("[error] Failed to open repository: {}", e); process::exit(1) }) } else { println!("[info ] Cloning cosmos/cosmos-sdk repository..."); - let url = "https://github.com/cosmos/cosmos-sdk"; - - let repo = Repository::clone(url, &self.sdk_subdir()).unwrap_or_else(|e| { + let repo = Repository::clone(COSMOS_SDK_URL, &sdk_path).unwrap_or_else(|e| { println!("[error] Failed to clone the SDK repository: {}", e); process::exit(1) }); - println!("[info ] Cloned at '{}'", self.sdk_subdir().display()); + println!("[info ] Cloned at '{}'", sdk_path.display()); repo }; @@ -87,17 +89,25 @@ impl CloneCmd { println!("[info ] Cloning cosmos/ibc-go repository..."); - let ibc_url = "https://github.com/cosmos/ibc-go"; + let ibc_path = self.ibc_subdir(); + let ibc_repo = if ibc_path.exists() { + println!("[info ] Found IBC Go source at '{}'", sdk_path.display()); - let ibc_repo = Repository::clone(ibc_url, &self.ibc_subdir()).unwrap_or_else(|e| { - println!("[error] Failed to clone the IBC repository: {}", e); - process::exit(1) - }); + Repository::open(&sdk_path).unwrap_or_else(|e| { + println!("[error] Failed to open repository: {}", e); + process::exit(1) + }) + } else { + Repository::clone(IBC_GO_URL, &ibc_path).unwrap_or_else(|e| { + println!("[error] Failed to clone the IBC Go repository: {}", e); + process::exit(1) + }) + }; - println!("[info ] Cloned at '{}'", self.ibc_subdir().display()); + println!("[info ] Cloned at '{}'", ibc_path.display()); checkout_commit(&ibc_repo, &self.ibc_go_commit).unwrap_or_else(|e| { println!( - "[error] Failed to checkout IBC commit {}: {}", + "[error] Failed to checkout IBC Go commit {}: {}", self.ibc_go_commit, e ); process::exit(1) diff --git a/proto-compiler/src/cmd/compile.rs b/proto-compiler/src/cmd/compile.rs index 733ee8b5ee..92b389f4f4 100644 --- a/proto-compiler/src/cmd/compile.rs +++ b/proto-compiler/src/cmd/compile.rs @@ -27,13 +27,16 @@ pub struct CompileCmd { impl CompileCmd { pub fn run(&self) { - let tmp = TempDir::new("ibc-proto").unwrap(); + let tmp_sdk = TempDir::new("ibc-proto-sdk").unwrap(); + Self::output_version(&self.sdk, tmp_sdk.as_ref(), "COSMOS_SDK_COMMIT"); + Self::compile_sdk_protos(&self.sdk, tmp_sdk.as_ref()); - Self::output_version(&self.sdk, tmp.as_ref(), "COSMOS_SDK_COMMIT"); - Self::output_version(&self.ibc, tmp.as_ref(), "COSMOS_IBC_COMMIT"); - Self::compile_sdk_protos(&self.sdk, tmp.as_ref()); - Self::compile_ibc_protos(&self.ibc, &self.sdk, tmp.as_ref()); - Self::copy_generated_files(tmp.as_ref(), &self.out); + let tmp_ibc = TempDir::new("ibc-proto-ibc-go").unwrap(); + Self::output_version(&self.ibc, tmp_ibc.as_ref(), "COSMOS_IBC_COMMIT"); + Self::compile_ibc_protos(&self.ibc, tmp_ibc.as_ref()); + + // Merge the generated files into a single directory, taking care not to overwrite anything + Self::copy_generated_files(tmp_sdk.as_ref(), tmp_ibc.as_ref(), &self.out); } fn output_version(dir: &Path, out_dir: &Path, commit_file: &str) { @@ -45,7 +48,7 @@ impl CompileCmd { std::fs::write(path, rev).unwrap(); } - fn compile_ibc_protos(ibc_dir: &Path, sdk_dir: &Path, out_dir: &Path) { + fn compile_ibc_protos(ibc_dir: &Path, out_dir: &Path) { println!( "[info ] Compiling IBC .proto files to Rust into '{}'...", out_dir.display() @@ -59,7 +62,6 @@ impl CompileCmd { let proto_includes_paths = [ format!("{}/proto", ibc_dir.display()), - format!("{}/proto/cosmos", sdk_dir.display()), format!("{}/third_party/proto", ibc_dir.display()), ]; @@ -182,7 +184,7 @@ impl CompileCmd { } } - fn copy_generated_files(from_dir: &Path, to_dir: &Path) { + fn copy_generated_files(from_dir_sdk: &Path, from_dir_ibc: &Path, to_dir: &Path) { println!( "[info ] Copying generated files into '{}'...", to_dir.display() @@ -193,7 +195,8 @@ impl CompileCmd { create_dir_all(&to_dir).unwrap(); // Copy new compiled files (prost does not use folder structures) - let errors = WalkDir::new(from_dir) + // Copy the SDK files first + let errors_sdk = WalkDir::new(from_dir_sdk) .into_iter() .filter_map(|e| e.ok()) .filter(|e| e.file_type().is_file()) @@ -210,9 +213,53 @@ impl CompileCmd { .filter_map(|e| e.err()) .collect::>(); - if !errors.is_empty() { - for e in errors { - println!("[error] Error while copying compiled file: {}", e); + if !errors_sdk.is_empty() { + for e in errors_sdk { + println!("[error] Error while copying SDK-compiled file: {}", e); + } + + panic!("[error] Aborted."); + } + + // Copy the IBC-go files second, double-checking if anything is overwritten + let errors_ibc = WalkDir::new(from_dir_ibc) + .into_iter() + .filter_map(|e| e.ok()) + .filter(|e| e.file_type().is_file()) + .map(|e| { + let generated_fname = e.file_name().to_owned().into_string().unwrap(); + let prefix = &generated_fname[0..6]; + + let target_fname = format!( + "{}/{}", + to_dir.display(), + generated_fname, + ); + + // If it's a cosmos-relevant file and it exists, we should not overwrite it. + if Path::new(&target_fname).exists() && prefix.eq("cosmos") { + let original_cosmos_file = std::fs::read(target_fname.clone()).unwrap(); + let new_cosmos_file = std::fs::read(e.path()).unwrap(); + if original_cosmos_file != new_cosmos_file { + println!( + "[warn ] Cosmos-related file exists already {}! Ignoring the one generated from IBC-go {:?}", + target_fname, e.path() + ); + } + Ok(0) + } else { + copy( + e.path(), + target_fname, + ) + } + }) + .filter_map(|e| e.err()) + .collect::>(); + + if !errors_ibc.is_empty() { + for e in errors_ibc { + println!("[error] Error while copying IBC-go compiled file: {}", e); } panic!("[error] Aborted.");