LacunaSpace / basicmac

BasicMAC LoRaWAN stack that supports (but is not limited to) Arduino
Other
76 stars 18 forks source link

Export script error #21

Closed jpmeijers closed 3 years ago

jpmeijers commented 3 years ago

I did a checkout of the current master branch, and followed the steps to export an Arduino library. Running the command ends in a failure:

$ target/arduino/export.sh --link ~/Arduino/libraries/BasicMAC
target/arduino/export.sh: 45: local: Code/basicmac/target/arduino: bad variable name

Omitting --link seems to work.

matthijskooijman commented 3 years ago

Hm, the erronous line seems to be https://github.com/LacunaSpace/basicmac/blob/cfb7f3d8fbf3127a19c78b2799e3e864ab21be0f/target/arduino/export.sh#L45

But I don't understand what happens there. Your exact same command works fine here (Ubuntu Focal, using dash as shell). The Code/basicmac/target/arduino looks like the directory of the script, which is what $(cd "$(dirname "$1")" && pwd) is intended to return, though it also seems truncated somewhat, so maybe there is some special character in the path to your git clone?

Some additional questions:

jpmeijers commented 3 years ago

OS: Ubuntu 20.04.1 Shell: man sh open the dash manual. Path: /home/jpmeijers/Izinto/Arduino Code/basicmac - which contains a space and might be the cause.

matthijskooijman commented 3 years ago

Ah, I can reproduce that here.

What I think happens is that the local keyword actually does wordsplitting on its arguments after variable replacement (similar to how word-splitting works with other commands), so local SRCDIR=with space actually tries to declare two variables. This is different from a normal assignment (without local), which (IIRC) only does word-splitting before variable replacement.

An easy fix would be to just add quotes around the variable value, which fixes this. However, looking at the manpage for dash, it doesn't actually document that you can assign values as part of a local line, or have multiple lines. Looking at POSIX sh (which is what I'm actually targeting by specifying /bin/sh in the hashbang), it does not seem to support local at all (though apparently pretty much all practically used implementations support it).

Since this script is small enough to not really need local variables anyway, I've fixed this by just dropping the local keyword. Could you see if #22 works for you?

jpmeijers commented 3 years ago

22 seems to solve my error.