Fix(Warnings): change XFS and ReiserFS messages#2561
Fix(Warnings): change XFS and ReiserFS messages#2561SimonFair wants to merge 5 commits intounraid:masterfrom
Conversation
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdated ReiserFS and XFS v4 deprecation warnings to include a computed human-readable timeline toward a fixed deadline of 2030-10-01; the timeline (years/months or “as soon as possible” if past the deadline) is appended to array and pool migration recommendation texts while preserving existing severities. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/plugins/dynamix/include/Helpers.php`:
- Line 322: Update the grammatical error in the ReiserFS deprecation message in
Helpers.php: find the array entry with the key 'message' containing "ReiserFS is
deprecated and is no longer supported in Unraid. You will need to downgrade to
Unraid 7.2 to action" (occurrences near the existing entries at lines 322 and
685) and replace the trailing phrase "to action" with a correct form such as "to
take action" (i.e., "You will need to downgrade to Unraid 7.2 to take action")
in both places so the message reads clearly.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
emhttp/plugins/dynamix/include/Helpers.php
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
emhttp/plugins/dynamix/include/Helpers.php (1)
685-685:⚠️ Potential issue | 🟡 MinorFix the remaining ReiserFS message typo for consistency.
Line 685 still says “to action”. It should match Line 322 (“to take action”) to avoid inconsistent user-facing text.
Suggested fix
- 'message' => 'ReiserFS is deprecated and is no longer supported in Unraid. You will need to downgrade to Unraid 7.2 to action' + 'message' => 'ReiserFS is deprecated and is no longer supported in Unraid. You will need to downgrade to Unraid 7.2 to take action'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/include/Helpers.php` at line 685, Fix the typo in the user-facing ReiserFS deprecation message: locate the message string (the array entry with key 'message' containing "ReiserFS is deprecated and is no longer supported in Unraid. You will need to downgrade to Unraid 7.2 to action") in Helpers.php and change the trailing phrase to "to take action" so it matches the other occurrence (line with "to take action") for consistent wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/plugins/dynamix/include/Helpers.php`:
- Around line 822-835: The deadline DateTime is set to '2030-09-30' which
represents midnight at the start of that day, so comparisons with $now exclude
the whole 30th; update the $deadline creation in Helpers.php (the new
DateTime('2030-09-30') instances) to represent the end of September 30th (e.g.
new DateTime('2030-09-30 23:59:59') or new DateTime('2030-10-01') and use a
strict < comparison accordingly) so that the entire day is included; make the
same change for the other occurrence identified around the $deadline usage at
the later block (the second instance near line 870).
---
Duplicate comments:
In `@emhttp/plugins/dynamix/include/Helpers.php`:
- Line 685: Fix the typo in the user-facing ReiserFS deprecation message: locate
the message string (the array entry with key 'message' containing "ReiserFS is
deprecated and is no longer supported in Unraid. You will need to downgrade to
Unraid 7.2 to action") in Helpers.php and change the trailing phrase to "to take
action" so it matches the other occurrence (line with "to take action") for
consistent wording.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
emhttp/plugins/dynamix/include/Helpers.php
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
emhttp/plugins/dynamix/include/Helpers.php (1)
330-333:⚠️ Potential issue | 🟠 MajorFix mounted-check condition:
exec()returns empty string when command produces no output.The
mountpoint -qcommand produces no STDOUT, soexec()returns an empty string (which evaluates to false in boolean context). The current condition fails at the first&&even when$ret == 0, preventing XFS v4 detection from running. Split the execution from return-code checking.🛠️ Proposed fix
- if (is_dir($mountPoint) && exec("mountpoint -q " . escapeshellarg($mountPoint) . " 2>/dev/null", $output, $ret) && $ret == 0) { - // Check for crc=0 which indicates XFS v4 - $xfsInfo = shell_exec("xfs_info " . escapeshellarg($mountPoint) . " 2>/dev/null"); - if ($xfsInfo && strpos($xfsInfo, 'crc=0') !== false) { - $warnings[] = [ - 'type' => 'xfs_v4', - 'severity' => 'critical', - 'message' => _('XFS v4 is deprecated and will not be supported in future Unraid releases. Please migrate to XFS immediately.') - ]; - } - } + if (is_dir($mountPoint)) { + exec("mountpoint -q " . escapeshellarg($mountPoint) . " 2>/dev/null", $output, $ret); + if ($ret == 0) { + // Check for crc=0 which indicates XFS v4 + $xfsInfo = shell_exec("xfs_info " . escapeshellarg($mountPoint) . " 2>/dev/null"); + if ($xfsInfo && strpos($xfsInfo, 'crc=0') !== false) { + $warnings[] = [ + 'type' => 'xfs_v4', + 'severity' => 'critical', + 'message' => _('XFS v4 is deprecated and will not be supported in future Unraid releases. Please migrate to XFS immediately.') + ]; + } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/include/Helpers.php` around lines 330 - 333, The mounted-check condition incorrectly relies on exec()'s return value (which is an empty string for mountpoint -q) causing the XFS v4 detection to be skipped; update the block around is_dir($mountPoint) so you first call exec("mountpoint -q " . escapeshellarg($mountPoint) . " 2>/dev/null", $output, $ret) and then check $ret === 0 (instead of the truthiness of exec's return) before running shell_exec("xfs_info " . escapeshellarg($mountPoint) ...) and the strpos(..., 'crc=0') check; ensure you still use escapeshellarg and preserve the existing $xfsInfo and crc=0 logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/plugins/dynamix/include/Helpers.php`:
- Line 337: Multiple XFS v4 helper paths in Helpers.php use inconsistent policy
text/severity; pick a single policy and apply it to all occurrences. Update the
XFS v4-related array entries (the entries containing the 'message' and
'severity' keys for XFS v4 in Helpers.php) so all three locations use the same
severity and message text (e.g., change the message at the occurrence currently
using "critical + migrate immediately" to match the other occurrences' "notice"
severity and "migrate before end of September 2030" wording), ensuring the
'message' string and 'severity' value are identical across the entries that
reference XFS v4.
---
Outside diff comments:
In `@emhttp/plugins/dynamix/include/Helpers.php`:
- Around line 330-333: The mounted-check condition incorrectly relies on
exec()'s return value (which is an empty string for mountpoint -q) causing the
XFS v4 detection to be skipped; update the block around is_dir($mountPoint) so
you first call exec("mountpoint -q " . escapeshellarg($mountPoint) . "
2>/dev/null", $output, $ret) and then check $ret === 0 (instead of the
truthiness of exec's return) before running shell_exec("xfs_info " .
escapeshellarg($mountPoint) ...) and the strpos(..., 'crc=0') check; ensure you
still use escapeshellarg and preserve the existing $xfsInfo and crc=0 logic.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
emhttp/plugins/dynamix/include/Helpers.php
| 'type' => 'xfs_v4', | ||
| 'severity' => 'critical', | ||
| 'message' => _('XFS v4 is deprecated and will not be supported in future Unraid releases. Please migrate to XFS v5 immediately') | ||
| 'message' => _('XFS v4 is deprecated and will not be supported in future Unraid releases. Please migrate to XFS immediately.') |
There was a problem hiding this comment.
Unify XFS v4 policy text and severity across helper paths.
Line 337 says critical + migrate immediately, while Line 705/Line 870 says notice + migrate before end of September 2030. Users can receive conflicting guidance depending on UI path. Please align to one policy message/severity model.
Also applies to: 705-705, 870-870
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@emhttp/plugins/dynamix/include/Helpers.php` at line 337, Multiple XFS v4
helper paths in Helpers.php use inconsistent policy text/severity; pick a single
policy and apply it to all occurrences. Update the XFS v4-related array entries
(the entries containing the 'message' and 'severity' keys for XFS v4 in
Helpers.php) so all three locations use the same severity and message text
(e.g., change the message at the occurrence currently using "critical + migrate
immediately" to match the other occurrences' "notice" severity and "migrate
before end of September 2030" wording), ensuring the 'message' string and
'severity' value are identical across the entries that reference XFS v4.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
emhttp/plugins/dynamix/include/Helpers.php (1)
337-337:⚠️ Potential issue | 🟠 MajorAlign XFS v4 severity/message across helper paths
Line 337 still emits
critical+ “migrate ... immediately”, while the UI flow at Line 870 (backed by the notice payload around Line 705) communicates a non-critical, deadline-based migration. This can show conflicting guidance depending on which helper is used.Suggested alignment
- 'severity' => 'critical', - 'message' => _('XFS v4 is deprecated and will not be supported in future Unraid releases. Please migrate to XFS v5 immediately.') + 'severity' => 'notice', + 'message' => _('XFS v4 is deprecated and will not be supported in future Unraid releases. You have until 2030 to migrate to XFS v5.')Also applies to: 870-870
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/include/Helpers.php` at line 337, The XFS v4 notice emitted in Helpers.php (the array with 'message' => 'XFS v4 is deprecated...') is using a critical severity and an "immediately" message that conflicts with the non-critical, deadline-based migration flow used elsewhere (around the helper that emits the notice at line 870 / payload near 705). Update the helper emitting that array so its severity matches the other path (change from 'critical' to the same level used at line 870, e.g., 'warning'/'notice') and revise the message text to the deadline-based wording used in the other helper (remove "immediately" and mirror the existing notice payload wording) so both helpers present identical severity and guidance; locate the array/object in Helpers.php that builds the XFS v4 notice and make these two synchronized changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@emhttp/plugins/dynamix/include/Helpers.php`:
- Line 337: The XFS v4 notice emitted in Helpers.php (the array with 'message'
=> 'XFS v4 is deprecated...') is using a critical severity and an "immediately"
message that conflicts with the non-critical, deadline-based migration flow used
elsewhere (around the helper that emits the notice at line 870 / payload near
705). Update the helper emitting that array so its severity matches the other
path (change from 'critical' to the same level used at line 870, e.g.,
'warning'/'notice') and revise the message text to the deadline-based wording
used in the other helper (remove "immediately" and mirror the existing notice
payload wording) so both helpers present identical severity and guidance; locate
the array/object in Helpers.php that builds the XFS v4 notice and make these two
synchronized changes.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
emhttp/plugins/dynamix/include/Helpers.php
2 participants
Summary by CodeRabbit