Skip to content

Add --ignore-inode option to backup cmd.#2047

Closed
giacomocariello wants to merge 3 commits intorestic:masterfrom
gstruct:ignore_inode_feat
Closed

Add --ignore-inode option to backup cmd.#2047
giacomocariello wants to merge 3 commits intorestic:masterfrom
gstruct:ignore_inode_feat

Conversation

@giacomocariello
Copy link
Copy Markdown

@giacomocariello giacomocariello commented Oct 16, 2018

What is the purpose of this change? What does it change?

This PR adds --ignore-inode option to backup command, in order to support those filesystems that don't support fixed inodes, typically Fuse, pCloud, etc.

Was the change discussed in an issue or in the forum before?

It was already discussed #1631.

Checklist

  • I have read the Contribution Guidelines
  • I have added tests for all changes in this PR
  • I have added documentation for the changes (in the manual)
  • There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
  • 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

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 16, 2018

Codecov Report

Merging #2047 into master will decrease coverage by 4.1%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2047      +/-   ##
==========================================
- Coverage   51.18%   47.07%   -4.11%     
==========================================
  Files         176      176              
  Lines       14065    14069       +4     
==========================================
- Hits         7199     6623     -576     
- Misses       5817     6449     +632     
+ Partials     1049      997      -52
Impacted Files Coverage Δ
cmd/restic/cmd_backup.go 44.91% <100%> (+0.78%) ⬆️
internal/archiver/archiver.go 68.62% <90.9%> (ø) ⬆️
internal/backend/b2/b2.go 0% <0%> (-80.69%) ⬇️
internal/backend/swift/swift.go 0% <0%> (-78.45%) ⬇️
internal/backend/gs/gs.go 0% <0%> (-73.61%) ⬇️
internal/backend/azure/azure.go 0% <0%> (-69.46%) ⬇️
internal/backend/swift/config.go 36.95% <0%> (-54.35%) ⬇️
internal/archiver/blob_saver.go 100% <0%> (+4.93%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4b39ae...9e5e9a1. Read the comment docs.

@pabs3
Copy link
Copy Markdown

pabs3 commented Oct 29, 2018

This reduced the amount of time it took for converting my rdiff-backup snapshots to restic snapshots from over an hour to under 8 minutes. I would love to see it merged into restic mainline.

Copy link
Copy Markdown
Member

@fd0 fd0 left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution! I've had a look at the code, while it get's the job done, a lot of places must be touched to pass in an option of type ModifiedIgnores, which only has one field. That feels overly generic to me.

I'd like to propose a different approach:

  • Add a new field to the Archiver struct, IgnoreInode bool
  • Change the signature of fileChanged like this:
func fileChanged(fi os.FileInfo, node *restic.Node, ignoreInode bool) bool
  • Pass the arch.IgnoreInode to fileChanged

The default value for the bool is false, so the inode is checked.

What do you think? Please let me know if you need any help!

Other nitpick: Can you please remove the trailing . from the summary line fro the commit messages? Thanks!

@giacomocariello
Copy link
Copy Markdown
Author

@fd0 yup, no problem on my side. I'll try and apply your requests. The idea of having a struct to handle multiple types of ignore flags was probably overengineered at this stage.

@fd0
Copy link
Copy Markdown
Member

fd0 commented Jan 6, 2019

Hey, are you still interested in working on this? Is there anything you need, or are you stuck?

@robvalca
Copy link
Copy Markdown

I'm really interested in this feature, do you know if it will go in, finally? :)

@fd0
Copy link
Copy Markdown
Member

fd0 commented Feb 21, 2019

@robvalca this PR is incomplete, it looks like @giacomocariello did not have the time to rework it yet. So we need to wait until this happens (or somebody else volunteers to do it) :)

@robvalca
Copy link
Copy Markdown

@fd0, @giacomocariello do you think that I could take on this? I'm newbie in golang (and btw in github collab) but with the work already done and some help I think that I could finish this. :)

HeikoBornholdt added a commit to HeikoBornholdt/restic that referenced this pull request Mar 10, 2019
@fd0
Copy link
Copy Markdown
Member

fd0 commented Mar 16, 2019

Superseded by #2205, I'm closing this Pr. Thank you very much for your work @giacomocariello!

@fd0 fd0 closed this Mar 16, 2019
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.

6 participants