Skip to content

Add mapping from win build paths to WSL paths#700

Closed
JanKrivanek wants to merge 2 commits into
VerifyTests:mainfrom
JanKrivanek:proto/wsl-support
Closed

Add mapping from win build paths to WSL paths#700
JanKrivanek wants to merge 2 commits into
VerifyTests:mainfrom
JanKrivanek:proto/wsl-support

Conversation

@JanKrivanek
Copy link
Copy Markdown
Contributor

Problem

Running under WSL crashes due to mixing windows and subsystem mapped files
#654

Proposed solution

Added prototype mapper function mapping the build time paths to mapped WSL paths.
Utility would need to be called everywhere where build time paths are being used (so AttributeReader and all methods using CallerFilePathAttribute)

What is missing

Adding IoHelpers.GetMappedBuildPath to all methods using CallerFilePathAttribute, only few sample usages were treated (so that debugging of WithDirectory test could be performed)

@SimonCropp
Copy link
Copy Markdown
Member

can IoHelpers.GetMappedBuildPath be placed deeper in the stack?

how do we test this in CI?

@JanKrivanek
Copy link
Copy Markdown
Contributor Author

Good questions - I do not have good answers though.

Deeper down the stack would require replacing of CallerFilePathAttribute functionality (that's detected and value injected via roslyn compiler) with custom implementation. That would probably mean harvesting the info from StackTrace utils - this wouldn't however work in release builds.

CI testing - probably just a bit more abstracting and unit testing path replacing cases (and keeping identity for local run), but no easy test guaranteeing proper functionality on WSL.

@SimonCropp
Copy link
Copy Markdown
Member

SimonCropp commented Nov 4, 2022

Deeper down the stack would require replacing of CallerFilePathAttribute functionality (that's detected and value injected via roslyn compiler) with custom implementation. That would probably mean harvesting the info from StackTrace utils - this wouldn't however work in release builds.

but at the time GetMappedBuildPath is called the CallerFilePathAttribute has already been injected into the sourceFile variable. that value will now be correct no matter where in the stack it is used

@JanKrivanek
Copy link
Copy Markdown
Contributor Author

that value will now be correct no matter where in the stack it is used

Can you please elaborate more? Do you suggesting we should have a different attribute that would take care about providing properly mapped path behind the sceenes?

The CallerFilePathAttribute is known to the compiler and that will take care about injecting the value (https://github.com/dotnet/roslyn/blob/795612c361a8d5d9efde45f5f00da214e39a9659/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs#L1393-L1396). Without the compiler support we cannot do any better - so the CallerFilePathAttribute still needs to be used. However that's not providing correct path in case of remote debugging (WSL). So in that case we need to explicitly remap somewhere in the executing code the remapping would need to be called explicitly.

But I may have very likely just misunderstood you.

Anyway - I just wanted to dump my hack to make WSL working as a sample code for possible reference in case anyone would want to play with supporting WSL remote debug scenario - not sure if worth pursuing further.

@SimonCropp
Copy link
Copy Markdown
Member

not sure if worth pursuing further.

i think it is worth pursuing

@JanKrivanek
Copy link
Copy Markdown
Contributor Author

i think it is worth pursuing

I feel I'm in need of other opinions and ideas here :-)

Just to quickly brainstorm - I can imagine following ways of solving the paths mapping:

  • As close to actual usage of directory info as possible (which is probably ResolveDirectory, and that might be it).
    • Advantages: hopefully less clutter and centralizing dir resolving logic to single location
    • Disadvantages
      • System.IO.Path utility won't work properly (splits on current platform dir separator char only) - so will need to be replaced with custom logic.
      • Custom injectable logic (e.g. DerivePathInfo) might need to handle this situation as well
  • As close to value being passed to the execution as possible (so the original approach of this PR)
    • Advantages: Simple to implement, lower risk for hidden unexpected behavior (caused by downstream logic performing platform specific logic on paths that are comming from different platform)
    • Disadvantages: mapping logic needs to be called from all Verify entrypoints (maintanence concerns)
  • Runtime 'magic' to map the path early, but without (or with low) engagement from the actual code of Verify entrypoints. The feasibilty would need to be investigated
    • Advantages: less clutter code, less possibilities for screw-up
    • Disadvanateges: feasibility?, obfuscated readability

Did you imagine any other way? Or did you had any of those in mind? Any prefference?

Thinking about this topic - I tend to prefer the first option. But trying to see other ideas before trying to stitch something together.

@JanKrivanek
Copy link
Copy Markdown
Contributor Author

Superseded by #729

@JanKrivanek JanKrivanek closed this Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants