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

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

Open
inosik opened this issue Jan 13, 2025 · 10 comments
Open

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

inosik opened this issue Jan 13, 2025 · 10 comments
Labels
Area-VS-FSI VS window and commands for F# Interactive Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code.
Milestone

Comments

@inosik
Copy link

inosik commented Jan 13, 2025

Repro steps

  1. Ctrl + N in Visual Studio and select F# script

  2. Add #r "nuget: ini-parser" to the file

    It's important to type this into the file instead of pasting from the clipboard.

  3. Observe C:\Users\...\.nuget\packages

Expected behavior

Only ini-parser directory should appear.

Actual behavior

The following directories appear:

  • in/1.0.0/
  • ini/1.0.7/
  • ini-parser/3.4.0/

Every package on nuget.org whose ID is a prefix of the desired package is downloaded.

Another example: I tried to use microsoft.powershell.5.1.referenceassemblies once. Here are the packages that got downloaded:

Image

I have a bunch of packages in my NuGet cache directory that I never used (knowingly), but whose IDs are all prefixes of packages I did use:

This also looks like this is the reason why it takes so long for IntelliSense to work for the package I try to use.

Related information

Provide any related information (optional):

  • Operating system: Windows 11 23H2
  • Editing Tools: VS 17.12.3
@github-actions github-actions bot added this to the Backlog milestone Jan 13, 2025
@inosik inosik changed the title "#r "nuget ..." downloads unneeded packages "#r nuget ..." downloads unneeded packages Jan 13, 2025
@vzarytovskii
Copy link
Member

vzarytovskii commented Jan 13, 2025

"#r: nuget..." uses dotnet restore, it is generating a project + nuget.confg (latter is based on the one it finds the closes iirc) under ~/.packagemanagement.
For the example from the OP, it will generate the following:
Project.fsproj:

<Project Sdk='Microsoft.NET.Sdk'>

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <RuntimeIdentifier>osx-arm64</RuntimeIdentifier>
    <IsPackable>false</IsPackable>
    <DisableFSharpCorePreviewCheck>true</DisableFSharpCorePreviewCheck>     <!-- Disable preview FSharp.Core current DotNet Sdks    -->

    <!-- Disable automagic FSharp.Core resolution when not using with FSharp scripts -->
    <DisableImplicitFSharpCoreReference Condition="'.fsx' != '.fsx'">true</DisableImplicitFSharpCoreReference>
    <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>

    <!-- Temporary fix some sdks, shipped internally with broken parameterization -->
    <FSharpCoreImplicitPackageVersion Condition="'$(FSharpCoreImplicitPackageVersion)' == '{{FSharpCoreShippedPackageVersion}}'">4.7.0</FSharpCoreImplicitPackageVersion>
    <FSharpCoreImplicitPackageVersion Condition="'$(FSharpCoreImplicitPackageVersion)' == '{{FSharpCorePreviewPackageVersion}}'">4.7.1-*</FSharpCoreImplicitPackageVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include='Microsoft.NETFramework.ReferenceAssemblies' Version='1.0.0' Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'"/>
  </ItemGroup>

  <ItemGroup><PackageReference Include='ini-parser' Version='*' /></ItemGroup>

  <Target Name="RetrieveNuspecIdAndVersion" Inputs="@(NuspecFiles)" Outputs="%(Identity).Dummy">
    <XmlPeek
        Condition="'%(NuspecFiles.Identity)'!=''"
        XmlInputPath="%(NuspecFiles.Identity)"
        Query="/a:package/a:metadata/a:id/child::text()"
        Namespaces="&lt;Namespace Prefix='a' Uri='http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd' /&gt;">
      <Output TaskParameter='Result' PropertyName ='Id' />
    </XmlPeek>
    <XmlPeek
        Condition="'%(NuspecFiles.Identity)'!=''"
        XmlInputPath="%(NuspecFiles.Identity)"
        Query="/a:package/a:metadata/a:version/child::text()"
        Namespaces="&lt;Namespace Prefix='a' Uri='http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd' /&gt;">
      <Output TaskParameter='Result' PropertyName ='Version' />
    </XmlPeek>
    <ItemGroup>
      <NugetPackageInfo
          Include="%(NuspecFiles.Identity)"
          Condition = "'$(Id)' != '' and '$(Version)' != ''">
        <NugetPackageId>$(Id)</NugetPackageId>
        <NugetPackageVersion>$(Version)</NugetPackageVersion>
        <PackageRoot>$([MSBuild]::EnsureTrailingSlash("$([System.IO.Path]::GetDirectoryName('%(NuspecFiles.Identity)'))").Replace('\', '/'))</PackageRoot>
        <AssetType>package</AssetType>
      </NugetPackageInfo>
    </ItemGroup>
  </Target>

  <Target Name="RetrieveNuspecMetadatas">
    <ItemGroup>
        <PropertyNames Include = "Pkg$([System.String]::Copy('%(PackageReference.FileName)').Replace('.','_'))" />
        <PropertyNames Include = "Pkg$([System.String]::Copy('%(PackageReference.FileName)%(PackageReference.Extension)').Replace('.','_'))"/>
        <NuspecFiles Include="$(%(PropertyNames.FileName))\*.nuspec" />
    </ItemGroup>
  </Target>

  <Target Name="ComputePackageRootsForInteractivePackageManagement"
          DependsOnTargets="ResolveReferences;ResolveSdkReferences;ResolveTargetingPackAssets;ResolveSDKReferences;GenerateBuildDependencyFile;RetrieveNuspecMetadatas;RetrieveNuspecIdAndVersion">
      <ItemGroup>
        <__InteractiveReferencedAssemblies Include = "@(ReferencePath)" />
        <__InteractiveReferencedAssembliesCopyLocal Include = "@(RuntimeCopyLocalItems)" Condition="'$(TargetFrameworkIdentifier)'!='.NETFramework'" />
        <__InteractiveReferencedAssembliesCopyLocal Include = "@(ReferenceCopyLocalPaths)" Condition="'$(TargetFrameworkIdentifier)'=='.NETFramework'" />
        <__ConflictsList Include="%(_ConflictPackageFiles.ConflictItemType)=%(_ConflictPackageFiles.Filename)%(_ConflictPackageFiles.Extension)" />
      </ItemGroup>

      <PropertyGroup>
        <__Conflicts>@(__ConflictsList, ';');</__Conflicts>
      </PropertyGroup>

      <ItemGroup>
        <InteractiveResolvedFile Include="@(__InteractiveReferencedAssemblies)"
                                 Condition="$([System.String]::new($(__Conflicts)).Contains($([System.String]::new('Reference=%(__InteractiveReferencedAssemblies.Filename)%(__InteractiveReferencedAssemblies.Extension);'))))"
                                 KeepDuplicates="false">
            <NormalizedIdentity Condition="'%(Identity)'!=''">$([System.String]::Copy('%(Identity)').Replace('\', '/'))</NormalizedIdentity>
            <NormalizedPathInPackage Condition="'%(__InteractiveReferencedAssemblies.PathInPackage)'!=''">$([System.String]::Copy('%(__InteractiveReferencedAssemblies.PathInPackage)').Replace('\', '/'))</NormalizedPathInPackage>
            <PositionPathInPackage Condition="'%(InteractiveResolvedFile.NormalizedPathInPackage)'!=''">$([System.String]::Copy('%(InteractiveResolvedFile.NormalizedIdentity)').IndexOf('%(InteractiveResolvedFile.NormalizedPathInPackage)'))</PositionPathInPackage>
            <PackageRoot Condition="'%(InteractiveResolvedFile.NormalizedPathInPackage)'!='' and '%(InteractiveResolvedFile.PositionPathInPackage)'!='-1'">$([System.String]::Copy('%(InteractiveResolvedFile.NormalizedIdentity)').Substring(0, %(InteractiveResolvedFile.PositionPathInPackage)).Replace('\', '/'))</PackageRoot>
            <InitializeSourcePath Condition="Exists('%(InteractiveResolvedFile.PackageRoot)content\%(InteractiveResolvedFile.NugetPackageId).fsx')">%(InteractiveResolvedFile.PackageRoot)content\%(InteractiveResolvedFile.NugetPackageId).fsx</InitializeSourcePath>
            <IsNotImplementationReference>$([System.String]::Copy('%(__InteractiveReferencedAssemblies.PathInPackage)').StartsWith('ref/'))</IsNotImplementationReference>
            <NuGetPackageId>%(__InteractiveReferencedAssemblies.NuGetPackageId)</NuGetPackageId>
            <NuGetPackageVersion>%(__InteractiveReferencedAssemblies.NuGetPackageVersion)</NuGetPackageVersion>
        </InteractiveResolvedFile>

        <InteractiveResolvedFile Include="@(__InteractiveReferencedAssembliesCopyLocal)" KeepDuplicates="false">
            <NormalizedIdentity Condition="'%(Identity)'!=''">$([System.String]::Copy('%(Identity)').Replace('\', '/'))</NormalizedIdentity>
            <NormalizedPathInPackage Condition="'%(__InteractiveReferencedAssembliesCopyLocal.PathInPackage)'!=''">$([System.String]::Copy('%(__InteractiveReferencedAssembliesCopyLocal.PathInPackage)').Replace('\', '/'))</NormalizedPathInPackage>
            <PositionPathInPackage Condition="'%(InteractiveResolvedFile.NormalizedPathInPackage)'!=''">$([System.String]::Copy('%(InteractiveResolvedFile.NormalizedIdentity)').IndexOf('%(InteractiveResolvedFile.NormalizedPathInPackage)'))</PositionPathInPackage>
            <PackageRoot Condition="'%(InteractiveResolvedFile.NormalizedPathInPackage)'!='' and '%(InteractiveResolvedFile.PositionPathInPackage)'!='-1'">$([System.String]::Copy('%(InteractiveResolvedFile.NormalizedIdentity)').Substring(0, %(InteractiveResolvedFile.PositionPathInPackage)).Replace('\', '/'))</PackageRoot>
            <InitializeSourcePath Condition="Exists('%(__InteractiveReferencedAssembliesCopyLocal.PackageRoot)content\%(__InteractiveReferencedAssembliesCopyLocal.NugetPackageId).fsx')">%(__InteractiveReferencedAssembliesCopyLocal.PackageRoot)content\%(__InteractiveReferencedAssembliesCopyLocal.NugetPackageId).fsx</InitializeSourcePath>
            <IsNotImplementationReference>$([System.String]::Copy('%(__InteractiveReferencedAssembliesCopyLocal.PathInPackage)').StartsWith('ref/'))</IsNotImplementationReference>
            <NuGetPackageId>%(__InteractiveReferencedAssembliesCopyLocal.NuGetPackageId)</NuGetPackageId>
            <NuGetPackageVersion>%(__InteractiveReferencedAssembliesCopyLocal.NuGetPackageVersion)</NuGetPackageVersion>
        </InteractiveResolvedFile>

        <NativeIncludeRoots
            Include="@(RuntimeTargetsCopyLocalItems)"
            Condition="'%(RuntimeTargetsCopyLocalItems.AssetType)' == 'native'">
            <PackageRoot>$([MSBuild]::EnsureTrailingSlash("$([System.String]::Copy('%(FullPath)').Substring(0, $([System.String]::Copy('%(FullPath)').LastIndexOf('runtimes'))))").Replace('\','/'))</PackageRoot>
            <Path>%(FullPath).Replace('\', '/'))</Path>
        </NativeIncludeRoots>

        <NativeIncludeRoots
            Include="@(NativeCopyLocalItems)"
            Condition="'%(NativeCopyLocalItems.AssetType)' == 'native'">
            <PackageRoot>$([MSBuild]::EnsureTrailingSlash("$([System.String]::Copy('%(FullPath)').Substring(0, $([System.String]::Copy('%(FullPath)').LastIndexOf('runtimes'))))").Replace('\','/'))</PackageRoot>
        </NativeIncludeRoots>

        <PropertyNames Include = "Pkg$([System.String]::Copy('%(PackageReference.FileName)').Replace('.','_'))" />
        <PropertyNames Include = "Pkg$([System.String]::Copy('%(PackageReference.FileName)%(PackageReference.Extension)').Replace('.','_'))"/>

        <ProvidedPackageRoots Include = "$(%(PropertyNames.FileName))" Condition="'$(%(PropertyNames.FileName))' != ''">
          <ParentDirectory>$([System.IO.Path]::GetDirectoryName('$(%(PropertyNames.FileName))'))</ParentDirectory>
          <NugetPackageId>$([System.IO.Path]::GetFileName('%(ProvidedPackageRoots.ParentDirectory)'))</NugetPackageId>
          <NugetPackageVersion>$([System.IO.Path]::GetFileName('$(%(PropertyNames.FileName))'))</NugetPackageVersion>
          <AssetType>package</AssetType>
          <PackageRoot>$([MSBuild]::EnsureTrailingSlash('$(%(PropertyNames.FileName))'))</PackageRoot>
          <PackageRoot>$([System.String]::Copy('%(ProvidedPackageRoots.PackageRoot)').Replace('\', '/'))</PackageRoot>
        </ProvidedPackageRoots>
      </ItemGroup>
  </Target>

  <Target Name="InteractivePackageManagement"
          DependsOnTargets="ComputePackageRootsForInteractivePackageManagement"
          BeforeTargets="CoreCompile"
          AfterTargets="PrepareForBuild">

    <ItemGroup>
      <ResolvedReferenceLines Remove='*' />
      <ResolvedReferenceLines
          Condition="(@(InteractiveResolvedFile->Count()) &gt; 0) AND (('%(InteractiveResolvedFile.NugetPackageId)'!='FSharp.Core') or ('.fsx'!='.fsx' and '%(InteractiveResolvedFile.NugetPackageId)'=='FSharp.Core'))"
          Include="%(InteractiveResolvedFile.NugetPackageId),%(InteractiveResolvedFile.NugetPackageVersion),%(InteractiveResolvedFile.PackageRoot),$([System.String]::Copy('%(InteractiveResolvedFile.FullPath)').Replace('\','/')),%(InteractiveResolvedFile.AssetType),%(InteractiveResolvedFile.IsNotImplementationReference),%(InteractiveResolvedFile.InitializeSourcePath),"
          KeepDuplicates="false" />
      <ResolvedReferenceLines
          Condition="(@(NativeIncludeRoots->Count()) &gt; 0) AND (('%(NativeIncludeRoots.NugetPackageId)'!='FSharp.Core') or ('.fsx'!='.fsx' and '%(NativeIncludeRoots.NugetPackageId)'=='FSharp.Core'))"
          Include="%(NativeIncludeRoots.NugetPackageId),%(NativeIncludeRoots.NugetPackageVersion),%(NativeIncludeRoots.PackageRoot),,%(NativeIncludeRoots.AssetType),,,$([System.String]::Copy('%(NativeIncludeRoots.FullPath)').Replace('\','/'))"
          KeepDuplicates="false" />
      <ResolvedReferenceLines
          Condition="(@(NugetPackageInfo->Count()) &gt; 0) AND (('%(NugetPackageInfo.NugetPackageId)'!='FSharp.Core') or ('.fsx'!='.fsx' and '%(ProvidedPackageRoots.NugetPackageId)'=='FSharp.Core'))"
          Include="%(NugetPackageInfo.NugetPackageId),%(NugetPackageInfo.NugetPackageVersion),%(NugetPackageInfo.PackageRoot),,%(NugetPackageInfo.AssetType),,,"
          KeepDuplicates="false" />
    </ItemGroup>

    <WriteLinesToFile Lines='@(ResolvedReferenceLines)'
                      File='$(MSBuildProjectFullPath).resolvedReferences.paths'
                      Overwrite='True' WriteOnlyWhenDifferent='True' />
  </Target>

</Project>

NuGet.config:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <packageSources>
    <clear />
    <add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
  </packageSources>
</configuration>

I can't see how it can be an issue of the DependencyManager, since it doesn't restore anything.

Additionally, I don't see anything restored beside needed package locally on my laptop. No in or ini packages.

Here's dotnet restore --force -bl, restore part of binlog (on 8.0.400):

Image

@baronfel is it something that rings a bell?

@inosik
Copy link
Author

inosik commented Jan 13, 2025

I should note that this doesn't reproduce when the script is run using dotnet fsi script.fsx either or when I paste #r "nuget: ini-parser" to the file at once. Maybe it's Visual Studio running the whole process for every keystroke?

@vzarytovskii
Copy link
Member

vzarytovskii commented Jan 13, 2025

I should note that this doesn't reproduce when the script is run using dotnet fsi script.fsx either or when I paste #r "nuget: ini-parser" to the file at once. Maybe it's Visual Studio running the whole process for every keystroke?

That would be...interesting. I hope service doesn't invoke dependency manager on every keystroke...

But it sounds that it's what happens, since it needs to bring packages into scope to get intellisense, and typecheck the script, but it seems there's no debounce happening.

@majocha
Copy link
Contributor

majocha commented Jan 13, 2025

My guess it is not as much every keystroke as every valid #r reference. You start typing and Visual Studio makes requests to parse and check
#r "nuget: in"
#r "nuget: ini"
and so on.

I'd expect Visual Studio throttle things on its own by cancelling outdated requests when the document changes. Probably we need to add some cancellation checks inside ComputeClosureOfScriptText

LoadClosure.ComputeClosureOfScriptText(

@inosik
Copy link
Author

inosik commented Jan 14, 2025

The generated Project.fsproj is regenerated every couple of seconds (the time it takes to run dotnet restore I guess) with every package ID that existed at some point while typing:

  • i
  • in
  • ini
  • ini-
  • ...
  • ini-parser

Edit: The file doesn't change after the package ID is complete. Looks like at least it isn't wriltten to disk on every keystroke.

@T-Gro
Copy link
Member

T-Gro commented Jan 15, 2025

My guess it is not as much every keystroke as every valid #r reference. You start typing and Visual Studio makes requests to parse and check #r "nuget: in" #r "nuget: ini" and so on.

I'd expect Visual Studio throttle things on its own by cancelling outdated requests when the document changes. Probably we need to add some cancellation checks inside ComputeClosureOfScriptText

fsharp/src/Compiler/Service/BackgroundCompiler.fs

Line 1353 in b204f87

LoadClosure.ComputeClosureOfScriptText(

Cancellation checks and throttling would be the way to go, I agree.

One could also avoid incomplete package names by change of the syntax (like insisting on a postfix symbol to make it clear that user is finished typing), but that would be a breaking change as of now.
The syntax is now terminated by double-quotes, but IDEs already pair them up so the syntax is valid even when user is still typing.

@KevinRansom
Copy link
Member

@T-Gro Cancellation isn't going to help a very slow typist avoid downloading a package that is partially specified while typing slow.

@vzarytovskii
Copy link
Member

@T-Gro Cancellation isn't going to help a very slow typist avoid downloading a package that is partially specified while typing slow.

I wonder if few things can be done:

  1. Builtin debounce in the dependency manager.
  2. Cancellation.
  3. Force-triggering it if user newlined after the #r: nuget.

It should also help with slow typers, but won't fix it for 100% of the cases.

@T-Gro
Copy link
Member

T-Gro commented Jan 15, 2025

@T-Gro Cancellation isn't going to help a very slow typist avoid downloading a package that is partially specified while typing slow.

Unless we change the syntax/UX to include a specific "mark" which the user signalized that name is complete, the "slow typist" scenario will always be there.

It is not similar to anything we have done before, but what about only treating the nuget specification as finished once the focus moves to a different line? i.e. once the cursor isn't on the line with the #r
(I have no clue if we can easily access that info from the editor)

@majocha
Copy link
Contributor

majocha commented Jan 15, 2025

We could track the buffer changed events in tooling and compare with caret position.

Instead of force triggering the dependency manager It might be better to add a way to temporarily suppress it.

@T-Gro T-Gro added Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Area-VS-FSI VS window and commands for F# Interactive and removed Needs-Triage labels Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VS-FSI VS window and commands for F# Interactive Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code.
Projects
Status: New
Development

No branches or pull requests

5 participants