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

Fixes #3574. Enabling trim mode breaks program at runtime (v2 only) #3582

Merged
merged 32 commits into from
Jul 12, 2024

Conversation

BDisp
Copy link
Collaborator

@BDisp BDisp commented Jul 4, 2024

Fixes

Proposed Changes/Todos

  • Add SourceGenerationContext

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

README:

Terminal.Gui C# SelfContained

This example shows how to use the Terminal.Gui library to create a simple self-contained single file GUI application in C#.

With Debug the .csproj is used and with Release the latest nuget package is used, either in Solution Configurations or in Profile Publish.

To publish the self-contained single file in Debug or Release mode, it is not necessary to select it in the Solution Configurations, just choose the Debug or Release configuration in the Publish Profile.

When executing the file directly from the self-contained single file and needing to debug it, it will be necessary to attach it to the debugger, just like any other standalone application. However, when trying to attach the file running on Linux or macOS to the debugger, it will issue the error "Failed to attach to process: Unknown Error: 0x80131c3c". This issue has already been reported on Developer Community. Maybe it would be a good idea to vote in favor of this fix because I think Visual Studio for macOS is going to be discontinued and we need this fix to remotely attach a process running on Linux or macOS to Windows 11.

@BDisp BDisp requested a review from tig as a code owner July 4, 2024 22:28
Copy link
Collaborator

@tig tig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand most of these changes, but I'm super excited you figured it out!

Nice work!!!

@tig
Copy link
Collaborator

tig commented Jul 5, 2024

I think @dodexahedron should review this before we merge.

@tig tig requested a review from dodexahedron July 6, 2024 22:21
@tig
Copy link
Collaborator

tig commented Jul 7, 2024

@BDisp would it be possible (needed?) to fold the work @tznind did on

into this PR?

@tig
Copy link
Collaborator

tig commented Jul 7, 2024

FYI - tests are failing because of work i'm doing on trying to fix #3052. JFYI

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 7, 2024

@tig, can you test the Analyzers unit tests on Linux to see if they are failing assertions due to the newline format, please? Thanks.

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 7, 2024

@tig, can you test the Analyzers unit tests on Linux to see if they are failing assertions due to the newline format, please? Thanks.

But in the VS2022, please.

image

@tig
Copy link
Collaborator

tig commented Jul 7, 2024

@tig, can you test the Analyzers unit tests on Linux to see if they are failing assertions due to the newline format, please? Thanks.

But in the VS2022, please.

image

Yep.

image

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 8, 2024

The reason unit tests from the Analyzers project are not failing to execute actions is because the repository files are automatically written with the LF format on new lines when it is pushed to the respective operating system.
In Visual Studio 2022 it is using WSL to run unit tests but using the Windows 11 directory (/mnt/{drive}/{repo}) whose newline format is CRLF. Ideally, Visual Studio 2022 would first push the code to WSL, thus transforming the format of new lines as LF, and then run the unit tests. I wonder if this is possible.

@Mersid
Copy link

Mersid commented Jul 9, 2024

This dude right here, absolute mad lad 😎

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 10, 2024

@Mersid did you already test this PR? Say if it's working with you. Thanks.

@tig
Copy link
Collaborator

tig commented Jul 11, 2024

@dodexahedron - Can we please get your review of this?

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 11, 2024

It's failing again because the JETBRAINS_ANNOTATIONS.

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 11, 2024

I have to remove JETBRAINS_ANNOTATIONS from the Terminal.Gui project to avoid this error.

╰─ .\SelfContained.exe
Unhandled exception. System.IO.FileNotFoundException:
File name: 'JetBrains.Annotations, Version=4242.42.42.42, Culture=neutral, PublicKeyToken=1010a0d8d6380325'
   at System.ModuleHandle.ResolveTypeHandle(Int32, RuntimeTypeHandle[], RuntimeTypeHandle[])
   at System.Reflection.RuntimeModule.ResolveType(Int32, Type[], Type[])
   at System.Reflection.CustomAttribute.FilterCustomAttributeRecord(MetadataToken, MetadataImport& , RuntimeModule, MetadataToken, RuntimeType, Boolean, ListBuilder`1&, RuntimeType& , IRuntimeMethodInfo& , Boolean& )
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1&, RuntimeModule, Int32, RuntimeType, Boolean, ListBuilder`1)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule, Int32, Int32, RuntimeType)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimePropertyInfo, RuntimeType)
   at System.Reflection.RuntimePropertyInfo.GetCustomAttributes(Type, Boolean)
   at System.Attribute.InternalGetCustomAttributes(PropertyInfo, Type, Boolean)
   at System.Attribute.GetCustomAttributes(MemberInfo, Type, Boolean)
   at System.Attribute.GetCustomAttribute(MemberInfo, Type, Boolean)
   at System.Attribute.GetCustomAttribute(MemberInfo, Type)
   at System.Reflection.CustomAttributeExtensions.GetCustomAttribute(MemberInfo, Type)
   at Terminal.Gui.ConfigurationManager.<>c.<Initialize>b__48_8(PropertyInfo prop) in E:\Repos\CSharp\gui-cs\Terminal.Gui\Terminal.Gui\Configuration\ConfigurationManager.cs:line 526
   at System.Linq.Enumerable.Any[TSource](IEnumerable`1, Func`2)
   at Terminal.Gui.ConfigurationManager.<>c.<Initialize>b__48_2(<>f__AnonymousType1`2 <>h__TransparentIdentifier0) in E:\Repos\CSharp\gui-cs\Terminal.Gui\Terminal.Gui\Configuration\ConfigurationManager.cs:line 524
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at Terminal.Gui.ConfigurationManager.Initialize() in E:\Repos\CSharp\gui-cs\Terminal.Gui\Terminal.Gui\Configuration\ConfigurationManager.cs:line 533
   at Terminal.Gui.ConfigurationManager.Reset() in E:\Repos\CSharp\gui-cs\Terminal.Gui\Terminal.Gui\Configuration\ConfigurationManager.cs:line 342
   at Terminal.Gui.ConfigurationManager.get_Settings() in E:\Repos\CSharp\gui-cs\Terminal.Gui\Terminal.Gui\Configuration\ConfigurationManager.cs:line 163
   at Terminal.Gui.ConfigurationManager.Load(Boolean reset) in E:\Repos\CSharp\gui-cs\Terminal.Gui\Terminal.Gui\Configuration\ConfigurationManager.cs:line 269
   at Terminal.Gui.Application.InternalInit(ConsoleDriver driver, String driverName, Boolean calledViaRunT) in E:\Repos\CSharp\gui-cs\Terminal.Gui\Terminal.Gui\Application\Application.cs:line 248
   at Terminal.Gui.Application.Run[T](Func`2 errorHandler, ConsoleDriver driver) in E:\Repos\CSharp\gui-cs\Terminal.Gui\Terminal.Gui\Application\Application.cs:line 702
   at SelfContained.Program.Main(String[]) in E:\Repos\CSharp\gui-cs\Terminal.Gui\SelfContained\Program.cs:line 13

@dodexahedron
Copy link
Collaborator

It's failing again because the JETBRAINS_ANNOTATIONS.

Yep. I mentioned that somewhere. All that constant does is exactly that - sets the condition that makes them transitive.

It's fine to remove it, especially since I would have ended up taking it out in #3558 anyway.

I thought I did already take it out in one of the PRs that just merged though?

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 12, 2024

I thought I did already take it out in one of the PRs that just merged though?

See this comment https://github.com/gui-cs/Terminal.Gui/pull/3600/files#r1675696417 please.

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 12, 2024

I removed the Terminal.Gui.Analyzers.Internal project from the Terminal.Gui project, deleted the Directory.Build.* and added the JetBrains.Annotations PackageReference to the UICatalog and UnitTests projects. Here the commit abdde3a.

@tig
Copy link
Collaborator

tig commented Jul 12, 2024

@BDisp Ready for me to merge this now?

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 12, 2024

@BDisp Ready for me to merge this now?

Yes.

@tig tig merged commit f1fe7e7 into gui-cs:v2_develop Jul 12, 2024
3 checks passed
@BDisp
Copy link
Collaborator Author

BDisp commented Jul 12, 2024

I could also suggest publishing an update in the nuget and then testing in the Release version with the new version, if possible. Thanks.

@BDisp BDisp deleted the v2_3574_self-contained-single-file branch July 12, 2024 17:47
@dodexahedron
Copy link
Collaborator

I removed the Terminal.Gui.Analyzers.Internal project from the Terminal.Gui project, deleted the Directory.Build.* and added the JetBrains.Annotations PackageReference to the UICatalog and UnitTests projects. Here the commit abdde3a.

A rebase on v2_develop after the merges would have taken care of that too.

@dodexahedron
Copy link
Collaborator

I could also suggest publishing an update in the nuget and then testing in the Release version with the new version, if possible. Thanks.

@tig @BDisp @tznind

For anyone wanting to test nuget packaging:

You can dotnet pack locally and just add a local nuget source so you can install the package via standard nuget workflows. Then, multiple projects can make use of it and it doesn't disappear on you. It's the way you test before deploying/publishing to nuget.org or another public repository.

To set up a local feed, just make two directories in a common path outside of any git repos (this only needs to be done once):

  • One for the output of dotnet pack -o path/to/pack/output/directory for any projects you build packages from.
  • One to be your local nuget repository feed (path/to/feed/directory).

To update your feed with your latest builds, run nuget init path/to/pack/output/directory path/to/feed/directory

To use the feed, just add that feed as a nuget source.

Note that our current nuget.config in the repository root will nuke your custom feed because of the clear element, so either remove that line or just add your feed in that file instead.

Sourcelink warnings or errors are resolved by adding a mapping for your feed.

I prefer to do it by just explicitly specifying a custom nuget.config file on the command line when testing the nuget restore/dotnet restore, so it only uses the local feed when I explicitly want it to and doesn't affect anything otherwise.

@dodexahedron
Copy link
Collaborator

dodexahedron commented Jul 12, 2024

A similar but simpler procedure can also be used to set up a local powershell gallery, for ease of installing/importing powershell modules without needing to navigate to the containing directory.

I have several such custom local psrepositories on all of my dev machines, as well as PSDrives for solution directories as PSDrives that are created when powershell launches, as part of my powershell profile. That's actually also a big part of how the github actions I'm working on work, under the hood, to provide the illusion of multiple job steps in a github workflow being a single session, even though they aren't.

A really nice benefit of the PSDrive thing is it enables you to use identical paths for things, regardless of OS platform.

For example, I have one for Terminal.Gui defined like this in my powershell profile:

New-PSDrive -Name TGRoot -PSProvider FileSystem -Scope Global -Root (Resolve-Path V:\repos\Terminal.Gui)

On my linux environments, the root path part of that points to ~/dev/repos/Terminal.Gui.

That allows me to always just use TGRoot: as the root path for anything in a powershell session that has anything to do with Terminal.Gui, and I never have to touch it again since it's automatic at pwsh launch.

That is also how I manage to trick the github cache into using the same cache across macos, linux, and windows, even though you normally can't do that.

@dodexahedron
Copy link
Collaborator

dodexahedron commented Jul 13, 2024

Hm... You know what...

Yeah, a LOT of stuff reverted after that, including stuff from other PRs.

There's a strong possibility of regressions beyond just the project references and that PR.

I'm in the middle of working on stuff though, or I'd walk back through the history to see what else might have gotten clobbered. I did notice the solution filters are also gone, though, and that the Directory.Build.* files came back, which also shouldn't have happened.

@dodexahedron
Copy link
Collaborator

About the root cause of that sort of thing:

Almost definitely happened during merge conflict resolution on a branch.

Be extra careful to analyze conflicts that are not just edits on each side, and are instead add/edit/remove on one side and a different operation on the other side.

Those merge errors are a lot easier to catch with proper rebasing/merging procedures.

@dodexahedron
Copy link
Collaborator

Oh yeah.

There's also https://int.nugettest.org/ for when you want to test it over the internet but not live on nuget.org.

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 13, 2024

To testing nuget packages locally I use the following:

In the nuget.config:

<add key="Terminal.Gui" value="{Drive}:\{Path}\Terminal.Gui\Terminal.Gui\bin\Release" />

I created a Package.ps1 file with:

$Version = "2.0.0"

# Delete previous package
rm -r $env:USERPROFILE\.nuget\packages\terminal.gui\$Version

# Install dependencies
dotnet restore

# Build Release
dotnet build --no-restore -c Release

# Pack
dotnet pack -c Release --include-symbols -p:Version=$Version

@dodexahedron
Copy link
Collaborator

Yep. That's the way. 🙂 At least to pack it. Still need to test the install/restore and build in both visual studio and dotnet cli afterward.

Side note:

Another option for anyone who doesn't want to bother with the nuget.config is you can call dotnet restore --source path/to/directory/containing/the/package --force --force-evaluate to restore from just that path, ignoring other sources. You can specify --source multiple times in the command, if you want to also have it consider other sources, too.

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

Successfully merging this pull request may close these issues.

Enabling trim mode breaks program at runtime (v2 only)
4 participants