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

[boost-python] Allow feature python2 on Unix #18465

Merged
merged 2 commits into from
Jul 16, 2021
Merged

[boost-python] Allow feature python2 on Unix #18465

merged 2 commits into from
Jul 16, 2021

Conversation

Osyotr
Copy link
Contributor

@Osyotr Osyotr commented Jun 15, 2021

Since #18219 it is possible to build python2 for Unix. This PR also removes OS restrictions from python2 feature of boost-python.

@ghost
Copy link

ghost commented Jun 15, 2021 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@NancyLi1013 NancyLi1013 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jun 16, 2021
NancyLi1013
NancyLi1013 suggested changes Jun 16, 2021
View reviewed changes
ports/boost-python/vcpkg.json Outdated Show resolved Hide resolved
@Osyotr Osyotr requested a review from NancyLi1013 June 16, 2021 19:02
@NancyLi1013
Copy link
Contributor

Have you tested the feature python2 on all platforms? @Osyotr

@NancyLi1013 NancyLi1013 added requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function and removed requires:author-response labels Jun 17, 2021
@Osyotr
Copy link
Contributor Author

Osyotr commented Jun 17, 2021

@NancyLi1013 I've tested it on x64-windows and x64-linux-dynamic. Have no devices with macos. Perhaps it can be tested with azure pipelines somehow?

@NancyLi1013
Copy link
Contributor

@NancyLi1013 I've tested it on x64-windows and x64-linux-dynamic. Have no devices with macos. Perhaps it can be tested with azure pipelines somehow?

You can make sure it work fine on x86-windows, x64-windows-static and x64-linux. For macOS, it's optional. You can test it if you have azure pipelines.

@Osyotr
Copy link
Contributor Author

Osyotr commented Jun 17, 2021

@NancyLi1013
x86-windows - success
x64-windows - success
x64-windows-static - success
x64-linux - success

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function labels Jun 18, 2021
@NancyLi1013
Copy link
Contributor

LGTM, thanks for your PR @Osyotr.

@ras0219-msft
Copy link
Contributor

These changes will need to be reflected into https://github.com/microsoft/vcpkg/tree/master/scripts/boost before merging; I can look at doing that when I have time or if @Osyotr would like to work on it first that would be awesome!

@Osyotr
Copy link
Contributor Author

Osyotr commented Jun 22, 2021

@ras0219-msft although the file structure is intuitive, having some kind of readme for generate-ports.ps1 would be cool.
I've grep'ed the whole scripts/boost directory and changed everything related.

@Osyotr Osyotr requested a review from NancyLi1013 June 28, 2021 14:46
@NancyLi1013 NancyLi1013 removed the info:reviewed Pull Request changes follow basic guidelines label Jul 12, 2021
@NancyLi1013
Copy link
Contributor

@Osyotr

Could you please merge master to resole the conflicts?

@Osyotr Osyotr force-pushed the boost_python2 branch 2 times, most recently from 1979e1c to 69489a7 Compare July 12, 2021 20:29
@Osyotr
Copy link
Contributor Author

Osyotr commented Jul 12, 2021

@NancyLi1013
Done.

@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Jul 13, 2021
@BillyONeal BillyONeal merged commit 8d5a426 into microsoft:master Jul 16, 2021
@BillyONeal
Copy link
Member

Thanks for your contribution!

@Osyotr Osyotr deleted the boost_python2 branch July 16, 2021 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NancyLi1013 NancyLi1013 NancyLi1013 approved these changes

Assignees

@NancyLi1013 NancyLi1013

Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Osyotr @NancyLi1013 @ras0219-msft @BillyONeal