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

CliWrap only outputs foreign characters in Mac Catalyst app #261

Closed
4 of 5 tasks
beeradmoore opened this issue Oct 17, 2024 · 6 comments · Fixed by #262
Closed
4 of 5 tasks

CliWrap only outputs foreign characters in Mac Catalyst app #261

beeradmoore opened this issue Oct 17, 2024 · 6 comments · Fixed by #262
Labels

Comments

@beeradmoore
Copy link

beeradmoore commented Oct 17, 2024

Version

v3.6.6

Platform

.NET 8, macOS 15.0.1, M3 MacBook Pro

Steps to reproduce

  • Create MacCatalyst app (dotnet new maccatalyst --name MyApp)
  • Add CliWrap nuget
  • Attempt to use CliWrap

Details

Reproduction of the issue is in my CliWrapMacCatalystTestApp repo. This is a new app, but the main bits to see are,

var process = new Process();
process.StartInfo.UseShellExecute = false;
process.StartInfo.RedirectStandardOutput = true;
process.StartInfo.FileName = "date";
process.Start();
string output = process.StandardOutput.ReadToEnd();
process.WaitForExit();

Debug.WriteLine("## System.Diagnostics.Process ##");
Debug.WriteLine(output);

Task.Run(async () =>
{
	var result = await Cli.Wrap("date")
		.ExecuteBufferedAsync();

	Debug.WriteLine("## StandardOutput ##");
	Debug.WriteLine(result.StandardOutput);
	Debug.WriteLine("## StandardError ##");
	Debug.WriteLine(result.StandardError);
	Debug.WriteLine("## ##");
});

The app that brought me here was an AvaloniaUI app I am porting to .NET MAUI. I first thought this was a MAUI issue as it did not happen previously. But then I replicated it in a Mac Catalyst standalone app. I then thought it was a .NET macOS issue, but then I could not replicate it in a MacOS App (non-MacCatalyst .NET mac app). Then I found that in Mac Catalyst I could use System.Diagnostics.Process fine. So I now assume it is an issue with this library (or the library is not using System.Diagnostics.Process under the hood).

Additionally CliWrap does not have this issue in a .NET console app on the same machine. It seems specific to only Mac Catalyst apps, even if I disable the sandbox.

From the sample above, the expected output is the current date.

The actual output is the result data is foreign characters. I assume some sort of encoding issue.

## System.Diagnostics.Process ##
Fri Oct 18 08:49:37 AEDT 2024
## StandardOutput ##
牆⁩捏⁴㠱〠㨸㤴㌺‷䕁呄㈠㈰਴
## StandardError ##

## ##

I am not able to test on an Intel based mac at this time.

Checklist

  • I have looked through existing issues to make sure that this bug has not been reported before
  • I have provided a descriptive title for this issue
  • I have made sure that this bug is reproducible on the latest version of the package
  • I have provided all the information needed to reproduce this bug as efficiently as possible
  • I have sponsored this project
@Tyrrrz
Copy link
Owner

Tyrrrz commented Oct 18, 2024

Does anything change if you explicitly set output encoding to UTF8?

@beeradmoore
Copy link
Author

beeradmoore commented Oct 19, 2024

Thanks for the quick response @Tyrrrz.

Changing the encoding to UTF8 with .ExecuteBufferedAsync(Encoding.UTF8); indeed fixes the issue.

I thought checking what Encoding.Default would show what is wrong here but that shows UTF8Encoding.UTF8EncodingSealed. This does not pass the check of if (Encoding.UTF8.Equals(Encoding.Default)) but my system also tells me Encoding.UTF8 is UTF8Encoding.UTF8EncodingSealed 🤷‍♂️

Using .ExecuteBufferedAsync(Encoding.Default); also works.

Looking at the source, appears that the default is Console.OutputEncoding which for me (debug builds in JetBrains Rider) is UnicodeEncoding. Using .ExecuteBufferedAsync(Console.OutputEncoding); produces the broken output as expected.

EDIT: Leaving this here as well incase others come across it in search. Using the non-buffered output

var stdOutBuffer = new StringBuilder();
var stdErrBuffer = new StringBuilder();

var result2 = await Cli.Wrap("date")
	.WithStandardOutputPipe(PipeTarget.ToStringBuilder(stdOutBuffer))
	.WithStandardErrorPipe(PipeTarget.ToStringBuilder(stdErrBuffer))
	.ExecuteAsync();
var stdOut = stdOutBuffer.ToString();
var stdErr = stdErrBuffer.ToString();

Debug.WriteLine("## (non-buffered) StandardOutput ##");
Debug.WriteLine(stdOut);
Debug.WriteLine("## (non-buffered) StandardError ##");
Debug.WriteLine(stdErr);

will also produce the error,

## (non buffered) StandardOutput ##
慓⁴捏⁴㤱ㄠ㨲㜰ㄺ‵䕁呄㈠㈰਴
## (non buffered) StandardError ##

But using encoding in the pipe target (eg. PipeTarget.ToStringBuilder(stdOutBuffer, Encoding.UTF8)) resolves the issue as expected.

I am unsure if this is something that is considered broken, or something that can be fixed, or something that should be mentioned in the readme. But the solution is very simple, and hopefully if anyone else searches for it they also find this.

I couldn't see any way to change it at the Mac Catalyst level.

Happy to have this issue closed if you are happy to have it closed.

@Tyrrrz
Copy link
Owner

Tyrrrz commented Oct 20, 2024

This is interesting, as it appears that the default encoding is resolved incorrectly. If System.Diagnostics.Process works correctly, then I would consider it a bug in CliWrap. I'm just wondering what they're using to resolve the default encoding there, if it's not Console.OutputEncoding 🤔

Update: it looks like Encoding.Default is actually the correct default to use in this case, instead of Console.OutputEncoding.

@beeradmoore
Copy link
Author

I tried to dig through dotnet runtime repo and xamarin-macios repo to see if I could find info as to why Mac Catalyst is doing what it was doing.

If you don't mind sharing, what did you find out about Encoding.Default as being the desired default?

@Tyrrrz
Copy link
Owner

Tyrrrz commented Oct 22, 2024

I've looked at the source code for the Process class here:

https://source.dot.net/#System.Diagnostics.Process/System/Diagnostics/Process.Unix.cs,462

image

Actually, Console.OutputEncoding might still be valid on Windows, just not on Unix.

@beeradmoore
Copy link
Author

Ah, thanks! At least I was in the right repo looking for it, just looking for the wrong things.

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 a pull request may close this issue.

2 participants