meson: verify that all shell_versions match gnome_shell_req
#1514
Conversation
amezin
force-pushed
the
meson-shell-versions-check
branch
from
2625685 to
b29ee21
Compare
📝 WalkthroughWalkthroughA runtime validation is added to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
meson.build
🧰 Additional context used
🪛 GitHub Check: ci / bundle
meson.build
[failure] 51-51:
Problem encountered: Shell version 46.alpha does not match >=46.0
[failure] 51-51:
Problem encountered: Shell version 46.alpha does not match >=46.0
| foreach shell_version : shell_versions | ||
| if shell_version.contains('.') | ||
| shell_versions_expanded += [shell_version] | ||
| else | ||
| foreach suffix : ['0', '1', '9', 'alpha', 'beta', 'rc'] | ||
| shell_versions_expanded += f'@shell_version@.@suffix@' | ||
| endforeach | ||
| endif | ||
| endforeach |
There was a problem hiding this comment.
Pre-release version suffixes cause validation to fail.
The expansion logic includes 'alpha', 'beta', and 'rc' suffixes, which create pre-release versions (e.g., 46.alpha) that semantically come before the corresponding release version (e.g., 46.0). In version comparison semantics, 46.alpha < 46.0, so these pre-release versions fail the >=46.0 requirement.
This is causing the CI failure: Shell version 46.alpha does not match >=46.0
To validate that short versions like "46" fall within the required range, you only need to check the .0 release version. The additional minor versions ('1', '9') and pre-release suffixes are unnecessary and cause false negatives.
🔎 Proposed fix
Remove the problematic suffixes and only check .0 for short versions:
foreach shell_version : shell_versions
if shell_version.contains('.')
shell_versions_expanded += [shell_version]
else
- foreach suffix : ['0', '1', '9', 'alpha', 'beta', 'rc']
- shell_versions_expanded += f'@shell_version@.@suffix@'
- endforeach
+ shell_versions_expanded += f'@[email protected]'
endif
endforeach📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach shell_version : shell_versions | |
| if shell_version.contains('.') | |
| shell_versions_expanded += [shell_version] | |
| else | |
| foreach suffix : ['0', '1', '9', 'alpha', 'beta', 'rc'] | |
| shell_versions_expanded += f'@shell_version@.@suffix@' | |
| endforeach | |
| endif | |
| endforeach | |
| foreach shell_version : shell_versions | |
| if shell_version.contains('.') | |
| shell_versions_expanded += [shell_version] | |
| else | |
| shell_versions_expanded += f'@[email protected]' | |
| endif | |
| endforeach |
🤖 Prompt for AI Agents
In meson.build around lines 38 to 46 the expansion of short shell versions
incorrectly adds pre-release suffixes ('alpha','beta','rc') and unnecessary
minor versions, producing versions like "46.alpha" which compare lower than
"46.0" and fail the ">=46.0" check; change the expansion so that when a short
version (no dot) is encountered only the ".0" release variant is added (remove
'alpha','beta' and other extra suffixes and minor variants) so validation checks
use e.g. "46.0" only.
| foreach shell_version : shell_versions_expanded | ||
| foreach shell_version_req : gnome_shell_req | ||
| if not shell_version.version_compare(shell_version_req) | ||
| error(f'Shell version @shell_version@ does not match @shell_version_req@') | ||
| endif | ||
| endforeach | ||
| endforeach |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider simplifying the validation approach.
Once the expansion logic is fixed to only include .0 versions for short version strings, this nested loop validation will work correctly. However, checking one representative version (e.g., "46.0") per short version (e.g., "46") is sufficient to verify that the shell version falls within the required range.
The current nested loop approach is correct but potentially over-engineered for this use case.
🧰 Tools
🪛 GitHub Check: ci / bundle
[failure] 51-51:
Problem encountered: Shell version 46.alpha does not match >=46.0
[failure] 51-51:
Problem encountered: Shell version 46.alpha does not match >=46.0
🤖 Prompt for AI Agents
In meson.build around lines 48 to 54, the nested foreach loops validate every
expanded shell version against every requirement which is more complex than
necessary; instead, after fixing expansion to produce only .0 variants for short
versions, reduce validation to check a single representative expanded version
per short version (e.g., "46.0" for "46") against the gnome_shell_req list:
deduplicate or map short->representative expanded version and iterate that
reduced set, performing version_compare against the requirements and erroring if
any representative version does not satisfy the requirement.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.