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

Issue 1225 #1226

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Issue 1225 #1226

wants to merge 3 commits into from

Conversation

ErikEJ
Copy link

@ErikEJ ErikEJ commented Mar 26, 2024

fixes #1225

Description: Look for SqlPackage.exe presence and version in the Visual Studio DAC folder introduced in later VS versions

Documentation changes required: N

Added unit tests: N

Attached related issue: #1225

Checklist:

  • Version was bumped - please check that version of the extension, task or library has been bumped.
  • Checked that applied changes work as expected.

Verified with

. "$PSScriptRoot/SqlPackageOnTargetMachines.ps1"
Get-SqlPackageOnTargetMachine

fixes microsoft#1225

tested and confirmed locally with:

. "$PSScriptRoot/SqlPackageOnTargetMachines.ps1"

Get-SqlPackageOnTargetMachine
@ErikEJ
Copy link
Author

ErikEJ commented Mar 26, 2024

@dzsquared FYI! Please have a look 😄

@b3go
Copy link

b3go commented Mar 26, 2024

Tested and works for 2019 and 2022. Output:

PS C:\WINDOWS\system32> Get-SqlPackageOnTargetMachine
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\Extensions\Microsoft\SQLDB\DAC\150\SqlPackage.exe
PS C:\WINDOWS\system32> Get-SqlPackageOnTargetMachine
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\Extensions\Microsoft\SQLDB\DAC\SqlPackage.exe

@ErikEJ
Copy link
Author

ErikEJ commented Mar 26, 2024

@b3go Thanks for confirming!

@ErikEJ
Copy link
Author

ErikEJ commented Mar 26, 2024

@dzsquared wonder if this is enough to also affect the azuresqldeployment task?

@ErikEJ
Copy link
Author

ErikEJ commented Mar 26, 2024

Looks like there is duplicate code in another repo: https://github.com/microsoft/azure-pipelines-tasks/blob/master/Tasks%2FSqlAzureDacpacDeploymentV1%2FFindSqlPackagePath.ps1#L348-L352

@dzsquared
Copy link

Often the version from the dac.msi installer (C:\Program Files\Microsoft SQL Server\160\DAC\bin) will be newer than the VS version, causing it to be selected over the VS version - probably in part why we haven't seen this behavior crop up much.

Just fyi - the version in azure-pipelines-tasks has another PR open on it to enable more versatility - microsoft/azure-pipelines-tasks#19648

Finally, the dotnet tool version of Sqlpackage was mentioned by @b3go and indeed we're hoping to integrate it with these tasks in the future - but this change doesn't need to be held up for that larger adjustment.

thank you @ErikEJ!

@ErikEJ
Copy link
Author

ErikEJ commented Mar 27, 2024

probably in part why we haven't seen this behavior crop up much.

Ah, that's why there have been no issues from hosted agents.

@PaulVrugt
Copy link

Looks like there is duplicate code in another repo: https://github.com/microsoft/azure-pipelines-tasks/blob/master/Tasks%2FSqlAzureDacpacDeploymentV1%2FFindSqlPackagePath.ps1#L348-L352

I'm a bit confused. We are using the SqlAzureDacpacDeployment@1 task, and it seems to take the C:\Program Files\Microsoft SQL Server\160\DAC\bin sqlpackage version over the visual studio one, even if the visual studio one is newer. Which caused microsoft/DacFx#427 for us. It seems that this PR doesn't fix this issue. But is there an active PR somewhere to fix the SqlAzureDacpacDeployment@1 as well?

@ErikEJ
Copy link
Author

ErikEJ commented Mar 27, 2024

@PaulVrugt Are you still affected by this - I think the version in C:\Program Files\Microsoft SQL Server\160\DAC\bin has been updated on all the Hosted agents now. But I still plan to do a PR in the other repo (to at least align the logic)

@ErikEJ
Copy link
Author

ErikEJ commented Mar 27, 2024

@dzsquared @PaulVrugt Gave up on the other task, too many moving parts

@PaulVrugt
Copy link

@PaulVrugt Are you still affected by this - I think the version in C:\Program Files\Microsoft SQL Server\160\DAC\bin has been updated on all the Hosted agents now. But I still plan to do a PR in the other repo (to at least align the logic)

Well yes, because we are not using hosted agents, we use private agents. Once I updated visual studio on the private agent image, the error started appearing. We now fixed it by updating sqlpackage on the private agent manually, but this would have been prevented if the task worked properly and found the sqlpackage of visual studio (which was updated with the visual studio update)

@ErikEJ
Copy link
Author

ErikEJ commented Mar 27, 2024

@PaulVrugt Got it!

I encountered multiple issues trying to fix the other task:

@PaulVrugt
Copy link

@PaulVrugt Got it!

I encountered multiple issues trying to fix the other task:

Lol. Ok so in summary:

  • The SqlAzureDacpacDeploymentV1 task seems completely broken as it comes to finding the correct sqlpackage
  • You gave up on fixing this due to the moving parts

Well, thanks at least for trying. We'll create our own safeguards to keep sqlpackage.exe up to date at the location SqlAzureDacpacDeploymentV1 IS finding it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: SqlPackageOnTargetMachines.ps1 isn't able to find sqlpackage.exe from Visual Studio installation
4 participants