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

WIP Fix dotnetcore #2256

Closed
wants to merge 77 commits into from
Closed

WIP Fix dotnetcore #2256

wants to merge 77 commits into from

Conversation

matthid
Copy link
Member

@matthid matthid commented Apr 18, 2017

Same as #2252 but from within the fsprojects Repository to improve collaboration with @cloudRoutine.

This should remove various hacks added over the time when I was kind of offline.

TODO:

  • proper runtime support in InstallModel
  • disabled "simplify" code-path for msbuild targets -> this part was reviewed, I think the current behavior is more deterministic and if we simplify conditions for targets we should do the same for references...
  • (Minor) "Restoring packages for group X" message is shown twice (one for regular step and one for the runtime deps)

Breaks:

  • /Lib/... is lo longer supported (folder in nuget package needs to be /lib/... works again - feels wrong
  • Currently runtime support is removed (see above)

Adds:

  • InstallModel can properly resolve reference and runtime assemblies (required in FAKE vNext for compilation and running of scripts)
  • This enables the complete nuget replacement in dotnetcore world at a later time.
  • Incidentally Improves C++ support

if List.isEmpty installModel.CompileLibFolders then
// TODO: Ask forki about this wtf
let folders = calcLegacyReferenceLibFolders [{ FullPath ="lib/Default.dll"; PathWithinPackage = "lib/Default.dll" }]
{ installModel with CompileLibFolders = folders }
Copy link
Member Author

Choose a reason for hiding this comment

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

@forki ...

@forki
Copy link
Member

forki commented Apr 19, 2017 via email

let path = Path.Combine(dir.FullName.ToLower(), subFolderName)
if dir.Exists then
dir.GetDirectories()
|> Array.filter (fun fi -> String.equalsIgnoreCase fi.FullName path)
|> Array.collect (fun dir -> dir.GetFiles(searchPattern, SearchOption.AllDirectories))
|> Array.map (fun file ->
let fullPath = Path.GetFullPath file.FullName;
Copy link
Member Author

Choose a reason for hiding this comment

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

";" :)

Install Settings\n\
%A" self.Name deps self.Source self.Settings
"%A\nDependencies -\n%s\nSource - %A\nInstall Settings\n%A"
self.Name deps self.Source self.Settings
Copy link
Member Author

Choose a reason for hiding this comment

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

Did you change this because the tooling is struggling with it :P

Copy link
Member

Choose a reason for hiding this comment

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

yea it's the stupid error because of the backslash at the end of the line

member this.RemoveIfCompletelyEmpty() = InstallModel.removeIfCompletelyEmpty this

static member CreateFromLibs(packageName, packageVersion, frameworkRestrictions:FrameworkRestriction list, libs : UnparsedPackageFile seq, targetsFiles, analyzerFiles, nuspec : Nuspec) =
InstallModel.createFromLibs packageName packageVersion frameworkRestrictions libs targetsFiles analyzerFiles nuspec
Copy link
Member Author

Choose a reason for hiding this comment

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

Wow not even git recognizes that this file existed before and thinks it was deleted and added again :)

dep.SetAttributeValue(XName.Get "version", "0.0")
else
dep.SetAttributeValue(XName.Get "version", version)
dep
Copy link
Member Author

Choose a reason for hiding this comment

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

I hand-crafted @forki's fix in here because git wouldn't recognize this file anymore :)

@cloudRoutine
Copy link
Member

@matthid I think you can close this in lieu of #2267 at this point
I think all of your commits are incorporated at this point and the conflicts have been resolved, but you may want to check out the PR to make sure I didn't accidentally drop or clobber anything during the messy merge I just got through 😆

@matthid
Copy link
Member Author

matthid commented Apr 25, 2017

Ok, let me include and test the appveyor package on fake...
And your pr title and description is now way off ...

@matthid matthid closed this Apr 25, 2017
@matthid matthid deleted the fix_dotnetcore branch May 6, 2017 13:46
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.

None yet

4 participants