-
Notifications
You must be signed in to change notification settings - Fork 24
Adding support for .gitignore file like ignorefiles (and fix Windows build) #302
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
base: main
Are you sure you want to change the base?
Changes from all commits
4a79ad3
ffc8831
a182b03
5dba308
e2b7cab
78f8fcb
541365c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -80,11 +80,8 @@ enum Command { | |||||
| /// Print copied file names. | ||||||
| #[arg(long, short)] | ||||||
| verbose: bool, | ||||||
| #[arg(long, short)] | ||||||
| exclude: Vec<String>, | ||||||
| /// Read a list of globs to exclude from this file. | ||||||
| #[arg(long, short = 'E')] | ||||||
| exclude_from: Vec<String>, | ||||||
| #[clap(flatten)] | ||||||
| path_filter: PathFilterOptions, | ||||||
| /// Don't print statistics after the backup completes. | ||||||
| #[arg(long)] | ||||||
| no_stats: bool, | ||||||
|
|
@@ -122,10 +119,8 @@ enum Command { | |||||
| /// Select the version from the archive to compare: by default, the latest. | ||||||
| #[arg(long, short)] | ||||||
| backup: Option<BandId>, | ||||||
| #[arg(long, short)] | ||||||
| exclude: Vec<String>, | ||||||
| #[arg(long, short = 'E')] | ||||||
| exclude_from: Vec<String>, | ||||||
| #[clap(flatten)] | ||||||
| path_filter: PathFilterOptions, | ||||||
| #[arg(long)] | ||||||
| include_unchanged: bool, | ||||||
|
|
||||||
|
|
@@ -159,11 +154,8 @@ enum Command { | |||||
| #[command(flatten)] | ||||||
| stos: StoredTreeOrSource, | ||||||
|
|
||||||
| #[arg(long, short)] | ||||||
| exclude: Vec<String>, | ||||||
|
|
||||||
| #[arg(long, short = 'E')] | ||||||
| exclude_from: Vec<String>, | ||||||
| #[clap(flatten)] | ||||||
| path_filter: PathFilterOptions, | ||||||
|
|
||||||
| /// Print entries as json. | ||||||
| #[arg(long, short)] | ||||||
|
|
@@ -217,10 +209,8 @@ enum Command { | |||||
| force_overwrite: bool, | ||||||
| #[arg(long, short)] | ||||||
| verbose: bool, | ||||||
| #[arg(long, short)] | ||||||
| exclude: Vec<String>, | ||||||
| #[arg(long, short = 'E')] | ||||||
| exclude_from: Vec<String>, | ||||||
| #[clap(flatten)] | ||||||
| path_filter: PathFilterOptions, | ||||||
| #[arg(long = "only", short = 'i')] | ||||||
| only_subtree: Option<Apath>, | ||||||
| #[arg(long)] | ||||||
|
|
@@ -239,10 +229,8 @@ enum Command { | |||||
| #[arg(long)] | ||||||
| bytes: bool, | ||||||
|
|
||||||
| #[arg(long, short)] | ||||||
| exclude: Vec<String>, | ||||||
| #[arg(long, short = 'E')] | ||||||
| exclude_from: Vec<String>, | ||||||
| #[clap(flatten)] | ||||||
| path_filter: PathFilterOptions, | ||||||
| }, | ||||||
|
|
||||||
| /// Check that an archive is internally consistent. | ||||||
|
|
@@ -293,6 +281,21 @@ struct StoredTreeOrSource { | |||||
| backup: Option<BandId>, | ||||||
| } | ||||||
|
|
||||||
| #[derive(Debug, Parser)] | ||||||
| struct PathFilterOptions { | ||||||
| /// Ignore paths matching the glob pattern. | ||||||
| #[arg(long, short, conflicts_with = "ignore_file")] | ||||||
| exclude: Vec<String>, | ||||||
|
|
||||||
| /// Read a list of globs to exclude from this file. | ||||||
| #[arg(long, short = 'E', conflicts_with = "ignore_file")] | ||||||
| exclude_from: Vec<String>, | ||||||
|
|
||||||
| /// Specify a gitignore like ignorefile | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| #[arg(long, short)] | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's just specify
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea |
||||||
| ignore_file: Option<PathBuf>, | ||||||
|
sourcefrog marked this conversation as resolved.
|
||||||
| } | ||||||
|
|
||||||
| /// Show debugging information. | ||||||
| #[derive(Debug, Subcommand)] | ||||||
| enum Debug { | ||||||
|
|
@@ -328,6 +331,16 @@ impl std::process::Termination for ExitCode { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| impl PathFilterOptions { | ||||||
| pub fn to_exclude(&self) -> Result<Exclude> { | ||||||
| if let Some(file) = &self.ignore_file { | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we could potentially allow both and have them stack, but it doesn't have to be in this PR. |
||||||
| Exclude::from_ignorefile(file) | ||||||
| } else { | ||||||
| Exclude::from_patterns_and_files(&self.exclude, &self.exclude_from) | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| impl Command { | ||||||
| #[tokio::main] | ||||||
| async fn run(&self, monitor: Arc<TermUiMonitor>) -> Result<ExitCode> { | ||||||
|
|
@@ -336,15 +349,14 @@ impl Command { | |||||
| Command::Backup { | ||||||
| archive, | ||||||
| changes_json, | ||||||
| exclude, | ||||||
| exclude_from, | ||||||
| path_filter, | ||||||
| long_listing, | ||||||
| no_stats, | ||||||
| source, | ||||||
| verbose, | ||||||
| } => { | ||||||
| let options = BackupOptions { | ||||||
| exclude: Exclude::from_patterns_and_files(exclude, exclude_from)?, | ||||||
| exclude: path_filter.to_exclude()?, | ||||||
| change_callback: make_change_callback( | ||||||
| *verbose, | ||||||
| *long_listing, | ||||||
|
|
@@ -425,15 +437,14 @@ impl Command { | |||||
| archive, | ||||||
| source, | ||||||
| backup, | ||||||
| exclude, | ||||||
| exclude_from, | ||||||
| path_filter, | ||||||
| include_unchanged, | ||||||
| json, | ||||||
| } => { | ||||||
| let st = stored_tree_from_opt(archive, backup).await?; | ||||||
| let source = SourceTree::open(source)?; | ||||||
| let options = DiffOptions { | ||||||
| exclude: Exclude::from_patterns_and_files(exclude, exclude_from)?, | ||||||
| exclude: path_filter.to_exclude()?, | ||||||
| include_unchanged: *include_unchanged, | ||||||
| }; | ||||||
| let mut bw = BufWriter::new(stdout); | ||||||
|
|
@@ -474,16 +485,15 @@ impl Command { | |||||
| Command::Ls { | ||||||
| json, | ||||||
| stos, | ||||||
| exclude, | ||||||
| exclude_from, | ||||||
| path_filter, | ||||||
| long_listing, | ||||||
| } => { | ||||||
| let exclude = Exclude::from_patterns_and_files(exclude, exclude_from)?; | ||||||
| let path_filter = path_filter.to_exclude()?; | ||||||
| if let Some(archive) = &stos.archive { | ||||||
| // TODO: Option for subtree. | ||||||
| let mut stitch = stored_tree_from_opt(archive, &stos.backup) | ||||||
| .await? | ||||||
| .iter_entries(Apath::root(), exclude, monitor.clone()); | ||||||
| .iter_entries(Apath::root(), path_filter, monitor.clone()); | ||||||
| while let Some(entry) = stitch.next().await { | ||||||
| // Strip off index internals like addresses; this seems | ||||||
| // like not quite the right way to do it, maybe the types should | ||||||
|
|
@@ -499,7 +509,7 @@ impl Command { | |||||
| // TODO: Can maybe unify these more when the source tree iter is also async. | ||||||
| let entry_iter = SourceTree::open(stos.source.clone().unwrap())?.iter_entries( | ||||||
| Apath::root(), | ||||||
| exclude, | ||||||
| path_filter, | ||||||
| monitor.clone(), | ||||||
| )?; | ||||||
| for entry in entry_iter { | ||||||
|
|
@@ -520,7 +530,7 @@ impl Command { | |||||
| } => { | ||||||
| use std::io::Read; | ||||||
|
|
||||||
| let archive = Archive::open(Transport::new(archive)?)?; | ||||||
| let archive = Archive::open(Transport::new(archive).await?).await?; | ||||||
| let options = MountOptions { clean: *cleanup }; | ||||||
| let projection = match mount(archive, destination, options) { | ||||||
| Ok(handle) => handle, | ||||||
|
|
@@ -555,8 +565,7 @@ impl Command { | |||||
| changes_json, | ||||||
| verbose, | ||||||
| force_overwrite, | ||||||
| exclude, | ||||||
| exclude_from, | ||||||
| path_filter, | ||||||
| only_subtree, | ||||||
| long_listing, | ||||||
| no_stats, | ||||||
|
|
@@ -565,7 +574,7 @@ impl Command { | |||||
| let archive = Archive::open(Transport::new(archive).await?).await?; | ||||||
| let _ = no_stats; // accepted but ignored; we never currently print stats | ||||||
| let options = RestoreOptions { | ||||||
| exclude: Exclude::from_patterns_and_files(exclude, exclude_from)?, | ||||||
| exclude: path_filter.to_exclude()?, | ||||||
| only_subtree: only_subtree.clone(), | ||||||
| band_selection, | ||||||
| overwrite: *force_overwrite, | ||||||
|
|
@@ -582,10 +591,9 @@ impl Command { | |||||
| Command::Size { | ||||||
| stos, | ||||||
| bytes, | ||||||
| exclude, | ||||||
| exclude_from, | ||||||
| path_filter, | ||||||
| } => { | ||||||
| let exclude = Exclude::from_patterns_and_files(exclude, exclude_from)?; | ||||||
| let exclude = path_filter.to_exclude()?; | ||||||
| let size = if let Some(archive) = &stos.archive { | ||||||
| stored_tree_from_opt(archive, &stos.backup) | ||||||
| .await? | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,13 +25,15 @@ use std::iter::empty; | |
| use std::path::Path; | ||
|
|
||
| use globset::{GlobBuilder, GlobSet, GlobSetBuilder}; | ||
| use ignore::gitignore::{Gitignore, GitignoreBuilder}; | ||
|
|
||
| use super::*; | ||
|
|
||
| /// Describes which files to exclude from a backup, restore, etc. | ||
| #[derive(Clone, Debug)] | ||
| pub struct Exclude { | ||
| globset: GlobSet, | ||
| pub enum Exclude { | ||
| Pattern(GlobSet), | ||
| Ignorefile(Gitignore), | ||
| // TODO: Control of matching cachedir. | ||
| } | ||
|
|
||
|
|
@@ -61,16 +63,22 @@ impl Exclude { | |
| for path in exclude_from { | ||
| add_patterns_from_file(&mut gsb, path.as_ref())?; | ||
| } | ||
| Ok(Exclude { | ||
| globset: gsb.build()?, | ||
| }) | ||
| Ok(Exclude::Pattern(gsb.build()?)) | ||
| } | ||
|
|
||
| pub fn from_ignorefile(file: impl AsRef<Path>) -> Result<Exclude> { | ||
| let mut builder = GitignoreBuilder::new("/"); | ||
|
sourcefrog marked this conversation as resolved.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the effect of this is that the root of the backup is matched by the root of gitignore patterns: in the same place that the git tree root would normally match? That makes sense.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this is correct. |
||
|
|
||
| if let Some(error) = builder.add(file) { | ||
| return Err(error.into()); | ||
| } | ||
|
|
||
| Ok(Exclude::Ignorefile(builder.build()?)) | ||
| } | ||
|
|
||
| /// Exclude nothing, even items that might be excluded by default. | ||
| pub fn nothing() -> Exclude { | ||
| Exclude { | ||
| globset: GlobSet::empty(), | ||
| } | ||
| Exclude::Pattern(GlobSet::empty()) | ||
| } | ||
|
|
||
| /// True if this apath should be excluded. | ||
|
|
@@ -80,7 +88,10 @@ impl Exclude { | |
| A: ?Sized, | ||
| { | ||
| let apath: Apath = apath.into(); | ||
| self.globset.is_match(apath) | ||
| match self { | ||
| Self::Pattern(globset) => globset.is_match(apath), | ||
| Self::Ignorefile(inner) => inner.matched(apath, false).is_ignore(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,6 @@ | |
|
|
||
| use std::time::Duration; | ||
|
|
||
|
|
||
| use crate::stats::Sizes; | ||
|
|
||
| pub fn bytes_to_human_mb(s: u64) -> String { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you erroneously deleted this line which is causing the test failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦 how the hell did I miss that line in my own review of the changes....
Good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well that had be a very funny but yet unlucky circumstance.
Not sure how we should handle this conflict in this case.