replace path placeholder stored as wide string #4307
Conversation
The path placeholder may be stored in binaries in different encodings. This change adds support for UTF16 or UTF32 to the already supported UTF8 encoding. It should cover wchar_t and wxString (from wxWidgets library). Apparently this the placeholder was not substituted on Windows, and this has not changed. I only moved the call to replace_pyzzer_entry_point_shebang() out of binary_replace() to avoid calling it more than once. Removed has_pyzzer_entry_point() - it was redundant as the same check is done inside of replace_pyzzer_entry_point_shebang(). Doing binary_replace() 3 times can affect performance - no idea how much.
|
Apologies it's been a while since I've had some time to spend here. Finally getting open PRs down to a manageable level. Definitely appreciate the PR and am sympathetic to the problem being addressed. Like you I think though, not 100% yet on the implementation. I'm wondering if we could avoid three file scans by compiling a more sophisticated regex. I'm thinking something like http://stackoverflow.com/questions/15175142/how-can-i-do-multiple-substitutions-using-regex-in-python If you want to spend a little more time here, that'd be awesome. Otherwise, I'll take a crack at it when I get a chance. If you do, I'd like to target the PR to the upcoming 4.4 release. To do that, something like |
|
Thanks for looking into it. Yes, scanning files three times is not good. I won't get a chance to do anything here this week, but I'll try to get back to it at some point. Of course I don't mind if you beat me to it. |
|
Hi there, thank you for your contribution to Conda! This issue has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs. If you would like this issue to remain open please:
NOTE: If this issue was closed prematurely, please leave a comment and we will gladly reopen the issue. In case this issue was originally about a project that is covered by the Anaconda issue tracker (e.g. Anaconda, Miniconda, packages built by Anaconda, Inc. like Anaconda Navigator etc), please reopen the issue there again. Thanks! |
|
Hi again! |
Fix for #4298.
I made a patch for conda/core/portability.py. I don't know if anything else should be changed (conda-build?). I tested it only on one example, so it's not much tested.
Doing binary_replace() in three variants may affect performance - no idea how
much.
The path placeholder may be stored in binaries in different encodings.
This change adds support for UTF16 or UTF32 to the already supported
UTF8 encoding. It should cover wchar_t and wxString (from wxWidgets
library).
Apparently the placeholder was not substituted on Windows,
and this has not changed. I only moved the call to
replace_pyzzer_entry_point_shebang() out of binary_replace()
to avoid calling it more than once.
Removed has_pyzzer_entry_point() - it was redundant as the same check
is done inside of replace_pyzzer_entry_point_shebang().