Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (29)
📝 WalkthroughWalkthroughSwitches OpenCV/Tesseract from prebuilt downloads to Git submodules and CMake-based source builds, adds CMake cache option files (full/slim), updates CI/workflow paths and caches, introduces Windows OpenCV build script and vcpkg integration, and removes the DebuggerVisualizers project and related files. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/ubuntu.yml (1)
59-67:⚠️ Potential issue | 🟠 MajorInvalidate the OpenCV cache when the full build profile changes.
The cached install tree depends on
cmake/opencv_build_options.cmake(used in the Build OpenCV step at line 72), but the cache key only varies byOPENCV_VERSIONand distro. If this configuration file changes, the cache will still match exactly, causing the build step to be skipped and outdated artifacts to be reused.🔑 Suggested key update
- key: opencv-${{ env.OPENCV_VERSION }}-ubuntu-${{ matrix.ubuntu }} + key: opencv-${{ env.OPENCV_VERSION }}-ubuntu-${{ matrix.ubuntu }}-${{ hashFiles('.github/workflows/ubuntu.yml', 'cmake/opencv_build_options.cmake') }}Apply this change to both the restore (line 67) and save (line 89) cache actions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ubuntu.yml around lines 59 - 67, The OpenCV cache key must include the build options file so changes to cmake/opencv_build_options.cmake invalidate the cache; update the cache key used in both the "Restore OpenCV cache" and the corresponding "Save OpenCV cache" steps to append a hash of that file (e.g., include ${{ hashFiles('cmake/opencv_build_options.cmake') }}) alongside OPENCV_VERSION and matrix.ubuntu so the cache is recomputed when the full build profile changes..github/workflows/linux-arm.yml (1)
51-58:⚠️ Potential issue | 🟠 MajorUse the same full key for restore and save, including the new cmake options file.
The restore and save cache keys don't match: the restore key at line 58 includes
${{ hashFiles('.github/workflows/linux-arm.yml') }}, but the save key at line 82 is missing this suffix entirely. This meanssteps.opencv-cache.outputs.cache-hitwill never be'true', and OpenCV will always be rebuilt. Additionally,cmake/opencv_build_options.cmakeis now used in the build at line 66 but not included in the cache key, so changes to the build options won't invalidate the cache.🔑 Suggested key updates
Line 58 (restore): - key: opencv-arm-${{ env.OPENCV_VERSION }}-${{ hashFiles('.github/workflows/linux-arm.yml') }} + key: opencv-arm-${{ env.OPENCV_VERSION }}-${{ hashFiles('.github/workflows/linux-arm.yml', 'cmake/opencv_build_options.cmake') }} Line 82 (save): - key: opencv-arm-${{ env.OPENCV_VERSION }} + key: opencv-arm-${{ env.OPENCV_VERSION }}-${{ hashFiles('.github/workflows/linux-arm.yml', 'cmake/opencv_build_options.cmake') }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/linux-arm.yml around lines 51 - 58, The restore and save cache keys must match and include the cmake options file so cache hits work and option changes invalidate the cache: update the key used in the "Restore OpenCV cache" step (id: opencv-cache) to be identical to the key used in the save step (save job for OpenCV cache) and append hashFiles('cmake/opencv_build_options.cmake') (or include that file path in the same hashFiles call) so both restore and save use the full key including the workflow file and the cmake/opencv_build_options.cmake hash..github/workflows/ubuntu-slim.yml (1)
48-56:⚠️ Potential issue | 🟠 MajorInvalidate the slim OpenCV cache when the slim profile changes.
The cache key at lines 48–56 and 107–114 does not include
cmake/opencv_build_options_slim.cmake, so the cache won't invalidate when the slim module list or feature toggles change. Add the configuration file to the cache key:🔑 Suggested key update
- key: opencv-${{ env.OPENCV_VERSION }}-ubuntu-${{ matrix.ubuntu }}-slim + key: opencv-${{ env.OPENCV_VERSION }}-ubuntu-${{ matrix.ubuntu }}-slim-${{ hashFiles('.github/workflows/ubuntu-slim.yml', 'cmake/opencv_build_options_slim.cmake') }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ubuntu-slim.yml around lines 48 - 56, The cache key for the slim OpenCV cache (the step named "Restore OpenCV cache (slim)" with id opencv-cache and the corresponding save step) doesn't include the slim build configuration, so changes to cmake/opencv_build_options_slim.cmake won't invalidate the cache; update both the restore and save cache key expressions (the opencv-${{ env.OPENCV_VERSION }}-ubuntu-${{ matrix.ubuntu }}-slim keys) to incorporate the cmake/opencv_build_options_slim.cmake file (e.g., include its checksum or path expansion in the key) so the cache is regenerated when that config changes.
🧹 Nitpick comments (4)
.gitmodules (3)
4-6: Consider adding branch tracking for easier upstream updates.While the submodule configuration is syntactically correct, specifying a
branchfield would make it easier to track and update to the latest upstream changes.📌 Suggested enhancement
[submodule "opencv"] path = opencv url = https://github.com/opencv/opencv.git + branch = 4.xChoose the appropriate branch (e.g.,
4.x,5.x, ormaster) based on your OpenCV version requirements.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitmodules around lines 4 - 6, Add an explicit branch entry to the opencv submodule configuration so updates track the upstream branch; modify the [submodule "opencv"] section (the entry with path = opencv and url = https://github.com/opencv/opencv.git) to include a branch = <branch-name> (e.g., branch = 4.x or branch = master) that matches the project’s OpenCV compatibility, then run git submodule sync and git submodule update --remote to start tracking that branch.
7-9: Consider adding branch tracking for easier upstream updates.Similar to the opencv submodule, specifying a
branchfield would simplify tracking and updating the opencv_contrib submodule. Ensure the branch matches the one used for the opencv submodule to maintain compatibility.📌 Suggested enhancement
[submodule "opencv_contrib"] path = opencv_contrib url = https://github.com/opencv/opencv_contrib.git + branch = 4.x🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitmodules around lines 7 - 9, Add a branch tracking entry for the opencv_contrib submodule in .gitmodules so it follows the same branch as the opencv submodule; update the opencv_contrib submodule block (submodule "opencv_contrib") to include a branch = <branch-name> line matching the opencv submodule's branch to simplify upstream updates and keep compatibility.
4-9: Operational advice: Consider shallow clones for large OpenCV repositories.The opencv and opencv_contrib repositories are quite large (several hundred MB to GBs). To improve clone performance and reduce disk usage, consider using shallow clones in your CI/CD workflows:
git submodule update --init --depth 1 opencv opencv_contribAdditionally, ensure that contributor documentation includes clear instructions on initializing submodules after cloning the repository.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitmodules around lines 4 - 9, The .gitmodules lists large submodules "opencv" and "opencv_contrib" which slow clones; update CI and contributor docs to perform shallow submodule clones for these entries by changing the CI pipeline step that initializes submodules to use depth 1 (e.g. run git submodule update --init --depth 1 opencv opencv_contrib) and add a short note to CONTRIBUTING.md (or README) describing how contributors should initialize these submodules after cloning; reference the .gitmodules entries "opencv" and "opencv_contrib" when making the changes so the CI job and docs target the correct submodules..gitignore (1)
122-125: Ignore the newsrc/build*trees too.This PR now generates
src/buildandsrc/build-slim, so those artifacts will start showing up as untracked files in local dev flows.♻️ Suggested ignore entries
# Local OpenCV / Tesseract build outputs (generated by build_opencv_windows.ps1) /opencv/build/ /opencv_artifacts/ /tesseract_artifacts/ +/src/build/ +/src/build-slim/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 122 - 125, Add ignore patterns for the newly generated build trees so local untracked files don't appear: update the .gitignore to include "src/build/" and "src/build-slim/" (and optionally "src/build-*" to catch variations). Ensure these exact directory patterns match the generated folders referenced in the diff (src/build, src/build-slim) and are placed in the appropriate section of .gitignore so they take effect for local dev flows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/windows.yml:
- Around line 50-58: Update the OpenCV cache key so it invalidates when build
inputs change: modify the cache steps (e.g., the "Restore OpenCV cache" step
with id opencv-cache and the matching save cache step) to include a fingerprint
of relevant inputs such as cmake/opencv_build_options.cmake and the OpenCV
source (or a file-list/hash of sources) in addition to OPENCV_VERSION; generate
a stable hash of those files (or use actions/checkout@v3 commit SHA) and append
it to the existing key (e.g., opencv-${{ env.OPENCV_VERSION }}-windows-x64-${{
hashFiles('cmake/opencv_build_options.cmake','path/to/opencv/**') }} ) so any
change to build options or the OpenCV sources will miss the cache and trigger a
rebuild.
- Line 12: The workflow uses a mutable TESSERACT_VCPKG_CACHE_KEY and runs `vcpkg
install tesseract:x64-windows-static` without any manifest or baseline pinning,
which allows non-reproducible Tesseract/Leptonica binaries; fix by adding a
vcpkg manifest (vcpkg.json) with a builtin-baseline entry or by adding a vcpkg
bootstrap step that checks out and uses a specific vcpkg commit hash before
`vcpkg install`, and update references to TESSERACT_VCPKG_CACHE_KEY so cache
invalidation corresponds to the pinned baseline/commit.
In `@build_opencv_windows.ps1`:
- Around line 84-85: The build directory $buildDir is reused across different
Visual Studio generators causing stale CMakeCache.txt collisions; change the
script to make the build tree generator-specific (e.g., derive $buildDir using
the detected generator or $vsMajor like "$RepoRoot/opencv/build-vs$vsMajor") or,
alternatively, ensure the script removes/cleans the existing CMake cache and
build files before running configure; update any references to $buildDir and
keep $installDir untouched.
In `@cmake/opencv_build_options.cmake`:
- Around line 19-20: The wrapper build is trying to compile barcode.cpp (which
depends on cv::barcode::BarcodeDetector) while the full OpenCV build disables
BUILD_opencv_barcode in cmake/opencv_build_options.cmake; to fix, update the
CI/workflow CMake invocations that build OpenCvSharpExtern to pass -D
NO_BARCODE=ON so src/CMakeLists.txt will skip barcode.cpp, or alternatively
re-enable BUILD_opencv_barcode in cmake/opencv_build_options.cmake; modify the
workflows that call cmake (or the opencv_build_options.cmake file) so both the
full OpenCV build and the wrapper agree on barcode support.
In `@src/OpenCvSharpExtern/CMakeLists.txt`:
- Around line 64-65: The Windows test project is missing the FFmpeg plugin DLL;
either add a CopyToOutputDirectory entry for "opencv_videoio_ffmpeg4130_64.dll"
into OpenCvSharp.Tests.Windows.csproj or modify the CMakeLists.txt post-build
copy logic (the block that sets TEST_OUTPUT_DIR and copies native artifacts) to
also copy that DLL into the OpenCvSharp.Tests.Windows output folder; update the
post-build copy targets to include the extra destination (or add the DLL include
to the OpenCvSharp.Tests.Windows.csproj) so the opencv_videoio_ffmpeg4130_64.dll
is present for Windows test runs.
---
Outside diff comments:
In @.github/workflows/linux-arm.yml:
- Around line 51-58: The restore and save cache keys must match and include the
cmake options file so cache hits work and option changes invalidate the cache:
update the key used in the "Restore OpenCV cache" step (id: opencv-cache) to be
identical to the key used in the save step (save job for OpenCV cache) and
append hashFiles('cmake/opencv_build_options.cmake') (or include that file path
in the same hashFiles call) so both restore and save use the full key including
the workflow file and the cmake/opencv_build_options.cmake hash.
In @.github/workflows/ubuntu-slim.yml:
- Around line 48-56: The cache key for the slim OpenCV cache (the step named
"Restore OpenCV cache (slim)" with id opencv-cache and the corresponding save
step) doesn't include the slim build configuration, so changes to
cmake/opencv_build_options_slim.cmake won't invalidate the cache; update both
the restore and save cache key expressions (the opencv-${{ env.OPENCV_VERSION
}}-ubuntu-${{ matrix.ubuntu }}-slim keys) to incorporate the
cmake/opencv_build_options_slim.cmake file (e.g., include its checksum or path
expansion in the key) so the cache is regenerated when that config changes.
In @.github/workflows/ubuntu.yml:
- Around line 59-67: The OpenCV cache key must include the build options file so
changes to cmake/opencv_build_options.cmake invalidate the cache; update the
cache key used in both the "Restore OpenCV cache" and the corresponding "Save
OpenCV cache" steps to append a hash of that file (e.g., include ${{
hashFiles('cmake/opencv_build_options.cmake') }}) alongside OPENCV_VERSION and
matrix.ubuntu so the cache is recomputed when the full build profile changes.
---
Nitpick comments:
In @.gitignore:
- Around line 122-125: Add ignore patterns for the newly generated build trees
so local untracked files don't appear: update the .gitignore to include
"src/build/" and "src/build-slim/" (and optionally "src/build-*" to catch
variations). Ensure these exact directory patterns match the generated folders
referenced in the diff (src/build, src/build-slim) and are placed in the
appropriate section of .gitignore so they take effect for local dev flows.
In @.gitmodules:
- Around line 4-6: Add an explicit branch entry to the opencv submodule
configuration so updates track the upstream branch; modify the [submodule
"opencv"] section (the entry with path = opencv and url =
https://github.com/opencv/opencv.git) to include a branch = <branch-name> (e.g.,
branch = 4.x or branch = master) that matches the project’s OpenCV
compatibility, then run git submodule sync and git submodule update --remote to
start tracking that branch.
- Around line 7-9: Add a branch tracking entry for the opencv_contrib submodule
in .gitmodules so it follows the same branch as the opencv submodule; update the
opencv_contrib submodule block (submodule "opencv_contrib") to include a branch
= <branch-name> line matching the opencv submodule's branch to simplify upstream
updates and keep compatibility.
- Around line 4-9: The .gitmodules lists large submodules "opencv" and
"opencv_contrib" which slow clones; update CI and contributor docs to perform
shallow submodule clones for these entries by changing the CI pipeline step that
initializes submodules to use depth 1 (e.g. run git submodule update --init
--depth 1 opencv opencv_contrib) and add a short note to CONTRIBUTING.md (or
README) describing how contributors should initialize these submodules after
cloning; reference the .gitmodules entries "opencv" and "opencv_contrib" when
making the changes so the CI job and docs target the correct submodules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 63e851cb-de5b-41ba-8d5a-83f369033b88
📒 Files selected for processing (17)
.github/workflows/linux-arm.yml.github/workflows/ubuntu-slim.yml.github/workflows/ubuntu.yml.github/workflows/windows.yml.gitignore.gitmodulesbuild_opencv_windows.ps1cmake/opencv_build_options.cmakecmake/opencv_build_options_slim.cmakedownload_opencv_windows.ps1download_tesseract_windows.ps1nuget/OpenCvSharp4.runtime.win.csprojnuget/OpenCvSharp4.runtime.win.slim.csprojopencvopencv_contribsrc/CMakeLists.txtsrc/OpenCvSharpExtern/CMakeLists.txt
💤 Files with no reviewable changes (2)
- download_tesseract_windows.ps1
- download_opencv_windows.ps1
…_environment_setup # Conflicts: # .gitmodules
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit