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

[cppgraphqlgen] Fix --head for new branch/option #15549

Merged
merged 6 commits into from
Jan 13, 2021

Conversation

wravery
Copy link
Contributor

@wravery wravery commented Jan 9, 2021

Describe the pull request

  • What does your PR fix?

I'm not ready to update cppgraphqlgen to the v3.4.1 release yet because it depends on a change in pegtl which hasn't been released as v3.1.1 yet. However, installing both of those with --head should work, and right now it doesn't work for cppgraphqlgen. I renamed the default branch from master to main, and there's a new GRAPHQL_UPDATE_VERSION=OFF CMake option which the portfile needs to specify. Using that option with a prior commit will just be ignored.

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

No, as far as I know the CI baselines should all work.

Yes.

@wravery wravery marked this pull request as ready for review January 9, 2021 23:50
Copy link
Contributor

@NancyLi1013 NancyLi1013 left a comment

Choose a reason for hiding this comment

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

Could you please also bump version in CONTROL file?

@NancyLi1013
Copy link
Contributor

NancyLi1013 commented Jan 11, 2021

Hi @wravery

Thanks for your PR.

Have you tested if ./vcpkg install cppgraphqlgen--head works fine on your local?

I tried to install on my machine, it posted the error like this:

CMake Error at CMakeLists.txt:76 (add_subdirectory):
  The source directory

    F:/tool/vcpkg/buildtrees/cppgraphqlgen/src/head/main-211b762c03.clean/PEGTL

  does not contain a CMakeLists.txt file.

@NancyLi1013 NancyLi1013 added the category:port-bug The issue is with a library, which is something the port should already support label Jan 11, 2021
@wravery
Copy link
Contributor Author

wravery commented Jan 11, 2021

It does require that you install pegtl with --head as well. Otherwise it tries to use the submodule copy of it, and that doesn't work with vcpkg.

@NancyLi1013
Copy link
Contributor

Thanks for your response @wravery.

Have installed successfully now after install pegtl with --head.

@wravery
Copy link
Contributor Author

wravery commented Jan 11, 2021

Could you please also bump version in CONTROL file?

Since I'm not actually changing the commit, I just added -1 to the date-based version. Let me know if there's a better version scheme to handle that situation.

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jan 12, 2021
@NancyLi1013
Copy link
Contributor

LGTM now, thanks.

@dan-shaw dan-shaw merged commit fc2c8d2 into microsoft:master Jan 13, 2021
@wravery wravery deleted the cppgraphqlgen branch January 16, 2021 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants