grub-btrfs icon indicating copy to clipboard operation
grub-btrfs copied to clipboard

Fix bashism

Open bastien-roucaries opened this issue 1 year ago • 6 comments

Hi,

Could you please check and merge this ?

For debian we will like to only use dash for this package

bastien-roucaries avatar Jan 27 '24 17:01 bastien-roucaries

Note that it is an incomplete fix bashism

bastien-roucaries avatar Jan 27 '24 17:01 bastien-roucaries

Thanks for your contribution. Shouldn't the shebang then be /bin/sh as well?

https://github.com/Antynea/grub-btrfs/blob/465b56107f1a9a983e132d17441c2d63cdc16392/41_snapshots-btrfs#L1C1-L1C21

Schievel1 avatar Jan 29 '24 09:01 Schievel1

@bastien-roucaries could you explain that a bit more? You haven't fixed all bashisms in this, yet you did enough for dash to work with this. Does that mean dash works with some bashisms, not all?

I still have this error when doing shellcheck -s sh:

In 41_snapshots-btrfs line 301:
        local path_snapshot=${snap[@]:13:${#snap[@]}}
        ^-----------------^ SC3043 (warning): In POSIX sh, 'local' is undefined.
                            ^-----------------------^ SC2068 (error): Double quote array expansions to avoid re-splitting elements.
                            ^-----------------------^ SC2124 (warning): Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.

When doing shellcheck -s dash I get tons of errors like this:

In 41_snapshots-btrfs line 548:
    name_microcode=("${list_ucode[@]##*"/"}")
                   ^------------------------^ SC3030 (error): In dash, arrays are not supported.

Schievel1 avatar Feb 19 '24 13:02 Schievel1

@Schievel1 it is a first pass for eliminating bashism. For array I will need more review

bastien-roucaries avatar Feb 19 '24 13:02 bastien-roucaries

Ok, I get it. Then you will follow up with more PRs regarding this? This is a ton of work to work around the arrays here, isn't it?}

Schievel1 avatar Feb 19 '24 13:02 Schievel1

@Schievel1 not necesseraily if we could get the code clearer by refactoring

bastien-roucaries avatar Feb 19 '24 13:02 bastien-roucaries

Thanks for your contribution. I am looking forward to test following PRs from you that purge bashisms :)

Schievel1 avatar Mar 06 '24 13:03 Schievel1