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

Resolves mesonPath to program absolute path #206

Merged
merged 2 commits into from
Nov 29, 2023
Merged

Resolves mesonPath to program absolute path #206

merged 2 commits into from
Nov 29, 2023

Conversation

xclaesse
Copy link
Member

ProcessExecution() does not lookup into PATH.

Fixes: #181

@xclaesse
Copy link
Member Author

CC @paveloom @lukester1975. Please give this a try.

@xclaesse
Resolves mesonPath to program absolute path
7e5972b 
ProcessExecution() does not lookup into PATH.

Fixes: mesonbuild#181
tristan957
tristan957 reviewed Nov 28, 2023
View reviewed changes
src/utils.ts
@@ -220,3 +222,7 @@ export function whenFileExists(ctx: vscode.ExtensionContext, file: string, liste
listener();
}
}

export function mesonProgram(): string {
return which.sync(extensionConfiguration("mesonPath"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this a variable instead? Otherwise, we are calling this a lot.

Copy link
Member Author

Choose a reason for hiding this comment

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

It calls it only once when creating tasks, but calls again if we refresh tasks in case the option changed.

@tristan957
Copy link
Contributor

@ZZH-Finzlize can you also try this PR?

@ZZH-Finalize
Copy link

@tristan957 Yes, I tried this PR, it can solve #203 as well, and I noticed that this PR will call meson program by absolute path

Test result:
image

@tristan957 tristan957 merged commit 4132e78 into mesonbuild:main Nov 29, 2023
5 checks passed
@lukester1975
Copy link
Contributor

Hmm. ProcessExecution does look in the path though. Nothing would have ever worked otherwise (the default for mesonPath is just meson, after all)...

In fact, trying direnv and before any changes related to #181, it picks up meson from .envrc just fine for me. Maybe the initial issue from #181 was just casued by a misconfiguration?

@paveloom
Copy link
Contributor

There is hardly anything to misconfigure. I checked and even commented on #186.

3e9cd23:

vscode-meson-before.mp4

5a103c4:

vscode-meson-after.mp4

@lukester1975
Copy link
Contributor

I guess I may have run vscode from the already direnved environment such that the PATH was correct, without really needing the direnv plugin to do anything. Maybe ProcessExecution only sees the pre-direnved environment's PATH...

Anyway no matter since this latest iteration does the job, was just more curious about what was going on!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tristan957 tristan957 tristan957 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Meson isn't picked up from the PATH
5 participants
@xclaesse @tristan957 @ZZH-Finalize @lukester1975 @paveloom