-
Notifications
You must be signed in to change notification settings - Fork 42
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
Dev/add condition for nuspec #284
Dev/add condition for nuspec #284
Conversation
configuration=$(Configuration); | ||
tfm=$(TargetFramework); | ||
packagetype=Dacpac; | ||
$(NuspecProperties) |
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.
This change allows for additional properties to be passed in when using the nuspec file.
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 faced the same issue (custom <NuspecProperties>
from csproj file being ignored) and reached the same conclusion. I can confirm this fixes the issue.
@@ -26,7 +26,7 @@ | |||
<EnableDefaultCompileItems>false</EnableDefaultCompileItems> | |||
<EnableDefaultEmbeddedResourceItems>false</EnableDefaultEmbeddedResourceItems> | |||
<NoPackageAnalysis>True</NoPackageAnalysis> | |||
<NuspecFile>$(MSBuildProjectName).nuspec</NuspecFile> | |||
<NuspecFile Condition="Exists('$(MSBuildProjectName).nuspec')">$(MSBuildProjectName).nuspec"</NuspecFile> |
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.
This removes the requirement for the nuspec file to exist allowing the msbuild sdk to generate the nuspec file
@@ -35,6 +35,19 @@ | |||
|
|||
<!-- For CPS/VS support. Importing in .props allows any subsequent targets to redefine this if needed --> | |||
<Target Name="CompileDesignTime" /> | |||
|
|||
<Target Name="SFCustomizePacking" AfterTargets="CoreBuild"> |
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.
this will add the single project dacpac or all dacpacs in the build folder to be added to the nuget package automatically.
Converted back to draft as there looks to be an issue with how I'm building the file paths when executed on azure devops. |
@@ -46,22 +46,24 @@ | |||
IncrementalClean; | |||
PostBuildEvent | |||
</CoreBuildDependsOn> | |||
<GenerateNuspecDependsOn>$(GenerateNuspecDependsOn);SetNuSpecProperties</GenerateNuspecDependsOn> | |||
<GenerateNuspecDependsOn>$(GenerateNuspecDependsOn);SetNuSpecProperties;_PackageDatabaseResults</GenerateNuspecDependsOn> | |||
<PackageType Condition="'$(PackageType)'==''" >dacpac</PackageType> |
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.
Hmm, I seem to remember that actually setting the package type breaks some things as you can no longer install these packages anymore. Regardless though we're not actually using this value currently.
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.
Without doing this some times the packages were being built incorrectly and didn't not have all of their children referenced correctly.
@@ -35,6 +35,35 @@ | |||
|
|||
<!-- For CPS/VS support. Importing in .props allows any subsequent targets to redefine this if needed --> | |||
<Target Name="CompileDesignTime" /> | |||
|
|||
<Target Name="_PackageDatabaseResults" Condition="'$(IsDatabaseProject)'=='True'" BeforeTargets="_GetPackageFiles"> |
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.
What does this section do exactly?
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.
This control the build dependency order. When I had multiple projects without this feature they were sometime building out of order and not collecting all the child dependencies when the build was running multithreaded.
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.
Oh, it also sets the dacpac as content so they are correctly included in the nuget package. (sorry it's been a month since I last looked at this.)
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.
Hate to be a Pain, but any idea when this will be ready to merge?? Does it work? Currently I need to pack these projects differently than standard projects until this feature is completed...
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.
It worked when I sent the pr a few months back. I have a work around for my own projects so if you guys don't want to accept the change and own it then no worries from me.
Fixes #283
These changes allow for generating nuspec instead of having to provide one.