NuGet/Home

TargetPath not set correctly for contentFiles

Closed this issue · 17 comments

clairernovotny commented

When trying to use contentFiles with UWP project, the files were not copied to the expected subdirectory.

Here's the nuspec:
https://gist.github.com/onovotny/5d007fd323a8393412b9

I expected Doc2.txt to be in the AppX\Resources\Doc2.txt directory. Instead, it was in AppX\Doc2.txt

Looking at a diagnostic build, it seems like the TargetPath metadata was not set correctly:

1>        C:\Users\oren\.nuget\packages\TestWhitelist\1.2.0\contentFiles\any\uap10.0\Resources\Doc2.txt
1>                NuGetItemType=Content
1>                NuGetPackageId=TestWhitelist
1>                NuGetPackageVersion=1.2.0
1>                OriginalItemSpec=C:\Users\oren\.nuget\packages\TestWhitelist\1.2.0\contentFiles\any\uap10.0\Resources\Doc2.txt
1>                TargetPath=Doc2.txt
yishaigalatzer commented

Looks like a bug to me, the flatten attribute is false by default which means the path should be preserved.

CC @jasonmalinowski @emgarten

emgarten commented

Flatten here pertains to the output folder, this is for the path within the assembly or appx. We should support this but it may need more design work.

yishaigalatzer commented

I don't understand, this is a documented feature, why would it need more design work?

clairernovotny commented

AppX should preserve the output directory structure, I don't think there's specific work for that. I don't think even the output dir is correct here based on the TargetPath meta.

dotMorten commented

Any update on this? This is a complete showstopper for me. I need various shader and data files deployed in the correct folder structure. This makes it really hard to move away from ExtensionSDKs

jasonmalinowski commented

@onovotny, was there a particular reason you wrote your nuspec as you did instead of

<files include="any/uap10.0/Resources/**/*.*" buildAction="None" copyToOutput="true" flatten="false" />

We do set TargetPath in this case for what you were expecting. For Content types specifically I'm not sure if there's other side effects you need from that type or if this is sufficient. When we designed this feature we intended copyToOutput="true" to be the case when you do need to control the output structure, and the buildAction stuff when you're embedding things like .cs files that need to be passed to the compiler, and it doesn't matter what "path" that's coming from. If there's a reason for either you or @dotMorten why switching to None w/ copyToOutput="true" isn't sufficient for you, this isn't a hard change for us to make. I admit my view from the low-level MSBuild stuff here isn't affording me a good view of the end-to-end you're trying to achieve.

dotMorten commented

That does it for me. Thanks!

clairernovotny commented

There may also be an issue with Android resources not generating correctly when used as contentFiles.

To repro, create a package with the following
https://gist.github.com/onovotny/5d007fd323a8393412b9#file-gistfile1-txt-L23
https://gist.github.com/onovotny/5d007fd323a8393412b9#file-gistfile1-txt-L17

then create an android project that uses project.json instead of packages.config. Now add that package and build.

In theory, the android project's resource.cs file should be updated at build time with numeric ID's that correspond to each AndroidResource Item. They're missing from the packaged ones though. This may be an issue with the Android task that uses AndroidResource, but I'm not sure.

jasonmalinowski commented

Fair enough. I've made dotnet/NuGet.BuildTasks#15 to specify the TargetPath. We'll give that some testing this upcoming week to make sure this doesn't regress anything else.

jasonmalinowski commented

And just an update for @onovotny, @dotMorten and others, due to a few last minute adventures we weren't able to get this into Update 3, but we'll hopefully get it into a future Visual Studio "15" Preview. I'll also look into getting you test bits so you can verify the proposed fix does actually fix everything you expect it to.

robv8r commented

Was this ever resolved?

emgarten commented

@robv8r looks like the PR was not merged.

jasonmalinowski commented

@emgarten Yeah, I got pulled over to other things. We still want to take this? Happy to dust it off or hand it off to somebody else. :-)

emgarten commented

I think this is still useful for UWP which uses this build task. I'll let others chime in also if they are hitting this issue still.

rrelyea commented

@natidea - PackageReference scenarios could still use this work. A PR that wasn't merged.
Also, we'd want to ensure PackageRef csproj and PackageRef sdkproj both work well here.
/cc @jainaashish