justinethier / cyclone

:cyclone: A brand-new compiler that allows practical application development using R7RS Scheme. We provide modern features and a stable system capable of generating fast native binaries.
http://justinethier.github.io/cyclone/
MIT License
823 stars 42 forks source link

Allow prepending include/library search path through -COPT/-CLNK #482

Closed nmeum closed 2 years ago

nmeum commented 2 years ago

This commit separates include/library search directory options from "normal" compiler/linker options and places options passed via the -COPT/-CLNK command-line flags in-between. This allows overwriting the default search paths, since contrary to all other options, the search paths must be prepend for an -I/-L option to take precedence over an existing one.

To test this, compare the new concatenation of linker/compiler options for test.scm with the previous one:

$ ./cyclone -d -A . -COPT '-Iinclude' -CLNK '-L.' /tmp/test.scm
gcc /tmp/test.c  -O2 -Wall -Iinclude -I/usr/include -c -o /tmp/test.o
gcc /tmp/test.o  /usr/lib/cyclone/scheme/cyclone/common.o  /usr/lib/cyclone/scheme/base.o  /usr/lib/cyclone/scheme/write.o  -pthread -lcyclone -lck -lm -lcyclonebn -ldl  -O2 -fPIC -Wall -Wno-shift-negative-value -Wno-unused-command-line-argument -Wl,--export-dynamic -L. -L/usr/lib -o /tmp/test

The important thing to take away here is:

  1. -Iinclude (as specified via -COPT) is added before the default -I/usr/include but after -O2 -Wall etc.
  2. -L. (as specified via -CLNK is added before the default -L/usr/lib but after -Wl,--export-dynamic etc.

This should (hopefully) make it entirely unnecessary to ever build Cyclone twice in order to have all changes in the current source tree take effect. Without this change, Cyclone will always use system header/libraries of the previous installation since the library/include search path was previously appended instead of prepended (see #476).

Please review this with extra care.

Fixes #476

justinethier commented 2 years ago

Thanks for the contributions @nmeum! This will take some time to review and test...

nmeum commented 2 years ago

Thanks for the contributions @nmeum! This will take some time to review and test...

Sure, take your time :) See also: My inline comments above regarding stuff I wasn't entirely sure about.

justinethier commented 2 years ago

Merged - thanks @nmeum !