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

[mono] Add support for backslash escaped colon in MONO_PATH #90526

Closed

Conversation

kotlarmilos
Copy link
Member

Fixes #90422

@filipnavara
Copy link
Member

Doesn't it strip the : now?

@@ -140,6 +140,18 @@ mono_set_assemblies_path (const char* path)
assemblies_path = dest = split;
while (*split) {
char *tmp = *split;
int len = strlen(tmp);
// If the current string ends with a backslash, append the next string to it
while (len > 0 && tmp[len - 1] == '\\') {
Copy link
Member

Choose a reason for hiding this comment

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

Windows paths are expected to have backslashes, so this should probably only be executed when !Windows (or when G_SEARCHPATH_SEPARATOR_S == ':')

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's necessarily a problem but the escaping mechanism seems rather inconsistent. This detects only backslash at the end of path that was followed by :. In Windows \: is never valid. In Linux it could be but then you can write it as \\: I guess.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right, G_SEARCHPATH_SEPARATOR_S is going to be ; on Windows. That makes it a bit more messy.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the idea is to escape \\: on non-windows platforms which should allow correct path resolution. I parametrized the concat token and added #ifndef HOST_WIN32 macro. Do you see any other issues with the escaping mechanism?

@SamMonoRT
Copy link
Member

#90544 seems a cleaner approach.

@kotlarmilos
Copy link
Member Author

Closing this PR in favor of #90544.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 14, 2023
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.

The AOT compiler doesn't support a MONO_PATH with a colon
4 participants