Skip to content

Commit

Permalink
feat: do not alter file ownership
Browse files Browse the repository at this point in the history
BREAKING CHANGE: this module no longer attempts to change file ownership automatically
  • Loading branch information
nlf committed Oct 12, 2022
1 parent f57bb4d commit d30f284
Show file tree
Hide file tree
Showing 24 changed files with 178 additions and 518 deletions.
2 changes: 1 addition & 1 deletion lib/content/read.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const fs = require('@npmcli/fs')
const fs = require('fs/promises')
const fsm = require('fs-minipass')
const ssri = require('ssri')
const contentPath = require('./path')
Expand Down
6 changes: 2 additions & 4 deletions lib/content/rm.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
'use strict'

const util = require('util')

const fs = require('fs/promises')
const contentPath = require('./path')
const { hasContent } = require('./read')
const rimraf = util.promisify(require('rimraf'))

module.exports = rm

async function rm (cache, integrity) {
const content = await hasContent(cache, integrity)
// ~pretty~ sure we can't end up with a content lacking sri, but be safe
if (content && content.sri) {
await rimraf(contentPath(cache, content.sri))
await fs.rm(contentPath(cache, content.sri), { recursive: true, force: true })
return true
} else {
return false
Expand Down
14 changes: 5 additions & 9 deletions lib/content/write.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
'use strict'

const events = require('events')
const util = require('util')

const contentPath = require('./path')
const fixOwner = require('../util/fix-owner')
const fs = require('@npmcli/fs')
const fs = require('fs/promises')
const moveFile = require('../util/move-file')
const Minipass = require('minipass')
const Pipeline = require('minipass-pipeline')
const Flush = require('minipass-flush')
const path = require('path')
const rimraf = util.promisify(require('rimraf'))
const ssri = require('ssri')
const uniqueFilename = require('unique-filename')
const fsm = require('fs-minipass')
Expand Down Expand Up @@ -40,7 +37,7 @@ async function write (cache, data, opts = {}) {
return { integrity: sri, size: data.length }
} finally {
if (!tmp.moved) {
await rimraf(tmp.target)
await fs.rm(tmp.target, { recursive: true, force: true })
}
}
}
Expand Down Expand Up @@ -111,7 +108,7 @@ async function handleContent (inputStream, cache, opts) {
return res
} finally {
if (!tmp.moved) {
await rimraf(tmp.target)
await fs.rm(tmp.target, { recursive: true, force: true })
}
}
}
Expand Down Expand Up @@ -152,7 +149,7 @@ async function pipeToTmp (inputStream, cache, tmpTarget, opts) {

async function makeTmp (cache, opts) {
const tmpTarget = uniqueFilename(path.join(cache, 'tmp'), opts.tmpPrefix)
await fixOwner.mkdirfix(cache, path.dirname(tmpTarget))
await fs.mkdir(path.dirname(tmpTarget), { recursive: true })
return {
target: tmpTarget,
moved: false,
Expand All @@ -163,10 +160,9 @@ async function moveToDestination (tmp, cache, sri, opts) {
const destination = contentPath(cache, sri)
const destDir = path.dirname(destination)

await fixOwner.mkdirfix(cache, destDir)
await fs.mkdir(destDir, { recursive: true })
await moveFile(tmp.target, destination)
tmp.moved = true
await fixOwner.chownr(cache, destination)
}

function sizeError (expected, found) {
Expand Down
49 changes: 19 additions & 30 deletions lib/entry-index.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
'use strict'

const util = require('util')
const crypto = require('crypto')
const fs = require('@npmcli/fs')
const {
appendFile,
mkdir,
readFile,
readdir,
rm,
writeFile,
} = require('fs/promises')
const Minipass = require('minipass')
const path = require('path')
const ssri = require('ssri')
const uniqueFilename = require('unique-filename')

const contentPath = require('./content/path')
const fixOwner = require('./util/fix-owner')
const hashToSegments = require('./util/hash-to-segments')
const indexV = require('../package.json')['cache-version'].index
const moveFile = require('@npmcli/move-file')
const _rimraf = require('rimraf')
const rimraf = util.promisify(_rimraf)

module.exports.NotFoundError = class NotFoundError extends Error {
constructor (cache, key) {
Expand Down Expand Up @@ -65,7 +68,7 @@ async function compact (cache, key, matchFn, opts = {}) {

const setup = async () => {
const target = uniqueFilename(path.join(cache, 'tmp'), opts.tmpPrefix)
await fixOwner.mkdirfix(cache, path.dirname(target))
await mkdir(path.dirname(target), { recursive: true })
return {
target,
moved: false,
Expand All @@ -74,24 +77,17 @@ async function compact (cache, key, matchFn, opts = {}) {

const teardown = async (tmp) => {
if (!tmp.moved) {
return rimraf(tmp.target)
return rm(tmp.target, { recursive: true, force: true })
}
}

const write = async (tmp) => {
await fs.writeFile(tmp.target, newIndex, { flag: 'wx' })
await fixOwner.mkdirfix(cache, path.dirname(bucket))
await writeFile(tmp.target, newIndex, { flag: 'wx' })
await mkdir(path.dirname(bucket), { recursive: true })
// we use @npmcli/move-file directly here because we
// want to overwrite the existing file
await moveFile(tmp.target, bucket)
tmp.moved = true
try {
await fixOwner.chownr(cache, bucket)
} catch (err) {
if (err.code !== 'ENOENT') {
throw err
}
}
}

// write the file atomically
Expand Down Expand Up @@ -123,7 +119,7 @@ async function insert (cache, key, integrity, opts = {}) {
metadata,
}
try {
await fixOwner.mkdirfix(cache, path.dirname(bucket))
await mkdir(path.dirname(bucket), { recursive: true })
const stringified = JSON.stringify(entry)
// NOTE - Cleverness ahoy!
//
Expand All @@ -133,19 +129,13 @@ async function insert (cache, key, integrity, opts = {}) {
//
// Thanks to @isaacs for the whiteboarding session that ended up with
// this.
await fs.appendFile(bucket, `\n${hashEntry(stringified)}\t${stringified}`)
await fixOwner.chownr(cache, bucket)
await appendFile(bucket, `\n${hashEntry(stringified)}\t${stringified}`)
} catch (err) {
if (err.code === 'ENOENT') {
return undefined
}

throw err
// There's a class of race conditions that happen when things get deleted
// during fixOwner, or between the two mkdirfix/chownr calls.
//
// It's perfectly fine to just not bother in those cases and lie
// that the index entry was written. Because it's a cache.
}
return formatEntry(cache, entry)
}
Expand Down Expand Up @@ -180,7 +170,7 @@ function del (cache, key, opts = {}) {
}

const bucket = bucketPath(cache, key)
return rimraf(bucket)
return rm(bucket, { recursive: true, force: true })
}

module.exports.lsStream = lsStream
Expand Down Expand Up @@ -246,7 +236,7 @@ async function ls (cache) {
module.exports.bucketEntries = bucketEntries

async function bucketEntries (bucket, filter) {
const data = await fs.readFile(bucket, 'utf8')
const data = await readFile(bucket, 'utf8')
return _bucketEntries(data, filter)
}

Expand All @@ -266,9 +256,8 @@ function _bucketEntries (data, filter) {
let obj
try {
obj = JSON.parse(pieces[1])
} catch (e) {
// Entry is corrupted!
return
} catch (_) {
// eslint-ignore-next-line no-empty-block
}
// coverage disabled here, no need to test with an entry that parses to something falsey
// istanbul ignore else
Expand Down Expand Up @@ -331,7 +320,7 @@ function formatEntry (cache, entry, keepAll) {
}

function readdirOrEmpty (dir) {
return fs.readdir(dir).catch((err) => {
return readdir(dir).catch((err) => {
if (err.code === 'ENOENT' || err.code === 'ENOTDIR') {
return []
}
Expand Down
10 changes: 5 additions & 5 deletions lib/rm.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
'use strict'

const util = require('util')

const { rm } = require('fs/promises')
const glob = require('./util/glob.js')
const index = require('./entry-index')
const memo = require('./memoization')
const path = require('path')
const rimraf = util.promisify(require('rimraf'))
const rmContent = require('./content/rm')

module.exports = entry
Expand All @@ -25,7 +24,8 @@ function content (cache, integrity) {

module.exports.all = all

function all (cache) {
async function all (cache) {
memo.clearMemoized()
return rimraf(path.join(cache, '*(content-*|index-*)'))
const paths = await glob(path.join(cache, '*(content-*|index-*)'), { silent: true, nosort: true })
return Promise.all(paths.map((p) => rm(p, { recursive: true, force: true })))
}
91 changes: 0 additions & 91 deletions lib/util/fix-owner.js

This file was deleted.

7 changes: 7 additions & 0 deletions lib/util/glob.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict'

const { promisify } = require('util')
const glob = promisify(require('glob'))

const globify = (pattern) => pattern.split('//').join('/')
module.exports = (path, options) => glob(globify(path), options)
2 changes: 1 addition & 1 deletion lib/util/move-file.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const fs = require('@npmcli/fs')
const fs = require('fs/promises')
const move = require('@npmcli/move-file')
const pinflight = require('promise-inflight')

Expand Down
13 changes: 3 additions & 10 deletions lib/util/tmp.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
'use strict'

const fs = require('@npmcli/fs')

const fixOwner = require('./fix-owner')
const { withTempDir } = require('@npmcli/fs')
const fs = require('fs/promises')
const path = require('path')

module.exports.mkdir = mktmpdir
Expand All @@ -23,11 +22,5 @@ function withTmp (cache, opts, cb) {
cb = opts
opts = {}
}
return fs.withTempDir(path.join(cache, 'tmp'), cb, opts)
}

module.exports.fix = fixtmpdir

function fixtmpdir (cache) {
return fixOwner(cache, path.join(cache, 'tmp'))
return withTempDir(path.join(cache, 'tmp'), cb, opts)
}

0 comments on commit d30f284

Please sign in to comment.