Skip to content

compilers: Detect Vala compiler for the requested machine#10735

Closed
oleavr wants to merge 1 commit into
mesonbuild:masterfrom
frida:fix/vala-compiler-detection
Closed

compilers: Detect Vala compiler for the requested machine#10735
oleavr wants to merge 1 commit into
mesonbuild:masterfrom
frida:fix/vala-compiler-detection

Conversation

@oleavr

@oleavr oleavr commented Aug 24, 2022

Copy link
Copy Markdown
Contributor

Instead of unconditionally detecting it for the build machine.
The rationale for making this use the build machine was wrong.
The valac transpiler needs search paths for the host machine
(specifically, the --vapidir default paths).

This reverts part of af846a1.

@oleavr oleavr requested a review from dcbaker as a code owner August 24, 2022 15:43
@eli-schwartz

Copy link
Copy Markdown
Member

This seems wrong and the argument in the other PR for why you want to use the build machine is pretty sensible IMO.

I'm not sure why you want to revert it, it basically sounds like "but I want to do it wrong"? It basically lacks explanation.

...

What's wrong with specifying it in the native file? The PR should explicitly rationalize why you find that to be unsatisfactory, rather than leaving reviewers to try to guess.

@codecov

codecov Bot commented Aug 24, 2022

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (adf09b8) 70.23% compared to head (9f8b501) 50.42%.

❗ Current head 9f8b501 differs from pull request most recent head 3e2c3f8. Consider uploading reports for the commit 3e2c3f8 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #10735       +/-   ##
===========================================
- Coverage   70.23%   50.42%   -19.82%     
===========================================
  Files         219      203       -16     
  Lines       48534    44152     -4382     
  Branches    11516     9786     -1730     
===========================================
- Hits        34090    22264    -11826     
- Misses      12009    19887     +7878     
+ Partials     2435     2001      -434     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@oleavr

oleavr commented Aug 24, 2022

Copy link
Copy Markdown
Contributor Author

The PR should explicitly rationalize why you find that to be unsatisfactory, rather than leaving reviewers to try to guess.

Oops, indeed. Apologies.

What's wrong with specifying it in the native file?

It couples the choice to what's chosen for the build machine, when e.g. pkgconfig is decoupled from it. Meson currently assumes that if it finds a dependency through pkgconfig, there will be a corresponding .vapi on the Vala compiler's vapi search path.

I think it would make sense to fall back to using the build machine's selection though, but unconditionally coupling things seems unnecessary.

@eli-schwartz

eli-schwartz commented Aug 24, 2022

Copy link
Copy Markdown
Member

Ah okay, so the vapidir is host machine specific.

So the issue (and the commit message) is not/should not be:

The cross-file may want to specify a particular compiler.

But rather:

The rationale for making this use the build machine was wrong. The valac transpiler needs search paths for the host machine (specifically, the --vapidir default paths).

@eli-schwartz

Copy link
Copy Markdown
Member

I wonder if it's possible to somehow detect this. Probably setting the cross file to ['/usr/bin/valac', '--vapidir', '/path/to/sysroot/usr/share/vala/vapi/'] or something would allow using a native version anyway. It might be possible to be clever about this.

@oleavr oleavr force-pushed the fix/vala-compiler-detection branch from a9abd9c to cb15ef1 Compare August 24, 2022 18:04
@oleavr

oleavr commented Aug 24, 2022

Copy link
Copy Markdown
Contributor Author

Ah okay, so the vapidir is host machine specific.

So the issue (and the commit message) is not/should not be:

The cross-file may want to specify a particular compiler.

But rather:

The rationale for making this use the build machine was wrong. The valac transpiler needs search paths for the host machine (specifically, the --vapidir default paths).

Thanks! Just updated the issue description and commit message.

Instead of unconditionally detecting it for the build machine.
The rationale for making this use the build machine was wrong.
The Vala compiler needs search paths for the host machine
(specifically, the `--vapidir` default paths).

This reverts part of af846a1.
@bonzini

bonzini commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Closing in favor of #15736.

@bonzini bonzini closed this Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants