Conversation
Updated run.sh to use 'timeout' for all dpkg -l queries to prevent script hangs if the dpkg database is locked. Enhanced test-script.sh with a new test_dpkg_hang function and integrated it into the bug fix test suite to verify proper timeout handling for dpkg database hangs.
Bump displayed version in casaos-fix-docker-api-version/run.sh from 2025.11.0 to 2025.11.1 for consistency across script output and help messages.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughBash scripts: bumped displayed version to 2025.11.1; added Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to Reviews > Disable Cache setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)casaos-fix-docker-api-version/test-script.sh (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (6)
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
casaos-fix-docker-api-version/run.sh(8 hunks)casaos-fix-docker-api-version/test-script.sh(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
casaos-fix-docker-api-version/run.sh (1)
casaos-fix-docker-api-version/test-script.sh (2)
apply_docker_api_override(193-260)remove_docker_api_override(263-293)
casaos-fix-docker-api-version/test-script.sh (1)
casaos-fix-docker-api-version/run.sh (1)
print_info(28-59)
🪛 Shellcheck (0.11.0)
casaos-fix-docker-api-version/run.sh
[warning] 321-321: Declare and assign separately to avoid masking return values.
(SC2155)
casaos-fix-docker-api-version/test-script.sh
[warning] 1624-1624: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 1627-1627: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-fix-script
- GitHub Check: test-fix-script
🔇 Additional comments (9)
casaos-fix-docker-api-version/run.sh (5)
62-62: Version bump consistently applied across headers and outputs.The version string updates from 2025.11.0 to 2025.11.1 are correctly applied in all script headers and informational messages. No issues detected.
Also applies to: 1575-1575, 1583-1583, 1591-1591
93-93: Timeout wrapping applied to dpkg -l query in display_versions().The timeout 10 wrapper prevents hangs when the package database is locked. The fallback error message provides appropriate feedback. Implementation is correct.
965-965: Timeout correctly applied to dpkg check in remove_standalone_docker_compose().The timeout 10 wrapper prevents hangs when querying installed docker-compose packages. Error handling with the fallback path is appropriate.
1088-1088: Timeout wrapping applied to dpkg check in downgrade_docker().The timeout prevents hangs before removing existing Docker packages. The conditional flow correctly handles both timeout and non-timeout scenarios.
2024-2024: Timeout wrapping consistently applied to final dpkg output queries.Both the summary output and diagnostic information sections correctly wrap dpkg -l with timeout 10. Error messages provide appropriate user-facing feedback when timeouts occur.
Also applies to: 2080-2080
casaos-fix-docker-api-version/test-script.sh (4)
1597-1732: New test_dpkg_hang() function comprehensively validates dpkg timeout behavior.The test function follows the established pattern from other hang tests (test_snap_hang, test_casaos_hang) and correctly verifies:
- Timeout command exits with code 124 within expected timeframe (~10s)
- Mock dpkg hang is properly simulated
- display_versions()-like behavior handles timeout gracefully
- Fallback error message is displayed when timeout occurs
The implementation is thorough and well-structured.
1294-1294: Help text updated to include new test-dpkg command.The usage documentation correctly adds the new
test-dpkgcommand description at lines 1294 and 1320, maintaining consistency with other similar commands and providing clear instructions.Also applies to: 1320-1320
1920-1922: Command dispatcher correctly integrated for test-dpkg aliases.The case statement at lines 1920-1922 properly handles the
test-dpkg|dpkg|test-dpkg-hangcommand aliases and invokes test_dpkg_hang(), maintaining consistency with other test command patterns.
1751-1751: Test counter and sequencing updated correctly in test_all_bugfixes().The test numbering has been updated from "6 of 6" to "7 of 7" consistently throughout the test_all_bugfixes() function:
- Line 1751: Test 1 of 7 (GPG)
- Line 1767: Test 2 of 7 (GPG download)
- Line 1783: Test 3 of 7 (Snap)
- Line 1799: Test 4 of 7 (netns)
- Line 1815: Test 5 of 7 (daemon)
- Line 1831: Test 6 of 7 (CasaOS)
- Line 1847: Test 7 of 7 (dpkg)
The new test is properly integrated into the test suite with correct sequencing.
Also applies to: 1767-1767, 1783-1783, 1799-1799, 1815-1815, 1831-1831, 1847-1847
Updated variable assignment in run.sh and test-script.sh to use separate declaration and assignment lines, improving POSIX shell compatibility and readability.
Refactored test_dpkg_hang to declare local variables upfront, avoid SC2155 issues, and use explicit paths for the mock dpkg command. Updated PATH handling and hash table clearing to ensure the mock is used, and improved the test script to override dpkg for more reliable testing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
casaos-fix-docker-api-version/test-script.sh (1)
1597-1748: LGTM: Well-structured dpkg hang test with one SC2155 style fix needed.The new
test_dpkg_hang()function follows established patterns fromtest_snap_hang()andtest_casaos_hang(), correctly simulates dpkg database locks with a mock binary, and verifies timeout behavior within expected bounds. The upfront variable declarations at line 1605 properly address SC2155 concerns, separating declarations from assignments throughout the function.One minor refinement for consistency: line 1666 should not re-declare
test_scriptaslocalsince it was already declared at function scope on line 1605. This combines declaration + assignment, which contradicts the SC2155 pattern applied earlier in the function.Apply this diff to line 1666:
- # Extract just the function from run.sh - but use explicit mock path - local test_script="/tmp/test-dpkg-func-$$.sh" + # Extract just the function from run.sh - but use explicit mock path + test_script="/tmp/test-dpkg-func-$$.sh"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
casaos-fix-docker-api-version/test-script.sh(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
casaos-fix-docker-api-version/test-script.sh (1)
casaos-fix-docker-api-version/run.sh (1)
print_info(28-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-fix-script
- GitHub Check: test-fix-script
🔇 Additional comments (2)
casaos-fix-docker-api-version/test-script.sh (2)
1287-1331: Usage and help text updated correctly for new test command.Lines 1294 and 1320 add
test-dpkgdocumentation to both the main help output and usage examples, maintaining consistency with existing bug-fix test descriptions.
1750-1887: Test numbering and dispatcher integration correct.All references to "Test X of 6" have been updated to "Test X of 7" (lines 1760, 1767, 1783, 1799, 1815, 1831, 1847, 1863), and the new dpkg test is properly wired into
test_all_bugfixes()as test 7 of 7. The main function dispatcher (lines 1936-1938) correctly routes the three command aliases (test-dpkg,dpkg,test-dpkg-hang) totest_dpkg_hang.
The 'local' keyword was removed from the 'test_script' variable declaration in test-script.sh to ensure compatibility and correct scoping outside of functions.
1 participant
This pull request improves the reliability of the CasaOS Docker Version Fix scripts by adding robust handling for potential hangs in
dpkg -lcommands, which can occur if the package database is locked. The changes ensure that any calls todpkg -lare wrapped with a timeout, preventing the scripts from hanging indefinitely. Additionally, the test suite is enhanced to verify this behavior with a dedicated test fordpkghangs.Robust handling for dpkg hangs:
dpkg -linrun.share now wrapped withtimeout 10to prevent the script from hanging if the package database is locked. This applies to displaying Docker package versions, checking installed packages, and removing Docker-related packages. [1] [2] [3] [4] [5] [6]Test suite enhancements:
test_dpkg_hang, is added totest-script.shto simulate and verify handling of a hangingdpkg -lcommand using a mock dpkg binary and timeout logic.test-dpkgcommand and results. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]Other improvements:
2025.11.0to2025.11.1in all relevant output locations for clarity. [1] [2]These changes make the CasaOS Docker Version Fix scripts more robust and easier to test, especially in environments where package database locks are common.