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

Remove ExtraPackageVersionProps now that these version numbers should match without our intervention. #14769

Closed

Conversation

crummel
Copy link

@crummel crummel commented Oct 14, 2022

Fixes dotnet/source-build#3071.

At one point the versions for MS.AspNetCore.App.Runtime and MS.AspNetCore.App.Runtime.platform didn't match and we needed this workaround. Now they are produced with the same unstable versions so we should be able to remove these lines.

@tmds and @ayakael, would you mind giving this a try in your environments?

Copy link
Member

@MichaelSimons MichaelSimons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve - pending validation on Alpine.

@ayakael
Copy link

ayakael commented Oct 15, 2022

Approve - pending validation on Alpine.

I tested this on dotnet6 as I hadn't applied @tmds changes yet to that version, and it looks good. I'd have to fix my musl patches for dotnet7 to test there, but I don't expect there to be any difference. The backport process is clean as well.

@tmds
Copy link
Member

tmds commented Oct 17, 2022

This looks like a proper fix to me (dotnet/source-build#3121).

CI doesn't pass:

/tarball/src/installer/artifacts/source-build/self/src/src/redist/targets/GenerateLayout.targets(389,5): error MSB4018: (NETCORE_ENGINEERING_TELEMETRY=Build) The "DownloadFile" task failed unexpectedly.
System.AggregateException: One or more errors occurred. (Could not find file '/tarball/artifacts/obj/x64/Release/blob-feed/assets/aspnetcore-runtime-internal-7.0.0-rc.2.22476.2-linux-x64.tar.gz'.)
 ---> System.IO.FileNotFoundException: Could not find file '/tarball/artifacts/obj/x64/Release/blob-feed/assets/aspnetcore-runtime-internal-7.0.0-rc.2.22476.2-linux-x64.tar.gz'.
File name: '/tarball/artifacts/obj/x64/Release/blob-feed/assets/aspnetcore-runtime-internal-7.0.0-rc.2.22476.2-linux-x64.tar.gz'
   at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirError)
   at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, UnixFileMode openPermissions, Int64& fileLength, UnixFileMode& filePermissions, Func`4 createOpenException)
   at System.IO.FileSystem.CopyFile(String sourceFullPath, String destFullPath, Boolean overwrite)
   at Microsoft.DotNet.Arcade.Sdk.DownloadFile.DownloadFromUriAsync(String uri) in /_/src/Microsoft.DotNet.Arcade.Sdk/src/DownloadFile.cs:line 108
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at Microsoft.DotNet.Arcade.Sdk.DownloadFile.Execute() in /_/src/Microsoft.DotNet.Arcade.Sdk/src/DownloadFile.cs:line 90
   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask)

I think the sources that get built for the tarball may not actually contain the change introduced by this PR.
In #14549 I made the change using a patch against installer.

@ayakael
Copy link

ayakael commented Oct 17, 2022

This looks like a proper fix to me (#14492 (comment)).

CI doesn't pass:

/tarball/src/installer/artifacts/source-build/self/src/src/redist/targets/GenerateLayout.targets(389,5): error MSB4018: (NETCORE_ENGINEERING_TELEMETRY=Build) The "DownloadFile" task failed unexpectedly.
System.AggregateException: One or more errors occurred. (Could not find file '/tarball/artifacts/obj/x64/Release/blob-feed/assets/aspnetcore-runtime-internal-7.0.0-rc.2.22476.2-linux-x64.tar.gz'.)
 ---> System.IO.FileNotFoundException: Could not find file '/tarball/artifacts/obj/x64/Release/blob-feed/assets/aspnetcore-runtime-internal-7.0.0-rc.2.22476.2-linux-x64.tar.gz'.
File name: '/tarball/artifacts/obj/x64/Release/blob-feed/assets/aspnetcore-runtime-internal-7.0.0-rc.2.22476.2-linux-x64.tar.gz'
   at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirError)
   at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, UnixFileMode openPermissions, Int64& fileLength, UnixFileMode& filePermissions, Func`4 createOpenException)
   at System.IO.FileSystem.CopyFile(String sourceFullPath, String destFullPath, Boolean overwrite)
   at Microsoft.DotNet.Arcade.Sdk.DownloadFile.DownloadFromUriAsync(String uri) in /_/src/Microsoft.DotNet.Arcade.Sdk/src/DownloadFile.cs:line 108
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at Microsoft.DotNet.Arcade.Sdk.DownloadFile.Execute() in /_/src/Microsoft.DotNet.Arcade.Sdk/src/DownloadFile.cs:line 90
   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask)

I think the sources that get built for the tarball may not actually contain the change introduced by this PR. In #14549 I made the change using a patch against installer.

Testing against NET 7, indeed this still causes issues for aspnetcore. Patch by @tmds works better.

@crummel
Copy link
Author

crummel commented Oct 19, 2022

Due to how close to the wire we're getting I think this is going to have to go into servicing. Closing this in favor of #14549 and an eventual backport of it.

@crummel crummel closed this Oct 19, 2022
@tmds
Copy link
Member

tmds commented Oct 20, 2022

Closing this in favor of #14549 and an eventual backport of it.

I had removed the patch when I re-targeted main.

What you have here should work.

I think the Build_Tarball_x64 fails because it does a fresh check-out of installer for the tarball, which doesn't yet include the changes you've made in this PR.

In #14549 I had implemented it as a patch against installer, so it would be applied to the tarball on the CI builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants