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

Fix native library sample #270

Merged
merged 6 commits into from
Oct 27, 2020
Merged

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Oct 26, 2020

No description provided.

@jkotas
Copy link
Member Author

jkotas commented Oct 26, 2020

--require-defined seems to be a better fit for fixing this than the whole-archive.

@@ -63,10 +63,6 @@ The .NET Foundation licenses this file to you under the MIT license.
<NativeLibrary Include="crypt32.lib" />
</ItemGroup>

<PropertyGroup>
<IlcProcessEntrypoint Condition="$(IlcProcessEntrypoint) == ''">wmainCRTStartup</IlcProcessEntrypoint>
Copy link
Member Author

Choose a reason for hiding this comment

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

I have started by introducing another variable for the SharedLib init entrypoint, but then decided against it as unnecessary abstraction. I think we should wait for a real case why it is useful.

Copy link
Member

Choose a reason for hiding this comment

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

It would be useful for the @kant2002 inspired project file of my Doom fire effect: https://github.com/MichalStrehovsky/sharpfire/blob/fee39991c22b39d263933d0fd1ec25d585d82e6e/nogui.csproj#L4 (that's why I added option to set your own entrypoint when I was messing with it).

I had to avoid specifying WinExe because then the entrypoint gets set up from here and breaks things.

Arguably I never ended up using it because I have better things to do than maintaining my old hacks :).

Two weeks after the hack I learned about /ALTERNATENAME that can be used for this too I guess.

I don't particularly care, just wanted to share the history.

Copy link
Member

Choose a reason for hiding this comment

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

Actually we might be able to use /ALTERNATENAME to remove the need for the /ENTRY line.

Something like /ALTERNATENAME:wWinMainCRTStartup=wmainCRTStartup should also do the trick and we don't even need to condition it on WinExe.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have left it as is. The ALTERNATENAME trick is neat, but i did not look like an improvement to me.

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

Thank you!

@jkotas jkotas added the area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation label Oct 26, 2020
@jkotas jkotas merged commit cb591ee into dotnet:feature/NativeAOT Oct 27, 2020
@jkotas jkotas deleted the fix-library branch October 27, 2020 00:31
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this pull request Jul 21, 2023
Undoes workaround from dotnet/runtimelab#270. Since bootstrapper no longer ships as a static library (it's an object file instead), we should no longer need this hack to force linker into looking at the archive.
MichalStrehovsky added a commit to dotnet/runtime that referenced this pull request Jul 25, 2023
Undoes workaround from dotnet/runtimelab#270. Since bootstrapper no longer ships as a static library (it's an object file instead), we should no longer need this hack to force linker into looking at the archive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants