Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Revert "Ensure using the given executable path."#25606

Closed
MichalStrehovsky wants to merge 2 commits into
masterfrom
revert-10525-fix-5631
Closed

Revert "Ensure using the given executable path."#25606
MichalStrehovsky wants to merge 2 commits into
masterfrom
revert-10525-fix-5631

Conversation

@MichalStrehovsky
Copy link
Copy Markdown
Member

Reverts #10525.

This broke probing for .ni.exe images. If we run corerun foo.exe for something that has a foo.ni.exe, we wouldn't pick up the native image because foo.exe is considered trusted and foo.ni.exe is not. This means we would fall back to JIT. But only if foo.exe is not in CORE_ROOT. If it is, it works...

The pull request broke crossgen testing that was eventually worked around in #11272, but it's also breaking the tests in tests\src\readytorun\tests (we just didn't find out because they're falling back to JIT). We could work around there too, but this took half a day for me to investigate, so I don't want to leave the landmine there for someone else to lose a leg over (the fact that ni probing works if you put the exe and ni.exe into CORE_ROOT was a major timesink when troubleshooting).

I don't see much point in this change. It's fixing the case when someone copied something.exe to CORE_ROOT and then tries to corerun some\other\version\of\something.exe, but it doesn't fix the situation when the something.exe depends on dependency.dll - we would still pick up dependency.dll from CORE_ROOT, which is likely the wrong version. The only way to win this game is not to play it - don't put stuff in CORE_ROOT.

@maryamariyan
Copy link
Copy Markdown

Thank you for your contribution. As announced in dotnet/coreclr#27549 this repository will be moving to dotnet/runtime on November 13. If you would like to continue working on this PR after this date, the easiest way to move the change to dotnet/runtime is:

  1. In your coreclr repository clone, create patch by running git format-patch origin
  2. In your runtime repository clone, apply the patch by running git apply --directory src/coreclr <path to the patch created in step 1>

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

/azp run coreclr-ci

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@AaronRobinsonMSFT AaronRobinsonMSFT added the post-consolidation PRs which will be hand ported to dotnet/runtime label Nov 9, 2019
@maryamariyan
Copy link
Copy Markdown

Thank you for your contribution. As announced in #27549 the dotnet/runtime repository will be used going forward for changes to this code base. Closing this PR as no more changes will be accepted into master for this repository. If you’d like to continue working on this change please move it to dotnet/runtime.

@stephentoub stephentoub deleted the revert-10525-fix-5631 branch February 10, 2020 16:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-AssemblyLoader post-consolidation PRs which will be hand ported to dotnet/runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants