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

ICU-20160 Add check for Python 3 in autoconf and nmake. #157

Merged
merged 4 commits into from
Sep 21, 2018
Merged

ICU-20160 Add check for Python 3 in autoconf and nmake. #157

merged 4 commits into from
Sep 21, 2018

Conversation

sffc
Copy link
Member

@sffc sffc commented Sep 20, 2018 •
edited
Loading

Checklist

@sffc sffc requested a review from srl295 September 20, 2018 06:32
@sffc
Copy link
Member Author

sffc commented Sep 20, 2018

@jefgen Suggestions on where to put this for the Windows build?

@sffc sffc requested a review from jefgen September 20, 2018 06:41
@jefgen
Copy link
Member

jefgen commented Sep 20, 2018 •
edited
Loading

@sffc I'm trying to think of a location that wouldn't break anyone... I think maybe one location might be in the file makedata.mak as an informational message only (so as to not break the build).

File: https://github.com/unicode-org/icu/blob/master/icu4c/source/data/makedata.mak

Perhaps adding something like this near the beginning of the file?

!MESSAGE Information: Notice: ICU 64 will require Python 3 to build. See ICU-10923 for more information.

srl295
srl295 previously approved these changes Sep 20, 2018
View reviewed changes
Copy link
Member

@srl295 srl295 left a comment

Choose a reason for hiding this comment

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

going to approve this but with suggestions

icu4c/source/configure.ac Outdated Show resolved Hide resolved
@sffc sffc changed the title ICU-20160 Add autoconf check for python3. ICU-20160 Add check for Python 3 in autoconf and nmake. Sep 21, 2018
@sffc
Copy link
Member Author

sffc commented Sep 21, 2018

Added Windows and made changes requested by Steven. Please take another look.

jefgen
jefgen previously approved these changes Sep 21, 2018
View reviewed changes
Copy link
Member

@jefgen jefgen left a comment

Choose a reason for hiding this comment

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

LGTM for the Windows changes. Thanks Shane.

@sffc
Copy link
Member Author

sffc commented Sep 21, 2018

One more try on Windows. It looks like py -3 works but not python3, at least not based on the official Python 3 installation from https://www.python.org/downloads/.

@sffc
Copy link
Member Author

sffc commented Sep 21, 2018

@srl295 @jefgen Can I have approval to merge this in?

@sffc sffc merged commit 9fc064c into unicode-org:master Sep 21, 2018
@sffc sffc deleted the ICU-20160 branch September 21, 2018 22:08
sffc added a commit to sffc/icu that referenced this pull request Sep 26, 2018
@sffc
ICU-20160 Add check for Python 3 in autoconf and nmake. (unicode-org#157
636d849 
)
sffc added a commit to sffc/icu that referenced this pull request Sep 26, 2018
@sffc
ICU-20160 Add check for Python 3 in autoconf and nmake. (unicode-org#157
c14f309 
)
sffc added a commit to sffc/icu that referenced this pull request Sep 27, 2018
@sffc
ICU-20160 Add check for Python 3 in autoconf and nmake. (unicode-org#157
de4537d 
)
sffc added a commit to sffc/icu that referenced this pull request Sep 27, 2018
@sffc
ICU-20160 Add check for Python 3 in autoconf and nmake. (unicode-org#157
236afad 
)
sffc added a commit to sffc/icu that referenced this pull request Sep 27, 2018
@sffc
ICU-20160 Add check for Python 3 in autoconf and nmake. (unicode-org#157
844c2a0 
)
sffc added a commit to sffc/icu that referenced this pull request Sep 27, 2018
@sffc
ICU-20160 Add check for Python 3 in autoconf and nmake. (unicode-org#157
1bf2923 
)
sffc added a commit to sffc/icu that referenced this pull request Sep 27, 2018
@sffc
ICU-20160 Add check for Python 3 in autoconf and nmake. (unicode-org#157
d271106 
)
sffc added a commit to sffc/icu that referenced this pull request Sep 27, 2018
@sffc
ICU-20160 Add check for Python 3 in autoconf and nmake. (unicode-org#157
a38beef 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srl295 srl295 srl295 approved these changes

@jefgen jefgen jefgen left review comments

Assignees

@srl295 srl295

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sffc @jefgen @srl295