Skip to content

Fsck command#373

Merged
technoweenie merged 12 commits intogit-lfs:masterfrom
michael-k:fsck
Jun 18, 2015
Merged

Fsck command#373
technoweenie merged 12 commits intogit-lfs:masterfrom
michael-k:fsck

Conversation

@michael-k
Copy link
Contributor

This PR is built on #280 by @zeroshirts and #290 by @technoweenie.

I transferred the test to the new test environment. Should be enough to close #262.

@zeroshirts:
I squashed a05e331 (Add LFS fsck command) and 1c8c54d (code review changes) together and moved file.Close() right after _, err = io.Copy(oidHash, f).¹ I hope this is ok with you!?

¹ See comment in 2f6c3be. If io.Copy fails, we still want to close the file.

@michael-k
Copy link
Contributor Author

Some comments on questions raised in #290:

Is fsck the right name?

Consistent with git and git-annex. → Yes

Is deleting the corrupt objects the right default?

Potential data loss. → No

I'm not sure what we could do to fix files.

Not git-lfs' job.

Observations (not necessarily blockers for this PR)

Nice things we should document somewhere in order to do them in other PRs. I'd say the current user interface is a good default.

@christophershirk
Copy link

LGTM
edit: I didn't vet the integration test.. my eyes glaze over on shell scripts =]

@technoweenie technoweenie mentioned this pull request Jun 11, 2015
38 tasks
@technoweenie
Copy link
Contributor

Awesome, thanks for picking this back up. I'd like to ship this with Git LFS v0.5.2 some time next week.

We do need to add a man page for this.

@michael-k
Copy link
Contributor Author

👍 I've put the man page on my todo list for the next days.

@technoweenie technoweenie mentioned this pull request Jun 17, 2015
16 tasks
@technoweenie
Copy link
Contributor

sha256sum: command not found

I use shasum -a 256, but it may be easier to write one in Go. I have no idea what we can use that will work cross platform.

@technoweenie
Copy link
Contributor

@michael-k
Copy link
Contributor Author

The gist3 link returns a 404. This one works, though: https://gist.github.com/technoweenie/85e1611dff2289bd7e18

I changed sha256sum to shasum -a 256. Other tests are using shasum -a 256 too, therefore I thing it's out of scope for this PR to move to a custom hash program.

@michael-k
Copy link
Contributor Author

I added a man page. From my POV this PR is ready.

@technoweenie
Copy link
Contributor

Other tests are using shasum -a 256 too, therefore I thing it's out of scope for this PR to move to a custom hash program.

Ideally all shell scripts in git-lfs are written for lowest common denominator sh environments. I have no idea what the standard shasum command is, or if there even is one. I agree that it's not worth adding a custom command until we need to though.

technoweenie added a commit that referenced this pull request Jun 18, 2015
@technoweenie technoweenie merged commit 955a06d into git-lfs:master Jun 18, 2015
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 15, 2025
The "git lfs fsck" command accepts a --dry-run option, which was
introduced in commit 7cfaa63 of
PR git-lfs#373, but which is not documented in our git-lfs-fsck(1) manual page,
so we now add an entry for it into the list of options supported by
the command.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 27, 2025
The "git lfs fsck" command accepts a --dry-run option, which was
introduced in commit 7cfaa63 of
PR git-lfs#373, but which is not documented in our git-lfs-fsck(1) manual page,
so we now add an entry for it into the list of options supported by
the command.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 27, 2025
The "git lfs fsck" command accepts a --dry-run option, which was
introduced in commit 7cfaa63 of
PR git-lfs#373, but which is not documented in our git-lfs-fsck(1) manual page,
so we now add an entry for it into the list of options supported by
the command.
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 fsck (bitrot detection)

3 participants