Add git-lfs support
ClosedPublic

Authored by demon on Dec 18 2017, 4:22 PM.

Details

Reviewers
mmodell
Group Reviewers
Release-Engineering-Team
Commits
rMSCA25b9fbf8b411: Add git-lfs support
Patch without arc
git checkout -b D921 && curl -L https://phabricator.wikimedia.org/D921?download=true | git apply
Summary

Also supports git-fat. Por que no los dos?

Test Plan

Test some more

Diff Detail

Repository
rMSCA Scap
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
demon created this revision.Dec 18 2017, 4:22 PM
Restricted Application added a reviewer: mmodell. · View Herald TranscriptDec 18 2017, 4:22 PM
Restricted Application added a reviewer: Release-Engineering-Team. · View Herald Transcript
Restricted Application added a project: Release-Engineering-Team. · View Herald Transcript
demon requested review of this revision.Dec 18 2017, 4:24 PM
demon edited the summary of this revision. (Show Details)Dec 18 2017, 4:25 PM
mmodell accepted this revision as: mmodell.Dec 20 2017, 9:00 PM

This looks good, other than teh backward compatibility break. Should we retain support for the old config or just force deployers to update their scap.cfg without warning?

This revision is now accepted and ready to land.Dec 20 2017, 9:00 PM

What b/c break? Nobody should be using git_binary_manager yet, I just introduced it a few days ago. git_fat should still work with a warning to the client.

demon updated this revision to Diff 2468.Jan 3 2018, 10:33 PM

Rebased against master

thcipriani added inline comments.
scap/git.py
92

I don't think we can get rid of this function.

It's unneeded for already initialized repos, but for repositories that already exist that want to add a file from git-fat, or for new repositories that don't exist on targets, or for new targets we'll still need to run git fat init to set the smudge and clean filters for the repository.

IIRC git-lfs side-steps the need for initialization somehow (setting-up filters as part of the initial pull I think?), but I read that code before XMas and don't remember all the details...

demon added inline comments.Jan 12 2018, 5:23 PM
scap/git.py
92

I remember reading the source for git-fat and it lead me to thinking this wasn't necessary anymore. I could be wrong.

(Also, I thought I landed this....)

thcipriani added inline comments.Jan 12 2018, 10:37 PM
scap/git.py
92

It looks like git fat still needs this.

Re-reading the source for git-fat, it looks like what it does on git fat pull is rsync everything down to .git/fat/objects and then calls git checkout-index --index --force [file] on each file it perceives to be a fat file (files that are 74 bytes long and when unrehydrated start with the magic cookie #$# git-fat ) which causes the smudge filter to be re-run to put the file in its final location, but it looks like the only place that the smudge filter is setup is in the init command: https://github.com/wikimedia/operations-debs-git-fat/blob/master/git-fat#L576-L583 so for new targets or new deployments we'll need to run git fat init once to ensure that the smudge filter is set correctly.

demon added inline comments.Jan 12 2018, 10:42 PM
scap/git.py
92

I see I see. The overhead of checking if the smudge filters are initialized (plus making sure they're accurate if they're later changed) seems lame. We could just call git fat init unconditionally in that scenario.

thcipriani added inline comments.Jan 14 2018, 7:37 PM
scap/git.py
92

Seems fine in testing git fat init repeatedly doesn't exit 0 or emit anything on stderr:

fat-test (master)
(/^ヮ^)/*:・゚✧ git fat init
Git fat already configured, check configuration in .git/config
fat-test (master)
(/^ヮ^)/*:・゚✧ git fat init
Git fat already configured, check configuration in .git/config
fat-test (master)
(/^ヮ^)/*:・゚✧ git fat init
Git fat already configured, check configuration in .git/config
fat-test (master)
(/^ヮ^)/*:・゚✧ git fat init
Git fat already configured, check configuration in .git/config
fat-test (master)
(/^ヮ^)/*:・゚✧ git fat init 2>/dev/null
Git fat already configured, check configuration in .git/config
fat-test (master)
(/^ヮ^)/*:・゚✧ echo $?
0
demon updated this revision to Diff 2484.Jan 14 2018, 9:50 PM

Restores git.fat_init() and git.fat_isinitialized()

demon marked 5 inline comments as done.Jan 14 2018, 10:32 PM
demon updated this revision to Diff 2488.Jan 16 2018, 5:34 PM
  • Rewrite to avoid git_binary_manager -> git_largefile_managers rename
  • Should be more better back-compat
demon marked 5 inline comments as not done.Jan 16 2018, 5:35 PM

All done with the comments, should be better now.

demon marked 5 inline comments as done.Jan 16 2018, 5:35 PM

(whoops on inline comments un-done-ing)

demon edited the summary of this revision. (Show Details)Jan 16 2018, 8:04 PM
mmodell accepted this revision.Jan 31 2018, 10:18 PM
This revision was automatically updated to reflect the committed changes.