Open erenfro opened 1 year ago
Hang on, this requires at least Bash 4.0 to work, so I may have to go with the more "POSIX" friendly version of lowercasing.
yadm doesn't currently have a dependency on tr
, but it does on awk
, so perhaps it would be better to use:
"${AWK_PROGRAM[0]}" '{print tolower($0)}'
instead of tr
?
This also allows us to remove the unneeded pipe |
operation, so
value="$(echo $value | tr '[:upper:]' '[:lower:]')"
becomes edited
value=$("${AWK_PROGRAM[0]}" '{print tolower($0)}' <<<"${value}")
and
echo "$distro" | tr '[:upper:]' '[:lower:]'
becomes edited
"${AWK_PROGRAM[0]}" '{print tolower($0)}' <<<"${distro}"
awk is viable as well, yes. I could also fix #430 as well, with a method I looked over, just a couple lines that looks at ID_LIKE and ID, which is literally a simple 3-line change. I'll make these changes and commit and let you see. :)
Using the "${AWK_PROGRAM[0]}" '{print tolower($0)}' "${distro}"
caused a surge of new issues with it trying to open files named various "fedora", "arch", "linuxmint", "opensuse", "pop" in my case, with "No such file or directory", so I reverted to the echo | awk method which is POSIX standard, and it's working.
Sorry, that should have read
"${AWK_PROGRAM[0]}" '{print tolower($0)}' <<<"${distro}"
<<<
is supported by bash 3
Done, tested, and verified. Thankfully I have a MacBook Pro with bash 3 handy to test with. :)
@erenfro I don't know why, but neither
tr '[:upper:]' '[:lower:]'
or
tr '[[:upper:]]' '[[:lower:]]'
work on OpenWRT's BusyBox v1.33.2 (2023-04-14 11:34:26 UTC) multi-call binary.
Fortunately, awk's tolower()
does work.
Of course tr '[A-Z]' '[a-z]'
works as well.
I like this change. I think using awk seems like the best route (@rasa thanks for the input about OpenWRT).
I think that maybe this needs to be broadened. With this change distro and distro_family are case insensitive. Perhaps this should be extended to: arch, os, hostname, user, and class. Would those also make sense to be case insensitive?
Yep I could agree with that, arch, user, and class. I'll look into that today and see about updating the PR if you'd like. And also, do you want me to squash commits, (and if so, reference of how to do so if plausible, since I've only ever done that once LOL)
You don't have to squash the commits. The bigger challenge will be updating the tests. (as is always the case)
I've thought about this some more, and I've decided to create a Bash 3 compatible function that uses printf
. Today, yadm doesn't have any actual Awk dependency unless you use the built in templates, and I'd like it to remain dependency free.
The basic approach will be to iterate over every char of the string, and determine the ascii value:
ascii_val=$(printf "%d" "'$char'")
Then only change the chars with values between 65 & 90 (adding 32 and converting them to lower case).
I think this should be good for these values, and not require Awk.
This works for me, and is bash 3 compliant:
lc() {
local i=0 c
while printf -v c %d "'${1:(i++):1}"; do
((c)) || break
((c>64&&c<91)) && ((c+=32))
printf "\x$(printf %x $c)"
done
}
((${BASH_VERSINFO[0]}>3)) && function lc { printf %s "${1,,}"; }
and appears to work with non-ASCII (UTF-8) strings.
@erenfro I don't know why, but neither
tr '[:upper:]' '[:lower:]'
ortr '[[:upper:]]' '[[:lower:]]'
work on OpenWRT's BusyBox v1.33.2 (2023-04-14 11:34:26 UTC) multi-call binary. Fortunately, awk'stolower()
does work. Of coursetr '[A-Z]' '[a-z]'
works as well.
Looks like, since yadm is using /bin/sh which means POSIX compliance, the only real POSIX compliant solution that seems to work globally is basically utilizing awk's tolower() method.
echo "$a" | awk '{print tolower($0)}'
As local
is not POSIX compliant, and I can't tell for sure if using tr '[A-Z]' '[a-z]'
is compliant since the standard is more on the use of tr '[:upper:]' '[:lower:]'
.
Either way, this problem continues to hit me still to this day, dealing with working on various servers both ID=debian and ID_LIKE=debian, and this not following form. :)
Looks like, since yadm is using /bin/sh which means POSIX compliance…
@erenfro While the yadm command does begin with #!/bin/sh
(for compatibility), it immediately executes exec bash
here.
Note that yadm does not use any bash 4+ features, as bash 3 is often installed on MacOS. It doesn’t depend on /bin/sh
, or POSIX compliance. See here.
As
local
is not POSIX compliant.
local
is available in bash 3. See http://tiswww.case.edu/php/chet/bash/NEWS .
What does this PR do?
It allows differences between lsb_release and os-release to continue to work by simply comparing values of key in filename and value from detection in lowercase, gracefully allowing prior existing cases to work as they should.
What issues does this PR fix or reference?
455
430
Previous Behavior
Comparison was using lsb_release, which formalized output -OR- went to os-release which ID is always lowercase per spec, causing differences in results based on which method was used for detection. It also accounts for distro_family that is using ID_LIKE, or if not available would revert to ID for base distros.
New Behavior
Comparison lowercases the value provided from the alternative processor, and from the output of the distro detection, to provide consistent matching behavior that works, now and for all previous use-cases.
Have tests been written for this change?
No
Have these commits been signed with GnuPG?
Yes
Please review yadm's Contributing Guide for best practices.