Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Vala: unity build should merge Vala code, not C code #5280

Open
rilian-la-te opened this issue Apr 17, 2019 · 2 comments
Open

Vala: unity build should merge Vala code, not C code #5280

rilian-la-te opened this issue Apr 17, 2019 · 2 comments
Labels
bug language:vala

Comments

@rilian-la-te
Copy link
Contributor

When 2 C files generated from vala code merged in C code - unity builds failed almost always, because there are duplicated hidden C function.

Simple example included:

meson-bug.tar.gz
@rilian-la-te
Copy link
Contributor Author

If this build with just meson build it builds.

meson bld
The Meson build system
Version: 0.49.2
Source dir: /home/rilian/Projects/vala-meson-bug
Build dir: /home/rilian/Projects/vala-meson-bug/bld
Build type: native build
Project name: vala-bug
Project version: r1
cNative C compiler: cc (gcc 8.2.1 "cc (GCC) 8.2.1 20181127")
Native Vala compiler: valac (valac 0.44.3)
Build machine cpu family: x86_64
Build machine cpu: x86_64
Found pkg-config: /usr/bin/pkg-config (1.6.1)
Dependency glib-2.0 found: YES 2.60.1
Dependency gio-2.0 found: YES 2.60.1
Dependency gio-unix-2.0 found: YES 2.60.1
Dependency gmodule-2.0 found: YES 2.60.1
Dependency gtk+-3.0 found: YES 3.24.8
Library m found: YES
Build targets in project: 1
Found ninja-1.9.0 at /usr/bin/ninja
[rilian@5x5linux-arch vala-meson-bug]$ cd bld
[rilian@5x5linux-arch bld]$ ninja
[1/4] Compiling Vala source ../runner1.vala ../runner2.vala.
../runner1.vala:5.12-5.21: warning: local variable `tst' declared but never used
		string[] tst = args;
		         ^^^^^^^^^^
../runner2.vala:5.12-5.21: warning: local variable `tst' declared but never used
		string[] tst = args;
		         ^^^^^^^^^^
Compilation succeeded - 2 warning(s)
[4/4] Linking target vala-bug.

But if I try to build it with meson build --unity on it fails.

meson build --unity on
The Meson build system
Version: 0.49.2
Source dir: /home/rilian/Projects/vala-meson-bug
Build dir: /home/rilian/Projects/vala-meson-bug/build
Build type: native build
Project name: vala-bug
Project version: r1
Native C compiler: cc (gcc 8.2.1 "cc (GCC) 8.2.1 20181127")
Native Vala compiler: valac (valac 0.44.3)
Build machine cpu family: x86_64
Build machine cpu: x86_64
Found pkg-config: /usr/bin/pkg-config (1.6.1)
Dependency glib-2.0 found: YES 2.60.1
Dependency gio-2.0 found: YES 2.60.1
Dependency gio-unix-2.0 found: YES 2.60.1
Dependency gmodule-2.0 found: YES 2.60.1
Dependency gtk+-3.0 found: YES 3.24.8
Library m found: YES
Build targets in project: 1
Found ninja-1.9.0 at /usr/bin/ninja
[rilian@5x5linux-arch vala-meson-bug]$ cd build
[rilian@5x5linux-arch build]$ ninja
[1/3] Compiling Vala source ../runner1.vala ../runner2.vala.
../runner1.vala:5.12-5.21: warning: local variable `tst' declared but never used
		string[] tst = args;
		         ^^^^^^^^^^
../runner2.vala:5.12-5.21: warning: local variable `tst' declared but never used
		string[] tst = args;
		         ^^^^^^^^^^
Compilation succeeded - 2 warning(s)
[2/3] Compiling C object 'vala-bug@exe/meson-generated_vala-bug-unity.c.o'.
FAILED: vala-bug@exe/meson-generated_vala-bug-unity.c.o 
cc -Ivala-bug@exe -I. -I.. -I../ -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/lib/libffi-3.2.1/include -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/gio-unix-2.0 -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/include/fribidi -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libdrm -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/dbus-1.0/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=gnu11 -O2 -g -fstack-protector -pthread  -MD -MQ 'vala-bug@exe/meson-generated_vala-bug-unity.c.o' -MF 'vala-bug@exe/meson-generated_vala-bug-unity.c.o.d' -o 'vala-bug@exe/meson-generated_vala-bug-unity.c.o' -c 'vala-bug@exe/vala-bug-unity.c'
In file included from vala-bug@exe/vala-bug-unity.c:1:
runner1.c: In function ‘run_main’:
runner1.c:51:7: warning: variable ‘_tst_size_’ set but not used [-Wunused-but-set-variable]
In file included from vala-bug@exe/vala-bug-unity.c:2:
runner2.c: In function ‘run_func’:
runner2.c:51:7: warning: variable ‘_tst_size_’ set but not used [-Wunused-but-set-variable]
In file included from vala-bug@exe/vala-bug-unity.c:2:
runner2.c: At top level:
runner2.c:72:1: error: redefinition of ‘_vala_array_destroy’
In file included from vala-bug@exe/vala-bug-unity.c:1:
runner1.c:81:1: note: previous definition of ‘_vala_array_destroy’ was here
In file included from vala-bug@exe/vala-bug-unity.c:2:
runner2.c:87:1: error: redefinition of ‘_vala_array_free’
In file included from vala-bug@exe/vala-bug-unity.c:1:
runner1.c:96:1: note: previous definition of ‘_vala_array_free’ was here
In file included from vala-bug@exe/vala-bug-unity.c:2:
runner2.c:87:1: warning: ‘_vala_array_free’ defined but not used [-Wunused-function]

@AnsgarKlein AnsgarKlein mentioned this issue Nov 30, 2020
Closed
@AnsgarKlein
Copy link

This problem still exists. The correct way for unity builds for Vala code is merging the Vala code not the generated C code.

I tested with:

  • Fedora 32
  • Meson 0.55.3 from Fedora 32 repo as well as Meson 0.56.0 via pip
  • Python 3.8.6
  • Ninja 1.10.1
  • Both gcc and clang

I have created a workaround that checks if building a unity build with meson.is_unity() and concatenates all sources using a custom_target with a bash script. It would of course be nice if meson could handle this correctly by default.

I've attached that workaround and sample code for reproducing this issue.

vala-unity.tar.gz

dcbaker added a commit to dcbaker/meson that referenced this issue Feb 5, 2021
@dcbaker
vala: Disable unity builds
838fe09 
Our approach to unity builds with vala is broken, you cannot unify the
generated C files, as they contain duplicate symbols. We would need to
instead combine the files while they are still in their vala form, then
convert that to C and compile the unified C file.

This does not fix the linked issue, as this removed the ability to do
vala unity builds, but it does allow running vala with `--unity=on`.

Related: mesonbuild#5280
dcbaker added a commit that referenced this issue Feb 6, 2021
@dcbaker
vala: Disable unity builds
456844a 
Our approach to unity builds with vala is broken, you cannot unify the
generated C files, as they contain duplicate symbols. We would need to
instead combine the files while they are still in their vala form, then
convert that to C and compile the unified C file.

This does not fix the linked issue, as this removed the ability to do
vala unity builds, but it does allow running vala with `--unity=on`.

Related: #5280
tristan957 pushed a commit to tristan957/meson that referenced this issue Mar 2, 2021
@dcbaker @tristan957
vala: Disable unity builds
7a4df0e 
Our approach to unity builds with vala is broken, you cannot unify the
generated C files, as they contain duplicate symbols. We would need to
instead combine the files while they are still in their vala form, then
convert that to C and compile the unified C file.

This does not fix the linked issue, as this removed the ability to do
vala unity builds, but it does allow running vala with `--unity=on`.

Related: mesonbuild#5280
AnsgarKlein added a commit to AnsgarKlein/GPG-Gui that referenced this issue Jun 29, 2021
@AnsgarKlein
Add bash script to support unity builds
4df9728 
Meson currently does not support unity builds for vala sources out of
the box: Older versions of Meson will try to unify the resulting
C sources which fails because unifying them leads to duplicated
symbols.
Newer versions of Meson will simply refuse to unify vala sources and
print a warning message.

This commit introduces a bash script which simply checks if a unity
build is requested and replaces all vala source files with a
concatenation of those source files.

This work-around works with both the older and newer behavior of Meson
although the newer versions will still print a warning.

This is a work-around for the following Meson bug:
Vala: unity build should merge Vala code, not C code
mesonbuild/meson#5280
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug language:vala
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rilian-la-te @AnsgarKlein @dcbaker