Introduce Blob.GetFilteredContent#534
Conversation
LibGit2Sharp/Core/Proxy.cs
Outdated
There was a problem hiding this comment.
I think in general we have been pretty lax about checking / handling the case where an UIntPtr overflows an int (how often will this happen...) - but we could check that the value we are getting back can be handled by an int and throw an exception if it doesn't...
|
@ethomson The Travis build is failing. Could you please take a look at this, please? |
LibGit2Sharp/Core/Proxy.cs
Outdated
There was a problem hiding this comment.
Could we drop this method and make the callers rather invoke git_blob_filtered_content_stream?
There was a problem hiding this comment.
@jamill it looks like we're consuming byte[] instead of Streams, would removing this hurt you?
But still, it seems like nice parity with the Content property.
There was a problem hiding this comment.
We can work with the stream as well. I think this property is still nice parity with the Content property.
|
@nulltoken This introduces a |
|
For reference, I'm open to changing the behavior of |
|
@ethomson Very nice refactoring. Thanks! One last nitpick: The travis build still seems to be unhappy. Could you please troubleshoot these failures? |
LibGit2Sharp/Blob.cs
Outdated
There was a problem hiding this comment.
The git_blob_filtered_content doc states the following
If no filters need to be applied, then the out buffer will just be populated with
a pointer to the raw content of the blob. In that case, be careful to not free the
blob until done with the buffer.
Considering the case where no filter is applied, wouldn't we streaming back garbage data when osw is eventually disposed (and the associated pointer is freed)?
There was a problem hiding this comment.
Thinking about this ^^, I wonder if we couldn't somehow reuse the RawContentStream introduced with #324.
There was a problem hiding this comment.
I don't think that there's any way to coerce this into a RawContentStream, no; we need to get the git_buf before constructing the stream. If you can think of something clever, though, I'm all ears.
|
I rebased this (and updated the stream to handle disposing the |
|
Rebased and fixed the mono failures...! |
|
Should fix #178 |
@ethomson How much do you hate this? diff --git a/LibGit2Sharp.Tests/BlobFixture.cs b/LibGit2Sharp.Tests/BlobFixture.cs
index 313be99..c35437f 100644
--- a/LibGit2Sharp.Tests/BlobFixture.cs
+++ b/LibGit2Sharp.Tests/BlobFixture.cs
@@ -1,4 +1,5 @@
-using System.IO;
+using System;
+using System.IO;
using System.Linq;
using System.Text;
using LibGit2Sharp.Tests.TestHelpers;
@@ -31,16 +32,7 @@ public void CanGetBlobAsFilteredText()
var text = blob.ContentAsText(new FilteringOptions("foo.txt"));
- ConfigurationEntry<bool> autocrlf = repo.Config.Get<bool>("core.autocrlf");
-
- if (autocrlf != null && autocrlf.Value)
- {
- Assert.Equal("hey there\r\n", text);
- }
- else
- {
- Assert.Equal("hey there\n", text);
- }
+ Assert.Equal("hey there" + Environment.NewLine, text);
}
}
@@ -140,16 +132,7 @@ public void CanReadBlobFilteredStream()
{
string content = tr.ReadToEnd();
- ConfigurationEntry<bool> autocrlf = repo.Config.Get<bool>("core.autocrlf");
-
- if (autocrlf != null && autocrlf.Value)
- {
- Assert.Equal("hey there\r\n", content);
- }
- else
- {
- Assert.Equal("hey there\n", content);
- }
+ Assert.Equal("hey there" + Environment.NewLine, content);
}
}
}
diff --git a/LibGit2Sharp/Core/GitBufStream.cs b/LibGit2Sharp/Core/GitBufStream.cs
deleted file mode 100644
index c686ae9..0000000
--- a/LibGit2Sharp/Core/GitBufStream.cs
+++ /dev/null
@@ -1,25 +0,0 @@
-using System;
-using System.IO;
-using LibGit2Sharp.Core.Handles;
-
-namespace LibGit2Sharp.Core
-{
- internal class GitBufStream : UnmanagedMemoryStream
- {
- private readonly GitBuf buf;
- private readonly ObjectSafeWrapper wrapper;
-
- internal unsafe GitBufStream(GitBuf buf, ObjectSafeWrapper wrapper) : base((byte*)buf.ptr, (int)buf.size)
- {
- this.buf = buf;
- this.wrapper = wrapper;
- }
-
- protected override void Dispose(bool disposing)
- {
- base.Dispose(disposing);
- buf.Dispose();
- wrapper.Dispose();
- }
- }
-}
diff --git a/LibGit2Sharp/Core/Proxy.cs b/LibGit2Sharp/Core/Proxy.cs
index e9be97f..bb131ac 100644
--- a/LibGit2Sharp/Core/Proxy.cs
+++ b/LibGit2Sharp/Core/Proxy.cs
@@ -76,20 +76,15 @@ public static ObjectId git_blob_create_fromfile(RepositorySafeHandle repo, FileP
public static UnmanagedMemoryStream git_blob_filtered_content_stream(RepositorySafeHandle repo, ObjectId id, FilePath path, bool check_for_binary_data)
{
var buf = new GitBuf();
- var osw = new ObjectSafeWrapper(id, repo);
+ var handle = new ObjectSafeWrapper(id, repo).ObjectPtr;
- try
- {
- Ensure.ZeroResult(NativeMethods.git_blob_filtered_content(buf, osw.ObjectPtr, path, check_for_binary_data));
- }
- catch (Exception)
- {
- osw.Dispose();
- buf.Dispose();
- throw;
- }
-
- return new GitBufStream(buf, osw);
+ return new RawContentStream(handle, h =>
+ {
+ Ensure.ZeroResult(NativeMethods.git_blob_filtered_content(buf, h, path, check_for_binary_data));
+ return buf.ptr;
+ },
+ h => (long) buf.size,
+ new[] { buf });
}
public static byte[] git_blob_rawcontent(RepositorySafeHandle repo, ObjectId id, int size)
@@ -104,7 +99,8 @@ public static byte[] git_blob_rawcontent(RepositorySafeHandle repo, ObjectId id,
public static UnmanagedMemoryStream git_blob_rawcontent_stream(RepositorySafeHandle repo, ObjectId id, Int64 size)
{
- return new RawContentStream(id, repo, NativeMethods.git_blob_rawcontent, size);
+ var handle = new ObjectSafeWrapper(id, repo).ObjectPtr;
+ return new RawContentStream(handle, NativeMethods.git_blob_rawcontent, h => size);
}
public static Int64 git_blob_rawsize(GitObjectSafeHandle obj)
diff --git a/LibGit2Sharp/Core/RawContentStream.cs b/LibGit2Sharp/Core/RawContentStream.cs
dissimilarity index 63%
index 825b097..7b788eb 100644
--- a/LibGit2Sharp/Core/RawContentStream.cs
+++ b/LibGit2Sharp/Core/RawContentStream.cs
@@ -1,30 +1,69 @@
-using System;
-using System.IO;
-using LibGit2Sharp.Core.Handles;
-
-namespace LibGit2Sharp.Core
-{
- internal class RawContentStream : UnmanagedMemoryStream
- {
- private readonly ObjectSafeWrapper wrapper;
-
- internal RawContentStream(ObjectId id, RepositorySafeHandle repo,
- Func<GitObjectSafeHandle, IntPtr> bytePtrProvider, long length)
- : this(new ObjectSafeWrapper(id, repo), bytePtrProvider, length)
- {
- }
-
- unsafe RawContentStream(ObjectSafeWrapper wrapper,
- Func<GitObjectSafeHandle, IntPtr> bytePtrProvider, long length)
- : base((byte*)bytePtrProvider(wrapper.ObjectPtr).ToPointer(), length)
- {
- this.wrapper = wrapper;
- }
-
- protected override void Dispose(bool disposing)
- {
- base.Dispose(disposing);
- wrapper.SafeDispose();
- }
- }
-}
+using System;
+using System.Collections.Generic;
+using System.IO;
+using LibGit2Sharp.Core.Handles;
+
+namespace LibGit2Sharp.Core
+{
+ internal class RawContentStream : UnmanagedMemoryStream
+ {
+ private readonly GitObjectSafeHandle handle;
+ private readonly ICollection<IDisposable> linkedResources;
+
+ internal unsafe RawContentStream(
+ GitObjectSafeHandle handle,
+ Func<GitObjectSafeHandle, IntPtr> bytePtrProvider,
+ Func<GitObjectSafeHandle, long> sizeProvider,
+ ICollection<IDisposable> linkedResources = null)
+ : base((byte*)Wrap(handle, bytePtrProvider, linkedResources).ToPointer(),
+ Wrap(handle, sizeProvider, linkedResources))
+ {
+ this.handle = handle;
+ this.linkedResources = linkedResources;
+ }
+
+ private static T Wrap<T>(
+ GitObjectSafeHandle handle,
+ Func<GitObjectSafeHandle, T> provider,
+ IEnumerable<IDisposable> linkedResources)
+ {
+ T value;
+
+ try
+ {
+ value = provider(handle);
+ }
+ catch
+ {
+ Dispose(handle, linkedResources);
+ throw;
+ }
+
+ return value;
+ }
+
+ private static void Dispose(
+ GitObjectSafeHandle handle,
+ IEnumerable<IDisposable> linkedResources)
+ {
+ handle.SafeDispose();
+
+ if (linkedResources == null)
+ {
+ return;
+ }
+
+ foreach (IDisposable linkedResource in linkedResources)
+ {
+ linkedResource.Dispose();
+ }
+ }
+
+ protected override void Dispose(bool disposing)
+ {
+ base.Dispose(disposing);
+
+ Dispose(handle, linkedResources);
+ }
+ }
+}
diff --git a/LibGit2Sharp/LibGit2Sharp.csproj b/LibGit2Sharp/LibGit2Sharp.csproj
index 6931791..620847f 100644
--- a/LibGit2Sharp/LibGit2Sharp.csproj
+++ b/LibGit2Sharp/LibGit2Sharp.csproj
@@ -74,7 +74,6 @@
<Compile Include="Core\EncodingMarshaler.cs" />
<Compile Include="PushOptions.cs" />
<Compile Include="Core\GitBuf.cs" />
- <Compile Include="Core\GitBufStream.cs" />
<Compile Include="FilteringOptions.cs" />
<Compile Include="UnbornBranchException.cs" />
<Compile Include="LockedFileException.cs" /> |
|
Sure! |
|
Updated with your fancy new |
Get the content of a blob as it would be checked out to the working directory via the filters.
|
AWE-SOME ❤️ |
|
Great stuff, thanks @nulltoken ! |
4 participants
Get the content of a blob as it would be checked out to the working directory via the filters.