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

Cannot view asset dif if it is in Git LFS and not already downloaded to local LFS storage #78

Open
DJm00n opened this issue Jul 24, 2018 · 2 comments
Assignees
Labels

Comments

@DJm00n
Copy link

DJm00n commented Jul 24, 2018

Seems like UE Git plugin runs git cat-file --filters to produce temp file to compare with but its binary content is prepended with Downloading $filename ($size) text sometimes.
Source of issue that git cat-file --filters calls smudge filter hook and git-lfs.exe under the hood and outputs text to stderr if file is not already in local FLS storage (.git/lfs/objects):
https://github.com/git-lfs/git-lfs/blob/d2341b5c06cb70cc660f0e2cb9c2012855998566/lfs/gitfilter_smudge.go#L87

I think you need find a way to ignore stderr output here:
https://github.com/SRombauts/UE4GitPlugin/blob/46d94ff40f7b6bcc73639f9473eb03d63649747f/Source/GitSourceControl/Private/GitSourceControlUtils.cpp#L1014

I just added new param void * PipeErrorChild to FGenericPlatformProcess and implemented it on each platform:

-       static FProcHandle CreateProc( const TCHAR* URL, const TCHAR* Parms, bool bLaunchDetached, bool bLaunchHidden, bool bLaunchReallyHidden, uint32* OutProcessID, int32 PriorityModifier, const TCHAR* OptionalWorkingDirectory, void* PipeWriteChild, void * PipeReadChild = nullptr);
+       static FProcHandle CreateProc( const TCHAR* URL, const TCHAR* Parms, bool bLaunchDetached, bool bLaunchHidden, bool bLaunchReallyHidden, uint32* OutProcessID, int32 PriorityModifier, const TCHAR* OptionalWorkingDirectory, void* PipeWriteChild, void * PipeReadChild = nullptr, void * PipeErrorChild = nullptr);

Current implementation just maps stderr to PipeWriteChild on Windows and Mac but ignores it at all on Linux.
Maybe you can find a better way to fix this obvious bug.

@SRombauts SRombauts self-assigned this Sep 11, 2018
@SRombauts SRombauts added the bug label Sep 11, 2018
@SRombauts
Copy link
Owner

ha, yes, I've already encounter this, it's even possible I have already consider quite the same fix, but I try to avoid modifying the engine low level APIs since it's hard to get these change accepted upstream

@geordiemhall
Copy link

geordiemhall commented Feb 13, 2022

Not sure if a different workaround was ever found for this, but it looks like UE5 will finally have a way to separate stdout and stderr we could use to properly fix this LFS issue
https://github.com/EpicGames/UnrealEngine/commit/06f3ad8098ab3c87f91dc5b0a1fe40a78502b646

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants