Remove escaping of backslashes to support literal ${var} #218
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fhunleth
force-pushed
the
escape-dollars
branch
5 times, most recently
from
7f41e95 to
6a809d8
Compare
THIS IS A BACKWARDS INCOMPATIBLE CHANGE THAT AFFECTS FW FILE CREATION.
It does not affect processing of fw files so you don't need to worry
about change fwup versions on existing devices or worry about applying
.fw files created with old versions of fwup.
Automatic escaping of backslashes made it impossible to write `${var}`
to a U-Boot variable. Here's why. When doing this, you have to remember
that `fwup` evaluates variable substitution twice - once when making the
.fw file and once when applying it. You obviously have to escape the `$`
when creating the .fw file. That made sense. Then to survive the apply
step, you'd think that you could double escape the `$`. You'd be wrong,
though, since `fwup` was escaping the backslashes that you were adding
writing the configuration to the .fw file. Therefore, the variable
substitution was guaranteed to happen since you couldn't double escape a
`$`.
Amazingly, the auto-escaping behavior was only tested in the regression
tests via the exec test on Windows - a combination that would never be
used for real.
When you run into this issue, it's weird enough to be pretty confusing,
imho. Hopefully that and how rare should have been in real uses cases
makes this something that no one actually did.
This changes string processing to not automatically escape backslashes
when creating the .fw file. It is possible to escape a `$` through to
the end. This allows you to write a U-Boot environment variable with a
`${var}` in it. This also locks down string processing behaviors by
adding unit tests.
fhunleth
force-pushed
the
escape-dollars
branch
from
6a809d8 to
545d54d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
THIS IS A BACKWARDS INCOMPATIBLE CHANGE. THE PR IS IN DRAFT MODE TO SEE IF IT
ACTUALLY AFFECTS REAL USE CASES AND TO THINK ABOUT MAKING IT OPT IN.
Automatic escaping of backslashes made it impossible to write
${var}to aU-Boot variable. When doing this, you have to remember that
fwupevaluatesvariable substitution twice - once when making the .fw file and once when
applying it. You obviously have to escape the
$when creating the .fw file.That made sense. Then to survive the apply step, you'd think that you could
double escape the
$. You'd be wrong, though, sincefwupwas escaping thebackslashes that you were adding. Therefore, the variable substitution was
guaranteed to happen since you couldn't double escape a
$.Amazingly, this behavior was never tested in the regression tests. When you run
into it, it's weird enough to be pretty confusing, imho. Hopefully that and how
rare should have been in real uses cases makes this something that no one
actually did.
This changes string processing to not automatically escape backslashes so that
it is possible to escape a
$through to the end. This allows you to write aU-Boot environment variable with a
${var}in it. This locks down stringprocessing behaviors by adding unit tests.