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

Add PaketReference MSBuild item in projects instead of paket.references #2606

Open
0x53A opened this issue Aug 11, 2017 · 13 comments
Open

Add PaketReference MSBuild item in projects instead of paket.references #2606

0x53A opened this issue Aug 11, 2017 · 13 comments
Labels
enhancement sdk: dotnet/sdk

Comments

@0x53A
Copy link
Contributor

0x53A commented Aug 11, 2017 •
edited
Loading

Instead of having the references in a second file paket.references, I would like to include them into the project file.

This could look like this:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net461</TargetFramework>    
  </PropertyGroup>

  <ItemGroup>
    <PaketReference Include="FSharp.Core" />
    <PaketReference Include="xunit" Group="Test" />
  </ItemGroup>
  
  <ItemGroup>
    <Compile Include="Library.fs" />
  </ItemGroup>

  
  <Import Project="..\.paket\Paket.Restore.targets" />
  
</Project>

Advantages:

  • Similar to the Nuget PackageReference, so maybe easier to explain to someone coming from nuget (just replace all PackageReferences with PaketReferences, and remove the explicit versions, because the versions are handled by paket)
  • It can be dynamically programmed. For example let's say I want to include some Analyzer in all my C# projects.
    ** Currently I need to explicitly add it to all paket.reference files. The most I could do is to then write a unit test that verifies that yes, all projects really import it. If I create a new project, and forget it, the UT fails.
    ** With this feature, I could solve this via MSBuild programming and e.g. a Directory.props
  • The new project format is nice and pretty (well, relative for xml) readable. So why split it up into two files?
  • You could do a conditional reference (this would solve NewSdk and MultiTargeting and Conditions #2596) via <PaketReference Include="FSharp.Core" Group="netcore" Condition=" '$(TargetFramework)' != 'net35' " />

Edit:

I just saw that this was already proposed at #2019.

I guess it was unlikely that I would be the only one to think of this.
@forki
Copy link
Member

forki commented Aug 11, 2017 via email

@forki
Copy link
Member

forki commented Aug 11, 2017 via email

@0x53A
Copy link
Contributor Author

0x53A commented Aug 11, 2017

Yes we are interested in in this. Like making it work in addition to the
existing references files.

I assume that in the first stage, both would be additive? That is, if I have PaketReferences and a paket.references file, the sum of both should be used?

PR with PoC code very welcome

Sure, unfortunately the day has only 24 hours and I need to prioritize this against catching up with GoT ;-)

I want to finish the other stuff I already started first, and next week I am on vacation. If until after then no-one else has started with anything, I will try, but don't necessarily wait for me.

@forki
Copy link
Member

forki commented Aug 11, 2017 via email

@matthid
Copy link
Member

matthid commented Aug 11, 2017

I like the idea. I'm not sure about additive bahavior, because this can lead to unexpected results if one parent directory contains a paket.references file...

@forki
Copy link
Member

forki commented Aug 11, 2017 via email

@0x53A
Copy link
Contributor Author

0x53A commented Aug 20, 2017

@forki Can we deprecate parent paket.references? Not remove them (yet), but print a warning?

@matthid
Copy link
Member

matthid commented Aug 20, 2017

Note that I already added this to the (wish-)list of breaking changes for the next major (I really think we should just go ahead and do it. Therefore, I'm fully supporting the warning.

@0x53A 0x53A mentioned this issue Aug 21, 2017
Closed
@forki
Copy link
Member

forki commented Aug 21, 2017 via email

@matthid
Copy link
Member

matthid commented Aug 21, 2017

Why not for all? I never saw anyone actually using them? (I don't count "by accident" which is most of the time not what you want)

I can show you some examples of where it hurts or was added a workaround (empty files)

@matthid
Copy link
Member

matthid commented Aug 21, 2017

Question is imho more what should be the replacement, and to answer this we need to know some real life examples of people using it

@matthid
Copy link
Member

matthid commented Aug 27, 2017

Regarding the parent paket.references file: When doing #2675 I noticed they are not even properly supported when doing msbuild restore for old and new-style projects.

So they aren't even working as expected TODAY.

@0x53A
Copy link
Contributor Author

0x53A commented Aug 27, 2017

Ok, so IMHO we should remove support for them, but maybe print a warning if we detect that an old version would have used them.

@0x53A 0x53A mentioned this issue Aug 27, 2017
Closed
@enricosada enricosada changed the title [Idea/BrainStorming] NewSdk: Add PaketReference MSBuild item Add PaketReference MSBuild item instead of paket.references Jan 24, 2018
@enricosada enricosada changed the title Add PaketReference MSBuild item instead of paket.references Add PaketReference MSBuild item in projects instead of paket.references Jan 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement sdk: dotnet/sdk
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@forki @enricosada @matthid @0x53A