Warn on empty continuation lines only, not on comment-only lines #35004
Conversation
This file contains hidden or 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
tianon
reviewed
Sep 27, 2017
builder/dockerfile/parser/parser.go
Outdated
| @@ -331,8 +335,12 @@ func trimWhitespace(src []byte) []byte { | |||
| return bytes.TrimLeftFunc(src, unicode.IsSpace) | |||
| } | |||
|
|
|||
| func isComment(line []byte) bool { | |||
| return bytes.HasPrefix(trimWhitespace(line), []byte("#")) | |||
There was a problem hiding this comment.
Do you think this ought to re-use tokenComment to keep a single-source-of-truth, or maybe it doesn't matter?
There was a problem hiding this comment.
oh, good one, yes, can do
Commit 8d1ae76 added deprecation warnings for empty continuation lines, but also treated comment-only lines as empty. This patch distinguishes empty continuation lines from comment-only lines, and only outputs warnings for the former. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
dont-warn-for-comment-only-lines
branch
from
8ae6836 to
2fd736a
Compare
|
@tianon done 👍 |
|
Looks like a much better solution than mine. It's much clearer what the patch is changing, and why. |
dgroup
pushed a commit
to dgroup/threecopies
that referenced
this pull request
Sep 29, 2017
During docker image building procedure you may observe warning `[WARNING]: Empty continuation line found`. This is docker bug, see more details here: - moby/moby#35004 - appropriate/docker-jetty#79
|
For future reference; this patch was included in docker 17.10 docker-archive/docker-ce@f6d296e |
|
Since 17.09 is a "stable" release, would it make sense to add this to the pile of potential-backport-candidates for a future 17.09.1? 😇 |
|
@tianon I'll add it to the pile for consideration, but it may be considered "non-critical" as the issue doesn't break the build (but, yes, it's confusing/annoying) |
|
Thanks! Definitely agree with that assessment -- I just want to make sure it's in the pile of things being considered if something critical enough to warrant a 17.09.1 does come up. 👍 |
|
Hurray! I just updated to the latest docker-for-mac edge build, and can confirm that I no longer get giant sees of red text in my |
AlexGoico
added a commit
to AlexGoico/docker-pygit2
that referenced
this pull request
Nov 15, 2017
There are warnings that would be emitted because of empty line continuation characters. This commit should eliminate the warnings in the next release of docker. See: moby/moby#35004
mrled
added a commit
to mrled/psyops
that referenced
this pull request
Jan 7, 2018
Grumble, grumble Fixes warnings based on: * moby/moby#33719 * moby/moby#35004
dcarley
added a commit
to CircleCI-Archived/docker-coreos-toolbox-ubuntu
that referenced
this pull request
Mar 29, 2019
This was appearing in the build output:
[WARNING]: Empty continuation line found in:
RUN apt-get update && apt-get upgrade -y && addgroup --gid 500 core && adduser --home /home/core --shell /bin/bash --uid 500 --gid 500 --disabled-password --system core && addgroup --gid 253 fleet && adduser core fleet && addgroup --gid 233 coredocker && adduser core coredocker && addgroup --gid 248 core-systemd-journal && adduser core core-systemd-journal && DEBIAN_FRONTEND=noninteractive apt-get install -y tzdata sudo net-tools inetutils-ping bash-completion mc tmux openssh-client && apt-get clean && ln -s /lib/x86_64-linux-gnu/libdevmapper.so.1.02.1 /lib/x86_64-linux-gnu/libdevmapper.so.1.02 && sed -i "s/%!s(MISSING)udo\s*ALL=(ALL:ALL)\s*ALL/%!s(MISSING)udo ALL=(ALL:ALL) NOPASSWD:ALL/" /etc/sudoers && adduser core sudo && mv /etc/bash.bashrc /etc/bash.basrc.coreos && echo ". /etc/bash.bashrc.coreos\nPS1=\"\\[\\033[01;35m\\]toolbox\\[\\033[01;34m\\] \$PS1\"" >/etc/bash.bashrc
[WARNING]: Empty continuation lines will become errors in a future release.
This should be fixed by using a later version of Docker to build per
moby/moby#35004
dcarley
added a commit
to CircleCI-Archived/docker-coreos-toolbox-ubuntu
that referenced
this pull request
Mar 29, 2019
This was appearing in the build output:
[WARNING]: Empty continuation line found in:
RUN apt-get update && apt-get upgrade -y && addgroup --gid 500 core && adduser --home /home/core --shell /bin/bash --uid 500 --gid 500 --disabled-password --system core && addgroup --gid 253 fleet && adduser core fleet && addgroup --gid 233 coredocker && adduser core coredocker && addgroup --gid 248 core-systemd-journal && adduser core core-systemd-journal && DEBIAN_FRONTEND=noninteractive apt-get install -y tzdata sudo net-tools inetutils-ping bash-completion mc tmux openssh-client && apt-get clean && ln -s /lib/x86_64-linux-gnu/libdevmapper.so.1.02.1 /lib/x86_64-linux-gnu/libdevmapper.so.1.02 && sed -i "s/%!s(MISSING)udo\s*ALL=(ALL:ALL)\s*ALL/%!s(MISSING)udo ALL=(ALL:ALL) NOPASSWD:ALL/" /etc/sudoers && adduser core sudo && mv /etc/bash.bashrc /etc/bash.basrc.coreos && echo ". /etc/bash.bashrc.coreos\nPS1=\"\\[\\033[01;35m\\]toolbox\\[\\033[01;34m\\] \$PS1\"" >/etc/bash.bashrc
[WARNING]: Empty continuation lines will become errors in a future release.
This should be fixed by using a later version of Docker to build per
moby/moby#35004
phred
added a commit
to phred/smolboi
that referenced
this pull request
Aug 26, 2023
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.
Alternative approach to #34333
Commit 8d1ae76 added deprecation warnings for empty continuation lines, but also treated comment-only lines as empty.
This patch distinguishes empty continuation lines from comment-only lines, and only outputs warnings for the former.
ping @dnephin @tianon