* [PATCH] cmake.bbclass: use `cmake --build` to build & install
@ 2017-05-01 19:42 Cody P Schafer
2017-05-01 22:26 ` Andre McCurdy
0 siblings, 1 reply; 6+ messages in thread
From: Cody P Schafer @ 2017-05-01 19:42 UTC (permalink / raw)
To: openembedded-core
Rather than presuming `make` is the generator, use cmake's generic
`cmake --build` feature (which knows to call the appropriate generator).
Both DESTDIR and VERBOSE still behave as intended when used as
environment variables instead of make variable-arguments.
This makes it more straight forward for others to select other cmake
generators (many folks have been reaching for `ninja` lately).
Signed-off-by: Cody P Schafer <dev@codyps.com>
---
meta/classes/cmake.bbclass | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
index 12df617ad8..05ec50d385 100644
--- a/meta/classes/cmake.bbclass
+++ b/meta/classes/cmake.bbclass
@@ -135,13 +135,11 @@ cmake_do_configure() {
do_compile[progress] = "percent"
cmake_do_compile() {
- cd ${B}
- base_do_compile VERBOSE=1
+ VERBOSE=1 cmake --build '${B}'
}
cmake_do_install() {
- cd ${B}
- oe_runmake 'DESTDIR=${D}' install
+ DESTDIR='${D}' cmake --build '${B}' --target install
}
EXPORT_FUNCTIONS do_configure do_compile do_install do_generate_toolchain_file
--
2.12.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] cmake.bbclass: use `cmake --build` to build & install
2017-05-01 19:42 [PATCH] cmake.bbclass: use `cmake --build` to build & install Cody P Schafer
@ 2017-05-01 22:26 ` Andre McCurdy
2017-05-02 15:49 ` Cody P Schafer
0 siblings, 1 reply; 6+ messages in thread
From: Andre McCurdy @ 2017-05-01 22:26 UTC (permalink / raw)
To: Cody P Schafer; +Cc: OE Core mailing list
On Mon, May 1, 2017 at 12:42 PM, Cody P Schafer <dev@codyps.com> wrote:
> Rather than presuming `make` is the generator, use cmake's generic
> `cmake --build` feature (which knows to call the appropriate generator).
>
> Both DESTDIR and VERBOSE still behave as intended when used as
> environment variables instead of make variable-arguments.
>
> This makes it more straight forward for others to select other cmake
> generators (many folks have been reaching for `ninja` lately).
>
> Signed-off-by: Cody P Schafer <dev@codyps.com>
> ---
> meta/classes/cmake.bbclass | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
> index 12df617ad8..05ec50d385 100644
> --- a/meta/classes/cmake.bbclass
> +++ b/meta/classes/cmake.bbclass
> @@ -135,13 +135,11 @@ cmake_do_configure() {
>
> do_compile[progress] = "percent"
> cmake_do_compile() {
> - cd ${B}
Removing the redundant "cd ${B}" should perhaps be a separate commit?
> - base_do_compile VERBOSE=1
> + VERBOSE=1 cmake --build '${B}'
This looks like it will cause parallel make and EXTRA_OEMAKE options
to be ignored?
> }
>
> cmake_do_install() {
> - cd ${B}
> - oe_runmake 'DESTDIR=${D}' install
> + DESTDIR='${D}' cmake --build '${B}' --target install
> }
>
> EXPORT_FUNCTIONS do_configure do_compile do_install do_generate_toolchain_file
> --
> 2.12.2
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cmake.bbclass: use `cmake --build` to build & install
2017-05-01 22:26 ` Andre McCurdy
@ 2017-05-02 15:49 ` Cody P Schafer
2017-05-02 16:49 ` Andre McCurdy
0 siblings, 1 reply; 6+ messages in thread
From: Cody P Schafer @ 2017-05-02 15:49 UTC (permalink / raw)
To: Andre McCurdy; +Cc: OE Core mailing list
>> do_compile[progress] = "percent"
>> cmake_do_compile() {
>> - cd ${B}
>
> Removing the redundant "cd ${B}" should perhaps be a separate commit?
I'm not really seeing that as useful, change right now can be viewed
as "moves the ${B} from being in `cd` to being part of the command
line for `cmake --build`". Is fairly straight forward because cmake
requires a path be specified (where `make` makes it optional).
>
>> - base_do_compile VERBOSE=1
>> + VERBOSE=1 cmake --build '${B}'
>
> This looks like it will cause parallel make and EXTRA_OEMAKE options
> to be ignored?
That is a good point. I don't think EXTRA_OEMAKE should matter here
too much as configuration for cmake-based builds all is read at
configure time, but the ${PARALLEL_MAKE} flag is definitely an issue.
Doing this could work:
VERBOSE=1 cmake --build '${B}' -- ${EXTRA_OEMAKE}
(And similar for install)
And for the case of `-jN` like flags, make & ninja happen to be
compatible (they share a few other flags, like `-l` and `-n`.
Another alternative is passing ${PARALLEL_MAKE} (and
${PARALLEL_MAKEINST}) instead of directly using ${EXTRA_OEMAKE}.
Also: I took a look at what else `run_oemake` is doing, and it
contains some output printing (`bbnote`s the make command & `die`s on
make failure), which could be added directly around the `cmake
--build` invocations instead.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cmake.bbclass: use `cmake --build` to build & install
2017-05-02 15:49 ` Cody P Schafer
@ 2017-05-02 16:49 ` Andre McCurdy
2017-05-02 19:07 ` Cody P Schafer
0 siblings, 1 reply; 6+ messages in thread
From: Andre McCurdy @ 2017-05-02 16:49 UTC (permalink / raw)
To: Cody P Schafer; +Cc: OE Core mailing list
On Tue, May 2, 2017 at 8:49 AM, Cody P Schafer <dev@codyps.com> wrote:
>>> do_compile[progress] = "percent"
>>> cmake_do_compile() {
>>> - cd ${B}
>>
>> Removing the redundant "cd ${B}" should perhaps be a separate commit?
>
> I'm not really seeing that as useful, change right now can be viewed
> as "moves the ${B} from being in `cd` to being part of the command
> line for `cmake --build`". Is fairly straight forward because cmake
> requires a path be specified (where `make` makes it optional).
If ${B} is the default directory for both these tasks then "cd ${B}"
is redundant and can be removed. If it is not the default directory,
then removing it will potentially break do_compile_append() and
do_install_append() functions which rely on running in ${B}. That
logic doesn't depend on whether or not "cmake --build" takes care of
changing directory on it's own, so if "cd ${B}" is redundant, then
removing it should be standalone commit.
>>
>>> - base_do_compile VERBOSE=1
>>> + VERBOSE=1 cmake --build '${B}'
>>
>> This looks like it will cause parallel make and EXTRA_OEMAKE options
>> to be ignored?
>
> That is a good point. I don't think EXTRA_OEMAKE should matter here
> too much as configuration for cmake-based builds all is read at
> configure time, but the ${PARALLEL_MAKE} flag is definitely an issue.
>
> Doing this could work:
>
> VERBOSE=1 cmake --build '${B}' -- ${EXTRA_OEMAKE}
>
> (And similar for install)
>
> And for the case of `-jN` like flags, make & ninja happen to be
> compatible (they share a few other flags, like `-l` and `-n`.
>
> Another alternative is passing ${PARALLEL_MAKE} (and
> ${PARALLEL_MAKEINST}) instead of directly using ${EXTRA_OEMAKE}.
>
> Also: I took a look at what else `run_oemake` is doing, and it
> contains some output printing (`bbnote`s the make command & `die`s on
> make failure), which could be added directly around the `cmake
> --build` invocations instead.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cmake.bbclass: use `cmake --build` to build & install
2017-05-02 16:49 ` Andre McCurdy
@ 2017-05-02 19:07 ` Cody P Schafer
2017-05-02 20:46 ` Andre McCurdy
0 siblings, 1 reply; 6+ messages in thread
From: Cody P Schafer @ 2017-05-02 19:07 UTC (permalink / raw)
To: Andre McCurdy; +Cc: OE Core mailing list
On Tue, May 2, 2017 at 12:49 PM, Andre McCurdy <armccurdy@gmail.com> wrote:
> On Tue, May 2, 2017 at 8:49 AM, Cody P Schafer <dev@codyps.com> wrote:
>>>> do_compile[progress] = "percent"
>>>> cmake_do_compile() {
>>>> - cd ${B}
>>>
>>> Removing the redundant "cd ${B}" should perhaps be a separate commit?
>>
>> I'm not really seeing that as useful, change right now can be viewed
>> as "moves the ${B} from being in `cd` to being part of the command
>> line for `cmake --build`". Is fairly straight forward because cmake
>> requires a path be specified (where `make` makes it optional).
>
> If ${B} is the default directory for both these tasks then "cd ${B}"
> is redundant and can be removed. If it is not the default directory,
> then removing it will potentially break do_compile_append() and
> do_install_append() functions which rely on running in ${B}. That
> logic doesn't depend on whether or not "cmake --build" takes care of
> changing directory on it's own, so if "cd ${B}" is redundant, then
> removing it should be standalone commit.
meta/classes/base.bbclass
307:do_configure[dirs] = "${B}"
326:do_compile[dirs] = "${B}"
${B} is the default dir for do_compile & do_configure.
Are we concerned about prepend scripts changing the directory to
something not-${B} & appends implicitly relying on ${B} being the
directory?
It seems that prepends/appends should avoid changing directories &/or
avoid depending on a specific directory. Many (but not all) of the
appends I've seen are pretty explicitly about the dirs they need
(Using ${B}, ${S}, and ${WORKDIR} as needed instead of assuming the
dir). Is there a policy on what the right thing to do is w.r.t. cwd
changes within tasks & specifically within appends/prepends to tasks?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cmake.bbclass: use `cmake --build` to build & install
2017-05-02 19:07 ` Cody P Schafer
@ 2017-05-02 20:46 ` Andre McCurdy
0 siblings, 0 replies; 6+ messages in thread
From: Andre McCurdy @ 2017-05-02 20:46 UTC (permalink / raw)
To: Cody P Schafer; +Cc: OE Core mailing list
On Tue, May 2, 2017 at 12:07 PM, Cody P Schafer <dev@codyps.com> wrote:
> On Tue, May 2, 2017 at 12:49 PM, Andre McCurdy <armccurdy@gmail.com> wrote:
>> On Tue, May 2, 2017 at 8:49 AM, Cody P Schafer <dev@codyps.com> wrote:
>>>>> do_compile[progress] = "percent"
>>>>> cmake_do_compile() {
>>>>> - cd ${B}
>>>>
>>>> Removing the redundant "cd ${B}" should perhaps be a separate commit?
>>>
>>> I'm not really seeing that as useful, change right now can be viewed
>>> as "moves the ${B} from being in `cd` to being part of the command
>>> line for `cmake --build`". Is fairly straight forward because cmake
>>> requires a path be specified (where `make` makes it optional).
>>
>> If ${B} is the default directory for both these tasks then "cd ${B}"
>> is redundant and can be removed. If it is not the default directory,
>> then removing it will potentially break do_compile_append() and
>> do_install_append() functions which rely on running in ${B}. That
>> logic doesn't depend on whether or not "cmake --build" takes care of
>> changing directory on it's own, so if "cd ${B}" is redundant, then
>> removing it should be standalone commit.
>
> meta/classes/base.bbclass
> 307:do_configure[dirs] = "${B}"
> 326:do_compile[dirs] = "${B}"
>
> ${B} is the default dir for do_compile & do_configure.
>
> Are we concerned about prepend scripts changing the directory to
> something not-${B} & appends implicitly relying on ${B} being the
> directory?
No, no concerns about that. The point is simply that you appear to
have identified two redundant "cd ${B}" commands in cmake.bbclass and
removing them could/should be a standalone commit rather than being
mixed up with the "cmake --build" change. If every patch only does one
thing then it makes the review process a little easier.
> It seems that prepends/appends should avoid changing directories &/or
> avoid depending on a specific directory. Many (but not all) of the
> appends I've seen are pretty explicitly about the dirs they need
> (Using ${B}, ${S}, and ${WORKDIR} as needed instead of assuming the
> dir). Is there a policy on what the right thing to do is w.r.t. cwd
> changes within tasks & specifically within appends/prepends to tasks?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-02 20:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-01 19:42 [PATCH] cmake.bbclass: use `cmake --build` to build & install Cody P Schafer
2017-05-01 22:26 ` Andre McCurdy
2017-05-02 15:49 ` Cody P Schafer
2017-05-02 16:49 ` Andre McCurdy
2017-05-02 19:07 ` Cody P Schafer
2017-05-02 20:46 ` Andre McCurdy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.