Proposal: new hook to detect LFS attribute inconsistencies · Issue #1020 · pre-commit/pre-commit-hooks · GitHub

There are two common errors when working with LFS:

Keeping files as general objects after setting LFS attributes (LFS produces a warnings like "Encountered 1 file(s) that should have been pointers, but weren't")
Keeping LFS pointers after removing the attributes.

Proposed implementation of a check-lfs-attributes hook: https://github.com/klimkin/pre-commit-hooks/tree/feature/check-lfs-attributes

doesn't lfs already have things in place to prevent this?

Below, after tracking the file you must add it again to create proper commit. I don't think LFS can prevent this kind of escape unless we use some kind of Git hook.

@xfailif_no_gitlfs
def test_regular_object_but_tracked_by_lfs(temp_git_dir_as_cwd, capsys):  # pragma: no cover
    temp_git_dir_as_cwd.join('a.bin').write('a')
    cmd_output('git', 'lfs', 'install', '--local')
    cmd_output('git', 'add', 'a.bin')
    cmd_output('git', 'lfs', 'track', '*.bin')
    assert main(('a.bin',)) == 1
    out, _ = capsys.readouterr()
    assert 'a.bin is tracked by LFS but added as a regular object' in out

right but when a push is executed lfs checks this right?

It doesn't seem LFS is checking for inconsistencies. You can push .gitattributes without converting to pointers.

Looking at the LFS pre-push hook https://github.com/git-lfs/git-lfs/blob/b96d77b9563a8fd10c4e03f8f8898a0777ead9a6/commands/command_pre_push.go#L19, it only handles the logic of pushing the content for the pointers.

that really feels like something lfs should just fix -- I thought their pre-push did more validation than that?

I also think that check-added-large-files is sufficient to catch this case already

There are two cases to catch:

Files with lfs attribute, but committed as general objects
Files without lfs attribute, but committed as LFS pointers

@asottile Are you suggesting adding the checks to check-added-large-files?

that really feels like something lfs should just fix -- I thought their pre-push did more validation than that?

Did it?

Proposal: new hook to detect LFS attribute inconsistencies · Issue #1020 · pre-commit/pre-commit-hooks · GitHub

Repository navigation

Proposal: new hook to detect LFS attribute inconsistencies #1020

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Proposal: new hook to detect LFS attribute inconsistencies · Issue #1020 · pre-commit/pre-commit-hooks · GitHub

Repository navigation

Proposal: new hook to detect LFS attribute inconsistencies #1020

Description

Activity

asottile commented on Mar 2, 2024

klimkin commented on Mar 4, 2024

asottile commented on Mar 4, 2024

klimkin commented on Mar 5, 2024

asottile commented on Mar 5, 2024

asottile commented on Mar 5, 2024

klimkin commented on Mar 5, 2024

klimkin commented on Mar 5, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions