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

vala: Port Vala to Meson #969

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

vala: Port Vala to Meson #969

wants to merge 6 commits into from

Conversation

Diego-Ivan
Copy link
Contributor

@Diego-Ivan Diego-Ivan commented Jul 12, 2024

This PR ports Vala from scripts to Meson, and introduces a minimal Vala template with the Workbench API file and a meson build file.

Fixes #964
Fixes #913

@Diego-Ivan Diego-Ivan requested a review from lw64 as a code owner July 12, 2024 16:09
@Diego-Ivan Diego-Ivan requested a review from sonnyp July 12, 2024 16:10
@Diego-Ivan Diego-Ivan marked this pull request as draft July 12, 2024 16:39
@Diego-Ivan Diego-Ivan marked this pull request as ready for review July 13, 2024 21:23
Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

One thing to keep in mind is we need to keep compiling existing Vala project so please make sure that's working

I will let @lw64 do the first review

@Diego-Ivan
Copy link
Contributor Author

@sonnyp Existing sessions and all the demos work as expected. Projects don't, because meson won't accept the file paths given by a portal. I tested this with Rust/cargo and it happens too.

A possible solution might be copying the Vala sources to a temporary folder that meson accepts (like the cache directory) and compile it there. The source files are usually small so it shouldn't be expensive. What do you think?

@sonnyp
Copy link
Contributor

sonnyp commented Jul 16, 2024

meson won't accept the file paths given by a portal

why not?

@Diego-Ivan
Copy link
Contributor Author

Same as #846 unfortunately :(

@sonnyp
Copy link
Contributor

sonnyp commented Oct 7, 2024

Same as #846 unfortunately :(

I see

A possible solution might be copying the Vala sources to a temporary folder that meson accepts (like the cache directory) and compile it there. The source files are usually small so it shouldn't be expensive. What do you think?

Sounds interesting, perhaps we can solve #846 the same way if it turns out to be a good idea.

I'm also interested to avoid having a builddir in the project directory.

Wanna give it a try?

@sonnyp
Copy link
Contributor

sonnyp commented Oct 7, 2024

I tested this and besides projects it seems to work great. I also merged main into your branch and fixed conflicts.

@Diego-Ivan
Copy link
Contributor Author

I'm also interested to avoid having a builddir in the project directory.

I think it would be better too, since Meson's builddirs can break between major versions, and old projects may break as a consequence of this.

I'll give it a try this week :)

Initial work to port Vala to Meson
This function will allow to create temporary directory. This will
allow, for instance, meson to create a builddir that will not be
stored inside a session.
This allows to compile projects without granting access to the filesystem.
Additionally, all sessions opened that compile Vala code will use
the same build directory, allowing us to reuse the same meson
configuration.
@Diego-Ivan
Copy link
Contributor Author

I think this PR is ready! A little summary of the changes:

  • Vala projects will be built in the /tmp/vala directory, instead of the working directory of the session.
  • The /tmp/vala folder will contain all the files necessary to compile a session. Every time Run is clicked, the main.vala file in the tmp folder will be replaced by the demo's/project's main.vala. Probably we can do this with Rust too, so we don't have to grant filesystem permissions to compile projects.

Also updated the newer demos in workbenchdev/demos#187, tested locally and CI passed, so this is ready for review.

Comment on lines +355 to +364
const current_dir = Gio.File.new_for_path(GLib.get_current_dir());

const template_dir = GLib.getenv("FLATPAK_ID")
? Gio.File.new_for_path(
`/app/share/${GLib.getenv("FLATPAK_ID")}/langs/vala/template`,
)
: current_dir.resolve_relative_path("src/langs/vala/template");

const api_file = template_dir.get_child("workbench.vala");
api_file.copy(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const current_dir = Gio.File.new_for_path(GLib.get_current_dir());
const template_dir = GLib.getenv("FLATPAK_ID")
? Gio.File.new_for_path(
`/app/share/${GLib.getenv("FLATPAK_ID")}/langs/vala/template`,
)
: current_dir.resolve_relative_path("src/langs/vala/template");
const api_file = template_dir.get_child("workbench.vala");
api_file.copy(
const template_dir = Gio.File.new_for_path(pkg.pkgdatadir).get_child("langs/vala/template");
template_dir.get_child("workbench.vala").copy(

Comment on lines -63 to -70

// Takes a string starting with the line
// #!/usr/bin/env -S vala workbench.vala --pkg gtk4 --pkg libadwaita-1
// and return ["--pkg", "gtk4", "--pkg", "libadwaita-1"]
// FIXME: consider using https://docs.gtk.org/glib/struct.OptionContext.html instead
function getValaCompilerArguments(text) {
return text.split("\n")[0]?.split("-S vala ")[1]?.split(" ") || [];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

"--vapi",
"/dev/null",
...args,
const meson_clean = meson_launcher.spawnv([
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why cleaning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When replacing the file in the build dir, meson will not detect any changes on the file, thus not compiling anything and just running the project right away. This is a problem when a user has two open projects and working on them simultaneously, so I went for cleaning the build files and recompiling. I thought this approach was OK since we don't really compile much.

import { createTmpDir, copy } from "../../util.js";
import { setupValaProject } from "./vala.js";

let ready_to_build = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about why this is needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this to know if the Vala builddir was already setup (as it needs a meson file and the workbench API). This could be done checking if a build file (like workbench.vala) exists, but I went for this instead as it is one less I/O operation.

Comment on lines +26 to +33
dependency('libadwaita-1'),
dependency('shumate-1.0'),
dependency('libportal-gtk4'),
dependency('libsoup-3.0'),
dependency('webkitgtk-6.0'),
dependency('gstreamer-1.0'),
dependency('gtksourceview-5'),
dependency('json-glib-1.0'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dependency('libadwaita-1'),
dependency('shumate-1.0'),
dependency('libportal-gtk4'),
dependency('libsoup-3.0'),
dependency('webkitgtk-6.0'),
dependency('gstreamer-1.0'),
dependency('gtksourceview-5'),
dependency('json-glib-1.0'),
dependency('libadwaita-1'),
dependency('shumate-1.0'),
dependency('libportal-gtk4'),
dependency('libsoup-3.0'),
dependency('webkitgtk-6.0'),
dependency('gstreamer-1.0'),
dependency('gtksourceview-5'),
dependency('json-glib-1.0'),
dependency('libspelling-1'),

Comment on lines +12 to +14
// VLS needs the project to be already setup once it starts,
// otherwise it won't pick it up later.
setupValaProject(file.get_parent()).catch(console.error);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to move the meson and workbench.vala files under a subdir? to avoid cluttering the project

and we might need meson.build for other purposes too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I think this is possible. Would that subdir still be inside the project folder? Or do you mean saving just the main.vala in the project directory and get rid of all of the build stuff? I'm guessing this last option is also possible.


const tmp_dirs = {};

export async function createTmpDir(filename) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll check them out!

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.

Consider porting Vala to Meson Vala diagnostics unknown symbols for Gtk and Workbench
2 participants