Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Worktree.Add: Support Add deleted files, fixes #571 #577

Merged
merged 7 commits into from
Sep 5, 2017
Merged

Worktree.Add: Support Add deleted files, fixes #571 #577

merged 7 commits into from
Sep 5, 2017

Conversation

grunenwflorian
Copy link
Contributor

In order to fix the bug mentioned in the issue, I modified the Add method of Worktree to handle the case where a file is removed but is still in the repository.

My fix just ensure that the repo and the filesystem are always in sync.

I also found what I think to be a false positive in the TestFilenameNormalization test.
The wrong file were added.

@grunenwflorian
Copy link
Contributor Author

I cannot reproduce that error. Any idea why ?

@mcuadros
Copy link
Contributor

mcuadros commented Sep 4, 2017

Its a test randomly failing, don't worry

@@ -266,6 +266,23 @@ func (w *Worktree) Add(path string) (plumbing.Hash, error) {
return h, err
}

func (w *Worktree) removeIfDeleted(path string, hash plumbing.Hash, err error) (plumbing.Hash, error) {
pathError, ok := err.(*os.PathError)
if ok && pathError.Op == "lstat" || w.isErrNotExist(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use the built-in functionos.IsNotExist?

@codecov
Copy link

codecov bot commented Sep 4, 2017

Codecov Report

Merging #577 into master will decrease coverage by 0.55%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #577      +/-   ##
==========================================
- Coverage   78.01%   77.46%   -0.56%     
==========================================
  Files         129      129              
  Lines        9922     9941      +19     
==========================================
- Hits         7741     7701      -40     
- Misses       1338     1410      +72     
+ Partials      843      830      -13
Impacted Files Coverage Δ
worktree_status.go 74.24% <100%> (+0.22%) ⬆️
plumbing/transport/ssh/common.go 20.54% <0%> (-45.21%) ⬇️
plumbing/transport/ssh/auth_method.go 31.57% <0%> (-22.81%) ⬇️
plumbing/format/packfile/decoder.go 81.94% <0%> (+0.16%) ⬆️
plumbing/format/packfile/diff_delta.go 91.73% <0%> (+0.35%) ⬆️
plumbing/format/packfile/scanner.go 71.2% <0%> (+1.09%) ⬆️
plumbing/format/packfile/common.go 15% <0%> (+15%) ⬆️

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 770800d...d2e5d33. Read the comment docs.

@@ -266,6 +266,19 @@ func (w *Worktree) Add(path string) (plumbing.Hash, error) {
return h, err
}

func (w *Worktree) removeIfDeleted(path string, hash plumbing.Hash, err error) (plumbing.Hash, error) {
pathError, ok := err.(*os.PathError)
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to do the cast or check if is a lsstat, the function is smart enough for detect any non-existan file

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I rather having the if condition where the err is generated instead of having another function. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

GRUNENWALD Florian added 2 commits September 5, 2017 10:09
@mcuadros mcuadros changed the title Fix for the issue #571. Worktree.Add: Support Add deleted files, fixes #571 Sep 5, 2017
@mcuadros mcuadros merged commit a33a60d into src-d:master Sep 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants