Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate version #835

Merged
merged 7 commits into from
Mar 30, 2023
Merged
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
4 changes: 4 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Added

- Validation for `options.version`.

## [4.2.1] - 2023-03-28

### Changed
Expand Down
8 changes: 4 additions & 4 deletions docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ Install Node. Consult the [version manifest](https://nwjs.io/versions) before in

Clone the fork.

Created a new branch. If you are addressing a specific issue (for example #54), then name the branch `dev-54`. If you encounter a problem which has not been documented (even after searching all closed issues), either open an new issue or create a branch such as `fix/glob-nested-dirs`.
Creat a new branch. If you are addressing a specific issue (for example #54), then name the branch `dev-54`. If you encounter a problem which has not been documented (even after searching all closed issues), either open a new issue or create a branch such as `fix/glob-nested-dirs`.

## General Tips

Add inline code comments if the change you make may not be obvious to others. Add tests that cover the behavioru change. We use Node's test runner and Jest (slowly migrating away from it) for unit tests and Chromedriver for e2e tests.
Add inline code comments if the change you make may not be obvious to others. Add tests that cover the behaviour change. We use Node's test runner and Jest (slowly migrating away from it) for unit tests and Chromedriver for e2e tests.

Our test coverage is not great. Do not assume the package works even if all tests pass. Dependency updates are handled by `dependabot` unless your code change requires a version bump.
Our test coverage is not great. Do not assume the package works even if all tests pass. Dependency updates are handled by `dependabot` unless your code change requires a version bump. If certain behaviour can be implemented using Node's standard library, that is always preferred over a third party library.

If you're uncertain about any of the above, make the pull request anyway. A maintainer/community member will help you out.
If you're unsure about any of the above, make the pull request anyway. A maintainer/community member will help you out.
8 changes: 4 additions & 4 deletions docs/install.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# Installation Guide

Every NW.js release includes a modified Node.js at a specific version. By installing a version greater/equal to NW.js's Node version, you get to use the latest features. In that case tt is recommended to use a [Node Version Manager](https://nodejs.org/en/download/package-manager) to manage multiple Node installations. Consult the [manifest](https://nwjs.io/versions) for what Node version to install.
Every NW.js release includes a modified Node.js at a specific version. It is recommended to [install](https://nodejs.org/en/download/package-manager) a version greater than or equal to NW.js's Node version. Consult the [version manifest](https://nwjs.io/versions) on the version to install.

With the environment setup, install `nw-builder` using npm:
With the environment set up, install `nw-builder` using npm:

Using npm:

Expand All @@ -14,13 +14,13 @@ npm i -D nw-builder

You may use alternate package managers:

Enable `corepack` if your Node version is above `v14.19.0` or `v16.9.0`:
Enable `corepack`:

```shell
corepack enable
```

You may install it via npm if your version does not match:
Or install it if your version does not include it:

```shell
npm i -g corepack
Expand Down
10 changes: 5 additions & 5 deletions docs/usage-basic.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Basic Usage

Depending on your needs, `nw-builder` can be used via a JavaScript module, via command line interface or via `package.json`. Please note that if `package.json` is used, it will override the module or CLI configuration. A single function is exported which performs a single build using a specific [build flavor](https://nwjs.readthedocs.io/en/latest/For%20Users/Advanced/Build%20Flavors/) for a specific platform and architecture.
Depending on your use case, `nw-builder` can be used via a JavaScript module, command line interface or `package.json`. Please note that if `package.json` is used, it will override the module or CLI configuration.

## Import package

Expand Down Expand Up @@ -29,7 +29,7 @@ let nwbuild = undefined;
Module usage:

```javascript
nwbuild({ ...options });
nwbuild();
```

CLI usage:
Expand All @@ -42,8 +42,8 @@ npx nwbuild

```json
{
"nwbuild": {
...
}
"nwbuild": {}
}
```

By default, it performs a build using the latest version for the host platform and architecture. The next few guides will show you how to customize this behaviour.
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@
"test:jest": "node --experimental-vm-modules node_modules/jest/bin/jest.js",
"test:unit": "node --test-reporter=spec --test test/unit/index.js",
"test:e2e": "node --test-reporter=spec --test e2e/index.js",
"test:manual:get": "cd test/fixture && nwbuild --mode=get",
"test:manual:dev": "cd test/fixture && nwbuild --mode=run ./app --no-glob",
"test:manual:bld": "cd test/fixture && nwbuild --mode=build ./app --no-glob"
"test:manual:get": "cd test/fixture && nwbuild --mode=get --version=0.74.0",
"test:manual:dev": "cd test/fixture && nwbuild --mode=run --version=0.74.0 ./app --no-glob",
"test:manual:bld": "cd test/fixture && nwbuild --mode=build --version=0.74.0 ./app --no-glob"
},
"devDependencies": {
"eslint": "^8.36.0",
Expand Down
12 changes: 6 additions & 6 deletions src/nwbuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { develop } from "./run/develop.js";
import { isCached } from "./util/cache.js";
import { replaceFfmpeg } from "./util/ffmpeg.js";
import { getFiles } from "./util/files.js";
import { getManifest } from "./util/manifest.js";
import { getVersionManifest } from "./util/versionManifest.js";
import { parse } from "./util/parse.js";
import { validate } from "./util/validate.js";
import { xattr } from "./util/xattr.js";
Expand Down Expand Up @@ -99,7 +99,7 @@ const nwbuild = async (options) => {

if (options.mode !== "get") {
files = options.glob ? await getFiles(options.srcDir) : options.srcDir;
manifest = await getManifest(files, options.glob);
manifest = await getVersionManifest(files, options.glob);
if (typeof manifest?.nwbuild === "object") {
options = manifest.nwbuild;
}
Expand Down Expand Up @@ -129,16 +129,16 @@ const nwbuild = async (options) => {
options.cacheDir,
options.manifestUrl,
);
// Remove leading "v" from version string
options.version = releaseInfo.version.slice(1);

await validate(options, releaseInfo);

// Remove leading "v" from version string
options.version = releaseInfo.version.slice(1);

// Variable to store nwDir file path
nwDir = resolve(
options.cacheDir,
`nwjs${options.flavor === "sdk" ? "-sdk" : ""}-v${options.version}-${
options.platform
`nwjs${options.flavor === "sdk" ? "-sdk" : ""}-v${options.version}-${options.platform
}-${options.arch}`,
);

Expand Down
1 change: 1 addition & 0 deletions src/util/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { getPlatform } from "./platform.js";
* @return {Promise<object>} Options
*/
export const parse = async (options, pkg) => {
options = options ?? {};
options.mode = options.mode ?? "build";

options.version = options.version ?? "latest";
Expand Down
4 changes: 3 additions & 1 deletion src/util/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ export const validate = async (options, releaseInfo) => {
`Unknown mode ${options.mode}. Expected "get", "run" or "build".`,
);
}
// We don't validate version since getReleaseInfo already does that
if (typeof releaseInfo === "undefined") {
throw new Error("The specified version does not exist in the version manifest. Disable cache to redownload the version manifest. If you still get this error, that means the version you've specified is incorrect.");
}
if (!releaseInfo.flavors.includes(options.flavor)) {
throw new Error(
`${options.flavor} flavor is not supported by this download server.`,
Expand Down
2 changes: 1 addition & 1 deletion src/util/manifest.js → src/util/versionManifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { basename } from "node:path";
* @param {boolean} glob Whether or not the files are glob patterns
* @return {Promise<object>} NW.js manifest file
*/
export const getManifest = async (files, glob) => {
export const getVersionManifest = async (files, glob) => {
let manifest;

if (glob === false) {
Expand Down