FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Provide more info about failuers to load CLR assemblies by lostmsu · Pull Request #1076 · pythonnet/pythonnet · GitHub

/ pythonnet Public

Provide more info about failuers to load CLR assemblies - #1076

Merged
lostmsu merged 1 commit into
pythonnet:masterfrom
losttech:PR/ImportErrorImprovement
Aug 19, 2020
ConversationCommits (1)ChecksFiles changed
Merged

Provide more info about failuers to load CLR assemblies#1076
lostmsu merged 1 commit into
pythonnet:masterfrom
losttech:PR/ImportErrorImprovement

Conversation

lostmsu commented Mar 4, 2020

Copy link
Copy Markdown
Member

What does this implement/fix? Explain your changes.

  1. When trying to implicitly load assemblies, and that fails not because an assembly is missing, but because loading failed for some reason, emit Python warning.
  2. When trying to import a module in our import hook, if the module name is an assembly name, and CLR fails to load it, and Python also fails to find a module with the same name, add the exceptions we got from both during the attempt into __cause__ attribute of the final ImportError.

BREAKING: clr.AddReference will now throw exceptions besides FileNotFoundException.

Additional: a few uses of BorrowedReference

Does this close any currently open issues?

This addresses #261
It is an alternative to #298

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example

lostmsu force-pushed the PR/ImportErrorImprovement branch from 645687e to 4e1ea84 Compare March 4, 2020 02:47
lostmsu mentioned this pull request Mar 4, 2020
Closed
lostmsu force-pushed the PR/ImportErrorImprovement branch 2 times, most recently from ccf5203 to e6610eb Compare April 27, 2020 21:35
lostmsu force-pushed the PR/ImportErrorImprovement branch from e6610eb to 46a51d4 Compare May 13, 2020 21:05

codecov-io commented May 14, 2020

Copy link
Copy Markdown

Codecov Report

Merging #1076 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1076   +/-   ##
=======================================
  Coverage   86.66%   86.66%           
=======================================
  Files           1        1           
  Lines         300      300           
=======================================
  Hits          260      260           
  Misses         40       40
Flag Coverage Δ
#setup_linux 65.33% <ø> (ø)
#setup_windows 72.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72fae73...46a51d4. Read the comment docs.

lostmsu added this to the 3.0.0 milestone May 14, 2020
filmor reviewed Aug 15, 2020
View reviewed changes
Comment thread src/runtime/assemblymanager.cs Outdated
if (Path.IsPathRooted(name))
{
if (!Path.HasExtension(name))
if (!Path.HasExtension(name) && Runtime.InteropVersion < new Version(3, 0))

filmor Aug 15, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If this was for compatibility, you can probably remove it.

filmor commented Aug 15, 2020

Copy link
Copy Markdown
Member

Fine by me, but please add a changelog entry for the breaking change.

lostmsu force-pushed the PR/ImportErrorImprovement branch 4 times, most recently from 58ef45f to 37df647 Compare August 18, 2020 20:43
do not eat all exceptions when trying to add CLR reference, provide m…
6eb9258 
…ore info when CLR assemblies are failed to be loaded

1. When trying to implicitly load assemblies, and that fails NOT because an assembly is missing, but because loading failed for some reason, emit Python warning.
2. When trying to import a module in our import hook, if the module name is an assembly name, and we fail to load it, and Python also fails to find a module with the same name, add the exceptions we got during the attempt to load it into __cause__ of the final ImportError

BREAKING: clr.AddReference will now throw exceptions besides FileNotFoundException.

Additional: a few uses of BorrowedReference

This addresses pythonnet#261
It is an alternative to pythonnet#298
lostmsu force-pushed the PR/ImportErrorImprovement branch from 37df647 to 6eb9258 Compare August 18, 2020 21:14

lostmsu commented Aug 18, 2020

Copy link
Copy Markdown
Member Author

I dropped the update, that tried to remove legacy "CLR." module from this change, as it started growing in scope, and is not directly related to this change.

codecov-commenter commented Aug 18, 2020 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #1076 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1076   +/-   ##
=======================================
  Coverage   86.25%   86.25%           
=======================================
  Files           1        1           
  Lines         291      291           
=======================================
  Hits          251      251           
  Misses         40       40
Flag Coverage Δ
#setup_linux 64.94% <ø> (ø)
#setup_windows 72.50% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e1fc2e...6eb9258. Read the comment docs.

lostmsu merged commit 5b989cb into pythonnet:master Aug 19, 2020
lostmsu deleted the PR/ImportErrorImprovement branch August 19, 2020 06:29
filmor mentioned this pull request Feb 15, 2021
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

filmor filmor approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL