Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Saving an old revision of an LFS file saves only the LFS pointer #6146

Open
markonius opened this issue Jan 21, 2019 · 17 comments
Open

Saving an old revision of an LFS file saves only the LFS pointer #6146

markonius opened this issue Jan 21, 2019 · 17 comments
Assignees
Labels
:neckbeard: status: pull request only Requested changes won't be implemented by the Core team, community PRs are welcome type: bug 🐛 up-for-grabs Easy tasks for those looking to get involved. Refer to https://up-for-grabs.net/

Comments

@markonius
Copy link
Contributor

Current behaviour

When viewing the history of an LFS-tracked file, if I click "Save as" in the right click menu of an old revision of the file, only the LFS pointer exists in the saved file.

If, however, I choose "Difftool selected < - > local", the entire file is shown in KDiff.

Expected behaviour

The entire file should be saved to disk when selecting "Save as" on an old revision of an LFS-tracked file.

Steps to reproduce

  1. Have a file in git-LFS
  2. Commit some changes to it several times
  3. Find the file in the file tree
  4. Select "Viev history"
  5. Try to save an old revision of the file
  6. Observe that the file is corrupted (only the LFS pointer is saved)
  7. Try to diff it
  8. Observe that the entire file is visible in KDiff

Screenshots

image
image
image

Did this work in previous version of GitExtensions

As far as I'm aware, no.

Environment

  • Git Extensions 3.00.00.4433
  • Build fca7cf2
  • Git 2.19.1.windows.1
  • Microsoft Windows NT 10.0.17134.0
  • .NET Framework 4.7.3260.0
  • DPI 96dpi (no scaling)

Diagnostics

@vbjay
Copy link
Contributor

vbjay commented Jan 21, 2019 via email

@RussKie
Copy link
Member

RussKie commented Jan 21, 2019

@markonius are you in a position to debug it?

@markonius
Copy link
Contributor Author

markonius commented Jan 22, 2019

Yo, I can spare some time. Gonna try upgrading git first. I'm available on gitter currently.

EDIT: Updating Git had no effect

@gerhardol
Copy link
Member

gerhardol commented Jan 22, 2019

GE uses what Git supplies here
Git does not use the LFS filters for git-diff, only git-difftool
So to provide a solution, GE would have to detect that the file is LFS and wrap the git-diff call

The textual diff uses

git -c diff.submodule=short -c diff.noprefix=false diff --no-color --unified=3 --patience -M -C 

The GUI tool uses

git difftool --gui --no-prompt -M -C 

I use LFS quite a lot and do not believe this is a limitation. Files are set to LFS because they are binary (no diff anyway) or very big (where plain text diff is not usable). A 3rd party difftool like Beyond Compare makes more sense.

I recommend registering this as a feature, to wrap the diff for LFS files, but for me personally it has low priority.

@RussKie RussKie added :neckbeard: status: pull request only Requested changes won't be implemented by the Core team, community PRs are welcome up-for-grabs Easy tasks for those looking to get involved. Refer to https://up-for-grabs.net/ labels Jan 22, 2019
@markonius
Copy link
Contributor Author

@gerhardol Well, I'm mostly concerned with the "Save as" functionality, not the diff itself, since that is the case when I would be prepared to download a big file from the server.

@gerhardol
Copy link
Member

Well, I'm mostly concerned with the "Save as" functionality

That will also require special handling, if the object is a LFS object it has to be fetched first and the LFS only runs on checking out.
It may be possible to force the LFS filter to run here though.

@vbjay
Copy link
Contributor

vbjay commented Jan 25, 2019

@markonius
Copy link
Contributor Author

@vbjay As far as I understand, this downloads the files to the working directory. I found git lfs smudge though. The documentation says: "Read a Git LFS pointer file from standard input and write the contents of the corresponding large file to standard output. " I'm gonna try playing with it and see where it gets me.

@vbjay
Copy link
Contributor

vbjay commented Jan 25, 2019

No fetch does not download to working directory.

It was in response to

That will also require special handling, if the object is a LFS object it has to be fetched first

Where does git store objects by default? .git\objects Where does lfs store objects? Think about lfs fetch as the same as git fetch.

@markonius
Copy link
Contributor Author

Oh OK. So, in theory, running git-lfs-fetch --include="my/file" and then copying the file to the user specified location wold achieve the result?
Would git lfs smudge be a cleaner solution still?
https://github.com/git-lfs/git-lfs/blob/master/docs/man/git-lfs-smudge.1.ronn

@vbjay
Copy link
Contributor

vbjay commented Jan 25, 2019

No. You would not copy anything. You know. smudge will do it all. We can use fetch when needing to go offline( think working on an airplane)

@vbjay
Copy link
Contributor

vbjay commented Jan 25, 2019

Just be mindful of git-for-windows/git#1063

@markonius
Copy link
Contributor Author

OK, sorry, but you lost me.

No. You would not copy anything. You know. smudge will do it all. We can use fetch when needing to go offline( think working on an airplane)

I don't understand this comment at all 😮
Sorry if I'm being a bit ignorant here about GE and git in general, I was just a user of both until now.

As a starting point, let's agree I want to fix the issue I originaly reported (make "Save as" work with LFS).
Therefore, the file would need to be downloaded from the server and then saved to a user specified location. If we use git lfs fetch, the file would end up (if I figured it out correctly) in .git/lfs/objects. If on the other hand we use git lfs smudge, (if I understand correctly) its STDOUT could be directed straight to the user specified location on disk.

What's your take on this? Am I missing something?

@vbjay
Copy link
Contributor

vbjay commented Jan 25, 2019

Oh OK. So, in theory, running git-lfs-fetch --include="my/file" and then copying the file

Nope. Smudge.

We would provide fetch for I want to work offline and still have access to lfs files.

@vbjay
Copy link
Contributor

vbjay commented Jan 25, 2019

So yes. We would write the output of smudge to the path passed in to save as.

@vbjay
Copy link
Contributor

vbjay commented Jan 25, 2019

Just to point out offline flow:

You have cloned and checked out a repo. Let's say you are going to do some work on an airplane( no internet). The lfs files will need to be fetched before you can check them out. You could fetch recent or all or whatever. This would pull down the lfs files to your repo folder. You could then go offline and checkout without any issue of not having the file. Later on the lfs cache could be cleaned to clear up the local storage if desired.

@markonius
Copy link
Contributor Author

I'm poking around and gonna try to figure out how to implement this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:neckbeard: status: pull request only Requested changes won't be implemented by the Core team, community PRs are welcome type: bug 🐛 up-for-grabs Easy tasks for those looking to get involved. Refer to https://up-for-grabs.net/
Projects
None yet
Development

No branches or pull requests

4 participants