Skip to content
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

Makefile: Fix for {Net,Free,Open}BSD and generic $CFLAGS in environ #36

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

lanodan
Copy link
Contributor

@lanodan lanodan commented Jan 22, 2024

  • CFLAGS ?= inherits from the environment, so shouldn't have -std=c99
  • In practice everything but Darwin uses -shared
  • NetBSD has a broken CMSG_DATA macro when -D_POSIX_C_SOURCE=200809L is set
    their bug but probably better to make it work for now

@feld
Copy link
Contributor

feld commented Jan 22, 2024

This has been compile tested on:

FreeBSD 14
OpenBSD 7.4
NetBSD 9.3
MacOS 14 Sonoma
Debian 12.2 Bookworm

@feld
Copy link
Contributor

feld commented Jan 22, 2024

There may be a bug with Alpine Linux? Investigating...

==> exile
mkdir -p priv
cc -std=c99 -I/usr/local/lib/erlang/usr/include -shared -D_POSIX_C_SOURCE=200809L -Wall -Werror -Wextra -Wno-unused-parameter -pedantic -O2 -fPIC c_src/exile.c -o priv/exile.so
mkdir -p priv
cc -std=c99 -Wall -Werror -Wextra -Wno-unused-parameter -pedantic -O2 -fPIC c_src/spawner.c -o priv/spawner
c_src/spawner.c: In function ‘exec_process’:
c_src/spawner.c:115:25: error: ‘O_CLOEXEC’ undeclared (first use in this function); did you mean ‘FD_CLOEXEC’?
  115 |   if (set_flag(r_cmdin, O_CLOEXEC) < 0 || set_flag(w_cmdout, O_CLOEXEC) < 0 ||
      |                         ^~~~~~~~~
      |                         FD_CLOEXEC
c_src/spawner.c:115:25: note: each undeclared identifier is reported only once for each function it appears in
make: *** [Makefile:24: priv/spawner] Error 1
could not compile dependency :exile, "mix compile" failed. You can recompile this dependency with "mix deps.compile exile", update it with "mix deps.update exile" or clean it with "mix deps.clean exile"

@lanodan lanodan force-pushed the makefile-portability branch 2 times, most recently from f8d5812 to 486f069 Compare January 22, 2024 18:39
@lanodan
Copy link
Contributor Author

lanodan commented Jan 22, 2024 via email

@feld
Copy link
Contributor

feld commented Jan 22, 2024

The force pushed version appears to have fixed it

@feld
Copy link
Contributor

feld commented Jan 22, 2024

MacOS is breaking with the latest change as it doesn't like -D_POSIX_C_SOURCE=200809L which was previously not getting set for MacOS

@feld
Copy link
Contributor

feld commented Jan 22, 2024

MacOS is breaking with the latest change as it doesn't like -D_POSIX_C_SOURCE=200809L which was previously not getting set for MacOS

MacOS build now working

Copy link
Owner

@akash-akya akash-akya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the support and general Makefile cleanup!

Makefile Outdated Show resolved Hide resolved
- `CFLAGS ?=` inherits from the environment, so shouldn't have -std=c99
- In practice everything but Darwin uses `-shared`
- NetBSD and MacOS have a broken CMSG_DATA macro when -D_POSIX_C_SOURCE=200809L
is set their bug but probably better to make it work for now, specially in
MacOS case
@akash-akya akash-akya merged commit 87931cb into akash-akya:master Jan 24, 2024
5 checks passed
@lanodan lanodan deleted the makefile-portability branch January 24, 2024 07:57
@feld
Copy link
Contributor

feld commented Jan 30, 2024

bleh, I just did a clean build on my FreeBSD box and it's also blowing up with -D_POSIX_C_SOURCE=200809L. I guess I missed testing the final variant of this Makefile where that value was coming through to FreeBSD.

@akash-akya
Copy link
Owner

@feld so the Makefile still does not work for FreeBSD is it? If you have a fix, please share

@feld feld mentioned this pull request Feb 19, 2024
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