-
Notifications
You must be signed in to change notification settings - Fork 767
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
Update the SDK #1431
Update the SDK #1431
Conversation
@@ -2,7 +2,6 @@ | |||
|
|||
<PropertyGroup> | |||
<TargetFrameworks>netcoreapp3.0;net461</TargetFrameworks> | |||
<RuntimeIdentifiers>win7-x64;linux-x64;osx-x64</RuntimeIdentifiers> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? These were useful for testing the windows service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, they're useful for net461 maybe but we now make native exes by default. I was also just trying to see if this would fix the issue I was running into but it didn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was using them for standalone publish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? You had to do that in the past to get an exe, not you don't. What other reason did you have to self contain deploying this app?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Services run as different users without access to the repo local runtime. It's easier to deploy the runtime with the service than to tell the service how to find it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can publish self contained without putting RIDS in the prodject file and doing a RID restore. That's been the case since 2.x.
Anybody know wtf this is https://github.com/aspnet/Extensions/blob/92acd8b95901f1e8f47d3d128cb5f7c38b684644/src/Shared/src/Directory.Build.props#L9 |
This is how we hacked the SDK into producing '.Sources' packages. |
I see this was added: @dsplaisted How do we build a package source only package? Is there anyway to disable this? |
What is a package source only package? The check was added to make sure that when you specify "no build", it doesn't actually run the Build target. I'm not sure there's a point in disabling the check, if you are getting the error and want to disable the check, I think you'd get the same result by no longer specifying NoBuild. However, I don't know anything about the hack Nate refers to to produce source packages, so that could be an exception. |
6e350c1
to
c00275a
Compare
A source package is a .nupkg which only contains *.cs content files, no .dll. I think <NoBuild> was set to suppress the C# compiler from ever running. These projects are using a .csproj as a hack to produce a .nupkg since we don’t have the ability to pack a nuspec directly.
|
I believe it is possible by now to use the Pack target to create a .nupkg from a .nuspec. I think if you're getting the NoBuildRequested error, it means that some target that was running had a dependency on the Build target, so essentially you would be compiling the code anyway if the error wasn't there. |
I’m not aware of any changes to nuget and packing. Sure, you can call the pack target and provide a nuspec….but you still need a .csproj file in order to invoke the Pack target, right? That’s what these dummy .csproj files are for.
|
Yes, you still need a dummy .csproj. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Further investigation into the CodeCheck.ps1 failures on ref assemblies has turned up a combination of problems:
- The GenAPI tool doesn't consider LangVersion when generating source GenAPI generates invalid C# arcade#2555
- New versions of Roslyn are emitting IsReadOnlyAttribute for auto-properties Compiler is emitting
IsReadOnlyAttribute
for getter only auto properties when not targeting 8.0 roslyn#35113
@davidfowl we could try updating LangVersion to target 8.0. If that doesn't work, we'll need to hold off on this change until we find a workaround or fix.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s), but failed to run 1 pipeline(s). |
PR checks are passing now |
FINALLY! 😄 |
* Update the SDK * Remove rids * set GeneratePackgeOnBuild to true \n\nCommit migrated from dotnet/extensions@b473a78
* Update the SDK * Remove rids * set GeneratePackgeOnBuild to true Commit migrated from dotnet/extensions@b473a78
* Update the SDK * Remove rids * set GeneratePackgeOnBuild to true Commit migrated from dotnet/extensions@b473a78
* Update the SDK * Remove rids * set GeneratePackgeOnBuild to true Commit migrated from dotnet/extensions@b473a78
* Update the SDK * Remove rids * set GeneratePackgeOnBuild to true Commit migrated from dotnet/extensions@b473a78
I'd like to try out netstandard2.1 in various extensions projects