Skip to content
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

fix(convertfs): error in conditional expressions (bsc#1228847) (SL-Micro-6.0:Update) #356

Conversation

aafeijoo-suse
Copy link
Collaborator

With POSIX conditionals using single brackets, -n followed by a variable without quotes is always true.

Also, use $ROOT instead of hardcoding /sysroot.

With POSIX conditionals using single brackets, `-n` followed by a variable
without quotes is always true.

Also, use `$ROOT` instead of hardcoding `/sysroot`.
@aafeijoo-suse aafeijoo-suse requested a review from tblume as a code owner August 5, 2024 07:40
@@ -50,7 +50,7 @@ if [ ! -L "$ROOT"/var/lock -a -e "$ROOT"/var/lock ]; then
ln -sfn ../run/lock "$ROOT"/var/lock
fi

[ -n $SUBVOLIDVAR ] && umount $ROOT/var
[[ -n $SUBVOLIDVAR ]] && umount $ROOT/var
[ -w $ROOT ] && mount -o remount,ro $ROOT
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you also fix this like in the shellcheck commit in pr 355?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean adding also the whole shellcheck commit to all code streams or just this line? I haven't added the whole commit to old code streams on purpose because I think is more "a code improvement" than a bug fix, and it would be submitted via maintenance update, but I can submit it also if you want.

If your concern is the 2 lines with [ -w $ROOT ], the script exits at the beginning if $ROOT is not a directory.

if [[ ! -d $ROOT ]]; then
echo "Usage: $0 <rootdir>"
exit 1
fi

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, just wanted to avoid possible errors due to missing double square brackets.
But if it's only cosmetic I'm fine without.

Copy link
Collaborator

@tblume tblume left a comment

Choose a reason for hiding this comment

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

Ok

@aafeijoo-suse aafeijoo-suse merged commit a89da38 into openSUSE:SL-Micro-6.0_Update Aug 7, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants