Page MenuHomeFreeBSD
Differential D10748

Make ninja opt-out in cmake.mk.
ClosedPublic
Actions

Authored by tcberner on May 15 2017, 7:47 PM.
Tags
None
Referenced Files
F163655300: D10748.id29802.diff
Sat, Jul 25, 5:32 AM
F163633338: D10748.id29196.diff
Sat, Jul 25, 12:58 AM
F163604645: D10748.id29813.diff
Fri, Jul 24, 8:49 PM
Unknown Object (File)
Fri, Jul 24, 12:24 PM
Unknown Object (File)
Wed, Jul 22, 6:31 PM
Unknown Object (File)
Wed, Jul 22, 4:56 PM
Unknown Object (File)
Wed, Jul 22, 4:40 PM
Unknown Object (File)
Wed, Jul 22, 3:17 PM
View All Files
Subscribers
adamw
antoine
lifanov

Details

Reviewers
rakuco
mat
kwm
Group Reviewers
kde
O5: Ports Framework(Owns No Changed Paths)
portmgr
Commits
rP444324: Make ninja opt-out in cmake.mk
Summary

As kwm@ reported a quite significant build-speedup in wekbit2-gtk3 when using ninja instead of gmake, I'm interested to know whether using the ninja-generator of cmake could lead to at least some amount of speedup in other ports as well -- as cmake is used by many ports, this could be significant for the package builders.

At the moment cmake.mk has an opt-in to use ninja. Make this an opt-out.

Test Plan
  • Exp-run to see what breaks?
  • Pick some ports and compare build times?

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 9659
Build 10099: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Owners added a reviewer: O5: Ports Framework.May 15 2017, 7:47 PM
Herald added a reviewer: portmgr. · View Herald TranscriptMay 15 2017, 7:47 PM
Harbormaster completed remote builds in B9284: Diff 28375.May 15 2017, 7:48 PM
antoine added a subscriber: antoine.May 15 2017, 7:50 PM

Yes this breaks some ports.

tcberner added a comment.May 15 2017, 7:52 PM
In D10748#222537, @antoine wrote:

Yes this breaks some ports.

How many? ^^

tcberner updated this revision to Diff 28376.May 15 2017, 8:09 PM

As there are apparently some ports breaking with ninja, make disabling ninja an argument to cmake.mk noninja instead of a variable. This way the broken ports can be fixed cleanly by just setting USES=cmake:<otherargs>,noninja.

I will test-build the ports depending on ninja... to see what breaks.

Harbormaster completed remote builds in B9285: Diff 28376.May 15 2017, 8:09 PM
tcberner updated this revision to Diff 28380.May 15 2017, 8:43 PM
  • Add upstream fix for cmake/ninja to graphics/ilmbase.
Harbormaster completed remote builds in B9287: Diff 28380.May 15 2017, 8:43 PM
tcberner updated this revision to Diff 28381.May 15 2017, 9:03 PM

Don't use ninja for databases/mysql56-server:

ninja: error: dependency cycle: sql/GenServerSource -> sql/CMakeFiles/GenServerSource -> sql/sql_builtin.cc -> cmake_order_depends_target_sql -> sql/GenDigestServerSource -> sql/CMakeFiles/GenDigestServerSource -> sql/lex_token.h -> sql/gen_lex_token -> sql/GenServerSource
lifanov added a subscriber: lifanov.EditedMay 16 2017, 5:03 PM

Maybe we could call noninja just "make"?

USES=cmake:outsource,make
# make                use make instead of ninja
adamw added a subscriber: adamw.May 16 2017, 5:07 PM
In D10748#222843, @lifanov wrote:

Maybe we could call noninja just "make"?

USES=cmake:outsource,make
# make                use make instead of ninja

cmake runs cmake, not make. noninja is clearer.

tcberner updated this revision to Diff 28424.May 16 2017, 5:27 PM

Mark some more ports to not use ninja -- still ~500 more ports to go.

tcberner updated this revision to Diff 28480.May 17 2017, 7:31 PM

Ok, I think I should have gotten more-or-less all...

5-6% of the affected ports needed noninja.

tcberner added a comment.EditedMay 29 2017, 8:52 AM

Ping portmgr -- what's the opinion on this?

tcberner updated this revision to Diff 28968.May 29 2017, 9:07 AM

Regenearte against current ports-tree for exp-run.

tcberner updated this revision to Diff 29022.May 30 2017, 4:38 PM

Fix

  • mysql57-server
  • mysql80-server
  • gemrb
  • kde-workspace
tcberner updated this revision to Diff 29026.May 30 2017, 4:45 PM

Fix

  • devel/flang
kwm edited edge metadata.May 31 2017, 8:38 AM

I just realized that this change doesn't have a ports/CHANGES yet, could you prepare one?

Like describe this change and how people can opt-out if ninja isn't supported.

rakuco edited edge metadata.Jun 1 2017, 3:14 PM

Sorry for chiming in so late.

I think this diff also obsoletes https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=213331 in the sense that we'd no longer have USES=cmake together with USES=gmake (only USES=cmake:noninja gmake, which is fine).

To complement what @kwm said, this patch is lacking documentation: right now, I don't think it's clear to other ports contributors when to add noninja at all. I tested a handful of the ports that need noninja here on Poudriere and they seem to fall into two categories:

  • Broken CMake files that have broken dependency rules that confuse ninja.
  • Ports setting BUILD_WRKSRC to build only part of the source code. Since build.ninja is generated in the top-level work directory, calling ninja from a subdirectory fails.

I wonder if it makes sense to try to fix the second case automatically in Uses/cmake.mk.

tcberner updated this revision to Diff 29110.Jun 1 2017, 4:44 PM
  • Fix x11-themes/kde4-style-bespin
  • add CHANGES as requested by kwm@
tcberner added a comment.Jun 1 2017, 4:45 PM

I wonder if it makes sense to try to fix the second case automatically in Uses/cmake.mk.

Sure, it's worth a try

tcberner updated this revision to Diff 29196.Jun 4 2017, 8:20 PM

I could not consitently reproduce the build failures of

  • devel/doxygen
  • math/stp

on i386. So make them not use ninja (for now).

Further drop the USES=gmake from

  • graphics/osg-devel

which was added for FreeBSD 10.x

Additionally, mark ports ignored that set USES=cmake gmake.

Harbormaster completed remote builds in B9659: Diff 29196.Jun 4 2017, 8:20 PM
rakuco added a comment.Jun 6 2017, 11:12 AM

I've added some inline comments about ports that do not need noninja. In addition to what you added to CHANGES, I'd expect some documentation in Uses/cmake.mk as well, otherwise it's not much better than not adding anything at all. Specifically, there needs to be some documentation explaining that noninja is a stop-gap measure that's normally used by:

  • meta-ports (or ports that change BUILD_WRKSRC)
  • Ports with broken CMake rules (ninja refuses to work with circular dependencies, multiple rules that generate the same files or custom targets that do not quote $ symbols properly).
  • Ports that call CMake themselves as part of the build, as we end up with something that uses make and ninja at different points.
comms/uhd/Makefile
29

I've fixed this in rP442624 so you can drop the noninja here.

databases/clickhouse/Makefile
37

It worked fine here without noninja, is it really needed?

databases/mariadb100-server/Makefile
32

This one also worked fine without noninja here.

databases/soci/Makefile
16

This too. It's broken in -HEAD with or without the change.

devel/bullet/Makefile
19

You can drop noninja if you pass -DINSTALL_LIBS:BOOL=ON to CMake.

devel/compiler-rt/Makefile
22

This one works if you stop setting MAKE_ARGS.

devel/rapidjson/Makefile
26

This one works if you drop MAKE_ARGS (which is superfluous anyway).

devel/xxhash/Makefile
18

I think this works without noninja if you import https://github.com/Cyan4973/xxHash/commit/5ab73ee82245b586f69762411edc7824d778ee2a

games/gemrb/Makefile
27

Fixed in rP442686.

games/spring/Makefile
35

Fixed in rP442691.

graphics/cuneiform/Makefile
18

This port is just broken on HEAD, it builds fine with ninja on 10.3-i386.

graphics/opencv/Makefile
17

The port built fine here without it.

graphics/partio/Makefile
13

Fixed in rP442692.

lang/sagittarius-scheme/Makefile
23

Fixed in rP442750.

net/libproxy/Makefile
16

This port built fine without noninja here on HEAD.

www/wt/Makefile
25

This port built fine on HEAD without noninja here.

mat edited edge metadata.Jun 9 2017, 1:00 PM

I have trouble figuring out if there are more ports using cmake that should use ninja that ports that should not use ninja. And so, I am wondering if it should not be a USES=cmake:ninja to enable it.

Mk/Uses/cmake.mk
104

remove empty line.

rakuco added a comment.Jun 9 2017, 1:04 PM
In D10748#230050, @mat wrote:

I have trouble figuring out if there are more ports using cmake that should use ninja that ports that should not use ninja. And so, I am wondering if it should not be a USES=cmake:ninja to enable it.

Using freshports.org/devel/cmake as reference and filtering out ports that no longer exist, there are 1317 ports that have a build-time dependency on CMake, ~70 of which don't work with ninja as far as I can see.

mat added a comment.Jun 9 2017, 1:20 PM

Mmmm, ok, good for me :-)

Next question, do we really a need a USES=ninja, so, are there ports that use USES=ninja but not USES=cmake (and that could not use USES=cmake) ?

antoine added a comment.Jun 9 2017, 3:51 PM
In D10748#230070, @mat wrote:

Mmmm, ok, good for me :-)

Next question, do we really a need a USES=ninja, so, are there ports that use USES=ninja but not USES=cmake (and that could not use USES=cmake) ?

Ports using meson for instance.

tcberner updated this revision to Diff 29437.Jun 10 2017, 6:18 PM

Revert setting noninja for the ports mentioned by @rakuco.

tcberner updated this revision to Diff 29438.Jun 10 2017, 7:19 PM

Make the patch to devel/xxhash apply.

Harbormaster completed remote builds in B9787: Diff 29438.Jun 10 2017, 7:19 PM
tcberner updated this revision to Diff 29439.Jun 10 2017, 7:24 PM

Drop USES=gmake from comms/uhd lang/sagittarius-scheme, now that they don't set noninja, they need to have gmake dropped.

Harbormaster completed remote builds in B9788: Diff 29439.Jun 10 2017, 7:24 PM
tcberner updated this revision to Diff 29490.Jun 12 2017, 2:55 PM
tcberner marked 5 inline comments as done.
  • Don't use ninja, if the BUILD_WRKSRC or INSTALL_WRKSRC is different from CONFIGURE_WRKSRC
  • Fix devel/bullet by adding the mentioned CMAKE_ARGS.
Harbormaster completed remote builds in B9817: Diff 29490.Jun 12 2017, 2:55 PM
tcberner updated this revision to Diff 29492.Jun 12 2017, 3:55 PM
  • Add a bit more comment to the noninja option.
  • Remove some noninja's that are now handled by the _WRKSRC work around
tcberner updated this revision to Diff 29702.Jun 16 2017, 5:32 AM

Readd noninja to databases/mariadb100-server

tcberner updated this revision to Diff 29719.Jun 16 2017, 2:35 PM

Remove ninja from the remaining evolution ports.

Harbormaster completed remote builds in B9935: Diff 29719.Jun 16 2017, 2:35 PM
tcberner updated this revision to Diff 29801.Jun 18 2017, 9:06 PM
  • Remove CMAKE_NINJA from graphics/rawtherapee
  • Add CMAKE_NINJA to SANITY_DEPRECATED in bsd.sanity.mk
tcberner updated this revision to Diff 29802.Jun 18 2017, 9:23 PM

Minor cleanup: remove ninja from ports having USES=cmake.

Harbormaster completed remote builds in B9975: Diff 29802.Jun 18 2017, 9:24 PM
mat added inline comments.Jun 19 2017, 1:18 PM
Mk/bsd.sanity.mk
180 ↗(On Diff #29802)

I think it should go to SANITY_NOTNEEDED instead.

tcberner updated this revision to Diff 29813.Jun 19 2017, 3:29 PM

Put CMAKE_NINJA into SANITY_NOTNEEDED instead.

tcberner added a comment.Jun 25 2017, 7:01 AM

Ping portmgr and kde -- is this good to go now?

rakuco accepted this revision.Jun 25 2017, 11:11 AM

lgtm from my side, thanks for working on this. In your commit message, make sure you mention why some changes were done (e.g. why MAKE_ARGS was removed from compiler-rt).

Closed by commit rP444324: Make ninja opt-out in cmake.mk (authored by tcberner). · Explain WhyJun 25 2017, 9:08 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
14 lines
Mk/
Uses/
16 lines
astro/
marble/
2 lines
audio/
csound6/
2 lines
cad/
kicad/
2 lines
opencascade/
2 lines
openvsp/
2 lines
chinese/
fcitx/
2 lines
ibus-chewing/
2 lines
comms/
uhd/
2 lines
usrp/
2 lines
databases/
clickhouse/
2 lines
mariadb100-server/
2 lines
mariadb55-server/
2 lines
mysql55-server/
2 lines
mysql56-server/
2 lines
mysql57-server/
2 lines
mysql80-server/
2 lines
mysqlwsrep56-server/
2 lines
percona55-server/
2 lines
percona56-server/
2 lines
percona57-server/
2 lines
soci/
2 lines
tarantool/
2 lines
xtrabackup/
2 lines
deskutils/
kdeconnect/
2 lines
kdepim4/
2 lines
owncloudclient/
2 lines
devel/
bullet/
2 lines
compiler-rt/
2 lines
doxygen/
2 lines
flang/
2 lines
kapptemplate/
2 lines
kdevelop-kde4/
2 lines
kdevelop-php/
2 lines
libzrtpcpp/
2 lines
mongo-c-driver/
2 lines
rapidjson/
2 lines
xxhash/
2 lines
editors/
codelite/
2 lines
kate-plugin-pate/
2 lines
games/
gemrb/
2 lines
hedgewars/
2 lines
openclonk/
2 lines
palomino/
2 lines
solarus/
2 lines
spring/
2 lines
graphics/
cuneiform/
2 lines
digikam-kde4/
2 lines
ilmbase/
files/
59 lines
inkscape/
2 lines
luxrender/
2 lines
mitsuba/
1 line
opencv/
2 lines
opensubdiv/
2 lines
osg-devel/
6 lines
osgearth/
2 lines
partio/
2 lines
xcftools/
2 lines
irc/
weechat/
2 lines
lang/
beignet/
2 lines
io/
2 lines
sagittarius-scheme/
2 lines
mail/
libcmime/
2 lines
libvmime/
2 lines
spmfilter-clamav/
2 lines
spmfilter/
2 lines
math/
metis/
2 lines
parmetis/
2 lines
stp/
2 lines
multimedia/
avidemux/
2 lines
gstreamer-qt4/
2 lines
phonon-designerplugin/
2 lines
qt5-phonon4-designerplugin/
2 lines
zoneminder-h264/
2 lines
zoneminder/
2 lines
net-im/
licq/
2 lines
ring-gnome/
2 lines
ring-libclient/
2 lines
net-p2p/
eiskaltdcpp-daemon/
2 lines
eiskaltdcpp-qt/
2 lines
net/
ceph-devel/
2 lines
libproxy/
2 lines
tigervnc/
2 lines
science/
paraview/
2 lines
security/
kwalletmanager/
2 lines
libzrtpcppcore/
2 lines
sysutils/
baloo/
2 lines
fluent-bit/
2 lines
kcm-polkit-kde/
2 lines
osquery/
2 lines
textproc/
zxing-cpp/
2 lines
www/
h2o/
2 lines
wt/
2 lines
x11-themes/
kde4-style-bespin/
2 lines
kde4-windeco-crystal/
2 lines
qtcurve/
2 lines
x11-toolkits/
mygui/
2 lines
x11/
kde4-runtime/
2 lines
kde4-workspace/
2 lines
libkonq/
2 lines
virtualgl/
2 lines

Diff 29196

View Options

CHANGES

Loading...
View Options

Mk/Uses/cmake.mk

Loading...
View Options

astro/marble/Makefile

Loading...
View Options

audio/csound6/Makefile

Loading...
View Options

cad/kicad/Makefile

Loading...
View Options

cad/opencascade/Makefile

Loading...
View Options

cad/openvsp/Makefile

Loading...
View Options

chinese/fcitx/Makefile

Loading...
View Options

chinese/ibus-chewing/Makefile

Loading...
View Options

comms/uhd/Makefile

Loading...
View Options

comms/usrp/Makefile

Loading...
View Options

databases/clickhouse/Makefile

Loading...
View Options

databases/mariadb100-server/Makefile

Loading...
View Options

databases/mariadb55-server/Makefile

Loading...
View Options

databases/mysql55-server/Makefile

Loading...
View Options

databases/mysql56-server/Makefile

Loading...
View Options

databases/mysql57-server/Makefile

Loading...
View Options

databases/mysql80-server/Makefile

Loading...
View Options

databases/mysqlwsrep56-server/Makefile

Loading...
View Options

databases/percona55-server/Makefile

Loading...
View Options

databases/percona56-server/Makefile

Loading...
View Options

databases/percona57-server/Makefile

Loading...
View Options

databases/soci/Makefile

Loading...
View Options

databases/tarantool/Makefile

Loading...
View Options

databases/xtrabackup/Makefile

Loading...
View Options

deskutils/kdeconnect/Makefile

Loading...
View Options

deskutils/kdepim4/Makefile

Loading...
View Options

deskutils/owncloudclient/Makefile

Loading...
View Options

devel/bullet/Makefile

Loading...
View Options

devel/compiler-rt/Makefile

Loading...
View Options

devel/doxygen/Makefile

Loading...
View Options

devel/flang/Makefile

Loading...
View Options

devel/kapptemplate/Makefile

Loading...
View Options

devel/kdevelop-kde4/Makefile

Loading...
View Options

devel/kdevelop-php/Makefile

Loading...
View Options

devel/libzrtpcpp/Makefile

Loading...
View Options

devel/mongo-c-driver/Makefile

Loading...
View Options

devel/rapidjson/Makefile

Loading...
View Options

devel/xxhash/Makefile

Loading...
View Options

editors/codelite/Makefile

Loading...
View Options

editors/kate-plugin-pate/Makefile

Loading...
View Options

games/gemrb/Makefile

Loading...
View Options

games/hedgewars/Makefile

Loading...
View Options

games/openclonk/Makefile

Loading...
View Options

games/palomino/Makefile

Loading...
View Options

games/solarus/Makefile

Loading...
View Options

games/spring/Makefile

Loading...
View Options

graphics/cuneiform/Makefile

Loading...
View Options

graphics/digikam-kde4/Makefile.common

Loading...
View Options

graphics/ilmbase/files/patch-git_8eed7012

Loading...
View Options

graphics/inkscape/Makefile

Loading...
View Options

graphics/luxrender/Makefile

Loading...
View Options

graphics/mitsuba/Makefile

Loading...
View Options

graphics/opencv/Makefile

Loading...
View Options

graphics/opensubdiv/Makefile

Loading...
View Options

graphics/osg-devel/Makefile

Loading...
View Options

graphics/osgearth/Makefile

Loading...
View Options

graphics/partio/Makefile

Loading...
View Options

graphics/xcftools/Makefile

Loading...
View Options

irc/weechat/Makefile

Loading...
View Options

lang/beignet/Makefile

Loading...
View Options

lang/io/Makefile

Loading...
View Options

lang/sagittarius-scheme/Makefile

Loading...
View Options

mail/libcmime/Makefile

Loading...
View Options

mail/libvmime/Makefile

Loading...
View Options

mail/spmfilter-clamav/Makefile

Loading...
View Options

mail/spmfilter/Makefile

Loading...
View Options

math/metis/Makefile

Loading...
View Options

math/parmetis/Makefile

Loading...
View Options

math/stp/Makefile

Loading...
View Options

multimedia/avidemux/Makefile.common

Loading...
View Options

multimedia/gstreamer-qt4/Makefile

Loading...
View Options

multimedia/phonon-designerplugin/Makefile

Loading...
View Options

multimedia/qt5-phonon4-designerplugin/Makefile

Loading...
View Options

multimedia/zoneminder-h264/Makefile

Loading...
View Options

multimedia/zoneminder/Makefile

Loading...
View Options

net-im/licq/Makefile

Loading...
View Options

net-im/ring-gnome/Makefile

Loading...
View Options

net-im/ring-libclient/Makefile

Loading...
View Options

net-p2p/eiskaltdcpp-daemon/Makefile

Loading...
View Options

net-p2p/eiskaltdcpp-qt/Makefile

Loading...
View Options

net/ceph-devel/Makefile

Loading...
View Options

net/libproxy/Makefile

Loading...
View Options

net/tigervnc/Makefile

Loading...
View Options

science/paraview/Makefile

Loading...
View Options

security/kwalletmanager/Makefile

Loading...
View Options

security/libzrtpcppcore/Makefile

Loading...
View Options

sysutils/baloo/Makefile

Loading...
View Options

sysutils/fluent-bit/Makefile

Loading...
View Options

sysutils/kcm-polkit-kde/Makefile

Loading...
View Options

sysutils/osquery/Makefile

Loading...
View Options

textproc/zxing-cpp/Makefile

Loading...
View Options

www/h2o/Makefile

Loading...
View Options

www/wt/Makefile

Loading...
View Options

x11-themes/kde4-style-bespin/Makefile

Loading...
View Options

x11-themes/kde4-windeco-crystal/Makefile

Loading...
View Options

x11-themes/qtcurve/Makefile

Loading...
View Options

x11-toolkits/mygui/Makefile

Loading...
View Options

x11/kde4-runtime/Makefile

Loading...
View Options

x11/kde4-workspace/Makefile

Loading...
View Options

x11/libkonq/Makefile

Loading...
View Options

x11/virtualgl/Makefile

Loading...