Page MenuHomeFreeBSD

Add SRCS_SUBDIRS to allow Makefiles to put objects in subdirectories
ClosedPublic

Authored by dim on Feb 28 2015, 1:50 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 27, 8:25 AM
Unknown Object (File)
Sat, Apr 27, 8:24 AM
Unknown Object (File)
Sat, Apr 27, 4:30 AM
Unknown Object (File)
Mon, Apr 22, 8:24 AM
Unknown Object (File)
Dec 31 2023, 9:43 AM
Unknown Object (File)
Dec 31 2023, 9:43 AM
Unknown Object (File)
Dec 20 2023, 2:27 AM
Unknown Object (File)
Dec 14 2023, 1:03 PM
Subscribers

Details

Summary

We have some software in the base system that includes more than one file with the same name (in different directories). In some cases we wish to build these into a single library or program, which does not work with our current build infrastructure.

This change allows the Makefile to set EXTRA_OBJDIRS to a list of subdirectories, and those paths can be included in SRCS.

EXTRA_OBJDIRS=foo bar

SRCS=foo/file1.c \
        bar/file1.c

Putting in review for comments for now.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

emaste retitled this revision from to Add EXTRA_OBJDIRS to allow Makefiles to put objects in subdirectories.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added reviewers: dim, imp, brooks.
share/mk/bsd.obj.mk
12 ↗(On Diff #4024)

Need a description for EXTRA_OBJDIRS

91 ↗(On Diff #4024)

Extra ${CANONICALOBJDIR} here, and ${.CURDIR}${dir} is not correct - should be something like ${.CURDIR}${dir:S/^${CANONICALOBJDIR}//}

I understand the motivation, but this change makes me nervous. How will linkers find these moved files?

In D1984#5, @imp wrote:

I understand the motivation, but this change makes me nervous. How will linkers find these moved files?

Linking is not a problem because the transformation for ${OBJS} leaves the subdir intact: OBJS+= ${SRCS:N*.h:R:S/$/.o/g}

The link ends up looking like:

cc -O2 -pipe   -std=gnu99 -fstack-protector   -Qunused-arguments  -o test foo/file.o bar/file.o

There is one caveat, the double-suffix rules in sys.mk do not include the -o argument:

.c.o:
        ${CC} ${CFLAGS} -c ${.IMPSRC}
        ${CTFCONVERT_CMD}

so the object file ends up in ${CANONICALOBJDIR}, not the subdirectory. I did not notice this before because bsd.lib.mk provides its own double-suffix rules that do include -o:

.cc.o .C.o .cpp.o .cxx.o:
        ${CXX} ${STATIC_CXXFLAGS} ${CXXFLAGS} -c ${.IMPSRC} -o ${.TARGET}

and the case I've been testing here is working on building a single libllvm

With sys.mk updated to specify -o and this example Makefile:

PROG=test
SRCS=foo/file.c bar/file.c
MAN=
.include <bsd.prog.mk>

the output looks like:

joule% make    
cc -O2 -pipe   -std=gnu99 -fstack-protector   -Qunused-arguments -c foo/file.c -o foo/file.o
cc -O2 -pipe   -std=gnu99 -fstack-protector   -Qunused-arguments -c bar/file.c -o bar/file.o
cc -O2 -pipe   -std=gnu99 -fstack-protector   -Qunused-arguments  -o test foo/file.o bar/file.o

I will upload a new diff later on with sys.mk changes incorporated.

share/mk/bsd.obj.mk
84 ↗(On Diff #4024)

Also, after some thought SRCS_SUBDIRS may be a better name than EXTRA_OBJDIRS, to make it more clear to the application or library Makefile writer.

emaste retitled this revision from Add EXTRA_OBJDIRS to allow Makefiles to put objects in subdirectories to Add SRCS_SUBDIRS to allow Makefiles to put objects in subdirectories.
emaste edited edge metadata.
  • Rename variable to SRCS_SUBDIRS
  • Add comment in bsd.obj.mk
  • Correct message showing created directory name
  • Add -o ${TARGET} to double-suffix rules in sys.mk to fix use by other than bsd.lib.mk consumers

How does this work with 'reach over' sources?
If the file doesn't start with / or ../, then can't you guess the proper subdirs already? Why add an extra variable?

In D1984#11, @imp wrote:

How does this work with 'reach over' sources?

I assume you mean with multiple settings of .PATH? It should work fine, although is not a use case that's motivating this.

If the file doesn't start with / or ../, then can't you guess the proper subdirs already? Why add an extra variable?

Yeah, it wasn't immediately obvious to me how that could be done in make syntax, but it turns out it's not too bad with something like ${SRCS:M*/*:C/(^.*)\/[^\/]*$/\1/}

In D1984#12, @emaste wrote:
In D1984#11, @imp wrote:

How does this work with 'reach over' sources?

I assume you mean with multiple settings of .PATH? It should work fine, although is not a use case that's motivating this.

If the file doesn't start with / or ../, then can't you guess the proper subdirs already? Why add an extra variable?

Yeah, it wasn't immediately obvious to me how that could be done in make syntax, but it turns out it's not too bad with something like ${SRCS:M*/*:C/(^.*)\/[^\/]*$/\1/}

If you aren't worried about / or ../ starting src files, it's simpler to say:
${SRCS:H:O:u}
to get a what you were calling SRC_SUBDIRS. :H to strip off the foo.c part (everything but the last component), :O to sort and :u to throw away duplicates.

I'm not convinced that an explicit SRCS_SUBDIRS adds value. In CheriBSD we're just building it internally with an expression like imp suggests. See:

https://github.com/CTSRD-CHERI/cheribsd/commit/c29c584dbde5597cb6af232cdcfa192138ba534e
https://github.com/CTSRD-CHERI/cheribsd/commit/50c751149412beac7c03069530c5f7c6c98d8f6c

The one caution with this change is that you must run "make obj" in the directory (or use buildworld et al to do it for you) or all sorts of weird things happen. With projects/bmake we're moving toward mandatory objdirs so I think this is fine, but there may be some confusion.

Oh, I didn't realize this already existed in cheribsd.

In D1984#15, @emaste wrote:

Oh, I didn't realize this already existed in cheribsd.

I'd only made the change to create the object dirs. The change to explicitly use -o ${.TARGET} was done as a local makefile hack rather than doing in sys.mk where it belongs.

dim edited reviewers, added: emaste; removed: dim.
dim edited edge metadata.

So, let's try this then. It seems to work fine for me...

share/mk/bsd.obj.mk
53–54 ↗(On Diff #4187)

Now that the use of this is simplified I wonder if we shouldn't just change the .for loop below: .for dir in ${SRCS:H:O:u} (with an appropriate comment)

brooks edited edge metadata.

Either this version of Ed's proposed variant seems good to me.

This revision is now accepted and ready to land.Mar 12 2015, 2:36 PM
dim edited edge metadata.

Eliminate unnecessary EXTRAOBJDIRS variable.

This revision now requires review to proceed.Mar 12 2015, 8:01 PM
imp edited edge metadata.

Assuming this has gone through make universe, I'm cool with it now.

This revision is now accepted and ready to land.Mar 12 2015, 8:06 PM
dim updated this revision to Diff 4222.

Closed by commit rS279980 (authored by @dim).