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

Is UnityYAMLMerge actually being used? #943

Closed
Piotr-B opened this issue Oct 17, 2018 · 3 comments
Closed

Is UnityYAMLMerge actually being used? #943

Piotr-B opened this issue Oct 17, 2018 · 3 comments
Labels
bug

Comments

@Piotr-B
Copy link

@Piotr-B Piotr-B commented Oct 17, 2018

Description

While investigating using this plugin for our next project I noticed that the UnityYAMLMerge tool seems to get configured for use but never actually gets launched.

When creating a project with the plugin the following configuration options get set:

.gitattributes:

# Unity files
*.meta -text merge=unityamlmerge diff
*.unity -text merge=unityamlmerge diff
*.asset -text merge=unityamlmerge diff
*.prefab -text merge=unityamlmerge diff

.git/config:

[merge "unityyamlmerge"]
	cmd = 'C:\\Program Files\\Unity 2018.1.1f1\\Editor\\Data\\Tools\\UnityYAMLMerge.exe' merge -p $BASE $REMOTE $LOCAL $MERGED
	trustExitCode = false

This looks well and good but:

  • Attributes file misspells the tool as "unityamlmerge" (note the missing "y").
  • Config file doesn't misspell the tool name.
  • Even if the names did match the tool still wouldn't run because the merge attribute looks for a custom low-level merge driver in the config file in the following format:
[merge "mergeDriverName"]
	name = Human-readable name
	driver = mergeDriverExecutable %O %A %B %L %P
	recursive = binary
  • The config file as is uses the custom merge driver header [merge "unityyamlmerge"] but external merge tool fields: cmd = trustExitCode =. These are different things but appear to be mixed here. A mergetool section is supposed to look like this:
[mergetool "extMerge"]
  cmd = extMerge "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
  trustExitCode = false

But for that to work it also requires this entry, which is missing:

[merge]
  tool = extMerge

And I think that would launch the tool for all file types not just the select ones.

Disclaimer:
I'm not very proficient in git so I might be missing something here but this is what it looks like to me after investigating the code and docs.

I'm using GitHub for Unity 1.1.0 and Unity 2018.1.1f1.

References

https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration#_external_merge_tools
https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver
https://git-scm.com/docs/git-config#git-config-mergetool
https://docs.unity3d.com/Manual/SmartMerge.html

# Unity files
*.meta -text merge=unityamlmerge diff
*.unity -text merge=unityamlmerge diff
*.asset -text merge=unityamlmerge diff
*.prefab -text merge=unityamlmerge diff

GitClient.SetConfig("merge.unityyamlmerge.cmd", yamlMergeCommand, GitConfigSource.Local).Catch(e => {
Logger.Error(e, "Error setting merge.unityyamlmerge.cmd");
return true;
}).RunSynchronously();
GitClient.SetConfig("merge.unityyamlmerge.trustExitCode", "false", GitConfigSource.Local).Catch(e => {
Logger.Error(e, "Error setting merge.unityyamlmerge.trustExitCode");
return true;
}).RunSynchronously();

@StanleyGoldman
Copy link
Member

@StanleyGoldman StanleyGoldman commented Oct 19, 2018

Hey @Piotr-B. Honestly, thanks for your analysis.
Obviously we intend for UnityYamlMerge to get configured correctly, but yea, we have some bugs.

I will try next week to investigate and resolve the issues your raised.
Maybe, if you have, you can help me review/test.

@StanleyGoldman StanleyGoldman added the bug label Oct 19, 2018
@fabiopolimeni
Copy link

@fabiopolimeni fabiopolimeni commented Nov 22, 2018

Hi guys, I would like to start using GitHub for Unity for our next project, and this bug sounds pretty important to me. At what point you honestly are with this?

Thanks,
Fabio

@StanleyGoldman
Copy link
Member

@StanleyGoldman StanleyGoldman commented Nov 26, 2018

Hey @fabiopolimeni thanks for checking our stuff out.
Sorry, we are a bit behind with things. Shana and I have a few different projects we maintain.
We are planning a release in December and this bug should be fixed in that release.

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

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.