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

BUG: Ignore breathe duplicate C++ warnings in docs CI #327

Merged
merged 2 commits into from
Mar 21, 2022

Conversation

tbirdso
Copy link
Contributor

@tbirdso tbirdso commented Mar 13, 2022

Resolves #266

@github-actions github-actions bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels Mar 13, 2022
@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 14, 2022

I'm unclear on whether CTestCustom.cmake is being successfully applied here. After lowering the maximum number of warnings I am still seeing 50 warnings being reported in CDash.

@dzenanz
Copy link
Member

dzenanz commented Mar 15, 2022

As far as I can tell, the current error is:

Cloning into 'zlib'...
fatal: remote error: 
  The unauthenticated git protocol on port 9418 is no longer supported.
Please see https://github.blog/2021-09-01-improving-git-protocol-security-github/ for more information.

quick fix is to use https:// protocol for fetching.

@dzenanz
Copy link
Member

dzenanz commented Mar 15, 2022

[7/40] Performing download step (git clone) for 'zlib'
FAILED: zlib-prefix/src/zlib-stamp/zlib-download 
cd /home/runner/work/bld && /home/runner/work/_temp/685782585/cmake-3.18.3-Linux-x86_64/bin/cmake -P /home/runner/work/bld/zlib-prefix/tmp/zlib-gitclone.cmake && /home/runner/work/_temp/685782585/cmake-3.18.3-Linux-x86_64/bin/cmake -E touch /home/runner/work/bld/zlib-prefix/src/zlib-stamp/zlib-download
Cloning into 'zlib'...
fatal: remote error: 
  The unauthenticated git protocol on port 9418 is no longer supported.
Please see https://github.blog/2021-09-01-improving-git-protocol-security-github/ for more information.

...

-- Had to git clone more than once:
          3 times.
CMake Error at zlib-prefix/tmp/zlib-gitclone.cmake:31 (message):
  Failed to clone repository: 'git://github.com/commontk/zlib.git'

@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 15, 2022

Thanks @dzenanz , I pushed a change this morning removing most warning exceptions from CTestCustom.cmake.in to try to verify whether changes to the file are having an impact. Unfortunately I can't tell whether this error is actually related 🤔 I am going to try re-running CI and see if the error persists.

@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 15, 2022

Observed the same warnings on re-run, so something is happening with CTestCustom.cmake, but I'm still not sure why the regular expressions for excluding the duplication warning are not being picked up. Reverting so that other warning exceptions are populated.

@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 15, 2022

The zlib error is present in main branch CI after rerun. I will enter a new issue and PR to track. @dzenanz as you said this should be fixable by forcing https rather than git to be used for retrieving files.

@tbirdso tbirdso mentioned this pull request Mar 15, 2022
@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 16, 2022

Rebased on master to fix zlib warnings.

@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 16, 2022

Unclear why CI is not launched from the force push. Seems I am not having much luck with Github today. 😦

I have re-run previous CI for this PR that I believe will reflect superbuild fixes: https://github.com/InsightSoftwareConsortium/ITKSphinxExamples/actions/runs/1988322694

@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 16, 2022

The CI re-run did not capture updated changes. It looks like Github Actions is having an outage, may be best to wait for coverage to resume before attempting further changes. https://www.githubstatus.com/

@thewtex
Copy link
Member

thewtex commented Mar 17, 2022

Seems I am not having much luck with Github today

I was also seeing lag with GitHub CI triggering yesterday.

@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 18, 2022

From what I can tell CTestCustom.cmake is having no impact on CI.

After removing most warning exceptions, decreasing the number of warnings and errors, and adding additional regex expressions to except, CDash results are unchanged at 50 warnings about duplicate C++ declarations.

@thewtex @dzenanz Do you have thoughts on steps we can take for debugging here? As this file is expected to solely affect CDash behavior I am a bit limited in what testing I can do.

@dzenanz
Copy link
Member

dzenanz commented Mar 18, 2022

@hjmjohnson recently made some docs-related PR (InsightSoftwareConsortium/ITK#3311), he might have some suggestions. I have not dealt with docs building in years (maybe ever). I would have to dive in, just like you.

@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 19, 2022

Aha, new findings:

  • The CTEST_CUSTOM_WARNING_EXCEPTION variable is extended in the docs CI procedure when we create dashboard.cmake to drive the CTest dashboard procedure. We can resolve Duplicate declaration warnings in dashboard #266 by editing build-test-publish.yml accordingly.
  • CI failed at the publish-artifact stage due to relative pathing. Perhaps the absolute pathing requirement is recent? We can work around it by using the absolute path to the work directory.
  • My current guess as to why CTestCustom.cmake is not affecting dashboard results is that the file is not moved to the right location as part of the superbuild procedure. After running the CTest command locally I see that CTestCustom.cmake is configured in the ITKEx-build subfolder of the superbuild, while it should be at the top level directory. Given that the build process has not actually been using it, perhaps it would be prudent to simply delete it? Alternatively we can move it to the top level in the CI procedure.

EDIT: On second though, CTestCustom.cmake is likely still effective in setting the warnings/errors max count in build-test-cxx CI which does not appear to rely on the SuperBuild. The better solution here would be to move the file to the superbuild top-level directory as part of CI.

@tbirdso tbirdso marked this pull request as ready for review March 21, 2022 14:28
@tbirdso tbirdso requested a review from dzenanz March 21, 2022 14:28
@tbirdso tbirdso merged commit ef1b9ea into InsightSoftwareConsortium:master Mar 21, 2022
@tbirdso
Copy link
Contributor Author

tbirdso commented Mar 21, 2022

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate declaration warnings in dashboard
3 participants