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

Fix issue where locking a file would fail on Linux #55

Merged
merged 1 commit into from
Nov 27, 2017

Conversation

CraftedCart
Copy link
Contributor

When locking a file (At least on Linux), it would fail with the following message in the log

Failed to call git rev-parse --show-toplevel: exit status 128
[2017.11.27-00.38.26:982][724]LogSourceControl: Warning: RunLfsCommand(lfs lock) ReturnCode=2:

git lfs will error with that message if the current working directory is not within the git directory. For whatever reason passing the directory as OptionalWorkingDirectory in CreateProc doesn't seem to work.

So I modified the git command to take the git directory with the -C argument.
Now when locking a file, the plugin will build a command like `git -C "/home/user/Unreal Projects/AwesomeProject lfs lock path/to/file.uasset".

@SRombauts SRombauts self-assigned this Nov 27, 2017
@SRombauts SRombauts added the bug label Nov 27, 2017
@SRombauts SRombauts changed the title Fix issue where locking a file would fail Fix issue where locking a file would fail on Linux Nov 27, 2017
@SRombauts
Copy link
Owner

Thanks for this @CraftedCart.

The lock command is working on my Windows install, so it looks like a difference of implementation under Linux.

I didn't know of this "-C" command line flag, perhaps was it not present under git 1.9.x when I started this Git Plugin...

@SRombauts
Copy link
Owner

SRombauts commented Nov 27, 2017

This option appeared in git 1.8.5 (Q4 2013, see RelNotes):

Just like "make -C ", "git -C ..." tells Git
to go there before doing anything else.

So I can safely assume that anyone using the Git plugin should now have a more recent Git version (which was still quite new when I started).

@SRombauts
Copy link
Owner

Hum, the fact is that FLinuxPlatformProcess::CreateProc() does not use the OptionalWorkingDirectory!

@SRombauts
Copy link
Owner

Ok, so I will merge your patch "as-is". I will refactor and re-test it properly later when I have more time.

Thanks again!

@SRombauts SRombauts merged commit 8e2d4dc into SRombauts:master Nov 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants