Page MenuHomePhabricator

build: lib: harmonize the use of package_c_args in all libs
ClosedPublic

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

Details

Summary

Add it to subprojects which are not using it and remove and old
ELEMENTARY_BUILD define we no longer use. This allows us to have a
central place in the main meson.build file to set this variable.

Depends on D11853

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 accepted this revision.Tue, May 19, 4:53 AM
This revision is now accepted and ready to land.Tue, May 19, 4:53 AM
bu5hm4n requested changes to this revision.Tue, May 19, 7:46 AM

Looks good beside the edje_external things.

src/edje_external/elementary/meson.build
48 ↗(On Diff #30334)

That is wrong, and will simply use the package_c_args of the last defined package (Note, meson does not have scoping)

src/edje_external/emotion/meson.build
6 ↗(On Diff #30334)

That is wrong, and will simply use the package_c_args of the last defined package (Note, meson does not have scoping)

This revision now requires changes to proceed.Tue, May 19, 7:46 AM
stefan_schmidt added inline comments.Wed, May 20, 1:15 AM
src/edje_external/elementary/meson.build
48 ↗(On Diff #30334)

Not sure I understand what you mean here. For me it looks like elm and emotion_package_c_args get simply the same value assigned in the elm or emotion meson.build file from package_c_args.
What do I miss here? Is this a problem because edje_external is not part of the normal subprojects we define?

jptiz added a comment.Fri, May 22, 8:29 AM

Figured out what @bu5hm4n was saying, I think.

src/edje_external/elementary/meson.build
48 ↗(On Diff #30334)

Ok, I got it.

Is this a problem because edje_external is not part of the normal subprojects we define?

Sort of but not exactly, it's because of the order things occur and where package_c_args is set. First, tracking where are package_c_args mentions:

$ grep -Rn "package_c_args" **/meson.build
meson.build:362:  package_c_args = [
src/bindings/cxx/eolian_cxx/meson.build:71:                   cpp_args : package_c_args,
src/bindings/cxx/meson.build:28:  package_c_args = [
src/bindings/mono/eolian_mono/meson.build:8:                   cpp_args : package_c_args,
src/bin/edje/epp/meson.build:18:          package_c_args,
src/bin/edje/meson.build:29:        c_args : package_c_args,
src/bin/edje/meson.build:64:        c_args : package_c_args,
src/bin/edje/meson.build:71:        c_args : package_c_args,
src/bin/edje/meson.build:78:        c_args : package_c_args,
src/bin/edje/meson.build:85:        c_args : package_c_args,
src/bin/edje/meson.build:92:        c_args : package_c_args,
src/bin/edje/meson.build:108:        c_args : package_c_args,
src/bin/edje/meson.build:115:        c_args : package_c_args,
src/bin/efreet/meson.build:17:        c_args : package_c_args,
src/bin/efreet/meson.build:27:        c_args : package_c_args,
src/bin/efreet/meson.build:35:        c_args : package_c_args,
src/bin/efreet/meson.build:43:        c_args : package_c_args,
src/bin/elementary/meson.build:173:  package_c_args =  package_c_args + ['-fPIC']
src/bin/elementary/meson.build:182:        c_args : package_c_args + [
src/bin/elementary/meson.build:194:        c_args : package_c_args + [
src/bin/elementary/meson.build:210:        c_args : package_c_args,
src/bin/elementary/meson.build:223:          c_args : package_c_args,
src/bin/elementary/meson.build:236:        c_args : package_c_args,
src/bin/elementary/meson.build:265:        c_args : package_c_args,
src/bin/elementary/meson.build:287:          c_args : package_c_args,
src/bin/elementary/meson.build:307:        c_args : package_c_args + [
src/bin/embryo/meson.build:23:        c_args : package_c_args,
src/bin/eolian/meson.build:18:        c_args : package_c_args,
src/bin/ethumb_client/meson.build:6:        c_args : package_c_args,
src/bin/ethumb_client/meson.build:14:        c_args : package_c_args,
src/bin/ethumb_client/meson.build:23:        c_args : package_c_args,
src/bin/ethumb/meson.build:5:        c_args : package_c_args,
src/edje_external/elementary/meson.build:48:    c_args : elm_package_c_args,
src/edje_external/emotion/meson.build:6:    c_args : emotion_package_c_args,
src/lib/ecore_con/meson.build:190:    c_args : package_c_args,
src/lib/ecore_drm/meson.build:28:    c_args : package_c_args,
src/lib/ecore_imf_evas/meson.build:17:    c_args : package_c_args,
src/lib/ecore_imf/meson.build:20:    c_args : package_c_args,
src/lib/ecore/meson.build:200:    c_args : package_c_args,
src/lib/ecore_sdl/meson.build:20:    c_args : package_c_args,
src/lib/ecore_wayland/meson.build:31:    c_args : package_c_args,
src/lib/ecore_win32/meson.build:28:      c_args : package_c_args,
src/lib/edje/meson.build:152:    c_args : [package_c_args],
src/lib/eeze/meson.build:93:    c_args : package_c_args,
src/lib/efl_canvas_wl/meson.build:40:    c_args : package_c_args,
src/lib/efreet/meson.build:39:    c_args : [package_c_args, '-DDATA_DIR="'+dir_data+'"'],
src/lib/efreet/meson.build:54:    c_args : package_c_args,
src/lib/efreet/meson.build:70:    c_args : package_c_args,
src/lib/eio/meson.build:74:    c_args : package_c_args,
src/lib/elementary/meson.build:1013:elm_package_c_args =  package_c_args + ['-DELEMENTARY_BUILD=1']
src/lib/elementary/meson.build:1020:    c_args : elm_package_c_args,
src/lib/elua/meson.build:12:    c_args : package_c_args,
src/lib/embryo/meson.build:27:    c_args : package_c_args,
src/lib/emotion/meson.build:52:    c_args : package_c_args,
src/lib/emotion/meson.build:56:emotion_package_c_args = package_c_args
src/lib/eolian/meson.build:42:    c_args : package_c_args,
src/lib/ephysics/meson.build:28:    c_args : package_c_args,
src/lib/ethumb_client/meson.build:21:    c_args : package_c_args,
src/lib/ethumb/meson.build:20:    c_args : package_c_args,
src/modules/emotion/gstreamer1/meson.build:24:    c_args : package_c_args,
src/modules/ethumb/emotion/meson.build:31:  c_args : package_c_args,
src/tests/efl_mono/meson.build:185:  cpp_args : package_c_args + [
src/tests/efreet/meson.build:41:  package_c_args,
src/tests/efreet/meson.build:62:    package_c_args,
src/tests/eolian_cxx/meson.build:64:  # package_c_args contains -D definitions for the package
src/tests/eolian_cxx/meson.build:65:  cpp_args : package_c_args +[

So there are only 2 places where package_c_args is set:

meson.build:362:  package_c_args = [
src/bindings/cxx/meson.build:28:  package_c_args = [

In meson.build, the build order is basically (considering only the relevant parts for this issue):

  1. Packages (listed as a giant table in subprojects variable looped through a foreach).
  2. src/bindings/cxx, which sets package_c_args (see above) in the middle of a foreach loop (in the same fashion of subprojects);
  3. src/lib/edje_external, which is now (with this patch) using the overwritten package_c_args.

Yeah, exactly what @jptiz said.

The variable package_c_args is only defined in the loop over the subprojects, and bindings, but not yet in the edje externals.

Thanks a lot you two for the explanation. I will keep the settign of the emotion end elm package_c_args in place to have the correct values passed up here.

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

Update to keep the extra variables for edje_external

bu5hm4n accepted this revision.Mon, May 25, 7:08 AM
This revision is now accepted and ready to land.Mon, May 25, 7:08 AM
jptiz accepted this revision.Mon, May 25, 7:29 AM

Passing tests :)

Closed by commit rEFL3ca9d7282515: build: lib: harmonize the use of package_c_args in all libs (authored by stefan_schmidt, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyTue, May 26, 1:16 AM
This revision was automatically updated to reflect the committed changes.