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

[Experiment] Rework the dependencies in fake-cli / Fake.Runtime a little. #2825

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Numpsy
Copy link
Contributor

@Numpsy Numpsy commented Sep 19, 2024

Description

I'm not sure if this is a sensible thing to try, without knowing the history of it or anything, but -

I was looking at the failure in the .NET 8.0 update PR at https://github.com/fsprojects/FAKE/actions/runs/10942268784/job/30378864009#step:10:12614 and the issue described at #2722 and was wondering - Is there actually a need to package the Microsoft.Build assemblies in the fake-cli nuget package - as in, are they actually used by any of the code in fake-cli or are they just pulled in as part of a dependency chain and not actually used?

So, I tried bending things a bit to include a couple of pieces of Fake.DotNet.Cli inline in Fake.Runtime (just the bits for getting global.json and executing 'dotnet --version' and excluding any dependencies on MSBuild) and the build appears to work.

It's not very pretty due to how the code is layed out in a single module, but here it is in acse the idea is of any interest.

module internal GlobalJson

open System.IO
open Newtonsoft.Json.Linq
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would switch to System.Text.Json as the next step in the next PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've had that idea on the thought pile as it'd be nice to use the inbox version of System.Text,Json instead of needing Newtonsoft, though I also need to see how you go about including the STJ NuGet package in the .NET Standard 2.0 build and using the inbox version in the .NET 6.0 build

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rebased on top of the System.Text.Json change

Copy link
Contributor

Test Results

    9 files  ±0      9 suites  ±0   40m 28s ⏱️ -34s
  439 tests ±0    439 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 251 runs  ±0  1 251 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 5e53e3a. ± Comparison against base commit 048b97b.

@@ -4,20 +4,25 @@ namespace Fake.DotNet
/// .NET Core + CLI tools helpers
/// </summary>
[<RequireQualifiedAccess>]
#if FAKE_INTERNAL_DOTNET_CORE_CLI
module InternalDotNet =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was because when I tried making the module internal inside Fake.Runtime I got
image
so the name was an attempt at avoiding any clashes

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.

2 participants