Build and test OpenCC with Bazel on Windows#1082
Conversation
…tform-specific issues Agent-Logs-Url: https://github.com/BYVoid/OpenCC/sessions/486fc760-0820-4478-b99a-e71e3bbd2db8 Co-authored-by: BYVoid <245270+BYVoid@users.noreply.github.com>
…/chdir Agent-Logs-Url: https://github.com/BYVoid/OpenCC/sessions/486fc760-0820-4478-b99a-e71e3bbd2db8 Co-authored-by: BYVoid <245270+BYVoid@users.noreply.github.com>
Agent-Logs-Url: https://github.com/BYVoid/OpenCC/sessions/806bc585-a3b6-4934-b8e7-2a846d8efe30 Co-authored-by: BYVoid <245270+BYVoid@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07dfe2d91c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| build:windows --cxxopt=/std:c++17 | ||
| build:windows --host_cxxopt=/std:c++17 |
There was a problem hiding this comment.
The new build:windows entries are never applied unless Bazel is invoked with --config=windows (or --enable_platform_specific_config), and this workflow still runs plain bazel build / bazel test on windows-latest. Since Bazel’s --enable_platform_specific_config defaults to false, the /std:c++17 flags in this file won’t reach MSVC, so the Windows CI job can still compile with the wrong language standard.
Useful? React with 👍 / 👎.
All other paths passed to the shell command are already quoted via QuotePath(), but the -c config path was left unquoted. This could break on CMake builds where CMAKE_SOURCE_DIR contains spaces. https://claude.ai/code/session_01FUbRQbj3PxYv445SuwXDm2
…lied The build:windows config block requires --enable_platform_specific_config (or explicit --config=windows) to take effect. Without it, the /std:c++17 flags never reach MSVC on Windows CI. Adding the enable flag makes Bazel automatically apply the matching platform config. https://claude.ai/code/session_01FUbRQbj3PxYv445SuwXDm2
…dent fopen Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bazel build //:openccand the test suite were broken on Windows due to POSIX-only APIs, Unix-specific runfiles path discovery, and missing C++17 flags for MSVC. Additionally, acmd.exequote-stripping bug causedCommandLineConvertTestto fail in CMake builds on Windows.CI
windows-latestto the Bazel CI matrix (.github/workflows/bazel.yml)Build config
.bazelrc: setsbuild:windows --cxxopt=/std:c++17— MSVC defaults to C++14 under Bazeltest/CommandLineConvertTest.cppgetcwd/chdirwithportable_getcwd()/portable_chdir()wrappers (_getcwd/_chdirfrom<direct.h>on MSVC)command_line.exefirst inOpenccCommand()(Bazel runfiles manifest uses.exeforcc_binary)QuotePath()to handle spaces_WIN32inTestCommand():system()callscmd.exe /C <cmd>. When<cmd>starts with"(quoted exe path) but doesn't end with", cmd.exe strips the leading"and removes the last"anywhere in the string — corrupting the output file path and producing "The filename, directory name, or volume label syntax is incorrect." Wrapping in outer quotes makes cmd.exe strip only that outer pair, leaving the inner quoted paths intact.data/dictionary/DictionaryTest.cpp+BUILD.bazel.runfiles/_mainsuffix-search onargv[0]withRunfiles::CreateForTest()+Rlocation()— the symlink-based layout assumed by the old approach doesn't exist on Windows; Bazel uses a manifest file instead@bazel_tools//tools/cpp/runfilesdep todictionary_test