Skip to content

Commit

Permalink
chore: remove the 'autoTranslateSSH' feature (#1006)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: The `autoTranslateSSH` feature has been removed since it's kind of hacky, and it's trivial to implement your own version using the `data.suggestion` property of the `UnknownTransportError` when something fails.
  • Loading branch information
billiegoose authored Jan 27, 2020
1 parent 2ae9b78 commit 7b04575
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 82 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ At the time of writing, the following breaking changes are planned:
- [x] The `signing` function argument of `log` will be removed, and `log` will simply always return a payload. **Update: Actually, it now returns the same kind of objects as `readCommit` because that just makes a lot of sense.** (This change is to simplify the type signature of `log` so we don't need function overloading; it is the only thing blocking me from abandoning the hand-crafted `index.d.ts` file and generating the TypeScript definitions directly from the JSDoc tags that already power the website docs.)
- [ ] Any functions that currently return `Buffer` objects will instead return `Uint8Array` so we can eventually drop the bloated Buffer browser polyfill.
- [x] The `pattern` and globbing options will be removed from `statusMatrix` so we can drop the dependencies on `globalyzer` and `globrex`, but you'll be able to bring your own `filter` function instead.
- [ ] The `autoTranslateSSH` feature will be removed, since it's trivial to implement using just the `UnknownTransportError.data.suggestion`
- [x] The `autoTranslateSSH` feature will be removed, since it's trivial to implement using just the `UnknownTransportError.data.suggestion`

## Getting Started

Expand Down
59 changes: 20 additions & 39 deletions __tests__/test-fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe('fetch', () => {
value: `http://${localhost}:9999`
})
// Test
let err = null
let err
try {
await fetch({
gitdir,
Expand All @@ -97,50 +97,31 @@ describe('fetch', () => {
expect(err.code).toEqual(E.UnknownTransportError)
})

it('shallow fetch using autoTranslateSSH param (from Github)', async () => {
const { fs, gitdir } = await makeFixture('test-fetch-cors')
await config({
gitdir,
path: 'http.corsProxy',
value: `http://${localhost}:9999`
})
// Test
await fetch({
gitdir,
depth: 1,
singleBranch: true,
remote: 'ssh',
ref: 'test-branch-shallow-clone',
autoTranslateSSH: true
})
expect(await fs.exists(`${gitdir}/shallow`)).toBe(true)
const shallow = (await fs.read(`${gitdir}/shallow`)).toString('utf8')
expect(shallow === '92e7b4123fbf135f5ffa9b6fe2ec78d07bbc353e\n').toBe(true)
})

it('shallow fetch using autoTranslateSSH config (from Github)', async () => {
const { fs, gitdir } = await makeFixture('test-fetch-cors')
it('the SSH -> HTTPS UnknownTransportError suggestion feature', async () => {
const { gitdir } = await makeFixture('test-fetch-cors')
await config({
gitdir,
path: 'http.corsProxy',
value: `http://${localhost}:9999`
})
await config({
gitdir,
path: 'isomorphic-git.autoTranslateSSH',
value: true
})
// Test
await fetch({
gitdir,
depth: 1,
singleBranch: true,
remote: 'ssh',
ref: 'test-branch-shallow-clone'
})
expect(await fs.exists(`${gitdir}/shallow`)).toBe(true)
const shallow = (await fs.read(`${gitdir}/shallow`)).toString('utf8')
expect(shallow === '92e7b4123fbf135f5ffa9b6fe2ec78d07bbc353e\n').toBe(true)
let err
try {
await fetch({
gitdir,
depth: 1,
singleBranch: true,
remote: 'ssh',
ref: 'test-branch-shallow-clone'
})
} catch (e) {
err = e
}
expect(err).toBeDefined()
expect(err.code).toBe(E.UnknownTransportError)
expect(err.data.suggestion).toBe(
'https://github.com/isomorphic-git/isomorphic-git.git'
)
})

it('shallow fetch since (from Github)', async () => {
Expand Down
5 changes: 1 addition & 4 deletions src/commands/clone.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import { init } from './init.js'
* @param {object} [args.headers = {}] - Additional headers to include in HTTP requests, similar to git's `extraHeader` config
* @param {import('events').EventEmitter} [args.emitter] - [deprecated] Overrides the emitter set via the ['emitter' plugin](./plugin_emitter.md)
* @param {string} [args.emitterPrefix = ''] - Scope emitted events by prepending `emitterPrefix` to the event name
* @param {boolean} [args.autoTranslateSSH] - Attempt to automatically translate SSH remotes into HTTP equivalents
*
* @returns {Promise<void>} Resolves successfully when clone completes
*
Expand Down Expand Up @@ -84,7 +83,6 @@ export async function clone ({
newSubmoduleBehavior = false,
noTags = false,
headers = {},
autoTranslateSSH = false,
// @ts-ignore
onprogress
}) {
Expand Down Expand Up @@ -139,8 +137,7 @@ export async function clone ({
relative,
singleBranch,
headers,
tags: !noTags,
autoTranslateSSH
tags: !noTags
})
if (fetchHead === null) return
ref = ref || defaultBranch
Expand Down
17 changes: 2 additions & 15 deletions src/commands/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { join } from '../utils/join.js'
import { pkg } from '../utils/pkg.js'
import { cores } from '../utils/plugins.js'
import { splitLines } from '../utils/splitLines.js'
import { translateSSHtoHTTP } from '../utils/translateSSHtoHTTP.js'
import { parseUploadPackResponse } from '../wire/parseUploadPackResponse.js'
import { writeUploadPackRequest } from '../wire/writeUploadPackRequest.js'

Expand Down Expand Up @@ -64,7 +63,6 @@ import { config } from './config'
* @param {object} [args.headers] - Additional headers to include in HTTP requests, similar to git's `extraHeader` config
* @param {boolean} [args.prune] - Delete local remote-tracking branches that are not present on the remote
* @param {boolean} [args.pruneTags] - Prune local tags that don’t exist on the remote, and force-update those tags that differ
* @param {boolean} [args.autoTranslateSSH] - Attempt to automatically translate SSH remotes into HTTP equivalents
* @param {import('events').EventEmitter} [args.emitter] - [deprecated] Overrides the emitter set via the ['emitter' plugin](./plugin_emitter.md).
* @param {string} [args.emitterPrefix = ''] - Scope emitted events by prepending `emitterPrefix` to the event name.
*
Expand Down Expand Up @@ -115,7 +113,6 @@ export async function fetch ({
headers = {},
prune = false,
pruneTags = false,
autoTranslateSSH = false,
// @ts-ignore
onprogress // deprecated
}) {
Expand Down Expand Up @@ -150,8 +147,7 @@ export async function fetch ({
singleBranch,
headers,
prune,
pruneTags,
autoTranslateSSH
pruneTags
})
if (response === null) {
return {
Expand Down Expand Up @@ -242,8 +238,7 @@ async function fetchPackfile ({
singleBranch,
headers,
prune,
pruneTags,
autoTranslateSSH
pruneTags
}) {
const fs = new FileSystem(_fs)
// Sanity checks
Expand All @@ -263,14 +258,6 @@ async function fetchPackfile ({
})
}

// Try to convert SSH URLs to HTTPS ones
if (
autoTranslateSSH ||
(await config({ fs, gitdir, path: `isomorphic-git.autoTranslateSSH` }))
) {
url = translateSSHtoHTTP(url)
}

if (corsProxy === undefined) {
corsProxy = await config({ fs, gitdir, path: 'http.corsProxy' })
}
Expand Down
5 changes: 1 addition & 4 deletions src/commands/pull.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import { merge } from './merge'
* @param {Object} [args.author] - passed to [commit](commit.md) when creating a merge commit
* @param {Object} [args.committer] - passed to [commit](commit.md) when creating a merge commit
* @param {string} [args.signingKey] - passed to [commit](commit.md) when creating a merge commit
* @param {boolean} [args.autoTranslateSSH] - Attempt to automatically translate SSH remotes into HTTP equivalents
* @param {boolean} [args.noSubmodules = false] - If true, will not print out an error about missing submodules support. TODO: Skip checkout out submodules when supported instead.
* @param {boolean} [args.newSubmoduleBehavior = false] - If true, will opt into a newer behavior that improves submodule non-support by at least not accidentally deleting them.
*
Expand Down Expand Up @@ -74,7 +73,6 @@ export async function pull ({
author,
committer,
signingKey,
autoTranslateSSH = false,
noSubmodules = false,
newSubmoduleBehavior = false
}) {
Expand Down Expand Up @@ -105,8 +103,7 @@ export async function pull ({
token,
oauth2format,
singleBranch,
headers,
autoTranslateSSH
headers
})
// Merge the remote tracking branch into the local one.
await merge({
Expand Down
13 changes: 1 addition & 12 deletions src/commands/push.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { join } from '../utils/join.js'
import { pkg } from '../utils/pkg.js'
import { cores } from '../utils/plugins.js'
import { splitLines } from '../utils/splitLines.js'
import { translateSSHtoHTTP } from '../utils/translateSSHtoHTTP.js'
import { parseReceivePackResponse } from '../wire/parseReceivePackResponse.js'
import { writeReceivePackRequest } from '../wire/writeReceivePackRequest.js'

Expand Down Expand Up @@ -63,7 +62,6 @@ import { pack } from './pack.js'
* @param {string} [args.token] - See the [Authentication](./authentication.html) documentation
* @param {string} [args.oauth2format] - See the [Authentication](./authentication.html) documentation
* @param {object} [args.headers] - Additional headers to include in HTTP requests, similar to git's `extraHeader` config
* @param {boolean} [args.autoTranslateSSH] - Attempt to automatically translate SSH remotes into HTTP equivalents
* @param {import('events').EventEmitter} [args.emitter] - [deprecated] Overrides the emitter set via the ['emitter' plugin](./plugin_emitter.md).
* @param {string} [args.emitterPrefix = ''] - Scope emitted events by prepending `emitterPrefix` to the event name.
*
Expand Down Expand Up @@ -103,8 +101,7 @@ export async function push ({
password = authPassword,
token,
oauth2format,
headers = {},
autoTranslateSSH = false
headers = {}
}) {
try {
const fs = new FileSystem(_fs)
Expand All @@ -113,14 +110,6 @@ export async function push ({
url = await config({ fs, gitdir, path: `remote.${remote}.url` })
}

// Try to convert SSH URLs to HTTPS ones
if (
autoTranslateSSH ||
(await config({ fs, gitdir, path: `isomorphic-git.autoTranslateSSH` }))
) {
url = translateSSHtoHTTP(url)
}

if (corsProxy === undefined) {
corsProxy = await config({ fs, gitdir, path: 'http.corsProxy' })
}
Expand Down
4 changes: 0 additions & 4 deletions src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,6 @@ export function clone(args: WorkDir & GitDir & {
noGitSuffix?: boolean;
noTags?: boolean;
headers?: { [key: string]: string };
autoTranslateSSH?: boolean;
}): Promise<void>;

export function commit(args: GitDir & {
Expand Down Expand Up @@ -501,7 +500,6 @@ export function fetch(args: GitDir & {
prune?: boolean;
pruneTags?: boolean;
headers?: { [key: string]: string };
autoTranslateSSH?: boolean;
}): Promise<FetchResponse>;

export function findRoot(args: {
Expand Down Expand Up @@ -631,7 +629,6 @@ export function pull(args: WorkDir & GitDir & {
headers?: { [key: string]: string };
emitter?: EventEmitter;
emitterPrefix?: string;
autoTranslateSSH?: boolean;
author?: {
name?: string;
email?: string;
Expand Down Expand Up @@ -668,7 +665,6 @@ export function push(args: GitDir & {
headers?: { [key: string]: string };
emitter?: EventEmitter;
emitterPrefix?: string;
autoTranslateSSH?: boolean;
}): Promise<PushResponse>;

export function readBlob(args: GitDir & {
Expand Down
3 changes: 0 additions & 3 deletions src/models/GitConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ const schema = {
symlinks: bool,
ignorecase: bool,
bigFileThreshold: num
},
'isomorphic-git': {
autoTranslateSSH: bool
}
}

Expand Down

0 comments on commit 7b04575

Please sign in to comment.