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

PostDeployment script with SQLCMD :R is failing build #103

Open
rehnerik opened this issue May 25, 2022 · 17 comments · May be fixed by #497
Open

PostDeployment script with SQLCMD :R is failing build #103

rehnerik opened this issue May 25, 2022 · 17 comments · May be fixed by #497
Assignees
Labels
area: build sdk Related to Microsoft.Build.Sql SDK bug Something isn't working triaged

Comments

@rehnerik
Copy link

rehnerik commented May 25, 2022

  • SqlPackage or DacFx Version: 0.1.3 -preview
  • .NET Framework (Windows-only) or .NET Core: 6.0.3
  • Environment (local platform and source/target platforms): Windows 10, Azure Data Studio

Steps to Reproduce:

  1. Create new SQL Server Database project in azure data studio.
  2. Add folder and files that is not supposed to be built.
  3. Change your sqlproj file to look something like this
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" ToolsVersion="4.0">
  <Sdk Name="Microsoft.Build.Sql" Version="0.1.3-preview" />
  <PropertyGroup>
    <Name>DBName</Name>
    <ProjectGuid>{961FE558-959F-46CE-A911-5FE5B0779667}</ProjectGuid>
    <DSP>Microsoft.Data.Tools.Schema.Sql.Sql150DatabaseSchemaProvider</DSP>
    <ModelCollation>1033, CI</ModelCollation>
  </PropertyGroup>
  <Target Name="BeforeBuild">
    <Delete Files="$(BaseIntermediateOutputPath)\project.assets.json" />
  </Target>
  <ItemGroup>
    <PostDeploy Include="Script.PostDeployment.sql"/>
  </ItemGroup>
  <ItemGroup>
    <Build Remove="Users\somescript1.sql"/>
    <Build Remove="Users\somescript2.sql"/>
    <Build Remove="Users\somescript3.sql"/>
    <Build Remove="Users\somescript4.sql"/>
    <Build Remove="Users\somescript5.sql"/>
  </ItemGroup>
</Project>
  1. Add PostDeployment with SQLCMD :r targeting your files that is removed from the build.
  2. Try build project with: dotnet build /p:NetCoreBuild=true from cmd or build action from Azure Data Studio
  3. Build fails due to post deployment script.
  4. Output:
...DBName\Users\somescript1.sql(3,6,3,6): Build error SQL72008: Variable x is not defined. [...DBName\DBName.sqlproj]
...DBName\Users\somescript2.sql(12,25,12,25): Build error SQL72008: Variable x is not defined. [...DBName\DBName.sqlproj]
...DBName\Users\somescript3.sql(12,25,12,25): Build error SQL72008: Variable x is not defined. [...DBName\DBName.sqlproj]
...DBName\Users\somescript4.sql(12,25,12,25): Build error SQL72008: Variable x is not defined. [...DBName\DBName.sqlproj]
...DBName\Users\somescript5.sql(19,22,19,22): Build error SQL72007: The syntax check failed 'Incorrect syntax near ].' in the batch near: [...DBName\DBName.sqlproj]

(DacFx/SqlPackage/SSMS/Azure Data Studio)

@rehnerik rehnerik added the bug Something isn't working label May 25, 2022
@zijchen zijchen added the area: build sdk Related to Microsoft.Build.Sql SDK label May 25, 2022
@zijchen zijchen self-assigned this May 25, 2022
@zijchen
Copy link
Member

zijchen commented May 25, 2022

Hi @rehnerik I'm not able to reproduce this issue. I followed the steps to remove files from build but they are excluded as expected. Could you share a ZIP of a min-repro of your project? Thank you.

@rehnerik
Copy link
Author

Hi @zijchen
After further testing it's actually the post deployment script that is failing the build.

My PostDeployment script looks like this

USE $(DatabaseName)

:r .\Users\somescript1.sql
:r .\Users\somescript2.sql
:r .\Users\somescript3.sql
:r .\Users\somescript4.sql
:r .\Users\somescript5.sql

I have the script specified as post depoly in sqlproj as described in the docs: Pre/post-deployment scripts that are specified by the PreDeploy or PostDeploy tags are automatically excluded from build.

Like this:
<PostDeploy Include="Script.PostDeployment.sql"/>

@rehnerik rehnerik changed the title Build Remove not working as intended PostDeployment script is failing build May 27, 2022
@rehnerik
Copy link
Author

@zijchen I've updated first comment aswell.

@zijchen
Copy link
Member

zijchen commented Jun 6, 2022

@rehnerik everything you're describing seems to be correct, I'm not sure why they're still included in the build. Could you share your sqlproj file so I can take a look?

@rehnerik
Copy link
Author

rehnerik commented Jun 7, 2022

@zijchen

Here's sqlproj file:

<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build">
  <Sdk Name="Microsoft.Build.Sql" Version="0.1.3-preview" />
  <PropertyGroup>
    <Name>Issue103</Name>
    <ProjectGuid>{8E29F6E6-7263-4B44-9C26-D20634E15492}</ProjectGuid>
    <DSP>Microsoft.Data.Tools.Schema.Sql.Sql150DatabaseSchemaProvider</DSP>
    <ModelCollation>1033, CI</ModelCollation>
  </PropertyGroup>
  <Target Name="BeforeBuild">
    <Delete Files="$(BaseIntermediateOutputPath)\project.assets.json" />
  </Target>
  <ItemGroup>
    <PostDeploy Include="Script.PostDeployment.sql" />
  </ItemGroup>
  <ItemGroup>
    <Build Remove="Users\somescript1.sql" />
    <Build Remove="Users\somescript2.sql" />
    <Build Remove="Users\somescript3.sql" />
    <Build Remove="Users\somescript4.sql" />
    <Build Remove="Users\somescript5.sql" />
  </ItemGroup>
</Project>

@zijchen
Copy link
Member

zijchen commented Jun 7, 2022

I am able to reproduce this now, thanks for all the details.

The problem seems to be dotnet build picking up the files that are specified as :r script.sql in the post deployment script.

I tried this in SSDT and it works, so this is specific to the SDK. Looking into this...

@zijchen zijchen changed the title PostDeployment script is failing build PostDeployment script with SQLCMD :R is failing build Jun 7, 2022
@rehnerik
Copy link
Author

rehnerik commented Jun 8, 2022

@zijchen Yes it works in SSDT. Thanks for looking into this.

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 8, 2022

@zijchen A lot of work went into fixing this in your "sister" project: rr-wfm/MSBuild.Sdk.SqlProj@01929bd

@alexirion10
Copy link

+1 I'm running into this issue now trying to convert to the SDK as well

@dlaplante75
Copy link

dlaplante75 commented Jul 5, 2022

FYI, having a similar issue here. if you need another example.

I have those additional scripts setup as though because that's how it was setup by ssdt
oddly, I just now tried (didn't know you could do that....) and that worked but the files were now absent from the "project explorer".

putting both and does exactly what I would expect coming from a ssdt world (files in the project explorer, build succeeds and the additional scripts included in the "final" PostDeploy.sql file inside the DacPac

image

@rehnerik
Copy link
Author

@zijchen Any news on this issue?

@zijchen
Copy link
Member

zijchen commented Aug 19, 2022

I haven't had a chance to look into this yet, but I'll prioritize this as it appears to be a common issue.

@zijchen
Copy link
Member

zijchen commented Sep 7, 2022

Ok here is the behavior I'm observing. I think the parsing of pre/post-deployment scripts is supposed to happen, we have some 72xxx error codes that are specific for pre/post scripts.
image

There are some buggy behavior I am observing around when they get parsed though:

  • When the included scripts are at the project root, they aren't parsed (this seems like a bug).
  • When the included scripts are in a folder, they get parsed and errors are emitted if there are invalid syntax or SQLCMD variables that are not added to the project.

I am able to reproduce the above with both SDK-style and in SSDT 2019/2022 too, so I don't think this is limited to the SDK.

@zijchen
Copy link
Member

zijchen commented Sep 7, 2022

Ok here is the behavior I'm observing. I think the parsing of pre/post-deployment scripts is supposed to happen, we have some 72xxx error codes that are specific for pre/post scripts. image

There are some buggy behavior I am observing around when they get parsed though:

  • When the included scripts are at the project root, they aren't parsed (this seems like a bug).
  • When the included scripts are in a folder, they get parsed and errors are emitted if there are invalid syntax or SQLCMD variables that are not added to the project.

I am able to reproduce the above with both SDK-style and in SSDT 2019/2022 too, so I don't think this is limited to the SDK.

Actually this is not entirely true. I think there's a bug with our batch parser. It seems to be ignoring any invalid syntax at the beginning of any batch, so the ordering of the included scripts made a difference.

At sqlproj build time, any file included by SQLCMD :r are read, parsed, and added to the postdeploy.sql in the DACPAC recursively. If you unzip a dacpac and inspect it, you will notice all the :r are replaced by the script contents.

The parsing that happens at this step will generate the errors seen above, @rehnerik the 72008 error from your original post should go away if you add x as a SQLCMD variable to the project.

The batch parser error I'm seeing ignores invalid syntax until a valid token, which meant the ordering of the included scripts mattered. This led me to assume there was a bug where root level scripts aren't being parsed correctly.

If I have the following T-SQL in an included script, build passes:

alsdjflajioewo
CREATE TABLE Table1 (Col1 INT NULL)

This also works:

USE [$(DatabaseName)]
GO

aweijoawjfoiwao
CREATE TABLE Table1 (Col1 INT NULL)
GO

But this generates error (expected):

CREATE TABLE Table1 (Col1 INT NULL)
oweiafjiojefaj

@NenoLoje
Copy link

NenoLoje commented Apr 3, 2024

@zijchen Any updates on this issue?

@zijchen
Copy link
Member

zijchen commented Aug 27, 2024

@NenoLoje Do you have a sample project that reproduces the issue? I believe the original issue was due to misconfigured SQLCMD variables.

Also the issue with parser ignoring invalid syntax is incorrect too. The parser treats single-token strings as identifiers, as they could be a stored procedure being executed, hence why no error is thrown.

@robertmclaws
Copy link

A note for anyone reading this thread, you can simplify pre- and post-build script exclusions outlined in the OP with this:

<Build Remove="Post-Deployment\*.sql" />
<None Include="Post-Deployment\*.sql" />

Fun fact, if you change the file extension from .dacpac to .zip, you can extract the contents and use BeyondCompare to see if there are discrepancies in the output scripts.

@zijchen zijchen linked a pull request Sep 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build sdk Related to Microsoft.Build.Sql SDK bug Something isn't working triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants