Paket should not add <Paket> element in csporj file to cs files that are added via nuggets #1513

Closed
tsibelman opened this Issue Mar 13, 2016 · 31 comments

Comments

Projects
None yet
3 participants
@tsibelman
Contributor

tsibelman commented Mar 13, 2016

Following paket.dependencieswill

source https://www.nuget.org/api/v2/
nuget Microsoft.Orleans.Templates.Interfaces 

Adds empty orleans.codegen.cs file to csproj like this

    <Compile Include="Properties\orleans.codegen.cs">
      <Paket>True</Paket>
    </Compile>

This unexpected element hides file from Orleans code generator, and no code is created. The failure is detected only during run time, right now we manually remove this property.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 13, 2016

Member

Can't you use the no-content option?

Member

forki commented Mar 13, 2016

Can't you use the no-content option?

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 13, 2016

Contributor

Then I will need to add this file manually ?

Contributor

tsibelman commented Mar 13, 2016

Then I will need to add this file manually ?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 13, 2016

Member

Ok I don't really understand the problem. What is Orleans expecting?

Member

forki commented Mar 13, 2016

Ok I don't really understand the problem. What is Orleans expecting?

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 13, 2016

Contributor

It expecting to have this file in the csproject but it do not expect it to have<Paket>True</Paket>under it.

Contributor

tsibelman commented Mar 13, 2016

It expecting to have this file in the csproject but it do not expect it to have<Paket>True</Paket>under it.

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 13, 2016

Contributor

It should be just <Compile Include="Properties\orleans.codegen.cs"/>

Contributor

tsibelman commented Mar 13, 2016

It should be just <Compile Include="Properties\orleans.codegen.cs"/>

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 13, 2016

Member

Tbh this sounds like a bug in that tool

Member

forki commented Mar 13, 2016

Tbh this sounds like a bug in that tool

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 13, 2016

Contributor

I don't sure if it a bug, Paket is not a part of csproj XSD specification so varius tools that do XCD based validation could fail.

Contributor

tsibelman commented Mar 13, 2016

I don't sure if it a bug, Paket is not a part of csproj XSD specification so varius tools that do XCD based validation could fail.

@isaacabraham

This comment has been minimized.

Show comment
Hide comment
@isaacabraham

isaacabraham Mar 13, 2016

Contributor

@tsibelman What's the XSD spec say though - I wouldn't have thought that adding an extra element inside the Compile node would have caused an issue?

Contributor

isaacabraham commented Mar 13, 2016

@tsibelman What's the XSD spec say though - I wouldn't have thought that adding an extra element inside the Compile node would have caused an issue?

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 13, 2016

Contributor

It defined inside Microsoft.Build.Commontypes.xsd as you can see Compile has number of possible predefined child elements and Paket is not one of them.

 <xs:element name="Compile" substitutionGroup="msb:Item">
        <xs:annotation>
            <xs:documentation><!-- _locID_text="Compile" _locComment="" -->Source files for compiler</xs:documentation>
        </xs:annotation>        
        <xs:complexType>           
            <xs:complexContent>
                <xs:extension base="msb:SimpleItemType">
                    <xs:sequence minOccurs="0" maxOccurs="unbounded">                       
                        <xs:choice>
                            <xs:element name="SubType"/>
                            <xs:element name="DependentUpon"/>
                            <xs:element name="AutoGen">
                                <xs:annotation>
                                    <xs:documentation><!-- _locID_text="Compile_AutoGen" _locComment="" -->Whether file was generated from another file (boolean)</xs:documentation>
                                </xs:annotation>
                            </xs:element>                                 
                            <xs:element name="DesignTime"/>
                            <xs:element name="Link">
                                <xs:annotation>
                                    <xs:documentation><!-- _locID_text="Compile_Link" _locComment="" -->Notional path within project to display if the file is physically located outside of the project file's cone (optional)</xs:documentation>
                                </xs:annotation>
                            </xs:element>
                            <xs:element name="DesignTimeSharedInput"/>
                            <xs:element name="Visible">
                                <xs:annotation>
                                    <xs:documentation><!-- _locID_text="Compile_InProject" _locComment="" -->Display in user interface (optional, boolean)</xs:documentation>
                                </xs:annotation>
                            </xs:element>                                
                            <xs:element name="CopyToOutputDirectory"/>
                            <xs:element name="VBMyExtensionTemplateID"/>
                        </xs:choice>
                    </xs:sequence>
                    <!-- redefine Include just to give a specific description -->
                    <xs:attribute name="Include" type="xs:string" use="optional">
                        <xs:annotation>
                            <xs:documentation><!-- _locID_text="Compile_Include" _locComment="" -->Semi-colon separated list of source files (wildcards are allowed)</xs:documentation>
                        </xs:annotation>
                    </xs:attribute>                    
                </xs:extension>
            </xs:complexContent>
        </xs:complexType>
Contributor

tsibelman commented Mar 13, 2016

It defined inside Microsoft.Build.Commontypes.xsd as you can see Compile has number of possible predefined child elements and Paket is not one of them.

 <xs:element name="Compile" substitutionGroup="msb:Item">
        <xs:annotation>
            <xs:documentation><!-- _locID_text="Compile" _locComment="" -->Source files for compiler</xs:documentation>
        </xs:annotation>        
        <xs:complexType>           
            <xs:complexContent>
                <xs:extension base="msb:SimpleItemType">
                    <xs:sequence minOccurs="0" maxOccurs="unbounded">                       
                        <xs:choice>
                            <xs:element name="SubType"/>
                            <xs:element name="DependentUpon"/>
                            <xs:element name="AutoGen">
                                <xs:annotation>
                                    <xs:documentation><!-- _locID_text="Compile_AutoGen" _locComment="" -->Whether file was generated from another file (boolean)</xs:documentation>
                                </xs:annotation>
                            </xs:element>                                 
                            <xs:element name="DesignTime"/>
                            <xs:element name="Link">
                                <xs:annotation>
                                    <xs:documentation><!-- _locID_text="Compile_Link" _locComment="" -->Notional path within project to display if the file is physically located outside of the project file's cone (optional)</xs:documentation>
                                </xs:annotation>
                            </xs:element>
                            <xs:element name="DesignTimeSharedInput"/>
                            <xs:element name="Visible">
                                <xs:annotation>
                                    <xs:documentation><!-- _locID_text="Compile_InProject" _locComment="" -->Display in user interface (optional, boolean)</xs:documentation>
                                </xs:annotation>
                            </xs:element>                                
                            <xs:element name="CopyToOutputDirectory"/>
                            <xs:element name="VBMyExtensionTemplateID"/>
                        </xs:choice>
                    </xs:sequence>
                    <!-- redefine Include just to give a specific description -->
                    <xs:attribute name="Include" type="xs:string" use="optional">
                        <xs:annotation>
                            <xs:documentation><!-- _locID_text="Compile_Include" _locComment="" -->Semi-colon separated list of source files (wildcards are allowed)</xs:documentation>
                        </xs:annotation>
                    </xs:attribute>                    
                </xs:extension>
            </xs:complexContent>
        </xs:complexType>
@isaacabraham

This comment has been minimized.

Show comment
Hide comment
@isaacabraham

isaacabraham Mar 13, 2016

Contributor

Perhaps a Paket=true attribute rather than an element would work?

Contributor

isaacabraham commented Mar 13, 2016

Perhaps a Paket=true attribute rather than an element would work?

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 13, 2016

Contributor

I think it can work

Contributor

tsibelman commented Mar 13, 2016

I think it can work

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 13, 2016

Member

Nope. IIRC the attribute failed somewhere in normal csproj process.
On Mar 13, 2016 6:15 PM, "tsibelman" notifications@github.com wrote:

I think it can work


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

Member

forki commented Mar 13, 2016

Nope. IIRC the attribute failed somewhere in normal csproj process.
On Mar 13, 2016 6:15 PM, "tsibelman" notifications@github.com wrote:

I think it can work


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

@isaacabraham

This comment has been minimized.

Show comment
Hide comment
@isaacabraham

isaacabraham Mar 13, 2016

Contributor

Can you confirm this - remove the element, replace with attribute, does it work? Not saying that that's the right solution - Orleans is open source, we should find out what it's doing.

Contributor

isaacabraham commented Mar 13, 2016

Can you confirm this - remove the element, replace with attribute, does it work? Not saying that that's the right solution - Orleans is open source, we should find out what it's doing.

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 13, 2016

Contributor

Maybe this info should not be stored in csproj at all ? It already available in nugets.

Contributor

tsibelman commented Mar 13, 2016

Maybe this info should not be stored in csproj at all ? It already available in nugets.

@isaacabraham

This comment has been minimized.

Show comment
Hide comment
@isaacabraham

isaacabraham Mar 13, 2016

Contributor

@tsibelman it's needed for upgrades between packages I think.

Contributor

isaacabraham commented Mar 13, 2016

@tsibelman it's needed for upgrades between packages I think.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 13, 2016

Member

It's needed for paket to understand where people opt out of default
behavior. If you remove the flag our set it to false then we don't touch it
anymore.
On Mar 13, 2016 6:48 PM, "Isaac Abraham" notifications@github.com wrote:

@tsibelman https://github.com/tsibelman it's needed for upgrades
between packages I think.


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

Member

forki commented Mar 13, 2016

It's needed for paket to understand where people opt out of default
behavior. If you remove the flag our set it to false then we don't touch it
anymore.
On Mar 13, 2016 6:48 PM, "Isaac Abraham" notifications@github.com wrote:

@tsibelman https://github.com/tsibelman it's needed for upgrades
between packages I think.


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

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 13, 2016

Contributor

@forki if people choose option content: once it looks logical to not add this element at all, what do you think ?

Contributor

tsibelman commented Mar 13, 2016

@forki if people choose option content: once it looks logical to not add this element at all, what do you think ?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 14, 2016

Member

Mhm that could make sense. Yes
On Mar 13, 2016 19:20, "tsibelman" notifications@github.com wrote:

@forki https://github.com/forki if people choose option content: once
it looks logical to not add this element at all, what do you think ?


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

Member

forki commented Mar 14, 2016

Mhm that could make sense. Yes
On Mar 13, 2016 19:20, "tsibelman" notifications@github.com wrote:

@forki https://github.com/forki if people choose option content: once
it looks logical to not add this element at all, what do you think ?


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

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 14, 2016

Contributor

@forki great, this change would definatly help us

Contributor

tsibelman commented Mar 14, 2016

@forki great, this change would definatly help us

@forki forki closed this in 8d75dff Mar 14, 2016

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 14, 2016

Member

done. please try again

Member

forki commented Mar 14, 2016

done. please try again

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 14, 2016

Contributor

I tried I see that it still adding it

I declared content: once in the start of paket.dependencies and run this command paket convert-from-nuget -f

Contributor

tsibelman commented Mar 14, 2016

I tried I see that it still adding it

I declared content: once in the start of paket.dependencies and run this command paket convert-from-nuget -f

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 14, 2016

Member

yes. try to remove the reference from the csproj to make it believe that it starts from scratch.

Member

forki commented Mar 14, 2016

yes. try to remove the reference from the csproj to make it believe that it starts from scratch.

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 14, 2016

Contributor

Can it work on conversion from nugget ?

Contributor

tsibelman commented Mar 14, 2016

Can it work on conversion from nugget ?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 14, 2016

Member

did not try that

Member

forki commented Mar 14, 2016

did not try that

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 14, 2016

Contributor

I wanted it to work during conversion by adding paket.template with content: once, looks like it not working :(

Contributor

tsibelman commented Mar 14, 2016

I wanted it to work during conversion by adding paket.template with content: once, looks like it not working :(

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 14, 2016

Member

I made it a bit more agressive. The test shows it's not adding the paket tag any more. In fact it's removing it.

Member

forki commented Mar 14, 2016

I made it a bit more agressive. The test shows it's not adding the paket tag any more. In fact it's removing it.

forki added a commit that referenced this issue Mar 14, 2016

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 14, 2016

Contributor

It works perfectly thank you

Contributor

tsibelman commented Mar 14, 2016

It works perfectly thank you

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 20, 2016

Contributor

Looks like in latest version this behavior not working

Contributor

tsibelman commented Mar 20, 2016

Looks like in latest version this behavior not working

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 20, 2016

Member

Are you sure? We didn't change that.
On Mar 20, 2016 13:32, "tsibelman" notifications@github.com wrote:

Looks like in latest version this behavior not working


You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#1513 (comment)

Member

forki commented Mar 20, 2016

Are you sure? We didn't change that.
On Mar 20, 2016 13:32, "tsibelman" notifications@github.com wrote:

Looks like in latest version this behavior not working


You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#1513 (comment)

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 20, 2016

Contributor

Let's me double check it

Contributor

tsibelman commented Mar 20, 2016

Let's me double check it

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 20, 2016

Contributor

All good it was my mistake

Contributor

tsibelman commented Mar 20, 2016

All good it was my mistake

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