Git: track the repositories containing each commit (#884)

This modifies the Git `Repository` data structure so that for every
commit, we track the `RepoId`s of repos containing that commit. This way
we will be able to do things like hyperlink to the right url for that
commit.

`loadRepository` has been modified to set the initial `repoId`.
`mergeRepository` has been updated to ensure that it concatenates the
`repoId`s properly.
Tests were added for both cases.

The example-git snapshot has been updated accordingly.

Test plan: `yarn test --full`
This commit is contained in:
Dandelion Mané 2018-09-21 17:06:14 -07:00 committed by GitHub
parent b506d40efd
commit 3b962eacea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 138 additions and 26 deletions

View File

@ -20,7 +20,7 @@ export default function cloneAndLoadRepository(repoId: RepoId): Repository {
const tmpdir = tmp.dirSync({unsafeCleanup: true}); const tmpdir = tmp.dirSync({unsafeCleanup: true});
const git = localGit(tmpdir.name); const git = localGit(tmpdir.name);
git(["clone", cloneUrl, ".", "--quiet"]); git(["clone", cloneUrl, ".", "--quiet"]);
const result = loadRepository(tmpdir.name, "HEAD"); const result = loadRepository(tmpdir.name, "HEAD", repoId);
tmpdir.removeCallback(); tmpdir.removeCallback();
return result; return result;
} }

View File

@ -1,4 +1,30 @@
{ {
"commitToRepoId": {
"3715ddfb8d4c4fd2a6f6af75488c82f84c92ec2f": {
"sourcecred/example-git": true
},
"69c5aad50eec8f2a0a07c988c3b283a6490eb45b": {
"sourcecred/example-git": true
},
"8d287c3bfbf8455ef30187bf5153ffc1b6eef268": {
"sourcecred/example-git": true
},
"c08ee3a4edea384d5291ffcbf06724a13ed72325": {
"sourcecred/example-git": true
},
"c2b51945e7457546912a8ce158ed9d294558d294": {
"sourcecred/example-git": true
},
"c90f6424017f787bbbaf22e4082a01355546f7e3": {
"sourcecred/example-git": true
},
"d160cca97611e9dfed642522ad44408d0292e8ea": {
"sourcecred/example-git": true
},
"e8b7a8f19701cd5a25e4a097d513ead60e5f8bcc": {
"sourcecred/example-git": true
}
},
"commits": { "commits": {
"3715ddfb8d4c4fd2a6f6af75488c82f84c92ec2f": { "3715ddfb8d4c4fd2a6f6af75488c82f84c92ec2f": {
"hash": "3715ddfb8d4c4fd2a6f6af75488c82f84c92ec2f", "hash": "3715ddfb8d4c4fd2a6f6af75488c82f84c92ec2f",

View File

@ -9,9 +9,15 @@
*/ */
// @flow // @flow
import * as MapUtil from "../../util/map";
import type {GitDriver} from "./gitUtils"; import type {GitDriver} from "./gitUtils";
import type {Repository, Hash, Commit} from "./types"; import type {Repository, Commit} from "./types";
import {localGit} from "./gitUtils"; import {localGit} from "./gitUtils";
import {
repoIdToString,
type RepoId,
type RepoIdString,
} from "../../core/repoId";
/** /**
* Load a Git repository from disk into memory. The `rootRef` should be * Load a Git repository from disk into memory. The `rootRef` should be
@ -21,7 +27,8 @@ import {localGit} from "./gitUtils";
*/ */
export function loadRepository( export function loadRepository(
repositoryPath: string, repositoryPath: string,
rootRef: string rootRef: string,
repoId: RepoId
): Repository { ): Repository {
const git = localGit(repositoryPath); const git = localGit(repositoryPath);
try { try {
@ -32,18 +39,18 @@ export function loadRepository(
git(["rev-parse", "--verify", "HEAD"]); git(["rev-parse", "--verify", "HEAD"]);
} catch (e) { } catch (e) {
// No data in the repository. // No data in the repository.
return {commits: {}}; return {commits: {}, commitToRepoId: {}};
} }
const commits = findCommits(git, rootRef); const rawCommits = findCommits(git, rootRef);
return {commits: objectMap(commits)}; const commits = MapUtil.toObject(new Map(rawCommits.map((x) => [x.hash, x])));
} const repoIdString = repoIdToString(repoId);
const repoIdStringSet: () => {[RepoIdString]: true} = () => ({
function objectMap<T: {+hash: Hash}>(ts: $ReadOnlyArray<T>): {[Hash]: T} { [((repoIdString: RepoIdString): any)]: true,
const result = {};
ts.forEach((t) => {
result[t.hash] = t;
}); });
return result; const commitToRepoId = MapUtil.toObject(
new Map(rawCommits.map(({hash}) => [hash, repoIdStringSet()]))
);
return {commits, commitToRepoId};
} }
function findCommits(git: GitDriver, rootRef: string): Commit[] { function findCommits(git: GitDriver, rootRef: string): Commit[] {

View File

@ -2,6 +2,7 @@
import tmp from "tmp"; import tmp from "tmp";
import {makeRepoId, repoIdToString} from "../../core/repoId";
import {createExampleRepo} from "./example/exampleRepo"; import {createExampleRepo} from "./example/exampleRepo";
import {localGit} from "./gitUtils"; import {localGit} from "./gitUtils";
import {loadRepository} from "./loadRepository"; import {loadRepository} from "./loadRepository";
@ -25,15 +26,38 @@ describe("plugins/git/loadRepository", () => {
// In case of failure, run // In case of failure, run
// src/plugins/git/loadRepositoryTest.sh --updateSnapshot // src/plugins/git/loadRepositoryTest.sh --updateSnapshot
// to update the snapshot, then inspect the resulting changes. // to update the snapshot, then inspect the resulting changes.
expect(loadRepository(repository.path, "HEAD")).toEqual( expect(
require("./example/example-git.json") loadRepository(
); repository.path,
"HEAD",
makeRepoId("sourcecred", "example-git")
)
).toEqual(require("./example/example-git.json"));
});
it("sets the right repoId for every commit", () => {
const gitRepository = createExampleRepo(mkdtemp());
const repoId = makeRepoId("sourcecred", "example-git");
const repository = loadRepository(gitRepository.path, "HEAD", repoId);
for (const commitHash of Object.keys(repository.commits)) {
expect(Object.keys(repository.commitToRepoId[commitHash])).toEqual([
repoIdToString(repoId),
]);
}
}); });
it("processes an old commit", () => { it("processes an old commit", () => {
const repository = createExampleRepo(mkdtemp()); const repository = createExampleRepo(mkdtemp());
const whole = loadRepository(repository.path, "HEAD"); const whole = loadRepository(
const part = loadRepository(repository.path, repository.commits[1]); repository.path,
"HEAD",
makeRepoId("sourcecred", "example-git")
);
const part = loadRepository(
repository.path,
repository.commits[1],
makeRepoId("sourcecred", "example-git")
);
// Check that `part` is a subset of `whole`... // Check that `part` is a subset of `whole`...
Object.keys(part.commits).forEach((hash) => { Object.keys(part.commits).forEach((hash) => {
@ -50,7 +74,11 @@ describe("plugins/git/loadRepository", () => {
const repository = createExampleRepo(mkdtemp()); const repository = createExampleRepo(mkdtemp());
const invalidHash = "0".repeat(40); const invalidHash = "0".repeat(40);
expect(() => { expect(() => {
loadRepository(repository.path, invalidHash); loadRepository(
repository.path,
invalidHash,
makeRepoId("sourcecred", "example-git")
);
}).toThrow("fatal: bad object 0000000000000000000000000000000000000000"); }).toThrow("fatal: bad object 0000000000000000000000000000000000000000");
}); });
@ -58,8 +86,15 @@ describe("plugins/git/loadRepository", () => {
const repositoryPath = mkdtemp(); const repositoryPath = mkdtemp();
const git = localGit(repositoryPath); const git = localGit(repositoryPath);
git(["init"]); git(["init"]);
expect(loadRepository(repositoryPath, "HEAD")).toEqual({ expect(
loadRepository(
repositoryPath,
"HEAD",
makeRepoId("sourcecred", "example-git")
)
).toEqual({
commits: {}, commits: {},
commitToRepoId: {},
}); });
}); });
}); });

View File

@ -6,18 +6,23 @@ import type {Repository} from "./types";
export function mergeRepository( export function mergeRepository(
repositories: $ReadOnlyArray<Repository> repositories: $ReadOnlyArray<Repository>
): Repository { ): Repository {
const newRepository = {commits: {}}; const newCommits = {};
for (const {commits} of repositories) { const newCommitToRepoId = {};
for (const {commits, commitToRepoId} of repositories) {
for (const commitHash of Object.keys(commits)) { for (const commitHash of Object.keys(commits)) {
const existingCommit = newRepository.commits[commitHash]; const existingCommit = newCommits[commitHash];
if ( if (
existingCommit != null && existingCommit != null &&
!deepEqual(existingCommit, commits[commitHash]) !deepEqual(existingCommit, commits[commitHash])
) { ) {
throw new Error(`Conflict between commits at ${commitHash}`); throw new Error(`Conflict between commits at ${commitHash}`);
} }
newRepository.commits[commitHash] = commits[commitHash]; newCommits[commitHash] = commits[commitHash];
const newRepos = commitToRepoId[commitHash];
const existingRepos = newCommitToRepoId[commitHash] || {};
const combinedRepoIdsForCommit = {...newRepos, ...existingRepos};
newCommitToRepoId[commitHash] = combinedRepoIdsForCommit;
} }
} }
return newRepository; return {commits: newCommits, commitToRepoId: newCommitToRepoId};
} }

View File

@ -1,11 +1,14 @@
//@flow //@flow
import {makeRepoId, repoIdToString, type RepoIdString} from "../../core/repoId";
import type {Repository} from "./types"; import type {Repository} from "./types";
import {mergeRepository} from "./mergeRepository"; import {mergeRepository} from "./mergeRepository";
describe("plugins/git/mergeRepository", () => { describe("plugins/git/mergeRepository", () => {
describe("mergeRepository", () => { describe("mergeRepository", () => {
const empty: Repository = Object.freeze({commits: {}}); const empty: Repository = Object.freeze({commits: {}, commitToRepoId: {}});
const repoId1 = repoIdToString(makeRepoId("repo", "1"));
const repoId2 = repoIdToString(makeRepoId("repo", "2"));
const repository1: Repository = Object.freeze({ const repository1: Repository = Object.freeze({
commits: { commits: {
commit1: { commit1: {
@ -21,6 +24,10 @@ describe("plugins/git/mergeRepository", () => {
parentHashes: ["commit1"], parentHashes: ["commit1"],
}, },
}, },
commitToRepoId: {
commit1: {[((repoId1: RepoIdString): any)]: true},
commit2: {[((repoId1: RepoIdString): any)]: true},
},
}); });
const repository2: Repository = Object.freeze({ const repository2: Repository = Object.freeze({
commits: { commits: {
@ -37,6 +44,10 @@ describe("plugins/git/mergeRepository", () => {
parentHashes: ["commit1"], parentHashes: ["commit1"],
}, },
}, },
commitToRepoId: {
commit1: {[((repoId2: RepoIdString): any)]: true},
commit3: {[((repoId2: RepoIdString): any)]: true},
},
}); });
it("returns empty repository with no arguments", () => { it("returns empty repository with no arguments", () => {
@ -59,6 +70,20 @@ describe("plugins/git/mergeRepository", () => {
} }
} }
}); });
it("commitToRepoId tracks every repository containing each commit", () => {
const merged = mergeRepository([repository1, repository2]);
expect(merged.commitToRepoId).toEqual({
commit1: {
[((repoId2: RepoIdString): any)]: true,
[((repoId1: RepoIdString): any)]: true,
},
commit2: {[((repoId1: RepoIdString): any)]: true},
commit3: {[((repoId2: RepoIdString): any)]: true},
});
});
it("merging a repo with itself returns that repo", () => {
expect(mergeRepository([repository1, repository1])).toEqual(repository1);
});
it("throws an error if merging a repository with conflicting commits", () => { it("throws an error if merging a repository with conflicting commits", () => {
const conflictingRepository: Repository = Object.freeze({ const conflictingRepository: Repository = Object.freeze({
commits: { commits: {
@ -69,6 +94,11 @@ describe("plugins/git/mergeRepository", () => {
parentHashes: ["commit0"], parentHashes: ["commit0"],
}, },
}, },
commitToRepoId: {
commit1: {
[((repoId1: RepoIdString): any)]: true,
},
},
}); });
expect(() => expect(() =>
mergeRepository([repository1, conflictingRepository]) mergeRepository([repository1, conflictingRepository])

View File

@ -1,5 +1,6 @@
// @flow // @flow
import {makeRepoId} from "../../core/repoId";
import * as GN from "./nodes"; import * as GN from "./nodes";
import {description} from "./render"; import {description} from "./render";
import type {Repository} from "./types"; import type {Repository} from "./types";
@ -19,6 +20,9 @@ describe("plugins/git/render", () => {
parentHashes: [], parentHashes: [],
}, },
}, },
commitToRepoId: {
[exampleHash]: {[(makeRepoId("sourcecred", "example-git"): any)]: true},
},
}); });
it("commit snapshots as expected", () => { it("commit snapshots as expected", () => {

View File

@ -1,7 +1,12 @@
// @flow // @flow
import type {RepoIdString} from "../../core/repoId";
export type Repository = {| export type Repository = {|
+commits: {[Hash]: Commit}, +commits: {[Hash]: Commit},
// For every commit, track all the RepoIds of repos
// containing this commit.
+commitToRepoId: {[Hash]: {+[RepoIdString]: true}},
|}; |};
export type Hash = string; export type Hash = string;
export type Commit = {| export type Commit = {|