SRombauts / UEGitPlugin

Unreal Engine 5 Git LFS 2 Source Control Plugin (beta)
http://srombauts.github.io/UEGitPlugin
MIT License
808 stars 165 forks source link

LFS 2: unable to unlock unmodified files in UE 4.20 #79

Closed JuKu closed 5 years ago

JuKu commented 5 years ago

Hi,

we are using Unreal Engine 4.20 with your git plugin. Also we have a git repository with LFS enabled and want to lock files. So we opened a .umap file and click on "Source Control" --> "Checkout". This works fine and it seems to be the file was locked. Now first question: is this file locked only locally or also on remote (origin) repository? And next: How can i unlock the file if i dont want to make any changes to this file?

Thank you!

JuKu commented 5 years ago

On locking i get this message:

The following assets could not be successfully checked out from source control:
/Game/Maps/Hallway

But after this message the lock is set: dd

But i cannot unlock the file: ddkopie

Logs file: GOTY.log

SRombauts commented 5 years ago

Are you using a github repository? You need Git LFS 2 support on the server! I'll also have a look at the log file, and let you know.

To answer your questions:

  1. Locks are supposed to be server side, but the server needs to support this!
  2. Unlock is normally done with revert (not working for you...)

But for sure, using a server without proper git lfs 2 file locks support should not leads to your broken situation. My bad!

JuKu commented 5 years ago

@SRombauts I am using a self-hosted gitlab installation with enabled git lfs support. I can also manually lock and unlock files with git console.

dd

ddkopie

Repository settings on gitlab:

dd

So i think git lfs is enabled and works fine on server, or? Btw, if i lock a file with UE4 git plugin, file is really locked. But the plugin doesnt detect, that the file was locked successfully.

SRombauts commented 5 years ago

Indeed Gitlab seems to support file locking, at long last!

So I'll check this, I think it might be a bug in the way the plugin handle locking. I was developed and tested with Github only, and might not parse the results in a generic way.

JuKu commented 5 years ago

@SRombauts Thanks!

sensvos commented 5 years ago

Hello, it seems like recognizing Locked files as revertable did the job for me:

GitSourceControlState.cpp:

bool FGitSourceControlState::CanRevert() const
{   
    return IsModified() || LockState == ELockState::Locked; // Allow reverting locked, unchanged files.
}

It works as expected, yet I am not sure if it does not have other implications, I am looking forward to hearing from you about your insight.

SRombauts commented 5 years ago

Ooh, yes! This is a new thing from 4.20! A colleague of mine changed UE4.20 to actually use CanRevert()!

Edit: what I mean is that this callback was unused until recently, so it is not surprising it is now causing problems.

JuKu commented 5 years ago

@SRombauts I understand. Thanks for that information! :)

SRombauts commented 5 years ago

Two more things, after some reading and testing:

  1. I will go for the following variant, as used in the Perforce and the standard Git plugin:

    bool FGitSourceControlState::CanRevert() const
    {
    return CanCheckIn();
    }
  2. Your modification is not working, the Revert operation is doing nothing if the asset is not modified. There is also the need to change from GitSourceControlOperations.cpp, in the function GetMissingVsExistingFiles(), else if(State->IsModified()) should also be modified to now use CanRevert() instead This I have not had the time to think fully, it does not satisfy me, as it might not work as intended, calling "git revert" when in fact no modification are done to the file. I might use one more else if()

Please let me know if you agree

sensvos commented 5 years ago

Argh, sorry it seems like I modified the GetMissingVsExistingFiles() function as well and I forgot about it, I totally agree :)

JuKu commented 5 years ago

@SRombauts Thanks for that great support!

@SRombauts How can i use the new version of plugin? Do i have to compile it self or does UE4 already compiling the plugin itself?

EDIT: UE4 returns this error message:

dd

SRombauts commented 5 years ago

@JuKu I'll release a new version of the plugin quite soon, still need some testing

JuKu commented 5 years ago

@SRombauts Is there a new version out there?

alexgchap commented 5 years ago

@SRombauts Hello, First of all, great plugin!

Sadly, I am experiencing the same issue as mentioned above.

I am using gitea, a self-hosted git server taht supports git lfs 2.0. When using source control > checkout on a .uasset in UE's content browser, The file actually gets locked on the server, which is great.

The problem is that I also get the error : "The following assets could not be successfully checked out from source control: xxx.uasset"

I am then unable to submit to source control or even revert... When I submit to source control, the plugin prompts (in the lower right corner) No assets to check in...

I am using UE4.22 Is there any update on this? This is a roadblock for me...

SRombauts commented 5 years ago

Hi @alexgchap and @JuKu, I have released many versions of the plugin since this issue has been closed: if it is still not working for you, I would like you to open a new bug report, with some log to help me diagnose and think of a fix. Also, as stated elsewhere, I am not using LFS anymore so the support is not that good on my part as I would love too, sorry.

Sébastien