gui-cs / Terminal.Gui

Cross Platform Terminal UI toolkit for .NET
MIT License
9.54k stars 680 forks source link

Localization not working on self-contained single-file. #3611

Closed BDisp closed 1 month ago

BDisp commented 1 month ago

The reason is the resx files are embedded on the executable file and thus there is no satellite assemblies for each culture in the folder. The solution is to return a list of cultures that we know are supported. However, if the project contains the <InvariantGlobalization>true</InvariantGlobalization> setting only the invariant culture is supported. For other cultures to be supported, it is necessary not to use this configuration or use it as <InvariantGlobalization>false</InvariantGlobalization>.

dodexahedron commented 1 month ago

You mean that a consuming application that publishes as single-file is having that trouble, right?

BDisp commented 1 month ago

You mean that a consuming application that publishes as single-file is having that trouble, right?

Yes.

BDisp commented 1 month ago

You mean that a consuming application that publishes as single-file is having that trouble, right?

There was also a bug for apps not publishing as single-file where the AppContext.BaseDirectory was not executing as it should. Previously, assembly.Location was used but a warning always appeared that it was not supported in trimmed. So I now chose to use assembly.GetName ().Name. Previously, this bug was not detected because SupportedCultures was only asserted with the GetSupportedCultures method, which always returns the same value regardless of whether it was incorrect. Therefore, I added the Assert.Equal (4, Application.SupportedCultures.Count) instruction to the unit test, whose value must be manually updated whenever a new resource file is added. I would prefer it to be automated but it is not possible. Only in applications that are published as non-single-file can it be automated because it is searched by the satellite assembly directories.

dodexahedron commented 1 month ago

I would suggest actually doing a full AoT, not just single-file, build for this csproj. The reason is that AoT brings in even tighter restrictions which would be a good idea for us to comply with.

Thankfully, it's better with .net8 than it was even with .net7.

For debug symbols, you'll want to add, in a PropertyGroup: <StripSymbols>false</StripSymbols>, too.

For some reference material, while you're working on that:

dodexahedron commented 1 month ago

In fact, thinking about it...

We should probably add another unit test project, at some point, specifically for testing AoT, to protect against us doing anything in the future that breaks people's AoT builds.

BDisp commented 1 month ago

I already tried to add an unit test project but we only can create a Process but isn't possible to make the test on what matter.

dodexahedron commented 1 month ago

Yeah, that'll likely require a bit more work. It can be addressed later, anyway, especially since single-file has to be working perfectly before AoT is even possible.

dodexahedron commented 1 month ago

Also, you may find this document useful for this issue: https://github.com/dotnet/runtime/blob/main/docs/tools/illink/data-formats.md#descriptor-format

dodexahedron commented 1 month ago

We also very likely will need to use a lot of [DynamicallyAccessedMembers] and related attributes on any non-sealed types, especially the various View types, so that derived types in AoT builds don't break and other related possibilities.

Alternatively, the descriptor and substitution XML files are likely to be less work and more durable, but just need a bit more careful design.

dodexahedron commented 1 month ago

Another thought:

Rather than having a separate SingleFile csproj, how about just adding a build configuration to the sample projects that does single-file and AoT? Then you'll get confirmation that things done in them work, for free.

dodexahedron commented 1 month ago

Also note that bool is not blittable, which has consequences for PInvoke and AoT.

Any types that contain bool fields are not blittable and may need appropriate marshalling attributes applied to ensure correct and consistent behavior for all build scenarios.

BDisp commented 1 month ago

Another thought:

Rather than having a separate SingleFile csproj, how about just adding a build configuration to the sample projects that does single-file and AoT? Then you'll get confirmation that things done in them work, for free.

The others projects are examples for simple implementation. The SelfContained project is for test and ensuring that AoT and single-file are working properly. So, @tig is right when he said that examples must be the simplest possible. The SelfContained is really for testing stuff.

dodexahedron commented 1 month ago

That's not relevant to what I'm suggesting.

The samples themselves don't need to change. What I'm suggesting is adding another build configuration, so that you don't have to do the work to verify that all of the things that the samples already cover again, in another project. And you'd need to do exactly that, otherwise, because simply being able to compile and run a barebones app does not prove that the library is compatible. It just means that the code covered in that app doesn't fail as written.

The nature of self-contained is a bit tricky, because it's still subject to JIT compilation, by the same rules as any other application, but unreferenced code that the trimmer removed won't be noticed until you call it, because no attempt to compile it to a binary has happened yet. Roslyn compilation generated a .net assembly, the IL linker walked the IL code graph and aggressively removed anything it couldn't guarantee was in use, and then it got packaged into a zip archive (single-file is still separate files, just zipped together and extracted at runtime, which is why they start up slower - especially on first launch).

But use of the existing unit test project isn't sufficient, because it touches so much stuff all at once that not much of TG itself is going to get trimmed. Only stuff where coverage metrics show 0% might get trimmed.

Although, a thought that just occurred to me is that we might be able to use build configurations on that to also target the trim scenarios. Here's what I'm thinking right now for that:

Basically, we would use conditional inclusion/exclusion of portions of the test project code, for each of several new build configurations. This would ensure we're allowing a different portion of the TG code to become trimmable for each different configuration, by not being referenced.

For running the tests, all you have to do is specify the configuration, which is also trivial on the github runners, using a matrix, and would happen in parallel.

Not directly associated with the above, we can make this all a lot easier on ourselves by moving some namespaces around, since you can control trimming at the namespace level as well. For example, the root namespace's members as well as various base types like ConsoleDriver, View, and configuration-related stuff would be the majority of what we need to explicitly mark as not trimmable, leaving the rest up to the consumer's application code to dictate at compile-time.

Rooting or otherwise exempting the entire assembly isn't a viable solution, as that's basically just ignoring the problem and removing one of the value propositions of trimming in the first place, which is size reduction on disk and in memory of the resulting assembly, whether it be single-file, R2R, AoT, or just explicitly trimmed without specifically using any of those features. Common examples of that last one are containerized applications or small embedded devices (think Raspberry Pi for example), both of which typically aim to be or need to be as lean as possible).

While we can't reasonably cover all scenarios, testing each view type, in isolation, with trimming in its various forms involved, is probably sufficient for the vast majority of possibilities.

dodexahedron commented 1 month ago

What that all sums up to is that claiming trim compatibility really can't be done without one or more of the following, none of which actually are what trim-compatibility implies, except maybe for the last one:

Unless one or more of the above concessions (or some others, since that's certainly not an exhaustive list) are made and very loudly documented, the goal of making TG trim-compatible or anything that depends on trim-compatibility isn't attainable to a degree that we should advertise/list/claim that TG is, in fact, trim-compatible, as a general statement.

But if we do some (hopefully not terribly onerous) work to provide at least some hard proof to support more specific claims, such as any of the previously-mentioned ways of testing it out or any others that achieve similar results, we could legitimately make at least the standard claim that we've tested what you see in the tests and the library supports what you see in the test results of CI builds.

Trimming is just too complex to claim support with anything less and, if we exempt the whole assembly, we would need to explicitly say that is what's being done, and that run-time behavior is unknown and potentially unstable.

dodexahedron commented 1 month ago

If it sounds like too much, the simplest thing to do, at the end of the day, is to just not say anything at all about trimming. Though a CYA for people filing issues about it could be as simple as something to the effect of "we don't know," or something a wee bit more verbose like:

While some incompatibilities with assembly trimming using the .net 8.0 SDK have been addressed to varying degrees, trimming of the Terminal.Gui assembly or its transitive dependencies is untested and not guaranteed to work to any extent, for any component, unless explicitly noted and documented otherwise in that component.

Or we just don't say anything at all about trimming and fall back on the MIT license clause that no guarantee of fitness for any purpose is made.

BDisp commented 1 month ago

I don't know how you intend to implement these build configurations. If you can do this and keep the examples as simple as possible, so much the better. I'm just trying hard to make trim work with both sigle-file and Aot. By the way, what does R2R mean? For now I know that it is not possible to have single-file and Aot configurations simultaneously. That's why I created a new project "NativeAot" to test this configuration. For now, single file is working with win-x64 platform. On other platforms it gives an error but I can't debug it due to the error that occurs when trying to attach it to the debugger, as you already know and mentioned in the README. Typically, I use the following code to attach the application to the debugger before the Application.Init call:

 while (!Debugger.IsAttached)
 {
 Thread.Sleep (500);
 }

 Debugger.Break();

Using the SelfContained project: With the win-x64 platform it works well and I can connect to the debugger. With the linux-x64 platform, it gives an error and I can't attach it to the debugger, because it gives the error "Unable to start debugging. Failed to attach to process: Unknown Error: 0x80131c3c"

Using the NativeAot project: Both with the win-x64 or linux-x64 platforms they don't work well, with the assertions failing and commenting them displays the screen in black and white and I can't attach them to the debugger.

dodexahedron commented 1 month ago

It's done with a single ItemGroup with the first element excluding all view tests, which looks like <Compile Remove="ViewTests/*.cs" /> (or whatever path the tests live under), and then one for each view type test file with a condition for the configuration (or even just a build variable - doesn't even need to be a whole configuration). Those look like <Compile Include="ViewTests/TableView.cs" Condition=" '$(ViewTypeUnderTest)' == 'TableView' " />.[^1]

To run them, when you call dotnet test, you just do this: dotnet test SingleFile.csproj -e ViewTypeUnderTest=TableView.

That then just gets parameterized for the CI runs so they still all happen at once. In github, that's just a matrix, or it can even be done entirely in the msbuild files or compiler response files. Super easy and many ways to achieve it.

As part of the build improvement issue I'm working on, I'm actually doing similar things to some of the conditionally compiled code we have to clean things up and make it easier to invoke at will.

[^1]: I'd actually probably have two ItemGroups - one used in visual studio, so all files are included, and the other which would only be selected when specifying one of those scenario build variables.

dodexahedron commented 1 month ago

Actually... It can be even simpler than that...

A single inclusion after the exclude, and it would look similar to <Compile Include="ViewTests/$(ViewTestCodeFile)" />, and you'd just give the file name to the command line, so you don't even need to have a separate include for every test file.

dodexahedron commented 1 month ago

MSBuild is extremely flexible and powerful. Consequently, it may initially be daunting to approach. But, once you get the hang of a few simple common tasks like this, suddenly you turn into SuperBuilder. 😅