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

Failed the validation when use relative links #895

Merged
merged 27 commits into from
Sep 16, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
09d1589
Failed the validation when use relative links when enable the checkLi…
sima-zhu Aug 18, 2020
6d75043
Merge branch 'master' into relative_link
sima-zhu Sep 8, 2020
1b200a7
Addressed feedback
sima-zhu Sep 9, 2020
04887d7
Fixed the bug of pkg name.
sima-zhu Sep 10, 2020
fcc0f0c
Update eng/common/scripts/Verify-Links.ps1
sima-zhu Sep 10, 2020
6acc9af
Merge branch 'relative_link' of https://github.com/sima-zhu/azure-sdk…
sima-zhu Sep 10, 2020
02d51a6
Added link guidance link
sima-zhu Sep 10, 2020
901acdf
Merge branch 'master' into relative_link
sima-zhu Sep 10, 2020
b48ad15
Update eng/common/scripts/Verify-Links.ps1
sima-zhu Sep 10, 2020
e93148b
Update eng/common/scripts/Verify-Links.ps1
sima-zhu Sep 10, 2020
519b473
Move all vars above
sima-zhu Sep 11, 2020
1e5a167
Silent the archor links
sima-zhu Sep 11, 2020
122f192
Silent the archor links
sima-zhu Sep 11, 2020
824a3e1
Romove root url
sima-zhu Sep 11, 2020
3f2d8cd
change the var name
sima-zhu Sep 11, 2020
b3a1f7e
missing parathesis
sima-zhu Sep 11, 2020
0fe62c0
adjust the parameters
sima-zhu Sep 11, 2020
27e9f6f
Pass the right parameters
sima-zhu Sep 11, 2020
ea224d4
default value
sima-zhu Sep 11, 2020
53fd6e3
remove comma
sima-zhu Sep 11, 2020
7f8cf43
fix the wrong check
sima-zhu Sep 11, 2020
d943b89
Skip the empty link
sima-zhu Sep 11, 2020
9ba0e2e
Silent the empty link
sima-zhu Sep 11, 2020
1cba484
small changes
sima-zhu Sep 11, 2020
b96fd2a
Added check for empty string
sima-zhu Sep 11, 2020
f4d1f6b
Added comments and more info in warning message
sima-zhu Sep 15, 2020
21e23aa
Turn off the feature
sima-zhu Sep 16, 2020
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
32 changes: 17 additions & 15 deletions eng/common/pipelines/templates/steps/verify-links.yml
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
parameters:
Directory: 'not-specified'
IgnoreLinksFile: "$(Build.SourcesDirectory)/eng/ignore-links.txt"

WorkingDirectory: $(System.DefaultWorkingDirectory)
ScriptDirectory: eng/common/scripts
steps:
- task: PowerShell@2
displayName: Link verification check
inputs:
pwsh: true
workingDirectory: $(Build.SourcesDirectory)/${{ parameters.Directory }}
filePath: eng/common/scripts/Verify-Links.ps1
arguments: >
-urls $(dir -r -i *.md)
-rootUrl "file://$(Build.SourcesDirectory)/${{ parameters.Directory }}"
-recursive: $false
-ignoreLinksFile ${{ parameters.IgnoreLinksFile }}
-branchReplaceRegex "^($env:SYSTEM_PULLREQUEST_SOURCEREPOSITORYURI.*/(?:blob|tree)/)master(/.*)$"
-branchReplacementName $env:SYSTEM_PULLREQUEST_SOURCECOMMITID
-devOpsLogging: $true
- task: PowerShell@2
displayName: Link verification check
inputs:
pwsh: true
workingDirectory: "${{ parameters.WorkingDirectory }}/${{ parameters.Directory }}"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this should contain directory.

Suggested change
workingDirectory: "${{ parameters.WorkingDirectory }}/${{ parameters.Directory }}"
workingDirectory: "${{ parameters.WorkingDirectory }}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The script is running against all the markdown under working directory, so we need to take in the $serviceDirectory for workingDirectory, otherwise we will run the entire repo e.g. /azure-sdk-for-java

Copy link
Member

Choose a reason for hiding this comment

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

OK yes the dir is against the current working directory. However now we have a chance that the rooturl and the working directory are not in sync. We need to make those consistent.

filePath: ${{ parameters.ScriptDirectory }}/Verify-Links.ps1
arguments: >
-urls $(dir -r -i *.md)
sima-zhu marked this conversation as resolved.
Show resolved Hide resolved
-rootUrl "file://$(Build.SourcesDirectory)/${{ parameters.Directory }}"
-recursive: $false
-ignoreLinksFile ${{ parameters.IgnoreLinksFile }}
-branchReplaceRegex "^($env:SYSTEM_PULLREQUEST_SOURCEREPOSITORYURI.*/(?:blob|tree)/)master(/.*)$"
-branchReplacementName $env:SYSTEM_PULLREQUEST_SOURCECOMMITID
-devOpsLogging: $true
-checkLinkGuidance: $true
Copy link
Member

Choose a reason for hiding this comment

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

We need to make all these parameters options to the yml template so we can control them as needed.

Copy link
Member

Choose a reason for hiding this comment

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

Let do this for all the parameters: urls, recursive, branchreplaceregex, branchreplacementname, checklinkguidance

39 changes: 24 additions & 15 deletions eng/common/scripts/Verify-Links.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ param (
[string] $branchReplaceRegex = "^(https://github.com/.*/(?:blob|tree)/)master(/.*)$",
# the substitute branch name or SHA commit
[string] $branchReplacementName = "",
# flag to allow checking against azure sdk link guidance.
# flag to allow checking against azure sdk link guidance. Check link guidance here: https://aka.ms/azsdk/guideline/links
[bool] $checkLinkGuidance = $false
)

Expand Down Expand Up @@ -85,16 +85,18 @@ function ResolveUri ([System.Uri]$referralUri, [string]$link)
$linkUri = [System.Uri]$link;
# Our link guidelines do not allow relative links so only resolve them when we are not
# validating links against our link guidelines (i.e. !$checkLinkGuideance)
if(!$checkLinkGuidance) {
if (!$linkUri.IsAbsoluteUri) {
if ($checkLinkGuidance -and !$linkUri.IsAbsoluteUri) {
return $linkUri
}

if (!$linkUri.IsAbsoluteUri) {
# For rooted paths resolve from the baseUrl
if ($link.StartsWith("/")) {
Write-Verbose "rooturl = $rootUrl"
$linkUri = new-object System.Uri([System.Uri]$rootUrl, ".$link");
}
else {
$linkUri = new-object System.Uri($referralUri, $link);
}
if ($link.StartsWith("/")) {
Write-Verbose "rooturl = $rootUrl"
$linkUri = new-object System.Uri([System.Uri]$rootUrl, ".$link");
}
else {
$linkUri = new-object System.Uri($referralUri, $link);
}
}

Expand Down Expand Up @@ -193,11 +195,19 @@ function CheckLink ([System.Uri]$linkUri)
}
}

# Check if link uri includes locale info.
if ($checkLinkGuidance -and ($linkUri -match $locale)) {
LogWarning "DO NOT include locale $locale information in links: $linkUri."
$linkValid = $false
if ($checkLinkGuidance) {
# Check if the url is relative links
if (!$linkUri.IsAbsoluteUri) {
LogWarning "DO NOT use relative link $linkUri. Please use absolute link instead. Check here for more infomation: https://aka.ms/azsdk/guideline/links"
sima-zhu marked this conversation as resolved.
Show resolved Hide resolved
$linkValid = $false
}
# Check if link uri includes locale info.
if ($linkUri -match $locale) {
LogWarning "DO NOT include locale $locale information in links: $linkUri. Check here for more infomation: https://aka.ms/azsdk/guideline/links"
sima-zhu marked this conversation as resolved.
Show resolved Hide resolved
$linkValid = $false
}
}

$checkedLinks[$linkUri] = $linkValid
return $linkValid
}
Expand Down Expand Up @@ -270,7 +280,6 @@ $checkedPages = @{};
$checkedLinks = @{};
$badLinks = @{};
$pageUrisToCheck = new-object System.Collections.Queue

foreach ($url in $urls) {
$uri = NormalizeUrl $url
$pageUrisToCheck.Enqueue($uri);
Expand Down