Skip to content
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

Cache evaluated directories in isExcludedByFile #1326

Merged
merged 1 commit into from Oct 7, 2017

Conversation

fawick
Copy link
Member

@fawick fawick commented Oct 4, 2017

Fixes #1271.

@fawick
Copy link
Member Author

fawick commented Oct 4, 2017

@phillipp I'd be interested to learn whether this PR improves your usecase from #1271

@codecov-io
Copy link

codecov-io commented Oct 4, 2017

Codecov Report

Merging #1326 into master will decrease coverage by 5.37%.
The diff coverage is 26.31%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1326      +/-   ##
=========================================
- Coverage   51.98%   46.6%   -5.38%     
=========================================
  Files         134     134              
  Lines       13108   13123      +15     
=========================================
- Hits         6814    6116     -698     
- Misses       5463    6224     +761     
+ Partials      831     783      -48
Impacted Files Coverage Δ
cmd/restic/exclude.go 24.48% <23.52%> (-0.33%) ⬇️
cmd/restic/cmd_backup.go 27.82% <50%> (+0.2%) ⬆️
internal/backend/b2/b2.go 0% <0%> (-79.93%) ⬇️
internal/backend/azure/azure.go 0% <0%> (-75.35%) ⬇️
internal/backend/swift/swift.go 0% <0%> (-73.9%) ⬇️
internal/backend/gs/gs.go 0% <0%> (-65.6%) ⬇️
internal/backend/swift/config.go 34.37% <0%> (-56.25%) ⬇️
internal/checker/checker.go 72.02% <0%> (+3.83%) ⬆️

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 1dcfd64...a69b717. Read the comment docs.

if tagFilename == "" {
return false
}
dir, base := filepath.Split(filename)
if base == tagFilename {
return false // do not exclude the tagfile itself
}
if rc != nil {
Copy link
Member

Choose a reason for hiding this comment

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

It'd be nice if this were a method on the rc struct, and it should also work if rc is nil, so we can abstract that away, like rejected, ok := rc.Get(dir); if ok { return rejected }. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. The mutex must remain locked until the function returnes, though, so I wrap its Lock() and Unlock() methods, too.

}

rejected := isDirExcludedByFile(dir, tagFilename, header)
if rc != nil {
Copy link
Member

Choose a reason for hiding this comment

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

This one too, e.g. rc.Store(dir, rejected).

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member Author

@fawick fawick left a comment

Choose a reason for hiding this comment

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

@fd0 The changes you requested have been implemented.

@fd0 fd0 merged commit f0f17db into restic:master Oct 7, 2017
fd0 added a commit that referenced this pull request Oct 7, 2017
Cache evaluated directories in isExcludedByFile
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.

None yet

3 participants