[ci] fix linux parallel build - #7320
Conversation
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
Removing recursive library build rules while other directories still rely on $(SERVERLIBS)/$(SERVERLIBS_MIN) without explicit .la prerequisites can leave parallel builds vulnerable to ordering races.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates BOINC’s CI and build-system linkage rules to better support parallel (make -j) builds on Linux by relying on Automake/Libtool dependency handling rather than manual recursive make rules.
Changes:
- Switch Linux CI build steps (and MinGW CI app build) to run
make -j $(nproc --all). - Remove manual recursive library build rules from
Makefile.incland update scheduler/server link dependencies to useSCHED_DEPS. - Add BOINC license headers / update copyright year in build-related files.
File summaries
| File | Description |
|---|---|
sched/Makefile.am |
Introduces SCHED_DEPS and replaces $(SERVERLIBS) usage for scheduler/server programs. |
mingw/ci_make_apps.sh |
Updates copyright year and switches to parallel make. |
Makefile.incl |
Removes recursive library build rules; keeps shared library path variables. |
lib/Makefile.am |
Adjusts test/tool link lines to reference in-tree .la targets directly. |
.github/workflows/linux.yml |
Switches build steps to parallel make. |
Review details
Suppressed comments (1)
Makefile.incl:70
-
SERVERLIBS/SERVERLIBS_MINare still referenced by other directories (e.g., tools/Makefile.am:69-87 and vda/Makefile.am:28-34). With the recursive build rules for$(LIBSCHED)/$ (LIBBOINC*) removed above, those targets no longer have an explicit prerequisite that forces the corresponding.lafiles to be built first, so parallelmake -jcan still race depending on how Automake expands LDADD. Consider updating the remaining Makefile.am files to link directly against the.latargets (or add per-program *_DEPENDENCIES) so ordering is unambiguous.
SERVERLIBS = $(LIBSCHED) $(LIBBOINC_CRYPT) $(LIBBOINC) $(MYSQL_LIBS) $(PTHREAD_LIBS) $(RSA_LIBS) $(SSL_LIBS)
SERVERLIBS_MIN = $(LIBSCHED) $(LIBBOINC_CRYPT) $(LIBBOINC) $(PTHREAD_LIBS) $(RSA_LIBS) $(SSL_LIBS)
APPLIBS = $(LIBAPI) $(LIBBOINC)
FUHLIBS = $(LIBBOINC_CRYPT) $(LIBBOINC) $(RSA_LIBS) $(SSL_LIBS)
FUHLIBS_FCGI = $(LIBBOINC_CRYPT) $(LIBBOINC_FCGI) -lfcgi $(RSA_LIBS) $(SSL_LIBS)
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # dependencies to make sure libs gets compiled before | ||
| # programs linking to them: |
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="sched/Makefile.am">
<violation number="1" location="sched/Makefile.am:24">
P2: This PR removes the cross-directory library build rules from Makefile.incl (the `$(LIBSCHED): cd ...; ${MAKE} libsched.la`-style rules), but `tools/Makefile.am`, `vda/Makefile.am`, and `apps/Makefile.am` still use `$(SERVERLIBS)`/`$(SERVERLIBS_MIN)`/`$(APPLIBS)`, which expand to `$(top_builddir)/sched/libsched.la`, `$(top_builddir)/lib/libboinc.la`, and `$(top_builddir)/api/libboinc_api.la`. Those expanded paths now have no rule to build them. A direct `make -C tools`/`make -C vda`/`make -C apps` in a freshly-configured tree (or any build where those directories run before `lib`/`sched`/`api`) fails with `No rule to make target '.../libsched.la'`. The migration should be completed by switching tools, vda, and apps to local `.la`/`SCHED_DEPS`-style references so they no longer depend on the removed rules.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| AM_CPPFLAGS += $(MYSQL_CFLAGS) $(PTHREAD_CFLAGS) | ||
| AM_LDFLAGS += -static | ||
|
|
||
| SCHED_DEPS = libsched.la $(LIBBOINC_CRYPT) $(LIBBOINC) $(MYSQL_LIBS) $(PTHREAD_LIBS) $(RSA_LIBS) $(SSL_LIBS) |
There was a problem hiding this comment.
P2: This PR removes the cross-directory library build rules from Makefile.incl (the $(LIBSCHED): cd ...; ${MAKE} libsched.la-style rules), but tools/Makefile.am, vda/Makefile.am, and apps/Makefile.am still use $(SERVERLIBS)/$(SERVERLIBS_MIN)/$(APPLIBS), which expand to $(top_builddir)/sched/libsched.la, $(top_builddir)/lib/libboinc.la, and $(top_builddir)/api/libboinc_api.la. Those expanded paths now have no rule to build them. A direct make -C tools/make -C vda/make -C apps in a freshly-configured tree (or any build where those directories run before lib/sched/api) fails with No rule to make target '.../libsched.la'. The migration should be completed by switching tools, vda, and apps to local .la/SCHED_DEPS-style references so they no longer depend on the removed rules.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At sched/Makefile.am, line 24:
<comment>This PR removes the cross-directory library build rules from Makefile.incl (the `$(LIBSCHED): cd ...; ${MAKE} libsched.la`-style rules), but `tools/Makefile.am`, `vda/Makefile.am`, and `apps/Makefile.am` still use `$(SERVERLIBS)`/`$(SERVERLIBS_MIN)`/`$(APPLIBS)`, which expand to `$(top_builddir)/sched/libsched.la`, `$(top_builddir)/lib/libboinc.la`, and `$(top_builddir)/api/libboinc_api.la`. Those expanded paths now have no rule to build them. A direct `make -C tools`/`make -C vda`/`make -C apps` in a freshly-configured tree (or any build where those directories run before `lib`/`sched`/`api`) fails with `No rule to make target '.../libsched.la'`. The migration should be completed by switching tools, vda, and apps to local `.la`/`SCHED_DEPS`-style references so they no longer depend on the removed rules.</comment>
<file context>
@@ -1,11 +1,28 @@
AM_CPPFLAGS += $(MYSQL_CFLAGS) $(PTHREAD_CFLAGS)
AM_LDFLAGS += -static
+SCHED_DEPS = libsched.la $(LIBBOINC_CRYPT) $(LIBBOINC) $(MYSQL_LIBS) $(PTHREAD_LIBS) $(RSA_LIBS) $(SSL_LIBS)
+
if ENABLE_LIBRARIES
</file context>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core build dependency wiring and CI build behavior, which is high-impact and should be validated by a human with end-to-end build/test results.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
sched/Makefile.am:25
SCHED_DEPSduplicates the library list already defined inSERVERLIBS(viaMakefile.incl), which can easily drift over time ifSERVERLIBSis updated. You can avoid duplication by overridingLIBSCHEDlocally and derivingSCHED_DEPSfrom$(SERVERLIBS).
Makefile.incl:58
- The comment about "dependencies" is now misleading because the explicit recursive-make dependency rules were removed; this section only defines library paths/LDADD bundles. Updating the comment will prevent future readers from assuming ordering is enforced here.
# dependencies to make sure libs gets compiled before
# programs linking to them:
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary by cubic
Fixes Linux CI builds in parallel by replacing recursive make rules with automake-managed dependencies.
Refactors
make -j $(nproc --all)instead of serialmake.Makefile.inclthat forced serial ordering.SCHED_DEPSinsched/Makefile.amto replaceSERVERLIBSand dropped the unusedSERVERLIBS_FCGI.Written for commit 8a2e2ab. Summary will update on new commits.