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

Use OS-included Python 2.7 on macOS #14662

Merged
merged 1 commit into from
Nov 24, 2020
Merged

Conversation

LRFLEW
Copy link
Contributor

@LRFLEW LRFLEW commented Nov 20, 2020

This fixes building duktape*, as well as other ports that depend on python2 (I haven't done a full scan for what else would be included). I didn't find a Github issue for this, but it does relate to PR #10030.

This updates vcpkg_find_acquire_program(PYTHON2) to change PROGNAME from python2 to python on macOS. Apple includes Python 2.7 under this name, but not as python2, in all (supported) OS releases. It is deprecated, but it is included in 11.0 Big Sur from what I've read, and because Python 2 was removed from Homebrew, this will allow it to work for now at least.

* duktape still has a minor issue caused by easy_install needing to be executed as root, but installing pyyaml ahead of time allows the build to succeed, even with easy_install failing.

@JackBoosY JackBoosY added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Nov 20, 2020
Copy link
Contributor

@ras0219 ras0219 left a comment

Choose a reason for hiding this comment

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

LGTM; could you also consider adding a message to duktape about it's pyyaml dependency? Something like

if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin")
  message(STATUS "Duktape requires pyyaml to build -- this is available with `easy_install pyyaml`")
endif()

or somesuch

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Nov 20, 2020

I was planning on making an issue for the pyyaml duktape issue once this was merged. (I was trying to keep this change in the tooling separate from the changes in the port). There seems to be some complexity to it, because the fix could be changing easy_install pyyaml to either sudo easy_install pyyaml or easy_install --user pyyaml. There's also the fact that the easy_install command requires having run brew install libyaml ahead of time (from what I can tell at least).

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Nov 23, 2020
@strega-nil strega-nil merged commit 38752e2 into microsoft:master Nov 24, 2020
@strega-nil
Copy link
Contributor

@LRFLEW thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants