Skip to content

support for macOS and Windows #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

gerwin3
Copy link

@gerwin3 gerwin3 commented Jan 8, 2025

  • added CI
  • macOS support
  • Windows

Note that the MSVC ABI is NOT supported for Windows. Just the default mingw included ABI

@BanacialPC2
Copy link

Got it to compile on windows without specifying Windows MSVC using .version = "2.16.1-4", and zig "0.15.0-dev.77+aa8aa6625";

I actually had to include "nasmlib/ilog2.c", again, and rewrite two macros in include\nasmint.h
#define INT64_C(x) x##LL // instead of x ## i64
#define UINT64_C(x) x##ULL // instead of x ## ui64

These Flags I have set to 1, else is null or the default / linux thing (from ._XOPEN_SOURCE onwards)
.CFLAGS_STD_C17 = 1,
.HAVE_ACCESS = 1,
.HAVE_DECL_STRICMP = 1,
.HAVE_DECL_STRNICMP = 1,
.HAVE_FCNTL_H = 1,
.HAVE_IO_H = 1,
.HAVE_MEMPCPY = 1,
.HAVE_SNPRINTF = 1,
.HAVE_STDINT_H = 1,
.HAVE_STDLIB_H = 1,
.HAVE_STRICMP = 1,
.HAVE_STRING_H = 1,
.HAVE_STRNICMP = 1,
.HAVE_STRNLEN = 1,
.HAVE_SYS_STAT_H = 1,
.HAVE_SYS_TYPES_H = 1,
.HAVE_VSNPRINTF = 1,
.HAVE__CHSIZE = 1,
.HAVE__FILENO = 1,
.HAVE__FSEEKI64 = 1,
.HAVE__FSTATI64 = 1,
.HAVE__FULLPATH = 1,
.HAVE__STATI64 = 1,
.HAVE__FILELENGTHI64 = 1,
.PACKAGE_BUGREPORT = "",
.PACKAGE_NAME = "",
.PACKAGE_STRING = "",
.PACKAGE_TARNAME = "",
.PACKAGE_URL = "",
.PACKAGE_VERSION = "",
.STDC_HEADERS = 1,
._ALL_SOURCE = 1,
._DARWIN_C_SOURCE = 1,
.EXTENSIONS = 1,
._GNU_SOURCE = 1,
._HPUX_ALT_XOPEN_SOCKET_API = 1,
._MINIX = null,
._NETBSD_SOURCE = 1,
._OPENBSD_SOURCE = 1,
._POSIX_SOURCE = null,
._POSIX_1_SOURCE = null,
._POSIX_PTHREAD_SEMANTICS = 1,
.STDC_WANT_IEC_60559_ATTRIBS_EXT = 1,
.STDC_WANT_IEC_60559_BFP_EXT = 1,
.STDC_WANT_IEC_60559_DFP_EXT = 1,
.STDC_WANT_IEC_60559_FUNCS_EXT = 1,
.STDC_WANT_IEC_60559_TYPES_EXT = 1,
.STDC_WANT_LIB_EXT2 = 1,
.STDC_WANT_MATH_SPEC_FUNCS = 1,
._TANDEM_SOURCE = 1,

@gerwin3
Copy link
Author

gerwin3 commented Apr 3, 2025

Nice. Do the code changes required not break Linux/mac?

@Banacial
Copy link

Banacial commented Apr 5, 2025

Nice. Do the code changes required not break Linux/mac?

I used your build system, removed the windows depended usage of nasm/ilog2.c, removed the msvc dependence check and changed the config.h flags as told above.

Well, compiling without errors might just mean the compilation didn't encounter known errors, not that the build programm actually works. But I had no problems compiling with :

zig build -Dtarget=x86_64-linux -Doptimize=ReleaseSafe
zig build -Dtarget=x86_64-windows -Doptimize=ReleaseSafe
zig build -Dtarget=x86_64-macos -Doptimize=ReleaseSafe
zig build -Dtarget=aarch64-linux -Doptimize=ReleaseSafe

What bugs me is that I had to make changes to a nasm-native file: include/nasmint.h

zig cc -dM -E - < NUL (on my windows machine tells me that on my zig x86-64 windows) clang has for example predefined:

#define LLONG_WIDTH 64
#define LONG_LONG_MAX 9223372036854775807LL
#define LONG_MAX 2147483647L
#define LONG_WIDTH 32
and some more, but the nasmint.h defines under certain conditions

#define INT64_C(x) x ## i64
#define UINT64_C(x) x ## ui64

which clang cannot expand. So I with my limited compiler knowledge have no idea why that macro-branch is used instead of the other which define.

#define INT64_C(x) x ## L
#define UINT64_C(x) x ## UL

@Banacial
Copy link

Banacial commented Apr 5, 2025

oh and I have no idea what a yalm or .ci file is used for: Good old manual browser download boy, no fancy git

@Banacial
Copy link

Banacial commented Apr 5, 2025

Nice. Do the code changes required not break Linux/mac?

https://github.com/Banacial/Files_I_Need/blob/main/build_nasm.zig

@gerwin3
Copy link
Author

gerwin3 commented Apr 7, 2025

Nice. Do the code changes required not break Linux/mac?

I used your build system, removed the windows depended usage of nasm/ilog2.c, removed the msvc dependence check and changed the config.h flags as told above.

Well, compiling without errors might just mean the compilation didn't encounter known errors, not that the build programm actually works. But I had no problems compiling with :

zig build -Dtarget=x86_64-linux -Doptimize=ReleaseSafe zig build -Dtarget=x86_64-windows -Doptimize=ReleaseSafe zig build -Dtarget=x86_64-macos -Doptimize=ReleaseSafe zig build -Dtarget=aarch64-linux -Doptimize=ReleaseSafe

What bugs me is that I had to make changes to a nasm-native file: include/nasmint.h

zig cc -dM -E - < NUL (on my windows machine tells me that on my zig x86-64 windows) clang has for example predefined:

#define LLONG_WIDTH 64 #define LONG_LONG_MAX 9223372036854775807LL #define LONG_MAX 2147483647L #define LONG_WIDTH 32 and some more, but the nasmint.h defines under certain conditions

#define INT64_C(x) x ## i64 #define UINT64_C(x) x ## ui64

which clang cannot expand. So I with my limited compiler knowledge have no idea why that macro-branch is used instead of the other which define.

#define INT64_C(x) x ## L #define UINT64_C(x) x ## UL

Yeah bugs me too. Seems that it thinks MSVC is active even though x86_64-windows defaults to the GNU ABI is what I thought...

@gerwin3
Copy link
Author

gerwin3 commented Apr 7, 2025

ziglang/zig#9536
seems to imply it should not be defined 🤔

@gerwin3
Copy link
Author

gerwin3 commented Apr 8, 2025

Okay I think what happened was:

namsint.h checks _I64_MAX to for the MSVC defines. Mingw libc does NOT define _MSC_VER but it does define _I64_MAX since it is expected on Windows. This leads nasmint.h to define a bunch of constants that will not work on non-MSVC. I fixed it by changing the line to #if defined(_MSC_VER) && _I64_MAX == 9223372036854775807 which only triggers for MSVC compilers.

It now compiles :)

@gerwin3 gerwin3 marked this pull request as ready for review April 8, 2025 12:00
@gerwin3 gerwin3 changed the title support for macOS and Windows (msvc) support for macOS and Windows Apr 8, 2025
@gerwin3
Copy link
Author

gerwin3 commented Apr 8, 2025

@andrewrk

  • This PR adds support for macOS and Windows (MSVC ABI not supported).
  • It also adds a GitHub CI file that builds on each platform.

One thing that I'm not so sure about is if maybe some of the compilation flags may affect NASM output. Since there are no test cases in this repo there is no way to know for sure if there are any regressions. Maybe we could run the test suite in CI. It seems though we'd need to check in the "golden" files which are required to run the test cases against. I'm not even sure how to get those for Windows (probably by grabbing the NASM official binary).

@gerwin3
Copy link
Author

gerwin3 commented Apr 8, 2025

Note that this PR also unlocks Windows and macOS support for the ffmpeg package :)

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