Enable storage of meson.build in a non-root directory in the workspace #179

wolfpld · 2023-11-03T21:43:47Z

This PR allows the meson.build file to be placed in a subdirectory within the workspace.

The current behavior of the extension is inconsequential, as some checks already look for meson.build in the entire workspace, and the activation event fires when you manually open meson.build that is stored in a subdirectory, resulting in a prompt to configure the workspace. Of course, there's no support for such behavior within the extension, so things fail.

This is similar to #22, but works automatically. No user-facing configuration option is exposed. If multiple meson.build files are found in the workspace, the 'first' one is used. I plan to extend this to allow the user to switch between different meson.build configurations within the same workspace, but this PR is a prerequisite for those future changes.

This PR also fixes a regression introduced in 60dc5f7. This commit removed the "configure on open" condition, rendering the reconfigure prompt completely useless and severely degrading the new user experience. The PR restores the removed condition.

In an effort to make things more robust, multiple recalculations of the build directory in different parts of the code have been replaced by a single calculation on extension activation, which is then stored as a workspace-relative memento that can be referenced when needed.

To make things consistent, the working directory provided by debug configurations and when running tests is now set to the "source directory" in the meson setup understanding, i.e. the directory where the meson.build file resides.

There are two places in the code where vscode.workspace.rootPath is still used instead of the source directory. I can't check if this is valid or not.

Attempting to explore the targets in the Meson section of the activity bar fails with a s.sources is not iterable (cannot read property undefined) error message. This also happens in the currently deployed extension.
The useCompileCommands function provides data for Microsoft's C++ extension, which I am not interested in. The alternative clangd extension can use the same functionality with the compile_commands.json file, and properly providing it for clangd would also show how to do it for Microsoft's extension. But there's another can of worms to tackle to even try to do anything with it (https://mastodon.gamedev.place/@wolfpld/111346306341679920).

tristan957 · 2023-11-06T15:26:56Z

I plan to look at this for 1.17. Thanks!

…pace. This fixes logic mismatch between the activation events and the check for existence of project inside the workspace. When meson file is outside the root directory, the extension will still activate when the you open the file. The extension is still not able to handle non-root meson file, but at least now the failure mode is promptly exposed, instead of being hidden.

wolfpld · 2023-11-13T18:47:06Z

If multiple meson.build files are found in the workspace, the 'first' one is used.

The 'first' here is not well defined, as suspected. It seems that the file search is asynchronous, as I have seen the meson.build being randomly selected when entering the workspace. This is a regression that needs to be fixed by implementing a proper selection solution within this PR, not at a later stage. The problem may occur even if there's only one meson.build in the repository, for example if a third-party submodule also provides its own meson.build.

Another problem is the meson subdir command, which includes meson.build files from subdirectories. The solution here is to remove any meson.build files from the selection list that have other meson.build files referencing them.

wolfpld · 2023-11-14T15:52:58Z

I have added a selection dialog when there is more than one meson.build file in the workspace. It appears when the user chooses to do the initial configuration from the popup. The user's selection is saved persistently for the current workspace.

If the selection matches the (randomly chosen) first item in the array of found meson.build files, the reconfiguration command is issued as before, since everything is already set up as it should be.

Otherwise, a flag is set that will survive a window reload, and the reload command is issued immediately. When the extension initialization process starts after this reload, the previously selected meson.build is used instead of the first found file. The flag allows to bypass the configuration prompt and the meson.build selector.

While the meson.build selection is saved for the workspace, the code logic should handle cases where this file is moved to another directory by doing the selection process again.

Filtering the subdir commands is not yet handled.

Currently there's no way to switch to a different meson.build file after the initial selection, other than removing the build subdirectory. I would like to add this as a command and to the Meson sidebar.

tristan957 · 2023-11-14T16:58:09Z

Do you plan on adding the rest of the features in this PR?

wolfpld · 2023-11-14T16:58:52Z

Yes.

tristan957 · 2023-11-14T16:58:59Z

src/extension.ts

@@ -207,6 +207,10 @@ export async function activate(ctx: vscode.ExtensionContext) {
          break;
      }
    }
+
+    if (configureOnOpen === true) {


Can you remove the comparison?

tristan957 · 2023-11-14T16:59:38Z

src/extension.ts

@@ -21,6 +21,7 @@ import { SettingsKey, TaskQuickPickItem } from "./types";
 import { createLanguageServerClient } from "./lsp/common";

 export let extensionPath: string;
+export let workspaceState: vscode.Memento;


Can you explain a little more what this commit is doing, like what is a memento?

Memento is part of the VS Code API. I would expect the developer to be familiar with it.

https://code.visualstudio.com/api/references/vscode-api#Memento

I don't know the API and am not the original author. I just maintain it with bare minimum knowledge.

Sorry, that was not meant as a dig at you, but as a general way of saying that a developer working on this code would know the API provided by VS Code.

I will add a reference to https://www.chrishasz.com/blog/2020/07/28/vscode-how-to-use-local-storage-api/ to the commit message, which explains what it does a bit better than the lackluster VS Code documentation.

We definitely need more contributors to this extension. You seem to have a solid handle on this.

tristan957 · 2023-11-14T17:00:43Z

src/extension.ts

@@ -37,6 +39,8 @@ export async function activate(ctx: vscode.ExtensionContext) {
  const root = vscode.workspace.workspaceFolders[0].uri.fsPath;
  const buildDir = workspaceRelative(extensionConfiguration("buildFolder"));

+  workspaceState.update("mesonbuild.buildDir", buildDir);


Can we call it mesonbuild.buildFolder? Rename the variable too.

Personally, I do not like the Microsoft newspeak that replaces directory with folder. The buildDir name is pervasive in the repository.

[18:34 wolf@oberon:~/vscode-meson]% git grep buildDir | wc -l 78 [18:34 wolf@oberon:~/vscode-meson]% git grep buildFolder | wc -l 7

For example, the current main branch:

vscode-meson/src/utils.ts

Line 67 in 68f18fb

const buildDir = workspaceRelative(extensionConfiguration("buildFolder"));

Ok. Let's work on replacing the setting in the future. No big deal

tristan957 · 2023-11-14T17:03:35Z

src/extension.ts

@@ -45,7 +45,9 @@ export async function activate(ctx: vscode.ExtensionContext) {
  const mesonFile = mesonFiles[0];
  const sourceDir = dirname(mesonFile.fsPath);
  const buildDir = relativeBuildDir(mesonFile.fsPath);
+


Can you add this newline in the original commit?

tristan957 · 2023-11-14T17:03:54Z

src/utils.ts

@@ -79,7 +79,10 @@ export async function getTargetName(target: Target) {
  const layout = await _layoutPromise;

  if (layout === "mirror") {
-    let relativePath = path.relative(vscode.workspace.rootPath!, path.dirname(target.defined_in));
+    let relativePath = path.relative(


Can you make this const?

tristan957 · 2023-11-14T17:04:24Z

src/tests.ts

@@ -127,7 +128,7 @@ export async function testDebugHandler(
      name: `meson-debug-${test.name}`,
      type: debugType,
      request: "launch",
-      cwd: test.workdir || buildDir,
+      cwd: test.workdir || sourceDir,


What is the point of this?

To make things consistent, the working directory provided by debug configurations and when running tests is now set to the "source directory" in the meson setup understanding, i.e. the directory where the meson.build file resides.

It makes little sense to me to have different behavior when you run the tests and when you run the program through debug providers. If having the specific working directory for tests is important to the user, there's the test.workdir for that.

That would make sense to me, but other people may run the tests from the builddir. I don't know. I am fine accepting the patch

To be clear, I don't have a strong preference for how this should work, but it seems better to make it consistent across the board.

tristan957 · 2023-11-14T17:05:45Z

src/extension.ts

+        if (selection) {
+          if (mesonFiles[selection.index] !== mesonFile) {
+            vscode.commands.executeCommand("workbench.action.reloadWindow");
+          }
+        }


Should we just move this into the try block? Also, let's just combine the two boolean checks into 1.

Here I was mirroring this section:

vscode-meson/src/extension.ts

Lines 318 to 326 in 68f18fb

let taskItem;

try {

taskItem = await pickTask(mode);

} catch (err) {

// Pick cancelled.

}

if (taskItem != null) {

runTask(taskItem.task);

}

I'm not sure if the showQuickPick method is even able to throw. The documentation states that it returns "a promise that resolves to the selected items or undefined" and does not mention any error handling.

Yeah, the comment is wrong. The following does not work:

I have removed the try/catch section and added code do prevent reconfigure running if selection was cancelled.

For more information about VS Code mementos take a look at: https://www.chrishasz.com/blog/2020/07/28/vscode-how-to-use-local-storage-api/

wolfpld · 2023-11-14T18:23:15Z

All of your notes should now be addressed.

tristan957 · 2023-11-14T21:36:09Z

Looks like the build step failed. Can you see about fixing it?

wolfpld · 2023-11-14T21:42:40Z

The failure seems to be a fluke in the CI/CD system, not something related to the code itself. I don't think I have any way to rerun the test.

wolfpld · 2023-11-14T22:03:25Z

I force pushed the last commit with an updated date to rerun the checks. Now they all pass.

wolfpld mentioned this pull request Nov 7, 2023

Use CMake to compile the profiler wolfpld/tracy#655

Closed

tristan957 reviewed Nov 14, 2023

View reviewed changes

wolfpld added 10 commits November 14, 2023 18:29

Fix regression introduced in 60dc5f7.

8cf1371

Export calculated build dir as a workspace memento.

81d0b8f

For more information about VS Code mementos take a look at: https://www.chrishasz.com/blog/2020/07/28/vscode-how-to-use-local-storage-api/

Do not recalculate build dir.

926f9d8

Add function calculating build dir relative to meson file location.

afa845e

Find out where the meson file is and use it to get build dir location.

5478f01

Provide meson file location to meson setup.

0562eb7

Use source dir instead of root path.

deea33e

Run tests in source dir, not in the build dir.

a332687

Allow picking which build configuration should be used.

ea5affd

Remember user configuration selection from before reload.

8a74908

Do not run meson setup if meson.build selection was cancelled.

7451671

tristan957 merged commit 51e7d5f into mesonbuild:main Nov 14, 2023
5 checks passed

xclaesse mentioned this pull request Nov 16, 2023

Sort meson.build selection by directory depth first. #188

Closed

This was referenced Mar 22, 2024

[feature request] Support sourcedir beneath the workspace root for meson setup #101

Closed

Allow specifying a source directory in configuration #22

Closed

zenshosan mentioned this pull request May 15, 2024

Switch from multiple builddir to required builddir #233

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable storage of meson.build in a non-root directory in the workspace #179

Enable storage of meson.build in a non-root directory in the workspace #179

wolfpld commented Nov 3, 2023

tristan957 commented Nov 6, 2023

wolfpld commented Nov 13, 2023

wolfpld commented Nov 14, 2023

tristan957 commented Nov 14, 2023

wolfpld commented Nov 14, 2023

tristan957 Nov 14, 2023

tristan957 Nov 14, 2023

wolfpld Nov 14, 2023

tristan957 Nov 14, 2023

wolfpld Nov 14, 2023

wolfpld Nov 14, 2023

tristan957 Nov 14, 2023

tristan957 Nov 14, 2023

wolfpld Nov 14, 2023

tristan957 Nov 14, 2023

tristan957 Nov 14, 2023

tristan957 Nov 14, 2023

tristan957 Nov 14, 2023

wolfpld Nov 14, 2023

tristan957 Nov 14, 2023

wolfpld Nov 14, 2023

tristan957 Nov 14, 2023

wolfpld Nov 14, 2023

wolfpld Nov 14, 2023

wolfpld Nov 14, 2023

wolfpld commented Nov 14, 2023

tristan957 commented Nov 14, 2023

wolfpld commented Nov 14, 2023

wolfpld commented Nov 14, 2023

	let taskItem;
	try {
	taskItem = await pickTask(mode);
	} catch (err) {
	// Pick cancelled.
	}
	if (taskItem != null) {
	runTask(taskItem.task);
	}

Enable storage of meson.build in a non-root directory in the workspace #179

Enable storage of meson.build in a non-root directory in the workspace #179

Conversation

wolfpld commented Nov 3, 2023

tristan957 commented Nov 6, 2023

wolfpld commented Nov 13, 2023

wolfpld commented Nov 14, 2023

tristan957 commented Nov 14, 2023

wolfpld commented Nov 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wolfpld commented Nov 14, 2023

tristan957 commented Nov 14, 2023

wolfpld commented Nov 14, 2023

wolfpld commented Nov 14, 2023