AsahiLinux / widevine-installer

Widevine CDM installer for aarch64 systems
MIT License
97 stars 12 forks source link

Fix some bashisms, eg. on debian when the default shell is /bin/dash #1

Closed ashimokawa closed 1 year ago

ashimokawa commented 1 year ago
clementperon commented 1 year ago

Trying this with bash and got the same issues :+1:

ashimokawa commented 1 year ago

@clementperon

If you change the first line from #/bin/sh to #/bin/bash this will work without this patch. But making this run in other shells as well seems more desirable to me, hence the PR.

clementperon commented 1 year ago

@ashimokawa thanks did you perform other steps to make it work? I'm testing it on Ubuntu but doesn't work out of the box.

I think it's due too chromium installed via snap...

ashimokawa commented 1 year ago

@clementperon On debian I had to change

: ${LIBDIR:=/usr/lib64}

to

: ${LIBDIR:=/usr/lib}

But this is not scope of this PR. This is purely about compatibility for other shells, since "#/bin/sh" indicates it should run in other shells than bash also.

That being said, I think Ubuntu uses snaps for browsers which would probably need something different here. Making this work for other distributions is another problem which we should not discuss here.

leifliddy commented 1 year ago

I realize I'm late to the party, but technically speaking you should be using a different set of comparison operators for numbers https://stackabuse.com/comparing-numbers-in-bash/

ie

if [ "$COPY_CONFIGS" = 1 ]; then

should be

if [ "$COPY_CONFIGS" -eq 1 ]; then

Not sure if that's a "bashism" or not.....

marcan commented 1 year ago

Only if you decide "1" needs to be interpreted as a number there, and not a string. Both are equivalent and valid interpretations.

leifliddy commented 1 year ago

Sure...I mean strings are generally quoted though.