D10776: Make it possible to generate po files in parallel
This revision was automatically updated to reflect the committed changes. Closed by commit R249:5a70e96795f1: Make it possible to generate po files in parallel (authored by apol). REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10776?vs=29024=29113 REVISION DETAIL https://phabricator.kde.org/D10776 AFFECTED FILES cmake/build-pofiles.cmake cmake/build-tsfiles.cmake To: apol, #frameworks, sitter Cc: michaelh
D10776: Make it possible to generate po files in parallel
sitter accepted this revision. This revision is now accepted and ready to land. REPOSITORY R249 KI18n BRANCH master REVISION DETAIL https://phabricator.kde.org/D10776 To: apol, #frameworks, sitter Cc: michaelh
D10776: Make it possible to generate po files in parallel
apol marked an inline comment as done. REPOSITORY R249 KI18n REVISION DETAIL https://phabricator.kde.org/D10776 To: apol, #frameworks, sitter Cc: michaelh
D10776: Make it possible to generate po files in parallel
apol updated this revision to Diff 29024. apol added a comment. Fix typo REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10776?vs=28846=29024 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10776 AFFECTED FILES cmake/build-pofiles.cmake cmake/build-tsfiles.cmake To: apol, #frameworks, sitter Cc: michaelh
D10776: Make it possible to generate po files in parallel
sitter added inline comments.
INLINE COMMENTS
> build-tsfiles.cmake:75
> ${pmapc_file}
> RESULT_VARIABLE code
> )
This doesn't belong here, does it? It's also defined in execute_process() and
from looking at the docs it would only be set once.
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D10776
To: apol, #frameworks, sitter
Cc: michaelh
D10776: Make it possible to generate po files in parallel
apol updated this revision to Diff 28846. apol added a comment. EQUAL REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10776?vs=28845=28846 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10776 AFFECTED FILES cmake/build-pofiles.cmake cmake/build-tsfiles.cmake To: apol, #frameworks, sitter Cc: michaelh
D10776: Make it possible to generate po files in parallel
apol updated this revision to Diff 28845. apol marked 3 inline comments as done. apol added a comment. Remove debug info and spaces REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10776?vs=28724=28845 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10776 AFFECTED FILES cmake/build-pofiles.cmake cmake/build-tsfiles.cmake To: apol, #frameworks, sitter Cc: michaelh
D10776: Make it possible to generate po files in parallel
sitter added a comment.
Ingenious code!
You left some debug clutter behind though.
Also, the if i statement is inconsistent between the two files
`if (i EQUAL ${numberOfProcesses})` vs. `if (i GREATER
${numberOfProcesses})`
INLINE COMMENTS
> build-pofiles.cmake:45
> +)
> +if(code)
> +message(FATAL_ERROR "failed generating ${PO_DIR}")
I think `!(code EQUAL 0)` may be more explicit here.
> build-pofiles.cmake:62
> +math(EXPR i "${i}+1")
> +message(STATUS "::: ${i}")
> +if (i EQUAL ${numberOfProcesses})
debug
> build-pofiles.cmake:63
> +message(STATUS "::: ${i}")
> +if (i EQUAL ${numberOfProcesses})
> +message(STATUS "holt")
no space after if.
> build-pofiles.cmake:64
> +if (i EQUAL ${numberOfProcesses})
> +message(STATUS "holt")
> +_processCommands()
debug
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D10776
To: apol, #frameworks, sitter
Cc: michaelh
D10776: Make it possible to generate po files in parallel
apol updated this revision to Diff 28724. apol added a comment. Make sure we don't run more than nproc processes at the same time REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10776?vs=27871=28724 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10776 AFFECTED FILES cmake/build-pofiles.cmake cmake/build-tsfiles.cmake To: apol, #frameworks, sitter Cc: michaelh
D10776: Make it possible to generate po files in parallel
sitter added a comment. As discussed on telegram: the proposed change makes it a different problem. Now instead of running serial all the time, it'd fork bomb all the time (peaks at 3671 forks for plasma-desktop 5.12 when run on a fairly fast system with SSD). Also, missing adjustment to build-tsfiles.cmake ;) REPOSITORY R249 KI18n REVISION DETAIL https://phabricator.kde.org/D10776 To: apol, #frameworks, sitter Cc: michaelh
D10776: Make it possible to generate po files in parallel
apol created this revision. apol added reviewers: Frameworks, sitter. Restricted Application added a project: Frameworks. apol requested review of this revision. REVISION SUMMARY Instead of going file by file, feed them to cmake at once. Should improve build times on some systems. TEST PLAN tests still pass, got kalgebra translated REPOSITORY R249 KI18n BRANCH master REVISION DETAIL https://phabricator.kde.org/D10776 AFFECTED FILES cmake/build-pofiles.cmake To: apol, #frameworks, sitter Cc: michaelh
