Skip to content

Conversation

@msclock
Copy link

@msclock msclock commented Nov 19, 2024

This option enforces msvc to switch to _WIN64 of the header file vcruntime.h when building for x86, but __int64 does not exist actually.

#ifdef _WIN64
    typedef unsigned __int64 size_t;
    typedef __int64          ptrdiff_t;
    typedef __int64          intptr_t;
#else
    typedef unsigned int     size_t;
    typedef int              ptrdiff_t;
    typedef int              intptr_t;
#endif

And remove it to support x86 msvc build.

@danmar
Copy link
Owner

danmar commented Nov 19, 2024

Isn't _WIN64 supposed to be set by the compiler .. if it is not predefined you are supposed to use the non-_WIN64 typedefs?

I am not an expert in windows builds but it feels a bit hacky to me that you define _WIN64 unconditionally if a msvc compiler is used.

@msclock
Copy link
Author

msclock commented Nov 20, 2024

@danmar Sorry for my unclear explanation, vcruntime.h is part of MSVC compiler headers(C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.42.34433\include\vcruntime.h).

I package cppcheck as a python wheel. And it failed to build for x86 distribution in github ci. Now I just patch it temporarily in the vcpkg build.

@firewave
Copy link
Collaborator

Thanks for your contribution.

That change would cause x64 binaries not to be fully 64-bit - it needs to be handled differently. We have removed support for x86 in #5397 because we only ever properly supported it in Windows builds and there no longer are ways for other platforms to test it.

Why do you still need x86 builds?

Isn't _WIN64 supposed to be set by the compiler

No - it is specified in the Visual Studio projects explicitly for x64 projects. Same for WIN32. I do not know where _WIN32 is coming from. Needs some looking into (and possibly lead to some cleanups).

@firewave
Copy link
Collaborator

Okay - read up on multi-platform in CMake (it's been a while). I did not look into the defines yet.

How do you build Cppcheck? I assume you pass the -A option.

So to properly address the addition of _WIN64 only needs to be defined when CMAKE_GENERATOR_PLATFORM is x64.

@danmar
Copy link
Owner

danmar commented Nov 20, 2024

Thanks for the clarifications firewave

Why do you still need x86 builds?

yes I more or less wonder this too.. I assume you are stuck with 32-bit windows maybe you need support for some old DOS tool or something?

@msclock
Copy link
Author

msclock commented Nov 21, 2024

Why do you still need x86 builds?

there's many distributions from cibuildwheel based on arch and OS. And I just encountered the failed build on win32.

  +---------------+---------------------------+
  | OS            | Arch                      |
  +===============+===========================+
  | Windows       | | 64-bit                  |
  |               | | 32-bit                  |
  +---------------+---------------------------+
  | Linux Intel   | | manylinux2010+  x86_64  |
  |               | | musllinux_1_1+  x86_64  |
  |               | | manylinux2010+  i686    |
  |               | | musllinux_1_1+  i686    |
  +---------------+---------------------------+
  | Linux ARM     | | manylinux2014+  AArch64 |
  |               | | musllinux_1_1+  AArch64 |
  |               | | manylinux_2_31+ armv7l  |
  |               | | musllinux_1_2+  armv7l  |
  +---------------+---------------------------+
  | Linux PowerPC | | manylinux2014+  ppc64le |
  |               | | musllinux_1_1+  ppc64le |
  +---------------+---------------------------+
  | Linux IBM Z   | | manylinux2014+  s390x   |
  |               | | musllinux_1_1+  s390x   |
  +---------------+---------------------------+
  | macOS 10.10+  | Intel                     |
  +---------------+---------------------------+
  | macOS 11+     | Apple Silicon             |
  +---------------+---------------------------+

Anyway,I have passed through ci with some vcpkg tricks.And thanks for your all explanation. because of the unsupported test status in the repo and I'll close this.

@msclock msclock closed this Nov 21, 2024
@firewave
Copy link
Collaborator

there's many distributions from cibuildwheel based on arch and OS.

What are you using the wheel for? Are you building all of those? Are the build machines publicly available?

Given the variety of platforms that could be a nice extension of the CI. Too bad there is no ARM for Windows.

But I assume no tests are run in those builds so it would only be partially helpful in terms of getting it built but not having something actually working.

@danmar
Copy link
Owner

danmar commented Nov 21, 2024

Given the variety of platforms that could be a nice extension of the CI.

Yes.

@msclock
Copy link
Author

msclock commented Nov 22, 2024

What are you using the wheel for? Are you building all of those? Are the build machines publicly available?

I aim to package the wheel promptly for latest cppcheck update.

Currently I finished most distribution builds except Linux ARM with arch armv7l and Windowos Arm which I would have those two in the ETA.

There are some qemu setup in ci and docker commands seems to help cross-compliling and test. But it has unsupported tests in Win ARM as known.

@msclock
Copy link
Author

msclock commented Nov 23, 2024

except Linux ARM with arch armv7l and Windowos Arm

Now I have built all distros in that table from https://github.com/msclock/cppcheck-wheel/releases/tag/v1.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants