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

[tomlplusplus] Add new port #10786

Merged
merged 8 commits into from
Jul 31, 2020
Merged

Conversation

traversaro
Copy link
Contributor

Describe the pull request
This PR adds a port for the tomlplusplus library.
Furthermore, as tomlplusplus installs a CMake configuration file generated by meson, it also fixes the vcpkg_fixup_cmake_targets function to correctly deal with CMake configuration files generated by meson, see mesonbuild/meson#6955 for more info.

  • What does your PR fix?
    Fix [New Port Request] tomlplusplus #10667 .

  • Which triplets are supported/not supported? Have you updated the CI baseline?
    I would expect all the triplets to work fine, so I think it is not necessary to update the CI baseline.

  • Does your PR follow the maintainer guide?
    Yes

@traversaro
Copy link
Contributor Author

fyi @marzer @GiulioRomualdi

@traversaro traversaro changed the title Add port for tomlplusplus [tomlplusplus] Add new port Apr 11, 2020
@marzer
Copy link
Contributor

marzer commented Apr 11, 2020

Oh, nice! @traversaro any chance you could update this PR to sync with the current HEAD of toml++? I've been doing some bugfixing and maintenance stuff over the last few days that I've just locked in as minor release v1.2.3.

@traversaro
Copy link
Contributor Author

Oh, nice! @traversaro any chance you could update this PR to sync with the current HEAD of toml++? I've been doing some bugfixing and maintenance stuff over the last few days that I've just locked in as minor release v1.2.3.

Done! For reference, unless there are some changes in the build system, updating to a new version is just a matter of update the version in the CONTROL and portfile.cmake files, and the hash in the portfile.cmake file.

@marzer
Copy link
Contributor

marzer commented Apr 12, 2020

@traversaro Nice, thanks for the info!

@Cheney-W
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

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

@Cheney-W
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

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

@Cheney-W
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Cheney-W
Copy link
Contributor

@traversaro Thanks for your PR!
tomlplusplus installation failed in following triplets:
arm-uwp
arm64-windows
x64-uwp
x64-linux
x64-osx
Is this result expected? If yes, could you please update the vcpkg/scripts/ci.baseline.txt; if not, could you please fix this issue?

@traversaro
Copy link
Contributor Author

That is indeed not expected, at least x64-linux was tested explicitly and it was working fine, I will check the CI logs.

@traversaro
Copy link
Contributor Author

@Cheney-W I tried the port for x64-linux locally on Ubuntu 18.04 and it works fine. How can I get the artifacts with the log error? They seem not to be available in https://dev.azure.com/vcpkg/c1ee48cb-0df2-4ab3-8384-b1df5a79fe53/_build/results?buildId=34726 .

@Cheney-W
Copy link
Contributor

@traversaro You could get the log with following steps:
findlog

@traversaro
Copy link
Contributor Author

Thanks @Cheney-W ! The x64-linux failure message is:

-- Downloading https://github.com/marzer/tomlplusplus/archive/v1.2.3.tar.gz...
-- Extracting source /ci/myagent/_work/3/s/downloads/marzer-tomlplusplus-v1.2.3.tar.gz
-- Using source at /ci/myagent/_work/3/s/buildtrees/tomlplusplus/src/v1.2.3-915b44652d
CMake Error at scripts/cmake/vcpkg_find_acquire_program.cmake:340 (message):
  Could not find meson.  Please install it via your package manager:

      sudo apt-get install meson
Call Stack (most recent call first):
  scripts/cmake/vcpkg_configure_meson.cmake:94 (vcpkg_find_acquire_program)
  ports/tomlplusplus/portfile.cmake:9 (vcpkg_configure_meson)
  scripts/ports.cmake:90 (include)

So I guess this is something that should be addressed on the CI machine, if you think it is not feasible I will mark x64-linux` as a a failing triplet.

@Cheney-W
Copy link
Contributor

@traversaro
I tried to install tomlplusplus in my side, and I got the following error after installed meson, I used Ubuntu 18.04.4 LTS (GNU/Linux 5.0.0-1036-azure x86_64)
vcpkg/buildtrees/tomlplusplus/config-x64-linux-dbg-out.log:

The Meson build system
Version: 0.45.1
Source dir: vcpkg/buildtrees/tomlplusplus/src/v1.2.3-915b44652d
Build dir: vcpkg/buildtrees/tomlplusplus/x64-linux-dbg
Build type: native build
meson_options.txt:1:0: ERROR: Unknown type feature.
A full log can be found at vcpkg/buildtrees/tomlplusplus/x64-linux-dbg/meson-logs/meson-log.txt

vcpkg/buildtrees/tomlplusplus/x64-linux-dbg/meson-logs/meson-log.txt:

Build started at 2020-04-30T06:01:55.176244
Main binary: /usr/bin/python3
Python system: Linux
The Meson build system
Version: 0.45.1
Source dir: vcpkg/buildtrees/tomlplusplus/src/v1.2.3-915b44652d
Build dir: vcpkg/buildtrees/tomlplusplus/x64-linux-dbg
Build type: native build
meson_options.txt:1:0: ERROR: Unknown type feature.

@marzer
Copy link
Contributor

marzer commented Apr 30, 2020

@Cheney-W meson needs to be version 0.47 or higher

(See https://mesonbuild.com/Build-options.html)

@traversaro
Copy link
Contributor Author

In that case, probably indeed it make sense to mark x64-linux as unsupported (at least for the CI) and mark it as supported when the CI gets updated to use Ubuntu 20.04 .

@Cheney-W
Copy link
Contributor

Cheney-W commented May 6, 2020

@traversaro
About x64-linux, I agree to add tomlplusplus:x64-linux=fail to vcpkg/scripts/ci.baseline.txt.
About other failed triplets, could you please fix them?

@traversaro
Copy link
Contributor Author

@traversaro
About x64-linux, I agree to add tomlplusplus:x64-linux=fail to vcpkg/scripts/ci.baseline.txt.
About other failed triplets, could you please fix them?

Sorry for the delay. I checked the other triplets and they seems to be failing due to missing support for cross-compilation in https://github.com/microsoft/vcpkg/blob/5deea3b975fe62990973d73a102de818d83ba20c/scripts/cmake/vcpkg_configure_meson.cmake . I would soon look into that and either fix it or add them to the list of expected failures.

@traversaro
Copy link
Contributor Author

@Cheney-W for the time being I just explicitly disabled the unsupported platforms, and the relative issues are:

As on Linux the port can be successfully installed if you have a recent meson (for example if you are on a recent distro or you installed meson via pip) I added it to the ci.baseline.txt, but i did not explicitly disabled build on Linux via vcpkg_fail_port_install .

@Cheney-W
Copy link
Contributor

Cheney-W commented May 8, 2020

@traversaro Thanks for your reply!
Could you please add linux into vcpkg_fail_port_install first? You could add some comments about the meson issue in linux before the vcpkg_fail_port_install line. And we will remove linux from vcpkg_fail_port_install once the issue #11230 and #6645 is fixed.
BTW, could you please add Supports into the control file?

@traversaro
Copy link
Contributor Author

Hi @LilyWangL , thanks for resolving the conflicts. I see that the CI is now passing, but I don't know if all that changes in ci.baseline.txt were intentional or not.

@Cheney-W
Copy link
Contributor

@traversaro
All that changes in ci.baseline.txt are not intentional , because ci.baseline.txt recent have a lot of changes, we just merge the master version to your branch.

@traversaro
Copy link
Contributor Author

@traversaro
All that changes in ci.baseline.txt are not intentional , because ci.baseline.txt recent have a lot of changes, we just merge the master version to your branch.

Thanks, I will cleanup the ci.baseline.txt then.

@Cheney-W
Copy link
Contributor

@traversaro You don't need to do anything, now you just have to wait for this PR to be merged.

scripts/ci.baseline.txt Outdated Show resolved Hide resolved
scripts/ci.baseline.txt Outdated Show resolved Hide resolved
@BillyONeal
Copy link
Member

@Cheney-W It seems your editor is trying to change the line endings on ci.baseline.txt causing every line to be edited...

@Cheney-W
Copy link
Contributor

Looks like the original line endings on ci.baseline.txt is LF, but after applied the suggestion commit, It becomes CRLF.

@ras0219-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@traversaro
Copy link
Contributor Author

I merged the branch with master.

@marzer
Copy link
Contributor

marzer commented Jul 20, 2020

@traversaro FYI: https://github.com/marzer/tomlplusplus/releases/tag/v2.0.0

(I'm unlikely to make another release for a decent while since I have other things I need to catch up on)

@traversaro
Copy link
Contributor Author

@traversaro FYI: https://github.com/marzer/tomlplusplus/releases/tag/v2.0.0

(I'm unlikely to make another release for a decent while since I have other things I need to catch up on)

Thanks. To be honest I was trying to get this through as it is, and then once it goes through just do a PR with the new release, that should have much less spurious CI problems, as what is making the CI for this PR so flaky is that it includes a modification to vcpkg_fixup_cmake_targets.cmake, that triggers a re-build of ~600/700 ports.

@marzer
Copy link
Contributor

marzer commented Jul 20, 2020

Yeah that's fair. I'm happy to do the v2.0.0 PR myself once this one goes through.

@traversaro
Copy link
Contributor Author

The CI is now failing with a few magnum-plugins failures, I do not know if it is related but hopefully those will be fixed by #12458 .

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed info:reviewed Pull Request changes follow basic guidelines labels Jul 21, 2020
@strega-nil
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil strega-nil merged commit 00e4436 into microsoft:master Jul 31, 2020
@traversaro
Copy link
Contributor Author

Thanks a lot @strega-nil !

@strega-nil
Copy link
Contributor

Thanks to you @traversaro :)

hellozee pushed a commit to hellozee/vcpkg that referenced this pull request Sep 11, 2020
* vcpkg_fixup_cmake_targets: Add support for processing cmake config files generated by meson

In particular this adds a workaround for the differences between CMake and meson
described in mesonbuild/meson#6955

* Add tomlplusplus

* Resolve conflicts

* Update scripts/ci.baseline.txt

* [tomplusplus] Update ci.basline.txt

Co-authored-by: Billy Robert O'Neal III <bion@microsoft.com>
Co-authored-by: wangli28 <wangli28@beyondsoft.com>
Co-authored-by: Cheney Wang <38240633+Cheney-W@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Port Request] tomlplusplus
9 participants