Fixes #26930: Fix false positive in prerequisite check#26931
Fixes #26930: Fix false positive in prerequisite check#26931oluies wants to merge 2 commits intoopen-metadata:mainfrom
Conversation
the script reported "All prerequisites are met" despite missing dependencies. The issue was caused by calling check functions within subshells, which prevented the global status variable from updating.
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review ✅ ApprovedFixes false positive in prerequisite check that was incorrectly reporting all prerequisites as met. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Here is the completed PR description based on the fix we discussed. You can copy and paste this directly into your GitHub PR:
Describe your changes:
Fixes #26930
The
check_prerequisites.shscript had a logic flaw where it used command substitution$(...)to call thecheck_command_existencefunction. In Bash, this spawns a subshell, meaning any updates to the globalcodevariable (used to track failures) were lost once the subshell terminated. Consequently, the script would print errors for missing tools but still conclude with "✓ All prerequisites are met" and exit with a success code (0).Changes made:
codevariable to persist.check_versionto use local variables correctly, preventing logic errors during version comparison.whichcommand with the more portablecommand -v.How I tested these changes:
javaandantlr4and ran./scripts/check_prerequisites.sh.✗ ERROR: Some prerequisites are not met.at the end.1(or2) instead of0when dependencies are missing.Type of change:
Checklist:
I have read the [CONTRIBUTING](https://docs.open-metadata.org/developers/contribute) document.
My PR title is
Fixes #<ISSUE_NUMBER>: Fix false positive in prerequisite checkI have commented on my code, particularly in hard-to-understand areas.
I have verified the fix by running the script in an environment with missing dependencies.