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

Fixed for newer .net and .net framework and msbuilds #67

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

mitchcapper
Copy link

This fixes a few bugs (ie clean not cleaning out signed binaries), where the output binary would sometimes not take the newer signed version, avoids using reference dll's (note this part is a bit hacky it could be made more robust if it fails), and signature /runtime updates to make this work again. Several of Petar's multi-targeting commits as well.

@mitchcapper
Copy link
Author

mitchcapper commented Feb 27, 2024

Resolves #66, Resolves #65, Resolves #64, Resolves #62, Resolves #61, Resolves #58, Resolves #51, Resolves #38

</ItemGroup>
<ItemGroup>
<PackageReference Include="Mono.Cecil" Version="$(CecilVersion)" PrivateAssets="all" />
<PackageReference Include="Nerdbank.GitVersioning" Version="3.0.26" PrivateAssets="all" />
<PackageReference Include="Mono.Cecil" Version="0.11.5" PrivateAssets="all" />
Copy link

Choose a reason for hiding this comment

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

Should version be $(CecilVersion) ?

Copy link
Author

Choose a reason for hiding this comment

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

It wouldn't hurt anything but I don't think you gain anything either. The variable isn't used elsewhere and I don't see in Mono.cecil source itself.

Copy link

Choose a reason for hiding this comment

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

It's used in the None Include lines above

Copy link
Author

Choose a reason for hiding this comment

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

Good catch will update the pr with the fix

@arce-rvty
Copy link

Is it probable that this code will be merge into the master branch in the near future?

@mitchcapper
Copy link
Author

This repo hasn't been touched in 5 years, I wouldn't hold your breath. I did add @cmconti 's fix though

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