Bump fmtlib and spdlog by krnowak · Pull Request #20066 · envoyproxy/envoy

krnowak · 2022-02-21T16:38:07Z

Commit Message:
This updates fmtlib to 8.1.1 and spdlog to 1.9.2.

Additional Description:
Looks like fmtlib isn't formatting enumeration types as ints any more, so I'm explicitly casting them to ints. Without that, the build fails.

Risk Level:
Medium, logging library update.

Testing:
In CI only - I couldn't build the whole project as it takes too much space…

Docs Changes:
No.

Release Notes:
Not sure if dependency update is release-notes-worthy.

Platform Specific Features:
None.

Fix #18322 Fix #19439

Signed-off-by: Krzesimir Nowak <[email protected]>

repokitteh-read-only · 2022-02-21T16:38:13Z

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).
envoyproxy/dependency-shepherds assignee is @mattklein123

🐱

Caused by: #20066 was opened by krnowak.

see: more, trace.

Signed-off-by: Krzesimir Nowak <[email protected]>

The connection parameter was actually needed here. Signed-off-by: Krzesimir Nowak <[email protected]>

Signed-off-by: Krzesimir Nowak <[email protected]>

It looks like that after the update, non-fractional doubles are printed like integers (so they won't have trailing ".0" printed), which broke a test. Not sure if this trailing dot-zero is important, but try to keep it. Signed-off-by: Krzesimir Nowak <[email protected]>

Signed-off-by: Krzesimir Nowak <[email protected]>

The updated fmtlib doesn't print them anymore. Signed-off-by: Krzesimir Nowak <[email protected]>

This reverts commit 28ffc37. Signed-off-by: Krzesimir Nowak <[email protected]>

Signed-off-by: Krzesimir Nowak <[email protected]>

krnowak · 2022-02-24T11:57:35Z

/retest

repokitteh-read-only · 2022-02-24T11:57:39Z

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

🐱

Caused by: a #20066 (comment) was created by @krnowak.

see: more, trace.

krnowak · 2022-02-24T15:06:48Z

/retest

repokitteh-read-only · 2022-02-24T15:06:51Z

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

🐱

Caused by: a #20066 (comment) was created by @krnowak.

see: more, trace.

krnowak · 2022-02-25T07:09:13Z

/retest

repokitteh-read-only · 2022-02-25T07:09:17Z

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

🐱

Caused by: a #20066 (comment) was created by @krnowak.

see: more, trace.

krnowak · 2022-02-25T16:40:21Z

Looks like it's ready for review. Question though: static_cast<int> or enumToInt? The latter will turn an enumeration value to unsigned integer, so for cases where enumeration has negative values, the logs will show numbers around UINT32_MAX. That's why I went with static_cast<int>.

moderation · 2022-03-01T00:37:40Z

Thanks for updating these! LGTM for deps.
@envoyproxy/senior-maintainers for @krnowak code question

/lgtm deps

krnowak · 2022-03-01T07:14:33Z

Just remembered - another thing I updated was a test for text formatting of stats - the updated fmtlib does not print the .0 any more. Not sure if there's a way of telling fmtlib to use precision 1 or more, so it always would be showing at least one digit after the decimal point.

moderation · 2022-03-13T15:20:03Z

Just remembered - another thing I updated was a test for text formatting of stats - the updated fmtlib does not print the .0 any more. Not sure if there's a way of telling fmtlib to use precision 1 or more, so it always would be showing at least one digit after the decimal point.

Does this change make it a breaking change and require a changelog entry? /cc @htuch, @phlax . Would be good to land this from a dependency point of view

htuch · 2022-03-14T02:26:13Z

My $0.02 is that this isn't a breaking change, since any parsing code should be robust to this difference when interpreting a float. I can imagine contrived corner cases when not, but the likelihood of breakage seems low.

phlax · 2022-03-14T13:54:13Z

+1 re parsing this value as float/int

krnowak · 2022-03-14T15:38:02Z

Merged the main branch to see if it still builds. But I see that the bot reverted some decisions (removed waiting and readded deps), sorry about that.

phlax

/lgtm deps pending CI

repokitteh-read-only · 2022-03-14T16:23:04Z

please specify a single label can be specified

🐱

Caused by: a #20066 (review) was submitted by @phlax.

see: more, trace.

phlax · 2022-03-14T16:23:21Z

/lgtm deps

phlax · 2022-03-14T16:23:53Z

@krnowak you can add the waiting label at any time with /wait

Signed-off-by: Krzesimir Nowak <[email protected]>

mattklein123

Thanks!

mattklein123 · 2022-03-14T20:43:31Z

/retest

repokitteh-read-only · 2022-03-14T20:43:34Z

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

🐱

Caused by: a #20066 (comment) was created by @mattklein123.

see: more, trace.

Signed-off-by: Krzesimir Nowak <[email protected]>

krnowak added 2 commits February 21, 2022 17:24

bazel: bump fmtlib and spdlog

dfe4da8

Signed-off-by: Krzesimir Nowak <[email protected]>

fix logging

8e3b23e

Signed-off-by: Krzesimir Nowak <[email protected]>

krnowak requested review from junr03, lizan, mattklein123, snowp and yanavlasov as code owners February 21, 2022 16:38

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Feb 21, 2022

repokitteh-read-only bot assigned mattklein123 Feb 21, 2022

phlax mentioned this pull request Feb 21, 2022

[WIP] deps: Bump com_github_gabime_spdlog -> 1.9.2, com_github_fmtlib_fmt -> 8.0.1 #18483

Closed

krnowak mentioned this pull request Feb 21, 2022

CM: Add on-demand cluster discovery functionality #18723

Merged

krnowak added 2 commits February 21, 2022 18:26

fix logging in mysql proxy

1146f4e

Signed-off-by: Krzesimir Nowak <[email protected]>

fix logging in sip proxy

45296de

Signed-off-by: Krzesimir Nowak <[email protected]>

mattklein123 added the waiting label Feb 22, 2022

fix logging again in sip proxy

a09708f

The connection parameter was actually needed here. Signed-off-by: Krzesimir Nowak <[email protected]>

repokitteh-read-only bot removed the waiting label Feb 22, 2022

krnowak added 6 commits February 22, 2022 09:33

fix another logging in sip proxy

4b24bf7

Signed-off-by: Krzesimir Nowak <[email protected]>

fix logging in apple dns resolver

73aa7f3

Signed-off-by: Krzesimir Nowak <[email protected]>

fix another logging in apple dns resolver

308082d

Signed-off-by: Krzesimir Nowak <[email protected]>

Drop trailing ".0" from stats test text output

2ded7ed

The updated fmtlib doesn't print them anymore. Signed-off-by: Krzesimir Nowak <[email protected]>

Revert "Try to preserve the old formatting of histograms"

fd3429a

This reverts commit 28ffc37. Signed-off-by: Krzesimir Nowak <[email protected]>

krnowak force-pushed the krnowak/fmtlib-bump branch from 80102b1 to fd3429a Compare February 22, 2022 10:49

krnowak added 2 commits February 22, 2022 12:35

fix formatting

211ac1d

Signed-off-by: Krzesimir Nowak <[email protected]>

fix logging in dns filter

966fd2b

Signed-off-by: Krzesimir Nowak <[email protected]>

mattklein123 added the waiting label Feb 22, 2022

phlax mentioned this pull request Feb 24, 2022

Update com_github_fmtlib_fmt: 8.1.0 (current: 7.0.3) #19386

Closed

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Mar 1, 2022

Merge remote-tracking branch 'origin/main' into krnowak/fmtlib-bump

188d9f4

repokitteh-read-only bot added deps Approval required for changes to Envoy's external dependencies and removed waiting labels Mar 14, 2022

phlax reviewed Mar 14, 2022

View reviewed changes

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Mar 14, 2022

fix logging in sip proxy

b6113c2

Signed-off-by: Krzesimir Nowak <[email protected]>

mattklein123 approved these changes Mar 14, 2022

View reviewed changes

mattklein123 enabled auto-merge (squash) March 14, 2022 18:04

mattklein123 merged commit 5c4d4bd into envoyproxy:main Mar 14, 2022

krnowak deleted the krnowak/fmtlib-bump branch March 14, 2022 21:56

ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022

Bump fmtlib and spdlog (envoyproxy#20066)

554078e

Signed-off-by: Krzesimir Nowak <[email protected]>

Conversation

krnowak commented Feb 21, 2022 • edited by phlax Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Feb 21, 2022

Uh oh!

krnowak commented Feb 24, 2022

Uh oh!

repokitteh-read-only bot commented Feb 24, 2022

Uh oh!

krnowak commented Feb 24, 2022

Uh oh!

repokitteh-read-only bot commented Feb 24, 2022

Uh oh!

krnowak commented Feb 25, 2022

Uh oh!

repokitteh-read-only bot commented Feb 25, 2022

Uh oh!

krnowak commented Feb 25, 2022

Uh oh!

moderation commented Mar 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krnowak commented Mar 1, 2022

Uh oh!

moderation commented Mar 13, 2022

Uh oh!

htuch commented Mar 14, 2022

Uh oh!

phlax commented Mar 14, 2022

Uh oh!

krnowak commented Mar 14, 2022

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

repokitteh-read-only bot commented Mar 14, 2022

Uh oh!

phlax commented Mar 14, 2022

Uh oh!

phlax commented Mar 14, 2022

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

mattklein123 commented Mar 14, 2022

Uh oh!

repokitteh-read-only bot commented Mar 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

krnowak commented Feb 21, 2022 •

edited by phlax

Loading

moderation commented Mar 1, 2022 •

edited

Loading