Platform compatibility with MinGW and HPE NonStop #99
Conversation
This file contains 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
The NonStop platform does not have `mkdtemp()` available, which we rely on in `build_sandbox_path()`. Fix this issue by using `mktemp()` and `mkdir()` instead on this platform. Original-patch-by: Randall S. Becker <[email protected]> Signed-off-by: Patrick Steinhardt <[email protected]>
When using mingw-w64 to compile the code, and using `_stat()`, it is necessary to use `struct _stat`, too, and not `struct stat` (as the latter is incompatible with the "dashed" version because it is limited to 32-bit time types for backwards compatibility). Original-patch-by: Johannes Schindelin <[email protected]> Signed-off-by: Patrick Steinhardt <[email protected]>
When CLAR_FIXTURE_PATH is unset, the `fs_copy()` function seems not to be used. But it is declared as `static`, and GCC does not like that, complaining that it should not be declared/defined to begin with. We could mark this function as (potentially) unused by following the `MAYBE_UNUSED` pattern from Git's `git-compat-util.h`. However, this is a GCC-only construct that is not understood by Visual C. Besides, `clar` does not use that pattern at all. Instead, let's use the `((void)SYMBOL);` pattern that `clar` already uses elsewhere; This avoids the compile error by sorta kinda make the function used after a fashion. Note: GCC 14.x (which Git for Windows' SDK already uses) is able to figure out that this function is unused even though there are recursive calls between `fs_copy()` and `fs_copydir_helper()`; Earlier GCC versions do not detect that, and therefore the issue has been hidden from the regular Linux CI builds (where GCC 14.x is not yet used). That is the reason why this change is only made in the Windows-specific portion of `t/unit-tests/clar/clar/fs.h`. Original-patch-by: Johannes Schindelin <[email protected]> Signed-off-by: Patrick Steinhardt <[email protected]>
The `shellapi.h` header was included as of clar-test@136e763211aa, to have `SHFileOperation()` declared so that it could be called. However, clar-test@5ce31b69b525 removed that call, and therefore that `#include <shellapi.h>` is unnecessary. It is also unwanted in Git because this project uses a subset of Git for Windows' SDK in its CI builds that (for bandwidth reasons) excludes tons of header files, including `shellapi.h`. So let's remove it. Note: Since the `windows.h` header would include `shellapi.h` anyway, we also define `WIN32_LEAN_AND_MEAN` to avoid this and similar other unnecessary includes before including `windows.h`. Original-patch-by: Johannes Schindelin <[email protected]> Signed-off-by: Patrick Steinhardt <[email protected]>
dscho
approved these changes
Sep 5, 2024
|
Note that I'm wiring up support for Win32 CI in #100, which surfaces some of the Windows issues here. |
ethomson
approved these changes
Sep 5, 2024
|
Thanks! |
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.
Fix a couple of platform compatibility issues with MinGW and HPE NonStop. These patches are required by Git to make the clar work for all currently tested platforms.
The HPE NonStop changes have been picked from #96, removing the other changes to make the CFLAGS overridable. The Windows fixes have been picked from the downstream patch series in Git.
The patches were originally by @rsbeckerca and @dscho. I have noted authorship via
Original-patch-bytrailers, but am happy to attribute full authorship of the commits if preferred by the authors.