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

Configure options #89

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

Configure options #89

wants to merge 5 commits into from

Conversation

jwt27
Copy link

@jwt27 jwt27 commented Jul 15, 2023

Work in progress, pushing what I have so far:

First commit saves the user's $CFLAGS in the makefile, which I think is generally a good idea. Though there are some targets that produce multiple makefiles, not sure how practical it is for those.

Second commit then allows users to override the config.h macros via $CFLAGS. For example, to make a "release" version of the library for djgpp:

$ export CFLAGS='-O3 -DNDEBUG -DWANT_DEBUG=0 -DWANT_BSD_FATAL=0'
$ ./configur.sh djgpp
$ make -f djgpp.mak

This is just the easiest way to implement it, but maybe not the nicest to use. I'm looking into adding autoconf-style options like --enable-dhcp, and a --help that explains all of them.

@jwt27
Copy link
Author

jwt27 commented Jul 17, 2023

I'm looking into adding autoconf-style options

command.com may be a bit too simplistic for this :)

@jwt27
Copy link
Author

jwt27 commented Jul 18, 2023

OK, this should work. CI will complain about missing mkconf.exe for now. But let me know what you think of the idea.

The example above now becomes:

$ export CFLAGS='-O3 -DNDEBUG'
$ ./configur.sh --disable-debug --disable-bsd-fatal djgpp
$ make -f djgpp.mak

Note that for configur.bat, the --enable-x options have to be listed before the target names, and target "all" will ignore all options. The .sh script doesn't have these limitations.

util/mkconf.c Outdated Show resolved Hide resolved
util/mkconf.c Outdated Show resolved Hide resolved
/*
* Include user options set from configure script.
*/
#include "userconf.h" /* @NO_DEP */
Copy link
Owner

@gvanem gvanem Jul 18, 2023

Choose a reason for hiding this comment

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

So e.g Watcom's compiler on Windows should find this as build/watcom/userconf.h.
I don't see where in Makefile.all the I_CFLAGS = -I./build/watcom is.
Besides what if Watcom LARGE and Watcom WIN32 needs different userconf.h settings?

Copy link
Author

@jwt27 jwt27 Jul 18, 2023

Choose a reason for hiding this comment

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

It's in regular CFLAGS (line 586), together with -I. -I../inc.

Besides what if Watcom LARGE and Watcom WIN32 needs different userconf.h settings?

Yes, that is an issue. Those would need to be separate configure targets then. For now, the only way to do that is to configure once, build large, then reconfigure with different options, then build win32.

Copy link
Owner

Choose a reason for hiding this comment

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

It's in regular CFLAGS (line 586), together with -I. -I../inc.

But from the last CI build for Watcom/Win32:

wcc386 @build\watcom\win32\wcc.arg accept.c -fo=build\watcom\win32\accept.obj
config.h(193): Error! E1055: Unable to open 'userconf.h'

So some issue with the -I path.

Copy link
Author

Choose a reason for hiding this comment

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

Ahh, I had the path swapped (watcom/build instead of build/watcom). Fixed now.

Copy link
Owner

Choose a reason for hiding this comment

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

And also in src\test\makefile.all since the CI says:

wcc386 @wcc386.arg
..\config.h(193): Error! E1055: Unable to open 'userconf.h'
make: *** [watcom_f.mak:175: bind.exe] Error 1

etc.

I should fix appveyor-build.bat to error out in this case. This could work (for Watcom at least):

--- a/appveyor-script.bat
+++ b/appveyor-script.bat
@@ -403,9 +403,9 @@ exit /b 0
   if %BUILDER%. == cygwin.   make -f Cygwin_%BITS%.mak
   if %BUILDER%. == visualc.  make -f visualc_%BITS%.mak
   if %BUILDER%. == watcom. (
-     if %MODEL%. == large. make -f watcom_l.mak
-     if %MODEL%. == flat.  make -f watcom_f.mak
-     if %MODEL%. == win32. make -f watcom_w.mak
+     if %MODEL%. == large. (make -f watcom_l.mak & if not errorlevel == 0 goto test_failed)
+     if %MODEL%. == flat.  (make -f watcom_f.mak & if not errorlevel == 0 goto test_failed)
+     if %MODEL%. == win32. (make -f watcom_w.mak & if not errorlevel == 0 goto test_failed)
   )

   %_ECHO% "\e[1;33mRunning test 'cpu.exe' ---------------------------------------------------------------\e[0m"
@@ -417,6 +417,9 @@ exit /b 0
   %_ECHO% "\e[1;33mRunning test 'chksum.exe -s' ---------------------------------------------------------\e[0m"
   chksum.exe -s

+  exit /b 0
+
+:test_failed
+  exit /b 1
+
 :no_tests
   exit /b 0

Copy link
Author

Choose a reason for hiding this comment

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

And also in src\test\makefile.all since the CI says:

wcc386 @wcc386.arg
..\config.h(193): Error! E1055: Unable to open 'userconf.h'
make: *** [watcom_f.mak:175: bind.exe] Error 1

etc.

Should be fixed now.

util/mkconf.c Outdated Show resolved Hide resolved
util/mkmake.c Outdated Show resolved Hide resolved
@jwt27 jwt27 force-pushed the config branch 3 times, most recently from 0cb987d to df7dc8b Compare July 18, 2023 19:02
@@ -25,36 +25,50 @@ if not exist %WATT_ROOT%\src\makefile.all goto not_set
::
set MKMAKE=..\util\mkmake.exe
set MKDEP=..\util\mkdep.exe
set MKCONF=..\util\mkconf.exe
Copy link
Owner

Choose a reason for hiding this comment

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

I assume you mean that util/Makefile should build these as needed?
For now, just add the to this PR if possible.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, will add the binaries. It's a bit unfortunate that they have to be in the git repo. I suppose that's mainly due to the S-Lang dependency (which not everyone might have)?

Note, I did change the utils CFLAGS to -Os -g0, so they should waste a bit less space. Maybe it's an idea to upx them too?

Copy link
Owner

Choose a reason for hiding this comment

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

I suppose that's mainly due to the S-Lang dependency (which not everyone might have)?

Right. But mkconf.c doesn't need S-Lang AFAICS. And UPX is IMHO not needed.

Copy link
Author

Choose a reason for hiding this comment

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

So, there's not much of a reason to keep them in git right? I did just add the mkconf binaries though.

Problem is that binaries inflate the repo size (git clone must download all previous versions). And Git doesn't preserve file dates so Make can't tell if they need to be rebuilt.


::
:: Use these programs under all versions of Windows.
::
set MKMAKE=..\util\win32\mkmake.exe
set MKDEP=..\util\win32\mkdep.exe
set MKCONF=..\util\win32\mkconf.exe
Copy link
Owner

Choose a reason for hiding this comment

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

Ditto.

@jwt27 jwt27 marked this pull request as ready for review July 19, 2023 19:59
@jwt27 jwt27 mentioned this pull request Apr 15, 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.

2 participants