Update cmd.sh user_version.h sed command #35
Conversation
in light of changes made as part of nodemcu/nodemcu-firmware#2269
| @@ -57,7 +57,7 @@ cd /opt/nodemcu-firmware | |||
| # EXTRA_CCFLAGS="-DBUILD_DATE=... AND -DNODE_VERSION=..." to make turned into an escaping/expanding nightmare for which | |||
| # I never found a good solution | |||
| if [ "$CAN_MODIFY_VERSION" = true ]; then | |||
| sed -i 's/\(NodeMCU [^"]*\)/\1 built with Docker provided by frightanic.com\\n\\tbranch: '"$BRANCH"'\\n\\tcommit: '"$COMMIT_ID"'\\n\\tSSL: '"$SSL"'\\n/g' app/include/user_version.h | |||
| sed -i '/#define NODE_VERSION[[:space:]]/ s/$/ " built with Docker provided by frightanic.com\\n\\tbranch: '"$BRANCH"'\\n\\tcommit: '"$COMMIT_ID"'\\n\\tSSL: '"$SSL"'\\n"/g' app/include/user_version.h | |||
There was a problem hiding this comment.
The longer I think about this the more confused I get...don't get it. I see that it's correct, but why? Maybe the sed-ninja can explain that.
/#define NODE_VERSION[[:space:]]/is just the pattern to find the line to operate upon (I mean the one between the '/' is the pattern)- ' ' is purely cosmetic to enhance legibility
s/is thesedsubstitute- and then? does the
$/mean "substitute the EOL with what follows"?
In earlier NodeMCU versions we had something like #define NODE_VERSION "NodeMCU 1.5.4.1" and the whitespace before the " (which GitHub mangles) is a \t.
The 2.2 upgrade brings #define NODE_VERSION "NodeMCU " ESP_SDK_VERSION_STRING "." NODE_VERSION_XSTR(NODE_VERSION_INTERNAL) with two \t before the first ".
On the current dev version this would produce
NodeMCU 2.2.0.0 built with Docker provided by frightanic.com
branch: dev
commit: b81963a86deec5f4dc25e7d9f3883914095cf9dd
SSL: false
build created on 2018-04-04 21:05
powered by Lua 5.1.4 on SDK 2.2.1(cfd48f3)
which isn't so nice because of the "NodeMCU 2.2.0.0" vs. "SDK 2.2.1" but that's a firmware issue. Without this fix it would be
NodeMCU built with Docker provided by frightanic.com
branch: dev
commit: b81963a86deec5f4dc25e7d9f3883914095cf9dd
SSL: false
2.2.0.0 build created on 2018-04-04 23:17
powered by Lua 5.1.4 on SDK 2.2.1(cfd48f3)
There was a problem hiding this comment.
Yes, you've decoded the sed correctly. :)
There was a problem hiding this comment.
Hurray! This in turn means that the new expression is not backwards compatible.
There was a problem hiding this comment.
Does it? If the old line was #define NODE_VERSION\t\t"...", the new version will continue to match and append, I think?
There was a problem hiding this comment.
Hmmm, I guess not being a C developer I don't fully understand constraints of the .h syntax.
/tmp > echo '#define NODE_VERSION "NodeMCU 1.5.4.1"' |sed '/#define NODE_VERSION[[:space:]]/ s/$/ " built with Docker provided by frightanic.com"/g'
#define NODE_VERSION "NodeMCU 1.5.4.1" " built with Docker provided by frightanic.com"
The result looks fishy because of the " " . Or is this automatically concatenated at runtime?
There was a problem hiding this comment.
A relatively obscure feature of C is that adjacent string literals are, indeed, concatenated, thankfully, after pre-processing. If you're really curious, it's C11 5.1.1.2 bullet 6. ;)
There was a problem hiding this comment.
Apparently then, two string literals separated by a space are still considered adjacent. There I was thinking that string concatenation in PHP and Lua were obscure... Thanks for enlightening me 😜
There was a problem hiding this comment.
Mm, making this more (or less, depending on your view) exciting, this is strictly a compile-time thing. You can't concatenate strings at runtime this way. In any case, we good to merge this PR? :)
There was a problem hiding this comment.
Sorry, had to rush before I got around to actually push the button.
in light of changes made as part of
nodemcu/nodemcu-firmware#2269