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.

Sign up for GitHub

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

Jump to bottom

Fix vcpkg_fixup_cmake_targets to work correctly on MinGW #12104

Merged
merged 3 commits into from
Jul 31, 2020
Merged

Fix vcpkg_fixup_cmake_targets to work correctly on MinGW #12104

merged 3 commits into from
Jul 31, 2020

Conversation

koprok
Copy link
Contributor

@koprok koprok commented Jun 25, 2020

Handle executable suffix correctly on all targets.

@koprok koprok force-pushed the mingw-vcpkg-fixup-cmake-targets branch from 5b089a2 to df69d48 Compare June 26, 2020 10:44
@JackBoosY JackBoosY self-assigned this Jun 28, 2020
@JackBoosY JackBoosY added category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed requires:author-response labels Jun 28, 2020
@koprok koprok requested a review from JackBoosY June 29, 2020 08:06
JackBoosY
JackBoosY suggested changes Jun 30, 2020
View reviewed changes
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Please update the akali's version info to test the changes. See documentation.

@koprok
Copy link
Contributor Author

koprok commented Jun 30, 2020

Please update the akali's version info to test the changes. See documentation.

@JackBoosY, sorry I am not very sure what you mean. Could you please clarify? Do you want me to raise akali's port version (from 1.41 to say 1.41-1) and commit this change to my branch? Why is this necessary and why exactly akali's port? Sorry for asking, I am just curious to know... for the next time I submit some similar changes? :)

@JackBoosY
Copy link
Contributor

@koprok We need to bump the version of the port that uses vcpkg_fixup_cmake_targets to test whether the PR modification is correct.

@linquize
Copy link

linquize commented Jul 1, 2020

Should we follow the latest convention using Version: and Port-Version: pair?

@koprok
Copy link
Contributor Author

koprok commented Jul 1, 2020

Should we follow the latest convention using Version: and Port-Version: pair?

@linquize, I cannot find anything about such convention in Vcpkg documentation and I also don't see any port using it.

@linquize
Copy link

linquize commented Jul 1, 2020

libarchive is using the new Version: and Port-Version: pair

@koprok
Copy link
Contributor Author

koprok commented Jul 1, 2020

libarchive is using the new Version: and Port-Version: pair

Ahh sorry, you are right @linquize ! I was looking at my mingw-vcpkg-fixup-cmake-targets branch instead of upstream master. Should I use Port-Version instead of Version, @JackBoosY, what do you think ?

@koprok koprok requested a review from JackBoosY July 2, 2020 10:34
@JackBoosY
Copy link
Contributor

Opencc regression is note related with this PR.
We need to wait for fix it first.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 3, 2020
@koprok
Copy link
Contributor Author

koprok commented Jul 3, 2020

Opencc regression is note related with this PR.
We need to wait for fix it first.

Fixed in #12246

@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 8, 2020
JackBoosY
JackBoosY suggested changes Jul 8, 2020
View reviewed changes
ports/akali/CONTROL Outdated Show resolved Hide resolved
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jul 8, 2020
@JackBoosY JackBoosY removed the info:reviewed Pull Request changes follow basic guidelines label Jul 16, 2020
@JackBoosY
Copy link
Contributor

Waiting for merge #12458.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 20, 2020
@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed depends:different-pr This PR or Issue depends on a PR which has been filed 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 e2cdd07 into microsoft:master Jul 31, 2020
@koprok koprok deleted the mingw-vcpkg-fixup-cmake-targets branch August 3, 2020 07:06
hellozee pushed a commit to hellozee/vcpkg that referenced this pull request Sep 11, 2020
@koprok @JackBoosY @hellozee
[vcpkg] Fix vcpkg_fixup_cmake_targets to work correctly on MinGW (mic…
7a27e76 
…rosoft#12104)

* [vcpkg] Fix vcpkg_fixup_cmake_targets to work correctly on MinGW

* [akali] Bump version to test vcpkg_fixup_cmake_targets changes

* Update ports/akali/CONTROL

Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JackBoosY JackBoosY JackBoosY approved these changes

Assignees

@JackBoosY JackBoosY

Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@koprok @JackBoosY @linquize @strega-nil