-
Notifications
You must be signed in to change notification settings - Fork 520
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
Support content files from nuget #84
Support content files from nuget #84
Conversation
I really like this. It's very clean and follows the same techniques as we used before. Kudos. Would like to hear a 1 from someone who tested it. |
According to this one could use the same patterns like the ones found under |
The only content-enabled NuGet I used until today is this. It's a package with a single C# source file. NuGet will install the file with a The logic behind it is severely broken: I created a testing nupkg that also has a VB and a F# file next to the C# file. NuGet installs the C# file for any project type (so VB and F# libraries get the C# file) and omits all others. |
@agross What do you mean by "omits all others" ? IME NuGet adds all the files ... but the ones of the lang of the project are My company employs this technique in a Closed Source project to put VB and CS code into a single source code package. NuGet then does the right thing more or less upon installation - all files get added and only the relevant ones get compiled and there are no errors. The wart is that one needs to manually remove the folder of VB files if you're being anal and not having any junk. Ideally, there would be a way to say "this file only goes in a |
@bartelink I mean that if my test package contains |
Tried the PR (in context of #65) It changed:
to
Problems:
|
@theimowski So firstly, it looks V promising. Unfortunately, in the context of #59 it's obv a dealbreaker for me if every Even if the VS Therefore 'all' that's needed is to 'Just' insert them in order. BUT THEN: it would do something even NuGet can't do: Update jquery/bootstrap in a VS project! |
@bartelink So I tested again with a sample nuget containing empty files:
The I created a new VB Class Library and added the nuget. VS "installed" bar.cs. |
@agross ... So ? (I assume the For avoidance of doubt:
|
I have a hard time reproducing... The F# MVC 5 extension 'WebApi 2' template seems to have dep on bootstrap v 3.0.2, no jquery. 'MVC 5 and WebApi 2' template : bootstrap v3.0.2, jquery. |
Not sure what template does exacly. but its OS somewhere. bootstrap should dep on jquery and fact my deps file only lists boostrap says it does. most of my repro is with |
Let me know if I can help - can concoct a repo if necessary. I'm pretty sure the package with the id |
I'll have some update later today |
Cool, no rush, I'll be about whenever |
I added sorting by the content file path. And what about the "." files in Microsoft.Bcl package (not sure what they are meant for) |
@theimowski I personally am happy but :( [https://groups.google.com/d/msg/web-stack-fs/pn7PL9-Zo0g/WC4z8z518U4J](suggests this error and/or the fact that the sorting is required may be due to a specific version of VFPT e.g. 3.1.2 or/or whatever version introduced folder management) and hence not necessarily critical for Paket to do (Not sure though) Testing... |
@theimowski Was going to say: "Looks great" because it loaded first time Sorting the whole thing as it appears to have done is never going to fly unfortuntately - remember F# projects are order sensitive and the order of the nodes in the Any such sorting would need to be a matter of inserting the I'm not going to have time to research further for a while though. Regarding the other detritus... The sorting has made each I'm guessing when NuGet does an equivalent set of adds, it's using the VS API which is effectively filtering them out. More than likely some 20 year old code somewhere is silently dropping a Also. the folder names strongly suggest they belong to packages other than bootstrap/jquery so its a more general issue which likely has been introduced by your code? Still seeing the redundant I reckon the answer to a lot of this lies in what VS does with no VFPT when you use the wizard. That will tell:
If one has to install VFPT to trigger the issue (NuGet not upgrading, VS not liking what Paket does) then your changes will be 'rollback candidates' (sorry!) |
@bartelink But this feature is supposed only to deal with |
At the moment the alphabetical order in the lockfile is for convenience of
|
The issue is about order of the Content files in proj file rather than lockfile.
works fine, but:
results in a VS error when loading the project (such as @bartelink described). There was a suggestion that the VFPT extension could have something to do with it, however after disabling the extension, the problem didn't dissapear. To add content nodes to a fsproj from a nupkg, Paket has to manage all content nodes and sort them by the Include attr. So the question seems to be: are we fine with Paket managing all Content nodes in project file and keeping them in order? |
@forki The lockfile ordering is on a different issue thread unless I'm really missing something @theimowski @forki @agross More generally, there seems to be confusion creeping in so I'll give a TL;DR of my findings with the latest code improvement:- The recent work here attempts to sort the Content files to make VS happy. The problem (based on me pulling the branch) is that when you run it, |
I guess it means someone is going to have to figure it out then (us) rather than it being shrounded in an SEP field forevermore (Connect :D)
Don't mess with my project file man! Don't make a
Is there really no way to avoid this? (... formulating an approach, 20m :)) |
If I have a NuPkgs:
and A depends on B depends on C and my project has:
And it depends on package A then:
So the packages yield:
Recapping, my code has:
The following needs to be upheld:
Therefore the result can only be:
Now this may all seem a bit crazy and convoluted, but hierarchies and mixes between content and compile happen in packages and in projects. I'd be surprised if Desktop NuGet puts FS files from packages ( But pretty much all the rest of the orderings are extremely important. But the main thing all this needs to uphold is a fundamental claimed (and heretofore delivered) strength of Paket: No wacky diffs. So, each Paket update/install needs to not touch stuff it doesnt own, and needs to have deterministic and idempotent behavior for the stuff it does own. |
But, illustrating how messy it is, I made a slip in the above:
Needs to reorder as
i.e.
If the code behind all this is working with an Xml Document, I'd be trying to insert one by one as it seems simple. But I think that may be too naive, e.g. because of the last set of infernces. |
Anyone know a good contact on the VFPT team and/or VS team to @ into this conversation ? ( I pick @KevinRansom [via https://visualfsharp.codeplex.com/workitem/122 via https://groups.google.com/d/msg/web-stack-fs/pn7PL9-Zo0g/9eVPVhIfB3IJ] ) Is there really no way they can generate folders on the fly even if the project file doesnt group by folder completely? Another question is, what happens when you move files up and down in VS - can it mangle Packet entries? You do have to wonder what they were thinking when they decided to shoehord all this stuff in via the VS object model... I guess it was a nice MVP :) |
What about such approach? Reading existing (non-paket) Content nodes in the project file, and then if Content from dependencies is placed in the same dir, insert the node just before each corresponding, to maintain the previous order. If the Content from dependency is in different dir, then place it in a separate itemgroup beneath. |
Sounds good in principle. Wont get to test/look at code until much later tho' :( |
Ok, even if this approach is satisfactory, there will be yet one more thing to consider: When they do, shall we then copy all the stuff to the project directory? That would result in a mess. I'm pretty sure that NuGet in such cases simply determines which framework condition applies and copies just the respective files to project directory. |
Yeah, NuGet copies and then does loads of work diffing, see some of my comments re #65 While a proper solution using Having said that, I'd love to see it done in that manner and trigger the downstream work as long as there's something that will actually work in the short to medium term. (Ugly too would be to inject conditional logic/ a target that could copy the relevant files at build time!) |
Shall I close this PR for the moment and we'll wait with this feature till it's clearer how we should deal with overall different than |
But it nearly does enough to address #65 which would be a big win ! I think an MVP can be defined and then exotic behavior van be separated into other issues? Main things needed here are:
Perhaps #99 's flow can warn about packages that rely on more complex content behavior? |
can you please rebase on current master? |
6e92a65
to
7d89e18
Compare
5076236
to
2280b1f
Compare
Rebased the branch. |
@theimowski @forki Wrt #65 the current code in
V annoying problems:
Minor nitpicks:
|
No description provided.