From 54ca032730eee06c27995c07a0bf3486bf7532f2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 30 Aug 2018 09:47:12 -0400 Subject: [PATCH 1/6] Spec for --package-lock-only --- spec/install-spec.coffee | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/install-spec.coffee b/spec/install-spec.coffee index 6b73c8bb9..61b67fa08 100644 --- a/spec/install-spec.coffee +++ b/spec/install-spec.coffee @@ -233,6 +233,23 @@ describe 'apm install', -> expect(fs.existsSync(path.join(moduleDirectory, 'node_modules', 'test-module', 'package.json'))).toBeTruthy() expect(callback.mostRecentCall.args[0]).toEqual null + it 'respects --package-lock-only', -> + moduleDirectory = path.join(temp.mkdirSync('apm-test-module-'), 'test-module-with-dependencies') + wrench.copyDirSyncRecursive(path.join(__dirname, 'fixtures', 'test-module-with-dependencies'), moduleDirectory) + process.chdir(moduleDirectory) + expect(fs.existsSync(path.join(moduleDirectory, 'package-lock.json'))).toBeFalsy() + + callback = jasmine.createSpy('callback') + apm.run(['install', '--package-lock-only'], callback) + + waitsFor 'waiting for install to complete', 600000, -> + callback.callCount > 0 + + runs -> + expect(fs.existsSync(path.join(moduleDirectory, 'package-lock.json'))).toBeTruthy() + expect(fs.existsSync(path.join(moduleDirectory, 'node_modules', 'test-module'))).toBeFalsy() + expect(callback.mostRecentCall.args[0]).toEqual null + describe "when the packages directory does not exist", -> it "creates the packages directory and any intermediate directories that do not exist", -> atomHome = temp.path('apm-home-dir-') From 5af9ee7658ab104e20a579cd4081633a95ed63d6 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 30 Aug 2018 10:15:57 -0400 Subject: [PATCH 2/6] Pass --package-lock-only to "npm install" --- src/install.coffee | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/install.coffee b/src/install.coffee index 295fc973f..15115f734 100644 --- a/src/install.coffee +++ b/src/install.coffee @@ -33,6 +33,7 @@ class Install extends Command options.usage """ Usage: apm install [...] + apm install [--package-lock-only] apm install @ apm install apm install / @@ -43,7 +44,8 @@ class Install extends Command If no package name is given then all the dependencies in the package.json file are installed to the node_modules folder in the current working - directory. + directory. If --package-lock-only is specified, then the local package-lock.json + file will be created or brought up to date and no node_modules will be touched. A packages file can be specified that is a newline separated list of package names to install with optional versions using the @@ -57,6 +59,7 @@ class Install extends Command options.boolean('verbose').default('verbose', false).describe('verbose', 'Show verbose debug information') options.string('packages-file').describe('packages-file', 'A text file containing the packages to install') options.boolean('production').describe('production', 'Do not install dev dependencies') + options.boolean('package-lock-only').default('package-lock-only', false).describe('Only update package-lock.json') installNode: (callback) => installNodeArgs = ['install'] @@ -191,6 +194,7 @@ class Install extends Command installArgs.push('--silent') if options.argv.silent installArgs.push('--quiet') if options.argv.quiet installArgs.push('--production') if options.argv.production + installArgs.push('--package-lock-only') if options.argv.packageLockOnly if vsArgs = @getVisualStudioFlags() installArgs.push(vsArgs) From 683a2417ea897fdfe3c7ceb7781bd9d15102854a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 30 Aug 2018 12:16:15 -0400 Subject: [PATCH 3/6] Spec for handling packageDependencies for --package-lock-only --- ...install-test-module-with-dependencies.json | 13 ++++++ spec/install-spec.coffee | 40 ++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 spec/fixtures/install-test-module-with-dependencies.json diff --git a/spec/fixtures/install-test-module-with-dependencies.json b/spec/fixtures/install-test-module-with-dependencies.json new file mode 100644 index 000000000..52e7c717c --- /dev/null +++ b/spec/fixtures/install-test-module-with-dependencies.json @@ -0,0 +1,13 @@ +{ + "releases": { + "latest": "1.1.0" + }, + "name": "test-module-with-dependencies", + "versions": { + "1.1.0": { + "dist": { + "tarball": "http://localhost:3000/tarball/test-module-with-dependencies-1.1.0.tgz" + } + } + } +} diff --git a/spec/install-spec.coffee b/spec/install-spec.coffee index 61b67fa08..047a0a372 100644 --- a/spec/install-spec.coffee +++ b/spec/install-spec.coffee @@ -43,8 +43,12 @@ describe 'apm install', -> response.sendFile path.join(__dirname, 'fixtures', 'test-module-1.1.0.tgz') app.get '/tarball/test-module-1.2.0.tgz', (request, response) -> response.sendFile path.join(__dirname, 'fixtures', 'test-module-1.2.0.tgz') + app.get '/test-module2', (request, response) -> + response.sendFile path.join(__dirname, 'fixtures', 'install-test-module2.json') app.get '/tarball/test-module2-2.0.0.tgz', (request, response) -> response.sendFile path.join(__dirname, 'fixtures', 'test-module2-2.0.0.tgz') + app.get '/tarball/test-module-with-dependencies-1.1.0.tgz', (request, response) -> + response.sendFile path.join(__dirname, 'fixtures', 'test-module-with-dependencies-1.1.0.tgz') app.get '/packages/test-module', (request, response) -> response.sendFile path.join(__dirname, 'fixtures', 'install-test-module.json') app.get '/packages/test-module2', (request, response) -> @@ -55,6 +59,8 @@ describe 'apm install', -> response.sendFile path.join(__dirname, 'fixtures', 'install-test-module-with-bin.json') app.get '/packages/test-module-with-symlink', (request, response) -> response.sendFile path.join(__dirname, 'fixtures', 'install-test-module-with-symlink.json') + app.get '/packages/test-module-with-dependencies', (request, response) -> + response.sendFile path.join(__dirname, 'fixtures', 'install-test-module-with-dependencies.json') app.get '/tarball/test-module-with-symlink-5.0.0.tgz', (request, response) -> response.sendFile path.join(__dirname, 'fixtures', 'test-module-with-symlink-5.0.0.tgz') app.get '/tarball/test-module-with-bin-2.0.0.tgz', (request, response) -> @@ -233,7 +239,8 @@ describe 'apm install', -> expect(fs.existsSync(path.join(moduleDirectory, 'node_modules', 'test-module', 'package.json'))).toBeTruthy() expect(callback.mostRecentCall.args[0]).toEqual null - it 'respects --package-lock-only', -> + describe 'when --package-lock-only is specified', -> + it 'updates package-lock.json but not node_modules/', -> moduleDirectory = path.join(temp.mkdirSync('apm-test-module-'), 'test-module-with-dependencies') wrench.copyDirSyncRecursive(path.join(__dirname, 'fixtures', 'test-module-with-dependencies'), moduleDirectory) process.chdir(moduleDirectory) @@ -250,6 +257,37 @@ describe 'apm install', -> expect(fs.existsSync(path.join(moduleDirectory, 'node_modules', 'test-module'))).toBeFalsy() expect(callback.mostRecentCall.args[0]).toEqual null + it 'accounts for packageDependencies', -> + moduleDirectory = temp.mkdirSync('apm-test-module-') + CSON.writeFileSync path.join(moduleDirectory, 'package.json'), + name: 'has-package-deps' + version: '1.0.0' + dependencies: + 'test-module2': '^2.0.0' + packageDependencies: + 'test-module-with-dependencies': '1.1.0' + process.chdir(moduleDirectory) + + callback = jasmine.createSpy('callback') + apm.run(['install', '--package-lock-only'], callback) + + waitsFor 'waiting for install to complete', 600000, -> + callback.callCount > 0 + + runs -> + pjlock = CSON.readFileSync path.join(moduleDirectory, 'package-lock.json') + + expect(pjlock.dependencies['test-module'].version).toBe('1.2.0') + expect(pjlock.dependencies['test-module2'].version).toBe('2.0.0') + expect(pjlock.dependencies['test-module-with-dependencies'].version) + .toBe('http://localhost:3000/tarball/test-module-with-dependencies-1.1.0.tgz') + + expect(fs.existsSync(path.join(moduleDirectory, 'node_modules', 'test-module2'))).toBeFalsy() + expect(fs.existsSync(path.join(moduleDirectory, 'node_modules', 'test-module-with-dependencies'))).toBeFalsy() + expect(fs.existsSync(path.join(moduleDirectory, 'node_modules', 'test-module'))).toBeFalsy() + + expect(callback.mostRecentCall.args[0]).toEqual null + describe "when the packages directory does not exist", -> it "creates the packages directory and any intermediate directories that do not exist", -> atomHome = temp.path('apm-home-dir-') From ffa4edd7471732e3d0a2c7bda600ef30feb26204 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 30 Aug 2018 12:17:02 -0400 Subject: [PATCH 4/6] Resolve and save updated tarball URIs to package.json for packageDeps --- src/install.coffee | 138 ++++++++++++++++++++++++++++++++------------- 1 file changed, 99 insertions(+), 39 deletions(-) diff --git a/src/install.coffee b/src/install.coffee index 15115f734..c7100354e 100644 --- a/src/install.coffee +++ b/src/install.coffee @@ -148,6 +148,19 @@ class Install extends Command error = @getGitErrorMessage(pack) if error.indexOf('code ENOGIT') isnt -1 callback(error) + # Install resolved apm packages as npm dependencies by adding their tarball uris to the local package.json file. This + # allows us to account for packageDependencies when using --package-lock-only. + saveModules: (options, modules, callback) -> + CSON.readFile "package.json", (err, pjson) -> + if err? + callback(err) + return + + for module in modules + pjson.dependencies[module.name] = module.uri + + CSON.writeFile "package.json", pjson, callback + getGitErrorMessage: (pack) -> message = """ Failed to install #{pack.name} because Git was not found. @@ -244,6 +257,33 @@ class Install extends Command catch error false + # Query the API for an apm package. Resolve a version compatible with an optional requested version range + # and the local Atom installation. + # + # name - The name of the package as registered on atom.io. + # version - Optional version range. Leave as null to request the latest. + # callback - The function to invoke with any errors that occurred, or null, the package metadata, and + # the URI for the resolved package. + resolveRegisteredPackage: (name, version, callback) -> + @requestPackage name, (error, pack) => + if error? + @logFailure() + callback(error) + else + packageVersion = version ? @getLatestCompatibleVersion(pack) + unless packageVersion + @logFailure() + callback("No available version compatible with the installed Atom version: #{@installedAtomVersion}") + return + + {tarball} = pack.versions[packageVersion]?.dist ? {} + unless tarball + @logFailure() + callback("Package version: #{packageVersion} not found") + return + + callback(null, pack, tarball) + # Install the package with the given name and optional version # # metadata - The package metadata object with at least a name key. A version @@ -269,47 +309,35 @@ class Install extends Command if installGlobally process.stdout.write "to #{@atomPackagesDirectory} " - @requestPackage packageName, (error, pack) => + @resolveRegisteredPackage packageName, packageVersion, (error, pack, tarball) => if error? - @logFailure() callback(error) - else - packageVersion ?= @getLatestCompatibleVersion(pack) - unless packageVersion - @logFailure() - callback("No available version compatible with the installed Atom version: #{@installedAtomVersion}") - return + return - {tarball} = pack.versions[packageVersion]?.dist ? {} - unless tarball - @logFailure() - callback("Package version: #{packageVersion} not found") - return + commands = [] + installNode = options.installNode ? true + if installNode + commands.push @installNode + commands.push (next) => @installModule(options, pack, tarball, next) + if installGlobally and (packageName.localeCompare(pack.name, 'en', {sensitivity: 'accent'}) isnt 0) + commands.push (newPack, next) => # package was renamed; delete old package folder + fs.removeSync(path.join(@atomPackagesDirectory, packageName)) + next(null, newPack) + commands.push ({installPath}, next) -> + if installPath? + metadata = JSON.parse(fs.readFileSync(path.join(installPath, 'package.json'), 'utf8')) + json = {installPath, metadata} + next(null, json) + else + next(null, {}) # installed locally, no install path data - commands = [] - installNode = options.installNode ? true - if installNode - commands.push @installNode - commands.push (next) => @installModule(options, pack, tarball, next) - if installGlobally and (packageName.localeCompare(pack.name, 'en', {sensitivity: 'accent'}) isnt 0) - commands.push (newPack, next) => # package was renamed; delete old package folder - fs.removeSync(path.join(@atomPackagesDirectory, packageName)) - next(null, newPack) - commands.push ({installPath}, next) -> - if installPath? - metadata = JSON.parse(fs.readFileSync(path.join(installPath, 'package.json'), 'utf8')) - json = {installPath, metadata} - next(null, json) + async.waterfall commands, (error, json) => + unless installGlobally + if error? + @logFailure() else - next(null, {}) # installed locally, no install path data - - async.waterfall commands, (error, json) => - unless installGlobally - if error? - @logFailure() - else - @logSuccess() unless options.argv.json - callback(error, json) + @logSuccess() unless options.argv.json + callback(error, json) # Install the package with the given name and local path # @@ -357,14 +385,46 @@ class Install extends Command async.series(commands, callback) + # Modify a local package.json file by resolving any packageDependencies and + # adding their tarball URIs as normal npm dependencies. + # + # options - Install options. + # callback - Function to be invoked on completion, with an error as its first argument if one occurs. + savePackageDependencies: (options, callback) -> + resolutions = [] + + resolutionFn = ({name, version}, next) => + module = {name: name} + if version.startsWith('file:.') + module.uri = version + process.nextTick -> next(null, module) + else + @resolveRegisteredPackage name, version, (error, pack, tarball) -> + if error? + next(error) + return + + module.uri = tarball + next(null, module) + + specs = ({name, version} for name, version of @getPackageDependencies()) + async.mapLimit specs, 5, resolutionFn, (error, modules) => + if error? + callback(error) + return + @saveModules options, modules, callback + installDependencies: (options, callback) -> options.installGlobally = false commands = [] commands.push(@installNode) - commands.push (callback) => @installModules(options, callback) - commands.push (callback) => @installPackageDependencies(options, callback) + if options.argv.packageLockOnly + commands.push (cb) => @savePackageDependencies(options, cb) + else + commands.push (cb) => @installPackageDependencies(options, cb) + commands.push (cb) => @installModules(options, cb) - async.waterfall commands, callback + async.series commands, callback # Get all package dependency names and versions from the package.json file. getPackageDependencies: -> From 033b983875bf175429503dfaa4f3417d23b31f8f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 30 Aug 2018 13:06:57 -0400 Subject: [PATCH 5/6] Normalize file:./ dependencies like npm does --- spec/install-spec.coffee | 34 ++++++++++++++++++++++++++++++++++ src/install.coffee | 6 +++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/spec/install-spec.coffee b/spec/install-spec.coffee index 047a0a372..d73e369bb 100644 --- a/spec/install-spec.coffee +++ b/spec/install-spec.coffee @@ -288,6 +288,40 @@ describe 'apm install', -> expect(callback.mostRecentCall.args[0]).toEqual null + it 'normalizes file:. dependencies', -> + moduleDirectory = temp.mkdirSync('apm-test-module-') + vendorDirectory = path.join(moduleDirectory, 'vendor') + fs.mkdirSync(vendorDirectory) + wrench.copyDirSyncRecursive( + path.join(__dirname, 'fixtures', 'test-module'), + path.join(vendorDirectory, 'test-module') + ) + + CSON.writeFileSync path.join(moduleDirectory, 'package.json'), + name: 'has-file-dep' + version: '1.0.0' + dependencies: {} + packageDependencies: + 'test-module': 'file:./vendor/test-module' + process.chdir(moduleDirectory) + + callback = jasmine.createSpy('callback') + apm.run(['install', '--package-lock-only'], callback) + + waitsFor 'waiting for install to complete', 600000, -> + callback.callCount > 0 + + runs -> + pjson = CSON.readFileSync path.join(moduleDirectory, 'package.json') + expect(pjson.dependencies['test-module']).toBe('file:vendor/test-module') + + pjlock = CSON.readFileSync path.join(moduleDirectory, 'package-lock.json') + expect(pjlock.dependencies['test-module'].version).toBe('file:vendor/test-module') + + expect(fs.existsSync(path.join(moduleDirectory, 'node_modules', 'test-module'))).toBeFalsy() + + expect(callback.mostRecentCall.args[0]).toEqual null + describe "when the packages directory does not exist", -> it "creates the packages directory and any intermediate directories that do not exist", -> atomHome = temp.path('apm-home-dir-') diff --git a/src/install.coffee b/src/install.coffee index c7100354e..007ac443b 100644 --- a/src/install.coffee +++ b/src/install.coffee @@ -396,7 +396,11 @@ class Install extends Command resolutionFn = ({name, version}, next) => module = {name: name} if version.startsWith('file:.') - module.uri = version + if version.startsWith('file:./') + # Stay consistent with the way that npm normalizes these. + module.uri = "file:#{version.slice('file:./'.length)}" + else + module.uri = version process.nextTick -> next(null, module) else @resolveRegisteredPackage name, version, (error, pack, tarball) -> From 8bf7d435e4e0a354cc0a89699eb2e133f876ae18 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 30 Aug 2018 15:02:28 -0400 Subject: [PATCH 6/6] Pass file: URLs through path.normalize() --- spec/install-spec.coffee | 15 +++++++++++---- src/install.coffee | 10 +++------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/spec/install-spec.coffee b/spec/install-spec.coffee index d73e369bb..5ba2ca13e 100644 --- a/spec/install-spec.coffee +++ b/spec/install-spec.coffee @@ -292,10 +292,11 @@ describe 'apm install', -> moduleDirectory = temp.mkdirSync('apm-test-module-') vendorDirectory = path.join(moduleDirectory, 'vendor') fs.mkdirSync(vendorDirectory) - wrench.copyDirSyncRecursive( - path.join(__dirname, 'fixtures', 'test-module'), - path.join(vendorDirectory, 'test-module') - ) + for dep in ['test-module', 'test-module-two', 'test-module-three'] + wrench.copyDirSyncRecursive( + path.join(__dirname, 'fixtures', dep), + path.join(vendorDirectory, dep) + ) CSON.writeFileSync path.join(moduleDirectory, 'package.json'), name: 'has-file-dep' @@ -303,6 +304,8 @@ describe 'apm install', -> dependencies: {} packageDependencies: 'test-module': 'file:./vendor/test-module' + 'test-module-two': 'file:vendor/test-module-two' + 'test-module-three': 'file:./native-module/src/../../vendor/test-module-three' process.chdir(moduleDirectory) callback = jasmine.createSpy('callback') @@ -314,9 +317,13 @@ describe 'apm install', -> runs -> pjson = CSON.readFileSync path.join(moduleDirectory, 'package.json') expect(pjson.dependencies['test-module']).toBe('file:vendor/test-module') + expect(pjson.dependencies['test-module-two']).toBe('file:vendor/test-module-two') + expect(pjson.dependencies['test-module-three']).toBe('file:vendor/test-module-three') pjlock = CSON.readFileSync path.join(moduleDirectory, 'package-lock.json') expect(pjlock.dependencies['test-module'].version).toBe('file:vendor/test-module') + expect(pjlock.dependencies['test-module-two'].version).toBe('file:vendor/test-module-two') + expect(pjlock.dependencies['test-module-three'].version).toBe('file:vendor/test-module-three') expect(fs.existsSync(path.join(moduleDirectory, 'node_modules', 'test-module'))).toBeFalsy() diff --git a/src/install.coffee b/src/install.coffee index 007ac443b..37002182e 100644 --- a/src/install.coffee +++ b/src/install.coffee @@ -378,7 +378,7 @@ class Install extends Command for name, version of @getPackageDependencies() do (name, version) => commands.push (next) => - if version.startsWith('file:.') + if /^file:[^\/]/.test version @installLocalPackage(name, version, options, next) else @installRegisteredPackage({name, version}, options, next) @@ -395,12 +395,8 @@ class Install extends Command resolutionFn = ({name, version}, next) => module = {name: name} - if version.startsWith('file:.') - if version.startsWith('file:./') - # Stay consistent with the way that npm normalizes these. - module.uri = "file:#{version.slice('file:./'.length)}" - else - module.uri = version + if /^file:[^\/]/.test version + module.uri = 'file:' + path.normalize(version.slice('file:'.length)) process.nextTick -> next(null, module) else @resolveRegisteredPackage name, version, (error, pack, tarball) ->