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.

Sign up for GitHub

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

Jump to bottom

[wip] fix 2263: Framework restriction is lost for global build folder #2272

Merged
merged 6 commits into from
Apr 24, 2017
Merged

[wip] fix 2263: Framework restriction is lost for global build folder #2272

merged 6 commits into from
Apr 24, 2017

Conversation

0x53A
Copy link
Contributor

@0x53A 0x53A commented Apr 23, 2017 •
edited
Loading

ref #2263

0x53A added 4 commits April 24, 2017 00:50
@0x53A
fix fsprojects#2263
c161074 
use group framework restrictions to decide if a package is global or fw-specific
@0x53A
use restrictions from lock-file, because there AUTO is already resolv…
658f896 
…ed to something concrete
@0x53A
make tests compile again
3acd408
@0x53A
don't care about runtimes
1b2ca37
0x53A
0x53A commented Apr 23, 2017
View reviewed changes
src/Paket.Core/Files/ProjectFile.fs Outdated
@@ -1079,7 +1079,7 @@ module ProjectFile =

let importTargets = defaultArg installSettings.ImportTargets true

let allFrameworks = applyRestrictionsToTargets ((thirdOf3 kv.Value)) KnownTargetProfiles.AllProfiles
let allFrameworks = applyRestrictionsToTargets ((thirdOf3 kv.Value)) (KnownTargetProfiles.AllDotNetStandardProfiles @ KnownTargetProfiles.AllDotNetProfiles)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PLEASE REVIEW: I think we don't care about runtimes when deciding whether to install a .targets / .props, is that correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ref #2238

@0x53A
Copy link
Contributor Author

0x53A commented Apr 23, 2017 •
edited
Loading

Test failure:

 	System.dll!System.Uri.CreateThis(string uri, bool dontEscape, System.UriKind uriKind) + 0x105 bytes	
 	System.dll!System.Uri.Uri(string uriString) + 0x16 bytes	
>	Paket.Core.dll!Paket.Utils.createRelativePath(string root, string path) Line 163 + 0x18 bytes	F#
 	Paket.Core.dll!Paket.ProjectFileModule.globalTargetsNodes@930-4.Invoke(string path) Line 930 + 0x10 bytes	F#

image

The issue is that during these unit tests, the paths are already relative:

image

BUT during a real install, they are absolute:

image

0x53A added 2 commits April 24, 2017 01:37
@0x53A
fix superset direction
f8c82f8
@0x53A
update baselines.
d6a4271 
I HAVENT ACTUALLY VERIFIED THE CHANGES
@forki forki merged commit d6a4271 into fsprojects:master Apr 24, 2017
@0x53A
Copy link
Contributor Author

0x53A commented Apr 24, 2017

@forki was the baseline update ok? especially https://github.com/fsprojects/Paket/pull/2272/files#diff-c8216d6e9932fe764472cecc1af0914cR50

@forki
Copy link
Member

forki commented Apr 24, 2017 via email

@0x53A
Copy link
Contributor Author

0x53A commented Apr 24, 2017

that's why the PR was still tagged [WIP] =)

@0x53A 0x53A deleted the fix-targets branch April 28, 2017 18:48
@matthid matthid mentioned this pull request May 18, 2017
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0x53A @forki