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 the tests #7

Open
wants to merge 4 commits into
base: sea-assets
Choose a base branch
from
Open

Conversation

pipobscure
Copy link

This is slightly too complicated for using the suggestion features. Here is what's going on. Let me know in what way you want this.

src/json_parser.cc Outdated Show resolved Hide resolved
try {
({ stderr } = spawnSyncAndExitWithoutError('signtool', [ 'sign', '/fd', 'SHA256', targetExecutable ], {}));
({ child, stderr } = spawnSyncAndExitWithoutError('signtool', [ 'sign', '/fd', 'SHA256', targetExecutable ], { status: undefined }));
Copy link
Author

Choose a reason for hiding this comment

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

This wasn't working because it would have status === 1 when it fails because of missing certificates. Which means it throws and that leaves stderr not set.

So in order to catch this issue, you have to ignore the status and check it here.

certificatesFound = true;
} catch (err) {
if (!/SignTool Error: No certificates were found that met all the given criteria/.test(stderr)) {
throw err;
}
}
if (certificatesFound) {
spawnSyncAndExitWithoutError('signtool', 'verify', '/pa', 'SHA256', targetExecutable, {});
spawnSyncAndExitWithoutError('signtool', [ 'verify', '/pa', 'SHA256', targetExecutable ], {});
Copy link
Author

Choose a reason for hiding this comment

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

The args need to be passed in as an array. This was just never used probably (due to lack of certs?)

src/node_sea.cc Outdated Show resolved Hide resolved
When it's a short string, print it inline, otherwise print it
from a separate line. Also add the missing line breaks finally.
@joyeecheung joyeecheung force-pushed the sea-assets branch 2 times, most recently from 970f089 to 6bde66f Compare January 9, 2024 21:31
With this patch:

Users can now include assets by adding a key-path dictionary
to the configuration as the `assets` field. At build time, Node.js
would read the assets from the specified paths and bundle them into
the preparation blob. In the generated executable, users can retrieve
the assets using the `sea.getAsset()` and `sea.getAssetAsBlob()` API.

```json
{
  "main": "/path/to/bundled/script.js",
  "output": "/path/to/write/the/generated/blob.blob",
  "assets": {
    "a.jpg": "/path/to/a.jpg",
    "b.txt": "/path/to/b.txt"
  }
}
```

The single-executable application can access the assets as follows:

```cjs
const { getAsset } = require('node:sea');
// Returns a copy of the data in an ArrayBuffer
const image = getAsset('a.jpg');
// Returns a string decoded from the asset as UTF8.
const text = getAsset('b.txt', 'utf8');
// Returns a Blob containing the asset.
const blob = getAssetAsBlob('a.jpg');
```

Drive-by: update the  documentation to include a section dedicated
to the injected main script and refer to it as "injected main
script" instead of "injected module" because it's a script, not
a module.
This patch adds support for `sea.getRawAsset()` which is
similar to `sea.getAsset()` but returns the raw asset
in an array buffer without copying. Users should avoid
writing to the returned array buffer. If the injected
section is not marked as writable or not aligned,
writing to the raw asset is likely to result in a crash.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants