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

Make packages folder optional #2638

Merged
merged 19 commits into from Aug 26, 2017

Conversation

Projects
None yet
2 participants
@matthid
Member

matthid commented Aug 20, 2017

Current design:

  • It works with the storage: none and storage: packages setting

  • Some internal behavior has been changed to make this possible

    • Moved "paket.locked" to paket-files folder
    • Nuspec is now always loaded from .nuget folder (this breaks people
      doing manual changes to nuspec files in the "packages" folder)
    • Packages are always extracted into the .nuget/packages folder (even if
      they are cli-tools)
  • Introduce the required infrastructure

  • Make sure stuff still works

  • Test and add test for the new feature

  • Add review comments / change according to feedback

NOTE: Currently based on v2_vs_v3 branch (until that one is merged). This makes the diff easier to review (and easier for me ;))

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 20, 2017

Member

Whoa this seems to slowly start working. And the garbage collector is even removing everything :)

Member

matthid commented Aug 20, 2017

Whoa this seems to slowly start working. And the garbage collector is even removing everything :)

@matthid

Some comments to get feedback on (+reminders)

| NoPackagesFolder -> None
| ResolvedFolder f -> Some f
type PackagesFolderGroupConfig =

This comment has been minimized.

@matthid

matthid Aug 20, 2017

Member

I don't like the name and the location :/

@matthid

matthid Aug 20, 2017

Member

I don't like the name and the location :/

Show outdated Hide outdated docs/content/dependencies-file.md
Show outdated Hide outdated src/Paket.Core/Dependencies/PackageResolver.fs
Show outdated Hide outdated src/Paket.Core/Dependencies/NuGet.fs
Show outdated Hide outdated src/Paket.Core/Dependencies/NuGet.fs
Show outdated Hide outdated src/Paket.Core/Installation/DependencyChangeDetection.fs
Show outdated Hide outdated src/Paket.Core/Installation/GarbageCollection.fs
Show outdated Hide outdated src/Paket.Core/Installation/GarbageCollection.fs
Show outdated Hide outdated src/Paket.Core/Installation/GarbageCollection.fs
Show outdated Hide outdated src/Paket.Core/Installation/InstallProcess.fs

matthid added some commits Aug 20, 2017

Initial scetch in making the packages folder optional
- Move "paket.locked" to paket-files folder
- Nuspec is now always loaded from .nuget folder (this breaks people
doing manual changes to nuspec files in the "packages" folder)
- Packages are always extracted into the .nuget/packages folder (even if
they are cli-tools)
- This commit adds only the required infrastructure, but it should work
with the 'storage: none' and 'storage: packages' options.
add 'PackageInfo' to fallback to group settings automatically (for al…
…l 'higher' APIs). Fix bug in garbage collector.

@matthid matthid changed the base branch from v2_vs_v3_data to master Aug 21, 2017

@matthid matthid referenced this pull request Aug 21, 2017

Merged

Cleanup PublicAPI #2643

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 22, 2017

Member

There is still one bug like:

Paket failed with
-> ArgumentException: The input sequence was empty.
   Parameter name: source
   StackTrace:
        at Microsoft.FSharp.Collections.SeqModule.ExactlyOne[T](IEnumerable`1 source)
        at Paket.NuGet.GetContent@87.Invoke(Unit unitVar)
        at System.Lazy`1.CreateValue()
        at System.Lazy`1.LazyInitValue()
        at <StartupCode$Paket-Core>.$DependencyCache.exprs@207-5.Invoke(Unit unitVar)
        at Microsoft.FSharp.Control.AsyncBuilderImpl.callA@839.Invoke(AsyncParams`1 args)
     --- End of stack trace from previous location where exception was thrown ---
        at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
        at Microsoft.FSharp.Control.AsyncBuilderImpl.commit[a](AsyncImplResult`1 res)
        at Microsoft.FSharp.Control.CancellationTokenOps.RunSynchronouslyInCurrentThread[a](CancellationToken token, FSharpAsync`1 computation)
        at Microsoft.FSharp.Control.CancellationTokenOps.RunSynchronously[a](CancellationToken token, FSharpAsync`1 computation, FSharpOption`1 timeout)
        at Microsoft.FSharp.Control.FSharpAsync.RunSynchronously[T](FSharpAsync`1 computation, FSharpOption`1 timeout, FSharpOption`1 cancellationToken)
        at Paket.DependencyCache.SetupGroup(GroupName groupName)
        at Paket.DependencyCache.OrderedGroups(GroupName groupName)
        at Paket.LoadingScripts.ScriptGeneration.cachedGroups@184-1.Invoke(GroupName g)
        at Microsoft.FSharp.Collections.Internal.IEnumerator.map@74.DoMoveNext(b& )
        at Microsoft.FSharp.Collections.Internal.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
        at Microsoft.FSharp.Collections.Internal.IEnumerator.map@74.DoMoveNext(b& )
        at Microsoft.FSharp.Collections.Internal.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
        at Microsoft.FSharp.Collections.SeqModule.ToList[T](IEnumerable`1 source)
        at Paket.LoadingScripts.ScriptGeneration.generateScriptContent(PaketContext _arg1)
        at Paket.LoadingScripts.ScriptGeneration.scriptData@315-1.GenerateNext(IEnumerable`1& next)
        at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.MoveNextImpl()
        at Microsoft.FSharp.Collections.SeqModule.ToList[T](IEnumerable`1 source)
        at Paket.LoadingScripts.ScriptGeneration.scriptData@313.GenerateNext(IEnumerable`1& next)
        at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.MoveNextImpl()
        at Microsoft.FSharp.Collections.SeqModule.ToList[T](IEnumerable`1 source)
        at Paket.LoadingScripts.ScriptGeneration.constructScriptsFromData(DependencyCache depCache, FSharpList`1 groups, FSharpList`1 providedFrameworks, FSharpList`1 providedScriptTypes)
        at Paket.RestoreProcess.CreateScriptsForGroups(DependenciesFile dependenciesFile, LockFile lockFile, FSharpMap`2 groups)
        at Paket.RestoreProcess.Restore@471-2.Invoke(Unit unitVar0)
        at Paket.Utils.RunInLockedAccessMode[a](String rootFolder, FSharpFunc`2 action)
        at Paket.RestoreProcess.Restore(String dependenciesFileName, FSharpOption`1 projectFile, Boolean force, FSharpOption`1 group, FSharpList`1 referencesFileNames, Boolean ignoreChecks, Boolean failOnChecks, FSharpOption`1 targetFrameworks)
        at Paket.Dependencies.Restore(Boolean force, FSharpOption`1 group, FSharpList`1 files, Boolean touchAffectedRefs, Boolean ignoreChecks, Boolean failOnChecks, FSharpOption`1 targetFramework)
        at Paket.Dependencies.Restore(Boolean force, FSharpOption`1 group, Boolean onlyReferenced, Boolean touchAffectedRefs, Boolean ignoreChecks, Boolean failOnFailedChecks, FSharpOption`1 targetFramework)
        at Paket.Program.restore(ParseResults`1 results)
        at Paket.Program.main@737-12.Invoke(ParseResults`1 results)
        at Paket.Program.processWithValidation[T](Boolean silent, FSharpFunc`2 validateF, FSharpFunc`2 commandF, ParseResults`1 result)
        at Paket.Program.processCommand[a](Boolean silent, FSharpFunc`2 commandF, ParseResults`1 result)
        at Paket.Program.main()

Probably trying to analyze the wrong (empty) folder.

Member

matthid commented Aug 22, 2017

There is still one bug like:

Paket failed with
-> ArgumentException: The input sequence was empty.
   Parameter name: source
   StackTrace:
        at Microsoft.FSharp.Collections.SeqModule.ExactlyOne[T](IEnumerable`1 source)
        at Paket.NuGet.GetContent@87.Invoke(Unit unitVar)
        at System.Lazy`1.CreateValue()
        at System.Lazy`1.LazyInitValue()
        at <StartupCode$Paket-Core>.$DependencyCache.exprs@207-5.Invoke(Unit unitVar)
        at Microsoft.FSharp.Control.AsyncBuilderImpl.callA@839.Invoke(AsyncParams`1 args)
     --- End of stack trace from previous location where exception was thrown ---
        at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
        at Microsoft.FSharp.Control.AsyncBuilderImpl.commit[a](AsyncImplResult`1 res)
        at Microsoft.FSharp.Control.CancellationTokenOps.RunSynchronouslyInCurrentThread[a](CancellationToken token, FSharpAsync`1 computation)
        at Microsoft.FSharp.Control.CancellationTokenOps.RunSynchronously[a](CancellationToken token, FSharpAsync`1 computation, FSharpOption`1 timeout)
        at Microsoft.FSharp.Control.FSharpAsync.RunSynchronously[T](FSharpAsync`1 computation, FSharpOption`1 timeout, FSharpOption`1 cancellationToken)
        at Paket.DependencyCache.SetupGroup(GroupName groupName)
        at Paket.DependencyCache.OrderedGroups(GroupName groupName)
        at Paket.LoadingScripts.ScriptGeneration.cachedGroups@184-1.Invoke(GroupName g)
        at Microsoft.FSharp.Collections.Internal.IEnumerator.map@74.DoMoveNext(b& )
        at Microsoft.FSharp.Collections.Internal.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
        at Microsoft.FSharp.Collections.Internal.IEnumerator.map@74.DoMoveNext(b& )
        at Microsoft.FSharp.Collections.Internal.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
        at Microsoft.FSharp.Collections.SeqModule.ToList[T](IEnumerable`1 source)
        at Paket.LoadingScripts.ScriptGeneration.generateScriptContent(PaketContext _arg1)
        at Paket.LoadingScripts.ScriptGeneration.scriptData@315-1.GenerateNext(IEnumerable`1& next)
        at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.MoveNextImpl()
        at Microsoft.FSharp.Collections.SeqModule.ToList[T](IEnumerable`1 source)
        at Paket.LoadingScripts.ScriptGeneration.scriptData@313.GenerateNext(IEnumerable`1& next)
        at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.MoveNextImpl()
        at Microsoft.FSharp.Collections.SeqModule.ToList[T](IEnumerable`1 source)
        at Paket.LoadingScripts.ScriptGeneration.constructScriptsFromData(DependencyCache depCache, FSharpList`1 groups, FSharpList`1 providedFrameworks, FSharpList`1 providedScriptTypes)
        at Paket.RestoreProcess.CreateScriptsForGroups(DependenciesFile dependenciesFile, LockFile lockFile, FSharpMap`2 groups)
        at Paket.RestoreProcess.Restore@471-2.Invoke(Unit unitVar0)
        at Paket.Utils.RunInLockedAccessMode[a](String rootFolder, FSharpFunc`2 action)
        at Paket.RestoreProcess.Restore(String dependenciesFileName, FSharpOption`1 projectFile, Boolean force, FSharpOption`1 group, FSharpList`1 referencesFileNames, Boolean ignoreChecks, Boolean failOnChecks, FSharpOption`1 targetFrameworks)
        at Paket.Dependencies.Restore(Boolean force, FSharpOption`1 group, FSharpList`1 files, Boolean touchAffectedRefs, Boolean ignoreChecks, Boolean failOnChecks, FSharpOption`1 targetFramework)
        at Paket.Dependencies.Restore(Boolean force, FSharpOption`1 group, Boolean onlyReferenced, Boolean touchAffectedRefs, Boolean ignoreChecks, Boolean failOnFailedChecks, FSharpOption`1 targetFramework)
        at Paket.Program.restore(ParseResults`1 results)
        at Paket.Program.main@737-12.Invoke(ParseResults`1 results)
        at Paket.Program.processWithValidation[T](Boolean silent, FSharpFunc`2 validateF, FSharpFunc`2 commandF, ParseResults`1 result)
        at Paket.Program.processCommand[a](Boolean silent, FSharpFunc`2 commandF, ParseResults`1 result)
        at Paket.Program.main()

Probably trying to analyze the wrong (empty) folder.

matthid added some commits Aug 25, 2017

make isExtracted ignore any existing license files (this might be foo…
…led if a package only contains a specially named license file)

@matthid matthid changed the title from [WIP, Discuss] Make packages folder optional to Make packages folder optional Aug 26, 2017

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 26, 2017

Member

Ok if this goes green I consider this "ready".
Just to be sure here: While refactoring I did take some care to not break existing functionality (as marked above), so storage: packages or the default are probably still quite stable.
The new feature storage: none is in alpha/beta. Therefore we should advertise as that.
However, I think we can make a stable as long as the new feature clearly is advertised as alpha/beta.

Member

matthid commented Aug 26, 2017

Ok if this goes green I consider this "ready".
Just to be sure here: While refactoring I did take some care to not break existing functionality (as marked above), so storage: packages or the default are probably still quite stable.
The new feature storage: none is in alpha/beta. Therefore we should advertise as that.
However, I think we can make a stable as long as the new feature clearly is advertised as alpha/beta.

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 26, 2017

Member

Seems like I broke something :/

Member

matthid commented Aug 26, 2017

Seems like I broke something :/

matthid added some commits Aug 26, 2017

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 26, 2017

Member

I don't understand why I now get a PathTooLongException :/

Member

matthid commented Aug 26, 2017

I don't understand why I now get a PathTooLongException :/

@matthid matthid merged commit 6fe9cb9 into master Aug 26, 2017

1 of 4 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment