zstd icon indicating copy to clipboard operation
zstd copied to clipboard

Expose definable definitions to make in Makefile.

Open tansy opened this issue 3 months ago • 7 comments

Expose definable definitions to make in Makefile.

There are some definitions* in Makefile that seem to be exposed to external definition but there is no way to pass them to Makefile. Can you add some variable like `USER_CFLAGS' so user could pass additional options to Makefile and not have to manually change command line or edit the file. And describe example in the Readme.

Or, if it's already possible describe it in Readme.

  • like ZSTDCLI_CLEVEL_DEFAULT, ZSTDCLI_NBTHREADS_DEFAULT, ZSTD_NOBENCH, etc.

Also a request to enclose DISPLAY_LEVEL_DEFAULT in `#ifdef ... #endif', so it too could be passed through makefile.

+#ifndef DISPLAY_LEVEL_DEFAULT
 #define DISPLAY_LEVEL_DEFAULT 2
+#endif

Here's a diff for convenience. Just add a new line at the end.

tansy avatar Sep 22 '25 14:09 tansy

These are not Makefile variables, but C Preprocessor variables.

The way to pass such variables to the compiler via make is presumed standard, and looks like this:

CPPFLAGS="-DZSTDCLI_CLEVEL_DEFAULT=19 -DZSTDCLI_NBTHREADS_DEFAULT=16" make

It would be possible to add a paragraph in programs/README.md to explain that process and list preprocessor variables.

Cyan4973 avatar Sep 22 '25 18:09 Cyan4973

These are not Makefile variables, but C Preprocessor variables.

I know but that's not what I meant. Let me explain it - if a user passes CFLAGS to make it will replace the existing one.

$ echo "int main(){ return 0; }" > prog.c
$ cat << EOF > Makefile
CFLAGS += -O2 -Wall -Iinclude -DDEFINE1
LD=$(CC)

.c.o:
        $(CC) $(CFLAGS) -c -o $@ $<

prog: prog.o
        $(LD) -o $@ $+
EOF

$ make CFLAGS='-DDISPLAY_LEVEL_DEFAULT=1' -n
cc -DDISPLAY_LEVEL_DEFAULT=1 -c -o prog.o prog.c
cc -o prog prog.o

What I would expect to see is

$ make CFLAGS='-DDISPLAY_LEVEL_DEFAULT=1' -n
cc -O2 -Wall -Iinclude -DDEFINE1 -DDISPLAY_LEVEL_DEFAULT=1 -c -o prog.o prog.c
cc -o prog prog.o

Which I don't see doable with CFLAGS, only with separate variable. You can alway say that I can provide full CFLAGS generated by make but that's not reasonable option - how am I suppose to know what is full CFLAGS, not to mention it likely will change depending on target, system, whatever is detected...?

Just checked how it looks like with with zstd and, sure enough, it's just like that.

$ make zstd -n
CFLAGS=-O3 -Wall -Wextra -Wcast-qual -Wcast-align -Wshadow -Wstrict-aliasing=1 -Wswitch-enum -Wdeclaration-after-statement -Wstrict-prototypes -Wundef -Wpointer-arith -Wvla -Wformat=2 -Winit-self -Wfloat-equal -Wwrite-strings -Wredundant-decls -Wmissing-prototypes -Wc++-compat  -Wa,--noexecstack

# pass CFLAGS to the make:

$ make CFLAGS='-DDISPLAY_LEVEL_DEFAULT=1' zstd -n
CFLAGS=-DDISPLAY_LEVEL_DEFAULT=1

tansy avatar Sep 23 '25 02:09 tansy

  1. You should not use CFLAGS to pass preprocessor variables, use CPPFLAGS instead
  2. If you pass a variable after make, it indeed overrides any internal setting, which is probably not what is wanted in this instance. Pass it before the make as an environment variable. Like in the suggested example.

Cyan4973 avatar Sep 23 '25 02:09 Cyan4973

Didn't know that, but it worked.

$ CFLAGS='-DDISPLAY_LEVEL_DEFAULT=1' make
cc -DDISPLAY_LEVEL_DEFAULT=1 -O2 -Wall -Iinclude -DDEFINE1 -c -o prog.o prog.c
cc -o prog prog.o

It didn't go so well with zstd:

$ CFLAGS='-DDISPLAY_LEVEL_DEFAULT=1' make zstd -n
echo CFLAGS=-DDISPLAY_LEVEL_DEFAULT=1 -Wall -Wextra -Wcast-qual -Wcast-align -Wshadow -Wstrict-aliasing=1 -Wswitch-enum -Wdeclaration-after-statement -Wstrict-prototypes -Wundef -Wpointer-arith -Wvla -Wformat=2 -Winit-self -Wfloat-equal -Wwrite-strings -Wredundant-decls -Wmissing-prototypes -Wc++-compat  -Wa,--noexecstack

It lost `-O3' somehow.

tansy avatar Sep 23 '25 03:09 tansy

After passing said define through environment I get redefinition warning.

$ CFLAGS='-O3 -DDISPLAY_LEVEL_DEFAULT=1' make

fileio.c: In function ‘FIO_compressFilename’:
fileio.c:2150: warning: "DISPLAY_LEVEL_DEFAULT" redefined
 #define DISPLAY_LEVEL_DEFAULT 2

Which is surprising - how does it work then?

#ifndef DISPLAY_LEVEL_DEFAULT
#define DISPLAY_LEVEL_DEFAULT 2
#endif

use CPPFLAGS instead

Does CPP here stand for C Pre Processor?

tansy avatar Sep 23 '25 17:09 tansy

I don't think DISPLAY_LEVEL_DEFAULT was designed to be changed at compilation time. For this capability to exist, it must be prepared.

Cyan4973 avatar Sep 23 '25 20:09 Cyan4973

Reason to change it is simple - I want it to work like other *nix compressors, without flooding console with running statistics. If I want them, and sometimes I will, then I type `-v'. Typing `-q' every single time, specially in scripts is tiresome. And it would be better to change it during compilation then edit source file every time you build a program.

tansy avatar Sep 24 '25 20:09 tansy