From 4f3cb532bda854b507f5b3ef69e8c97e46b2c55f Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Thu, 24 Oct 2024 12:27:22 -0400 Subject: [PATCH 01/10] Add numbering for warnings --- src/cli.rs | 4 ++-- src/cmd/clone.rs | 4 ++-- src/cmd/fusesoc.rs | 2 +- src/cmd/parents.rs | 6 +++--- src/cmd/update.rs | 4 +++- src/cmd/vendor.rs | 10 +++++----- src/config.rs | 14 +++++++------- src/resolver.rs | 8 ++++---- src/sess.rs | 36 ++++++++++++++++++++---------------- 9 files changed, 47 insertions(+), 41 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 7dd6a7c8..3d7db1b3 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -224,7 +224,7 @@ pub fn main() -> Result<()> { })?; if !meta.file_type().is_symlink() { warnln!( - "Skipping link to package {} at {:?} since there is something there", + "[W01] Skipping link to package {} at {:?} since there is something there", pkg_name, path ); @@ -471,7 +471,7 @@ fn maybe_load_config(path: &Path, warn_config_loaded: bool) -> Result Result<()> { Err(Error::new("git fetch failed".to_string()))?; } } else { - warnln!("fetch not performed due to --local argument."); + warnln!("[W14] fetch not performed due to --local argument."); } println!( @@ -263,7 +263,7 @@ pub fn run(sess: &Session, path: &Path, matches: &ArgMatches) -> Result<()> { })?; if !meta.file_type().is_symlink() { warnln!( - "Skipping link to package {} at {:?} since there is something there", + "[W15] Skipping link to package {} at {:?} since there is something there", pkg_name, link_path ); diff --git a/src/cmd/fusesoc.rs b/src/cmd/fusesoc.rs index 82e9c91a..efe593e7 100644 --- a/src/cmd/fusesoc.rs +++ b/src/cmd/fusesoc.rs @@ -156,7 +156,7 @@ pub fn run_single(sess: &Session, matches: &ArgMatches) -> Result<()> { })?; if fuse_depend_string.len() > 1 { - warnln!("Depend strings may be wrong for the included dependencies!"); + warnln!("[W16] Depend strings may be wrong for the included dependencies!"); } Ok(()) diff --git a/src/cmd/parents.rs b/src/cmd/parents.rs index 5acc69ee..91e78027 100644 --- a/src/cmd/parents.rs +++ b/src/cmd/parents.rs @@ -59,7 +59,7 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { .unwrap(); // Filter out dependencies without a manifest if dep_manifest.is_none() { - warnln!("{} is shown to include dependency, but manifest does not have this information.", pkg_name.to_string()); + warnln!("[W17] {} is shown to include dependency, but manifest does not have this information.", pkg_name.to_string()); continue; } let dep_manifest = dep_manifest.unwrap(); @@ -79,7 +79,7 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { ); } else { // Filter out dependencies with mismatching manifest - warnln!("{} is shown to include dependency, but manifest does not have this information.", pkg_name.to_string()); + warnln!("[W17] {} is shown to include dependency, but manifest does not have this information.", pkg_name.to_string()); } } } @@ -132,7 +132,7 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { if sess.config.overrides.contains_key(dep) { warnln!( - "An override is configured for {} to {:?}", + "[W18] An override is configured for {} to {:?}", dep, sess.config.overrides[dep] ) diff --git a/src/cmd/update.rs b/src/cmd/update.rs index 67fd895b..8c8ad70d 100644 --- a/src/cmd/update.rs +++ b/src/cmd/update.rs @@ -65,7 +65,9 @@ pub fn new() -> Command { pub fn setup(matches: &ArgMatches) -> Result { let force_fetch = matches.get_flag("fetch"); if matches.get_flag("local") && matches.get_flag("fetch") { - warnln!("As --local argument is set for bender command, no fetching will be performed."); + warnln!( + "[W14] As --local argument is set for bender command, no fetching will be performed." + ); } Ok(force_fetch) } diff --git a/src/cmd/vendor.rs b/src/cmd/vendor.rs index 29a0faa0..8b1944c5 100644 --- a/src/cmd/vendor.rs +++ b/src/cmd/vendor.rs @@ -99,9 +99,9 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { git.clone().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."); + warnln!("[W07] Please ensure your public ssh key is added to the git server."); } - warnln!("Please ensure the url is correct and you have access to the repository."); + warnln!("[W07] Please ensure the url is correct and you have access to the repository."); Error::chain( format!("Failed to initialize git database in {:?}.", tmp_path), cause, @@ -280,7 +280,7 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { } }, None => { - warnln!("No patch directory specified for package {}, mapping {} => {}. Skipping patch generation.", vendor_package.name.clone(), patch_link.from_prefix.to_str().unwrap(), patch_link.to_prefix.to_str().unwrap()); + warnln!("[W15] No patch directory specified for package {}, mapping {} => {}. Skipping patch generation.", vendor_package.name.clone(), patch_link.from_prefix.to_str().unwrap(), patch_link.to_prefix.to_str().unwrap()); Ok(()) }, } @@ -330,7 +330,7 @@ pub fn init( // Check if includes exist for path in vendor_package.include_from_upstream.clone() { if !PathBuf::from(extend_paths(&[path.clone()], dep_path, true)?[0].clone()).exists() { - warnln!("{} not found in upstream, continuing.", path); + warnln!("[W16] {} not found in upstream, continuing.", path); } } @@ -361,7 +361,7 @@ pub fn init( })?; } else { warnln!( - "{} not found in upstream, continuing.", + "[W16] {} not found in upstream, continuing.", link_from.to_str().unwrap() ); } diff --git a/src/config.rs b/src/config.rs index d032f5d9..5618f4ae 100644 --- a/src/config.rs +++ b/src/config.rs @@ -401,7 +401,7 @@ impl Validate for PartialManifest { if !pre_output { p.extra.iter().for_each(|(k, _)| { warnln!( - "Ignoring unknown field `{}` in manifest package for {}.", + "[W03] Ignoring unknown field `{}` in manifest package for {}.", k, p.name ); @@ -455,7 +455,7 @@ impl Validate for PartialManifest { if !pre_output { self.extra.iter().for_each(|(k, _)| { warnln!( - "Ignoring unknown field `{}` in manifest for {}.", + "[W03] Ignoring unknown field `{}` in manifest for {}.", k, pkg.name ); @@ -548,7 +548,7 @@ impl Validate for PartialDependency { if !pre_output { self.extra.iter().for_each(|(k, _)| { warnln!( - "Ignoring unknown field `{}` in a dependency in manifest for {}.", + "[W03] Ignoring unknown field `{}` in a dependency in manifest for {}.", k, package_name ); @@ -808,14 +808,14 @@ impl Validate for PartialSources { let files = files?; if files.is_empty() && !pre_output { warnln!( - "No source files specified in a sourcegroup in manifest for {}.", + "[W04] No source files specified in a sourcegroup in manifest for {}.", package_name ); } if !pre_output { self.extra.iter().for_each(|(k, _)| { warnln!( - "Ignoring unknown field `{}` in sources in manifest for {}.", + "[W03] Ignoring unknown field `{}` in sources in manifest for {}.", k, package_name ); @@ -954,7 +954,7 @@ impl GlobFile for PartialSourceFile { }) .collect::>>()?; if out.is_empty() { - warnln!("No files found for glob pattern {:?}", path); + warnln!("[W05] No files found for glob pattern {:?}", path); } Ok(out) } else { @@ -1011,7 +1011,7 @@ impl Validate for PartialWorkspace { if !pre_output { self.extra.iter().for_each(|(k, _)| { warnln!( - "Ignoring unknown field `{}` in workspace configuration in manifest for {}.", + "[W03] Ignoring unknown field `{}` in workspace configuration in manifest for {}.", k, package_name ); diff --git a/src/resolver.rs b/src/resolver.rs index 5a008791..fc2d3302 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -103,7 +103,7 @@ impl<'ctx> DependencyResolver<'ctx> { // - the dependency is not in a clean state (i.e., was modified) if !ignore_checkout { if !is_git_repo { - warnln!("Dependency `{}` in checkout_dir `{}` is not a git repository. Setting as path dependency.\n\ + warnln!("[W06] Dependency `{}` in checkout_dir `{}` is not a git repository. Setting as path dependency.\n\ \tPlease use `bender clone` to work on git dependencies.\n\ \tRun `bender update --ignore-checkout-dir` to overwrite this at your own risk.", dir.as_ref().unwrap().path().file_name().unwrap().to_str().unwrap(), @@ -118,7 +118,7 @@ impl<'ctx> DependencyResolver<'ctx> { .stdout .is_empty()) { - warnln!("Dependency `{}` in checkout_dir `{}` is not in a clean state. Setting as path dependency.\n\ + warnln!("[W06] Dependency `{}` in checkout_dir `{}` is not in a clean state. Setting as path dependency.\n\ \tRun `bender update --ignore-checkout-dir` to overwrite this at your own risk.", dir.as_ref().unwrap().path().file_name().unwrap().to_str().unwrap(), &checkout.display()); @@ -375,7 +375,7 @@ impl<'ctx> DependencyResolver<'ctx> { Some(r) => r.clone(), None => { warnln!( - "No revision found in lock file for git dependency `{}`", + "[W21] No revision found in lock file for git dependency `{}`", name ); return None; @@ -632,7 +632,7 @@ impl<'ctx> DependencyResolver<'ctx> { return Err(e); } warnln!( - "Ignoring error for `{}` at `{}`: {}", + "[W20] Ignoring error for `{}` at `{}`: {}", name, self.sess.dependency_source(*con_src), e diff --git a/src/sess.rs b/src/sess.rs index 2fada0af..e3e85215 100644 --- a/src/sess.rs +++ b/src/sess.rs @@ -558,10 +558,10 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { .await .map_err(move |cause| { if url3.contains("git@") { - warnln!("Please ensure your public ssh key is added to the git server."); + warnln!("[W07] Please ensure your public ssh key is added to the git server."); } warnln!( - "Please ensure the url is correct and you have access to the repository." + "[W07] Please ensure the url is correct and you have access to the repository." ); Error::chain( format!("Failed to initialize git database in {:?}.", db_dir), @@ -591,10 +591,10 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { .await .map_err(move |cause| { if url3.contains("git@") { - warnln!("Please ensure your public ssh key is added to the git server."); + warnln!("[W07] Please ensure your public ssh key is added to the git server."); } warnln!( - "Please ensure the url is correct and you have access to the repository." + "[W07] Please ensure the url is correct and you have access to the repository." ); Error::chain( format!("Failed to update git database in {:?}.", db_dir), @@ -875,7 +875,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { CheckoutState::ToCheckout } else { warnln!( - "Workspace checkout directory set and has uncommitted changes, not updating {} at {}.\n\ + "[W19] Workspace checkout directory set and has uncommitted changes, not updating {} at {}.\n\ \tRun `bender checkout --force` to overwrite the dependency at your own risk.", name, path.display() @@ -884,7 +884,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { } } else { warnln!( - "Workspace checkout directory set and remote url doesn't match, not updating {} at {}.\n\ + "[W19] Workspace checkout directory set and remote url doesn't match, not updating {} at {}.\n\ \tRun `bender checkout --force` to overwrite the dependency at your own risk.", name, path.display() @@ -943,7 +943,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { .spawn_with(move |c| c.arg("fetch").arg("--all")) .await?; git.clone().spawn_with(move |c| c.arg("tag").arg(tag_name_1).arg(revision).arg("--force").arg("--no-sign")).map_err(|cause| { - warnln!("Please ensure the commits are available on the remote or run bender update"); + warnln!("[W08] Please ensure the commits are available on the remote or run bender update"); Error::chain( format!( "Failed to checkout commit {} for {} given in Bender.lock.\n", @@ -1001,7 +1001,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { for dep in (dep_iter_mut).iter_mut() { if let (_, config::Dependency::Path(ref path)) = dep { if !path.starts_with("/") { - warnln!("Path dependencies ({:?}) in git dependencies ({:?}) currently not fully supported. Your mileage may vary.", dep.0, top_package_name); + warnln!("[W09] Path dependencies ({:?}) in git dependencies ({:?}) currently not fully supported. Your mileage may vary.", dep.0, top_package_name); let sub_entries = db .clone() @@ -1117,14 +1117,14 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { match (&dep.source, version) { (DepSrc::Path(path), DepVer::Path) => { if !path.starts_with("/") { - warnln!("There may be issues in the path for {:?}.", dep.name); + warnln!("[W10] There may be issues in the path for {:?}.", dep.name); } let manifest_path = path.join("Bender.yml"); if manifest_path.exists() { match read_manifest(&manifest_path) { Ok(m) => { if dep.name != m.package.name { - warnln!("Dependency name and package name do not match for {:?} / {:?}, this can cause unwanted behavior", + warnln!("[W11] Dependency name and package name do not match for {:?} / {:?}, this can cause unwanted behavior", dep.name, m.package.name); // TODO: This should be an error } Ok(Some(self.sess.intern_manifest(m))) @@ -1149,7 +1149,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { ) { Ok(m) => { if dep.name != m.package.name { - warnln!("Dependency name and package name do not match for {:?} / {:?}, this can cause unwanted behavior", + warnln!("[W11] Dependency name and package name do not match for {:?} / {:?}, this can cause unwanted behavior", dep.name, m.package.name); // TODO: This should be an error } Ok(Some(self.sess.intern_manifest(m))) @@ -1157,7 +1157,11 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { Err(e) => Err(e), } } else { - warnln!("Manifest not found for {:?} at {:?}", dep.name, dep.source); + warnln!( + "[W12] Manifest not found for {:?} at {:?}", + dep.name, + dep.source + ); Ok(None) } } @@ -1212,7 +1216,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { Ok(Some(self.sess.intern_manifest(full))) } None => { - warnln!("Manifest not found for {:?}", dep.name); + warnln!("[W12] Manifest not found for {:?}", dep.name); Ok(None) } }; @@ -1229,7 +1233,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { None => "dead", } { - warnln!("Dependency name and package name do not match for {:?} / {:?}, this can cause unwanted behavior", + warnln!("[W11] Dependency name and package name do not match for {:?} / {:?}, this can cause unwanted behavior", dep.name, match manifest { Some(x) => &x.package.name, None => "dead" @@ -1385,7 +1389,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { if !m.dependencies.is_empty() { for i in m.dependencies.keys() { if !all_export_include_dirs.contains_key(i) { - warnln!("Name issue with {:?}, `export_include_dirs` not handled\n\tCould relate to name mismatch, see `bender update`", i); + warnln!("[W13] Name issue with {:?}, `export_include_dirs` not handled\n\tCould relate to name mismatch, see `bender update`", i); export_include_dirs.insert(i.clone(), IndexSet::new()); } else { export_include_dirs.insert( @@ -1694,7 +1698,7 @@ impl<'ctx> DependencyTable<'ctx> { if let DependencySource::Path(path) = &entry.source { if !path.exists() { warnln!( - "Dependency `{}` has source path `{}` which does not exist", + "[W22] Dependency `{}` has source path `{}` which does not exist", entry.name, path.display() ); From fc39c12ff369242b8720d01e50bc72bbdc6edccd Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Thu, 24 Oct 2024 15:02:07 -0400 Subject: [PATCH 02/10] Add partial warning suppression (config.rs missing) --- src/cli.rs | 32 ++++++++++++++++++----- src/cmd/clone.rs | 2 +- src/cmd/fusesoc.rs | 2 +- src/cmd/parents.rs | 8 +++--- src/cmd/update.rs | 5 ++-- src/resolver.rs | 22 +++++++++------- src/sess.rs | 64 +++++++++++++++++++++++++++++++--------------- 7 files changed, 92 insertions(+), 43 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 3d7db1b3..fd71f6a4 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -63,6 +63,15 @@ pub fn main() -> Result<()> { .value_parser(value_parser!(usize)) .help("Sets the maximum number of concurrent git operations"), ) + .arg( + Arg::new("suppress") + .long("suppress") + .global(true) + .num_args(1) + .action(ArgAction::Append) + .help("Suppresses specific warnings.") + .value_parser(value_parser!(String)), + ) .subcommand(cmd::update::new()) .subcommand(cmd::path::new()) .subcommand(cmd::parents::new()) @@ -95,6 +104,12 @@ pub fn main() -> Result<()> { // Parse the arguments. let matches = app.clone().get_matches(); + let suppressed_warnings: IndexSet = matches + .get_many::("suppress") + .unwrap_or_default() + .map(|s| s.to_owned()) + .collect(); + // Enable debug outputs if needed. if matches.contains_id("debug") && matches.get_flag("debug") { ENABLE_DEBUG.store(true, std::sync::atomic::Ordering::Relaxed); @@ -111,7 +126,7 @@ pub fn main() -> Result<()> { let mut force_fetch = false; if let Some(("update", intern_matches)) = matches.subcommand() { - force_fetch = cmd::update::setup(intern_matches)?; + force_fetch = cmd::update::setup(intern_matches, &suppressed_warnings)?; } // Determine the root working directory, which has either been provided via @@ -134,7 +149,7 @@ pub fn main() -> Result<()> { // Gather and parse the tool configuration. let config = load_config( &root_dir, - matches!(matches.subcommand(), Some(("update", _))), + matches!(matches.subcommand(), Some(("update", _))) && !suppressed_warnings.contains("W02"), )?; debugln!("main: {:#?}", config); @@ -156,6 +171,7 @@ pub fn main() -> Result<()> { matches.get_flag("local"), force_fetch, git_throttle, + suppressed_warnings, ); if let Some(("clean", intern_matches)) = matches.subcommand() { @@ -223,11 +239,13 @@ pub fn main() -> Result<()> { ) })?; if !meta.file_type().is_symlink() { - warnln!( - "[W01] Skipping link to package {} at {:?} since there is something there", - pkg_name, - path - ); + if !sess.suppress_warnings.contains("W01") { + warnln!( + "[W01] Skipping link to package {} at {:?} since there is something there", + pkg_name, + path + ); + } continue; } if path.read_link().map(|d| d != pkg_path).unwrap_or(true) { diff --git a/src/cmd/clone.rs b/src/cmd/clone.rs index 71d8786d..977f6c4a 100644 --- a/src/cmd/clone.rs +++ b/src/cmd/clone.rs @@ -151,7 +151,7 @@ pub fn run(sess: &Session, path: &Path, matches: &ArgMatches) -> Result<()> { { Err(Error::new("git fetch failed".to_string()))?; } - } else { + } else if !sess.suppress_warnings.contains("W14") { warnln!("[W14] fetch not performed due to --local argument."); } diff --git a/src/cmd/fusesoc.rs b/src/cmd/fusesoc.rs index efe593e7..62e1889d 100644 --- a/src/cmd/fusesoc.rs +++ b/src/cmd/fusesoc.rs @@ -155,7 +155,7 @@ pub fn run_single(sess: &Session, matches: &ArgMatches) -> Result<()> { Error::chain(format!("Unable to write corefile for {:?}.", &name), cause) })?; - if fuse_depend_string.len() > 1 { + if fuse_depend_string.len() > 1 && !sess.suppress_warnings.contains("W16") { warnln!("[W16] Depend strings may be wrong for the included dependencies!"); } diff --git a/src/cmd/parents.rs b/src/cmd/parents.rs index 91e78027..4479261b 100644 --- a/src/cmd/parents.rs +++ b/src/cmd/parents.rs @@ -59,7 +59,9 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { .unwrap(); // Filter out dependencies without a manifest if dep_manifest.is_none() { - warnln!("[W17] {} is shown to include dependency, but manifest does not have this information.", pkg_name.to_string()); + if !sess.suppress_warnings.contains("W17") { + warnln!("[W17] {} is shown to include dependency, but manifest does not have this information.", pkg_name.to_string()); + } continue; } let dep_manifest = dep_manifest.unwrap(); @@ -77,7 +79,7 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { ), ], ); - } else { + } else if !sess.suppress_warnings.contains("W17") { // Filter out dependencies with mismatching manifest warnln!("[W17] {} is shown to include dependency, but manifest does not have this information.", pkg_name.to_string()); } @@ -130,7 +132,7 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { } ); - if sess.config.overrides.contains_key(dep) { + if sess.config.overrides.contains_key(dep) && !sess.suppress_warnings.contains("W18") { warnln!( "[W18] An override is configured for {} to {:?}", dep, diff --git a/src/cmd/update.rs b/src/cmd/update.rs index 8c8ad70d..9b010e1f 100644 --- a/src/cmd/update.rs +++ b/src/cmd/update.rs @@ -62,9 +62,10 @@ pub fn new() -> Command { } /// Execute the `update` subcommand. -pub fn setup(matches: &ArgMatches) -> Result { +pub fn setup(matches: &ArgMatches, suppress_warnings: &IndexSet) -> Result { let force_fetch = matches.get_flag("fetch"); - if matches.get_flag("local") && matches.get_flag("fetch") { + if matches.get_flag("local") && matches.get_flag("fetch") && !suppress_warnings.contains("W14") + { warnln!( "[W14] As --local argument is set for bender command, no fetching will be performed." ); diff --git a/src/resolver.rs b/src/resolver.rs index fc2d3302..43964c00 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -103,11 +103,13 @@ impl<'ctx> DependencyResolver<'ctx> { // - the dependency is not in a clean state (i.e., was modified) if !ignore_checkout { if !is_git_repo { - warnln!("[W06] Dependency `{}` in checkout_dir `{}` is not a git repository. Setting as path dependency.\n\ - \tPlease use `bender clone` to work on git dependencies.\n\ - \tRun `bender update --ignore-checkout-dir` to overwrite this at your own risk.", - dir.as_ref().unwrap().path().file_name().unwrap().to_str().unwrap(), - &checkout.display()); + if !self.sess.suppress_warnings.contains("W06") { + warnln!("[W06] Dependency `{}` in checkout_dir `{}` is not a git repository. Setting as path dependency.\n\ + \tPlease use `bender clone` to work on git dependencies.\n\ + \tRun `bender update --ignore-checkout-dir` to overwrite this at your own risk.", + dir.as_ref().unwrap().path().file_name().unwrap().to_str().unwrap(), + &checkout.display()); + } self.checked_out .insert(depname, config::Dependency::Path(dir.unwrap().path())); } else if !(SysCommand::new(&self.sess.config.git) // If not in a clean state @@ -118,10 +120,12 @@ impl<'ctx> DependencyResolver<'ctx> { .stdout .is_empty()) { - warnln!("[W06] Dependency `{}` in checkout_dir `{}` is not in a clean state. Setting as path dependency.\n\ - \tRun `bender update --ignore-checkout-dir` to overwrite this at your own risk.", - dir.as_ref().unwrap().path().file_name().unwrap().to_str().unwrap(), - &checkout.display()); + if !self.sess.suppress_warnings.contains("W06") { + warnln!("[W06] Dependency `{}` in checkout_dir `{}` is not in a clean state. Setting as path dependency.\n\ + \tRun `bender update --ignore-checkout-dir` to overwrite this at your own risk.", + dir.as_ref().unwrap().path().file_name().unwrap().to_str().unwrap(), + &checkout.display()); + } self.checked_out .insert(depname, config::Dependency::Path(dir.unwrap().path())); } diff --git a/src/sess.rs b/src/sess.rs index e3e85215..392b32ba 100644 --- a/src/sess.rs +++ b/src/sess.rs @@ -78,6 +78,8 @@ pub struct Session<'ctx> { pub git_throttle: Arc, /// A toggle to disable remote fetches & clones pub local_only: bool, + /// A list of warnings to suppress. + pub suppress_warnings: IndexSet, } impl<'sess, 'ctx: 'sess> Session<'ctx> { @@ -91,6 +93,7 @@ impl<'sess, 'ctx: 'sess> Session<'ctx> { local_only: bool, force_fetch: bool, git_throttle: usize, + suppress_warnings: IndexSet, ) -> Session<'ctx> { Session { root, @@ -116,6 +119,7 @@ impl<'sess, 'ctx: 'sess> Session<'ctx> { cache: Default::default(), git_throttle: Arc::new(Semaphore::new(git_throttle)), local_only, + suppress_warnings, } } @@ -557,12 +561,14 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { }) .await .map_err(move |cause| { - if url3.contains("git@") { + if url3.contains("git@") && !self.sess.suppress_warnings.contains("W07") { warnln!("[W07] Please ensure your public ssh key is added to the git server."); } - warnln!( - "[W07] Please ensure the url is correct and you have access to the repository." - ); + if !self.sess.suppress_warnings.contains("W07") { + warnln!( + "[W07] Please ensure the url is correct and you have access to the repository." + ); + } Error::chain( format!("Failed to initialize git database in {:?}.", db_dir), cause, @@ -590,12 +596,14 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { }) .await .map_err(move |cause| { - if url3.contains("git@") { + if url3.contains("git@") && !self.sess.suppress_warnings.contains("W07") { warnln!("[W07] Please ensure your public ssh key is added to the git server."); } - warnln!( - "[W07] Please ensure the url is correct and you have access to the repository." - ); + if !self.sess.suppress_warnings.contains("W07") { + warnln!( + "[W07] Please ensure the url is correct and you have access to the repository." + ); + } Error::chain( format!("Failed to update git database in {:?}.", db_dir), cause, @@ -943,7 +951,9 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { .spawn_with(move |c| c.arg("fetch").arg("--all")) .await?; git.clone().spawn_with(move |c| c.arg("tag").arg(tag_name_1).arg(revision).arg("--force").arg("--no-sign")).map_err(|cause| { - warnln!("[W08] Please ensure the commits are available on the remote or run bender update"); + if !self.sess.suppress_warnings.contains("W08") { + warnln!("[W08] Please ensure the commits are available on the remote or run bender update"); + } Error::chain( format!( "Failed to checkout commit {} for {} given in Bender.lock.\n", @@ -1001,7 +1011,10 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { for dep in (dep_iter_mut).iter_mut() { if let (_, config::Dependency::Path(ref path)) = dep { if !path.starts_with("/") { - warnln!("[W09] Path dependencies ({:?}) in git dependencies ({:?}) currently not fully supported. Your mileage may vary.", dep.0, top_package_name); + if !self.sess.suppress_warnings.contains("W09") { + warnln!("[W09] Path dependencies ({:?}) in git dependencies ({:?}) currently not fully supported. \ + Your mileage may vary.", dep.0, top_package_name); + } let sub_entries = db .clone() @@ -1116,14 +1129,16 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { use self::DependencyVersion as DepVer; match (&dep.source, version) { (DepSrc::Path(path), DepVer::Path) => { - if !path.starts_with("/") { + if !path.starts_with("/") && !self.sess.suppress_warnings.contains("W10") { warnln!("[W10] There may be issues in the path for {:?}.", dep.name); } let manifest_path = path.join("Bender.yml"); if manifest_path.exists() { match read_manifest(&manifest_path) { Ok(m) => { - if dep.name != m.package.name { + if dep.name != m.package.name + && !self.sess.suppress_warnings.contains("W11") + { warnln!("[W11] Dependency name and package name do not match for {:?} / {:?}, this can cause unwanted behavior", dep.name, m.package.name); // TODO: This should be an error } @@ -1148,7 +1163,9 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { .join(format!("{}_manifest.yml", dep.name)), ) { Ok(m) => { - if dep.name != m.package.name { + if dep.name != m.package.name + && !self.sess.suppress_warnings.contains("W11") + { warnln!("[W11] Dependency name and package name do not match for {:?} / {:?}, this can cause unwanted behavior", dep.name, m.package.name); // TODO: This should be an error } @@ -1157,11 +1174,13 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { Err(e) => Err(e), } } else { - warnln!( - "[W12] Manifest not found for {:?} at {:?}", - dep.name, - dep.source - ); + if !self.sess.suppress_warnings.contains("W12") { + warnln!( + "[W12] Manifest not found for {:?} at {:?}", + dep.name, + dep.source + ); + } Ok(None) } } @@ -1216,7 +1235,9 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { Ok(Some(self.sess.intern_manifest(full))) } None => { - warnln!("[W12] Manifest not found for {:?}", dep.name); + if !self.sess.suppress_warnings.contains("W12") { + warnln!("[W12] Manifest not found for {:?}", dep.name); + } Ok(None) } }; @@ -1232,6 +1253,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { Some(x) => &x.package.name, None => "dead", } + && !self.sess.suppress_warnings.contains("W11") { warnln!("[W11] Dependency name and package name do not match for {:?} / {:?}, this can cause unwanted behavior", dep.name, match manifest { @@ -1389,7 +1411,9 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { if !m.dependencies.is_empty() { for i in m.dependencies.keys() { if !all_export_include_dirs.contains_key(i) { - warnln!("[W13] Name issue with {:?}, `export_include_dirs` not handled\n\tCould relate to name mismatch, see `bender update`", i); + if !self.sess.suppress_warnings.contains("W13") { + warnln!("[W13] Name issue with {:?}, `export_include_dirs` not handled\n\tCould relate to name mismatch, see `bender update`", i); + } export_include_dirs.insert(i.clone(), IndexSet::new()); } else { export_include_dirs.insert( From 9a414e8ada60b5875e464287f377299edc447676 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Wed, 30 Apr 2025 19:21:24 -0400 Subject: [PATCH 03/10] Finalize warning suppression --- CHANGELOG.md | 1 + src/cli.rs | 23 ++++-- src/cmd/script.rs | 2 +- src/config.rs | 184 ++++++++++++++++++++++++++++++---------------- src/sess.rs | 15 ++-- src/src.rs | 18 +++-- 6 files changed, 163 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 287842e1..2669045f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a - Add check to ensure files referenced in all manifests exist. - Add warnings for unknown fields in manifest. - Add support to indicate and read in external file lists in manifest. +- Enumerate warnings and add `--suppress` flag to hide warnings. ### Changed - `update`: Clean up alignment of manual resolution output. diff --git a/src/cli.rs b/src/cli.rs index fd71f6a4..60afbf2d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -69,7 +69,7 @@ pub fn main() -> Result<()> { .global(true) .num_args(1) .action(ArgAction::Append) - .help("Suppresses specific warnings.") + .help("Suppresses specific warnings. Use `all` to suppress all warnings.") .value_parser(value_parser!(String)), ) .subcommand(cmd::update::new()) @@ -110,6 +110,12 @@ pub fn main() -> Result<()> { .map(|s| s.to_owned()) .collect(); + let suppressed_warnings: IndexSet = if suppressed_warnings.contains("all") { + (1..18).map(|i| format!("W{:02}", i)).collect() + } else { + suppressed_warnings + }; + // Enable debug outputs if needed. if matches.contains_id("debug") && matches.get_flag("debug") { ENABLE_DEBUG.store(true, std::sync::atomic::Ordering::Relaxed); @@ -143,13 +149,14 @@ pub fn main() -> Result<()> { // Parse the manifest file of the package. let manifest_path = root_dir.join("Bender.yml"); - let manifest = read_manifest(&manifest_path)?; + let manifest = read_manifest(&manifest_path, &suppressed_warnings)?; debugln!("main: {:#?}", manifest); // Gather and parse the tool configuration. let config = load_config( &root_dir, matches!(matches.subcommand(), Some(("update", _))) && !suppressed_warnings.contains("W02"), + &suppressed_warnings, )?; debugln!("main: {:#?}", config); @@ -376,7 +383,7 @@ fn find_package_root(from: &Path) -> Result { } /// Read a package manifest from a file. -pub fn read_manifest(path: &Path) -> Result { +pub fn read_manifest(path: &Path, suppress_warnings: &IndexSet) -> Result { use crate::config::PartialManifest; use std::fs::File; debugln!("read_manifest: {:?}", path); @@ -387,12 +394,16 @@ pub fn read_manifest(path: &Path) -> Result { partial .prefix_paths(path.parent().unwrap()) .map_err(|cause| Error::chain(format!("Error in manifest prefixing {:?}.", path), cause))? - .validate("", false) + .validate("", false, suppress_warnings) .map_err(|cause| Error::chain(format!("Error in manifest {:?}.", path), cause)) } /// Load a configuration by traversing a directory hierarchy upwards. -fn load_config(from: &Path, warn_config_loaded: bool) -> Result { +fn load_config( + from: &Path, + warn_config_loaded: bool, + suppress_warnings: &IndexSet, +) -> Result { #[cfg(unix)] use std::os::unix::fs::MetadataExt; @@ -465,7 +476,7 @@ fn load_config(from: &Path, warn_config_loaded: bool) -> Result { // Validate the configuration. let mut out = out - .validate("", false) + .validate("", false, suppress_warnings) .map_err(|cause| Error::chain("Invalid configuration:", cause))?; out.overrides = out diff --git a/src/cmd/script.rs b/src/cmd/script.rs index ef8a9f6b..19b0154e 100644 --- a/src/cmd/script.rs +++ b/src/cmd/script.rs @@ -297,7 +297,7 @@ pub fn run(sess: &Session, matches: &ArgMatches) -> Result<()> { let srcs = srcs .flatten() .into_iter() - .map(|f| f.validate("", false)) + .map(|f| f.validate("", false, &sess.suppress_warnings)) .collect::>>()?; // Validate format-specific options. diff --git a/src/config.rs b/src/config.rs index 5618f4ae..810e31e0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -19,7 +19,7 @@ use std::str::FromStr; use glob::glob; -use indexmap::IndexMap; +use indexmap::{IndexMap, IndexSet}; use semver; use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; @@ -227,6 +227,7 @@ pub trait Validate { self, package_name: &str, pre_output: bool, + suppress_warnings: &IndexSet, ) -> std::result::Result; } @@ -242,12 +243,15 @@ where self, package_name: &str, pre_output: bool, + suppress_warnings: &IndexSet, ) -> std::result::Result { self.into_iter() - .map(|(k, v)| match v.validate(package_name, pre_output) { - Ok(v) => Ok((k, v)), - Err(e) => Err((k, e)), - }) + .map( + |(k, v)| match v.validate(package_name, pre_output, suppress_warnings) { + Ok(v) => Ok((k, v)), + Err(e) => Err((k, e)), + }, + ) .collect() } } @@ -262,12 +266,15 @@ where self, package_name: &str, pre_output: bool, + suppress_warnings: &IndexSet, ) -> std::result::Result { self.into_iter() - .map(|v| match v.validate(package_name, pre_output) { - Ok(v) => Ok(v), - Err(e) => Err(e), - }) + .map( + |v| match v.validate(package_name, pre_output, suppress_warnings) { + Ok(v) => Ok(v), + Err(e) => Err(e), + }, + ) .collect() } } @@ -283,8 +290,9 @@ where self, package_name: &str, pre_output: bool, + suppress_warnings: &IndexSet, ) -> std::result::Result { - self.0.validate(package_name, pre_output) + self.0.validate(package_name, pre_output, suppress_warnings) } } @@ -299,8 +307,9 @@ where self, package_name: &str, pre_output: bool, + suppress_warnings: &IndexSet, ) -> std::result::Result { - self.0.validate(package_name, pre_output) + self.0.validate(package_name, pre_output, suppress_warnings) } } @@ -358,9 +367,10 @@ impl PartialManifest { mut self, package_name: &str, pre_output: bool, + suppress_warnings: &IndexSet, ) -> Result { self.sources = Some(SeqOrStruct::new(PartialSources::new_empty())); - self.validate(package_name, pre_output) + self.validate(package_name, pre_output, suppress_warnings) } } @@ -394,17 +404,24 @@ impl PrefixPaths for PartialManifest { impl Validate for PartialManifest { type Output = Manifest; type Error = Error; - fn validate(self, _package_name: &str, pre_output: bool) -> Result { + fn validate( + self, + _package_name: &str, + pre_output: bool, + suppress_warnings: &IndexSet, + ) -> Result { let pkg = match self.package { Some(mut p) => { p.name = p.name.to_lowercase(); if !pre_output { p.extra.iter().for_each(|(k, _)| { - warnln!( - "[W03] Ignoring unknown field `{}` in manifest package for {}.", - k, - p.name - ); + if !suppress_warnings.contains("W03") { + warnln!( + "[W03] Ignoring unknown field `{}` in manifest package for {}.", + k, + p.name + ); + } }); } p @@ -416,7 +433,7 @@ impl Validate for PartialManifest { .into_iter() .map(|(k, v)| (k.to_lowercase(), v)) .collect::>() - .validate(&pkg.name, pre_output) + .validate(&pkg.name, pre_output, suppress_warnings) .map_err(|(key, cause)| { Error::chain( format!("In dependency `{}` of package `{}`:", key, pkg.name), @@ -426,9 +443,12 @@ impl Validate for PartialManifest { None => IndexMap::new(), }; let srcs = match self.sources { - Some(s) => Some(s.validate(&pkg.name, pre_output).map_err(|cause| { - Error::chain(format!("In source list of package `{}`:", pkg.name), cause) - })?), + Some(s) => Some( + s.validate(&pkg.name, pre_output, suppress_warnings) + .map_err(|cause| { + Error::chain(format!("In source list of package `{}`:", pkg.name), cause) + })?, + ), None => None, }; let exp_inc_dirs = self.export_include_dirs.unwrap_or_default(); @@ -442,23 +462,25 @@ impl Validate for PartialManifest { let frozen = self.frozen.unwrap_or(false); let workspace = match self.workspace { Some(w) => w - .validate(&pkg.name, pre_output) + .validate(&pkg.name, pre_output, suppress_warnings) .map_err(|cause| Error::chain("In workspace configuration:", cause))?, None => Workspace::default(), }; let vendor_package = match self.vendor_package { Some(vend) => vend - .validate(&pkg.name, pre_output) + .validate(&pkg.name, pre_output, suppress_warnings) .map_err(|cause| Error::chain("Unable to parse vendor_package", cause))?, None => Vec::new(), }; if !pre_output { self.extra.iter().for_each(|(k, _)| { - warnln!( - "[W03] Ignoring unknown field `{}` in manifest for {}.", - k, - pkg.name - ); + if !suppress_warnings.contains("W03") { + warnln!( + "[W03] Ignoring unknown field `{}` in manifest for {}.", + k, + pkg.name + ); + } }); } Ok(Manifest { @@ -530,7 +552,12 @@ impl PrefixPaths for PartialDependency { impl Validate for PartialDependency { type Output = Dependency; type Error = Error; - fn validate(self, package_name: &str, pre_output: bool) -> Result { + fn validate( + self, + package_name: &str, + pre_output: bool, + suppress_warnings: &IndexSet, + ) -> Result { let version = match self.version { Some(v) => Some(semver::VersionReq::parse(&v).map_err(|cause| { Error::chain( @@ -547,11 +574,13 @@ impl Validate for PartialDependency { } if !pre_output { self.extra.iter().for_each(|(k, _)| { - warnln!( - "[W03] Ignoring unknown field `{}` in a dependency in manifest for {}.", - k, - package_name - ); + if !suppress_warnings.contains("W03") { + warnln!( + "[W03] Ignoring unknown field `{}` in a dependency in manifest for {}.", + k, + package_name + ); + } }); } if let Some(path) = self.path { @@ -653,7 +682,12 @@ impl From> for PartialSources { impl Validate for PartialSources { type Output = Sources; type Error = Error; - fn validate(self, package_name: &str, pre_output: bool) -> Result { + fn validate( + self, + package_name: &str, + pre_output: bool, + suppress_warnings: &IndexSet, + ) -> Result { let external_flists: Result> = self .external_flists .clone() @@ -781,7 +815,7 @@ impl Validate for PartialSources { .map(|pre_glob_file| { if let PartialSourceFile::File(_) = pre_glob_file { // PartialSources .files item is pointing to PartialSourceFiles::file so do glob extension - pre_glob_file.glob_file() + pre_glob_file.glob_file(suppress_warnings) } else { // PartialSources .files item is pointing to PartialSourceFiles::group so pass on for recursion // to do glob extension in the groups.sources.files list of PartialSourceFiles::file @@ -803,10 +837,10 @@ impl Validate for PartialSources { let defines = self.defines.unwrap_or_default(); let files: Result> = post_glob_files .into_iter() - .map(|f| f.validate(package_name, pre_output)) + .map(|f| f.validate(package_name, pre_output, suppress_warnings)) .collect(); let files = files?; - if files.is_empty() && !pre_output { + if files.is_empty() && !pre_output && !suppress_warnings.contains("W04") { warnln!( "[W04] No source files specified in a sourcegroup in manifest for {}.", package_name @@ -814,11 +848,13 @@ impl Validate for PartialSources { } if !pre_output { self.extra.iter().for_each(|(k, _)| { - warnln!( - "[W03] Ignoring unknown field `{}` in sources in manifest for {}.", - k, - package_name - ); + if !suppress_warnings.contains("W03") { + warnln!( + "[W03] Ignoring unknown field `{}` in sources in manifest for {}.", + k, + package_name + ); + } }); } Ok(Sources { @@ -906,12 +942,19 @@ impl<'de> Deserialize<'de> for PartialSourceFile { impl Validate for PartialSourceFile { type Output = SourceFile; type Error = Error; - fn validate(self, package_name: &str, pre_output: bool) -> Result { + fn validate( + self, + package_name: &str, + pre_output: bool, + suppress_warnings: &IndexSet, + ) -> Result { match self { PartialSourceFile::File(path) => Ok(SourceFile::File(PathBuf::from(path))), - PartialSourceFile::Group(srcs) => Ok(SourceFile::Group(Box::new( - srcs.validate(package_name, pre_output)?, - ))), + PartialSourceFile::Group(srcs) => Ok(SourceFile::Group(Box::new(srcs.validate( + package_name, + pre_output, + suppress_warnings, + )?))), } } } @@ -923,14 +966,14 @@ pub trait GlobFile { /// The error type produced by validation. type Error; /// Validate self and convert to a full list of paths that exist - fn glob_file(self) -> Result; + fn glob_file(self, suppress_warnings: &IndexSet) -> Result; } impl GlobFile for PartialSourceFile { type Output = Vec; type Error = Error; - fn glob_file(self) -> Result> { + fn glob_file(self, suppress_warnings: &IndexSet) -> Result> { // let mut partial_source_files_vec: Vec = Vec::new(); // Only operate on files, not groups @@ -953,7 +996,7 @@ impl GlobFile for PartialSourceFile { )) }) .collect::>>()?; - if out.is_empty() { + if out.is_empty() && !suppress_warnings.contains("W05") { warnln!("[W05] No files found for glob pattern {:?}", path); } Ok(out) @@ -1001,7 +1044,12 @@ impl PrefixPaths for PartialWorkspace { impl Validate for PartialWorkspace { type Output = Workspace; type Error = Error; - fn validate(self, package_name: &str, pre_output: bool) -> Result { + fn validate( + self, + package_name: &str, + pre_output: bool, + suppress_warnings: &IndexSet, + ) -> Result { let package_links: Result> = self .package_links .unwrap_or_default() @@ -1010,11 +1058,13 @@ impl Validate for PartialWorkspace { .collect(); if !pre_output { self.extra.iter().for_each(|(k, _)| { - warnln!( - "[W03] Ignoring unknown field `{}` in workspace configuration in manifest for {}.", - k, - package_name - ); + if !suppress_warnings.contains("W03") { + warnln!( + "[W03] Ignoring unknown field `{}` in workspace configuration in manifest for {}.", + k, + package_name + ); + } }); } Ok(Workspace { @@ -1180,7 +1230,12 @@ impl Merge for PartialConfig { impl Validate for PartialConfig { type Output = Config; type Error = Error; - fn validate(self, package_name: &str, pre_output: bool) -> Result { + fn validate( + self, + package_name: &str, + pre_output: bool, + suppress_warnings: &IndexSet, + ) -> Result { Ok(Config { database: match self.database { Some(db) => env_path_from_string(db)?, @@ -1192,7 +1247,7 @@ impl Validate for PartialConfig { }, overrides: match self.overrides { Some(d) => d - .validate(package_name, pre_output) + .validate(package_name, pre_output, suppress_warnings) .map_err(|(key, cause)| { Error::chain(format!("In override `{}`:", key), cause) })?, @@ -1200,7 +1255,7 @@ impl Validate for PartialConfig { }, plugins: match self.plugins { Some(d) => d - .validate(package_name, pre_output) + .validate(package_name, pre_output, suppress_warnings) .map_err(|(key, cause)| Error::chain(format!("In plugin `{}`:", key), cause))?, None => IndexMap::new(), }, @@ -1322,7 +1377,12 @@ impl PrefixPaths for PartialVendorPackage { impl Validate for PartialVendorPackage { type Output = VendorPackage; type Error = Error; - fn validate(self, package_name: &str, pre_output: bool) -> Result { + fn validate( + self, + package_name: &str, + pre_output: bool, + suppress_warnings: &IndexSet, + ) -> Result { Ok(VendorPackage { name: match self.name { Some(name) => name, @@ -1334,7 +1394,7 @@ impl Validate for PartialVendorPackage { }, upstream: match self.upstream { Some(upstream) => upstream - .validate(package_name, pre_output) + .validate(package_name, pre_output, suppress_warnings) .map_err(|cause| { Error::chain("Unable to parse external import upstream", cause) })?, diff --git a/src/sess.rs b/src/sess.rs index 392b32ba..d2170f43 100644 --- a/src/sess.rs +++ b/src/sess.rs @@ -1067,8 +1067,9 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { cause, ) })?; - let mut full = - partial.validate_ignore_sources("", true).map_err(|cause| { + let mut full = partial + .validate_ignore_sources("", true, &self.sess.suppress_warnings) + .map_err(|cause| { Error::chain( format!( "Error in manifest of dependency `{}` at revision \ @@ -1134,7 +1135,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { } let manifest_path = path.join("Bender.yml"); if manifest_path.exists() { - match read_manifest(&manifest_path) { + match read_manifest(&manifest_path, &self.sess.suppress_warnings) { Ok(m) => { if dep.name != m.package.name && !self.sess.suppress_warnings.contains("W11") @@ -1161,6 +1162,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { .join(".bender") .join("tmp") .join(format!("{}_manifest.yml", dep.name)), + &self.sess.suppress_warnings, ) { Ok(m) => { if dep.name != m.package.name @@ -1209,8 +1211,9 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { cause, ) })?; - let mut full = - partial.validate_ignore_sources("", true).map_err(|cause| { + let mut full = partial + .validate_ignore_sources("", true, &self.sess.suppress_warnings) + .map_err(|cause| { Error::chain( format!( "Error in manifest of dependency `{}` at revision \ @@ -1299,7 +1302,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { .and_then(move |path| { let manifest_path = path.join("Bender.yml"); if manifest_path.exists() { - match read_manifest(&manifest_path) { + match read_manifest(&manifest_path, &self.sess.suppress_warnings) { Ok(m) => Ok(Some(self.sess.intern_manifest(m))), Err(e) => Err(e), } diff --git a/src/src.rs b/src/src.rs index e1cffbaa..243eee5f 100644 --- a/src/src.rs +++ b/src/src.rs @@ -50,12 +50,13 @@ impl<'ctx> Validate for SourceGroup<'ctx> { self, package_name: &str, pre_output: bool, + suppress_warnings: &IndexSet, ) -> crate::error::Result> { Ok(SourceGroup { files: self .files .into_iter() - .map(|f| f.validate(package_name, pre_output)) + .map(|f| f.validate(package_name, pre_output, suppress_warnings)) .collect::, Error>>()?, ..self }) @@ -340,7 +341,12 @@ impl<'ctx> From<&'ctx Path> for SourceFile<'ctx> { impl<'ctx> Validate for SourceFile<'ctx> { type Output = SourceFile<'ctx>; type Error = Error; - fn validate(self, package_name: &str, pre_output: bool) -> Result, Error> { + fn validate( + self, + package_name: &str, + pre_output: bool, + suppress_warnings: &IndexSet, + ) -> Result, Error> { match self { SourceFile::File(path) => { let env_path_buf = @@ -355,9 +361,11 @@ impl<'ctx> Validate for SourceFile<'ctx> { ))) } } - SourceFile::Group(srcs) => Ok(SourceFile::Group(Box::new( - srcs.validate(package_name, pre_output)?, - ))), + SourceFile::Group(srcs) => Ok(SourceFile::Group(Box::new(srcs.validate( + package_name, + pre_output, + suppress_warnings, + )?))), } } } From ad89d2fdd01937f8b46fd12db7d20747fa9c4bce Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Wed, 27 Nov 2024 10:02:13 -0500 Subject: [PATCH 04/10] Add additional comment to warning --- src/resolver.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/resolver.rs b/src/resolver.rs index 43964c00..5c0d1b47 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -122,6 +122,7 @@ impl<'ctx> DependencyResolver<'ctx> { { if !self.sess.suppress_warnings.contains("W06") { warnln!("[W06] Dependency `{}` in checkout_dir `{}` is not in a clean state. Setting as path dependency.\n\ + \tPlease use `bender clone` to work on git dependencies.\n\ \tRun `bender update --ignore-checkout-dir` to overwrite this at your own risk.", dir.as_ref().unwrap().path().file_name().unwrap().to_str().unwrap(), &checkout.display()); From 893161c38e2bd082ab6b66de5bef9943cc9a8e9d Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Wed, 30 Apr 2025 19:43:33 -0400 Subject: [PATCH 05/10] Add warning to clarify checkout actions --- src/cli.rs | 11 ++++++----- src/sess.rs | 22 +++++++++++++--------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 60afbf2d..4a2ad36d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -110,11 +110,12 @@ pub fn main() -> Result<()> { .map(|s| s.to_owned()) .collect(); - let suppressed_warnings: IndexSet = if suppressed_warnings.contains("all") { - (1..18).map(|i| format!("W{:02}", i)).collect() - } else { - suppressed_warnings - }; + let suppressed_warnings: IndexSet = + if suppressed_warnings.contains("all") || suppressed_warnings.contains("Wall") { + (1..19).map(|i| format!("W{:02}", i)).collect() + } else { + suppressed_warnings + }; // Enable debug outputs if needed. if matches.contains_id("debug") && matches.get_flag("debug") { diff --git a/src/sess.rs b/src/sess.rs index d2170f43..a619613d 100644 --- a/src/sess.rs +++ b/src/sess.rs @@ -882,21 +882,25 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { { CheckoutState::ToCheckout } else { + if !self.sess.suppress_warnings.contains("W19") { + warnln!( + "[W19] Workspace checkout directory set and has uncommitted changes, not updating {} at {}.\n\ + \tRun `bender checkout --force` to overwrite the dependency at your own risk.", + name, + path.display() + ); + } + CheckoutState::Clean + } + } else { + if !self.sess.suppress_warnings.contains("W19") { warnln!( - "[W19] Workspace checkout directory set and has uncommitted changes, not updating {} at {}.\n\ + "[W19] Workspace checkout directory set and remote url doesn't match, not updating {} at {}.\n\ \tRun `bender checkout --force` to overwrite the dependency at your own risk.", name, path.display() ); - CheckoutState::Clean } - } else { - warnln!( - "[W19] Workspace checkout directory set and remote url doesn't match, not updating {} at {}.\n\ - \tRun `bender checkout --force` to overwrite the dependency at your own risk.", - name, - path.display() - ); CheckoutState::Clean } } else { From 3bc2208d252062aa84bfa12da9b48b171027a095 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 4 Nov 2024 16:46:46 -0500 Subject: [PATCH 06/10] Add another warning number and suppression --- src/cli.rs | 2 +- src/resolver.rs | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 4a2ad36d..45a1b95d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -112,7 +112,7 @@ pub fn main() -> Result<()> { let suppressed_warnings: IndexSet = if suppressed_warnings.contains("all") || suppressed_warnings.contains("Wall") { - (1..19).map(|i| format!("W{:02}", i)).collect() + (1..20).map(|i| format!("W{:02}", i)).collect() } else { suppressed_warnings }; diff --git a/src/resolver.rs b/src/resolver.rs index 5c0d1b47..9acb0203 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -636,12 +636,14 @@ impl<'ctx> DependencyResolver<'ctx> { if id == con_src { return Err(e); } - warnln!( - "[W20] Ignoring error for `{}` at `{}`: {}", - name, - self.sess.dependency_source(*con_src), - e - ); + if !self.sess.suppress_warnings.contains("W20") { + warnln!( + "[W20] Ignoring error for `{}` at `{}`: {}", + name, + self.sess.dependency_source(*con_src), + e + ); + } Ok((*id, IndexSet::new())) } } From 62a71df01dc49e2687c3956a495889d3e0bfaf5c Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 12 May 2025 18:53:46 -0400 Subject: [PATCH 07/10] Add another warning suppression --- src/cli.rs | 2 +- src/resolver.rs | 10 ++++++---- src/sess.rs | 19 ++++++++++++------- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 45a1b95d..53d7d3ec 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -112,7 +112,7 @@ pub fn main() -> Result<()> { let suppressed_warnings: IndexSet = if suppressed_warnings.contains("all") || suppressed_warnings.contains("Wall") { - (1..20).map(|i| format!("W{:02}", i)).collect() + (1..22).map(|i| format!("W{:02}", i)).collect() } else { suppressed_warnings }; diff --git a/src/resolver.rs b/src/resolver.rs index 9acb0203..de235bba 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -379,10 +379,12 @@ impl<'ctx> DependencyResolver<'ctx> { match &locked_package.revision { Some(r) => r.clone(), None => { - warnln!( - "[W21] No revision found in lock file for git dependency `{}`", - name - ); + if !io.sess.suppress_warnings.contains("W21") { + warnln!( + "[W21] No revision found in lock file for git dependency `{}`", + name + ); + } return None; } }, diff --git a/src/sess.rs b/src/sess.rs index a619613d..3dbd9c17 100644 --- a/src/sess.rs +++ b/src/sess.rs @@ -141,15 +141,15 @@ impl<'sess, 'ctx: 'sess> Session<'ctx> { calling_package ); let src = DependencySource::from(cfg); - self.deps - .lock() - .unwrap() - .add(self.intern_dependency_entry(DependencyEntry { + self.deps.lock().unwrap().add( + self.intern_dependency_entry(DependencyEntry { name: name.into(), source: src, revision: None, version: None, - })) + }), + &self.suppress_warnings, + ) } /// Load a lock file. @@ -176,6 +176,7 @@ impl<'sess, 'ctx: 'sess> Session<'ctx> { .as_ref() .map(|s| semver::Version::parse(s).unwrap()), }), + &self.suppress_warnings, ); graph_names.insert(id, &pkg.dependencies); names.insert(name.clone(), id); @@ -1721,13 +1722,17 @@ impl<'ctx> DependencyTable<'ctx> { /// /// The reference with which the information can later be retrieved is /// returned. - pub fn add(&mut self, entry: &'ctx DependencyEntry) -> DependencyRef { + pub fn add( + &mut self, + entry: &'ctx DependencyEntry, + suppress_warnings: &IndexSet, + ) -> DependencyRef { if let Some(&id) = self.ids.get(&entry) { debugln!("sess: reusing {:?}", id); id } else { if let DependencySource::Path(path) = &entry.source { - if !path.exists() { + if !path.exists() && !suppress_warnings.contains("W22") { warnln!( "[W22] Dependency `{}` has source path `{}` which does not exist", entry.name, From e414b56bc2d8e24c7a6d038d52a012f426fe13d3 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 12 May 2025 19:06:29 -0400 Subject: [PATCH 08/10] Enable suppressing envvar error --- src/config.rs | 86 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 64 insertions(+), 22 deletions(-) diff --git a/src/config.rs b/src/config.rs index 810e31e0..f6a145eb 100644 --- a/src/config.rs +++ b/src/config.rs @@ -18,7 +18,6 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use glob::glob; - use indexmap::{IndexMap, IndexSet}; use semver; use serde::de::{Deserialize, Deserializer}; @@ -489,7 +488,19 @@ impl Validate for PartialManifest { sources: srcs, export_include_dirs: exp_inc_dirs .iter() - .map(|path| env_path_from_string(path.to_string())) + .filter_map(|path| match env_path_from_string(path.to_string()) { + Ok(parsed_path) => Some(Ok(parsed_path)), + Err(cause) => { + if suppress_warnings.contains("E30") { + if !suppress_warnings.contains("W30") { + warnln!("[W30] File not added, ignoring: {}", cause); + } + None + } else { + Some(Err(Error::chain("[E30]", cause))) + } + } + }) .collect::>>()?, plugins, frozen, @@ -693,7 +704,19 @@ impl Validate for PartialSources { .clone() .unwrap_or_default() .iter() - .map(|path| env_path_from_string(path.to_string())) + .filter_map(|path| match env_path_from_string(path.to_string()) { + Ok(p) => Some(Ok(p)), + Err(cause) => { + if suppress_warnings.contains("E30") { + if !suppress_warnings.contains("W30") { + warnln!("[W30] File not added, ignoring: {}", cause); + } + None + } else { + Some(Err(Error::chain("[E30]", cause))) + } + } + }) .collect(); let external_flist_list: Result)>> = external_flists? @@ -803,11 +826,21 @@ impl Validate for PartialSources { .files .into_iter() .chain(external_flist_groups?.into_iter()) - .map(|file| match file { - PartialSourceFile::File(file) => { - Ok(PartialSourceFile::File(env_string_from_string(file)?)) - } - other => Ok(other), + .filter_map(|file| match file { + PartialSourceFile::File(file) => match env_string_from_string(file) { + Ok(p) => Some(Ok(PartialSourceFile::File(p))), + Err(cause) => { + if suppress_warnings.contains("E30") { + if !suppress_warnings.contains("W30") { + warnln!("[W30] File not added, ignoring: {}", cause); + } + None + } else { + Some(Err(Error::chain("[E30]", cause))) + } + } + }, + other => Some(Ok(other)), }) .collect::>>()?; let post_glob_files: Vec = post_env_files @@ -831,7 +864,19 @@ impl Validate for PartialSources { .include_dirs .unwrap_or_default() .iter() - .map(|path| env_path_from_string(path.to_string())) + .filter_map(|path| match env_path_from_string(path.to_string()) { + Ok(p) => Some(Ok(p)), + Err(cause) => { + if suppress_warnings.contains("E30") { + if !suppress_warnings.contains("W30") { + warnln!("[W30] File not added, ignoring: {}", cause); + } + None + } else { + Some(Err(Error::chain("[E30]", cause))) + } + } + }) .collect(); let defines = self.defines.unwrap_or_default(); @@ -839,7 +884,8 @@ impl Validate for PartialSources { .into_iter() .map(|f| f.validate(package_name, pre_output, suppress_warnings)) .collect(); - let files = files?; + let files: Vec> = files?; + let files: Vec = files.into_iter().flatten().collect(); if files.is_empty() && !pre_output && !suppress_warnings.contains("W04") { warnln!( "[W04] No source files specified in a sourcegroup in manifest for {}.", @@ -940,21 +986,19 @@ impl<'de> Deserialize<'de> for PartialSourceFile { } impl Validate for PartialSourceFile { - type Output = SourceFile; + type Output = Option; type Error = Error; fn validate( self, package_name: &str, pre_output: bool, suppress_warnings: &IndexSet, - ) -> Result { + ) -> Result> { match self { - PartialSourceFile::File(path) => Ok(SourceFile::File(PathBuf::from(path))), - PartialSourceFile::Group(srcs) => Ok(SourceFile::Group(Box::new(srcs.validate( - package_name, - pre_output, - suppress_warnings, - )?))), + PartialSourceFile::File(path) => Ok(Some(SourceFile::File(PathBuf::from(path)))), + PartialSourceFile::Group(srcs) => Ok(Some(SourceFile::Group(Box::new( + srcs.validate(package_name, pre_output, suppress_warnings)?, + )))), } } } @@ -1099,10 +1143,8 @@ impl PrefixPaths for PathBuf { impl PrefixPaths for String { fn prefix_paths(self, prefix: &Path) -> Result { - Ok(prefix - .join(env_path_from_string(self)?) - .display() - .to_string()) + // env is resolved later, convert back to string here. + Ok(prefix.join(PathBuf::from(&self)).display().to_string()) } } From de826474a7716de839a92b32dc8119251c213d37 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 18 Nov 2024 15:26:45 -0500 Subject: [PATCH 09/10] Enable suppressing of nonexistent file error --- src/src.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/src.rs b/src/src.rs index 243eee5f..45f8ad21 100644 --- a/src/src.rs +++ b/src/src.rs @@ -352,11 +352,17 @@ impl<'ctx> Validate for SourceFile<'ctx> { let env_path_buf = crate::config::env_path_from_string(path.to_string_lossy().to_string())?; let exists = env_path_buf.exists() && env_path_buf.is_file(); - if exists { + if exists || suppress_warnings.contains("E31") { + if !(exists || suppress_warnings.contains("W31")) { + warnln!( + "[W31] File {} doesn't exist.", + env_path_buf.to_string_lossy() + ); + } Ok(SourceFile::File(path)) } else { Err(Error::new(format!( - "File {} doesn't exist", + "[E31] File {} doesn't exist", env_path_buf.to_string_lossy() ))) } From 12c77ae1a5f39c4cb56c9f870972de224b5deae4 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Fri, 7 Nov 2025 04:47:14 -0500 Subject: [PATCH 10/10] Add another warning suppression --- src/cli.rs | 2 +- src/resolver.rs | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 53d7d3ec..060cb3e6 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -112,7 +112,7 @@ pub fn main() -> Result<()> { let suppressed_warnings: IndexSet = if suppressed_warnings.contains("all") || suppressed_warnings.contains("Wall") { - (1..22).map(|i| format!("W{:02}", i)).collect() + (1..23).map(|i| format!("W{:02}", i)).collect() } else { suppressed_warnings }; diff --git a/src/resolver.rs b/src/resolver.rs index de235bba..47ba2ff1 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -466,11 +466,13 @@ impl<'ctx> DependencyResolver<'ctx> { match gv.revs.iter().position(|rev| *rev == hash.unwrap()) { Some(index) => index, None => { - warnln!( - "Locked revision `{:?}` for dependency `{}` not found in available revisions, allowing update.", - hash.unwrap(), - dep - ); + if !self.sess.suppress_warnings.contains("W23") { + warnln!( + "[W23] Locked revision `{:?}` for dependency `{}` not found in available revisions, allowing update.", + hash.unwrap(), + dep + ); + } self.locked.get_mut(dep.as_str()).unwrap().3 = false; continue; }