build: workaround for issues with en_US.UTF-8 locale. by PiotrSikora · Pull Request #15980 · envoyproxy/envoy

PiotrSikora · 2021-04-14T23:49:19Z

Signed-off-by: Piotr Sikora piotrsikora@google.com

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

repokitteh-read-only · 2021-04-14T23:49:23Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #15980 was opened by PiotrSikora.

see: more, trace.

PiotrSikora · 2021-04-15T01:41:31Z

/retest

repokitteh-read-only · 2021-04-15T01:41:35Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15980 (comment) was created by @PiotrSikora.

see: more, trace.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora · 2021-04-15T06:53:32Z

/retest

repokitteh-read-only · 2021-04-15T06:53:36Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15980 (comment) was created by @PiotrSikora.

see: more, trace.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora

NOTE: envoyproxy/envoy-build-tools#125 changed the default locale to en_US.UTF-8, which broke sed and docker load/save from stdin/stdout, so I've added LC_ALL=C in a few places to unbreak those tools.

phlax

hi @PiotrSikora comments inline

examples/cache/Dockerfile-service

phlax · 2021-04-15T12:03:41Z

ci/do_ci.sh

 function run_ci_verify () {
  echo "verify examples..."
-  docker load < "$ENVOY_DOCKER_BUILD_DIR/docker/envoy-docker-images.tar.xz"
+  LC_ALL=C docker load < "$ENVOY_DOCKER_BUILD_DIR/docker/envoy-docker-images.tar.xz"


im struggling to understand how this breaks - weve been using the updated build image for some time

also this is not run in the build image - so this overriding the locales in the CI (ie azure) image (afaict)

Yes, it changes locale in the CI, not build image, so that byte sequences passed to docker load/save are interpreted correctly.

Previously, it resulted in EOF during docker loadwhen reading from pipe, perhaps it was a flake, but there was another fallout because of the UTF-8 change and I was running against the clock. Unfortunatelly, I cannot find that run in Azure Pipelines now, though.

but docker load/save wasnt/isnt broken

phlax · 2021-04-15T12:04:04Z

test/exe/version_out_test.sh

 set -e -o pipefail

+# Undo LC_ALL=en_US.UTF-8, since it breaks sed.
+export LC_ALL=C


how does it break sed ?

i can see https://www.gnu.org/software/sed/manual/html_node/Locale-Considerations.html

but what is being seded here (afaict) is the version string which shouldnt contain any multibyte chars - which would imply the issue is more with the sed string - i might be wrong

==================== Test output for //test/exe:version_out_test: sed: -e expression #1, char 127: Invalid range end

See failed run: https://dev.azure.com/cncf/envoy/_build/results?buildId=71987&view=logs&j=5b1da4ae-91c5-53b4-2dce-4cdfc8f90e84&t=7ce850fe-192d-5106-1bd3-aa6c6dba22dc

not sure - but afaict both the linux image and the build-tools can be updated without this change

it may be necessary for some other reason that im not understanding - but my reading of the source is that its using sed to change the verssion string - and that should not have any kind of utf-8 issues

re this @PiotrSikora - i think - and i may be wrong - that the sed expression has an incorrect \ which is being interpreted as a utf-8 multibyte char but i really dont see how the change in the docker image recipe would affect this

ie its unrelated to the change in envoy-build-tools - it was always there - and its only when one of the tests fails that it has this issue

I mean, what can I say? See the CI failures from a few hours ago.

Also, the fact that updating Linux image alone works, and updating build-tools alone works, doesn't prove that both changes work fine together, does it? It's not like I was making all those changes for fun, they are direct results of failures on the CI.

moderation · 2021-04-15T13:46:37Z

/lgtm deps

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

…ls-20210414

PiotrSikora · 2021-04-15T18:12:39Z

/retest

repokitteh-read-only · 2021-04-15T18:12:43Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15980 (comment) was created by @PiotrSikora.

see: more, trace.

phlax

lgtm

the docker may/not be required but if we want to land/publish before waiting for confirmation either way this will resolve - as @PiotrSikora has said

PiotrSikora · 2021-04-15T20:06:58Z

Replaced by smaller #15997.

build: update envoy-build-tools to latest (2021-04-14).

5df5ec4

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Apr 14, 2021

review: undo LC_ALL=en_US.UTF-8, since it breaks sed.

b306d52

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora marked this pull request as ready for review April 15, 2021 02:42

PiotrSikora requested review from moderation and wrowe April 15, 2021 02:42

review: undo LC_ALL=en_US.UTF-8 in attempt to fix docker load/save.

29d0cb7

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora added 2 commits April 15, 2021 07:30

review: install gcc needed to build one of the Python deps.

6029376

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

review: install more deps.

314df90

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora added a commit to PiotrSikora/envoy that referenced this pull request Apr 15, 2021

review: sync fixes for the build image from envoyproxy#15980.

b7ffd06

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora requested review from htuch, mattklein123 and tonya11en April 15, 2021 10:37

PiotrSikora commented Apr 15, 2021

View reviewed changes

phlax requested changes Apr 15, 2021

View reviewed changes

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Apr 15, 2021

moderation previously approved these changes Apr 15, 2021

View reviewed changes

mattklein123 self-assigned this Apr 15, 2021

review: revert changes needed by cython.

952e18c

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

PiotrSikora dismissed moderation’s stale review via 952e18c April 15, 2021 17:31

Merge remote-tracking branch 'origin/main' into PiotrSikora/build-too…

5692bd9

…ls-20210414

PiotrSikora changed the title ~~build: update envoy-build-tools to latest (2021-04-14).~~ build: workaround for issues with en_US.UTF-8 locale. Apr 15, 2021

phlax approved these changes Apr 15, 2021

View reviewed changes

PiotrSikora closed this Apr 15, 2021

Conversation

PiotrSikora commented Apr 14, 2021

Uh oh!

repokitteh-read-only bot commented Apr 14, 2021

Uh oh!

PiotrSikora commented Apr 15, 2021

Uh oh!

repokitteh-read-only bot commented Apr 15, 2021

Uh oh!

PiotrSikora commented Apr 15, 2021

Uh oh!

repokitteh-read-only bot commented Apr 15, 2021

Uh oh!

PiotrSikora left a comment

Choose a reason for hiding this comment

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moderation commented Apr 15, 2021

Uh oh!

PiotrSikora commented Apr 15, 2021

Uh oh!

repokitteh-read-only bot commented Apr 15, 2021

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

PiotrSikora commented Apr 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants