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.

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

Improve Vala support #5302

Merged
merged 6 commits into from
Jul 5, 2024
Merged

Improve Vala support #5302

merged 6 commits into from
Jul 5, 2024

Conversation

al1-ce
Copy link
Contributor

@al1-ce al1-ce commented Jul 3, 2024

When compiling multiple files valac fails to find symbols (i.e SomeClass from a.vala using in b.vala), changed build to process all source files at once

When compiling multiple files valac fails to find symbols (i.e SomeClass
from a.vala using in b.vala), changed build to process all source files
at once
xmake/rules/vala/xmake.lua Outdated Show resolved Hide resolved
@al1-ce al1-ce marked this pull request as draft July 4, 2024 04:13
@al1-ce al1-ce marked this pull request as ready for review July 4, 2024 10:34
@al1-ce al1-ce requested a review from waruqi July 4, 2024 10:40
xmake/rules/vala/xmake.lua Outdated Show resolved Hide resolved
xmake/rules/vala/xmake.lua Outdated Show resolved Hide resolved
xmake/rules/vala/xmake.lua Outdated Show resolved Hide resolved
@waruqi
Copy link
Member

waruqi commented Jul 4, 2024

@al1-ce
Copy link
Contributor Author

al1-ce commented Jul 4, 2024

Checking rn

xmake/rules/vala/xmake.lua Outdated Show resolved Hide resolved
@al1-ce
Copy link
Contributor Author

al1-ce commented Jul 4, 2024

Strange, vala/lua test doesn't compile even with upstream rule file...

@al1-ce
Copy link
Contributor Author

al1-ce commented Jul 4, 2024

Implemented requested changes, all tests passing except for lua, but lua fails because of xmake (and maybe arch linux, it has headers who knows where) coz gcc fails to find lua includes from -Llua flag

Also fixed vala.vapidir and vala.flags not being applied and fixed vala.vapifile and vala.header locations being incorrect

@al1-ce al1-ce requested a review from waruqi July 4, 2024 17:32
xmake/rules/vala/xmake.lua Outdated Show resolved Hide resolved
xmake/rules/vala/xmake.lua Outdated Show resolved Hide resolved
@al1-ce
Copy link
Contributor Author

al1-ce commented Jul 5, 2024

Couldn't get valac build to run only when needed, but everything else should be correct now

@al1-ce al1-ce requested a review from waruqi July 5, 2024 10:06
@waruqi
Copy link
Member

waruqi commented Jul 5, 2024

But incremental compilation still doesn't work.

bash-5.2$ xmake
[ 20%]: compiling.vala src/mymath.vala
[ 70%]: compiling.vala src/main.vala
[100%]: build ok, spent 0.588s

bash-5.2$ xmake
[ 20%]: compiling.vala src/mymath.vala
[ 70%]: compiling.vala src/main.vala
[100%]: build ok, spent 0.571s

@waruqi waruqi added this to the v2.9.4 milestone Jul 5, 2024
@al1-ce
Copy link
Contributor Author

al1-ce commented Jul 5, 2024

Oh, I think I'm starting to understand how batchcmds work, not completely though...
Hard to figure out, not that much documentation about it, except for examples which do not explain anything

@waruqi
Copy link
Member

waruqi commented Jul 5, 2024

Oh, I think I'm starting to understand how batchcmds work, not completely though... Hard to figure out, not that much documentation about it, except for examples which do not explain anything

https://xmake.io/#/manual/custom_rule?id=custom-batch-compile-script-process-one-source-file-at-a-time

@waruqi waruqi merged commit 3f1be27 into xmake-io:dev Jul 5, 2024
19 checks passed
@al1-ce
Copy link
Contributor Author

al1-ce commented Jul 5, 2024

https://xmake.io/#/manual/custom_rule?id=custom-batch-compile-script-process-one-source-file-at-a-time

Again, shows how, but now why. Maybe worth adding some explaining comments to it?

Anyway, thanks for taking time to review and merge my PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants