Skip to content

Conversation

@BDisp
Copy link
Collaborator

@BDisp BDisp commented Oct 9, 2024

Fixes

Proposed Changes/Todos

  • Provide local packages in release mode

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

@BDisp BDisp requested a review from tig as a code owner October 9, 2024 15:13
@BDisp
Copy link
Collaborator Author

BDisp commented Oct 9, 2024

@tig I already fixed this issue locally but not yet in the github actions. Can you help me on this please. I don't have experience on this.

@tznind
Copy link
Collaborator

tznind commented Oct 10, 2024

Looks like it is currently failing on the windows style local packages?

/home/runner/work/Terminal.Gui/Terminal.Gui/NativeAot/NativeAot.csproj : error NU1301: The local source '/home/runner/work/Terminal.Gui/Terminal.Gui/Terminal.Gui/bin/Release' doesn't exist. [/home/runner/work/Terminal.Gui/Terminal.Gui/Terminal.sln]

If this source should only be used locally when checked out? you can remove it in the CI with:

dotnet nuget remove source LocalPackages

What is the goal of this PR?

To build a nuget package of Terminal.Gui in Release configuration to a directory and have the other two projects reference it?

@BDisp
Copy link
Collaborator Author

BDisp commented Oct 10, 2024

What is the goal of this PR?

To build a nuget package of Terminal.Gui in Release configuration to a directory and have the other two projects reference it?

Yes, only for the two projects that use the nuget package. It's to prevent, when the latest nuget version has significant changes, like now, to avoid the fail of these two projects at compile time, locally and in the github CI.

@tznind
Copy link
Collaborator

tznind commented Oct 10, 2024

Was this already merged in another PR? it looks quite broken...

I would:

Firstly move the nuget.config file to the subdirectories where its needed i.e. into the NativeAot and SelfContained directories (cut paste). This will prevent everyone trying to use this source even when it doesnt exist yet.

You may need to change the paths e.g. to ../Terminal.Gui/local_packages

Pack only Terminal.Gui:

cd Terminal.Gui && dotnet build --configuration Release
cd Terminal.Gui && dotnet pack --configuration Release --output ./local_packages

Remove the reference in the csproj files and add a new one to the Terminal.Gui in the local packages dir

cd SelfContained
dotnet remove package Terminal.Gui
dotnet add package Terminal.Gui --source LocalPackages

Or since at the moment its a project reference you might need

dotnet remove reference ..\Terminal.Gui\Terminal.Gui.csproj

@BDisp
Copy link
Collaborator Author

BDisp commented Oct 10, 2024

The main problem is that the nuget restore is always run first. See NuGet/Home#6746, it seems that it's not possible to use nuget.config per project which respect his configuration, only the one of the solution will be used.

@tznind
Copy link
Collaborator

tznind commented Oct 10, 2024

The closest one to the code is used I think so if you move it to subdirectory it will be used. Basically when you are cd into the directory and building it will use the one that is in the current directory otherwise it will look at parent directory.

@BDisp
Copy link
Collaborator Author

BDisp commented Oct 10, 2024

The closest one to the code is used I think so if you move it to subdirectory it will be used. Basically when you are cd into the directory and building it will use the one that is in the current directory otherwise it will look at parent directory.

Yes I know, but normally dotnet build --configuration Release is done in the solution location and is the nuget.confile in that folder that will be used, I think. Feel free to submit your own PR that could fix this locally and in the CI and I will close, mine. In mean wile I'll continue to discover any solution that can solve this. Thanks.

Note: I did a cherry-pick of this commit d77b899 in this PR.

@tznind
Copy link
Collaborator

tznind commented Oct 10, 2024

@BDisp
Copy link
Collaborator Author

BDisp commented Oct 10, 2024

Here is a first pass

https://github.com/BDisp/Terminal.Gui/pull/200/files

I can't merge it. When you have sure it work mark as ready for review, please. Thanks.

@BDisp
Copy link
Collaborator Author

BDisp commented Oct 10, 2024

I think I found a simpler solution for this. We cannot avoid the dotnet build running immediately the package restore, avoiding the pre-build take the necessary tasks for build and pack the Terminal.Gui in release and thus, NativeAot and SelfContained projects restoring the packages with available package sources. The solution I found was creating a .ps1 file for Windows and .sh file for Linux and Mac. When we need to build a release version then we use that files for build and pack and then restore and build NativeAot and SelfContained projects.

@tig
Copy link
Collaborator

tig commented Oct 10, 2024

Is this ready or not?

@BDisp
Copy link
Collaborator Author

BDisp commented Oct 10, 2024

Is this ready or not?

Yes it's.

@BDisp
Copy link
Collaborator Author

BDisp commented Oct 10, 2024

If you don't mind it will useful to merge this to avoid all the CI errors. Thanks.

@tig tig merged commit 9c6a305 into gui-cs:v2_develop Oct 11, 2024
4 checks passed
@tig
Copy link
Collaborator

tig commented Oct 11, 2024

@BDisp Release build is still failing

image

@BDisp BDisp deleted the v2_3784_selfcontained_nativeaot-local-release-package branch October 11, 2024 13:34
@BDisp
Copy link
Collaborator Author

BDisp commented Oct 11, 2024

@BDisp Release build is still failing

Solution here #3790 (comment).

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.

SelfContained and NativeAot projects should use the local package in the release mode.

3 participants