Skip to content

Generate a unique ID at compile time to work around a CLR bug #438

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

Closed
wants to merge 3 commits into from
Closed

Generate a unique ID at compile time to work around a CLR bug #438

wants to merge 3 commits into from

Conversation

phkelley
Copy link
Member

Alternative to #437 that generates a GUID at compile time and makes it accessible as a const String. The String is then used as the value for MarshalCookie on all custom marshaler declarations. The MarshalCookie is used in the hash key by the CLR when caching ICustomMarshalers. This will allow multiple LibGit2Sharp assemblies to co-exist and use custom marshalers.

The build task is not as clean as I would like it to be, because XBuild is limited in its functionality in a few ways. (We can't use inline C# code, for example, with the CodeTaskFactory.) I would prefer not to have the .DLL be checked into the repository, but it looks like the only way to get what we want here. Incremental build still works, too.

@dahlbyk
Copy link
Member

dahlbyk commented May 30, 2013

Too bad MarshalAsAttribute is sealed...wouldn't have to rely on a compile-time constant.

@phkelley
Copy link
Member Author

I agree that is unfortunate. But the CLR is using inside knowledge of the structure of the MarshalAsAttribute in building its cache of ICustomMarshaler instances. For the purposes of the cache, it knows about MarshalType, MarshalTypeRef, and MarshalCookie, and includes those in its cache key. The problem is that the MarshalType and MarshalTypeRef entries are not properly assembly-qualified :-(

@phkelley
Copy link
Member Author

@nulltoken Not sure where your feedback went, but I received it via e-mail and did both things.

  • Removed CustomBuildTasks from the solution
  • Moved the drop of CustomBuildTasks into the Lib directory with all the other checked-in binaries

@nulltoken
Copy link
Member

@phkelley I've also been working on this. I've merged your work with mine in nulltoken/marshal_cookie.

Can you please review my proposal? Unless I'm wrong, applying the following patch doesn't have any noticeable side effect.

diff --git a/LibGit2Sharp/UniqueIdentifier.targets b/LibGit2Sharp/UniqueIdentifier.targets
index ef65ddf..c3fed1d 100644
--- a/LibGit2Sharp/UniqueIdentifier.targets
+++ b/LibGit2Sharp/UniqueIdentifier.targets
@@ -13,14 +13,11 @@

   <!-- This target runs if any of the projects or .cs files for the project have changed since the last time
       the UniqueIdentifier.cs file was generated. -->
-  <Target Name="GenerateUniqueIdentifierCs"
-          Inputs="$(MSBuildThisFileFullPath);$(MSBuildAllProjects);@(Compile)"
-          Outputs="$(UniqueIdentifierPath)">
+  <Target Name="GenerateUniqueIdentifierCs">
     <GenerateUniqueIdentifierTask OutputFile="$(UniqueIdentifierPath)" />
   </Target>

   <Target Name="CleanUniqueIdentifierCs">
     <Delete Files="$(UniqueIdentifierPath)" />
   </Target>
-
 </Project>

@anaisbetts
Copy link
Contributor

I tried @nulltoken's version of this fix (nulltoken/libgit2sharp@2824450) and it is 💎 💎 💎, you guys are awesome!

@phkelley
Copy link
Member Author

Thanks @nulltoken. The difference there is that there is no incremental build if you don't put Inputs and Outputs on the Target. Adding the Inputs and Outputs makes the Target only run when there exists an item in the Inputs set that is newer than the UniqueIdentifier.cs file. (Inputs and Outputs can only make the target run less frequently, not more.)

So with your change, every time you hit Build in Visual Studio, you'll get a new LibGit2Sharp.dll with a new GUID for the UniqueIdentifier, even if nothing else has changed. With the Inputs and Outputs specified, you can run Build twice, and the second time it will not rebuild LibGit2Sharp.dll because it does not need to be rebuilt.

The goal I was going for was to make UniqueIdentifier.cs only be regenerated when we already were going to recompile -- because some .csproj had changed, or some .cs file had changed.

Either one is fine though!

@nulltoken
Copy link
Member

@phkelley Thanks a for for this very well explained answer! ❤️

I manually merged this, including the changes required to make UniqueIdentifier.cs regenerated when there's actually something to recompile.

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.

4 participants