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

Remove framework overrides #560

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Remove framework overrides #560

merged 1 commit into from
Jun 3, 2024

Conversation

Shane32
Copy link
Contributor

@Shane32 Shane32 commented Jun 2, 2024

Summary

The QRCoder solution won't build on my Windows 11 laptop with a fresh installation of Visual Studio 2022, even with the .NET Framework 3.5 development tools installed. It seems to be related to the framework override csproj properties. It builds fine when I remove them.

Can we remove these properties?

Test plan

Ensure it builds without these excess properties, both locally and in CI.

@codebude
Copy link
Owner

codebude commented Jun 2, 2024

The framework overrides were added a good 4 years ago by @csturm83 via 0d08405. Maybe he can still remember why.

Unfortunately, I no longer know. I can only assume that it was due to the build pipeline at the time via MyGet, where the .NET framework was not in the default path. But I'm not 100 percent sure. If csturm83 doesn't veto it, we can merge it.

@csturm83
Copy link
Contributor

csturm83 commented Jun 2, 2024

@codebude @Shane32 It was a workaround documented in #189 (comment). If the workaround is no longer needed to publish a 3.5 target in the current build pipeline, feel free to remove.

@Shane32
Copy link
Contributor Author

Shane32 commented Jun 2, 2024

How do we test if it is needed anymore? Dotnet pack from within CI?

@csturm83
Copy link
Contributor

csturm83 commented Jun 2, 2024

@Shane32 Not sure. There was an update in the comment that was referenced in the workaround (dotnet/msbuild#1333 (comment)) to reference ref assemblies via a NuGet package. You could try that if removing the overrides doesn't work.

@codebude
Copy link
Owner

codebude commented Jun 2, 2024

You could try that if removing the overrides doesn't work.

I would say it works. At least the pipeline logs say that the net35 binaries were successfully build: https://github.com/codebude/QRCoder/actions/runs/9334333660/job/25692441349?pr=560#step:5:13

@Shane32
Copy link
Contributor Author

Shane32 commented Jun 2, 2024

I had problems a while back where I was required to add the NuGet package reference to the .NET Reference Assemblies. However, I don't need to anymore, and in fact it breaks builds within the latest .NET SDK. See link below. The .NET team recommended that I remove the reference, even though there still exists MS documentation stating to include the reference for certain scenarios. Supposedly the reference is automatically included by the .NET SDK for .NET Framework builds or something. I would assume that the issue requiring the patch has since been resolved.

See: dotnet/aspnetcore#54544

@csturm83
Copy link
Contributor

csturm83 commented Jun 2, 2024

the net35 binaries were successfully build:

The original issue was surrounding dotnet pack. I suspect the issue in the SDK has since been resolved as @Shane32 suggests.

@codebude codebude merged commit 4a8b36b into codebude:master Jun 3, 2024
3 checks passed
@Shane32 Shane32 deleted the fix_build branch June 3, 2024 13:57
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.

3 participants