openSUSE / obs-scm-bridge

GNU General Public License v2.0
3 stars 7 forks source link

Don't pull lfs always #12

Closed dcermak closed 1 year ago

dcermak commented 1 year ago

When git-lfs is installed, then lfs archives will be always downloaded as well, which is in contrast to the documentation which says that this will only happen when the query lfs=1 is present in the URL. To prevent lfs archives from being pulled, we clone the repository with the environment variable GIT_LFS_SKIP_SMUDGE set to 1, which prevents automatic downloads (see https://stackoverflow.com/a/42021818).

Additionally, we have to use git lfs pull instead of git lfs fetch as fetch does not replace the placeholder text files with the binaries, it only retrieves them.

adrianschroeter commented 1 year ago

I wonder if we should go the opposite way and keep git default as it is (download LFS objects by default).

Instead we may turn this into an opt-out, where one can disable LFS downloads, eg. when working local.

So the bridge would behave like git by default.

Does anyone have an opinion about this?

dcermak commented 1 year ago

An opt out behavior has the disadvantage that it will do the "wrong thing" if you don't have git-lfs installed. The service should fail to run a git lfs pull if you don't have git-lfs installed, but it will happily clone the repository (only all your lfs files will be text files).

adrianschroeter commented 1 year ago

I don't mind a dependency to git-lfs to ensure that it is installed

dcermak commented 1 year ago

I don't mind a dependency to git-lfs to ensure that it is installed

Ok, done

adrianschroeter commented 1 year ago

hm, can you add an opt-out option for LFS? Maybe just setting the GIT_LFS_SKIP_SMUDGE env variable.

IMHO we need it for people who have less bandwith (or servers are slow). osc co --limit-size XXX was requested by multiple people urgently back then

dcermak commented 1 year ago

You can now opt out via the query lfs=0 and only via that query, lfs=anything_other_than_0 will do nothing

adrianschroeter commented 1 year ago

ah, that is fine. will rebasing and merge ...