MSBuild.Sdk.SqlProj icon indicating copy to clipboard operation
MSBuild.Sdk.SqlProj copied to clipboard

[Question] Is there a way to set up the project to only build if source files change?

Open kevcrooks opened this issue 4 years ago • 50 comments

At the moment, if I set up the project then run a build it will say "1 succeeded".

If I run the build again without changing any files, I'd like the build to be skipped (i.e. "1 up-to-date" instead of "1 succeeded" in Visual Studio)

Is there a way to do this, or is it not possible, because we always need to reproduce the dacpac file?

kevcrooks avatar Sep 22 '20 13:09 kevcrooks

We do currently support incremental builds. I'm not sure about what controls the specific verbiage coming out of MSBuild, but if no files are changed, the project isn't re-built.

jeffrosenberg avatar Sep 22 '20 13:09 jeffrosenberg

@kevcrooks It should not recreate the dacpac file on every build, but I do believe that Visual Studio still reports 1 succeeded instead of 1 up-to-date. I'm not entirely sure why that is. Perhaps the way we've implemented it doesn't match with the expectations that Visual Studio has.

Is this a problem for you? If so I would like to understand why. We shouldn't be "waisting" resources if that's your concern.

jmezach avatar Sep 22 '20 13:09 jmezach

Thank you, I'll check this, the main reason I'd like this is so I can add a dotnet publish to our Database as a post-build step (before my dependent projects), but I think it was getting into a build loop because the dotnet publish was doing another build.

I may not be doing this the best way, but I noticed that I was previously using 1.6.0, so I'll see if this works now I'm on 1.8.1 - please let me know if there's a better way for me to go about a publish as part of the build though

kevcrooks avatar Sep 22 '20 13:09 kevcrooks

I've just checked and I'm seeing the same on 1.8.1.

The main thing I'd like is to be able to deploy the Database as part of a build - but only if the build has changed (so I'm not needing to deploy every time). At the moment I've tried adding:

dotnet publish --no-build --no-restore --no-dependencies

as a post-build step, but this is triggering a recurring set of builds.

My backup plan is to create a separate project that only calls dotnet publish as a build step, and make all my other projects dependent on that one instead, but I wanted to check if there's a nicer way to do this? And also this has the issue of deploying the Database every time, even if the files haven't changed.

kevcrooks avatar Sep 22 '20 13:09 kevcrooks

@jmezach looking at the incremental build docs again, I think the difference here is that what's being skipped is our CoreCompile target, not the entire build. I'm not sure whether there's a way to actually skip the entire build, though, because we need some of the earlier targets in the build to generate the list of inputs to check.

jeffrosenberg avatar Sep 22 '20 14:09 jeffrosenberg

If you're doing a dotnet publish and the .dacpac hasn't been build yet, it should do a dotnet build for you. If the .dacpac has been build and none of the source files have changed it should just use the existing .dacpac that's already there.

If that's not the case, please let us know because then there might be a bug.

jmezach avatar Sep 22 '20 14:09 jmezach

@jeffrosenberg Hmm, maybe that could be resolved by making the Build target dependent on the targets we need and then put the Inputs and Outputs on that target instead.

jmezach avatar Sep 22 '20 14:09 jmezach

@jmezach do you know if there's a workaround I can do on my end in the meantime - to get a build/publish to from the project only when the files change (I.e. so that any other project depending on it will run the build/publish step as part of its build, but only when needed)?

kevcrooks avatar Sep 28 '20 09:09 kevcrooks

@kevcrooks It looks like you're specifically experiencing an issue with projects that depend on each other. We currently don't support project references yet (see #40) so I'm wondering how you've set things up. Could you share some of the relevant sections of your project file(s)?

jmezach avatar Sep 28 '20 09:09 jmezach

@jmezach apologies I don't think I explained quite right.

My SqlProj doesn't need any dependencies, but I have a separate (e.g. console app) project that I'd like to depend on the SqlProj project, so that when I build my console app, the SqlProj project will dotnet build and dotnet publish if needed.

Here's some sample projects I've set up: One issue I'm having is that the post-build publish step doesn't work, since it will cause the project to build in a loop, so I'd like a way around this:

<Project Sdk="MSBuild.Sdk.SqlProj/1.8.1">
    <PropertyGroup>
        <TargetFramework>netstandard2.0</TargetFramework>
        <SqlServerVersion>Sql130</SqlServerVersion>
        <TargetServerName>(localdb)\mssqllocaldb</TargetServerName>
        <TargetDatabaseName>TestDB</TargetDatabaseName>
    </PropertyGroup>
    <Target Name="PostBuild" AfterTargets="PostBuildEvent">
      <Exec Command="dotnet publish --no-build --no-restore --no-dependencies" />
    </Target>
</Project>
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.1</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <Compile Include="Program.fs" />
  </ItemGroup>

  <ItemGroup>
    <ProjectReference Include="..\Database1.csproj" />
  </ItemGroup>
    
  <ItemGroup>
    <PackageReference Include="FSharp.Data.SqlClient" Version="2.0.6" />
  </ItemGroup>

</Project>

The reason I'd like to have another project depend on the SqlProj, is that there's an F# connector will link to a live localhost DB to provide intellisense. But this requires the most up-to-date version, i.e. I need to perform dotnet publish when theSqlProj changes.

Ideally I'd like to not do a deployment on every build, but just when I've changed something in the SqlProj, if you have a way to do this?

kevcrooks avatar Sep 28 '20 11:09 kevcrooks

Ah, I think I understand what you want now. Basically you're looking for incremental publish (on top of incremental build) in that the publish is only performed if the .dacpac has actually changed. We already support incremental build, so I guess we could have the .dacpac as an Inputs on the Publish target.

But we'd also need an Outputs and I'm not quite sure what the best way would be to implement that. To do it correctly we'd need to check the actual database to see when that has last been updated which isn't trivial to do. Or perhaps we could write a file to the obj folder on each publish and we use that as the Outputs. That way it wouldn't republish the database if that file is newer (or the same) as the .dacpac. I think that'll work, but it won't account for any changes made to the actual database of course.

@ErikEJ @jeffrosenberg Would love to hear your thoughts on this design?

jmezach avatar Sep 28 '20 12:09 jmezach

I'm not convinced we would want to add an incremental publish step here, since as you said, we'd really need to check the actual database, which I think is too heavyweight an approach. One thing I'd like to point out here is that by its nature, the publish step is already incremental -- it reads the state of the database and applies only those updates needed.

I think what the OP really wants is to not bother moving on to the publish step at all based on whether a new build is needed. I think that means updating our incremental build to skip the entire build rather than just a single target, so that MSBuild views the build as "skipped", rather than "successful". No idea whether @jmezach's suggestion from last week will work for that, but it's worth a try.

In the meantime, @kevcrooks, I think the only viable "workaround" here is to just run the publish every time. I understand that's not ideal for you, but because of the way the publish works, if there's nothing to be done that step will just become a no-op.

jeffrosenberg avatar Sep 28 '20 12:09 jeffrosenberg

To clarify my comments above, my concern about the proposed design is that the last-updated time of the .dacpac file isn't necessarily a good way to judge whether a publish is needed -- we would really need to check the database. For example, what if the user is trying to publish to multiple environments? The first publish would succeed, but each subsequent publish would be skipped as not needed.

jeffrosenberg avatar Sep 28 '20 12:09 jeffrosenberg

Agree on "publish every time" - that is how DaxFX works - "desired state configuration"

ErikEJ avatar Sep 28 '20 13:09 ErikEJ

I think what the OP really wants is to not bother moving on to the publish step at all based on whether a new build is needed. I think that means updating our incremental build to skip the entire build rather than just a single target, so that MSBuild views the build as "skipped", rather than "successful". No idea whether @jmezach's suggestion from last week will work for that, but it's worth a try.

In the meantime, @kevcrooks, I think the only viable "workaround" here is to just run the publish every time. I understand that's not ideal for you, but because of the way the publish works, if there's nothing to be done that step will just become a no-op.

@jeffrosenberg yes that's correct, it's what I'm aiming for. However dotnet publish as a post-build step won't work at the moment until your suggestion with the incremental build is done. At the moment a publish triggers a build, which in turn triggers a publish (if set as a post-build step) and so on... so I may need to wait until the incremental build fix is complete to get this?

kevcrooks avatar Sep 28 '20 13:09 kevcrooks

I guess we could try the suggestion I made earlier and ship that as a beta version for @kevcrooks to try out. I'm fairly busy at the moment though, so I don't know when I'll have the time to make that change, so if there's anyone else who has time for this let me know and then I'll assign the issue.

jmezach avatar Sep 28 '20 14:09 jmezach

@jmezach I can give it a try today or tomorrow. If that doesn't work, I don't know how much additional time I can commit to playing around with this, but I'll give your suggestion a try and see what happens!

jeffrosenberg avatar Sep 28 '20 14:09 jeffrosenberg

@jeffrosenberg That would be awesome. I've assigned the issue to you for now. If it doesn't work let us know and we'll have another look at it.

jmezach avatar Sep 28 '20 14:09 jmezach

So far, I've managed to get the binlog to show that the Build step is skipped, but it hasn't resolved the problem of a circular reference when targeting the PostBuildEvent. I have a few more thoughts, so I'm going to stick with it for a bit longer, but might be a couple of days before I can get back to it.

jeffrosenberg avatar Sep 29 '20 22:09 jeffrosenberg

@kevcrooks I'm looking more closely at the MSBuild documentation, and I don't think <Target Name="PostBuild" AfterTargets="PostBuildEvent"> will work the way you expect it to. Based on the target build order, I believe your post-build publish step will run every time:

Before the target is executed, its Inputs attribute and Outputs attribute are compared. If MSBuild determines that any output files are out of date with respect to the corresponding input file or files, then MSBuild executes the target. Otherwise, MSBuild skips the target.

After the target is executed or skipped, any other target that lists it in an AfterTargets attribute is run. [Emphasis mine]

After quite a bit of testing, I believe the only way we can avoid this infinite loop is to remove the Publish target's dependence on Build. I could be open to that, because I think having a project file set up like this (a build with a post-build publish action) is pretty common and will lead many people into this sort of loop. On the other hand, if we don't double-check that Build has run before Publish, we risk the publish having unexpected effects, as expected changes won't be published if the user hasn't explicitly run Build first.

I'm not too familiar with dotnet build conventions, so I'm not sure what makes the most sense. Either way, I think a couple of sentences of documentation about the build order would be helpful. @jmezach @ErikEJ any thoughts about the Publish target's dependence on Build?

~~Regardless of what we decide on that, there is one change I tested while working on this that I think I'd like to send a PR for -- moving the incremental build Inputs and Outputs from the CoreCompile target to the Build target.~~ (EDIT: I played around with this a little more and I've changed my mind, apparently when doing this the CoreCompile target still runs)

jeffrosenberg avatar Oct 02 '20 12:10 jeffrosenberg

@jeffrosenberg Yes you're right, I've just tested with the following app, and dotnet build runs both the pre-build and post-build steps every time. (However there's no infinite build loop in this example, because it's able to detect everything is up-to-date) EDIT: The code below just runs powershell, but I checked it avoids the infinite loop when the post-build step contains dotnet publish too

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.1</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <Compile Include="Program.fs" />
  </ItemGroup>

  <Target Name="PostBuild" AfterTargets="PostBuildEvent">
    <Exec Command="powershell Write-Host &quot;post-build&quot;" />
  </Target>

  <Target Name="PreBuild" BeforeTargets="PreBuildEvent">
    <Exec Command="powershell Write-Host &quot;pre-build&quot;" />
  </Target>

</Project>

However from Visual Studio, once it detects that the project is "1 up-to-date" it doesn't run the pre- or post-builds after the first time: image

image

Most of the time I'll be using Visual Studio (and my motivation was to avoid unnecessary deployments when developing), so I think as long as we can get the "1 up-to-date" fix for Visual Studio builds then that would work great for me.

And although the post-build publish would always happen with dotnet build, at least it would only happen once (without the build loop), so it would work fine on our build server too.

kevcrooks avatar Oct 02 '20 13:10 kevcrooks

@jeffrosenberg I'm not entirely sure that we actually need the Publish target to depend on the Build target. We could perhaps just check if the output file exists (and fix #68 in the process ;)) and throw an error if it doesn't instructing people to run a dotnet build first.

Also, I'm not sure about this, but it might be that dotnet will implicitly run build before a publish. That might be something that is build into dotnet itself. I believe it does the same thing with restore when you run build. This is just conjecture on my part though, I haven't verified that ;).

jmezach avatar Oct 02 '20 14:10 jmezach

Just tried what I described above, but it does seem that if we remove the dependency on the Build target the dotnet publish command doesn't run dotnet build behind the scenes. Interestingly there is a --no-build argument for dotnet publish which would suggest it does do the build implicitly (since it works similarly to --no-restore). I guess I'll have to do some more digging to see what's going on.

jmezach avatar Oct 02 '20 17:10 jmezach

Looks like the up-to-date check is actually a Visual Studio feature that is separate from the MSBuild logic. I found this document that describes this feature. We might be able to implement that, although I'm not sure if this will solve the problem at hand.

jmezach avatar Oct 02 '20 18:10 jmezach

Nice catch, that definitely explains what we're seeing

jeffrosenberg avatar Oct 02 '20 18:10 jeffrosenberg

We should probably add a separate issue for tracking the implementation of the Visual Studio up-to-date-check. I might be able to throw something together this weekend and ship a beta version. Then we can ask @kevcrooks to evaluate whether it solves this issue. If it does that's nice, if it doesn't we can keep this open and explore further options.

jmezach avatar Oct 02 '20 18:10 jmezach

Thanks @jmezach, yes happy to test any beta version next week

kevcrooks avatar Oct 02 '20 19:10 kevcrooks

@kevcrooks I've just uploaded a beta version that should fix the up-to-date check issue (for which I've created a separate issue, see #74). Could you give that a try and see if this also resolves your issue? Should be just a matter of editing your .csproj and swapping out the version number in the Sdk attribute on the <Project> element. Please let us know how that goes.

jmezach avatar Oct 04 '20 08:10 jmezach

@kevcrooks I've just uploaded a beta version that should fix the up-to-date check issue (for which I've created a separate issue, see #74). Could you give that a try and see if this also resolves your issue? Should be just a matter of editing your .csproj and swapping out the version number in the Sdk attribute on the <Project> element. Please let us know how that goes.

@jmezach I just tried the following version: <Project Sdk="MSBuild.Sdk.SqlProj/1.9.0-beta.5.g60d0950ebe"> with the post-build:

  <Target Name="PostBuild" AfterTargets="PostBuildEvent">
    <Exec Command="dotnet publish --no-build --no-restore --no-dependencies" />
  </Target>

but I didn't notice any difference in behaviour (the build was looping continuously still) - was this the beta version you meant?

kevcrooks avatar Oct 05 '20 12:10 kevcrooks

@kevcrooks Yes, that seems to be the correct version. So I guess that just implementing the fast up-to-date check doesn't solve your issue then.

I had a look at what dotnet publish does behind the scenes, but it seems that the --no-build flag doesn't actually do anything. Looking at the source code it seems to have the same behaviour as --no-restore. So I can understand why the loop is happening, because dotnet publish will just run the Publish target which has a dependency on the Build target.

One thing you could try perhaps is to have your PostBuild target call the Publish target using <CallTarget>. That way you stay within the same MSBuild process so it should have figured out that the Build target has already run and shouldn't run it again.

jmezach avatar Oct 05 '20 13:10 jmezach