search

InterNetNews / inn

INN (InterNetNews) Usenet server
https://www.isc.org/othersoftware/#INN
Other
68 stars 13 forks source link

Build concurrency fixes #208

Closed ewxrjk closed 2 years ago

ewxrjk commented 2 years ago

We serialize build of top-level subdirectories. The previous attempt to support make -j was broken, since the subdirectories have dependencies between them and attempt to cross-invoke one another, leading to multiple concurrent make invocations on the same directory.

A couple of missing dependencies are also added. The sql-init.h dependency should be generated by makedepend, but it's confused about object code extensions. This change is a stopgap until that is addressed.

ewxrjk commented 2 years ago

(This is about #206 .)

pmetzger commented 2 years ago

Looks like this should work reasonably for the moment.

rathann commented 2 years ago

This seems to work for me on Fedora as well. I pushed the patch to Fedora development branch (rawhide). I'll monitor if the parallel make failure is still reproducible on our CI service (koschei). Thanks!

Julien-Elie commented 2 years ago

Worth including in our 2.6 branch (without the ovsqlite part only in 2.7/main), thanks Richard! Russ is working on an Automake build for main.

I see the use of "make -C" though. I have to test first if it works fine on non-GNU Make in an AIX and Solaris I have access to.

ewxrjk commented 2 years ago

The online man pages say they don't support make -C.

But is it really worth supporting such obsolete versions of make? When we still supported AIX/Solaris we always used GNU Make. (Who even runs a news server on AIX anyway?)

Julien-Elie commented 2 years ago

The build still works fine with the stock make in AIX 7.1 (I have just tested again). We could unconditionally require GNU make though in our documentation. INSTALL only mentions the use of GNU make if the include syntax does not work.

My remark was only in the sense that someone successfully building INN 2.6.4 with his make may encounter an error with INN 2.6.5 with the same make. He will have to use gmake.

Incidentally, maybe it will be necessary with Automake in INN 2.7.0. I do not know whether its output will work with these non-GNU make.

rra commented 2 years ago

Automake generally tries to support non-GNU make. One can introduce non-portable constructs if one isn't paying attention, but all the machinery generated by Automake itself should work with POSIX make.

pmetzger commented 2 years ago

I have a strong dogma that it is not necessary to produce a perfect fix all at once. Any fix that makes things strictly better than they were before (like this one) is a win.

Julien-Elie commented 2 years ago

I've not said that the patch will not be merged. I just still haven't had found a bit of time to tackle it, as well as other things at the same time. It will be present in the next release.

I will just change $(MAKE) -C directory to something like cd directory && $(MAKE) || exit 1 ; cd ..

Easy fix, notably not to introduce a build regression with non-GNU make.