Skip to content

Fix ilasm managed resource lookup on Linux. #42735

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

Merged
merged 3 commits into from
Oct 8, 2020
Merged

Fix ilasm managed resource lookup on Linux. #42735

merged 3 commits into from
Oct 8, 2020

Conversation

MarkpageBxl
Copy link
Contributor

ILAsm did not properly find managed resources if they were not in the
working directory of ILAsm itself. While there was a provision for
Windows-based systems using backslashes as directory separators, there
was no such provision for *nix-based systems using forward slashes.

This commit enables ILAsm to lookup using both types of directory
separators.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 25, 2020
@jkotas jkotas added area-ILTools-coreclr and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Sep 25, 2020
@runfoapp runfoapp bot mentioned this pull request Sep 29, 2020
@janvorli
Copy link
Member

@mlindstr thank you for your contribution. I have noticed that there are four other places in ilasm that use \\ as the only directory separator. So I was wondering if you would mind fixing those too as part of this PR.

ILAsm did not properly find managed resources if they were not in the
working directory of ILAsm itself. While there was a provision for
Windows-based systems using backslashes as directory separators, there
was no such provision for *nix-based systems using forward slashes.

This commit enables ILAsm to lookup using both types of directory
separators.
This fixes path handling on *nix targets.
@dnfadmin
Copy link

dnfadmin commented Oct 4, 2020

CLA assistant check
All CLA requirements met.

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for your contribution!

@janvorli janvorli closed this Oct 7, 2020
@janvorli janvorli reopened this Oct 7, 2020
@janvorli
Copy link
Member

janvorli commented Oct 8, 2020

The "Libraries Test Run checked coreclr Linux x64 Debug" failed due to unrelated #43166

@janvorli janvorli merged commit f7d251f into dotnet:master Oct 8, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants