Skip to content

Commit

Permalink
Improve binary detection
Browse files Browse the repository at this point in the history
libgit2 optimizes around not loading the content, when there's no
content callbacks configured.

So we aren't able to get a clear answer from radicle-surf if a file in a
diff is a binary or not.

A simple way without doing additional API requests is to check if the
file type is a executable and if there are no hunks (since we don't get
any deltas in binaries), this is a strong
indicator for a binary.

Signed-off-by: Sebastian Martinez <me@sebastinez.dev>
  • Loading branch information
sebastinez committed Sep 26, 2023
1 parent 9b15911 commit bd6ba78
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 5 deletions.
2 changes: 2 additions & 0 deletions httpd-client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
CommitHeader,
Diff,
DiffContent,
DiffFile,
HunkLine,
} from "./lib/project/commit.js";
import type { Issue, IssueState } from "./lib/project/issue.js";
Expand Down Expand Up @@ -46,6 +47,7 @@ export type {
CommitHeader,
Diff,
DiffContent,
DiffFile,
DiffResponse,
HunkLine,
Issue,
Expand Down
15 changes: 14 additions & 1 deletion httpd-client/lib/project/commit.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import type { z } from "zod";
export type { Commits, HunkLine, Commit, Diff, CommitHeader, DiffContent };
export type {
Commits,
HunkLine,
Hunks,
Commit,
Diff,
CommitHeader,
DiffFile,
DiffContent,
};

import { array, literal, number, object, optional, string, union } from "zod";
export { commitHeaderSchema, diffSchema, commitSchema, commitsSchema };
Expand Down Expand Up @@ -36,6 +45,8 @@ const deletionHunkLineSchema = object({
type: literal("deletion"),
});

type DiffFile = z.infer<typeof diffFileSchema>;

const diffFileSchema = object({
oid: string(),
mode: union([
Expand Down Expand Up @@ -64,6 +75,8 @@ const hunkLineSchema = union([
contextHunkLineSchema,
]);

type Hunks = z.infer<typeof changesetHunkSchema>;

const changesetHunkSchema = object({
header: string(),
lines: array(hunkLineSchema),
Expand Down
23 changes: 19 additions & 4 deletions src/views/projects/Changeset.svelte
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
<script lang="ts">
import type { BaseUrl, CommitBlob, Diff } from "@httpd-client";
import type {
BaseUrl,
Diff,
CommitBlob,
DiffContent,
DiffFile,
} from "@httpd-client";
import { pluralize } from "@app/lib/pluralize";
Expand Down Expand Up @@ -42,6 +48,15 @@
}
return s.join(", ");
};
// Since libgit2 optimizes around not loading the content, when no content callbacks are configured,
// and we don't want to loop over all diffs in radicle-surf to get a correct answer.
// We assume that a `blobExecutable` file with no hunks is a binary file.
function getFileType(diff: DiffContent, file: DiffFile): DiffContent["type"] {
return file.mode === "blobExecutable" && diff.hunks.length === 0
? "binary"
: diff.type;
}
</script>

<style>
Expand Down Expand Up @@ -77,7 +92,7 @@
{baseUrl}
revision={revision ?? files[file.new.oid].lastCommit.id}
filePath={file.path}
fileDiff={file.diff}
fileDiff={{ ...file.diff, type: getFileType(file.diff, file.new) }}
headerBadgeCaption="added" />
{/each}
{#each diff.deleted as file}
Expand All @@ -86,7 +101,7 @@
{baseUrl}
revision={revision ?? files[file.old.oid].lastCommit.id}
filePath={file.path}
fileDiff={file.diff}
fileDiff={{ ...file.diff, type: getFileType(file.diff, file.old) }}
headerBadgeCaption="deleted" />
{/each}
{#each diff.modified as file}
Expand All @@ -95,7 +110,7 @@
{baseUrl}
revision={revision ?? files[file.new.oid].lastCommit.id}
filePath={file.path}
fileDiff={file.diff} />
fileDiff={{ ...file.diff, type: getFileType(file.diff, file.new) }} />
{/each}
{#each diff.moved as file}
{#if file.diff}
Expand Down
7 changes: 7 additions & 0 deletions tests/e2e/project/commit.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ test("copied file", async ({ page }) => {
await expect(page.getByText("copied", { exact: true })).toBeVisible();
});

test("binary file detection in diffs", async ({ page }) => {
await page.goto(
`${sourceBrowsingUrl}/commits/335dd6dc89b535a4a31e9422c803199bb6b0a09a`,
);
await expect(page.getByText("Binary file")).toBeVisible();
});

test("navigation to source tree at specific revision", async ({ page }) => {
await page.goto(
`${sourceBrowsingUrl}/commits/0801aceeab500033f8d608778218657bd626ef73`,
Expand Down

1 comment on commit bd6ba78

@vercel
Copy link

@vercel vercel bot commented on bd6ba78 Sep 26, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

radicle-interface – ./

radicle-interface-git-master-radicle.vercel.app
app.radicle.xyz
radicle-interface-radicle.vercel.app

Please sign in to comment.