seeing-things / zwo

ZWO SDK and custom software for debugging and using it.
23 stars 4 forks source link

Improve `capture` build system #69

Closed bgottula closed 1 year ago

bgottula commented 1 year ago

The hand crafted Makefile-based build system for the capture program is a bit of a mess. It either needs to be refreshed, or more likely it should just be replaced with CMake.

Some specific things that ought to be addressed to the extent feasible:

jgottula commented 1 year ago

I can assist with this:

bgottula commented 1 year ago

Some thoughts from @jgottula via SMS:

I will be assuming that we’re using the distro-installed versions of both fmt and spdlog. (In other words: no fmt or spdlog headers or source files directly in-tree.)

You can do some preprocessor defs/undefs in the Makefile to override spdlog’s tweakme.h:

  • SPDLOG_FMT_EXTERNAL: define this so it will include the system’s standard fmt headers in /usr/include/ rather than the internal copy of fmtlib that spdlog includes
  • SPDLOG_USE_STD_FORMAT: undef this because std::format is pretty lame

Also check out the INSTALL file and the pkgconfig file:

  • SPDLOG_COMPILED_LIB: define this so all the in-header inline definitions of things that are already contained in the library file will be skipped over
  • SPDLOG_SHARED_LIB: define this since presumably you’re linking against a dynamic .so shared object (rather than a static .a library or an in-tree .CPP file

And then for fmt, I think all you need is just to make sure that FMT_HEADER_ONLY isn’t defined; AFAIK that’s entirely an explicit opt-in, so this probably doesn’t require undef’ing anything


Basically all of this stuff gets done in the capture program Makefile. And actually even then a lot of it is still automatic because it’ll be coming from pkg-config.

Also, unfortunately... if you dump the spdlog pkg-config file, which I think is at /usr/lib/x86_64-/pkgconfig/spdlog.pc, you'll notice that it actually does not have all the CPP defs it should have. Which is a problem.

bgottula commented 1 year ago

From @jgottula, continued:

Other things to go on the makefile list:

  • It shouldn't exist at all and should be cmake instead

  • It currently does a non-parallel dumbass build in one GCC invocation

  • It should do MAKEFLAGS += -j to ensure that it does stuff in parallel even if your make invocation is bad

  • The way the flags are named and organized is complete shit and I got halfway to reorganizing them

  • OpenCV preprocessor flags and linker flags should come from pkg-config, but we weren't doing that and instead hacked them in manually, because we didn't have the .pc file, because it's only provided in the massive libopencv-dev "metapackage"

  • Should have preprocessor flags and linker flags for both fmt and spdlog, acquired from pkg-config calls

  • God why did I use names like CC and CFLAGS when this is clearly a C++ makefile and should instead be using CXX (compiler), CPPFLAGS (preprocessor flags), CXXFLAGS (compiler flags)

  • Also lol why is the libusb-1.0 shit not using pkg-config either ugh

  • Instead of doing compile of a bunch of translation units and link all in a single call, there should be two rules: a %.o: %.cpp rule for compile and a second rule for link

  • The clean rule would then also need to delete .o files

  • Rule dependencies are a mess and have to be done carefully. The link step can unconditionally invoke make for zwo_fixer; that's fine, no dependency needed. All compile and link steps should (but currently don't) have a dependency on the Makefile. The compile rule should have additional dependencies on all the header files (including, ideally, the zwo_fixer header and possibly other external headers). The link rule should have dependencies on all the object files, but not the header files.

  • Should be using $(wildcard ...) more rather than naked *.whatever stuff

bgottula commented 1 year ago

Okay I've taken a stab at replacing makefiles with CMake for capture and zwo_fixer on the cmake branch. So far so good.

@jgottula interested in your review. I didn't carry over the compiler flags you had in the makefile for zwo_fixer since I'm not sure which are critical vs nice to have. (It seems to build and run fine without most of them).