Skip to content

Fix config format outputted by preinstall script #1149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 2, 2025

Conversation

dhurley
Copy link
Collaborator

@dhurley dhurley commented Jun 30, 2025

Proposed changes

Fix config format outputted by preinstall script.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • I have run make install-tools and have attached any dependency changes to this pull request
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • If applicable, I have updated any relevant documentation (README.md)
  • If applicable, I have tested my cross-platform changes on Ubuntu 22, Redhat 8, SUSE 15 and FreeBSD 13

@dhurley dhurley self-assigned this Jun 30, 2025
@dhurley dhurley requested a review from a team as a code owner June 30, 2025 10:49
@dhurley dhurley added the v3.x Issues and Pull Requests related to the major version v3 label Jun 30, 2025
@github-actions github-actions bot added bug Something isn't working dependencies labels Jun 30, 2025
@@ -130,8 +130,13 @@ command:
skip_verify: false
"

echo "$v3_config_contents" > "$v3_config_file" \
|| err_exit "Failed to write v3 config"
if [ -n "$( echo -e )" ]; then
Copy link
Contributor

@sean-breen sean-breen Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure this will only fail when echo itself isn't found

sh-3.2$ echo -e
-e
sh-3.2$ echo $?
0

|| err_exit "Failed to write v3 config"
if [ -n "$( echo -e )" ]; then
echo "$v3_config_contents" > "$v3_config_file" \
|| err_exit "Failed to write v3 config"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this err_exit condition will never be executed since the echo won't fail even with empty string etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sh-3.2$ echo "" > i_dont_exist
sh-3.2$ echo $?
0

@dhurley dhurley merged commit 4e44ae5 into main Jul 2, 2025
55 of 56 checks passed
@dhurley dhurley deleted the fix-preinstall-script-config branch July 2, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies v3.x Issues and Pull Requests related to the major version v3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants