-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use subdirectories in publish for native #675
Conversation
This sounds good. Does it run fine on the E2E test? We should co-ordinate checking in since CoreRT publish needs to have --native-subdirectory flag for --ilcpath to not break CoreRT CI. For your future PR, corerun/corehost has to be copied as well and the appdep build hack to drop inside this folder. |
@schellap Waiting on results here for E2E. |
This should help in packaging for cross compilation. The CI should pass once dotnet/cli#675 is ready to go. Cc @gkhanna79
@schellap I am seeing that on windows, the subdirectories are included and things are just fine. On Ubuntu, the files are getting flat packed. Can you quickly confirm that the Ubuntu NuGet Packages are structured to take advantage of the subdirectories? This will help diagnose what's going wrong here. |
@brthor, can you give permissions to your fork so I can buddy test for you? Or if you find it easy, you can clone dotnet/corert and switch to native-subdir branch and drop a CLI (your stage2) inside /bin/tools/cli/. |
Are your path separators good? |
@schellap added as a collaborator |
7909df8
to
68a4f89
Compare
@brthor, I'll test this on our repo once you are ready to go with CLI CI. |
Error: Unable to load DLL 'jitinterface': The specified module could not be found. @schellap Do you know how this dlll is being loaded? |
This dll is moved to the path It looks like this dll is picked up by ILCompiler, @schellap does it need accompanying changes? |
This needs to be PInvoked by ILC. Is there a way it can land next to ilc like ryujit? Eventually it might come from ryujit like package. |
The dll would need to not be in a subdirectory in the nuget package, so it doesn't get published to a subdirectory when publishing Think this: <Text><![CDATA[ <file src="../%(Identity)" target="runtimes/$(NuPkgRid)/native/lib/dotnet/%(Identity)" /> ]]></Text> Would need the path changed: <Text><![CDATA[ <file src="../%(Identity)" target="runtimes/$(NuPkgRid)/native/%(Identity)" /> ]]></Text> |
Ah, I see. I thought the lib/dotnet/.dll was required by NuGet and publish for it to be picked up. I was just going to ask if we can omit special folders like lib, sdk that are "known names" and split on "native/content". I will get a PR and new packages out for you! |
@brthor, having it at root of |
At that point we'd need to take it out of the native folder anyways wouldn't we? |
Yeah, I realized that later. Anyway, we would have to make too many changes, so it is a non-issue. |
How does this look for you? 1.0.4-prerelease-00005 |
I don't think we need to remove from the native folder as it is a native library. We would however change the layout of the SDK package to use the appropriate content folders. Note: jitinterface.dylib is not content but a PInvoke binary which should just do what other PInvoke binaries do. How is always using The real issue is that |
The changes, at a highlevel, look fine to me. However, have you looked into the OSX failures? Also, please do squash the commits. |
@gkhanna79 OSX failures are related to the discussion above. This would happen on Ubuntu as well but the test is skipped for the moment. Will definitely squash the commits, thanks. @schellap I see what you are saying with this dll that it will not be part of the contents and will remain in For example in Whereas in I think removing the |
@dotnet-bot test this please should incorporate that latest pushed package |
This gives support for files to publish in subdirectories from dependency nuget packages. Additionally Change Native Compilation to consume these paths.
69adfc5
to
89de0c2
Compare
@dotnet-bot test Ubuntu Debug Build please |
@Sridhar-MS This centos error seems to be caused by an outdated stage0 package. Can I safely ignore this for now, or should I wait on it for the merge? |
@brthor Let me quickly create an updated stage0 for centos. Merging this at this point will fail all the PRs. |
@dotnet-bot test CentOS7.1 Debug Build please |
@dotnet-bot test CentOS7.1 Release Build please |
@brthor The CentOS builds are passing, go ahead with the merge. Meanwhile I am also setting up the internal CI to push latest CentOS tarballs to azure blob storage. |
Excellent 👍 |
Use subdirectories in publish for native
…tem-error-664 Show duplicate item errors due to implicit items in design-time builds
First pass on including subdirectories for use in Native.
PTAL @schellap @gkhanna79