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.

Sign up for GitHub

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

Support artifacts published from any location #6268

Merged
merged 3 commits into from
Mar 15, 2018
Merged

Support artifacts published from any location #6268

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
093b743
Support artifacts published from any location
NCarlsonMSFT Jan 22, 2018
8ea1fe8
Remove use of pkgArtifactPath
NCarlsonMSFT Jan 24, 2018
9e27e9d
Added testing for a package built in a sub-path.
NCarlsonMSFT Mar 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Tasks/ServiceFabricUpdateManifests/Strings/resources.resjson/en-US/resources.resjson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
"loc.input.help.updateOnlyChanged": "Incrementally update only the packages that have changed. Use the [deterministic compiler flag](https://go.microsoft.com/fwlink/?LinkId=808668) to ensure builds with the same inputs produce the same outputs.",
"loc.input.label.pkgArtifactName": "Package Artifact Name",
"loc.input.help.pkgArtifactName": "The name of the artifact containing the application package for comparison.",
"loc.input.label.pkgArtifactPath": "Package Artifact Path",
"loc.input.help.pkgArtifactPath": "The path to the artifact containing the application package. [Variables](https://go.microsoft.com/fwlink/?LinkID=550988) and wildcards can be used in the path.",
"loc.input.label.logAllChanges": "Log all changes",
"loc.input.help.logAllChanges": "Compare all files in every package and log if the file was added, removed, or if its content changed. Otherwise, compare files in a package only until the first change is found for faster performance.",
"loc.input.label.compareType": "Compare against",
Expand Down Expand Up @@ -59,5 +61,6 @@
"loc.messages.FileChanged": "The file '{0}' has changed.",
"loc.messages.NoChanges": "Found no changes.",
"loc.messages.PdbWarning": "This package contains '*.pdb' files, which will change in every build even if the 'deterministic' compiler flag is used. Exclude these files from your package in order to accurately detect if the package has changed.",
"loc.messages.InvalidImageDigestValue": "The image digest value '{0}' found in file '{1}' is invalid. Expected a value in the format of 'endpoint/image_name@digest_value'."
"loc.messages.InvalidImageDigestValue": "The image digest value '{0}' found in file '{1}' is invalid. Expected a value in the format of 'endpoint/image_name@digest_value'.",
"loc.messages.PackageIsNotUnderArtifact": "The package path '{0}' is not under artifact path '{1}'."
}
1 change: 1 addition & 0 deletions Tasks/ServiceFabricUpdateManifests/Tests/Test-ApplicationVersions.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ try
$oldSuffix = ".OldSuffix"

Register-Mock Get-VstsInput { $pkgPath } -- -Name applicationPackagePath -Require
Register-Mock Get-VstsInput { $PSScriptRoot } -- -Name pkgArtifactPath -Require
Register-Mock Get-VstsInput { $newSuffix } -- -Name versionSuffix -Require
Register-Mock Get-VstsInput { $true } -- -Name updateOnlyChanged -Require
Register-Mock Find-VstsFiles { $pkgPath } -- -LegacyPattern $pkgPath -IncludeDirectories
Expand Down
19 changes: 15 additions & 4 deletions Tasks/ServiceFabricUpdateManifests/Update-ApplicationVersions.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
$compareType = Get-VstsInput -Name compareType -Require
$buildNumber = Get-VstsInput -Name buildNumber
$overwritePkgArtifact = ((Get-VstsInput -Name overwriteExistingPkgArtifact) -eq "true")
$pkgArtifactPath = Get-VstsInput -Name pkgArtifactPath -Require
if (-not $pkgArtifactPath.EndsWith('\'))
{
$pkgArtifactPath = $pkgArtifactPath + '\'
}

try
{
Expand All @@ -57,20 +62,26 @@

Write-Host (Get-VstsLocString -Key SearchingApplicationType -ArgumentList $appTypeName)

$oldAppPackagePath = Join-Path $oldDropLocation $newAppPackagePath.SubString((Get-VstsTaskVariable -Name Build.SourcesDirectory -Require).Length + 1)
if (-not $newAppPackagePath.StartsWith($pkgArtifactPath))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is very difficult to make user understand about this new mandatory input. Can we think of any alternative without additional input. Can we parse $newAppPackagePath to check if it contains a path fragment which matches path inside $oldPackagePath. For example, if new path is C:\a\b\c\ApplicationPackage.xml then we can iterate whether old package contains any of following path:

  • a\b\c\ApplicationPackage.xml
  • b\c\ApplicationPackage.xml
  • c\ApplicationPackage.xml
  • ApplicationPackage.xml

{
Write-Error (Get-VstsLocString -Key PackageIsNotUnderArtifact -ArgumentList @($newAppPackagePath, $pkgArtifactPath))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to return?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write-Error effectively throws an exception an halts execution

}
$relativePath = $newAppPackagePath.SubString($pkgArtifactPath.Length)

$oldAppPackagePath = Join-Path $oldDropLocation $relativePath
$oldAppManifestPath = Join-Path $oldAppPackagePath $appManifestName
if (Test-Path $oldAppManifestPath)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have UTs for this non-trivial logic

$oldAppManifestXml = [XML](Get-Content $oldAppManifestPath)

# Set the version to the version from the previous build (including its suffix). This will be overwritten if we find any changes, otherwise it will match the previous build by design.
# Set it before we search for changes so that we can compare the xml without the old version suffix causing a false positive.
# Set it before we search for changes so that we can compare the xml without the old version suffix causing a false positive.
$newAppManifestXml.ApplicationManifest.ApplicationTypeVersion = $oldAppManifestXml.ApplicationManifest.ApplicationTypeVersion
}
else
{
Write-Warning (Get-VstsLocString -Key NoManifestInPreviousBuild)
$updateAllVersions = $true
$updateAllVersions = $true
}
}
else
Expand All @@ -85,7 +96,7 @@
$logIndent = "".PadLeft(2)
foreach ($serviceManifestImport in $newAppManifestXml.ApplicationManifest.ServiceManifestImport)
{
$serviceVersion = Update-ServiceVersions -VersionValue $versionValue -ServiceName $serviceManifestImport.ServiceManifestRef.ServiceManifestName -NewPackageRoot $newAppPackagePath -OldPackageRoot $oldAppPackagePath -LogIndent $logIndent -UpdateAllVersions:$updateAllVersions -LogAllChanges:$logAllChanges -ReplaceVersion:$replaceVersion
$serviceVersion = Update-ServiceVersions -VersionValue $versionValue -ServiceName $serviceManifestImport.ServiceManifestRef.ServiceManifestName -NewPackageRoot $newAppPackagePath -OldPackageRoot $oldAppPackagePath -LogIndent $logIndent -UpdateAllVersions:$updateAllVersions -LogAllChanges:$logAllChanges -ReplaceVersion:$replaceVersion
$serviceManifestImport.ServiceManifestRef.ServiceManifestVersion = $serviceVersion
}

Expand Down
16 changes: 13 additions & 3 deletions Tasks/ServiceFabricUpdateManifests/task.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
],
"version": {
"Major": 2,
"Minor": 1,
"Patch": 10
"Minor": 2,
"Patch": 0
},
"minimumAgentVersion": "1.95.0",
"instanceNameFormat": "Update Service Fabric Manifests ($(updateType))",
Expand Down Expand Up @@ -85,6 +85,15 @@
"helpMarkDown": "The name of the artifact containing the application package for comparison.",
"visibleRule": "updateType = Manifest versions && updateOnlyChanged = true"
},
{
"name": "pkgArtifactPath",
"type": "string",
"label": "Package Artifact Path",
"defaultValue": "$(build.artifactstagingdirectory)",
"required": true,
"visibleRule": "updateType = Manifest versions && updateOnlyChanged = true",
"helpMarkDown": "The path to the artifact containing the application package. [Variables](https://go.microsoft.com/fwlink/?LinkID=550988) and wildcards can be used in the path."
},
{
"name": "logAllChanges",
"type": "boolean",
Expand Down Expand Up @@ -175,6 +184,7 @@
"FileChanged": "The file '{0}' has changed.",
"NoChanges": "Found no changes.",
"PdbWarning": "This package contains '*.pdb' files, which will change in every build even if the 'deterministic' compiler flag is used. Exclude these files from your package in order to accurately detect if the package has changed.",
"InvalidImageDigestValue": "The image digest value '{0}' found in file '{1}' is invalid. Expected a value in the format of 'endpoint/image_name@digest_value'."
"InvalidImageDigestValue": "The image digest value '{0}' found in file '{1}' is invalid. Expected a value in the format of 'endpoint/image_name@digest_value'.",
"PackageIsNotUnderArtifact": "The package path '{0}' is not under artifact path '{1}'."
}
}
16 changes: 13 additions & 3 deletions Tasks/ServiceFabricUpdateManifests/task.loc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
],
"version": {
"Major": 2,
"Minor": 1,
"Patch": 10
"Minor": 2,
"Patch": 0
},
"minimumAgentVersion": "1.95.0",
"instanceNameFormat": "ms-resource:loc.instanceNameFormat",
Expand Down Expand Up @@ -85,6 +85,15 @@
"helpMarkDown": "ms-resource:loc.input.help.pkgArtifactName",
"visibleRule": "updateType = Manifest versions && updateOnlyChanged = true"
},
{
"name": "pkgArtifactPath",
"type": "string",
"label": "ms-resource:loc.input.label.pkgArtifactPath",
"defaultValue": "$(build.artifactstagingdirectory)",
"required": true,
"visibleRule": "updateType = Manifest versions && updateOnlyChanged = true",
"helpMarkDown": "ms-resource:loc.input.help.pkgArtifactPath"
},
{
"name": "logAllChanges",
"type": "boolean",
Expand Down Expand Up @@ -175,6 +184,7 @@
"FileChanged": "ms-resource:loc.messages.FileChanged",
"NoChanges": "ms-resource:loc.messages.NoChanges",
"PdbWarning": "ms-resource:loc.messages.PdbWarning",
"InvalidImageDigestValue": "ms-resource:loc.messages.InvalidImageDigestValue"
"InvalidImageDigestValue": "ms-resource:loc.messages.InvalidImageDigestValue",
"PackageIsNotUnderArtifact": "ms-resource:loc.messages.PackageIsNotUnderArtifact"
}
}