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

add net471 target #1058

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add net471 target #1058

wants to merge 1 commit into from

Conversation

vashek
Copy link

@vashek vashek commented May 6, 2023

This is to take advantage of #1055

@drewnoakes
Copy link
Member

This looks good in theory, but I wonder if we can just bump the net47 target to net471 instead.

The downside is that anyone targeting net47 would then drop to net45.

The upside is we have a simpler matrix of configurations to build/ship/test.

Does anyone have any thoughts around this?

@vashek
Copy link
Author

vashek commented May 9, 2023

TBH I don't have an opinion either way, I don't consider myself any sort of expert on .NET and my experience with it is somewhat limited. I just find that taking advantage of the fix from #1055 - which happens to be essential for me and came at just the right moment, so huge thanks to @jonorossi - requires rebuilding NetMQ when the app targets .Net Framework higher than 4.7, so it would be quite convenient to have 4.7.1 (or I guess 4.8, in my case) included in the nuget package.

My backstory is that there's a proprietary app that uses NetMQ 3.3.3.4 and targets .Net Framework 4.8, and was made with little consideration for anything other than Windows on Intel, and I'm exploring making it run on a Raspberry using Mono. As is, it exhibits a native crash due to rdtsc. I found #1055 and so I tried a drop-in replacement of the NetMQ.dll with the 4.0.1.12 version, not really expecting to run at all (without rebuilding the app) - it ran, but with the same crash. So I rebuilt NetMQ for 4.7.1, replaced the DLL and voilà, it seems to run (or at least doesn't obviously crash, so far).

@vashek vashek marked this pull request as ready for review May 12, 2023 07:23
@vashek
Copy link
Author

vashek commented May 12, 2023

Marked as ready to maybe grab more attention. ;)
Regarding the failed test I wonder if that's just a spurious error, something timed out in netcoreapp3.1, specifically NetMQ.Tests.ClientServer.AsyncEnumerableCanceled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants