Skip to content

[boost-regex] fix build with icu on osx#16994

Merged
vicroms merged 4 commits intomicrosoft:masterfrom
autoantwort:boost-regex-icu-fix
Apr 8, 2021
Merged

[boost-regex] fix build with icu on osx#16994
vicroms merged 4 commits intomicrosoft:masterfrom
autoantwort:boost-regex-icu-fix

Conversation

@autoantwort
Copy link
Copy Markdown
Contributor

@autoantwort autoantwort commented Mar 31, 2021 •
edited by PhoebeHui
Loading

Describe the pull request

  • What does your PR fix? Fixes pipeline (discussed on discord)

  • Which triplets are supported/not supported? Have you updated the CI baseline? All supported

  • Does your PR follow the maintainer guide? Yes

Edit:
Failures in PR #14628 pipeline:

In file included from ../build/../src/icu.cpp:23:
In file included from ../include/boost/regex/icu.hpp:25:
In file included from /Users/vagrant/Data/installed/x64-osx/include/unicode/coll.h:62:
In file included from /Users/vagrant/Data/installed/x64-osx/include/unicode/ucol.h:17:
In file included from /Users/vagrant/Data/installed/x64-osx/include/unicode/unorm.h:25:
In file included from /Users/vagrant/Data/installed/x64-osx/include/unicode/unorm2.h:34:
/Users/vagrant/Data/installed/x64-osx/include/unicode/localpointer.h:71:51: warning: deleted function definitions are a C++11 extension [-Wc++11-extensions]
    static void* U_EXPORT2 operator new(size_t) = delete;
                                                  ^
/Users/vagrant/Data/installed/x64-osx/include/unicode/localpointer.h:72:53: warning: deleted function definitions are a C++11 extension [-Wc++11-extensions]
    static void* U_EXPORT2 operator new[](size_t) = delete;
                                                    ^
/Users/vagrant/Data/installed/x64-osx/include/unicode/localpointer.h:74:58: warning: deleted function definitions are a C++11 extension [-Wc++11-extensions]
    static void* U_EXPORT2 operator new(size_t, void*) = delete;
                                                         ^
/Users/vagrant/Data/installed/x64-osx/include/unicode/localpointer.h:224:34: warning: rvalue references are a C++11 extension [-Wc++11-extensions]
    LocalPointer(LocalPointer<T> &&src) U_NOEXCEPT : LocalPointerBase<T>(src.ptr) {
                                 ^
/Users/vagrant/Data/installed/x64-osx/include/unicode/localpointer.h:224:40: error: expected ';' at end of declaration list
    LocalPointer(LocalPointer<T> &&src) U_NOEXCEPT : LocalPointerBase<T>(src.ptr) {
                                       ^
                                       ;
/Users/vagrant/Data/installed/x64-osx/include/unicode/localpointer.h:399:30: warning: rvalue references are a C++11 extension [-Wc++11-extensions]
    LocalArray(LocalArray<T> &&src) U_NOEXCEPT : LocalPointerBase<T>(src.ptr) {
                             ^
/Users/vagrant/Data/installed/x64-osx/include/unicode/localpointer.h:399:36: error: expected ';' at end of declaration list
    LocalArray(LocalArray<T> &&src) U_NOEXCEPT : LocalPointerBase<T>(src.ptr) {
                                   ^
                                   ;

@PhoebeHui PhoebeHui self-assigned this Apr 1, 2021
@PhoebeHui
Copy link
Copy Markdown
Contributor

PhoebeHui commented Apr 1, 2021

@autoantwort, thanks for the PR!

Could you please link the PR which failed with boost-regex:x64-osx in CI testing?

@PhoebeHui PhoebeHui added the category:port-bug The issue is with a library, which is something the port should already support label Apr 1, 2021
@mathisloge
Copy link
Copy Markdown
Contributor

mathisloge commented Apr 1, 2021

@PhoebeHui #14628 that's the pr

This was referenced Apr 1, 2021
Closed
Closed
@JackBoosY JackBoosY mentioned this pull request Apr 1, 2021
Merged
@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Apr 1, 2021

Wait for #17014 merge first.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Apr 1, 2021
@PhoebeHui PhoebeHui requested a review from vicroms April 2, 2021 02:23
@PhoebeHui PhoebeHui added requires:discussion and removed depends:different-pr This PR or Issue depends on a PR which has been filed labels Apr 2, 2021
@autoantwort
Copy link
Copy Markdown
Contributor Author

autoantwort commented Apr 2, 2021

@PhoebeHui Why the label requires:discussion?

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Apr 5, 2021

We need colleagues to do more in-depth review.

@mathisloge
Copy link
Copy Markdown
Contributor

mathisloge commented Apr 5, 2021

pinging @ras0219-msft since he was a part of the discussion in discord.

PhoebeHui
PhoebeHui suggested changes Apr 7, 2021
View reviewed changes
Copy link
Copy Markdown
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.

The option seems only required on osx platform, so it may be better to restrict the platform.

autoantwort and others added 2 commits April 7, 2021 12:49
@autoantwort @PhoebeHui
Update ports/boost-regex/b2-options.cmake: Apply suggestion
533732f 
Co-authored-by: Phoebe <20694052+PhoebeHui@users.noreply.github.com>
@autoantwort
add version files
898bff9
@autoantwort autoantwort force-pushed the boost-regex-icu-fix branch from abfe0e4 to 898bff9 Compare April 7, 2021 10:50
@autoantwort autoantwort requested a review from PhoebeHui April 7, 2021 20:48
PhoebeHui
PhoebeHui approved these changes Apr 8, 2021
View reviewed changes
Copy link
Copy Markdown
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.

LGTM, thanks for your updates!

@PhoebeHui PhoebeHui added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels Apr 8, 2021
@vicroms vicroms merged commit 80d7989 into microsoft:master Apr 8, 2021
@autoantwort autoantwort deleted the boost-regex-icu-fix branch September 1, 2021 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vicroms vicroms Awaiting requested review from vicroms

1 more reviewer

@PhoebeHui PhoebeHui PhoebeHui approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@PhoebeHui PhoebeHui

Labels

category:port-bug The issue is with a library, which is something the port should already support info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@autoantwort @PhoebeHui @mathisloge @JackBoosY @vicroms