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

PackageRef project opt-in technique #4488

Open
clairernovotny opened this issue Feb 3, 2017 · 45 comments
Open

PackageRef project opt-in technique #4488

clairernovotny opened this issue Feb 3, 2017 · 45 comments

Comments

@clairernovotny
Copy link

clairernovotny commented Feb 3, 2017

The issue is that transitive refs with PackageRef isn’t working with P2P refs between CPS and Legacy projects. With PackageRefs, this is supposed to be supported. 

So build the attached project. Try to run Net46ConsoleAppLegacy.exe. It throws due to a missing Newtonsoft.json. Look at the bin\debug directory and you’ll see that the json ref is missing from the output.

By contrast, look at the Net46ConsoleAppCPS project and you’ll see that things work correctly.

There are a few issues in the legacy project:

  1. As you can see in the commented out code in Program.cs of Net46ConsoleAppLegacy, I cannot reference the bottom-most class library directly.
  2. I also cannot use a transitive NuGet reference there either.

Both of these work from the CPS version. This is very weird behavior to have a difference like this.

I get that P2P transitive refs are “new” for CPS, but transitive NuGet refs are not new and work today in VS 2015 with csproj+json. At the very least, I expect #2 to work and for the correct binaries to be in the output directory even if I cannot use types from ClassLibraryWithPackage in Net46ConsoleAppLegacy. The better scenario is that P2P should fully work here too and the Program.cs code in the CPS and Legacy version of the net46 exe should be the same.

SampleSolution.zip

@clairernovotny
Copy link
Author

clairernovotny commented Feb 3, 2017

This is going to be a very common scenario. People are creating new .NET Standard projects and will add p2p refs to them from their existing "legacy" projects. -- Xamarin, Desktop, etc.

The right binaries won't be in the output directory and people will be confused by this broken behavior.

The only workaround is that you have to add <RestoreProjectStyle>PackageReference</RestoreProjectStyle> to the legacy project.

@davkean
Copy link

davkean commented Feb 3, 2017

This user ran into it also: dotnet/sdk#757 (comment)

@clairernovotny
Copy link
Author

clairernovotny commented Feb 3, 2017

As another workaround if RestoreProjectStyle goes away is to use the following dummy packageref in the legacy csproj:

<PackageReference Include="Legacy2CPSWorkaround" Version="1.0.0">
  <PrivateAssets>All</PrivateAssets>
</PackageReference>

@dsplaisted
Copy link

dsplaisted commented Feb 3, 2017

What do you mean if RestoreProjectStyle goes away? How would that happen?

@rrelyea
Copy link
Contributor

rrelyea commented Feb 3, 2017

It isn't going away for NetCore projects.
We had to remove reading it for non-cps scenarios for RTM.

@dsplaisted
Copy link

dsplaisted commented Feb 3, 2017

@rrelyea Will there be a way to force a .NET Framework (non-CPS) project into PackageReference mode without actually adding a PackageReference item?

@rrelyea
Copy link
Contributor

rrelyea commented Feb 3, 2017

Another current workaround:
<PackageReference Update="PlaceholderToConvinceThisProjectToGetTransitivePackageReferenceFromProjectReferences"/>

@rrelyea rrelyea added this to the 4.0.1 milestone Feb 3, 2017
@clairernovotny
Copy link
Author

clairernovotny commented Feb 3, 2017

@rrelyea does that placeholder have to exist? will there be restore errors/failures if it's not there?

@rrelyea
Copy link
Contributor

rrelyea commented Feb 3, 2017

I was just trying to find a workaround that involved a
<PackageReference />
When I tried it like above, it said you need include, remove or update.
Your workaround is to use include with a package that is a noop.
You can also do remove or update. I chose update as less confusing than remove, and somewhat self documenting.

Clearly, we want a more elegant solution.

@clairernovotny
Copy link
Author

clairernovotny commented Feb 3, 2017

Just wasn't sure with Update what happens if it tries to update a package that doesn't exist.

@rrelyea
Copy link
Contributor

rrelyea commented Feb 3, 2017

I think both remove and update are ok to do with package names that don't exist.

@clairernovotny
Copy link
Author

clairernovotny commented Feb 18, 2017

@rrelyea I tried the suggestion of using <PackageReference Update="Legacy2CpsWorkaroundBogus"/> and also <PackageReference Update="Legacy2CpsWorkaroundBogus" Version="1.0.0"/> but neither worked. Neither made the restore correct.

The only thing that's worked so far is to use:

    <PackageReference Include="Legacy2CPSWorkaround" Version="1.0.0">
      <PrivateAssets>All</PrivateAssets>
    </PackageReference>

@nguerrera
Copy link

nguerrera commented Mar 9, 2017

Another report: dotnet/roslyn#17639

@rrelyea rrelyea modified the milestones: 4.1, Future-0 Mar 10, 2017
@rrelyea
Copy link
Contributor

rrelyea commented Mar 10, 2017

@jainaashish - We've been able to re-enable RestoreProjectStyle, in 4.1 already, right?

@jainaashish
Copy link
Contributor

jainaashish commented Mar 10, 2017

Yes, we've re-enabled RestoreProjectStyle property in 4.1

@karann-msft
Copy link
Contributor

karann-msft commented Jul 12, 2019

Per @rrelyea (this is rob typing) - let's spend a few hours this sprint (or soon) and discuss possible solutions and all aspects of the related problem - for new projects, old projects, etc...

@gadeweever
Copy link

gadeweever commented Feb 23, 2020

Solved the same issue with the same workaround from @James-Lester with a .net 4.7.1 project referencing a .net standard 2.0 library.

@nkolev92
Copy link
Member

nkolev92 commented Aug 5, 2022

RestoreProjectStyle is the recommended way for a project to opt into PackageReference. It is respected for old style csproj projects, and will work correctly as long as the project-system loading the project supports it.

At this point, I don't think there's a product action that needs to be taken.

Let me know if anyone disagrees.
We do need to improve the documentation around this topic.

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

No branches or pull requests