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

[cppcoro] add new port #10693

Merged
merged 8 commits into from
Apr 24, 2020
Merged

[cppcoro] add new port #10693

merged 8 commits into from
Apr 24, 2020

Conversation

luncliff
Copy link
Contributor

@luncliff luncliff commented Apr 4, 2020

Previously there was #7779 to add https://github.com/lewissbaker/cppcoro, but it is closed for some reason.

Since the fork defines minimum CMake configurations, I expect the port will be available in the most Desktop(or so-called non-mobile) triplets.

Related Issues

@NancyLi1013
Copy link
Contributor

/azp run

ports/cppcoro/CONTROL Outdated Show resolved Hide resolved
ports/cppcoro/portfile.cmake Outdated Show resolved Hide resolved
ports/cppcoro/portfile.cmake Show resolved Hide resolved
ports/cppcoro/portfile.cmake Outdated Show resolved Hide resolved
ports/cppcoro/portfile.cmake Outdated Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

Hi @luncliff
Thanks for this PR.
Could you please look into the regression on Linux and try to fix it?

@luncliff
Copy link
Contributor Author

luncliff commented Apr 7, 2020

Thanks for very fast review, @NancyLi1013.
I will apply the requested changes and push again within days.

For #10693 (comment),
If you think registration of the forked repo is inappropriate, please feel free to close this :)

@luncliff
Copy link
Contributor Author

luncliff commented Apr 7, 2020

For Linux build failure, the errors are something like this.

[1/20] /usr/bin/c++   -I/mnt/develop/vcpkg/buildtrees/cppcoro/src/0cdf31b6a5-b830c1617b/include -fPIC -g   -std=gnu++17 -MD -MT CMakeFiles/cppcoro.dir/lib/async_mutex.cpp.o -MF CMakeFiles/cppcoro.dir/lib/async_mutex.cpp.o.d -o CMakeFiles/cppcoro.dir/lib/async_mutex.cpp.o -c /mnt/develop/vcpkg/buildtrees/cppcoro/src/0cdf31b6a5-b830c1617b/lib/async_mutex.cpp
FAILED: CMakeFiles/cppcoro.dir/lib/async_mutex.cpp.o 
/usr/bin/c++   -I/mnt/develop/vcpkg/buildtrees/cppcoro/src/0cdf31b6a5-b830c1617b/include -fPIC -g   -std=gnu++17 -MD -MT CMakeFiles/cppcoro.dir/lib/async_mutex.cpp.o -MF CMakeFiles/cppcoro.dir/lib/async_mutex.cpp.o.d -o CMakeFiles/cppcoro.dir/lib/async_mutex.cpp.o -c /mnt/develop/vcpkg/buildtrees/cppcoro/src/0cdf31b6a5-b830c1617b/lib/async_mutex.cpp
In file included from /mnt/develop/vcpkg/buildtrees/cppcoro/src/0cdf31b6a5-b830c1617b/lib/async_mutex.cpp:6:
/mnt/develop/vcpkg/buildtrees/cppcoro/src/0cdf31b6a5-b830c1617b/include/cppcoro/async_mutex.hpp:8:10: fatal error: experimental/coroutine: No such file or directory
    8 | #include <experimental/coroutine>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
...

As far as I know, there are 2 problems.

  1. GCC 7 doesn't support C++ Coroutines (TS) feature. We need at least GCC 10
  2. The build requires libc++, instead of libstdc++

For case 1, the user can workaround with CC=clang, CXX=clang.
For case 2, (optional) triplet (x64-linux) customization is required to enforce specific version of libc++.

luncliff@CIC:/mnt/develop/vcpkg$ export CC=clang++-8 CXX=clang++-8
luncliff@CIC:/mnt/develop/vcpkg$ ./vcpkg install cppcoro
Computing installation plan...
The following packages will be built and installed:
    cppcoro[core]:x64-linux
Starting package 1/1: cppcoro:x64-linux
Building package cppcoro[core]:x64-linux...
-- Using cached /mnt/develop/vcpkg/downloads/luncliff-cppcoro-20b4217a65165d0ec7849fbe7970310cdf31b6a5.tar.gz                                                                                                              tar.gz
-- Using source at /mnt/develop/vcpkg/buildtrees/cppcoro/src/0cdf31b6a5-b830c1617b
-- Configuring x64-linux-dbg
-- Configuring x64-linux-rel
-- Building x64-linux-dbg
-- Building x64-linux-rel
-- Installing: /mnt/develop/vcpkg/packages/cppcoro_x64-linux/share/cppcoro/copyright
-- Performing post-build validation
-- Performing post-build validation done
Building package cppcoro[core]:x64-linux... done
Installing package cppcoro[core]:x64-linux...
Installing package cppcoro[core]:x64-linux... done
Elapsed time for package cppcoro:x64-linux: 28.41 s

Total elapsed time: 28.42 s

The package cppcoro:x64-linux provides CMake targets:

    find_package(cppcoro CONFIG REQUIRED)
    target_link_libraries(main PRIVATE cppcoro)

* update reference to tag and hash value
* ${PORT} for destination

Linux build support should marked unavailable?
ports/cppcoro/CONTROL Outdated Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

@luncliff
Thanks for your deep investigation about the regression on Linux.
As far as I'm concerned, most users installed gcc version less than 10.
For libc++, we prefer to use the triplet that vcpkg supports instead of custom triplet.
So currently I suggest that we had better disable the support for linux.

ports/cppcoro/portfile.cmake Outdated Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

/azp run

* copy the CMakeLists.txt file and start build with it
* change version to 2020.2

For now the repo doesn't have any tags.
Therefore its version followed the latest commit,
which is made in 2020 Feb.
@luncliff
Copy link
Contributor Author

luncliff commented Apr 10, 2020

Thanks for your feedback, @NancyLi1013 . I just embedded CMakeLists.txt, and marked Supports: !(uwp|linux).

@luncliff
Copy link
Contributor Author

luncliff commented Apr 10, 2020

Since the original repo doesn't have tag, I used version code 2020.02.
(the latest commit was made in 2020 Feb)

@NancyLi1013
Copy link
Contributor

If there is no released version, you can use a date that you accept the latest commit.
Please refer to this doc.

@luncliff
Copy link
Contributor Author

@NancyLi1013 I will check the other parts again :)

@NancyLi1013
Copy link
Contributor

@luncliff Thanks for your update.
Is there anything else you want to check or update now?

@luncliff
Copy link
Contributor Author

luncliff commented Apr 13, 2020

@luncliff Thanks for your update.
Is there anything else you want to check or update now?

Looks good to me with current state. Do I have to rebase the commits?

* vcpkg_fail_port_install on uwp/linux

mention PR #10693 so linux/clang users can see the port issues
ports/cppcoro/CMakeLists.txt Outdated Show resolved Hide resolved
ports/cppcoro/CMakeLists.txt Outdated Show resolved Hide resolved
ports/cppcoro/CMakeLists.txt Outdated Show resolved Hide resolved
* report error for clang-cl

build with VC++ in VS 2019 will fail by the header file.
ports/cppcoro/portfile.cmake Outdated Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@luncliff luncliff requested a review from JackBoosY April 22, 2020 10:49
@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed waiting for response labels Apr 23, 2020
@ras0219-msft ras0219-msft merged commit 84f94fb into microsoft:master Apr 24, 2020
@ras0219-msft
Copy link
Contributor

Thanks for the PR!

@luncliff
Copy link
Contributor Author

Appreciate for the reviews! Would you close the #5106?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants