Update manifest names #127

JunTaoLuo · 2017-05-25T02:38:58Z

Addresses #124 to clarify which manifests are problematic when error message is displayed.

DamianEdwards · 2017-05-25T19:25:35Z

build/repo.targets

@@ -69,7 +69,7 @@

    <ItemGroup>
      <PackageStoreManifestFiles Include="$(PackageCacheOutputPath)**\artifact.xml">
-        <DestinationFile>manifest.$(RID).xml</DestinationFile>
+        <DestinationFile>aspnetcore.manifest.$(RID).xml</DestinationFile>


This needs to match what I put in the issue, i.e. aspnetcore-store-$(VERSION)-$(RID).xml

JunTaoLuo · 2017-05-25T23:23:39Z

build/repo.targets

@@ -69,7 +76,8 @@

    <ItemGroup>
      <PackageStoreManifestFiles Include="$(PackageCacheOutputPath)**\artifact.xml">
-        <DestinationFile>manifest.$(RID).xml</DestinationFile>
+        <TimestampDestinationFile>aspnetcore-store-$(TimestampVersion)-$(RID).xml</TimestampDestinationFile>
+        <NoTimestampDestinationFile>aspnetcore-store-$(NoTimestampVersion)-$(RID).xml</NoTimestampDestinationFile>


Actually this isn't needed, we only produce the Timestamp package here. I need to update Coherence-Signed which hacks out a NoTimestamp version. https://github.com/aspnet/Coherence-Signed/blob/dev/dnx.msbuild#L482

JunTaoLuo · 2017-05-25T23:24:11Z

src/Microsoft.AspNetCore.All/Microsoft.AspNetCore.All.csproj

+    </PropertyGroup>
+
+    <Exec Command="powershell.exe -command &quot;(Get-Content $(ManifestTargetsFile)).replace('__MANIFEST_VERSION__','$(VersionPrefix)-$(VersionSuffix)') | Set-Content $(ManifestTargetsFile)&quot;" Condition="'$(OS)' == 'Windows_NT'"/>
+    <Exec Command="sed -i -e &quot;s/__MANIFEST_VERSION__/$(VersionPrefix)-$(VersionSuffix)/g&quot; $(ManifestTargetsFile)" Condition="'$(OS)' != 'Windows_NT'"/>


Not technically required since we only build the packages on windows in our CI

JunTaoLuo · 2017-05-25T23:25:38Z

src/Microsoft.AspNetCore.All/Microsoft.AspNetCore.All.csproj

@@ -22,4 +22,13 @@
    <Content Include="build\**\*.targets" PackagePath="%(Identity)" />
  </ItemGroup>

+  <Target Name="UpdateManifestVersionNumbers" BeforeTargets="PrepareForBuild">


I think PackageDependsOn is a better strategy for achieving what we want here. Will test it out.

natemcmaster · 2017-05-25T23:59:06Z

src/Microsoft.AspNetCore.All/build/PublishWithAspNetCoreTargetManifest.targets

@@ -4,7 +4,7 @@
  </PropertyGroup>

  <PropertyGroup Condition="'$(PublishWithAspNetCoreTargetManifest)'=='true'">
-    <TargetManifestFiles>$(TargetManifestFiles);$(MSBuildThisFileDirectory)manifest.win7-x64.xml;$(MSBuildThisFileDirectory)manifest.win7-x86.xml;$(MSBuildThisFileDirectory)manifest.osx-x64.xml;$(MSBuildThisFileDirectory)manifest.linux-x64.xml</TargetManifestFiles>
+    <TargetManifestFiles>$(TargetManifestFiles);$(MSBuildThisFileDirectory)aspnetcore-store-__MANIFEST_VERSION__-win7-x64.xml;$(MSBuildThisFileDirectory)aspnetcore-store-__MANIFEST_VERSION__-win7-x86.xml;$aspnetcore-store-__MANIFEST_VERSION__-osx-x64.xml;$(MSBuildThisFileDirectory)aspnetcore-store-__MANIFEST_VERSION__-linux-x64.xml</TargetManifestFiles>


Can we use glob instead so you don't have to worry about sed?

It looks like you just add this to the TargetManifestFileList item group.

<ItemGroup Condition="'$(PublishWithAspNetCoreTargetManifest)'=='true'"> <TargetManifestFileList Include="$(MSBuildThisFileDirectory)aspnetcore-store-*.xml" /> </ItemGroup >

cref https://github.com/dotnet/sdk/blob/43090565482d9a10481ff2298fb168fc819fa430/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Publish.targets#L33

That would be better actually. I'll go with that and try out the final packages to see if it works.

I am seeing the errors during publish, seems like globs do not work:

(RunResolvePublishAssemblies target) -> C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: The "ResolvePublishAssemblies" task failed unexpectedly. [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: System.Xml.XmlException: Cannot open 'file:///C:/Users/johluo/.nuget/packages/microsoft.aspnetcore.all/2.0.0-preview2-t0048339c4/build/*.xml'. The Uri parameter must be a file system relative or absolute path. ---> System.ArgumentException: Illegal characters in path. [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: Parameter name: path [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.IO.Path.GetFullPath(String path) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.Xml.XmlSystemPathResolver.GetEntity(Uri uri, String role, Type typeOfObjectToReturn) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: --- End of inner exception stack trace --- [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.Xml.XmlSystemPathResolver.GetEntity(Uri uri, String role, Type typeOfObjectToReturn) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.Xml.XmlTextReaderImpl.FinishInitUriString() [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.Xml.XmlTextReaderImpl..ctor(String uriStr, XmlReaderSettings settings, XmlParserContext context, XmlResolver uriResolver) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.Xml.XmlReaderSettings.CreateReader(String inputUri, XmlParserContext inputContext) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.Xml.XmlReader.Create(String inputUri, XmlReaderSettings settings) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at System.Xml.Linq.XDocument.Load(String uri, LoadOptions options) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at Microsoft.NET.Build.Tasks.StoreArtifactParser.Parse(String filterFile) [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at Microsoft.NET.Build.Tasks.ResolvePublishAssemblies.ExecuteCore() [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at Microsoft.NET.Build.Tasks.TaskBase.Execute() [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute() [C:\gh\tp\TestStore\TestStore.csproj] C:\Users\johluo\.dotnet\x64\sdk\2.0.0-preview2-006127\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Publish.targets(252,5): error MSB4018: at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__25.MoveNext() [C:\gh\tp\TestStore\TestStore.csproj]

Took a brief look but couldn't figure out why MSBuild isn't working as I expected. Will revert to using explicit file names.

natemcmaster · 2017-05-26T00:02:26Z

src/Microsoft.AspNetCore.All/Microsoft.AspNetCore.All.csproj

+      <ManifestTargetsFile>$(MSBuildThisFileDirectory)build\PublishWithAspNetCoreTargetManifest.targets</ManifestTargetsFile>
+    </PropertyGroup>
+
+    <Exec Command="powershell.exe -command &quot;(Get-Content $(ManifestTargetsFile)).replace('__MANIFEST_VERSION__','$(VersionPrefix)-$(VersionSuffix)') | Set-Content $(ManifestTargetsFile)&quot;" Condition="'$(OS)' == 'Windows_NT'"/>


This was the easiest way I could put the exact version number inside the the target file.

natemcmaster · 2017-05-26T15:26:01Z

Globs don't work in PropertyGroup. You need to use ItemGroup. Try this code:

  <ItemGroup Condition="'$(PublishWithAspNetCoreTargetManifest)'=='true'">
    <TargetManifestFileList Include="$(MSBuildThisFileDirectory)aspnetcore-store-*.xml" />
  </ItemGroup >

natemcmaster

Talked in person. It looks like we'll need an SDK with dotnet/sdk#1164 first before the ItemGroup approach will work. In the meantime, let's go with this. I'm not sure how fast we'll get a version of the SDK that includes dotnet/sdk#1245 or if it will make it into preview2

JunTaoLuo requested a review from DamianEdwards May 25, 2017 02:38

dnfclas added the cla-already-signed label May 25, 2017

DamianEdwards suggested changes May 25, 2017

View reviewed changes

JunTaoLuo force-pushed the johluo/rename-manifests branch from 438cf99 to f35f450 Compare May 25, 2017 23:20

JunTaoLuo requested review from pranavkm and natemcmaster May 25, 2017 23:21

JunTaoLuo commented May 25, 2017

View reviewed changes

natemcmaster reviewed May 25, 2017

View reviewed changes

natemcmaster reviewed May 26, 2017

View reviewed changes

DamianEdwards approved these changes May 26, 2017

View reviewed changes

natemcmaster approved these changes May 26, 2017

View reviewed changes

Update manifest names

1619166

JunTaoLuo force-pushed the johluo/rename-manifests branch from eb7eb05 to 1619166 Compare May 26, 2017 18:43

JunTaoLuo merged commit 1619166 into dev May 26, 2017

JunTaoLuo deleted the johluo/rename-manifests branch May 26, 2017 19:16

JunTaoLuo mentioned this pull request Jun 6, 2017

Use file globbing to resolve manifests #143

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update manifest names #127

Update manifest names #127

Uh oh!

JunTaoLuo commented May 25, 2017 •

edited

Loading

Uh oh!

DamianEdwards May 25, 2017

Uh oh!

JunTaoLuo May 25, 2017

Uh oh!

JunTaoLuo May 25, 2017

Uh oh!

JunTaoLuo May 25, 2017

Uh oh!

natemcmaster May 25, 2017

Uh oh!

natemcmaster May 26, 2017

Uh oh!

JunTaoLuo May 26, 2017

Uh oh!

JunTaoLuo May 26, 2017

Uh oh!

natemcmaster May 26, 2017

Uh oh!

JunTaoLuo May 26, 2017

Uh oh!

natemcmaster commented May 26, 2017

Uh oh!

natemcmaster left a comment

Uh oh!

Uh oh!

Update manifest names #127

Update manifest names #127

Uh oh!

Conversation

JunTaoLuo commented May 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

natemcmaster commented May 26, 2017

Uh oh!

natemcmaster left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JunTaoLuo commented May 25, 2017 •

edited

Loading