From 743ed9475f7d195a33c01e48965c05822d907d23 Mon Sep 17 00:00:00 2001 From: Luca Colagrande Date: Tue, 5 Sep 2023 10:29:43 +0200 Subject: [PATCH] Improve ReadMe and Warning information for `vendor` upstream linking (#131) * README: Mention version attribute unsupported in bender vendor * Fix error when using bender vendor with version reference * vendor: Raise warning on file missing upstream * Update Changelog --------- Co-authored-by: Michael Rogenmoser --- CHANGELOG.md | 2 ++ README.md | 2 +- src/cmd/vendor.rs | 73 ++++++++++++++++++++++++----------------------- 3 files changed, 41 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c146c6a..d0c4adbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## Unreleased +### Fixed +- Improve ReadMe and Warning information for `vendor` upstream linking. ## 0.27.2 - 2023-07-12 ### Added diff --git a/README.md b/README.md index c0b48f74..3bfc9bba 100644 --- a/README.md +++ b/README.md @@ -182,7 +182,7 @@ vendor_package: - name: lowrisc_opentitan # target directory target_dir: vendor/lowrisc_opentitan - # upstream dependency (i.e. git repository similar to dependencies) + # upstream dependency (i.e. git repository similar to dependencies, only supports commit hash) upstream: { git: "https://github.com/lowRISC/opentitan.git", rev: "47a0f4798febd9e53dd131ef8c8c2b0255d8c139" } # paths to include from upstream dependency. Per default, all paths are included. Optional. include_from_upstream: diff --git a/src/cmd/vendor.rs b/src/cmd/vendor.rs index bc239928..1b243a0c 100644 --- a/src/cmd/vendor.rs +++ b/src/cmd/vendor.rs @@ -88,12 +88,8 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { DependencySource::Git(ref url) => { let git = Git::new(tmp_path, &sess.config.git); rt.block_on(async { - // TODO MICHAERO: May need throttle - future::lazy(|_| { - stageln!("Cloning", "{} ({})", vendor_package.name, url); - Ok(()) - }) - .and_then(|_| git.spawn_with(|c| c.arg("clone").arg(url).arg("."))) + stageln!("Cloning", "{} ({})", vendor_package.name, url); + git.spawn_with(|c| c.arg("clone").arg(url).arg(".")) .map_err(move |cause| { if url.contains("git@") { warnln!("Please ensure your public ssh key is added to the git server."); @@ -103,24 +99,17 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { format!("Failed to initialize git database in {:?}.", tmp_path), cause, ) - }) - .and_then(|_| git.spawn_with(|c| c.arg("checkout").arg(match vendor_package.upstream { - config::Dependency::GitRevision(_, ref rev) => rev, - // config::Dependency::GitVersion(_, ref ver) => ver.to_str(), - _ => unimplemented!(), - }))) - .and_then(|_| async { - let rev_hash = match vendor_package.upstream { - config::Dependency::GitRevision(_, ref rev) => rev, - _ => unimplemented!(), - }; - if *rev_hash != git.spawn_with(|c| c.arg("rev-parse").arg("--verify").arg(format!("{}^{{commit}}", rev_hash))).await?.trim_end_matches('\n') { - Err(Error::new("Please ensure your vendor reference is a commit hash to avoid upstream changes impacting your checkout")) - } else { - Ok(()) - } - }) - .await + }).await?; + let rev_hash = match vendor_package.upstream { + config::Dependency::GitRevision(_, ref rev) => Ok(rev), + _ => Err(Error::new("Please ensure your vendor reference is a commit hash to avoid upstream changes impacting your checkout")), + }?; + git.spawn_with(|c| c.arg("checkout").arg(rev_hash)).await?; + if *rev_hash != git.spawn_with(|c| c.arg("rev-parse").arg("--verify").arg(format!("{}^{{commit}}", rev_hash))).await?.trim_end_matches('\n') { + Err(Error::new("Please ensure your vendor reference is a commit hash to avoid upstream changes impacting your checkout")) + } else { + Ok(()) + } })?; tmp_path.to_path_buf() @@ -290,8 +279,15 @@ pub fn init( apply_patches(rt, git, vendor_package.name.clone(), patch_link.clone())?; } + // Check if includes exist + for path in vendor_package.include_from_upstream.clone() { + if !PathBuf::from(extend_paths(&[path.clone()], dep_path)[0].clone()).exists() { + warnln!("{} not found in upstream, continuing.", path); + } + } + // Copy src to dst recursively. - match &patch_link.from_prefix.prefix_paths(dep_path).is_dir() { + match link_from.is_dir() { true => copy_recursively( &link_from, &link_to, @@ -304,16 +300,23 @@ pub fn init( .collect(), )?, false => { - std::fs::copy(&link_from, &link_to).map_err(|cause| { - Error::chain( - format!( - "Failed to copy {} to {}.", - link_from.to_str().unwrap(), - link_to.to_str().unwrap(), - ), - cause, - ) - })?; + if link_from.exists() { + std::fs::copy(&link_from, &link_to).map_err(|cause| { + Error::chain( + format!( + "Failed to copy {} to {}.", + link_from.to_str().unwrap(), + link_to.to_str().unwrap(), + ), + cause, + ) + })?; + } else { + warnln!( + "{} not found in upstream, continuing.", + link_from.to_str().unwrap() + ); + } } };