search

jasonwhite / rudolfs

A high-performance, caching Git LFS server with an AWS S3 and local storage back-end.
MIT License
392 stars 39 forks source link

fix: local disk incorrect key_to_path & make doc more clear. #33

Closed swordow closed 3 years ago

swordow commented 3 years ago

If starts rudolfs with --path=D:\some_path and repo path is D:\some_path\bare_repo and lfs inside the repo like D:\some_path\bare_repo\lfs and lfs config url is D:\some_path\bare_repo\lfs then the object could not be found for the key_to_path returned a wrong path.

jasonwhite commented 3 years ago

Hi @swordow, thanks for the PR. I think it's a cool idea to have this LFS server work with bare repositories. I hadn't considered that use-case before. I'm curious about your use-case as it looks like it is already possible to fetch LFS objects from a bare repository already with the file:///... syntax.

Unfortunately I cannot merge this as-is. It will have significant breakage for any existing LFS data prior to the change in directory structure. Ideally, any change would be backwards compatible for the on-disk format. (There are folks who use Rudolfs in production.)

Here are a couple of ideas on how to avoid this issue (if file:///) doesn't work in your case:

  1. Add a new option called --bare for rudolfs local. Then, the alternate directory structure could be used when this option is specified.
  2. Make encryption optional and change the --key option to no longer be a global option, but a per-backend option. Then, --bare and --key could be made mutually exclusive options.
swordow commented 3 years ago

Hi @swordow, thanks for the PR. I think it's a cool idea to have this LFS server work with bare repositories. I hadn't considered that use-case before. I'm curious about your use-case as it looks like it is already possible to fetch LFS objects from a bare repository already with the file:///... syntax.

Unfortunately I cannot merge this as-is. It will have significant breakage for any existing LFS data prior to the change in directory structure. Ideally, any change would be backwards compatible for the on-disk format. (There are folks who use Rudolfs in production.)

Here are a couple of ideas on how to avoid this issue (if file:///) doesn't work in your case:

  1. Add a new option called --bare for rudolfs local. Then, the alternate directory structure could be used when this option is specified.
  2. Make encryption optional and change the --key option to no longer be a global option, but a per-backend option. Then, --bare and --key could be made mutually exclusive options.

ok, nice ideas, i will try, tks.

swordow commented 3 years ago

Hi @swordow, thanks for the PR. I think it's a cool idea to have this LFS server work with bare repositories. I hadn't considered that use-case before. I'm curious about your use-case as it looks like it is already possible to fetch LFS objects from a bare repository already with the file:///... syntax.

Unfortunately I cannot merge this as-is. It will have significant breakage for any existing LFS data prior to the change in directory structure. Ideally, any change would be backwards compatible for the on-disk format. (There are folks who use Rudolfs in production.)

Here are a couple of ideas on how to avoid this issue (if file:///) doesn't work in your case:

  1. Add a new option called --bare for rudolfs local. Then, the alternate directory structure could be used when this option is specified.
  2. Make encryption optional and change the --key option to no longer be a global option, but a per-backend option. Then, --bare and --key could be made mutually exclusive options.

In my case, i mirrored a bare repo B from bare repo A with lfs data, and git clone repo C from bare repo B, the lfs data of bare repo B is inside the path/to/repoB/lfs/objects and the repo C's lfs config's url is set to path/to/repoB/lfs, but for the rudolfs's local backend, the key_to_path returned the path "objects/path/to/repoB/lfs/oid" which is not the real path which should be "path/to/repoB/lfs/objects/oid".

jasonwhite commented 3 years ago

@swordow Heads up -- some re-architecting was done. Much of the logic in main.rs was moved to lib.rs (to make testing easier). So, if you were adding the --bare option, you'll need to do a little rebasing. It should now also be possible to add a test for bare repositories in tests/test_local.rs.

swordow commented 3 years ago

@swordow Heads up -- some re-architecting was done. Much of the logic in main.rs was moved to lib.rs (to make testing easier). So, if you were adding the --bare option, you'll need to do a little rebasing. It should now also be possible to add a test for bare repositories in tests/test_local.rs.

Tks, had merged~