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

Remove unnecessary task to set BAZEL_EXEC_ROOT #313

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Oct 29, 2024

It turns out that we only need sourceMap's value to be set to the workspace folder with any arbitrary key. So not setting BAZEL_EXEC_ROOT is fine.

Motivation

When reviewing #312, I found that having bazel's execroot being the source map key is actually unnecessary. What's required seems to just be the value being the workspace folder. So in this PR I'm removing the task that sets BAZEL_EXEC_ROOT and updating launch.json entries.

Test plan

See included automated tests.

@st0012 st0012 self-assigned this Oct 29, 2024
Copy link

@amomchilov amomchilov left a comment

Choose a reason for hiding this comment

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

Does this behave correctly for finding source files not in our workspace? I fear it makes it so our repo is used for all files, even libraries whose source code isn't in our workspace

@amomchilov amomchilov changed the title Remove unnecessary task to set BAZEL_EXEC_ROOT Remove unnecessary task to set BAZEL_EXEC_ROOT Oct 29, 2024
@st0012
Copy link
Member Author

st0012 commented Oct 29, 2024

Can you give me a scenario to verify if that's the case?

It turns out that we only need `sourceMap`'s value to be set to the
workspace folder with any arbitrary key. So not setting `BAZEL_EXEC_ROOT`
is fine.
@st0012
Copy link
Member Author

st0012 commented Nov 4, 2024

Chatted with @amomchilov and we agree to go ahead first as it's working now. We'll revise the configuration if we found some cases where it causes unwanted noise...etc.

@st0012 st0012 merged commit 289ef2e into prism Nov 4, 2024
1 check passed
@st0012 st0012 deleted the simplify-debugging-actions branch November 4, 2024 18:46
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