Fixed non-ascii error when using COMPOSE_DOCKER_CLI_BUILD=1 for Buildkit

[smamessier]

When using

DOCKER_BUILDKIT=1 docker-compose build

on compose's master, with docker version 18.09.8
I'm getting

UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 24: ordinal not in range(128)

which can be traced back to this line: https://github.com/docker/compose/blob/master/compose/service.py#L1809

The problem seems to be that the line read form stdin has non-ascii characters and .startswith fails when comparing a raw string with the unicode prefix magic_word.

The culprit input line (which is part of the output of docker build) is

#1     duration: 26.788µs

which contains the utf-8 µ character: \xc2\xb5. startswith errors out as the prefix magic_word is a unicode string.

This fixes the problem by encoding magic_word before making the comparison.

[GordonTheTurtle]

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "fix_non_ascii_error" [email protected]:smamessier/compose.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

[thaJeztah]

thanks!

ping @chris-crone PTAL

[smamessier]

This seems to only work on Python 2.7 let me fix this.

[thaJeztah]

Looks to be green now 👍. I'm not a maintainer in this repository; wondering if we can / should have a test for this (not sure if that's possible, but just brainstorming 😂)

@ndeloof @chris-crone ptal

[smamessier]

You're right. Ideally we should add a test that breaks without this patch. I'm not a maintainer either so that would take me quite a bit of time to get into that.

[ndeloof]

I'm +0 on this as I'm a perfect python newbie :)
IIUC readline does return strings as bytes[] while "magic" is Unicode, right?
Then str will convert unicode string into a byte-array string?
Nice catch

[smamessier]

I realized this patch may not be necessary. I was basically trying to use compose's master using

python setup.py develop

This probably assumes python is Python 3? Since string types and return type of stdin.readline has changed since python 2.7, it was failing.

When I install compose using python3 setup.py develop I don't have this problem anymore.
So unless we're supporting python 2.7 - which I doubt - we can dismiss this patch.

[ndeloof]

We do support python 2.7 but should consider to stop with it, especially with python 2 EOL on January 1st 2020.
see #6890

[smamessier]

So should we merge this?

[thaJeztah]

@ulyssessouza @chris-crone ptal

[ulyssessouza]

Thank you @smamessier !

Could you please add a test for it. I just fear that someone can take this out believing that's unnecessary.

[ulyssessouza]

Actually just adding an if before this line would be better to mark it as a Python2 issue, like:

if six.PY2:
    line = str(line)

So that's even easier to take this off when we execute #6890

I can do that now, and we are good to merge

[ulyssessouza]

LGTM

[smamessier]

Thanks a lot!

[ulyssessouza]

Thank you @smamessier for the catch!