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

"A Godot Engine build callback failed" #29232

Closed
seadra opened this issue May 27, 2019 · 7 comments · Fixed by #29290
Closed

"A Godot Engine build callback failed" #29232

seadra opened this issue May 27, 2019 · 7 comments · Fixed by #29290

Comments

@seadra
Copy link

seadra commented May 27, 2019

Godot version:
51d7026

OS/device including version:

Manjaro

Issue description:

Mono build fails using MSBuild with the message (mono 5.20):

MSBUILD : error MSB1005: Specify a property and its value.
Switch: /p:GodotDefineConstants=GODOT_X11;GODOT_64;

For switch syntax, type "MSBuild /help"
ERROR: call_build: A Godot Engine build callback failed.

I removed mono folders, but it's still happening.

Could be related to #28786.

Steps to reproduce:

Minimal reproduction project:

@LinuxUserGD
Copy link
Contributor

More related to #29210

@neikeq
Copy link
Contributor

neikeq commented May 28, 2019

For whatever reason ProcessStartInfo treats /p:GodotDefineConstants="GODOT_X11;GODOT_64;" as /p:GodotDefineConstants=GODOT_X11;GODOT_64;. Amazing. It looks like /p:GodotDefineConstants=\"GODOT_X11;GODOT_64;\" (\\\" in code) works on linux. I assume this bug is not reproducible on Windows, @ShyRed can you check if such a change works on Windows as well? I don't want to break it by accident.

@neikeq
Copy link
Contributor

neikeq commented May 28, 2019

or perhaps this is a thing with msbuild and not ProcessStartInfo ¯_(ツ)_/¯

@ShyRed
Copy link
Contributor

ShyRed commented May 28, 2019

Just tried /p:GodotDefineConstants=\"GODOT_WINDOWS;GODOT_64;\" and it fails on Windows :(

@neikeq
Copy link
Contributor

neikeq commented May 28, 2019

Did you try on the command line or in Godot like this?:

@@ -369,7 +369,7 @@ bool GodotSharpBuilds::build_project_blocking(const String &p_config, const Vect
	MonoBuildInfo build_info(GodotSharpDirs::get_project_sln_path(), p_config);

	// Add Godot defines
-	String constants = "GodotDefineConstants=\"";
+	String constants = "GodotDefineConstants=\\\"";

	for (int i = 0; i < p_godot_defines.size(); i++) {
		constants += "GODOT_" + p_godot_defines[i].to_upper().replace("-", "_").replace(" ", "_").replace(";", "_") + ";";
@@ -379,7 +379,7 @@ bool GodotSharpBuilds::build_project_blocking(const String &p_config, const Vect
	constants += "GODOT_REAL_T_IS_DOUBLE;";
#endif

-	constants += "\"";
+	constants += "\\\"";
	build_info.custom_props.push_back(constants);

	if (!GodotSharpBuilds::get_singleton()->build(build_info)) {

@ShyRed
Copy link
Contributor

ShyRed commented May 28, 2019

Tried in Godot.

Also Google is full of results when searching for ProcessStartInfo and quoting. Seems to be quite a mess and I wouldn't be surprised if the whole issue was OS dependent with Windows requiring simple quotes while Linux etc. wants escaped quotes.
Microsoft actually added a ProcessStartInfo.ArgumentList Property which might solve the problem in an OS independent way, but it's not available for .Net Framework yet...

@neikeq
Copy link
Contributor

neikeq commented May 28, 2019

Welp, this sucks but we can always add some ifdefs.

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

Successfully merging a pull request may close this issue.

5 participants