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

C#: Encode GodotProjectDir as Base64 to prevent issues with special characters #74312

Merged

Conversation

raulsntos
Copy link
Member

@raulsntos raulsntos added this to the 4.1 milestone Mar 3, 2023
@raulsntos raulsntos requested a review from a team as a code owner March 3, 2023 17:19
@raulsntos raulsntos force-pushed the dotnet/godot-project-dir-base64 branch from 41affe6 to f949e94 Compare March 4, 2023 22:33
@akien-mga akien-mga merged commit afc9d38 into godotengine:master Mar 5, 2023
@akien-mga
Copy link
Member

Thanks!

@raulsntos raulsntos deleted the dotnet/godot-project-dir-base64 branch March 5, 2023 12:49
@geowarin
Copy link
Contributor

geowarin commented Mar 5, 2023

After this PR my build fails:

./modules/mono/build_scripts/build_assemblies.py --godot-output-dir=./bin \ --push-nupkgs-local ~/dev/LocalNugetSource

/home/geo/dev/godot/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Sample/Godot.SourceGenerators.Sample.csproj(10,5): 
error MSB4186: Invalid static method invocation syntax: "[MSBuild]::ConvertToBase64('$(GodotProjectDir)')". 
Method '[MSBuild]::ConvertToBase64' not found. Static method invocation should be of the form: $([FullTypeName]::Method()), e.g. $([System.IO.Path]::Combine(`a`, `b`)). 
Check that all parameters are defined, are of the correct type, and are specified in the right order.

dotnet --info:

.NET SDK (reflecting any global.json):
 Version:   6.0.114
 Commit:    346c0065dd

Runtime Environment:
 OS Name:     arch
 OS Version:  
 OS Platform: Linux
 RID:         arch-x64
 Base Path:   /usr/share/dotnet/sdk/6.0.114/

Host:
  Version:      7.0.3
  Architecture: x64
  Commit:       0a2bda10e8

.NET SDKs installed:
  6.0.114 [/usr/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.NETCore.App 6.0.14 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  DOTNET_ROOT       [/usr/share/dotnet]

global.json file:
  Not found

@raulsntos
Copy link
Member Author

raulsntos commented Mar 5, 2023

Your version of MSBuild is likely outdated, what is the output of dotnet msbuild --version? I think the ConvertToBase64 method was added in 17.3.0.

@geowarin
Copy link
Contributor

geowarin commented Mar 5, 2023

Indeed, I have a version < 17.3.0.

❯ dotnet msbuild --version
Microsoft (R) Build Engine version 17.0.1+b177f8fa7 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

17.0.1.11501%   

How to I update it? I think it's bundled by my distro.

@raulsntos
Copy link
Member Author

raulsntos commented Mar 5, 2023

MSBuild is part of the .NET SDK, can you update it? You have 6.0.114 installed, but 6.0.406 should be available.

@geowarin
Copy link
Contributor

geowarin commented Mar 5, 2023

I've flag the arch packages as out-of-date.

@neikeq
Copy link
Contributor

neikeq commented Mar 6, 2023

I think we should look for an alternative that doesn't require a newer MSBuild. Otherwise, we can't include it in 4.0.x as it's a breaking change. Is there a way in MSBuild to check if a method is available?

@geowarin
Copy link
Contributor

geowarin commented Mar 6, 2023

I agree with @neikeq project builds are going to fail with this kind of errors:

image

Which is not user friendly at all.
And is a breaking change.

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.0.2 (alongside #74479).

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