From cde19f12f4d277bbe24a1aaaa7c41eabd8e257b4 Mon Sep 17 00:00:00 2001 From: oO <47592289+oocytanb@users.noreply.github.com> Date: Mon, 7 Jun 2021 00:21:03 +0900 Subject: [PATCH 1/8] Fix build configuration for msvc --- setup.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index ed627c8..9113e58 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,9 @@ import os import subprocess +import sys from distutils.version import LooseVersion from glob import glob +from itertools import chain from os.path import exists, join from subprocess import run @@ -10,6 +12,8 @@ import setuptools.command.develop from setuptools import Extension, find_packages, setup +platform_is_windows = sys.platform == "win32" + version = "0.1.2" min_cython_ver = "0.21.0" @@ -26,13 +30,57 @@ raise ImportError("No supported version of Cython installed.") from Cython.Distutils import build_ext + msvc_extra_compile_args_config = [ + "/source-charset:utf-8", + "/execution-charset:utf-8", + ] + + # escape double quotes for msvc + def msvc_string_macro_arg(s): + n = len(s) + return ( + s.replace('\\', '\\\\').replace('"', '\\"') + if n >= 2 and s.startswith('"') and s.endswith('"') + else s + ) + + def msvc_macro(x): + (k, arg) = x + return (k, msvc_string_macro_arg(arg)) if type(arg) == str else x + + def msvc_define_macros(macros): + return list(map(msvc_macro, macros)) + + def msvc_extra_compile_args(compile_args): + cas = set(compile_args) + xs = filter(lambda x: x not in cas, msvc_extra_compile_args_config) + return list(chain(compile_args, xs)) + + def msvc_extension(ext): + ext.define_macros = msvc_define_macros( + ext.define_macros if hasattr(ext, 'define_macros') else [] + ) + + ext.extra_compile_args = msvc_extra_compile_args( + ext.extra_compile_args if hasattr(ext, 'extra_compile_args') else [] + ) + + class custom_build_ext(build_ext): + def build_extensions(self): + compiler_type_is_msvc = self.compiler.compiler_type == "msvc" + for entry in self.extensions: + if compiler_type_is_msvc: + msvc_extension(entry) + + build_ext.build_extensions(self) + cython = True except ImportError: cython = False if cython: ext = ".pyx" - cmdclass = {"build_ext": build_ext} + cmdclass = {"build_ext": custom_build_ext} else: ext = ".cpp" cmdclass = {} @@ -104,6 +152,7 @@ include_dirs=[np.get_include(), join(htsengine_src_top, "include")], extra_compile_args=[], extra_link_args=[], + libraries=["winmm"] if platform_is_windows else [], language="c++", ) ] @@ -150,7 +199,7 @@ def run(self): cmdclass["develop"] = develop -with open("README.md", "r") as fd: +with open("README.md", "r", encoding="utf8") as fd: long_description = fd.read() setup( From d05eb0bee396b562f2946b904fc854ffaad9e5c4 Mon Sep 17 00:00:00 2001 From: oO <47592289+oocytanb@users.noreply.github.com> Date: Fri, 11 Jun 2021 23:28:54 +0900 Subject: [PATCH 2/8] Fix handling of string macro --- setup.py | 80 +++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 59 insertions(+), 21 deletions(-) diff --git a/setup.py b/setup.py index 9113e58..24e34ae 100644 --- a/setup.py +++ b/setup.py @@ -2,6 +2,7 @@ import subprocess import sys from distutils.version import LooseVersion +from distutils.spawn import spawn from glob import glob from itertools import chain from os.path import exists, join @@ -35,8 +36,54 @@ "/execution-charset:utf-8", ] - # escape double quotes for msvc - def msvc_string_macro_arg(s): + def msvc_extra_compile_args(compile_args): + cas = set(compile_args) + xs = filter(lambda x: x not in cas, msvc_extra_compile_args_config) + return list(chain(compile_args, xs)) + + def test_quoted_arg_change(): + # Workaround for `distutils.spawn` problem on Windows python < 3.9 + # See details: [bpo-39763: distutils.spawn now uses subprocess (GH-18743)] + # (https://github.com/python/cpython/commit/1ec63b62035e73111e204a0e03b83503e1c58f2e) + + child_script = """ +import os +import sys +if len(sys.argv) > 4: + try: + os.makedirs(sys.argv[1], exist_ok=True) + with open(sys.argv[2], sys.argv[3]) as f: + f.write(sys.argv[4]) + except: + pass +""" + + try: + # write + package_build_dir = "build" + output_file = join(package_build_dir, "quoted_arg_output") + output_mode = "w" + arg_value = '"ARG"' + + spawn([ + sys.executable, + "-c", + child_script, + package_build_dir, + output_file, + output_mode, + arg_value + ]) + + # read + with open(output_file, "r") as f: + return f.readline() != arg_value + except: + return False + + need_to_escape_string_macro = platform_is_windows and test_quoted_arg_change() + + def escape_string_macro_arg(s): n = len(s) return ( s.replace('\\', '\\\\').replace('"', '\\"') @@ -44,33 +91,24 @@ def msvc_string_macro_arg(s): else s ) - def msvc_macro(x): + def escape_macro_element(x): (k, arg) = x - return (k, msvc_string_macro_arg(arg)) if type(arg) == str else x + return (k, escape_string_macro_arg(arg)) if type(arg) == str else x - def msvc_define_macros(macros): - return list(map(msvc_macro, macros)) - - def msvc_extra_compile_args(compile_args): - cas = set(compile_args) - xs = filter(lambda x: x not in cas, msvc_extra_compile_args_config) - return list(chain(compile_args, xs)) - - def msvc_extension(ext): - ext.define_macros = msvc_define_macros( - ext.define_macros if hasattr(ext, 'define_macros') else [] - ) - - ext.extra_compile_args = msvc_extra_compile_args( - ext.extra_compile_args if hasattr(ext, 'extra_compile_args') else [] - ) + def escape_macros(macros): + return list(map(escape_macro_element, macros)) class custom_build_ext(build_ext): def build_extensions(self): compiler_type_is_msvc = self.compiler.compiler_type == "msvc" for entry in self.extensions: if compiler_type_is_msvc: - msvc_extension(entry) + entry.extra_compile_args = msvc_extra_compile_args( + entry.extra_compile_args if hasattr(entry, "extra_compile_args") else [] + ) + + if need_to_escape_string_macro and hasattr(entry, "define_macros"): + entry.define_macros = escape_macros(entry.define_macros) build_ext.build_extensions(self) From 9e08bf0625b4a74abda778fe0fda8966d9bdc64b Mon Sep 17 00:00:00 2001 From: oO <47592289+oocytanb@users.noreply.github.com> Date: Sat, 12 Jun 2021 02:07:05 +0900 Subject: [PATCH 3/8] Catch OSError --- setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 24e34ae..ea0a106 100644 --- a/setup.py +++ b/setup.py @@ -54,7 +54,7 @@ def test_quoted_arg_change(): os.makedirs(sys.argv[1], exist_ok=True) with open(sys.argv[2], sys.argv[3]) as f: f.write(sys.argv[4]) - except: + except OSError: pass """ @@ -78,7 +78,7 @@ def test_quoted_arg_change(): # read with open(output_file, "r") as f: return f.readline() != arg_value - except: + except OSError: return False need_to_escape_string_macro = platform_is_windows and test_quoted_arg_change() From be6b8b09acd73ad635414ae8f7c76d4eccd07ebf Mon Sep 17 00:00:00 2001 From: oO <47592289+oocytanb@users.noreply.github.com> Date: Sat, 12 Jun 2021 13:42:00 +0900 Subject: [PATCH 4/8] Specify the encoding of quoted_arg file --- setup.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/setup.py b/setup.py index ea0a106..70bdc4b 100644 --- a/setup.py +++ b/setup.py @@ -49,11 +49,11 @@ def test_quoted_arg_change(): child_script = """ import os import sys -if len(sys.argv) > 4: +if len(sys.argv) > 5: try: os.makedirs(sys.argv[1], exist_ok=True) - with open(sys.argv[2], sys.argv[3]) as f: - f.write(sys.argv[4]) + with open(sys.argv[2], mode=sys.argv[3], encoding=sys.argv[4]) as f: + f.write(sys.argv[5]) except OSError: pass """ @@ -63,6 +63,7 @@ def test_quoted_arg_change(): package_build_dir = "build" output_file = join(package_build_dir, "quoted_arg_output") output_mode = "w" + output_encoding = "utf8" arg_value = '"ARG"' spawn([ @@ -72,11 +73,12 @@ def test_quoted_arg_change(): package_build_dir, output_file, output_mode, + output_encoding, arg_value ]) # read - with open(output_file, "r") as f: + with open(output_file, mode="r", encoding=output_encoding) as f: return f.readline() != arg_value except OSError: return False From 06d9e1d0a78df98e9c1fbb9a225b16fb93f09684 Mon Sep 17 00:00:00 2001 From: oO <47592289+oocytanb@users.noreply.github.com> Date: Sat, 12 Jun 2021 02:18:19 +0900 Subject: [PATCH 5/8] Add Windows to CI matrix --- .github/workflows/ci.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d9a4b83..cbb60b3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -12,9 +12,10 @@ on: jobs: build: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} strategy: matrix: + os: [ubuntu-latest, windows-latest] python-version: [3.7, 3.8, 3.9] steps: From 7d8428b01c006d2cf532515cf284f1edb581f822 Mon Sep 17 00:00:00 2001 From: oO <47592289+oocytanb@users.noreply.github.com> Date: Mon, 14 Jun 2021 20:00:45 +0900 Subject: [PATCH 6/8] Add `define_macros` function --- setup.py | 144 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 74 insertions(+), 70 deletions(-) diff --git a/setup.py b/setup.py index 70bdc4b..ed21ad7 100644 --- a/setup.py +++ b/setup.py @@ -26,92 +26,33 @@ except ImportError: _CYTHON_INSTALLED = False + +msvc_extra_compile_args_config = [ + "/source-charset:utf-8", + "/execution-charset:utf-8", +] + try: if not _CYTHON_INSTALLED: raise ImportError("No supported version of Cython installed.") from Cython.Distutils import build_ext - msvc_extra_compile_args_config = [ - "/source-charset:utf-8", - "/execution-charset:utf-8", - ] - def msvc_extra_compile_args(compile_args): cas = set(compile_args) xs = filter(lambda x: x not in cas, msvc_extra_compile_args_config) return list(chain(compile_args, xs)) - def test_quoted_arg_change(): - # Workaround for `distutils.spawn` problem on Windows python < 3.9 - # See details: [bpo-39763: distutils.spawn now uses subprocess (GH-18743)] - # (https://github.com/python/cpython/commit/1ec63b62035e73111e204a0e03b83503e1c58f2e) - - child_script = """ -import os -import sys -if len(sys.argv) > 5: - try: - os.makedirs(sys.argv[1], exist_ok=True) - with open(sys.argv[2], mode=sys.argv[3], encoding=sys.argv[4]) as f: - f.write(sys.argv[5]) - except OSError: - pass -""" - - try: - # write - package_build_dir = "build" - output_file = join(package_build_dir, "quoted_arg_output") - output_mode = "w" - output_encoding = "utf8" - arg_value = '"ARG"' - - spawn([ - sys.executable, - "-c", - child_script, - package_build_dir, - output_file, - output_mode, - output_encoding, - arg_value - ]) - - # read - with open(output_file, mode="r", encoding=output_encoding) as f: - return f.readline() != arg_value - except OSError: - return False - - need_to_escape_string_macro = platform_is_windows and test_quoted_arg_change() - - def escape_string_macro_arg(s): - n = len(s) - return ( - s.replace('\\', '\\\\').replace('"', '\\"') - if n >= 2 and s.startswith('"') and s.endswith('"') - else s - ) - - def escape_macro_element(x): - (k, arg) = x - return (k, escape_string_macro_arg(arg)) if type(arg) == str else x - - def escape_macros(macros): - return list(map(escape_macro_element, macros)) - class custom_build_ext(build_ext): def build_extensions(self): compiler_type_is_msvc = self.compiler.compiler_type == "msvc" for entry in self.extensions: if compiler_type_is_msvc: entry.extra_compile_args = msvc_extra_compile_args( - entry.extra_compile_args if hasattr(entry, "extra_compile_args") else [] + entry.extra_compile_args + if hasattr(entry, "extra_compile_args") + else [] ) - if need_to_escape_string_macro and hasattr(entry, "define_macros"): - entry.define_macros = escape_macros(entry.define_macros) - build_ext.build_extensions(self) cython = True @@ -127,6 +68,69 @@ def build_extensions(self): if not os.path.exists(join("pyopenjtalk", "openjtalk" + ext)): raise RuntimeError("Cython is required to generate C++ code") + +# Workaround for `distutils.spawn` problem on Windows python < 3.9 +# See details: [bpo-39763: distutils.spawn now uses subprocess (GH-18743)] +# (https://github.com/python/cpython/commit/1ec63b62035e73111e204a0e03b83503e1c58f2e) +def test_quoted_arg_change(): + child_script = """ +import os +import sys +if len(sys.argv) > 5: + try: + os.makedirs(sys.argv[1], exist_ok=True) + with open(sys.argv[2], mode=sys.argv[3], encoding=sys.argv[4]) as fd: + fd.write(sys.argv[5]) + except OSError: + pass +""" + + try: + # write + package_build_dir = "build" + file_name = join(package_build_dir, "quoted_arg_output") + output_mode = "w" + file_encoding = "utf8" + arg_value = '"ARG"' + + spawn([ + sys.executable, + "-c", + child_script, + package_build_dir, + file_name, + output_mode, + file_encoding, + arg_value + ]) + + # read + with open(file_name, mode="r", encoding=file_encoding) as fd: + return fd.readline() != arg_value + except Exception: + return False + + +def escape_string_macro_arg(s): + return s.replace('\\', '\\\\').replace('"', '\\"') + + +def escape_macro_element(x): + (k, arg) = x + return (k, escape_string_macro_arg(arg)) if type(arg) == str else x + + +def escape_macros(macros): + return list(map(escape_macro_element, macros)) + + +define_macros = ( + escape_macros + if platform_is_windows and test_quoted_arg_change() + else (lambda macros: macros) +) + + # open_jtalk sources src_top = join("lib", "open_jtalk", "src") @@ -171,14 +175,14 @@ def build_extensions(self): extra_compile_args=[], extra_link_args=[], language="c++", - define_macros=[ + define_macros=define_macros([ ("HAVE_CONFIG_H", None), ("DIC_VERSION", 102), ("MECAB_DEFAULT_RC", '"dummy"'), ("PACKAGE", '"open_jtalk"'), ("VERSION", '"1.10"'), ("CHARSET_UTF_8", None), - ], + ]), ) ] From 4ae90ef23ebe0df81406a5ce1b12eae4fd3ec9ff Mon Sep 17 00:00:00 2001 From: oO <47592289+oocytanb@users.noreply.github.com> Date: Wed, 16 Jun 2021 23:40:40 +0900 Subject: [PATCH 7/8] Incorporate the opinions of reviewer --- setup.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/setup.py b/setup.py index ed21ad7..e208534 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,7 @@ import os import subprocess import sys +from distutils.errors import DistutilsExecError from distutils.version import LooseVersion from distutils.spawn import spawn from glob import glob @@ -37,6 +38,13 @@ raise ImportError("No supported version of Cython installed.") from Cython.Distutils import build_ext + cython = True +except ImportError: + cython = False + +if cython: + ext = ".pyx" + def msvc_extra_compile_args(compile_args): cas = set(compile_args) xs = filter(lambda x: x not in cas, msvc_extra_compile_args_config) @@ -55,12 +63,6 @@ def build_extensions(self): build_ext.build_extensions(self) - cython = True -except ImportError: - cython = False - -if cython: - ext = ".pyx" cmdclass = {"build_ext": custom_build_ext} else: ext = ".cpp" @@ -107,7 +109,7 @@ def test_quoted_arg_change(): # read with open(file_name, mode="r", encoding=file_encoding) as fd: return fd.readline() != arg_value - except Exception: + except (DistutilsExecError, TypeError): return False @@ -124,7 +126,7 @@ def escape_macros(macros): return list(map(escape_macro_element, macros)) -define_macros = ( +custom_define_macros = ( escape_macros if platform_is_windows and test_quoted_arg_change() else (lambda macros: macros) @@ -175,7 +177,7 @@ def escape_macros(macros): extra_compile_args=[], extra_link_args=[], language="c++", - define_macros=define_macros([ + define_macros=custom_define_macros([ ("HAVE_CONFIG_H", None), ("DIC_VERSION", 102), ("MECAB_DEFAULT_RC", '"dummy"'), From e20bc37b4d0609ed5dfad2e10594a820df69fd7b Mon Sep 17 00:00:00 2001 From: oO <47592289+oocytanb@users.noreply.github.com> Date: Wed, 16 Jun 2021 23:45:09 +0900 Subject: [PATCH 8/8] Update open_jtalk submodule to point to the latest revision --- lib/open_jtalk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_jtalk b/lib/open_jtalk index 2221314..9572293 160000 --- a/lib/open_jtalk +++ b/lib/open_jtalk @@ -1 +1 @@ -Subproject commit 22213146d3491d708ec9b827851de8c0bbca29d7 +Subproject commit 957229334996d2c9d9fcb73cdb3f4d9c15bcdd57