search

fsprojects / FAKE

FAKE - F# Make
https://fake.build
Other
1.28k stars 581 forks source link

Update logic of resolving SdkReferenceAssemblies. #2639

Closed nojaf closed 2 years ago

nojaf commented 2 years ago

Description

nojaf commented 2 years ago

Oh, and I tested this on my local project and it worked on my machine 😊.

nojaf commented 2 years ago

Failing tests feel unrelated,

Failed:  3
    Fake.DotNet.Cli.IntegrationTests.Template tests/can install and run the template/can build a buildtask-dsl inline-dependencies template
    Fake.DotNet.Cli.IntegrationTests.Template tests/can install and run the template/can build a buildtask-dsl file-dependencies template
    Fake.DotNet.Cli.IntegrationTests.Template tests/can install and run the template/fails to build a target that doesn't exist
yazeedobaid commented 2 years ago

@nojaf Thanks for the PR, the template integration tests started to fail not sure why! This happens after we upgrade to net 6.0.101 in FAKE. I'll try to look at them later today.

Thanks

yazeedobaid commented 2 years ago

Tests should be fixed in this PR.

nojaf commented 2 years ago

Thanks @yazeedobaid, I'll rebase when https://github.com/fsprojects/FAKE/pull/2640 got merged in. Assuming my PR goes green then, could we have a hotfix release for fake-cli once this is merged?

yazeedobaid commented 2 years ago

@nojaf Yeah sure, if all goes well then we can release this as a hotfix.

yazeedobaid commented 2 years ago

Should we also use this algorithm in Fake.DotNet.Cli module? Specifically in the findPossibleDotnetCliPaths method https://github.com/fsprojects/FAKE/blob/c7273c70ff688d1e53d2b56809724ff55e3d9961/src/app/Fake.DotNet.Cli/DotNet.fs#L81-L97

cc: @matthid , @baronfel

baronfel commented 2 years ago

@yazeedobaid yes, but with one modification - the current algorithm supports checking the user-local install directories if as a last-resort, but the code from proj-info doesn't. I think that behavior should be kept for probing purposes, and TBH it should be added to proj-info as well. e.g. adding an |> Option.orElseWith tryFindFromDefaultUserDirs to this lazy code

mclark1129 commented 2 years ago

Guessing this is related, although not entirely the same as the original issue mentioned on the PR: https://github.com/fsprojects/FAKE/issues/2641

yazeedobaid commented 2 years ago

@nojaf I have merged #2640 can you please update our branch from release/next

Thanks

nojaf commented 2 years ago

@yazeedobaid rebased.

yazeedobaid commented 2 years ago

Thanks @nojaf For using this algorithm in Fake.DotNet.Cli module as well, I have opened an issue for it #2644 we can track it in that issue. I'll merge this now and release it

Thanks again