Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ jobs:
- name: Install dependencies
run: npm install

- if: "!contains(matrix.os, 'windows')"
name: Get nodeVersion for cache step
id: get-node-version
run: echo "NODE_VERSION=`node -e "console.log(require('./spec/config.json').nodeVersion)"`" > $GITHUB_OUTPUT

- if: "!contains(matrix.os, 'windows')"
name: Cache NodeJS source fixtures
uses: actions/cache@v3
with:
path: spec/fixtures/node-source
key: nodeVersion-${{ steps.get-node-version.outputs.NODE_VERSION }}

- if: "!contains(matrix.os, 'windows')"
name: Run tests 👩🏾‍💻
run: ./bin/npm test
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/node_modules/
/bin/node*
/lib/
/spec/fixtures/node-source/
.DS_Store
npm-debug.log
*~
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
"clean": "npm run clean:lib && npm run clean:bin",
"lint": "coffeelint src spec",
"coffee": "coffee --compile --output lib src",
"ensure-fixtures": "node script/ensure-node-source-fixture-files.js",
"build": "npm run clean:lib && npm run coffee",
"prepare": "npm run build",
"postinstall": "node script/postinstall.js",
"test": "npm run check-version && npm run lint && jasmine-focused --captureExceptions --coffee spec"
"specs": "jasmine-focused --captureExceptions --coffee spec",
"test": "npm run check-version && npm run lint && npm run ensure-fixtures && npm run specs"
},
"dependencies": {
"@atom/plist": "0.4.4",
Expand Down
184 changes: 184 additions & 0 deletions script/ensure-node-source-fixture-files.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
const fs = require("fs");
const path = require("path");
const https = require("https");

// This "nodeVersion" specifies the version of Node or Electron built against
// during certain specs. Update nodeVersion in this JSON file to run those specs
// against a different version of Node's or Electron's source files.
// This value is also used in those spec files, via this same json file.
// (Note: We may need to update this to work around build issues encountered
// when updating Node bundled with ppm.)
// (Note: This should ideally match Pulsar's current Electron version.)
// (Note: Electron forks (and mildly modifies) Node for inclusion in Electron.)
const nodeVersion = require("../spec/config.json").nodeVersion;

// This "distUrl" can be any Node.JS or Electron distribution mirror on the web,
// such as "https://nodejs.org/dist",
// or "https://artifacts.electronjs.org/headers/dist"...
// but it should probably be the official Electron dist URL, so we can test
// building dummy spec packages against the Electron version Pulsar uses.
const distUrl = "https://artifacts.electronjs.org/headers/dist";

// Important note: If you update the above `nodeVersion` to a different version,
// remember to calculate the new sha256sums and update them in the array below.
// Instructions:
// Delete any old files under "spec/fixtures/node-source",
// update `nodeVersion` to the desired version number above,
// re-run this script so it will download the new files,
// then calculate the SHA256SUMS of the updated files, like so:
// on Linux/macOS: `shasum -a 256 spec/fixtures/node-source/*`
// in Windows cmd: `for /r "spec\fixtures\node-source" %i in (*) do CertUtil -hashfile %i SHA256`
// in Windows PowerShell: `Get-FileHash -Algorithm SHA256 "spec\fixtures\node-source\*" | Format-List`
// And finally copy-paste the updated SHA256 hash values into the array below.
const filesToFetch = [
{
url: `${distUrl}/${nodeVersion}/node-${nodeVersion}-headers.tar.gz`,
filename: `node-${nodeVersion}-headers.tar.gz`,
sha256sum: "092a039e403f758f542a0f801acac8604e2d7a9b63d8f3c8c31df71c7ba8aac5"
},
{
url: `${distUrl}/${nodeVersion}/node-${nodeVersion}.tar.gz`,
filename: `node-${nodeVersion}.tar.gz`,
sha256sum: "092a039e403f758f542a0f801acac8604e2d7a9b63d8f3c8c31df71c7ba8aac5"
},
{
url: `${distUrl}/${nodeVersion}/win-x86/node.lib`,
filename: "node.lib",
sha256sum: "4c59ee4f9b78dfdd904cc211080acbbc485104d467c29df24bf45c4017ef638e"
},
{
url: `${distUrl}/${nodeVersion}/win-x64/node.lib`,
filename: "node_x64.lib",
sha256sum: "248a81dd4d5cdaf8c0497a4f6c6855c1e2db3e0439757bfed4f2c1e3c530d04e"
},
{
url: `${distUrl}/${nodeVersion}/SHASUMS256.txt`,
filename: "SHASUMS256.txt",
sha256sum: "8ceda90dbb1f65b9c1ca73321949c3ec6ed81f20f49215535d537164612930a7"
},
];

const sourceFixtureDir = path.resolve(__dirname, "..", "spec", "fixtures", "node-source");
fs.mkdirSync(sourceFixtureDir, { recursive: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally would wrap this in a try...catch block just to ensure we are aware of a failure and can fail gracefully.

Otherwise as this is a simple script file we may not want to fail gracefully, and in that case feel free to leave as is.

Copy link
Member Author

@DeeDeeG DeeDeeG Dec 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for the case of the path already existing, recursive: true makes it so no error is raised for that case.

It's inspired by how certain recursive Unix commands (cp, rm, etc.) don't raise certain errors when you run with --recursive.

See: https://nodejs.org/api/fs.html#fsmkdirpath-options-callback

Calling fs.mkdir() when path is a directory that exists results in an error only when recursive is false.

If that's the only error you had in mind, then it is handled by using the recursive: true option.

Otherwise if there are other concerns, I can look at try...catch. We do need to make that dir, or the script won't work. The whole objective in this script is to download some files to that dir. But we could print something more helpful than just the bare error and then exit, I guess.

Copy link
Member Author

@DeeDeeG DeeDeeG Dec 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to even making the dir in this script would be to commit the dir with an empty .keep file in it, so git can track the folder and we can commit the folder as already existing in the repo.

(git won't track a truly empty dir. So I guess it wants to track files and record their paths, not track dirs, per se. So I needed to either do the .keep thing and commit the dir in git, or generate the folder during this script.)

With that, we could avoid using fs.mkDirSync() in this script.


for (const details of filesToFetch) {
ensureFile(details);
} // This is the main loop, with support functions below.

async function ensureFile(details) {
// We check if the file already exists, and if so,
// make sure its sha256sum matches the expected/correct value.
// If it doesn't exist or has the wrong hash, try to re-download.

logVerbose("details is:");
logVerbose(details);

const destinationPath = path.resolve(sourceFixtureDir, details.filename);
logVerbose(`destinationPath is: ${destinationPath}`);

const existingFileIsCorrect = await verifyExistingFile(destinationPath, details.sha256sum);

if (!existingFileIsCorrect) {
logVerbose("Hash did not match, re-downloading...");
// Get the file
downloadFileToDestination(details.url, destinationPath)
.then(async function () {
console.log(`Successfully downloaded file from ${details.url}.`);
logVerbose("checking if hash matches in for...of loop.");
// Check its hash
const hashDidMatch = await verifyHash(destinationPath, details.sha256sum);
if (!hashDidMatch) {
console.error(`Hash did not match for ${destinationPath}`);
}
})
.catch(console.error);
}
}

async function verifyExistingFile(targetPath, expectedHash) {
if (fs.existsSync(targetPath)) {
logVerbose(`${targetPath} already exists.`);

logVerbose(`verifying hash for ${targetPath} in verifyExistingFile`);
const hashDidMatch = await verifyHash(targetPath, expectedHash);

if (hashDidMatch) {
// Successfully verified existing file. Return true.
console.log(`Existing file ${targetPath} successfully verified.`);
return true;
} else {
// Existing file's hash was wrong. Delete the old file.
console.error(`Hash did not match for ${targetPath}. Deleting.`);
fs.rmSync(targetPath);
}
} else {
// File did not actually exist.
logVerbose(`${targetPath} does not exist.`);
}
// If we haven't returned yet, verification did not succeed. Return false.
return false;
}

async function verifyHash(path, expectedHash) {
// Checks the hash of a given locally downloaded file against its expected value.
// Returns true/false.
// See: https://nodejs.org/api/crypto.html#class-hash for details.

// This module apparently needs to be required each time this function runs,
// or hashing multiple files in parallel can get messed up? Not sure why.
const { createHash } = require("node:crypto");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad you left the comment here, otherwise I was about to write up worried about unneeded resource usage.

But I've seen that before funnily enough in native modules that aren't context aware, or what's more likely is this builder works with a globally defined object (in relation to the actual module) and since NodeJS will require the same module it would likely actually retain it's state, including any global variables declared. So probably why it gets all worried about being used multiple times simultaneously.

Copy link
Member Author

@DeeDeeG DeeDeeG Dec 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm convinced there's a proper way to clear that reused state, but I tried a bunch of things and wasn't able to figure it out. I would not at all mind a fix for this that only require()s it once. But I couldn't figure it out myself, personally. All the hashes were coming out wrong starting at the second file I tried to hash, IIRC.


const hash = createHash("sha256");
hash.update(fs.readFileSync(path));
const actualHash = hash.digest("hex");
hash.end();
// The hash of the actual file on disk.

logVerbose(`expectedHash is: ${expectedHash}`);
logVerbose(`actualHash is: ${actualHash}`);
// The expected hash is from the array of objects at the top of the file.

if (actualHash === expectedHash) {
logVerbose("Hash verified successfully");
return true;
} else {
logVerbose("Hash did not match");
return false;
}
}

function downloadFileToDestination(url, filePath) {
// Actually downloads the desired file to disk.
// Returns a Promise.
// Based on https://scrapingant.com/blog/download-image-javascript

logVerbose(`Downloading ${url} to ${filePath}`);

return new Promise((resolve, reject) => {
https.get(url, (res) => {
// Technically any 2XX series HTTP response code means success,
// but in practice this should always be exactly "200"? Adjust if needed.
if (res.statusCode === 200) {
res.pipe(fs.createWriteStream(filePath))
.on("error", reject)
.once("close", resolve);
} else {
// Consume response data to free up memory
res.resume();
reject(new Error(`Request Failed With a Status Code: ${res.statusCode} for ${url}`));
}
});
});
}

function logVerbose(message) {
// Logs a bunch of verbose information, for debugging purposes.
// Run this script with "--verbose" to help troubleshoot issues.

// Note: Lots of stuff in this script is async and will print out-of-order.
// For example: Smaller files will finish downloading first,
// and their post-download log messages will print in the middle of other stuff.
if (process.argv.includes("--verbose")) {
console.log(message);
}
}
23 changes: 12 additions & 11 deletions spec/ci-spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ express = require 'express'
wrench = require 'wrench'
CSON = require 'season'
apm = require '../lib/apm-cli'
nodeVersion = require('./config.json').nodeVersion

describe 'apm ci', ->
[atomHome, resourcePath, server] = []
Expand All @@ -23,16 +24,16 @@ describe 'apm ci', ->
delete process.env.npm_config_cache

app = express()
app.get '/node/v10.20.1/node-v10.20.1.tar.gz', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-v10.20.1.tar.gz')
app.get '/node/v10.20.1/node-v10.20.1-headers.tar.gz', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-v10.20.1-headers.tar.gz')
app.get '/node/v10.20.1/node.lib', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node.lib')
app.get '/node/v10.20.1/x64/node.lib', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node_x64.lib')
app.get '/node/v10.20.1/SHASUMS256.txt', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'SHASUMS256.txt')
app.get "/node/#{nodeVersion}/node-#{nodeVersion}.tar.gz", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', "node-#{nodeVersion}.tar.gz")
app.get "/node/#{nodeVersion}/node-#{nodeVersion}-headers.tar.gz", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', "node-#{nodeVersion}-headers.tar.gz")
app.get "/node/#{nodeVersion}/win-x86/node.lib", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', 'node.lib')
app.get "/node/#{nodeVersion}/win-x64/node.lib", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', 'node_x64.lib')
app.get "/node/#{nodeVersion}/SHASUMS256.txt", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', 'SHASUMS256.txt')
app.get '/test-module-with-dependencies', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'install-locked-version.json')
app.get '/test-module', (request, response) ->
Expand All @@ -52,7 +53,7 @@ describe 'apm ci', ->
server.listen 3000, '127.0.0.1', ->
process.env.ATOM_ELECTRON_URL = "http://localhost:3000/node"
process.env.ATOM_PACKAGES_URL = "http://localhost:3000/packages"
process.env.ATOM_ELECTRON_VERSION = 'v10.20.1'
process.env.ATOM_ELECTRON_VERSION = "#{nodeVersion}"
process.env.npm_config_registry = 'http://localhost:3000/'
live = true
waitsFor -> live
Expand Down
23 changes: 12 additions & 11 deletions spec/clean-spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ express = require 'express'
http = require 'http'
wrench = require 'wrench'
apm = require '../lib/apm-cli'
nodeVersion = require('./config.json').nodeVersion

describe 'apm clean', ->
[moduleDirectory, server] = []
Expand All @@ -15,16 +16,16 @@ describe 'apm clean', ->

app = express()

app.get '/node/v10.20.1/node-v10.20.1.tar.gz', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-v10.20.1.tar.gz')
app.get '/node/v10.20.1/node-v10.20.1-headers.tar.gz', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-v10.20.1-headers.tar.gz')
app.get '/node/v10.20.1/node.lib', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node.lib')
app.get '/node/v10.20.1/x64/node.lib', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node_x64.lib')
app.get '/node/v10.20.1/SHASUMS256.txt', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'SHASUMS256.txt')
app.get "/node/#{nodeVersion}/node-#{nodeVersion}.tar.gz", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', "node-#{nodeVersion}.tar.gz")
app.get "/node/#{nodeVersion}/node-#{nodeVersion}-headers.tar.gz", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', "node-#{nodeVersion}-headers.tar.gz")
app.get "/node/#{nodeVersion}/win-x86/node.lib", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', 'node.lib')
app.get "/node/#{nodeVersion}/win-x64/node.lib", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', 'node_x64.lib')
app.get "/node/#{nodeVersion}/SHASUMS256.txt", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', 'SHASUMS256.txt')
app.get '/test-module', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'install-test-module.json')
app.get '/tarball/test-module-1.2.0.tgz', (request, response) ->
Expand All @@ -38,7 +39,7 @@ describe 'apm clean', ->
atomHome = temp.mkdirSync('apm-home-dir-')
process.env.ATOM_HOME = atomHome
process.env.ATOM_ELECTRON_URL = "http://localhost:3000/node"
process.env.ATOM_ELECTRON_VERSION = 'v10.20.1'
process.env.ATOM_ELECTRON_VERSION = "#{nodeVersion}"
process.env.npm_config_registry = 'http://localhost:3000/'

moduleDirectory = path.join(temp.mkdirSync('apm-test-module-'), 'test-module-with-dependencies')
Expand Down
4 changes: 4 additions & 0 deletions spec/config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"nodeVersion": "v12.2.3",
"comment": "This is used by specs and scripts. See 'script/ensure-node-source-fixture-files.js' for an explanation of what 'nodeVersion' is used for."
}
4 changes: 0 additions & 4 deletions spec/fixtures/SHASUMS256.txt

This file was deleted.

Binary file removed spec/fixtures/node-v10.20.1-headers.tar.gz
Binary file not shown.
Binary file removed spec/fixtures/node-v10.20.1.tar.gz
Binary file not shown.
Binary file removed spec/fixtures/node.lib
Binary file not shown.
Binary file removed spec/fixtures/node_x64.lib
Binary file not shown.
23 changes: 12 additions & 11 deletions spec/install-spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ http = require 'http'
wrench = require 'wrench'
apm = require '../lib/apm-cli'
Install = require '../lib/install'
nodeVersion = require('./config.json').nodeVersion

describe 'apm install', ->
[atomHome, resourcePath] = []
Expand All @@ -29,16 +30,16 @@ describe 'apm install', ->

beforeEach ->
app = express()
app.get '/node/v10.20.1/node-v10.20.1.tar.gz', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-v10.20.1.tar.gz')
app.get '/node/v10.20.1/node-v10.20.1-headers.tar.gz', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-v10.20.1-headers.tar.gz')
app.get '/node/v10.20.1/node.lib', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node.lib')
app.get '/node/v10.20.1/x64/node.lib', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node_x64.lib')
app.get '/node/v10.20.1/SHASUMS256.txt', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'SHASUMS256.txt')
app.get "/node/#{nodeVersion}/node-#{nodeVersion}.tar.gz", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', "node-#{nodeVersion}.tar.gz")
app.get "/node/#{nodeVersion}/node-#{nodeVersion}-headers.tar.gz", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', "node-#{nodeVersion}-headers.tar.gz")
app.get "/node/#{nodeVersion}/win-x86/node.lib", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', 'node.lib')
app.get "/node/#{nodeVersion}/win-x64/node.lib", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', 'node_x64.lib')
app.get "/node/#{nodeVersion}/SHASUMS256.txt", (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'node-source', 'SHASUMS256.txt')
app.get '/test-module', (request, response) ->
response.sendFile path.join(__dirname, 'fixtures', 'install-test-module.json')
app.get '/tarball/test-module-1.1.0.tgz', (request, response) ->
Expand Down Expand Up @@ -78,7 +79,7 @@ describe 'apm install', ->
process.env.ATOM_HOME = atomHome
process.env.ATOM_ELECTRON_URL = "http://localhost:3000/node"
process.env.ATOM_PACKAGES_URL = "http://localhost:3000/packages"
process.env.ATOM_ELECTRON_VERSION = 'v10.20.1'
process.env.ATOM_ELECTRON_VERSION = "#{nodeVersion}"
process.env.npm_config_registry = 'http://localhost:3000/'
live = true
waitsFor -> live
Expand Down
Loading