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

Pack target misses .pri files for Windows targets when multitargeting #4136

Closed
srivatsn opened this issue Dec 21, 2016 · 20 comments
Closed

Pack target misses .pri files for Windows targets when multitargeting #4136

srivatsn opened this issue Dec 21, 2016 · 20 comments
Assignees
Milestone

Comments

@srivatsn
Copy link

@srivatsn srivatsn commented Dec 21, 2016

Moved from dotnet/sdk#508 filed by @onovotny

When using msbuild /t:pack to generate a NuPkg on a multi-targeted project that includes Windows outputs (uap10.0, win8, wpa81, etc) that also contain a .pri file, the generated NuSpec used to create the package does not include the .pri file. That will lead to errors loading resources.

To repro:
Check out this commit from Zeroconf:
https://github.com/onovotny/Zeroconf/tree/39d47e8fea22a1eb19938392e0bc8f903181cf02

Run the following command msbuild /t:pack /p:Configuration=Release

Look at the NuSpec, it's missing the .pri files.

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Jan 17, 2017

@onovotny Could you update the build steps for the above project? I am getting the following errors:

"D:\temp\pri-files\Zeroconf\Zeroconf\Zeroconf.csproj" (Build target) (1:2) ->
(CheckForDuplicateItems target) ->
  C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Sdk
.DefaultItems.targets(123,5): error : Duplicate 'Compile' items were included. The .NET SDK includes 'Compile' items fr
om your project directory by default. You can either remove these items from your project file, or set the 'EnableDefau
ltCompileItems' property to 'false' if you want to explicitly include them in your project file. The duplicate items we
re: 'AdapterInformation.cs'; 'AsyncLock.cs'; 'Dns\Header.cs'; 'Dns\Question.cs'; 'Dns\Record.cs'; 'Dns\RecordA.cs'; 'Dn
s\RecordAAAA.cs'; 'Dns\RecordNSEC.cs'; 'Dns\RecordPTR.cs'; 'Dns\RecordReader.cs'; 'Dns\RecordSRV.cs'; 'Dns\RecordTXT.cs
'; 'Dns\RecordUnknown.cs'; 'Dns\Request.cs'; 'Dns\Response.cs'; 'Dns\RR.cs'; 'Dns\Structs.cs'; 'DomainService.cs'; 'INe
tworkInterface.cs'; 'Properties\AssemblyInfo.cs'; 'ServiceAnnouncement.cs'; 'ZeroconfRecord.cs'; 'ZeroconfResolver.Asyn
c.cs'; 'ZeroconfResolver.cs'; 'ZeroconfResolver.Observable.cs' [D:\temp\pri-files\Zeroconf\Zeroconf\Zeroconf.csproj]```
@clairernovotny
Copy link

@clairernovotny clairernovotny commented Jan 17, 2017

@rohit21agrawal I've just pushed an update to the dev branch with the updated glob.

@clairernovotny
Copy link

@clairernovotny clairernovotny commented Jan 31, 2017

Should this be in RC3 or only in RTM? I tested RC3 and I don't see the pri files in the package....

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Jan 31, 2017

This is in RTM

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Jan 31, 2017

The latest CLI will have this fix though

@clairernovotny
Copy link

@clairernovotny clairernovotny commented Feb 9, 2017

should this be in RC4? I just tested RC4 and still don't see the .pri file in the package.

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Feb 9, 2017

@onovotny yes this should be in RC4.

can you do a /v:diag build on your UWP project and give me the values of $(IncludeProjectPriFile), $(ProjectPriFile) and $(_TargetPathsToAssembiles) ?

@clairernovotny
Copy link

@clairernovotny clairernovotny commented Feb 9, 2017

Will do but you should be able to repro with Zeroconf. check out the dev branch of https://github.com/onovotny/zeroconf. Then in the src\zeroconf directory, do msbuild /t:restore zeroconf.csproj then msbuild /t:pack zeroconf.csproj.

Looking at the nupkg in the bin directory, the lib\uap10.0 folder doesn't have the pri.

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Feb 9, 2017

looking

@clairernovotny
Copy link

@clairernovotny clairernovotny commented Feb 9, 2017

IncludeProjectPriFile is true
ProjectPriFileName = Zeroconf.pri

_TargetPathsToAssembiles doesn't exist

@clairernovotny
Copy link

@clairernovotny clairernovotny commented Feb 9, 2017

Keep in mind that it's a multi-targeted project; UWP is one of the outputs

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Feb 9, 2017

_TargetPathsToAssemblies would exist when you call pack.
Also ProjectPriFullPath instead ProjectPriFileName

@clairernovotny
Copy link

@clairernovotny clairernovotny commented Feb 9, 2017

can you repro on my project? I tried it again and get the same result.

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Feb 9, 2017

yeah i am installing all the required workloads

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Feb 10, 2017

@onovotny turns out the .pri file is being included only when project has TargetFramework , and not TargetFrameworks . Figuring out why this is happening.

@rohit21agrawal rohit21agrawal changed the title Pack target misses .pri files for Windows targets Pack target misses .pri files for Windows targets when multitargeting Feb 10, 2017
@rohit21agrawal rohit21agrawal modified the milestones: 4.0 RTM, 4.0 RC4 Feb 10, 2017
@rrelyea
Copy link
Contributor

@rrelyea rrelyea commented Feb 10, 2017

We'll consider the fix when we see the code, etc... Please work on a fix.

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Feb 10, 2017

@rrelyea i have sent out a PR for this.

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Mar 2, 2017

i have merged the fix for this bug into dev branch for 4.3 milestone, but reopening this under 4.1 milestone for visibility.

CC: @rrelyea @DoRonMotter

@rohit21agrawal rohit21agrawal reopened this Mar 2, 2017
@clairernovotny
Copy link

@clairernovotny clairernovotny commented Mar 2, 2017

What's it going to take to get this into 4.1? Seems like a small fix?

@rohit21agrawal
Copy link
Contributor

@rohit21agrawal rohit21agrawal commented Mar 10, 2017

checked into 4.1.0-rtm via NuGet/NuGet.Client@0d68700

@rrelyea rrelyea closed this Mar 10, 2017
@clairernovotny clairernovotny mentioned this issue Jan 31, 2020
2 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.