Page MenuHomePhabricator

build: move EFL_BUILD to package_c_args used in all subprojects
ClosedPublic

Authored by stefan_schmidt on Tue, May 19, 4:29 AM.

Details

Summary

Original patch by Vincent Torri. Co-authored with Marcel Hollerbach.

fixup

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
stefan_schmidt created this revision.Tue, May 19, 4:29 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

vtorri requested changes to this revision.Tue, May 19, 4:36 AM

this is wrong until some other variable than package_c_args is used in binaries, tests, examples etc... EFL_BUILD lust be set ONLY for the libraries (and the benchlark stuff, which is an exception)

currently, package_c_args is also used in srb/bin meson.build files

This revision now requires changes to proceed.Tue, May 19, 4:36 AM

We can redefine package_c_args in line 408 to the state without -DEFL_BUILD=1. Otherwise, this looks exactly like the idea I had in mind when writing the mail. Thank you very much.

vtorri added a comment.EditedTue, May 19, 5:02 AM

what about setting EFL_BUILD only if subproject is mod or lib ? it seems more logical to set it when needed, not unset it when it's not needed

I agree that is would be more logical when only adding it to lib and mod, but it seems not as easy as I thought. When I do the following:

index 83703c2160..c64febec70 100644
--- a/meson.build
+++ b/meson.build
@@ -401,9 +401,11 @@ foreach package : subprojects
       set_variable(package_name + '_eot_files', pub_eo_types_files)
       set_variable(package_name + '_header_subdirs', package_header_subdirs)
       set_variable(package_name + '_eo_subdirs', package_eo_subdirs)
+      package_c_args += ['-DEFL_BUILD=1']
     endif
     if (package[2])
        subdir(join_paths(local_module, package_name))
+       package_c_args += ['-DEFL_BUILD=1']
     endif
     if (package[4])
        subdir(join_paths(local_bin, package_name))

I run into the following error:

FAILED: src/modules/ethumb/emotion/template.edj
/usr/bin/env EFL_RUN_IN_TREE=1 /home/stefan/EFL/efl/build/src/bin/edje/edje_cc -beta -fastcomp -id /home/stefan/EFL/efl/src/modules/ethumb/emotion -fd /home/stefan/EFL/efl/src/modules/ethumb/emotion -sd /home/stefan/EFL/efl/src/modules/ethumb/emotion -vd /home/stefan/EFL/efl/src/modules/ethumb/emotion -dd /home/stefan/EFL/efl/src/modules/ethumb/emotion -md /home/stefan/EFL/efl/src/modules/ethumb/emotion -td /home/stefan/EFL/efl/src/modules/ethumb/emotion ../src/modules/ethumb/emotion/template.edc src/modules/ethumb/emotion/template.edj
ERR<236309>:eo ../src/lib/eo/eo.c:809 _eo_class_funcs_set() Class 'Efl.Canvas.Object': NULL API not allowed (NULL->0x7f22b005ed7f 'unknown').
## Copy & Paste the below (until EOF) into a terminal, then hit Enter

eina_btlog << EOF
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/eina/libeina.so.1      0x7f22b032f717 0x7f22b02fb000
/home/stefan/EFL/efl/build/src/bin/edje/edje_cc  0x404fb8 0x400000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/eina/libeina.so.1      0x7f22b032e7d1 0x7f22b02fb000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/eina/libeina.so.1      0x7f22b033005a 0x7f22b02fb000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/evas/../eo/libeo.so.1  0x7f22af46b188 0x7f22af460000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/evas/../eo/libeo.so.1  0x7f22af46b863 0x7f22af460000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/evas/libevas.so.1      0x7f22b0066cf9 0x7f22aff27000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/evas/../eo/libeo.so.1  0x7f22af46c9f8 0x7f22af460000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/evas/../eo/libeo.so.1  0x7f22af46dd33 0x7f22af460000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/evas/libevas.so.1      0x7f22b0066e05 0x7f22aff27000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/evas/libevas.so.1      0x7f22b011b1b7 0x7f22aff27000
/home/stefan/EFL/efl/build/src/bin/edje/../../lib/evas/libevas.so.1      0x7f22b011a46c 0x7f22aff27000
/home/stefan/EFL/efl/build/src/bin/edje/edje_cc  0x4098da 0x400000
/home/stefan/EFL/efl/build/src/bin/edje/edje_cc  0x40df4a 0x400000
/home/stefan/EFL/efl/build/src/bin/edje/edje_cc  0x4061f8 0x400000
/lib64/libc.so.6         0x7f22af91d1a3 0x7f22af8f6000
/home/stefan/EFL/efl/build/src/bin/edje/edje_cc  0x404d1e 0x400000
EOF

[782/1420] Compiling C object 'src/lib/elementary/f70ca57@@elementary@sha/efl_ui_vg_animation.c.o'.

There is some place we are not reaching here. Need to look further.

You need to set that variable before the subdir call. and you need to set that variable to the old value before the subdir call to binary, best case before line 408.

Ah, before subdir was indeed the trick. When doing this I can leave the original package_c_args withour EFL_BUILD and only add it for libs and modules.

Going to change this romorrow and also look at the other review feedback.

stefan_schmidt edited the summary of this revision. (Show Details)

Onle use EFL_BUILD in libs and modules. Call before subdir().

ping, updated and working here.

bu5hm4n accepted this revision.Mon, May 25, 6:09 AM

Yeah, thats how i meant it. Thank you.

jptiz added a comment.Mon, May 25, 6:40 AM

I applyed the patch locally here (against updated master), but it fails with the same error @stefan_schmidt had.

bu5hm4n requested changes to this revision.Mon, May 25, 6:47 AM

Wait, this is still missing the resetting to the state before, in line 407. (I am now sitting again at a real laptop, i can provide what i meant after 17:15)

This revision now requires changes to proceed.Mon, May 25, 6:47 AM
stefan_schmidt edited the summary of this revision. (Show Details)

Update to reset args after lib and mod are done

jptiz accepted this revision.Mon, May 25, 8:58 AM

Getting the patch straight from devs/stefan/efl-dll works pretty fine and passes tests. :)

@vtorri, @bu5hm4n you had change requests for earlier versions. Happy to approve now?

vtorri accepted this revision.Tue, May 26, 12:57 AM
This revision was not accepted when it landed; it landed in state Needs Review.Tue, May 26, 1:16 AM
Closed by commit rEFL46cedab6ccd8: build: move EFL_BUILD to package_c_args used in all subprojects (authored by stefan_schmidt, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain Why
This revision was automatically updated to reflect the committed changes.