Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 559064 - cmake-utils.eclass (was sys-devel/llvm): ninja build ignores MAKEOPTS="--jobs=x --load-average=x"
Summary: cmake-utils.eclass (was sys-devel/llvm): ninja build ignores MAKEOPTS="--jobs...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Eclasses (show other bugs)
Hardware: All Linux
: Normal normal with 1 vote (vote)
Assignee: Gentoo KDE team
URL: https://github.com/gentoo/gentoo/pull...
Whiteboard:
Keywords: PATCH
: 583144 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-08-29 03:37 UTC by xpue
Modified: 2022-05-10 19:42 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description xpue 2015-08-29 03:37:16 UTC
Causing significant system load.

Reproducible: Always
Comment 1 Bernard Cafarelli gentoo-dev 2015-09-01 21:11:34 UTC
Thanks for the report, on which version do you have this problem?

Versions up to 3.6 use autoconf build so should work OK

Upcoming 3.7 and live ebuild use cmake/ninja build, is is the version you tested?
Comment 2 xpue 2015-09-02 03:24:13 UTC
Yes, this happened with -3.7.0_rc* and -9999.
Comment 3 Bernard Cafarelli gentoo-dev 2015-09-02 09:44:54 UTC
cmake-utils eclass only supports -j, -l and -k in MAKEOPTS with ninja(added in bug #490280), as these are the only options ninja itself understands (from "ninja --help")

The handbook and the wiki are not explicit about which options are supported in MAKEOPTS, I guess we could add a parameter conversion in the eclass (to convert to short names). @mgorny, what do you think?
Comment 4 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2015-09-02 18:46:55 UTC
This is something with which cmake-utils.eclass should be concerned.
Comment 5 Michael Palimaka (kensington) gentoo-dev 2015-09-03 08:51:01 UTC
I don't think it's a problem to add such a conversion, but I don't think any of the eclass maintainers use ninja to develop/test such a patch.
Comment 6 . 2016-04-04 19:49:37 UTC
Instead of the currently used, buggy and incompliant function. I present you here today this fully compliant masterpiece.

Not all options were implemented though, only those which make sense.

```
_ninjaopts_from_makeopts() {
    [[ ${NINJAOPTS+set} == set ]] && \
        return 0
    NINJAOPTS=
    local jobs= load= keep=
    set -- $MAKEOPTS
    while (( $# )); do
        case $1 in
            -j|--jobs)
                if [ -n "$2" ] && [[ "$2" =~ ^[0-9]+$ ]] ;then
                    jobs=$2
                    shift
                else
                    jobs=99
                fi
                ;;
            -l|--load-average)
                if [ -n "$2" ] && [[ "$2" =~ ^[0-9]+\.?[0-9]*$ ]] ;then
                    load=${2%%.*}
                    shift
                else
                    load=
                fi
                ;;
            -j*|-l*)
                local arg=${1:2}
                [[ ${1:1:1} == j ]] && jobs=$arg || load=$arg
                ;;
            --jobs=*|--load-average=*)
                local arg=${1##*=}
                [[ ${1:2:1} == j ]] && jobs=$arg || load=$arg
                ;;
            -k|--keep-going)
                keep=99
                ;;
        esac
        shift
    done
    [ -n "${jobs}" ] && NINJAOPTS+=" -j ${jobs}"
    [ -n "${keep}" ] && NINJAOPTS+=" -k ${keep}"
    [ -n "${load}" ] && NINJAOPTS+=" -l ${load}"
    export NINJAOPTS
}
```

I tested it with
```
test(){
    local expect="$1"
    shift
    printf "Test    : %s\n" "$*"
    printf "Expected:  %s\n" "$expect"
    printf "Got     : "
    (
        MAKEOPTS="$*" _ninjaopts_from_makeopts
        echo "${NINJAOPTS}"
    )
    echo '----------------------------------'
}

test '-l 1' -l1
test '-l 3' -l 3
test '-j 1' -j1
test '-j 3' -j 3
test '-j 5' --jobs=5
test '-j 5' --jobs 5
test '-l 4' --load-average=4
test '-l 4' --load-average 4
test '-j 2 -k 99' -k -j 2
test '-j 18 -k 99' -k -l -j -j 99 -k -j18 -l17 -l -k
test '-j 99 -k 99' -k -l -j -j 99 -k -j18 -l17 -l -k -j
test '-j 18 -k 99 -l 1' -k -l -j -j 99 -k -j18 -l17 -l -k -l1
test '-j 18 -k 99 -l 1' -k -l -j -j 99 -k -j18 -l17 -l -k -l 1
test '-j 99' -j 99.9948
```
Comment 7 . 2016-04-04 19:58:02 UTC
and to also handle
```
test '-l 14' -l14.9948
test '-l 14' --load-average=14.9948
```

change
```
load=$arg
```

to
```
load=${arg%%.*}
```
Comment 8 Michael Palimaka (kensington) gentoo-dev 2016-04-06 13:31:22 UTC
(In reply to Jan Chren (rindeal) from comment #6)
> Instead of the currently used, buggy and incompliant function. I present you
> here today this fully compliant masterpiece.
> 
> Not all options were implemented though, only those which make sense.

Thanks a lot for you work. For an eclass change of this type, the best way to get it merged is first to send the patch to the gentoo-dev mailing list for review.
Comment 9 . 2016-04-10 16:47:30 UTC
Submitted a PR https://github.com/gentoo/gentoo/pull/1228
Comment 10 Coacher 2016-05-18 11:05:29 UTC
*** Bug 583144 has been marked as a duplicate of this bug. ***
Comment 11 . 2016-07-13 12:31:41 UTC
Could someone review the PR? - https://github.com/gentoo/gentoo/pull/1481
Comment 12 Toralf Förster gentoo-dev 2016-09-01 12:15:45 UTC
Happens at my tinderbox too - and I do not use ninja there.
Comment 13 Chí-Thanh Christopher Nguyễn gentoo-dev 2016-09-01 12:38:54 UTC
This problem is preventing dev-qt/qtwebengine build on my Raspberry Pi 3, because it will fill up swap and the system will grind to a halt. Setting MAKEOPTS="-j1" had no effect, ninja would always build with 4 jobs.
Comment 14 Chí-Thanh Christopher Nguyễn gentoo-dev 2016-09-01 13:04:12 UTC
Sorry, the previous comment was not related to cmake-utils.eclass, I reported bug 592660 about it now.
Comment 15 Michael Palimaka (kensington) gentoo-dev 2017-06-03 16:05:33 UTC
This is fixed since the move from custom handling to ninja-utils:

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=b15a8079a781e1d9c6a469cbb784613cf209022b