From 9242cb60b60553190691d60f1ea390214d9c2e1a Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Wed, 31 Jul 2019 23:13:57 +0300 Subject: [PATCH 1/2] add git submodules support in github linking --- __tests__/lib/git/find_git.js | 20 +++++++++++++----- __tests__/lib/git/url_prefix.js | 25 ++++++++++++++++------ __tests__/utils.js | 23 ++++++++++++++++++++ package.json | 1 + src/git/find_git.js | 23 +++++++++++++------- src/git/url_prefix.js | 26 ++++++++++++++++------- src/github.js | 15 ++++++------- yarn.lock | 37 +++++++++++++++++++++++++++++++++ 8 files changed, 137 insertions(+), 33 deletions(-) diff --git a/__tests__/lib/git/find_git.js b/__tests__/lib/git/find_git.js index 8619ac986..768ddbb2e 100644 --- a/__tests__/lib/git/find_git.js +++ b/__tests__/lib/git/find_git.js @@ -5,12 +5,22 @@ const findGit = require('../../../src/git/find_git'); test('findGit', function() { mock(mockRepo.master); + const root = + path.parse(__dirname).root + path.join('my', 'repository', 'path'); + const masterPaths = findGit(path.join(root, 'index.js')); + mock.restore(); - const root = path.parse(__dirname).root; - - expect( - findGit(root + path.join('my', 'repository', 'path', 'index.js')) - ).toBe(root + path.join('my', 'repository', 'path', '.git')); + expect(masterPaths).toEqual({ + git: path.join(root, '.git'), + root: root + }); + mock(mockRepo.submodule); + const submodulePaths = findGit(path.join(root, 'index.js')); mock.restore(); + + expect(submodulePaths).toEqual({ + git: path.join(path.dirname(root), '.git', 'modules', 'path'), + root: root + }); }); diff --git a/__tests__/lib/git/url_prefix.js b/__tests__/lib/git/url_prefix.js index 0a917352f..2a7f53e75 100644 --- a/__tests__/lib/git/url_prefix.js +++ b/__tests__/lib/git/url_prefix.js @@ -5,20 +5,33 @@ const parsePackedRefs = getGithubURLPrefix.parsePackedRefs; test('getGithubURLPrefix', function() { mock(mockRepo.master); - - expect(getGithubURLPrefix('/my/repository/path/')).toBe( - 'https://github.com/foo/bar/blob/this_is_the_sha/' - ); - + const masterUrl = getGithubURLPrefix({ + git: '/my/repository/path/.git', + root: '/my/repository/path' + }); mock.restore(); + expect(masterUrl).toBe('https://github.com/foo/bar/blob/this_is_the_sha/'); + mock(mockRepo.detached); + const detachedUrl = getGithubURLPrefix({ + git: '/my/repository/path/.git', + root: '/my/repository/path' + }); + mock.restore(); - expect(getGithubURLPrefix('/my/repository/path/')).toBe( + expect(detachedUrl).toBe( 'https://github.com/foo/bar/blob/e4cb2ffe677571d0503e659e4e64e01f45639c62/' ); + mock(mockRepo.submodule); + const submoduleUrl = getGithubURLPrefix({ + git: '/my/repository/.git/modules/path', + root: '/my/repository/path' + }); mock.restore(); + + expect(submoduleUrl).toBe('https://github.com/foo/bar/blob/this_is_the_sha/'); }); test('parsePackedRefs', function() { diff --git a/__tests__/utils.js b/__tests__/utils.js index 874093df1..ed3e3ff4d 100644 --- a/__tests__/utils.js +++ b/__tests__/utils.js @@ -74,6 +74,29 @@ module.exports.mockRepo = { } } }, + submodule: { + '/my': { + repository: { + path: { + '.git': 'gitdir: ../.git/modules/path', + 'index.js': 'module.exports = 42;', + 'package.json': '{"repository": "foo/bar"}' + }, + '.git': { + modules: { + path: { + HEAD: 'ref: refs/heads/master', + refs: { + heads: { + master: 'this_is_the_sha' + } + } + } + } + } + } + } + }, malformed: { '/my': { repository: { diff --git a/package.json b/package.json index e8f0590f6..e69fe8f62 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "concat-stream": "^1.6.0", "diff": "^4.0.1", "doctrine-temporary-fork": "2.1.0", + "get-pkg-repo": "^4.1.0", "get-port": "^4.0.0", "git-url-parse": "^10.0.1", "github-slugger": "1.2.0", diff --git a/src/git/find_git.js b/src/git/find_git.js index 6a56a11f8..426b9091d 100644 --- a/src/git/find_git.js +++ b/src/git/find_git.js @@ -5,18 +5,25 @@ const fs = require('fs'); * Given a full path to a single file, iterate upwards through the filesystem * to find a directory with a .git file indicating that it is a git repository * @param filename any file within a repository - * @returns repository path + * @returns repository root & its .git folder paths */ function findGit(filename) { - const paths = filename.split(path.sep); - for (let i = paths.length; i > 0; i--) { - const p = path.resolve( - paths.slice(0, i).join(path.sep) + path.sep + '.git' - ); - if (fs.existsSync(p)) { - return p; + let root = path.resolve(filename); + while (root) { + root = path.dirname(root); + let git = path.join(root, '.git'); + if (!fs.existsSync(git)) continue; + + if (fs.statSync(git).isFile()) { + // git submodule + const matches = fs.readFileSync(git, 'utf8').match(/gitdir: (.*)/); + if (!matches) return null; + git = path.join(root, matches[1]); } + + return { root, git }; } + return null; } module.exports = findGit; diff --git a/src/git/url_prefix.js b/src/git/url_prefix.js index aedf19ae4..1f4fa5c81 100644 --- a/src/git/url_prefix.js +++ b/src/git/url_prefix.js @@ -2,6 +2,7 @@ const fs = require('fs'); const path = require('path'); const gitUrlParse = require('git-url-parse'); const getRemoteOrigin = require('remote-origin-url'); +const getPkgRepo = require('get-pkg-repo'); /** * Sometimes git will [pack refs](https://git-scm.com/docs/git-pack-refs) @@ -32,15 +33,15 @@ function parsePackedRefs(packedRefs, branchName) { * @returns {string} base HTTPS url of the GitHub repository * @throws {Error} if the root is not a git repo */ -function getGithubURLPrefix(root) { +function getGithubURLPrefix({ git, root }) { let sha; try { - const head = fs.readFileSync(path.join(root, '.git', 'HEAD'), 'utf8'); + const head = fs.readFileSync(path.join(git, 'HEAD'), 'utf8'); const branch = head.match(/ref: (.*)/); if (branch) { const branchName = branch[1]; - const branchFileName = path.join(root, '.git', branchName); - const packedRefsName = path.join(root, '.git', 'packed-refs'); + const branchFileName = path.join(git, branchName); + const packedRefsName = path.join(git, 'packed-refs'); if (fs.existsSync(branchFileName)) { sha = fs.readFileSync(branchFileName, 'utf8'); } else if (fs.existsSync(packedRefsName)) { @@ -57,9 +58,20 @@ function getGithubURLPrefix(root) { sha = head; } if (sha) { - const parsed = gitUrlParse(getRemoteOrigin.sync(root)); - parsed.git_suffix = false; // eslint-disable-line - return parsed.toString('https') + '/blob/' + sha.trim() + '/'; + let githubRootUrl; + if (git.indexOf(root) === 0) { + const origin = getRemoteOrigin.sync(root); + const parsed = gitUrlParse(origin); + parsed.git_suffix = false; // eslint-disable-line + githubRootUrl = parsed.toString('https'); + } else { + // git submodule; try looking at package.json + const repo = getPkgRepo( + JSON.parse(fs.readFileSync(path.join(root, 'package.json'))) + ); + githubRootUrl = repo.browse(); + } + return githubRootUrl + '/blob/' + sha.trim() + '/'; } } catch (e) { return null; diff --git a/src/github.js b/src/github.js index 46c497837..3edb435ad 100644 --- a/src/github.js +++ b/src/github.js @@ -10,15 +10,16 @@ const getGithubURLPrefix = require('./git/url_prefix'); * @returns {Object} comment with github inferred */ module.exports = function(comment) { - const repoPath = findGit(comment.context.file); - const root = repoPath ? path.dirname(repoPath) : '.'; - const urlPrefix = getGithubURLPrefix(root); - const fileRelativePath = comment.context.file - .replace(root + path.sep, '') - .split(path.sep) - .join('/'); + const paths = findGit(comment.context.file); + + const urlPrefix = paths && getGithubURLPrefix(paths); if (urlPrefix) { + const fileRelativePath = comment.context.file + .replace(paths.root + path.sep, '') + .split(path.sep) + .join('/'); + let startLine; let endLine; diff --git a/yarn.lock b/yarn.lock index 284f472b1..c9c6129c3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -825,6 +825,11 @@ lodash "^4.17.10" to-fast-properties "^2.0.0" +"@hutson/parse-repository-url@^3.0.0": + version "3.0.2" + resolved "https://registry.yarnpkg.com/@hutson/parse-repository-url/-/parse-repository-url-3.0.2.tgz#98c23c950a3d9b6c8f0daed06da6c3af06981340" + integrity sha512-H9XAx3hc0BQHY6l+IFSWHDySypcXsvsuLhgYLUGywmJ5pswRVQJUHpOsobnLYp2ZUaUlKiKDrgWWhosOwAEM8Q== + "@samverschueren/stream-to-observable@^0.3.0": version "0.3.0" resolved "https://registry.yarnpkg.com/@samverschueren/stream-to-observable/-/stream-to-observable-0.3.0.tgz#ecdf48d532c58ea477acfcab80348424f8d0662f" @@ -2677,6 +2682,16 @@ get-pkg-repo@^1.0.0: parse-github-repo-url "^1.3.0" through2 "^2.0.0" +get-pkg-repo@^4.1.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/get-pkg-repo/-/get-pkg-repo-4.1.0.tgz#bd2b109e297af8ec541ba271607aab439f9a8610" + integrity sha512-BHJovsEz9igoxU9Idfa9XjKr0OuAfg6/wInYegP0/M3efsdVtKo1DipPebwnTZXtL9gzaPLvJv74J/U68yiiMg== + dependencies: + "@hutson/parse-repository-url" "^3.0.0" + hosted-git-info "^2.1.4" + meow "^5.0.0" + through2 "^2.0.0" + get-port@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/get-port/-/get-port-4.0.0.tgz#373c85960138ee20027c070e3cb08019fea29816" @@ -4349,6 +4364,21 @@ meow@^4.0.0: redent "^2.0.0" trim-newlines "^2.0.0" +meow@^5.0.0: + version "5.0.0" + resolved "https://registry.yarnpkg.com/meow/-/meow-5.0.0.tgz#dfc73d63a9afc714a5e371760eb5c88b91078aa4" + integrity sha512-CbTqYU17ABaLefO8vCU153ZZlprKYWDljcndKKDCFcYQITzWCXZAVk4QMFZPgvzrnUQ3uItnIE/LoUOwrT15Ig== + dependencies: + camelcase-keys "^4.0.0" + decamelize-keys "^1.0.0" + loud-rejection "^1.0.0" + minimist-options "^3.0.1" + normalize-package-data "^2.3.4" + read-pkg-up "^3.0.0" + redent "^2.0.0" + trim-newlines "^2.0.0" + yargs-parser "^10.0.0" + merge-stream@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/merge-stream/-/merge-stream-1.0.1.tgz#4041202d508a342ba00174008df0c251b8c135e1" @@ -6711,6 +6741,13 @@ yallist@^3.0.0, yallist@^3.0.2: version "3.0.3" resolved "https://registry.yarnpkg.com/yallist/-/yallist-3.0.3.tgz#b4b049e314be545e3ce802236d6cd22cd91c3de9" +yargs-parser@^10.0.0: + version "10.1.0" + resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-10.1.0.tgz#7202265b89f7e9e9f2e5765e0fe735a905edbaa8" + integrity sha512-VCIyR1wJoEBZUqk5PA+oOBF6ypbwh5aNB3I50guxAL/quggdfs4TtNHQrSazFA3fYZ+tEqfs0zIGlv0c/rgjbQ== + dependencies: + camelcase "^4.1.0" + yargs-parser@^11.1.1: version "11.1.1" resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-11.1.1.tgz#879a0865973bca9f6bab5cbdf3b1c67ec7d3bcf4" From 82092c8a82f3fde6249e0c48c5a420bfcafb6a16 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Wed, 31 Jul 2019 23:54:28 +0300 Subject: [PATCH 2/2] more reliable submodule origin parsing --- __tests__/lib/git/find_git.js | 4 +-- __tests__/utils.js | 7 +++-- package.json | 3 +-- src/git/url_prefix.js | 24 ++++++++--------- yarn.lock | 51 +---------------------------------- 5 files changed, 21 insertions(+), 68 deletions(-) diff --git a/__tests__/lib/git/find_git.js b/__tests__/lib/git/find_git.js index 768ddbb2e..dd9886669 100644 --- a/__tests__/lib/git/find_git.js +++ b/__tests__/lib/git/find_git.js @@ -12,7 +12,7 @@ test('findGit', function() { expect(masterPaths).toEqual({ git: path.join(root, '.git'), - root: root + root }); mock(mockRepo.submodule); @@ -21,6 +21,6 @@ test('findGit', function() { expect(submodulePaths).toEqual({ git: path.join(path.dirname(root), '.git', 'modules', 'path'), - root: root + root }); }); diff --git a/__tests__/utils.js b/__tests__/utils.js index ed3e3ff4d..aec4d2c5c 100644 --- a/__tests__/utils.js +++ b/__tests__/utils.js @@ -79,10 +79,13 @@ module.exports.mockRepo = { repository: { path: { '.git': 'gitdir: ../.git/modules/path', - 'index.js': 'module.exports = 42;', - 'package.json': '{"repository": "foo/bar"}' + 'index.js': 'module.exports = 42;' }, '.git': { + config: + '[submodule "path"]\n' + + 'url = https://github.com/foo/bar\n' + + 'active = true', modules: { path: { HEAD: 'ref: refs/heads/master', diff --git a/package.json b/package.json index e69fe8f62..859d915ea 100644 --- a/package.json +++ b/package.json @@ -39,13 +39,13 @@ "concat-stream": "^1.6.0", "diff": "^4.0.1", "doctrine-temporary-fork": "2.1.0", - "get-pkg-repo": "^4.1.0", "get-port": "^4.0.0", "git-url-parse": "^10.0.1", "github-slugger": "1.2.0", "glob": "^7.1.2", "globals-docs": "^2.4.0", "highlight.js": "^9.15.5", + "ini": "^1.3.5", "js-yaml": "^3.10.0", "lodash": "^4.17.10", "mdast-util-inject": "^1.1.0", @@ -59,7 +59,6 @@ "remark-html": "^8.0.0", "remark-reference-links": "^4.0.1", "remark-toc": "^5.0.0", - "remote-origin-url": "0.4.0", "resolve": "^1.8.1", "stream-array": "^1.1.2", "strip-json-comments": "^2.0.1", diff --git a/src/git/url_prefix.js b/src/git/url_prefix.js index 1f4fa5c81..64bd4f0aa 100644 --- a/src/git/url_prefix.js +++ b/src/git/url_prefix.js @@ -1,8 +1,7 @@ const fs = require('fs'); const path = require('path'); const gitUrlParse = require('git-url-parse'); -const getRemoteOrigin = require('remote-origin-url'); -const getPkgRepo = require('get-pkg-repo'); +const ini = require('ini'); /** * Sometimes git will [pack refs](https://git-scm.com/docs/git-pack-refs) @@ -58,20 +57,21 @@ function getGithubURLPrefix({ git, root }) { sha = head; } if (sha) { - let githubRootUrl; + let origin; if (git.indexOf(root) === 0) { - const origin = getRemoteOrigin.sync(root); - const parsed = gitUrlParse(origin); - parsed.git_suffix = false; // eslint-disable-line - githubRootUrl = parsed.toString('https'); + const config = ini.parse( + fs.readFileSync(path.join(git, 'config'), 'utf8') + ); + origin = config['remote "origin"'].url; } else { - // git submodule; try looking at package.json - const repo = getPkgRepo( - JSON.parse(fs.readFileSync(path.join(root, 'package.json'))) + const config = ini.parse( + fs.readFileSync(path.join(git, '..', '..', 'config'), 'utf8') ); - githubRootUrl = repo.browse(); + origin = config[`submodule "${path.basename(git)}"`].url; } - return githubRootUrl + '/blob/' + sha.trim() + '/'; + const parsed = gitUrlParse(origin); + parsed.git_suffix = false; // eslint-disable-line + return parsed.toString('https') + '/blob/' + sha.trim() + '/'; } } catch (e) { return null; diff --git a/yarn.lock b/yarn.lock index c9c6129c3..02e1f4a60 100644 --- a/yarn.lock +++ b/yarn.lock @@ -825,11 +825,6 @@ lodash "^4.17.10" to-fast-properties "^2.0.0" -"@hutson/parse-repository-url@^3.0.0": - version "3.0.2" - resolved "https://registry.yarnpkg.com/@hutson/parse-repository-url/-/parse-repository-url-3.0.2.tgz#98c23c950a3d9b6c8f0daed06da6c3af06981340" - integrity sha512-H9XAx3hc0BQHY6l+IFSWHDySypcXsvsuLhgYLUGywmJ5pswRVQJUHpOsobnLYp2ZUaUlKiKDrgWWhosOwAEM8Q== - "@samverschueren/stream-to-observable@^0.3.0": version "0.3.0" resolved "https://registry.yarnpkg.com/@samverschueren/stream-to-observable/-/stream-to-observable-0.3.0.tgz#ecdf48d532c58ea477acfcab80348424f8d0662f" @@ -2682,16 +2677,6 @@ get-pkg-repo@^1.0.0: parse-github-repo-url "^1.3.0" through2 "^2.0.0" -get-pkg-repo@^4.1.0: - version "4.1.0" - resolved "https://registry.yarnpkg.com/get-pkg-repo/-/get-pkg-repo-4.1.0.tgz#bd2b109e297af8ec541ba271607aab439f9a8610" - integrity sha512-BHJovsEz9igoxU9Idfa9XjKr0OuAfg6/wInYegP0/M3efsdVtKo1DipPebwnTZXtL9gzaPLvJv74J/U68yiiMg== - dependencies: - "@hutson/parse-repository-url" "^3.0.0" - hosted-git-info "^2.1.4" - meow "^5.0.0" - through2 "^2.0.0" - get-port@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/get-port/-/get-port-4.0.0.tgz#373c85960138ee20027c070e3cb08019fea29816" @@ -3091,7 +3076,7 @@ inherits@^2.0.1, inherits@^2.0.3, inherits@~2.0.1, inherits@~2.0.3: version "2.0.3" resolved "https://registry.yarnpkg.com/inherits/-/inherits-2.0.3.tgz#633c2c83e3da42a502f52466022480f4208261de" -ini@^1.3.2, ini@^1.3.3, ini@~1.3.0: +ini@^1.3.2, ini@^1.3.5, ini@~1.3.0: version "1.3.5" resolved "https://registry.yarnpkg.com/ini/-/ini-1.3.5.tgz#eee25f56db1c9ec6085e0c22778083f596abf927" @@ -4364,21 +4349,6 @@ meow@^4.0.0: redent "^2.0.0" trim-newlines "^2.0.0" -meow@^5.0.0: - version "5.0.0" - resolved "https://registry.yarnpkg.com/meow/-/meow-5.0.0.tgz#dfc73d63a9afc714a5e371760eb5c88b91078aa4" - integrity sha512-CbTqYU17ABaLefO8vCU153ZZlprKYWDljcndKKDCFcYQITzWCXZAVk4QMFZPgvzrnUQ3uItnIE/LoUOwrT15Ig== - dependencies: - camelcase-keys "^4.0.0" - decamelize-keys "^1.0.0" - loud-rejection "^1.0.0" - minimist-options "^3.0.1" - normalize-package-data "^2.3.4" - read-pkg-up "^3.0.0" - redent "^2.0.0" - trim-newlines "^2.0.0" - yargs-parser "^10.0.0" - merge-stream@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/merge-stream/-/merge-stream-1.0.1.tgz#4041202d508a342ba00174008df0c251b8c135e1" @@ -4898,12 +4868,6 @@ parse-filepath@^1.0.2: map-cache "^0.2.0" path-root "^0.1.1" -parse-git-config@^0.2.0: - version "0.2.0" - resolved "https://registry.yarnpkg.com/parse-git-config/-/parse-git-config-0.2.0.tgz#272833fdd15fea146fb75d336d236b963b6ff706" - dependencies: - ini "^1.3.3" - parse-github-repo-url@^1.3.0: version "1.4.1" resolved "https://registry.yarnpkg.com/parse-github-repo-url/-/parse-github-repo-url-1.4.1.tgz#9e7d8bb252a6cb6ba42595060b7bf6df3dbc1f50" @@ -5450,12 +5414,6 @@ remark@^9.0.0: remark-stringify "^5.0.0" unified "^6.0.0" -remote-origin-url@0.4.0: - version "0.4.0" - resolved "https://registry.yarnpkg.com/remote-origin-url/-/remote-origin-url-0.4.0.tgz#4d3e2902f34e2d37d1c263d87710b77eb4086a30" - dependencies: - parse-git-config "^0.2.0" - remove-bom-buffer@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/remove-bom-buffer/-/remove-bom-buffer-3.0.0.tgz#c2bf1e377520d324f623892e33c10cac2c252b53" @@ -6741,13 +6699,6 @@ yallist@^3.0.0, yallist@^3.0.2: version "3.0.3" resolved "https://registry.yarnpkg.com/yallist/-/yallist-3.0.3.tgz#b4b049e314be545e3ce802236d6cd22cd91c3de9" -yargs-parser@^10.0.0: - version "10.1.0" - resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-10.1.0.tgz#7202265b89f7e9e9f2e5765e0fe735a905edbaa8" - integrity sha512-VCIyR1wJoEBZUqk5PA+oOBF6ypbwh5aNB3I50guxAL/quggdfs4TtNHQrSazFA3fYZ+tEqfs0zIGlv0c/rgjbQ== - dependencies: - camelcase "^4.1.0" - yargs-parser@^11.1.1: version "11.1.1" resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-11.1.1.tgz#879a0865973bca9f6bab5cbdf3b1c67ec7d3bcf4"