GetStashes / GetRefLog

[jairbubbles]

Hi @kekyo, I quickly look into adding stash support. Let me know what you think.

Stashes share the same structure than the ref log so we might keep the code / primitive structure in common.

Parsing code from libgit2 can be found here: https://github.com/libgit2/libgit2/blob/9d41a3fd694d983ade53fb602a58f6df25ce0656/src/libgit2/refdb_fs.c#L1938

NB: It would be better not to use Split / SubString to avoid allocations.

[kekyo]

@jairbubbles It's wonderful! Thanks for your contribution!
I've read the stream and don't see any major issues. Please give me a couple of days to review.

[jairbubbles]

@kekyo I updated the code to also allow to get RefLog on any reference. To allow this I change a bit PrimitiveReference so that we store the full relative path.

A few other notes:

• To allow using nullability attributes I added a dev NuGet package (https://github.com/manuelroemer/Nullable), I took the first one I found
• I updated the repo used for tests to add some stashes in it.
• Do you really need to target all those frameworks? It adds some friction when developing. (FYI, Microsoft guidance : https://learn.microsoft.com/en-us/dotnet/standard/library-guidance/cross-platform-targeting)

[kekyo]

@jairbubbles Thanks for the continued improvements! I will answer a few questions:

To allow using nullability attributes I added a dev NuGet package (https://github.com/manuelroemer/Nullable), I took the first one I found

I knew of this package, but had avoided it, but since you suggested it, I checked out a bit of the contents.
I decided that the format in which the source code is directly inserted makes it very easy to handle and is not a problem.
This decision also involved the following.

---

Do you really need to target all those frameworks? It adds some friction when developing. (FYI, Microsoft guidance : https://learn.microsoft.com/en-us/dotnet/standard/library-guidance/cross-platform-targeting)

We are creating a library component that is (supposed to be) the bottom line. That is to say, not the projects that make up the release product, such as the ASP.NET web project, its middleware, or the GUI application project.

A library component project may be referenced in an environment with assumptions that we may not have imagined when we were creating this. Therefore:

• Minimize dependencies on different components, and eliminate them if possible.
• Enable a wide range of targets.
• Minimize dependencies it mights create
  (e.g., include binaries for each platform version you deem important, rather than focusing on netstandard2.0)

I believe it is extremely important. In my paid work, this has often caused me to be unable to work with the library, to give up on very important functionality, or to spend a lot of time on non-essential implementation to get around constraints.

A simple and familiar example is Newtonsoft.Json. Because it has no external dependencies and supports a wide range of platforms, we can completely eliminate any prerequisite checks or restrictions when using this package. This has been and continues to be a tremendous advantage when dealing with JSON manipulation.

Also, in my local community, I often see people who have not migrated to the latest platforms and technologies, and are simply clinging to "Windows Forms". Yes, they continue to use .NET Framework 3.5 and do not even migrate to .NET Framework 4. It would be easy to say that is stupid, but the applications they have realized are uniquely cool and have forced me to change my mind. If we force such a project to migrate to the latest technology, we will probably kill a great ideas and projects. Or it may go to a completely different technology and never come back to .NET eco systems.

Back to the point, not just this GitReader, but most of my projects (the majority of which are library components) have been relentless in their support of them.

As long as this is not going to follow Microsoft's guidance (which I just learned for the first time :)
And as long as the cost is not terribly high, I will try to follow the above assumptions as much as possible.

---

It turns out that the Nullable package is inserted source code directly into the project, similar to how the C# compiler automatically generates some definitions as internal. This would not complicate the package dependencies, so we can determine that this is not a problem.

As for the Span<T> and related methods Split, Substring, this is also welcome if the System.Memory package is backported to older platforms, but for that reason I am not going to actively use it at the moment (In library components).

To compensate for this, GitReader takes care to continue to use the data once read as a preload buffer as much as possible, and to avoid asynchronous state transitions as much as possible (some inline code looks quite useless).

[jairbubbles]

Sure I understand but I see two different things:

• Supporting very old frameworks: it's your call, net35 is a dinosaur but if you have real use case, go for it!
• Supporting every single frameworks: it feels very useless except if you use specific features of each of them

I feel like you could simplify a lot by targeting something like net35;net461;netstandard2.0;net6.0. You would have a great compatibility with pretty much everything and the ability to use newest stuffs from net6.0.

[kekyo]

Thanks for your contribution, will review as soon as I can!
Please change the branch origin to develop (I just checked and it looks like there are no conflicts)

Sure I understand but I see two different things

Your questions are valid. Regarding the latter, including many platforms will reduce the number of library files included in the final built binary. For example, suppose we release a package that contains only netstandard2.0. If we use this in a project targeting net461, you will implicitly pull in a very large number of dependent libraries (in combination with other packages), resulting in a very large number of library files included in the published binary.

If one believes NuGet and Roslyn, then the resolution of these dependencies should have been done correctly, and I have no objection to the increase in required files. However, in my experience, I have frequently experienced dependency resolution failures in minor non-.NET Core environments such as UWP, Xamarin Forms and Unity that are not humanly understandable. Or sometimes, early System.Runtime.CompilerServices.Unsafe (resolved in the current version), or packages that depend on this package, would cause unintelligible build errors.

Whether this was a problem with the build environment, NuGet failing to resolve a complex version, a package creation error, or something else was too time consuming to analyze and I gave up trying to identify the problem.

I don't have that much in-depth knowledge of TFM or NuGet package versioning, but it seems to me that the average developer probably doesn't even know where the problem is and would not even recognize that this is caused by a complex versioning problem with TFM.

To return to the point, if the number of library files included in the final product is small (which is difficult to do in practice), the problem area can be reduced if trouble does occur. In some cases, simply examining the version of each file may provide some insight, or simply looking at it in ILSpy may solve the problem.

As I mentioned in my previous message, this is only an idea because it is a library component project, and as we move up the hierarchy of the application architecture, we will need to focus less and less on such issues, and conversely, it may well be possible to fix the target altogether. On the other hand, it is conceivable to fix the target completely.

Weigh that against supporting, say, netcoreapp2.2. Fortunately, this can be easily visualized by writing netcoreapp2.2 to see what problems are hidden there and how they can be dealt with (or how burdensome they are). I often get lost when I try to build it (in GitReader, I was lost on whether to use ValueTask or not). In such cases, I check this difference and try to support it if I feel the burden is not so great...

[jairbubbles]

If we use this in a project targeting net461, you will implicitly pull in a very large number of dependent libraries (in combination with other packages), resulting in a very large number of library files included in the published binary.

You're totally right! That's why I was proposing to keep net461. That's also suggested in the guidelines:

net35;net461;netstandard2.0 will cover all frameworks for the .NET Framework era.

For the .NET Core era, it's a bit different, netstandard2.0 is perfectly compatible for all of them. But you should target net5.0+ if you use modern APIs that are not part of this standard:

If you don't you should only target netstandard2.0:

[kekyo]

Found some minor integration point. No problem, It would be faster for me to fix those, so I will modify them here.

[kekyo]

@jairbubbles Thank you again, I am sorry for my selfishness. In my discussion with you, I realized that I fundamentally do not trust the package version resolution mechanism of NuGet/.NET. That is an idea that comes from my practical experience, and I am confident that your previous points are well understood and not wrong!

On that note, maybe I'm not talking anywhere near you, but I'm a big fan of Sam Fisher and I fight it daily on Aurora Island, New York City and Washington DC. Thanks for the great experience!

[jairbubbles]

🥳

I fundamentally do not trust the package version resolution mechanism of NuGet/.NET

Nuget has some subtleties but the TFM resolution is very simple. Apart from the .NET Standard mess , I have not been hit by any issues.

Thanks for the great experience!

I have little to do in those but Breakpoint is the last game I directly worked on!