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

fs: specify a file as existing if it's empty #4654

Merged
merged 1 commit into from Sep 28, 2021
Merged

Conversation

bk2204
Copy link
Member

@bk2204 bk2204 commented Sep 27, 2021

If the object has a size of zero, then we already have its object: it's the empty file, and we shouldn't need to download it from anywhere. If we inquire if it exists, then say it does, and say its location is the system's equivalent of /dev/null.

The only tricky case is if we're de-duplicating, linking, or writing to a file, in which case we should not try to use a link or replace on the file, since we neither want to link nor replace the system's /dev/null.

Fixes #4645

Copy link
Contributor

@chrisd8088 chrisd8088 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! The new tests definitely fail without these changes, which is excellent. Would there be any value in adding additional tests for the prune and dedup use cases as well?

Comment on lines +117 to +118
if srcFile == os.DevNull {
return true, nil
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this could move up to just below where srcFile is initialized, since we don't need dstFile yet. But it's a minor thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've moved it.

If the object has a size of zero, then we already have its object: it's
the empty file, and we shouldn't need to download it from anywhere.  If
we inquire if it exists, then say it does, and say its location is the
system's equivalent of /dev/null.

The only tricky case is if we're de-duplicating, linking, or writing to
a file, in which case we should not try to use a link or replace on the
file, since we neither want to link nor replace the system's /dev/null.
@bk2204
Copy link
Member Author

bk2204 commented Sep 28, 2021

I've added tests for the prune case; unfortunately, dedup doesn't run on my system, so I can't test it here.

To be clear, I believe prune worked before just fine, it's just that with my changes, we don't want to try to delete /dev/null.

@bk2204 bk2204 merged commit 1a4431a into git-lfs:main Sep 28, 2021
@bk2204 bk2204 deleted the empty-pull branch September 28, 2021 13:40
bk2204 added a commit that referenced this pull request Sep 28, 2021
fs: specify a file as existing if it's empty
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.

git-lfs 3.0.0 - zero byte files not tracked by git-lfs are resulting in Object does not exist on the server
2 participants