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.

Sign up for GitHub

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

Jump to bottom

DRAFT: paket move to FAKE-only #680

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from
Draft

DRAFT: paket move to FAKE-only #680

wants to merge 18 commits into from

Conversation

webwarrior-ws
Copy link
Contributor

Move from paket to Central Package Management.

@webwarrior-ws webwarrior-ws force-pushed the cpm-rebased-2 branch 4 times, most recently from 5f6bac1 to 9d66800 Compare January 15, 2024 11:42
@knocte
Copy link
Collaborator

knocte commented Jan 17, 2024

@webwarrior-ws can you try removing NU1605 now please?

@webwarrior-ws
Copy link
Contributor Author

@webwarrior-ws can you try removing NU1605 now please?

@knocte where is it?

@knocte
Copy link
Collaborator

knocte commented Jan 18, 2024

@knocte where is it?

FSharpLint/Directory.Build.props

Line 9 in 58dd6b6

<NoWarn>$(NoWarn);NU1605</NoWarn>

You were lucky that I was not afk now :) but please, next time, for obvious questions use grep ;)

@webwarrior-ws
Copy link
Contributor Author

Build fails with multiple errors if I remove NU1605 from <NoWarn>

@knocte
Copy link
Collaborator

knocte commented Jan 18, 2024

Build fails with multiple errors if I remove NU1605 from <NoWarn>

Aren't those easier to resolve now that there's no paket?

@knocte
Copy link
Collaborator

knocte commented Jan 18, 2024

If the last commit worked, then it shouldn't be marked as WIP anymore.

BTW when writing commit messages, if you are explaining 2 things that the commit does, please list them with bullets. Otherwise just listing each in a sentence, in the same paragraph, simply separated with a dot (with no way to join the sentences such as "hence", or something) feels kinda strange.

@webwarrior-ws webwarrior-ws force-pushed the cpm-rebased-2 branch 2 times, most recently from 680cd46 to 1b0c1e0 Compare January 18, 2024 14:10
@knocte
Copy link
Collaborator

knocte commented Jan 18, 2024

Good work wrt "build: re-enable NU1605" commit. Now, looking at it, it looks fairly simple. Are you sure we cannot separate into its own PR (aka: do it in master while it still paket-based)?

@webwarrior-ws
Copy link
Contributor Author

Good work wrt "build: re-enable NU1605" commit. Now, looking at it, it looks fairly simple. Are you sure we cannot separate into its own PR (aka: do it in master while it still paket-based)?

I tried, and it didn't work. Microsoft.Build.* versions are set by Visual Studio and cause package downgrades.

@knocte
Copy link
Collaborator

knocte commented Jan 19, 2024

I tried, and it didn't work. Microsoft.Build.* versions are set by Visual Studio and cause package downgrades.

I never mentioned VS here. The topic is paket vs CPM (at CI level).

@webwarrior-ws
Copy link
Contributor Author

I tried, and it didn't work. Microsoft.Build.* versions are set by Visual Studio and cause package downgrades.

I never mentioned VS here. The topic is paket vs CPM (at CI level).

NU1605 was disabled because build in Visual Studio failed. See 58dd6b6

@smoothdeveloper
Copy link
Contributor

Note about <NoWarn>$(NoWarn);...</NoWarn>, I think it is almost always better to use <WarningsNotAsErrors>$(WarningsNotAsErrors);...</WarningsNotAsErrors> instead.

@knocte
Copy link
Collaborator

knocte commented Jan 19, 2024

NU1605 was disabled because build in Visual Studio failed.

If you remove that NoWarn in master, then CI doesn't build. So we're talking about CI here, not VS.

webwarrior-ws added a commit to webwarrior-ws/FSharpLint that referenced this pull request Jan 23, 2024
@webwarrior-ws
build: change NoWarn to WarningsNotAsErrors
43e0eef 
Use WarningsNotAsErrors rather than NoWarn for NU1605 error
as suggested by smoothdeveloper in [1].

[1] fsprojects#680 (comment)
@webwarrior-ws webwarrior-ws mentioned this pull request Jan 23, 2024
Merged
knocte pushed a commit that referenced this pull request Jan 25, 2024
@webwarrior-ws @knocte
build: change NoWarn to WarningsNotAsErrors
acd40c5 
Use WarningsNotAsErrors rather than NoWarn for NU1605 error
as suggested by smoothdeveloper in [1].

[1] #680 (comment)
@webwarrior-ws
Copy link
Contributor Author

CI failure seems to be a random bug - CI run for the same commit in cpm-rebased-2 branch is successful: https://github.com/webwarrior-ws/FSharpLint/actions/runs/7828237116

Numpsy
Numpsy reviewed Feb 12, 2024
View reviewed changes
src/FSharpLint.Console/FSharpLint.Console.fsproj Outdated
@@ -3,6 +3,7 @@
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net6.0</TargetFramework>
<TargetFrameworks>net6.0</TargetFrameworks>
Copy link
Contributor

@Numpsy Numpsy Feb 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is this project file supposed to have both TargetFramework and TargetFrameworks?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch, @webwarrior-ws please fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

webwarrior-ws and others added 5 commits February 12, 2024 13:12
@webwarrior-ws
Migrate from Paket to (Central Pkg Management)NuGet
652750c 
CPM feature requires .NET 6.0.302 as explained in
https://devblogs.microsoft.com/nuget/introducing-central-package-management/
@webwarrior-ws
Added ExcludeAssets="runtime" to Ms.Build.* nuget refs to fix build
9d5c85e 
Build error was:
/home/runner/.nuget/packages/microsoft.build.locator/1.4.1/build/Microsoft.Build.Locator.targets(20,5): error : A PackageReference to Microsoft.Build.* without ExcludeAssets="runtime" exists in your
project. This will cause MSBuild assemblies to be copied to your output directory, causing your application to load them at runtime. To use the copy of MSBuild registered by MSBuildLocator, set
ExcludeAssets="runtime" on the MSBuild PackageReferences. To disable this check, set the property DisableMSBuildAssemblyCopyCheck=true in your project file (not recommended as you must distributed all
of MSBuild + associated toolset). Package(s) referenced: Microsoft.Build.Tasks.Core;Microsoft.Build.Framework;Microsoft.Build.Utilities.Core;Microsoft.Build
[/home/runner/work/FSharpLint/FSharpLint/src/FSharpLint.Core/FSharpLint.Core.fsproj]
@webwarrior-ws
Added nuget ref to Ms.Build.Locator to fix FNFE
1a25fcc 
To fix the following error in the build:
System.IO.FileNotFoundException : Could not load file or assembly
'Microsoft.Build.Locator, Version=1.0.0.0, Culture=neutral, PublicKeyToken=9dff12846e04bfbd'.
The system cannot find the file specified.
@knocte @webwarrior-ws
Update Ms.Build.Framework nuget to fix TLE
c88aecc 
Easy solution to upgrade as recommended in dotnet/interactive#1985 (comment)

The exception was:
Could not load type 'Microsoft.Build.Framework.Traits' from assembly
'Microsoft.Build.Framework, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
@knocte @webwarrior-ws
Add NuGet.Frameworks nuget to try fix FLE
1cafcf9 
Exception was:
System.IO.FileLoadException: Could not load file or assembly 'NuGet.Frameworks, Version=6.2.1.7, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. Could not find or load a specific file.
(0x80131621)
File name: 'NuGet.Frameworks, Version=6.2.1.7, Culture=neutral, PublicKeyToken=31bf3856ad364e35'
knocte and others added 13 commits February 12, 2024 13:12
@knocte @webwarrior-ws
Replace NuGet.Frameworks nuget to workaround FLE
27d08e0 
Previous commit adding the nuget reference didn't work. This is
a workaround that works:
dotnet/roslyn#61454 (comment)
@knocte @webwarrior-ws
Update Ms.Build.Tasks.Core nuget to fix MissingMethodEx
4818408 
Exception was:

Using "ResolveAssemblyReference" task from assembly "Microsoft.Build.Tasks.Core, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a".
Task "ResolveAssemblyReference"
The "ResolveAssemblyReference" task failed unexpectedly.
System.MissingMethodException: Method not found: 'Void Microsoft.Build.Eventing.MSBuildEventSource.RarOverallStop()'.
@knocte @webwarrior-ws
TEST CI
ba0f3cf
@webwarrior-ws
WIP: remove references to Paket.Restore.targets
7b09c9f 
Rmove references to Paket.Restore.targets from .fsproj files
as paket is no longer used.
@webwarrior-ws
WIP: upgrade somepackage versions
970c74b 
Upgraded FSC to 41.0.7,Ionide.ProjInfo. to 0.58, and
FSharp.Core to 6.0.0 as these are versions used in current
master branch.
@webwarrior-ws
WIP: upgrade Microsoft.Build version to 17.2.0
ee5343f
@webwarrior-ws
FunctionalTest: removedworkaround
74dd3f5 
Removedworkaround from FunctionalTest.fsproj that caused error
when building using Visual Studio.
@webwarrior-ws
WIP: revert Microsoft.Build version to 16.11
cb92dc6 
Revert Microsoft.Build and Microsoft.Build.Framework and
Microsoft.Build.Utilities.Core versions to 16.11.
@webwarrior-ws
WIP: made some functional tests pass
97838c2 
Made changes to FSharpLint.FunctionalTest.fsproj and
Directory.Packages.Props to avoid errors when loading projects
in functional tests.
@webwarrior-ws
WIP: set System.Configuration.ConfigurationManager to version 6.0
695c91b 
In attempt to fix this error in functional tests:
```
"Could not load file or assembly 'System.Configuration.ConfigurationManager, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. Could not find or load a specific file. (0x80131621)")"
```
@webwarrior-ws
Console: fix console app
deb32ec 
Add package references to fix `Could not load file or assembly ` bugs
that prevented console app from working.
@webwarrior-ws
Docs: fix docs build task
53814ed 
In Directory.Build.targets added PackageDownload deirectives
for packages used by Fornax and a target that copies dlls from
these downloaded packages to packages/docs directory as these
are no longer managed by Paket. Package versions are exact
because PackageDownload requires them to be exact.
@webwarrior-ws
build: re-enable NU1605
f4f90f7 
- re-enable NU1605 warning;
- fix package versions so there are no package downgrades which
caused NU1605 errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Numpsy Numpsy Numpsy left review comments

@knocte knocte knocte left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@webwarrior-ws @knocte @smoothdeveloper @Numpsy