Skip to content

Commit

Permalink
fix(shebangs): only convert CR when doing CRLF -> LF (#2)
Browse files Browse the repository at this point in the history 
* Finish fully promisifying

* Cleanup isHashbangFile implementation

* Only do dos2unix pass when CRs are present in hashbang file

* Rewrite dos2unix to not use streams

* Only convert shebang CR when converting CRs
  • Loading branch information
@iarna @zkat
iarna authored and zkat committed Mar 7, 2018
1 parent 853497c commit 43bf857
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 127 deletions.
166 changes: 66 additions & 100 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@

const path = require('path')
const fs = require('graceful-fs')
const linkIfExists = require('gentle-fs').linkIfExists
const cmdShimIfExists = require('cmd-shim').ifExists
const asyncMap = require('slide').asyncMap
const BB = require('bluebird')
const linkIfExists = BB.promisify(require('gentle-fs').linkIfExists)
const cmdShimIfExists = BB.promisify(require('cmd-shim').ifExists)
const open = BB.promisify(fs.open)
const close = BB.promisify(fs.close)
const stat = BB.promisify(fs.stat)
const read = BB.promisify(fs.read, {multiArgs: true})
const chmod = BB.promisify(fs.chmod)
const Transform = require('stream').Transform
const fsWriteStreamAtomic = require('fs-write-stream-atomic')
const readFile = BB.promisify(fs.readFile)
const writeFileAtomic = BB.promisify(require('write-file-atomic'))

module.exports = BB.promisify(binLinks)

Expand All @@ -29,127 +28,94 @@ function binLinks (pkg, folder, global, opts, cb) {
if (gnm) opts.log.silly('linkStuff', opts.pkgId, 'is installed into a global node_modules')
if (gtop) opts.log.silly('linkStuff', opts.pkgId, 'is installed into the top-level global node_modules')

asyncMap(
[linkBins, linkMans],
function (fn, cb) {
if (!fn) return cb()
opts.log.verbose(fn.name, opts.pkgId)
fn(pkg, folder, parent, gtop, opts, cb)
},
cb
)
BB.join(
linkBins(pkg, folder, parent, gtop, opts),
linkMans(pkg, folder, parent, gtop, opts)
).asCallback(cb)
}

function isHashbangFile (file) {
return open(file, 'r').then((fileHandle) => {
return new BB((resolve, reject) => {
fs.read(fileHandle, Buffer.from(new Array(2)), 0, 2, 0, function (err, bytesRead, buffer) {
close(fileHandle).then(() => {
resolve(!err && buffer.toString() === '#!')
}).catch(reject)
})
})
})
return open(file, 'r').then(fileHandle => {
return read(fileHandle, Buffer.alloc(2), 0, 2, 0).spread((_, buf) => {
if (!hasHashbang(buf)) return []
return read(fileHandle, Buffer.alloc(2048), 0, 2048, 0)
}).spread((_, buf) => buf && hasCR(buf), () => false)
.finally(() => close(fileHandle))
}).catch(() => false)
}

function hasHashbang (buf) {
const str = buf.toString()
return str.slice(0, 2) === '#!'
}

function hasCR (buf) {
return /^#![^\n]+\r\n/.test(buf)
}

function dos2Unix (file) {
return stat(file).then((stats) => {
let previousChunkEndedInCR = false
return new BB((resolve, reject) => {
fs.createReadStream(file)
.on('error', reject)
.pipe(new Transform({
transform: function (chunk, encoding, done) {
let data = chunk.toString()
if (previousChunkEndedInCR) {
data = '\r' + data
}
if (data[data.length - 1] === '\r') {
data = data.slice(0, -1)
previousChunkEndedInCR = true
} else {
previousChunkEndedInCR = false
}
done(null, data.replace(/\r\n/g, '\n'))
},
flush: function (done) {
if (previousChunkEndedInCR) {
this.push('\r')
}
done()
}
}))
.on('error', reject)
.pipe(fsWriteStreamAtomic(file))
.on('error', reject)
.on('finish', function () {
resolve(chmod(file, stats.mode))
})
})
return readFile(file, 'utf8').then(content => {
return writeFileAtomic(file, content.replace(/^(#![^\n]+)\r\n/, '$1\n'))
})
}

function getLinkOpts (opts, gently) {
return Object.assign({}, opts, { gently: gently })
}

function linkBins (pkg, folder, parent, gtop, opts, cb) {
function linkBins (pkg, folder, parent, gtop, opts) {
if (!pkg.bin || (!gtop && path.basename(parent) !== 'node_modules')) {
return cb()
return
}
var linkOpts = getLinkOpts(opts, gtop && folder)
var execMode = parseInt('0777', 8) & (~opts.umask)
var binRoot = gtop ? opts.globalBin
: path.resolve(parent, '.bin')
opts.log.verbose('linkBins', [pkg.bin, binRoot, gtop])

asyncMap(Object.keys(pkg.bin), function (bin, cb) {
return BB.map(Object.keys(pkg.bin), bin => {
var dest = path.resolve(binRoot, bin)
var src = path.resolve(folder, pkg.bin[bin])

linkBin(src, dest, linkOpts, function (er) {
if (er) return cb(er)
return linkBin(src, dest, linkOpts).then(() => {
// bins should always be executable.
// XXX skip chmod on windows?
fs.chmod(src, execMode, function (er) {
if (er && er.code === 'ENOENT' && opts.ignoreScripts) {
return cb()
}
if (er) return cb(er)
isHashbangFile(src).then((isHashbang) => {
if (isHashbang) {
opts.log.silly('linkBins', 'Converting line endings of hashbang file:', src)
return dos2Unix(src)
}
}).then(() => {
if (!gtop) return cb()
var dest = path.resolve(binRoot, bin)
var out = opts.parseable
? dest + '::' + src + ':BINFILE'
: dest + ' -> ' + src

if (!opts.json && !opts.parseable) {
opts.log.clearProgress()
console.log(out)
opts.log.showProgress()
}
cb()
}).catch(cb)
})
return chmod(src, execMode)
}).then(() => {
return isHashbangFile(src)
}).then(isHashbang => {
if (!isHashbang) return
opts.log.silly('linkBins', 'Converting line endings of hashbang file:', src)
return dos2Unix(src)
}).then(() => {
if (!gtop) return
var dest = path.resolve(binRoot, bin)
var out = opts.parseable
? dest + '::' + src + ':BINFILE'
: dest + ' -> ' + src

if (!opts.json && !opts.parseable) {
opts.log.clearProgress()
console.log(out)
opts.log.showProgress()
}
}).catch(err => {
if (err.code === 'ENOENT' && opts.ignoreScripts) return
throw err
})
}, cb)
})
}

function linkBin (from, to, opts, cb) {
function linkBin (from, to, opts) {
if (process.platform !== 'win32') {
return linkIfExists(from, to, opts, cb)
return linkIfExists(from, to, opts)
} else {
return cmdShimIfExists(from, to, cb)
return cmdShimIfExists(from, to)
}
}

function linkMans (pkg, folder, parent, gtop, opts, cb) {
if (!pkg.man || !gtop || process.platform === 'win32') return cb()
function linkMans (pkg, folder, parent, gtop, opts) {
if (!pkg.man || !gtop || process.platform === 'win32') return

var manRoot = path.resolve(opts.prefix, 'share', 'man')
opts.log.verbose('linkMans', 'man files are', pkg.man, 'in', manRoot)
Expand All @@ -160,20 +126,20 @@ function linkMans (pkg, folder, parent, gtop, opts, cb) {
acc[path.basename(man)] = man
return acc
}, {})
pkg.man = pkg.man.filter(function (man) {
var manpages = pkg.man.filter(function (man) {
return set[path.basename(man)] === man
})

asyncMap(pkg.man, function (man, cb) {
if (typeof man !== 'string') return cb()
BB.map(manpages, man => {
if (typeof man !== 'string') return
opts.log.silly('linkMans', 'preparing to link', man)
var parseMan = man.match(/(.*\.([0-9]+)(\.gz)?)$/)
if (!parseMan) {
return cb(new Error(
throw new Error(
man + ' is not a valid name for a man file. ' +
'Man files must end with a number, ' +
'and optionally a .gz suffix if they are compressed.'
))
)
}

var stem = parseMan[1]
Expand All @@ -182,6 +148,6 @@ function linkMans (pkg, folder, parent, gtop, opts, cb) {
var manSrc = path.resolve(folder, man)
var manDest = path.join(manRoot, 'man' + sxn, bn)

linkIfExists(manSrc, manDest, getLinkOpts(opts, gtop && folder), cb)
}, cb)
return linkIfExists(manSrc, manDest, getLinkOpts(opts, gtop && folder))
})
}
41 changes: 23 additions & 18 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@
"dependencies": {
"bluebird": "^3.5.0",
"cmd-shim": "^2.0.2",
"fs-write-stream-atomic": "^1.0.10",
"gentle-fs": "^2.0.0",
"graceful-fs": "^4.1.11",
"slide": "^1.1.6"
"write-file-atomic": "^2.3.0"
},
"devDependencies": {
"mkdirp": "^0.5.1",
Expand Down
Loading

0 comments on commit 43bf857

Please sign in to comment.