Open
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds Windows compatibility across the build system, filesystem utilities, and environment handling so the project can build and run on both Windows and Unix-like platforms.
- Updated GitHub Actions and CMake to detect and configure MSVC/Windows builds
- Introduced
#ifdef _WIN32branches for POSIX APIs in filesystem and shared logic - Added Windows-specific headers, typedefs, and WinAPI calls for file, directory, and time functions
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared.cpp | Wrapped POSIX includes and execute() with Windows stubs |
| src/format.cpp | Guarded strings.h include under _WIN32 |
| src/Table.cpp | Added Windows headers and _isatty logic |
| src/FS.h | Introduced windows.h include and mode_t typedef |
| src/FS.cpp | Implemented Windows fallbacks for access, mkdir, etc. |
| src/Datetime.cpp | Switched to _putenv_s/_tzset for timegm on Windows |
| cmake/CXXSniffer.cmake | Set /W4 flags when MSVC is detected |
| .github/workflows/cmake.yml | Added windows-latest runner and ctest invocation |
Comments suppressed due to low confidence (1)
.github/workflows/cmake.yml:41
- On Windows runners the default shell is PowerShell, so this Bash-style
ifmay not execute. Use${{ runner.os }}expressions or a PowerShell step for Windows-specific logic.
if [ "$RUNNER_OS" = "Windows" ]; then
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant changes to add native Windows compatibility to the project while maintaining support for other platforms. The changes include updates to the build system, platform-specific handling in the codebase, and various compatibility adjustments for Windows. Below is a categorized summary of the most important changes:
Build System Updates
.github/workflows/cmake.yml). This includes adding a Windows runner and modifying the CMake configuration, build, and test steps to handle Windows-specific paths and commands.Platform-Specific Code Adjustments
CXXSniffer.cmaketo use appropriate compiler flags for Windows (/W4) and other platforms (-Wall -Wextra).Datetime::timegminsrc/Datetime.cppto handle timezone environment variables differently on Windows using_putenv_sand_tzset. [1] [2]Windows-Specific Implementations in
FS.cppsrc/FS.cppto handle file system operations like directory creation, file locking, and path expansion on Windows. [1] [2]realpath,access,unlink) with Windows equivalents (e.g.,GetFullPathNameA,_access,_unlink) where applicable. [1] [2] [3]FindFirstFileAandFindNextFileA. [1] [2] [3]File and Directory Operations
mkdir,rmdir,chdir, and file truncation using_mkdir,_rmdir,_chdir, and_chsize. [1] [2] [3]FlushFileBufferson Windows instead offsyncorfdatasync.Minor Fixes
andwith&&insrc/PEG.cppas MSVC doesn't recognize them when using Windows headers.