Skip to content

Investigate.nuget #18393

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

Merged
merged 7 commits into from
Mar 24, 2025
Merged

Investigate.nuget #18393

merged 7 commits into from
Mar 24, 2025

Conversation

KevinRansom
Copy link
Contributor

@KevinRansom KevinRansom commented Mar 17, 2025

Fixes #18231 #r nuget ..." downloads unneeded packages.

What happens is, when typechecking in the ide, we try to fetch nuget packages after every key stroke, which is obviously not great because it means when typing #r "nuget: fsharp.data" we will go and fetch the f package from nuget.org, as well as fs and fsh.

This is clearly not ideal, this fix will not submit a #r nuget packagemanager line if the input caret is still on the line with #r "nuget:blah".

To cause the line to be submitted, the user must either move focus to a different window, or move the input caret to a different line. In my testing locally this actually works really well, it's probably pretty much what I think you would naively expect.

Quickinfo hovertext, is disabled if the packagemanager line has not yet been submitted. After submission, it should show the resolved .dll path.

There is one more delta in this fix, unrelated to the IDE, but part of this fix. The change is that by default, when we resolve nuget packages we disable package based build targets by adding: ExcludeAssets='build;buildTransitive;buildMultitargeting' to the package reference.

Most netsdk nuget packages do not need the build targets to execute during restore, which is our primary goal with #r nuget. And so this should be a neutral change. For those packages which must have their build targets executed during restore, this PR adds a new control to #r nuget, usepackagetargets=true.

For example:

#r "nuget:fsharp.data"

generates this:

<ItemGroup><PackageReference Include='fsharp.data' Version='*' ExcludeAssets='build;buildTransitive;buildMultitargeting' /></ItemGroup>

and

#r "nuget:fsharp.data, usepackagetargets=true"
generates this:
<ItemGroup><PackageReference Include='fsharp.data' Version='*'  /></ItemGroup>

I still need to add test cases for the usepackagetargets=true

It should be noted that: the contents of the "nuget: blah" is not part of the language specification for F#, package managers are an extension, hence the not at all F# nature of the syntax.

The public FCS API on the checker: GetProjectOptionsFromScript and GetProjectSnapshotFromScript

Has an additional optional argument: caret: Position

If it is present then the Position argument is the location of the caret in the ISourceText buffer.
If it is omitted then the old behavior occurs.

Online docs updated here: dotnet/docs#45467
to
image

/cc @baronfel , @vzarytovskii , @T-Gro

Copy link
Contributor

github-actions bot commented Mar 17, 2025

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/9.0.300.md
vsintegration/src docs/release-notes/.VisualStudio/17.14.md

@KevinRansom KevinRansom force-pushed the investigate.nuget branch 3 times, most recently from b4e9cef to 4f0d4f0 Compare March 18, 2025 05:49
@KevinRansom KevinRansom changed the title WIP: Investigate.nuget Investigate.nuget Mar 21, 2025
@KevinRansom KevinRansom marked this pull request as draft March 21, 2025 23:13
@KevinRansom KevinRansom marked this pull request as ready for review March 21, 2025 23:14
@T-Gro
Copy link
Member

T-Gro commented Mar 24, 2025

cc: @auduchinok , @TheAngryByrd , @baronfel :

One part of this change is a tooling heuristics to postpone downloads of packages from #r "nuget: ... while the caret is on the same line. This PR makes the caret position an optional part of the public API for script checking, so that you can decide about implementing the same for script design-time scenario in Ionide/Rider as well.

Passing in None will keep existing behavior of downloading packages while typing.

@TheAngryByrd
Copy link
Contributor

Originally we had some code that tried to do this, good to see it’s built in now!

@NatElkins
Copy link
Contributor

Will this still play nicely with things like FAKE scripts?

https://fake.build/guide/getting-started.html#Run-FAKE-using-the-FAKE-runner

@T-Gro
Copy link
Member

T-Gro commented Mar 24, 2025

Will this still play nicely with things like FAKE scripts?

https://fake.build/guide/getting-started.html#Run-FAKE-using-the-FAKE-runner

If the mentioned Fake package relies on custom build props/targets shipped with package, it will need the additional syntax for allowing that, this PR does think of that.

@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Mar 24, 2025
@KevinRansom KevinRansom merged commit 734e8a5 into dotnet:main Mar 24, 2025
33 checks passed
T-Gro added a commit that referenced this pull request Mar 27, 2025
)

* Versioning for 17.14 and 9.0.300 (#18222)

* more code flow

* Update fantomas to 7.0.1 (#18400)

* Update fantomas

* Update fantomas - vsintegration

* Investigate.nuget (#18393)

* initial

* testing

* temp

* Fantomas, readme

* temp

* nowarn quotes for fantomas

* tests

* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250320.3 (#18398)

Microsoft.SourceBuild.Intermediate.source-build-reference-packages
 From Version 9.0.0-alpha.1.25163.3 -> To Version 9.0.0-alpha.1.25170.3

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Fantomas (#18404)

* Fix false negative [<TailCall>] warning (#18399)

* Update dependencies from https://github.com/dotnet/msbuild build 20250324.8 (#18405)

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Framework , Microsoft.Build.Tasks.Core , Microsoft.Build.Utilities.Core
 From Version 17.13.21-preview-25169-06 -> To Version 17.13.22-preview-25174-08

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Kevin Ransom (msft) <[email protected]>

* Fix GC test that is flaky on Linux (#18408)

* update runtime to 9.0.3 (#18406)

---------

Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Petr <[email protected]>
Co-authored-by: Petr Pokorny <[email protected]>
Co-authored-by: Kevin Ransom (msft) <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Viktor Hofer <[email protected]>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dawe <[email protected]>
Co-authored-by: Jakub Majocha <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants