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

5.2.2 Regression - Paket incorrectly adds reference to System assemblies on full frameworks #2469

Closed
optical opened this Issue Jun 27, 2017 · 12 comments

Comments

Projects
None yet
4 participants
@optical

optical commented Jun 27, 2017

Description

When a .NET 4.5 project depends upon Microsoft.Bcl, paket incorrectly adds a reference to System.IO, System.Runtime and System.Threading.Tasks. Previous versions did not do this.

Repro steps

Download & save all the files found in https://gist.github.com/optical/102a08bd9fd71684195a6e3285b7ab8c

  1. Install paket 5.2.2
  2. Run paket install

Expected behavior

Those 3 system libraries are not referenced

Actual behavior

They are infact referenced (5.2.1 and earlier did not do this)

Known workarounds

Use version 5.2.1. Unsure if any other settings work around this.

@Ravadre

This comment has been minimized.

Show comment
Hide comment
@Ravadre

Ravadre Jun 28, 2017

We have similar issues (.Net 4.6, Paket 5.2.3). More references like System.Collections.Concurrent are incorrectly added to project files - this created duplicate references, as types from those libraries are already included in mscorlib.

In case of System.Collections.Concurrent, .Net Standard 1.3 libraries are included (superfluously):

<HintPath>..\packages\System.Collections.Concurrent\lib\netstandard1.3\System.Collections.Concurrent.dll</HintPath>

Downloading Paket 5.2.1 fixes the issue:

.paket\paket.bootstrapper.exe 5.2.1
.paket\paket.exe install

Ravadre commented Jun 28, 2017

We have similar issues (.Net 4.6, Paket 5.2.3). More references like System.Collections.Concurrent are incorrectly added to project files - this created duplicate references, as types from those libraries are already included in mscorlib.

In case of System.Collections.Concurrent, .Net Standard 1.3 libraries are included (superfluously):

<HintPath>..\packages\System.Collections.Concurrent\lib\netstandard1.3\System.Collections.Concurrent.dll</HintPath>

Downloading Paket 5.2.1 fixes the issue:

.paket\paket.bootstrapper.exe 5.2.1
.paket\paket.exe install

@optical optical changed the title from 5.2.2 Regression - Paket incorrectly adds reference to System assemblies on full framework 4.5 to 5.2.2 Regression - Paket incorrectly adds reference to System assemblies on full frameworks Jun 28, 2017

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 28, 2017

Member

@optical I think we do not have any relevant changes between 5.2.1 and 5.2.2

Member

forki commented Jun 28, 2017

@optical I think we do not have any relevant changes between 5.2.1 and 5.2.2

@optical

This comment has been minimized.

Show comment
Hide comment
@optical

optical Jun 28, 2017

@forki yeah I had a look at the changesets tagged between them to see if I could submit a PR, but couldn't see anything related. I don't know F# though, so possibly missed something.

Nevertheless, its easily reproduced. Stumps me!

optical commented Jun 28, 2017

@forki yeah I had a look at the changesets tagged between them to see if I could submit a PR, but couldn't see anything related. I don't know F# though, so possibly missed something.

Nevertheless, its easily reproduced. Stumps me!

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 28, 2017

Member

I think I found something. Give me a sec

Member

forki commented Jun 28, 2017

I think I found something. Give me a sec

@forki forki closed this in 9d7a564 Jun 28, 2017

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 28, 2017

Member

@matthid somehow we excluded the "." placeholders from our model - I pushed a workaround since it's a serious bug.
@optical @Ravadre please retry with 5.2.4

Member

forki commented Jun 28, 2017

@matthid somehow we excluded the "." placeholders from our model - I pushed a workaround since it's a serious bug.
@optical @Ravadre please retry with 5.2.4

@Ravadre

This comment has been minimized.

Show comment
Hide comment
@Ravadre

Ravadre Jun 28, 2017

@forki I can confirm that after updating from 5.2.1 to 5.2.4 and retrying everything works as expected (no changes to any .csproj in our solution after paket install).

Thanks a lot!

Ravadre commented Jun 28, 2017

@forki I can confirm that after updating from 5.2.1 to 5.2.4 and retrying everything works as expected (no changes to any .csproj in our solution after paket install).

Thanks a lot!

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Jun 28, 2017

Member

@forki Did you cherry-pick a commit of #2454? I said it's not ready and has this bug...

Member

matthid commented Jun 28, 2017

@forki Did you cherry-pick a commit of #2454? I said it's not ready and has this bug...

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 28, 2017

Member

Not sure how it got in

Member

forki commented Jun 28, 2017

Not sure how it got in

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 28, 2017

Member

@matthid yes looks like I messed this up. sorry. #2454 is now reverted

Member

forki commented Jun 28, 2017

@matthid yes looks like I messed this up. sorry. #2454 is now reverted

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Jun 28, 2017

Member

no problem I was just really confused because pr was still open

Member

matthid commented Jun 28, 2017

no problem I was just really confused because pr was still open

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 28, 2017

Member

as I said. no idea how I fucked that up. sorry again

Member

forki commented Jun 28, 2017

as I said. no idea how I fucked that up. sorry again

@optical

This comment has been minimized.

Show comment
Hide comment
@optical

optical Jun 28, 2017

Can confirm it is working for me too. Thanks for the prompt fix 👍

optical commented Jun 28, 2017

Can confirm it is working for me too. Thanks for the prompt fix 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment