Skip to content

Restore flags to include from file and exclude from file#4811

Merged
MichaelEischer merged 12 commits intorestic:masterfrom
konidev20:fix-gh-4781-restore-include-and-exclude-file-switches
Jun 10, 2024
Merged

Restore flags to include from file and exclude from file#4811
MichaelEischer merged 12 commits intorestic:masterfrom
konidev20:fix-gh-4781-restore-include-and-exclude-file-switches

Conversation

@konidev20
Copy link
Copy Markdown
Contributor

@konidev20 konidev20 commented May 19, 2024

What does this PR change? What problem does it solve?

Added --exclude-file, --iexclude-file, include-file and iinclude-file to the restore command.

Was the change previously discussed in an issue or on the forum?

Closes #4781

Checklist

  • I have read the contribution guidelines.
  • I have enabled maintainer edits.
  • I have added tests for all code changes.
  • I have added documentation for relevant changes (in the manual).
  • There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
  • I have run gofmt on the code in all commits.
  • All commit messages are formatted in the same style as the other commits in the repo.
  • I'm done! This pull request is ready for review.

@konidev20 konidev20 marked this pull request as draft May 19, 2024 18:05
@konidev20
Copy link
Copy Markdown
Contributor Author

@MichaelEischer please review this and let me know if I am on the right track.

Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

I'd prefer a different design. runRestore is already too long, so I'd like to extract the include/exclude pattern parsing code. Judging from a quick look at the exclude code used by the backup command (see cmd/restic/exclude.go), it should be possible to directly reuse the code for the exclude options. And it should be possible to adapt that pattern to add new include options. (Feel free to ask if you need more details to understand what I have in mind)

@konidev20
Copy link
Copy Markdown
Contributor Author

I'd prefer a different design. runRestore is already too long, so I'd like to extract the include/exclude pattern parsing code. Judging from a quick look at the exclude code used by the backup command (see cmd/restic/exclude.go), it should be possible to directly reuse the code for the exclude options. And it should be possible to adapt that pattern to add new include options. (Feel free to ask if you need more details to understand what I have in mind)

I wanted to propose this too. I will update this draft over the next weekend.

@konidev20 konidev20 force-pushed the fix-gh-4781-restore-include-and-exclude-file-switches branch from 5e33a19 to 1fda42b Compare June 1, 2024 08:50
@konidev20 konidev20 requested a review from MichaelEischer June 1, 2024 15:10
@konidev20 konidev20 marked this pull request as ready for review June 1, 2024 15:11
@konidev20 konidev20 force-pushed the fix-gh-4781-restore-include-and-exclude-file-switches branch from 8b6b792 to 24a247a Compare June 8, 2024 07:54
Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

I've found a few more issues. Currently restore --exclude-file ... does not work as hasExcludes is no longer correct. The same applies for include files.

@konidev20 konidev20 requested a review from MichaelEischer June 9, 2024 13:49
@konidev20
Copy link
Copy Markdown
Contributor Author

Tests failing is unrelated.

Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

I have a few more comment, after that we should be ready to go.

doc: update exclude and include docs
@konidev20 konidev20 requested a review from MichaelEischer June 9, 2024 20:30
Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the blazingly fast fixes!

@MichaelEischer MichaelEischer added this pull request to the merge queue Jun 10, 2024
Merged via the queue into restic:master with commit be05a17 Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restic restore include-file/exclude-file switches

2 participants