Bug 278533 - Mk/Uses/cmake.mk: use proper environment in do-test
Summary: Mk/Uses/cmake.mk: use proper environment in do-test
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Port Management Team
URL:
Keywords:
Depends on:
Blocks: 278500
  Show dependency treegraph
 
Reported: 2024-04-22 20:16 UTC by Matthias Andree
Modified: 2024-05-03 18:45 UTC (History)
7 users (show)

See Also:
tcberner: maintainer-feedback+


Attachments
git format-patch (to apply with git am) patch to fix cmake.mk's do-test: environment (1.28 KB, patch)
2024-04-22 20:16 UTC, Matthias Andree
mandree: maintainer-approval? (portmgr)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Andree freebsd_committer freebsd_triage 2024-04-22 20:16:59 UTC
Created attachment 250163 [details]
git format-patch (to apply with git am) patch to fix cmake.mk's do-test: environment

* the actual test run used MAKE_ENV but should use TEST_ENV
* also, ctest(1) defaults to running tests serially, so add
  CTEST_PARALLEL_LEVEL here and set it to _MAKE_JOBS_NUMBER.
  NOTE: cmake 3.29 changes semantics for _MAKE_JOBS_NUMBER empty or 0.

(Also note that Uses/cmake.mk advertises itself as supporting ctest... then it should deliver on that promise. Make it so.)

NOTE: please DO apply with git-am and then DO amend the commit message with this PR's number.
Comment 1 Gleb Popov freebsd_committer freebsd_triage 2024-04-22 20:40:00 UTC
The change makes sense to me.
Comment 2 Daniel Engberg freebsd_committer freebsd_triage 2024-04-22 20:44:01 UTC
I'm a bit concerned about enforcing multiple jobs by default as that doesn't seem to be common practice
Comment 3 Gleb Popov freebsd_committer freebsd_triage 2024-04-22 20:46:20 UTC
That's a valid point, but if I read this right, CMake changed the default in 3.29:

https://cmake.org/cmake/help/latest/manual/ctest.1.html#cmdoption-ctest-j
Comment 4 Matthias Andree freebsd_committer freebsd_triage 2024-04-22 21:14:23 UTC
Two things:
1. WITH_TESTING is opt-in (= disabled by default)
2. We shouldn't punish working ports, but let those broken ports sort their fall-out.
Comment 5 Tobias C. Berner freebsd_committer freebsd_triage 2024-05-03 07:14:04 UTC
lgtm
Comment 6 commit-hook freebsd_committer freebsd_triage 2024-05-03 18:45:47 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=cbee39bcd1a2472b8f3e24754f5bb3a3429b79a0

commit cbee39bcd1a2472b8f3e24754f5bb3a3429b79a0
Author:     Matthias Andree <mandree@FreeBSD.org>
AuthorDate: 2024-04-22 18:51:41 +0000
Commit:     Matthias Andree <mandree@FreeBSD.org>
CommitDate: 2024-05-03 18:44:39 +0000

    Mk/Uses/cmake.mk: use proper environment in do-test

    * the actual test run used MAKE_ENV but should use TEST_ENV
    * also, ctest(1) defaults to running tests serially, so add
      CTEST_PARALLEL_LEVEL here and set it to _MAKE_JOBS_NUMBER.
      NOTE: cmake 3.29 changes semantics for _MAKE_JOBS_NUMBER empty or 0.

    This was developed together with diizzy@ as a result of analysing
    performance regressions from PR 278500.

    PR:             278533
    Approved by:    portmgr@ (tcberner@)

 Mk/Uses/cmake.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)