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 fixups (Cygwin vs. Windows vs. Linux) #37

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

Conversation

koitsu
Copy link
Contributor

@koitsu koitsu commented Jan 22, 2023

The logic here can be confusing so I'll explain it with a chart:

Environment OS OSTYPE MSYSTEM Delete
Cygwin (Windows) Windows_NT cygwin rm -f
MSYS2 (Windows Windows_NT MINGW64 CLANG64 etc. rm -f
Windows native Windows_NT del /Q /F
FreeBSD freebsdXX.Y rm -f
Linux linux-gnu rm -f

The BSDs require GNU make (gmake) installed to use any of this (i.e. BSD make is not compatible with any of this, but that's expected).

Regarding $CC:

Assuming gcc isn't exactly safe everywhere nowadays. FreeBSD since 11.x has used clang instead of gcc (there is no gcc command), while "cc" is is a symlink to "clang", so it works fine. Thus, just rely on what GNU make defines $CC as by default; else we can override it elsewhere.

Most important:

AJ, I don't know what your development environment is like, so please test this before merging! I can't stress this enough.

I will have some more PRs coming soon that will modify further pieces of this, such as setting $CFLAGS to a very long list of warnings (and treating warnings as errors), a large PR to fix all those warnings (there are hundreds), as well as using -O2 -s to get smaller binaries and hopefully faster execution.

- Leverage $CC, as it's set by GNU make: https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html
- Detect Cygwin vs. native Windows vs. Linux better (and without shell)
- Deal with rm vs. del (native Windows) better

The logic here can be confusing so I'll explain it with a chart:

| Environment       | OS         | OSTYPE      |
| ----------------- | ---------- | ----------- |
| Cygwin on Windows | Windows_NT | cygwin      |
| Windows cmd.exe   | Windows_NT | not set     |
| FreeBSD           | not set    | freebsdXX.Y |
| Linux             | not set    | linux-gnu   |

MSYS2/MSYS/MinGW (on Windows) may add complications to this logic; I
haven't tested them, but hope they set $OSTYPE.

And the BSDs require GNU make (gmake) installed to use any of this (i.e.
BSD make is not compatible with any of this, but that's expected).

Regarding $CC:

Assuming gcc isn't exactly safe everywhere nowadays.  FreeBSD since 11.x
has used clang instead of gcc (there is no gcc command), while "cc" is
is a symlink to "clang", so it works fine.  Thus, just rely on what
GNU make defines $CC as by default; else we can override it elsewhere.
@freem freem self-assigned this May 20, 2023
@koitsu
Copy link
Contributor Author

koitsu commented May 30, 2023

I should add that the Cygwin and MSYS2 logic is correct, but might want BINARY=asm6f.exe since both environments support standard Windows binaries ending in .exe. Issue #23 indirectly points this out as well.

The OS detection portion, however, does work. (And on Linux/FreeBSD/etc. we definitely do not want binaries named asm6f.exe)

In short: to implement this correctly, I need to know a bit about your development environment. Otherwise if you want to rework this + implement it yourself using your own approach, go right ahead. :)

Either way, just do me 2 favours:

  1. Leave CC unset so that people can do things like CC=gcc make or CC=clang make to test both compilers. I do this, for example. The default value in GNU make is CC=cc. This method also works fine on FreeBSD (where gmake is required anyway).
  2. Please do use CFLAGS as demonstrated, as I will have an upcoming PR that greatly expands these flags to catch all sorts of problems (hence most of the PRs I've been submitting as of late).

koitsu added 4 commits August 22, 2023 19:54
Now that PRs 46 through 56 were merged, we can finally leverage a
good set of compiler warning flags.  These can be used with Clang
as well as GCC.

Additionally, we add -O2 to get optimisation.

And while here, add a note about how to build with Clang.
@koitsu
Copy link
Contributor Author

koitsu commented Aug 23, 2023

@freem This is now in a very good state. I have personally tested building on:

  • MSYS2 MINGW64 (gcc)
  • MSYS2 CLANG64 (Clang)
  • MSYS2 UCRT64 (gcc and Clang)
  • FreeBSD 11.x (gcc)
  • Linux (Ubuntu 22.04) (gcc)

I cannot test on "native" Windows for various reasons, but as long as it's GCC or Clang on native Windows it should (ideally) work. Obviously use of MSVC compiler will fail, but that would be a much bigger undertaking and not many people use Makefiles for such things anyway.

Please give this a try on your own dev environment and merge. Would love to get this finally closed out :)

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.

2 participants