-
Notifications
You must be signed in to change notification settings - Fork 47
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
Issues with SQLCMD special characters going from 2.6.0 -> 2.6.1 / 3.0.0 #701
Comments
Yeah, that sounds like a good idea. |
I am not sure if that's the whole issue: we are seeing the same problem even when we are passing in the value ourselves using |
@MSACATS Our |
I'm not sure I follow -- we are using it for local purposes (e.g. unit testing), and we now are unable to pass in certain values during deployment |
Please use sqlpackage for complex local deployment. It is very easy to install as a .NET global tool |
We're also using https://learn.microsoft.com/en-us/dotnet/api/microsoft.sqlserver.dac.dacpackage programmatically which is breaking as well. I'm concerned that the issue is that something is going wrong in the dacpac itself but I can only practically observe the outside behavior. |
I apologize for the back and forth -- I'm trying to get to the bottom of exactly what works and what doesn't. Here are some simple csproj examples that maybe will make the situation clearer. Digging through the readme, it looks like we may also have had an issue with the Value vs DefaultValue, which I've fixed in the versions below though it didn't make any evident difference. 2.6.0: works, if values are quoted. Build output below. Notice how there are two rounds of adding sqlcmd variables, wrapped with single quotes vs not (Simply unzip this on top of the other one, and overwrite the csproj)
2.6.1. If you simply update the previous one to 2.6.1 (keep the quotes) compilation fails, because now 2.6.1 is itself wrapping those values in quotes so presumably they are now double quoted. MsBuildSqlTest (2.6.1 unchanged, keep quotes).zip
If we just remove the quotes, in this one the deployment fails quite early. Having removed the quotes from the csproj we can see it isn't putting any quotes into the command line which trips up the shell. MsBuildSqlTest (2.6.1 remove quotes, keep all SQLCMD variables).zip
If we comment out the especially problematic ones, we get further but deployment fails. You can see this in the command line at the bottom ("...exited with code 9009") MsBuildSqlTest (2.6.1 remove quotes and problematic SQLCMD variables).zip
Is the 2.6.1 change that automatically adds quotes (https://github.com/rr-wfm/MSBuild.Sdk.SqlProj/pull/459/files) incompatible with something else later in the pipeline that also should now add quotes? Since we can't add them ourselves anymore (#459) that is tripping things up? |
Taking that last one (MsBuildSqlTest (2.6.1 remove quotes and problematic SQLCMD variables).zip) and moving to 2.7.0, we can see it's assigning SQLCMD once now with single quotes. But the command line is not wrapping the values in quotes (
|
Yes, there is a "dotnet build" bug which I think my PR will fix. For publish, please use sqlpackage |
Would it be possible to publish a beta package (or however that works), so we can test our workflow's scenarios? |
Sure, when the PR gets approved, we could possibly do that |
We could publish the NuGet packages of the PR build so that @MSACATS can see if it resolves their issue |
For now we were able to work around it by massaging the value in the DefaultValue, so this is no longer an acute need if you don't want to clutter things up |
@jmezach Both recent bug fix PRs are merged now, should we do a preview or just go for 3.0.1 ? |
I'm fine with releasing a 3.0.1. I'll get the ball rolling :). |
Wonderful, thanks - looking forward to see if our version update check works as expected! |
Perhaps a 3.1.0 is more appropriate though. There are quite a bit of changes in there compared to the 3.0.0 release. |
Yes, including VS item template support - makes sense |
Similarly "2.6.1", in hindsight, was an understatement... |
@MSACATS You are the first person hit by this, so not sure I agree. |
@MSACATS 3.1.0 is out, could you do us a favor a try that? |
I'm going from the perspective of semantic versioning (https://semver.org/); changing behavior is not a n.n.X version bump. I'm sympathetic to the argument that whether things are quoted is undefined or not, but at the same time it would pretty clearly have implications if the value newly being wrapped (2.6.1) did have quotes in it. Strictly speaking I'd say that no longer emitting the default value in this release is a behavior change (major version upgrade) but again, this is if we're using semantic versioning
We will try it. Actually our workaround (wrapping the values with quotes when using sqlpackage) did not work: the resulting SQLCMD value now has quotes in it, which is also wrong. We'll let you know the results |
Preliminary testing looks good on our side. |
@MSACATS Thanks for the heads up. |
Hi, upon upgrading from 2.6.0 to 3.0.0 we started having problems deploying (specifically our pre/post-deploy scripts), and saw this change in 2.6.1: https://github.com/rr-wfm/MSBuild.Sdk.SqlProj/pull/459/files
We confirmed that we have problems with 2.6.1 as well, but they manifested differently. As such we started focusing on 3.0.0 for a minimal repro. We've confirmed that the issue is with SQLCMD variables with special characters.
We tried to develop a minimal repro, however we kept finding more and more problematic characters so it has become increasingly complex. I assume it's some same root cause. I'm attaching it here (MsBuildSqlTest.zip) with instructions below.
In the meantime, is there any known way in 3.0.0 to use SQLCMD values with special characters? (
(space), etc.)?
&
,<
,Previously we simply had to wrap the value in quotes. Space seems to work (TestValue3), but the rest seem to hopelessly break the command line.
Instructions:
The text was updated successfully, but these errors were encountered: