Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix regression that broke string.format with list objects #9532

Merged

Conversation

eli-schwartz
Copy link
Member

String formatting should validly assume that printing a list means printing the list itself. Instead, something like this broke:

'one is: @0@ and two is: @1@'.format(['foo',  'bar'], ['baz'])

which would evaluate as:

'one is: foo and two is: bar'

or:

'the value of array option foobar is: @0@'.format(get_option('foobar'))

which should evaluate with '-Dfoobar=[]' as

'the value of array option foobar is: []'

But instead produced:

meson.build:7:0: ERROR: Format placeholder @0@ out of range.

Fixes #9530

String formatting should validly assume that printing a list means
printing the list itself. Instead, something like this broke:

'one is: @0@ and two is: @1@'.format(['foo',  'bar'], ['baz'])

which would evaluate as:

'one is: foo and two is: bar'

or:

'the value of array option foobar is: @0@'.format(get_option('foobar'))

which should evaluate with '-Dfoobar=[]' as

'the value of array option foobar is: []'

But instead produced:

meson.build:7:0: ERROR: Format placeholder @0@ out of range.

Fixes mesonbuild#9530
@eli-schwartz eli-schwartz added this to the 0.60.2 milestone Nov 4, 2021
@codecov
Copy link

codecov bot commented Nov 4, 2021

Codecov Report

Merging #9532 (558f9fe) into master (1104b82) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9532      +/-   ##
==========================================
- Coverage   67.41%   67.39%   -0.03%     
==========================================
  Files         396      396              
  Lines       85562    85564       +2     
  Branches    17665    17665              
==========================================
- Hits        57684    57665      -19     
- Misses      23214    23243      +29     
+ Partials     4664     4656       -8     
Impacted Files Coverage Δ
mesonbuild/interpreter/primitives/string.py 98.07% <100.00%> (+0.01%) ⬆️
mesonbuild/linkers/detect.py 41.59% <0.00%> (-7.08%) ⬇️
mesonbuild/scripts/vcstagger.py 87.50% <0.00%> (-4.17%) ⬇️
mesonbuild/modules/windows.py 69.00% <0.00%> (-3.00%) ⬇️
mesonbuild/compilers/cpp.py 43.96% <0.00%> (-2.06%) ⬇️
mesonbuild/compilers/c.py 47.57% <0.00%> (-1.95%) ⬇️
mesonbuild/compilers/detect.py 54.89% <0.00%> (-1.38%) ⬇️
mesonbuild/scripts/symbolextractor.py 51.70% <0.00%> (-0.86%) ⬇️
mesonbuild/interpreter/interpreter.py 82.23% <0.00%> (-0.11%) ⬇️
interpreter/primitives/string.py 98.07% <0.00%> (+0.01%) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1104b82...558f9fe. Read the comment docs.

@eli-schwartz
Copy link
Member Author

eli-schwartz commented Nov 5, 2021

@lazka

 [UNEXRUN]   frameworks: 15 llvm    (method=cmake link-static=True)

This is configured as:

        { "val": "cmake", "skip_on_jobname": ["msys2"] },

but apparently it now works? Do you know why, or if it is expected to stay that way? (I genuinely have no idea when tests are/were expected to be skipped, or why, to begin with.)

@eli-schwartz eli-schwartz merged commit 558f9fe into mesonbuild:master Nov 5, 2021
@eli-schwartz eli-schwartz deleted the string-format-noflatten branch November 5, 2021 04:29
@lazka
Copy link
Contributor

lazka commented Nov 5, 2021

I guess you mean { "val": "cmake", "skip_on_jobname": ["msys2"] }?

Why was it skipped in the first place?

@lazka
Copy link
Contributor

lazka commented Nov 5, 2021

ah, I'd guess this is msys2/MINGW-packages@345fe43 which got uploaded 4 days ago, so likely fixed and can be unskipped.

lazka added a commit to lazka/meson that referenced this pull request Nov 5, 2021
See mesonbuild#9532 (comment)
for context.

It's unclear why this was skipped exactly in the first place, but it started to
pass recently and MSYS2 fixed some cmake+llvm related things in
msys2/MINGW-packages@345fe43
at the same time, so this is likely the reason.
@lazka
Copy link
Contributor

lazka commented Nov 5, 2021

I've created #9534

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[regression] String formatting has regressed since 0.60. ERROR: Format placeholder @9@ out of range.
3 participants