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

Support content files from nuget #84

Merged
merged 6 commits into from
Sep 16, 2014

Conversation

theimowski
Copy link
Member

No description provided.

@forki
Copy link
Member

forki commented Sep 11, 2014

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.

@agross
Copy link
Contributor

agross commented Sep 11, 2014

According to this one could use the same patterns like the ones found under lib to specifiy different content for different target frameworks. The "root" for files to copy then becomes content/<matching framework>, otherwise it's content.

@agross
Copy link
Contributor

agross commented Sep 11, 2014

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 <Compile /> element.

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.

@bartelink
Copy link
Member

@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 <Compile vs <None.

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 CS, this one only in a FS, but that's orthogonal to the primary way of seggregating that NuGet offers: framework / whatever oddities you've discovered :D

@agross
Copy link
Contributor

agross commented Sep 11, 2014

@bartelink I mean that if my test package contains foo.cs and foo.vb I found that only foo.cs was added by NuGet VS, regardless of the project type. I'll test again.

@bartelink
Copy link
Member

Tried the PR (in context of #65)

It changed:

<Content Include="Content\bootstrap-theme.css" />
<Content Include="Content\bootstrap-theme.min.css" />
<Content Include="Content\bootstrap.css" />
<Content Include="Content\bootstrap.min.css" />
<Content Include="Scripts\bootstrap.js" />
<Content Include="Scripts\bootstrap.min.js" />
<Content Include="Scripts\jquery-2.1.1.min.map" />
<Content Include="Scripts\jquery-2.1.1.min.js" />
<Content Include="Scripts\jquery-2.1.1.js" />
<None Include="Scripts\jquery-2.1.1.intellisense.js" />

to

         <ItemGroup>
            <Content Include="web.config.transform">
              <Paket>True</Paket>
            </Content>
            <Content Include="monoandroid\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="monotouch\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="net45\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="portable-net45 win8 wp8 wpa81\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="portable-net45 win8 wpa81\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="portable-net451 win81\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="portable-net451 win81 wpa81\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="portable-win81 wp81 wpa81\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="sl4\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="sl5\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="win8\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="wp8\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="wpa81\_._">
              <Paket>True</Paket>
            </Content>
            <Content Include="Content\bootstrap-theme.css">
              <Paket>True</Paket>
            </Content>
            <Content Include="Content\bootstrap-theme.css.map">
              <Paket>True</Paket>
            </Content>
            <Content Include="Content\bootstrap-theme.min.css">
              <Paket>True</Paket>
            </Content>
            <Content Include="Content\bootstrap.css">
              <Paket>True</Paket>
            </Content>
            <Content Include="Content\bootstrap.css.map">
              <Paket>True</Paket>
            </Content>
            <Content Include="Content\bootstrap.min.css">
              <Paket>True</Paket>
            </Content>
            <Content Include="Scripts\bootstrap.js">
              <Paket>True</Paket>
            </Content>
            <Content Include="Scripts\bootstrap.min.js">
              <Paket>True</Paket>
            </Content>
            <Content Include="Scripts\jquery-2.1.1-vsdoc.js">
              <Paket>True</Paket>
            </Content>
            <Content Include="Scripts\jquery-2.1.1.js">
              <Paket>True</Paket>
            </Content>
            <Content Include="Scripts\jquery-2.1.1.min.js">
              <Paket>True</Paket>
            </Content>
            <Content Include="Scripts\jquery-2.1.1.min.map">
              <Paket>True</Paket>
            </Content>
          </ItemGroup>

Problems:

  1. Bug: Generates null <ItemGroup/> in other projects

  2. I think the intellisense file being lost could be a problem

  3. Won't load the .fsproj:

    The project 'Web.fsproj' could not be opened because opening it would cause a folder to be rendered multiple times in the solution explorer. One such problematic item is 'Content\bootstrap-theme.css'. To open this project in Visual Studio, first edit the project file and fix the problem.

  4. moved item group before my existing content

    The project '.Web.fsproj' could not be opened because opening it would cause a folder to be rendered multiple times in the solution explorer. One such problematic item is 'Content\Site.css'. To open this project in Visual Studio, first edit the project file and fix the problem.

  5. Merged 2 itemgroups

    same error

    • Moved Content to cluster beside Content :

      <Content Include="Content\bootstrap-theme.css">
        <Paket>True</Paket>
      </Content>
      <Content Include="Content\bootstrap-theme.css.map">
        <Paket>True</Paket>
      </Content>
      <Content Include="Content\bootstrap-theme.min.css">
        <Paket>True</Paket>
      </Content>
      <Content Include="Content\bootstrap.css">
        <Paket>True</Paket>
      </Content>
      <Content Include="Content\bootstrap.css.map">
        <Paket>True</Paket>
      </Content>
      <Content Include="Content\bootstrap.min.css">
        <Paket>True</Paket>
      </Content>
      
      <Content Include="Content\Site.css" />
      

    The project 'Web.fsproj' could not be opened because opening it would cause a folder to be rendered multiple times in the solution explorer. One such problematic item is 'Scripts\main.js'. To open this project in Visual Studio, first edit the project file and fix the problem

  • Clustered it with the cited file:

          <Content Include="Scripts\bootstrap.js">
        <Paket>True</Paket>
      </Content>
      <Content Include="Scripts\bootstrap.min.js">
        <Paket>True</Paket>
      </Content>
      <Content Include="Scripts\jquery-2.1.1-vsdoc.js">
        <Paket>True</Paket>
      </Content>
      <Content Include="Scripts\jquery-2.1.1.js">
        <Paket>True</Paket>
      </Content>
      <Content Include="Scripts\jquery-2.1.1.min.js">
        <Paket>True</Paket>
      </Content>
      <Content Include="Scripts\jquery-2.1.1.min.map">
        <Paket>True</Paket>
      </Content>
    
      <Content Include="Scripts\main.js" />
    
    • Loads!
    • Questions:
      • Why is it not complaining about _._ items ?

      • where did the following come from (and why am I not seeing it on screen)

            <Content Include="web.config.transform">
        

        True

      • Guess: .fsproj files don't support web transforms and drop them on the ground!

      • It should be noted that I have seen the cited errors before in various contexts, i.e. the .fsproj handling in VS2013 consistently rejects stuff that's not grouped by folder (but it doesn't crash or anything)

      • It should be noted that NuGet cannot update my jquery project - it croaks with nullrefs etc.

@bartelink
Copy link
Member

@theimowski So firstly, it looks V promising.

Unfortunately, in the context of #59 it's obv a dealbreaker for me if every paket update means hand-editing.

Even if the VS fsproj support was to start handling discontinuous folder groupings, older VSes wouldnt have it (And I hate the bad feelings that enter my body when I go anywhere near http://connect.microsoft.com)

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!

@agross
Copy link
Contributor

agross commented Sep 11, 2014

@bartelink So I tested again with a sample nuget containing empty files:

content/bar.cs
content/bar.vb

The I created a new VB Class Library and added the nuget. VS "installed" bar.cs.

@bartelink
Copy link
Member

@agross ... So ? (I assume the .cs is set to None in the vbproj and it compiles?)

For avoidance of doubt:

  • For me that corrobotates my earlier assertions (which is no a surprise as we have lots of source packages in the field and its used from both langs across many versions of VS and/or NuGet)

@theimowski
Copy link
Member Author

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.
What do you do after creating the project? Remove everything that Nuget pulled and replace it with Paket dependencies?

@bartelink
Copy link
Member

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 "bootstrap" "> 3" though (and there's wierd nonsense with Twitter.bootstrap etc. which you need to ignore!`. Log stuff and I'll read in 3h...

@bartelink
Copy link
Member

Let me know if I can help - can concoct a repo if necessary.

I'm pretty sure the package with the id bootstrap depends on jquery, i.e. anything you're seeing is just incomplete NuGet package installs by the template.

@theimowski
Copy link
Member Author

I'll have some update later today

@bartelink
Copy link
Member

Cool, no rush, I'll be about whenever

@theimowski
Copy link
Member Author

I added sorting by the content file path.
Will have to handle the framework conditions too - should we put the content nodes in the same itemgroup as references, so that we don't have to repeat the When condition=... sequence?

And what about the "." files in Microsoft.Bcl package (not sure what they are meant for)

@bartelink
Copy link
Member

@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...

@bartelink
Copy link
Member

@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 .fsproj dictates that.

Any such sorting would need to be a matter of inserting the <Paket>true</Paket> nodes judiciously in the right places. NB but as mentioned above if this is only a bug in VFPT then it can and should prob be addressed there.

I'm not going to have time to research further for a while though.

Regarding the other detritus...

The sorting has made each _._ entry get a top level node. Don't think it did so before.

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 HRESULT into the crannies of our chips. Def don't want Paket to mutate files so they need to be filtered at some point in the processing.

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 ItemGroups ?

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:

  • should _._ files go in the fsproj
  • is it OK for folders not to be homogenous ? (i.e. can NuGet update bootstrap from 3.02 to 3.2 successfully OOTB)

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!)

@theimowski
Copy link
Member Author

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 .fsproj dictates that.

@bartelink But this feature is supposed only to deal with Content files from the Content dir in nupkg, so it should not be an issue unless these files are marked to be compiled.

@forki @agross What do you think?

@forki
Copy link
Member

forki commented Sep 12, 2014

At the moment the alphabetical order in the lockfile is for convenience of
the user. It's just nicer to read. No part of the system relies on it. I
would like to keep it as long as we can, but if we really need to introduce
a different order than I have no problem with that.
On Sep 12, 2014 9:17 PM, "Tomasz Heimowski" [email protected]
wrote:

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 .fsproj dictates that.

@bartelink https://github.com/bartelink But this feature is supposed
only to deal with Content files from the Content dir in nupkg, so it
should not be an issue unless these files are marked to be compiled.

@forki https://github.com/forki @agross https://github.com/agross
What do you think?


Reply to this email directly or view it on GitHub
#84 (comment).

@theimowski
Copy link
Member Author

The issue is about order of the Content files in proj file rather than lockfile.
*.fsproj requires that the Content nodes which point to file, say "Scripts\ *.js" and "Content\ *.css" cannot overlap.
Example:

<Content Include="Content\bootstrap.css" />
<Content Include="Content\mystyles.css" />
<Content Include="Scripts\bootstrap.js" />

works fine, but:

<Content Include="Content\bootstrap.css" />
<Content Include="Scripts\bootstrap.js" />
<Content Include="Content\mystyles.css" />

results in a VS error when loading the project (such as @bartelink described).
For *.csproj there is no such problem.

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.
The order-sensitive fsproj nature should not be a concern here, because it affects only the compilable files, but not the ones marked as 'Content'.

So the question seems to be: are we fine with Paket managing all Content nodes in project file and keeping them in order?

@bartelink
Copy link
Member

@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,
a) the entire project file gets reordered
b) the previously inert folders are now showing

@bartelink
Copy link
Member

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.

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)

To add content nodes to a fsproj from a nupkg, Paket has to manage all content nodes and sort them by the Include attr.
The order-sensitive fsproj nature should not be a concern here, because it affects only the compilable files, but not the ones marked as 'Content'.

Don't mess with my project file man! Don't make a paket install cause random diffs to my files and mess up my order. It Paket put it there (<Paket>true</Paket>), it can take it out and put it back somewhere else. Otherwise leave it alone. Don't mess with the whitespace. Don't reorder etc.

So the question seems to be: are we fine with Paket managing all Content nodes in project file and keeping them in order

Is there really no way to avoid this?

(... formulating an approach, 20m :))

@bartelink
Copy link
Member

If I have a NuPkgs:

  • A
    • MyAppStuff\XX\a.fs
    • Scripts\a.js
    • AMain.fs
  • B
    • MyAppStuff\YY\b.fs
    • Scripts\b.js
  • C
    • MyAppStuff\Xx\c.fs
    • Scripts\c.js
    • CMain.fs

and A depends on B depends on C and my project has:

AssemblyInfo.fs
MyAppStuff\XX\Myextensions.fs
index.html 
Helpers.fs 
Scripts\z.fs

And it depends on package A then:

  • the fs files in B can depend on stuff in C so the most-dependeds files need to go first
  • I depend on the package so it's stuff needs to go first
  • when stuff goes in, it needs to go:
    • before (that's why I depend on it)
    • but in group order if it's in a folder

So the packages yield:

MyAppStuff\XX\a.fs (1)
Scripts\a.js (1)
MyAppStuff\YY\b.fs (2)
Scripts\b.js (2)
MyAppStuff\XX\c.fs (3)
Scripts\c.js (3)

Recapping, my code has:

  1. AssemblyInfo.fs
  2. MyAppStuff\XX\Myextensions.fs
  3. index.html
  4. Helpers.fs
  5. Scripts\z.fs

The following needs to be upheld:

  • relative order of my code needs to be as is
  • stuff from packages comes before my stuff
  • stuff from folders in packages comes before stuff in same folders already in project

Therefore the result can only be:

  • CMain.fs
  • AMain.fs
  • AssemblyInfo.fs
    • MyAppStuff\Xx\c.fs (3)
    • MyAppStuff\YY\b.fs (2)
    • MyAppStuff\XX\a.fs (1)
  • MyAppStuff\XX\Myextensions.fs
  • index.html
  • Helpers.fs
    • Scripts\c.js (3)
    • Scripts\b.js (2)
    • Scripts\a.js (1)
  • Scripts\z.fs

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 (AMain.fs above) first.

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.

@bartelink
Copy link
Member

But, illustrating how messy it is, I made a slip in the above:

MyAppStuff\XX\c.fs (3)
MyAppStuff\YY\b.fs (2)
MyAppStuff\XX\a.fs (1)
MyAppStuff\XX\Myextensions.fs

Needs to reorder as

MyAppStuff\XX\c.fs (3)
MyAppStuff\XX\a.fs (1)
MyAppStuff\XX\Myextensions.fs
MyAppStuff\YY\b.fs (2)

i.e. MyAppStuff\YY\b.fs had to go after myextensions.fs because

  • MyAppStuff\XX\ stuff goes before user MyAppStuff\XX\ stuff
  • MyAppStuff\XX\c.fs (3) is the most dependent so it needs to go in first
  • MyAppStuff\YY\b.fs (2) needs to go somewhere after
  • but Myextensions.fs needs to say with the MyAppStuff\XX stuff even if in principle it sould go before MyAppStuff\XX\a.fs (1)

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.

@bartelink
Copy link
Member

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 :)

@theimowski
Copy link
Member Author

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.

@bartelink
Copy link
Member

Sounds good in principle. Wont get to test/look at code until much later tho' :(

@theimowski
Copy link
Member Author

Ok, even if this approach is satisfactory, there will be yet one more thing to consider:
as @agross pointed, files in nupkg Content can follow the same framework conditions rules as the lib directory.

When they do, shall we then copy all the stuff to the project directory? That would result in a mess.
One solution could be not to copy the stuff, but refer to it directly from the packages directory (similar to how library references are added) - but then these files wouldn't get grouped into folders in a VS solution explorer.

I'm pretty sure that NuGet in such cases simply determines which framework condition applies and copies just the respective files to project directory.

@bartelink
Copy link
Member

Yeah, NuGet copies and then does loads of work diffing, see some of my comments re #65

While a proper solution using Conditionals would be great in theory, in practice I reckon 1001 .targets files around the planet will explode - i.e. Web Publishing and all that sort of thing is very a shaky beast.

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!)

@theimowski
Copy link
Member Author

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 lib directories in a nupkg?

@bartelink
Copy link
Member

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:

  • inserting in order
  • no adding null ItemGroups

Perhaps #99 's flow can warn about packages that rely on more complex content behavior?

@forki
Copy link
Member

forki commented Sep 16, 2014

can you please rebase on current master?

@theimowski theimowski force-pushed the support_content_files_from_nuget branch from 6e92a65 to 7d89e18 Compare September 16, 2014 13:57
@theimowski theimowski force-pushed the support_content_files_from_nuget branch from 5076236 to 2280b1f Compare September 16, 2014 14:57
@theimowski
Copy link
Member Author

Rebased the branch.
Keep in mind that Framework-specific directories under Content dir are not handled yet

@forki forki merged commit 2280b1f into fsprojects:master Sep 16, 2014
@theimowski theimowski deleted the support_content_files_from_nuget branch September 16, 2014 15:17
@forki
Copy link
Member

forki commented Sep 16, 2014

uvgtei6

Let's take this as a start and work from here. Thanks for submitting.

@bartelink
Copy link
Member

@theimowski @forki Wrt #65 the current code in 0.2.0-alpha011 works:

  • the right files get in the right places
  • the inorder insert works v well, no bugs seen

V annoying problems:

  • Any webapi packages leave stacks of junk in folders that I keep having to manually delete ton preserve my sanity. I assume this is no surprise but is there something tracking this issue?

Minor nitpicks:

  • I don't like the way bootstrap's fonts folder always wants to be at the bottom after every update (I want it up the top of my .fsproj where I don't see it)

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

Successfully merging this pull request may close these issues.

None yet

4 participants