Skip to content

[fmt] Update to 6.1.2#10080

Merged
strega-nil merged 5 commits intomicrosoft:masterfrom
curoky:upgrade_fmt
Mar 2, 2020
Merged

[fmt] Update to 6.1.2#10080
strega-nil merged 5 commits intomicrosoft:masterfrom
curoky:upgrade_fmt

Conversation

@curoky
Copy link
Contributor

@curoky curoky commented Feb 15, 2020

This PR changes:

  1. Update to latest revision 6.1.2
  2. Update patch fix-warning4189

Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@curoky, thanks for the PR!

Could you also bump the version in CONTROL file?

@PhoebeHui PhoebeHui self-assigned this Feb 15, 2020
@curoky
Copy link
Contributor Author

curoky commented Feb 15, 2020

Done.

@PhoebeHui
Copy link
Contributor

@curoky, the patch applied failed in CI testing, could you update the patch?

Checking patch include/fmt/format.h...
error: while searching for:

#ifndef FMT_FORMAT_H_
#define FMT_FORMAT_H_

#include
#include
#include

error: patch failed: include/fmt/format.h:32
error: include/fmt/format.h: patch does not apply

@curoky
Copy link
Contributor Author

curoky commented Feb 19, 2020

Done.
spdlog also needs to be upgraded or patched. Because the dependent header files are not in this version of fmt.

[1/8] /usr/bin/c++  -DFMT_LOCALE -DSPDLOG_COMPILED_LIB -DSPDLOG_FMT_EXTERNAL -I/home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/include -isystem /home/curoky/repos/fork/vcpkg/installed/x64-linux/include -fPIC -g   -Wall -Wextra -Wconversion -pedantic -Wfatal-errors -std=c++11 -MD -MT CMakeFiles/spdlog.dir/src/fmt.cpp.o -MF CMakeFiles/spdlog.dir/src/fmt.cpp.o.d -o CMakeFiles/spdlog.dir/src/fmt.cpp.o -c /home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/src/fmt.cpp
[2/8] /usr/bin/c++  -DFMT_LOCALE -DSPDLOG_COMPILED_LIB -DSPDLOG_FMT_EXTERNAL -I/home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/include -isystem /home/curoky/repos/fork/vcpkg/installed/x64-linux/include -fPIC -g   -Wall -Wextra -Wconversion -pedantic -Wfatal-errors -std=c++11 -MD -MT CMakeFiles/spdlog.dir/src/spdlog.cpp.o -MF CMakeFiles/spdlog.dir/src/spdlog.cpp.o.d -o CMakeFiles/spdlog.dir/src/spdlog.cpp.o -c /home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/src/spdlog.cpp
FAILED: CMakeFiles/spdlog.dir/src/spdlog.cpp.o 
/usr/bin/c++  -DFMT_LOCALE -DSPDLOG_COMPILED_LIB -DSPDLOG_FMT_EXTERNAL -I/home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/include -isystem /home/curoky/repos/fork/vcpkg/installed/x64-linux/include -fPIC -g   -Wall -Wextra -Wconversion -pedantic -Wfatal-errors -std=c++11 -MD -MT CMakeFiles/spdlog.dir/src/spdlog.cpp.o -MF CMakeFiles/spdlog.dir/src/spdlog.cpp.o.d -o CMakeFiles/spdlog.dir/src/spdlog.cpp.o -c /home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/src/spdlog.cpp
In file included from /home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/src/spdlog.cpp:13:
/home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/include/spdlog/details/pattern_formatter-inl.h: In member function ‘void spdlog::details::scoped_padder::pad_it(size_t)’:
/home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/include/spdlog/details/pattern_formatter-inl.h:76:9: error: ‘assert’ was not declared in this scope
   76 |         assert(count <= spaces_.size());
      |         ^~~~~~
compilation terminated due to -Wfatal-errors.
[3/8] /usr/bin/c++  -DFMT_LOCALE -DSPDLOG_COMPILED_LIB -DSPDLOG_FMT_EXTERNAL -I/home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/include -isystem /home/curoky/repos/fork/vcpkg/installed/x64-linux/include -fPIC -g   -Wall -Wextra -Wconversion -pedantic -Wfatal-errors -std=c++11 -MD -MT CMakeFiles/spdlog.dir/src/async.cpp.o -MF CMakeFiles/spdlog.dir/src/async.cpp.o.d -o CMakeFiles/spdlog.dir/src/async.cpp.o -c /home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/src/async.cpp
[4/8] /usr/bin/c++  -DFMT_LOCALE -DSPDLOG_COMPILED_LIB -DSPDLOG_FMT_EXTERNAL -I/home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/include -isystem /home/curoky/repos/fork/vcpkg/installed/x64-linux/include -fPIC -g   -Wall -Wextra -Wconversion -pedantic -Wfatal-errors -std=c++11 -MD -MT CMakeFiles/spdlog.dir/src/stdout_sinks.cpp.o -MF CMakeFiles/spdlog.dir/src/stdout_sinks.cpp.o.d -o CMakeFiles/spdlog.dir/src/stdout_sinks.cpp.o -c /home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/src/stdout_sinks.cpp
[5/8] /usr/bin/c++  -DFMT_LOCALE -DSPDLOG_COMPILED_LIB -DSPDLOG_FMT_EXTERNAL -I/home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/include -isystem /home/curoky/repos/fork/vcpkg/installed/x64-linux/include -fPIC -g   -Wall -Wextra -Wconversion -pedantic -Wfatal-errors -std=c++11 -MD -MT CMakeFiles/spdlog.dir/src/file_sinks.cpp.o -MF CMakeFiles/spdlog.dir/src/file_sinks.cpp.o.d -o CMakeFiles/spdlog.dir/src/file_sinks.cpp.o -c /home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/src/file_sinks.cpp
[6/8] /usr/bin/c++  -DFMT_LOCALE -DSPDLOG_COMPILED_LIB -DSPDLOG_FMT_EXTERNAL -I/home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/include -isystem /home/curoky/repos/fork/vcpkg/installed/x64-linux/include -fPIC -g   -Wall -Wextra -Wconversion -pedantic -Wfatal-errors -std=c++11 -MD -MT CMakeFiles/spdlog.dir/src/color_sinks.cpp.o -MF CMakeFiles/spdlog.dir/src/color_sinks.cpp.o.d -o CMakeFiles/spdlog.dir/src/color_sinks.cpp.o -c /home/curoky/repos/fork/vcpkg/buildtrees/spdlog/src/34724392a0-c671dd4aa9/src/color_sinks.cpp
ninja: build stopped: subcommand failed.

Should fix it in this PR?

@PhoebeHui
Copy link
Contributor

PhoebeHui commented Feb 19, 2020

@curoky, please also fix spdlog in this PR.

fmt also failed on CI testing, could you take a look? does it repro in your local machine?
-- Installing: C:/vsts/_work/3/s/packages/fmt_x86-windows/share/fmt/copyright
CMake Error at ports/fmt/portfile.cmake:24 (file):
file RENAME failed to rename
C:/vsts/_work/3/s/packages/fmt_x86-windows/debug/lib/fmtd.dll
to
C:/vsts/_work/3/s/packages/fmt_x86-windows/debug/bin/fmtd.dll

because: File exists

Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@curoky, thanks for updates!

Could you also bump version in spdlog CONTROL file?

@curoky
Copy link
Contributor Author

curoky commented Feb 20, 2020

Done.

@curoky
Copy link
Contributor Author

curoky commented Feb 20, 2020

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 10080 in repo microsoft/vcpkg

@PhoebeHui
Copy link
Contributor

@curoky, I have rerun CI testing on windows, let's wait for the results.

@PhoebeHui
Copy link
Contributor

@curoky, azure-kinect-sensor-sdk:x64-windows failed on CI testing, could you take a look?

Failures:

  • Clang-format not found
    CMake Error at CMakeLists.txt:303 (configure_file):
    configure_file Problem configuring file

-- Configuring incomplete, errors occurred!

@curoky
Copy link
Contributor Author

curoky commented Feb 21, 2020

The error message is not enough to conclude, try to run again? @PhoebeHui

@PhoebeHui
Copy link
Contributor

I have rerun the CI again, does the issue repro locally?

@PhoebeHui
Copy link
Contributor

The CI test pass.

@PhoebeHui PhoebeHui added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed waiting for response labels Feb 21, 2020
@kenr
Copy link
Contributor

kenr commented Feb 27, 2020

Is there anything stopping this to get merged? May I assist if so?

@PhoebeHui
Copy link
Contributor

@strega-nil, could you priority to review and merge this PR?

Copy link
Contributor

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! thanks for the PR, just some minor changes needed and then I'll merge :)

@@ -1,4 +1,4 @@
Source: fmt
Version: 6.0.0-1
Version: 6.1.2-1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be 6.1.2, instead of 6.1.2-1

@strega-nil
Copy link
Contributor

/azp run

@PhoebeHui
Copy link
Contributor

The azure-kinect-sensor-sdk failures is fake, after remove the tombstone, this issue solved.
Now the CI test pass.

@strega-nil strega-nil merged commit 705764c into microsoft:master Mar 2, 2020
@kevinlul kevinlul mentioned this pull request Mar 7, 2020
@curoky curoky deleted the upgrade_fmt branch March 18, 2020 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants