Handle non existence of app manifest in older build #4511
Conversation
| [CmdletBinding()] | ||
| param() | ||
|
|
||
| . "$PSScriptRoot\Test-ApplicationVersions.ps1" PreviousPackageNoManifest |
There was a problem hiding this comment.
Shouldn't you be testing that version got updated
There was a problem hiding this comment.
I am. Look at line 57 in Test-ApplicationVersions.ps1(called from this test) wher we are validating that new application manifest has updated version.
| @@ -32,6 +32,7 @@ | |||
| "loc.messages.PreviousBuildNumberLabel": "Previous build number: '{0}'", | |||
| "loc.messages.PreviousBuildLocationLabel": "Previous build location: '{0}'", | |||
| "loc.messages.NoPreviousSuccessfulBuild": "Could not find previous build to compare against.", | |||
| "loc.messages.NoManifestInPreviousBuild": "Previous build does not contains application manifest. Force updating application type version.", | |||
There was a problem hiding this comment.
Your message mentions only forcing update of the application manifest, but the code forces update of all manifests.
| else | ||
| { | ||
| Write-Warning (Get-VstsLocString -Key NoManifestInPreviousBuild) | ||
| $updateAllVersions = $true |
There was a problem hiding this comment.
Is it really necessary to re-version all of the services if there was no application manifest in the previous build? If there is a service manifest, and those files haven't changed why does the service need a new version?
There was a problem hiding this comment.
@NCarlsonMSFT is this a valid scenario? I would assume that any old package not containing applicationManifest.xml is an invalid package and should not be used for comparison.
There was a problem hiding this comment.
Fair point, it probably is safest to reversion everything. I mainly just wanted to avoid the performance implications of forcing an update to every service.
Earlier task used to fail if older build drop did not contain application manifest file. Now, comparison will be skipped and new version will be applied. I have checked that same fix is not needed for service manifests as the check already exists. Related issue #4399