Skip to content

Apply compile flags consistently#22

Merged
nrnrnr merged 3 commits intonrnrnr:masterfrom
turboencabulator:flags
Jun 27, 2023
Merged

Apply compile flags consistently#22
nrnrnr merged 3 commits intonrnrnr:masterfrom
turboencabulator:flags

Conversation

@turboencabulator
Copy link
Contributor

Another piece of #10:

  • Update compile flags: Add $(LDFLAGS) when linking, add a missing $(CFLAGS) to contrib/norman/numarkup, move flags from $(CC) to $(CFLAGS).
  • Having -Werror in the default $(CFLAGS) causes the fakepretty build to fail. Fix up those warnings/errors, and sort a few parts of the Makefile so that fakepretty (and nwmktemp) is on par with the other targets in src/c.

@nrnrnr
Copy link
Owner

nrnrnr commented Jun 22, 2022

Should CFLAGS and LDFLAGS be mentioned in src/INSTALL?

Sort some of the Makefile contents.  Add $(NWMKTEMPOBJS) for
consistency, and add nwmktemp to the clean rules.

Remove gitversion.o from $(FILES) and $(SRCS).  Both were likely
meant to contain gitversion.c, not .o.  The existence in $(FILES)
generates errors when building doc.dvi.  The existence in $(SRCS)
creates an undesired empty gitversion.c/.o during `make boot`; add
gitversion.c as a special file to clobber instead.
Since all three may contain spaces, quote them when passing to a
sub-make.

Remove ICONC, ICONT, and BIN from the contrib Makefiles since no leaf
Makefile currently makes use of them, and installation step 5 does not
mention them.
@turboencabulator
Copy link
Contributor Author

I've added a mention of CFLAGS and a couple other variables to the installation steps (latest commit), rebased on your latest commit, and fixed a couple issues with gitversion.c/.o.

@nrnrnr
Copy link
Owner

nrnrnr commented Jun 26, 2022

Thanks! I blew my noweb budget yesterday, but I'll hope to get back to this one soon.

@nrnrnr
Copy link
Owner

nrnrnr commented Jun 27, 2023

In the following line,

	(cd $(LIBSRC) && $(MAKE) "ICONT=$(ICONT)" "ICONC=$(ICONC)" LIB=$(LIB) BIN=$(BIN) install)

why are the Icon assignments quoted but the LIB and BIN assignments not quoted?

@nrnrnr nrnrnr merged commit fdff6b8 into nrnrnr:master Jun 27, 2023
@turboencabulator
Copy link
Contributor Author

In the following line,

	(cd $(LIBSRC) && $(MAKE) "ICONT=$(ICONT)" "ICONC=$(ICONC)" LIB=$(LIB) BIN=$(BIN) install)

why are the Icon assignments quoted but the LIB and BIN assignments not quoted?

I think it was primarily for attaching flags to the Icon commands (the ICONC=iconc -f l example in src/INSTALL) rather than dealing with paths containing spaces. It probably wouldn't hurt to quote everything everywhere to handle paths with spaces, but that does make everything ugly.

@turboencabulator turboencabulator deleted the flags branch June 28, 2023 07:05
@nrnrnr
Copy link
Owner

nrnrnr commented Jun 28, 2023

OK, let's leave it as is, then.

dbosk pushed a commit to dbosk/noweb that referenced this pull request Mar 6, 2024
dbosk pushed a commit to dbosk/noweb that referenced this pull request Mar 6, 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