WIP: Fix dotnetcore #2252
WIP: Fix dotnetcore #2252
Conversation
… parsing logic simpler in future steps.
| else | ||
| let nativePart = getNative path | ||
| let libPart = path.Substring(startPos + infix.Length + 1, endPos - startPos - infix.Length - 1) | ||
| Some (libPart + nativePart) |
There was a problem hiding this comment.
Removing this code is really gives one a good feeling :)
src/Paket.Core/Files/ProjectFile.fs
Outdated
| let sortedTargets = model.TargetsFileFolders |> List.sortBy (fun lib -> lib.Path) | ||
| //sortedTargets | ||
| //|> List.partition (fun lib -> allTargetProfiles = set lib.Targets ) | ||
| ([] : LibFolder list), sortedTargets |
There was a problem hiding this comment.
This seems to be some code-path to "simplify" the generated project file. However it seems like it was never actually used.
After removing runtime this seems to work again (and breaks some tests).
Should we get rid of this simplification (as it was never used) or should I update the tests?
| |> Option.map (fun (l,_) -> { Path = l; File = p; Runtime = RuntimeIdentifier.Any }) | ||
| |> Option.orElseWith (fun _ -> | ||
| (trySscanf "build/%A{noSeperator}" p.PathWithinPackage : string option) | ||
| |> Option.map (fun (_) -> { Path = Tfm.Empty; File = p; Runtime = RuntimeIdentifier.Any })) |
There was a problem hiding this comment.
I think this part is now quite readable. I'm open for suggestions though.
trySscanfis not completely type-safe (therefore the type annotations).- The
%A{scanner}"extension" was invented by me to solve the problem of scanning for files.
| <Import Project="..\..\packages\MultiTarget\build\MultiTarget.props" Condition="Exists('..\..\packages\MultiTarget\build\MultiTarget.props')" Label="Paket" /> | ||
| <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" /> |
There was a problem hiding this comment.
Not quite sure about this change.
What is the difference between a "global" and a "frameworkSpecific" msbuild file?
Paket seems to add global ones on top, but these tests seem to test the reverse?
Are those tests broken? What are the criteria for "global" msbuild?
There was a problem hiding this comment.
That was my try to fix #2227.
Nuget always adds props to the top. The distinction of "global" is specific to that, there is no such thing in nuget.
There was a problem hiding this comment.
@0x53A thanks. so global is everything directly in the build folder, correct?
Is the current output acceptable? I have no Idea what I did that the ordering with the existing item changed... If not can you spot the relevant change (it's ok if not but than I know I have to take a deeper look)
There was a problem hiding this comment.
@matthid I had tried to be smart. Global was everything either directly in the build folder, or for a list of frameworks that exactly matches the framework restriction. I think my use case still works (Fsharp.Compiler.Tools), I don't know if there are other problematic packages that Must be imported at the top.
There was a problem hiding this comment.
@0x53A perfect, if you can live with this new behavior life is good
There was a problem hiding this comment.
For why it moved: previously it was (incorrectly) tagged as framework-specific, not global:
With your simplified Partition(List.partition (fun lib -> "" = lib.Path.Name )), this is now correctly detected as global.
There was a problem hiding this comment.
Thanks for confirming this!
|
I think this is ready to be reviewed or get feedback, though not completely finished. I removed the runtime support as I think it is a cross cutting concern and therefore shouldn't be part of FrameworkIdentifier. Next step is to make runtime work again (but I think the big changes have been done). @forki can you make an alpha out of this (leaving this pr open) so I can start testing on fake vnext? |
|
Closing in favor of #2256 |
This should remove various hacks added over the time when I was kind of offline.
TODO:
Breaks:
/Lib/...is lo longer supported (folder in nuget package needs to be/lib/...Adds:
InstallModelcan properly resolve reference and runtime assemblies (required in FAKE vNext for compilation and running of scripts)