Skip to content

Increase AppVeyor git clone depth and lint try branch builds - #12346

Merged
seanbudd merged 8 commits into
masterfrom
fix-merge-base
May 7, 2021
Merged

Increase AppVeyor git clone depth and lint try branch builds#12346
seanbudd merged 8 commits into
masterfrom
fix-merge-base

Conversation

@seanbudd

@seanbudd seanbudd commented Apr 27, 2021

Copy link
Copy Markdown
Member

Link to issue number:

None

Summary of the issue:

AppVeyor builds occasionally fail, such as this one, due to git merge-base FETCH_HEAD HEAD failing after git fetch -q origin $env:APPVEYOR_REPO_BRANCH.

We have a clone_depth set to 1. There is a warning in the appveyor docs as follows

Be aware that if you do a lot of commits producing queued builds and depth number is too small git checkout operation following git clone can fail because requested commit is not present in a cloned repository.

Relevant stack trace

[00:10:47] if($env:APPVEYOR_PULL_REQUEST_NUMBER) {
[00:10:47]   $lintOutput = (Resolve-Path .\testOutput\lint\)
[00:10:47]   $lintSource = (Resolve-Path .\tests\lint\)
[00:10:47]   # When Appveyor runs for a pr,
[00:10:47]   # the build is made from a new temporary commit,
[00:10:47]   # resulting from the pr branch being merged into its base branch.
[00:10:47]   # Therefore to create a diff for linting, we must fetch the head of the base branch.
[00:10:47]   # In a PR, APPVEYOR_REPO_BRANCH points to the head of the base branch. 
[00:10:47]  git fetch -q origin $env:APPVEYOR_REPO_BRANCH
[00:10:47]  $flake8Output = "$lintOutput\PR-Flake8.txt"
[00:10:47]   .\runlint.bat FETCH_HEAD "$flake8Output" 
[00:10:47]   if($LastExitCode -ne 0) {
[00:10:47]    $errorCode=$LastExitCode
[00:10:47]    Add-AppveyorMessage "PR introduces Flake8 errors"
[00:10:47]   }
[00:10:47]   Push-AppveyorArtifact $flake8Output
[00:10:47]   $junitXML = "$lintOutput\PR-Flake8.xml"
[00:10:47]   py "$lintSource\createJunitReport.py" "$flake8Output" "$junitXML"
[00:10:47]   Push-AppveyorArtifact $junitXML
[00:10:47]   $wc = New-Object 'System.Net.WebClient'
[00:10:47]   $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", $junitXML)
[00:10:47]   if($errorCode -ne 0) { $host.SetShouldExit($errorCode) }
[00:10:47] }
[00:10:47] 
[00:10:47] Ensuring NVDA Python virtual environment
[00:10:47] call py "C:\projects\nvda\\tests\lint\genDiff.py" FETCH_HEAD "C:\projects\nvda\\tests\lint\_lint.diff"
[00:10:48] .\runlint.bat : Traceback (most recent call last):
[00:10:48] At line:11 char:3
[00:10:48] +   .\runlint.bat FETCH_HEAD "$flake8Output"
[00:10:48] +   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[00:10:48]     + CategoryInfo          : NotSpecified: (Traceback (most recent call last)::String) [], RemoteException
[00:10:48]     + FullyQualifiedErrorId : NativeCommandError
[00:10:48]  
[00:10:48]   File "C:\projects\nvda\\tests\lint\genDiff.py", line 65, in <module>
[00:10:48]     main(*getArgs(argv))
[00:10:48]   File "C:\projects\nvda\\tests\lint\genDiff.py", line 42, in main
[00:10:48]     diff = getDiff(baseBranch)
[00:10:48]   File "C:\projects\nvda\\tests\lint\genDiff.py", line 33, in getDiff
[00:10:48]     mergeBase: bytes = subprocess.check_output(mergeBaseCommand)
[00:10:48]   File "C:\Python38\lib\subprocess.py", line 411, in check_output
[00:10:48]     return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
[00:10:48]   File "C:\Python38\lib\subprocess.py", line 512, in run
[00:10:48]     raise CalledProcessError(retcode, process.args,
[00:10:48] subprocess.CalledProcessError: Command 'git merge-base FETCH_HEAD HEAD' returned non-zero exit status 1.
[00:10:48] 
[00:10:48] Deactivating NVDA Python virtual environment
[00:10:48] Resolve-Path : Cannot find path 'C:\projects\nvda\testOutput\lint\PR-Flake8.txt' because it does not exist.
[00:10:48] At C:\Program Files\AppVeyor\BuildAgent\Modules\build-worker-api\build-worker-api.psm1:207 char:18
[00:10:48] +     $fullPath = (Resolve-Path $Path).Path
[00:10:48] +                  ~~~~~~~~~~~~~~~~~~
[00:10:48]     + CategoryInfo          : ObjectNotFound: (C:\projects\nvd...t\PR-Flake8.txt:String) [Resolve-Path], ItemNotFoundException
[00:10:48]     + FullyQualifiedErrorId : PathNotFound,Microsoft.PowerShell.Commands.ResolvePathCommand
[00:10:48]  
[00:10:48] Push-AppveyorArtifactInternal : Cannot bind argument to parameter 'FullPath' because it is null.
[00:10:48] At C:\Program Files\AppVeyor\BuildAgent\Modules\build-worker-api\build-worker-api.psm1:209 char:42
[00:10:48] +     Push-AppveyorArtifactInternal -FullPath $fullPath -FileName $File ...
[00:10:48] +                                             ~~~~~~~~~
[00:10:48]     + CategoryInfo          : InvalidData: (:) [Push-AppveyorArtifactInternal], ParameterBindingValidationException
[00:10:48]     + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Appveyor.BuildAgent.Api.Utils.PushAppveyorArtifactInternalCmdlet
[00:10:48]  
[00:10:48] Flake8_output_file does not exist at C:\projects\nvda\testOutput\lint\\PR-Flake8.txt
[00:10:48] py : Traceback (most recent call last):
[00:10:48] At line:18 char:3
[00:10:48] +   py "$lintSource\createJunitReport.py" "$flake8Output" "$junitXML"
[00:10:48] +   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[00:10:48]     + CategoryInfo          : NotSpecified: (Traceback (most recent call last)::String) [], RemoteException
[00:10:48]     + FullyQualifiedErrorId : NativeCommandError
[00:10:48]  
[00:10:48]   File "C:\projects\nvda\tests\lint\\createJunitReport.py", line 65, in <module>
[00:10:48]     main()
[00:10:48]   File "C:\projects\nvda\tests\lint\\createJunitReport.py", line 60, in main
[00:10:48]     raise e
[00:10:48]   File "C:\projects\nvda\tests\lint\\createJunitReport.py", line 54, in main
[00:10:48]     raise RuntimeError(
[00:10:48] RuntimeError: Flake8_output_file does not exist at C:\projects\nvda\testOutput\lint\\PR-Flake8.txt
[00:10:48] 
[00:10:48] Resolve-Path : Cannot find path 'C:\projects\nvda\testOutput\lint\PR-Flake8.xml' because it does not exist.
[00:10:48] At C:\Program Files\AppVeyor\BuildAgent\Modules\build-worker-api\build-worker-api.psm1:207 char:18
[00:10:48] +     $fullPath = (Resolve-Path $Path).Path
[00:10:48] +                  ~~~~~~~~~~~~~~~~~~
[00:10:48]     + CategoryInfo          : ObjectNotFound: (C:\projects\nvd...t\PR-Flake8.xml:String) [Resolve-Path], ItemNotFoundException
[00:10:48]     + FullyQualifiedErrorId : PathNotFound,Microsoft.PowerShell.Commands.ResolvePathCommand
[00:10:48]  
[00:10:48] Push-AppveyorArtifactInternal : Cannot bind argument to parameter 'FullPath' because it is null.
[00:10:48] At C:\Program Files\AppVeyor\BuildAgent\Modules\build-worker-api\build-worker-api.psm1:209 char:42
[00:10:48] +     Push-AppveyorArtifactInternal -FullPath $fullPath -FileName $File ...
[00:10:48] +                                             ~~~~~~~~~
[00:10:48]     + CategoryInfo          : InvalidData: (:) [Push-AppveyorArtifactInternal], ParameterBindingValidationException
[00:10:48]     + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Appveyor.BuildAgent.Api.Utils.PushAppveyorArtifactInternalCmdlet
[00:10:48]  
[00:10:48] Exception calling "UploadFile" with "2" argument(s): "An exception occurred during a WebClient request."
[00:10:48] At line:21 char:67
[00:10:48] + ... /ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", $jun ...
[00:10:48] +                                              ~~~~~~~~~~~~~~~~~~~~
[00:10:48]     + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
[00:10:48]     + FullyQualifiedErrorId : WebException
[00:10:48]  

Additional issue tackled here: Linting isn't checked on try-builds.

Description of how this pull request fixes the issue:

  • Up the clone depth to unlimited (by removing setting the value)
  • Fetch master directly when performing a try-branch build so that lint checks can now run on try branch builds
  • recursively fetch submodules on demand.

Testing strategy:

Perform try builds and builds to master.

Known issues with pull request:

None

Changelog entry

None, effects build process

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

@seanbudd seanbudd self-assigned this Apr 27, 2021
@seanbudd
seanbudd marked this pull request as ready for review April 27, 2021 07:14
@seanbudd
seanbudd requested a review from a team as a code owner April 27, 2021 07:14
@seanbudd
seanbudd requested a review from michaelDCurran April 27, 2021 07:14
@seanbudd seanbudd changed the title Increase git clone depth and submodule recurse Increase AppVeyor git clone depth and lint try branch builds Apr 27, 2021
Comment thread appveyor.yml Outdated
$flake8Output = "$lintOutput\PR-Flake8.txt"
# Additionally, we can not use a clone_depth of 1, but must use an unlimited clone.
if($env:APPVEYOR_PULL_REQUEST_NUMBER) {
git fetch -q --recurse-submodules origin $env:APPVEYOR_REPO_BRANCH

@michaelDCurran michaelDCurran Apr 27, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need to recursively fetch submodules here? Same question for the fetch further down. Linting does not require the submodules I would have thought...
I would say that fetching the submodules (now twice) would be most of the 40 seconds added to the build.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not convinced it does anything after further investigation. I thought it was due to possible issues with merging - but that should only be an issue on the try branch builds if anything, not in this condition as the checked out branch should already have master merged in.

I think it might be easier to take it out and get a good example if any error is (still) thrown.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The time difference is now much more negligible

@seanbudd
seanbudd requested a review from michaelDCurran May 7, 2021 00:39
@seanbudd
seanbudd merged commit a110d14 into master May 7, 2021
@seanbudd
seanbudd deleted the fix-merge-base branch May 7, 2021 01:06
@nvaccessAuto nvaccessAuto added this to the 2021.1 milestone May 7, 2021
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.

3 participants