Skip to content

Commit b53d351

Browse files
committed
fix: triple with windows-msvc doesn't support -Cforce-unwind-tables=no (#12196)
1 parent d2ae8f2 commit b53d351

File tree

4 files changed

+163
-41
lines changed

4 files changed

+163
-41
lines changed

.github/actions/binary-limit/action.yml

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,27 @@ inputs:
77
description: "the increase size limit in bytes, default is 50kb"
88
default: "51200"
99
required: false
10+
platform:
11+
description: "target triple name used to locate the binary"
12+
default: "x86_64-unknown-linux-gnu"
13+
required: false
14+
15+
outputs:
16+
result:
17+
description: "JSON encoded size report"
18+
value: ${{ steps.size-report.outputs.result }}
1019

1120
runs:
1221
using: composite
1322

1423
steps:
1524
- name: GitHub Script
25+
id: size-report
1626
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd
1727
with:
1828
script: |
1929
const limit = parseInt("${{ inputs.size-threshold }}") || 51200;
30+
const platform = inputs.platform;
2031
const action = require("./.github/actions/binary-limit/binary-limit-script.js");
21-
await action({github, context, limit});
32+
const result = await action({github, context, limit, platform});
33+
core.setOutput("result", JSON.stringify(result));

.github/actions/binary-limit/binary-limit-script.js

Lines changed: 69 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,25 @@
11
const fs = require("node:fs");
22

3+
const SIZE_LIMIT_HEADING = "## 📦 Binary Size-limit";
4+
const DEFAULT_PLATFORM = "x86_64-unknown-linux-gnu";
5+
6+
const BINARY_PATHS = {
7+
"x86_64-unknown-linux-gnu": "rspack.linux-x64-gnu.node",
8+
"aarch64-apple-darwin": "rspack.darwin-arm64.node",
9+
"x86_64-pc-windows-msvc": "rspack.win32-x64-msvc.node"
10+
};
11+
12+
const PLATFORM_LABELS = {
13+
"x86_64-unknown-linux-gnu": "Linux x64 (glibc)",
14+
"aarch64-apple-darwin": "macOS arm64",
15+
"x86_64-pc-windows-msvc": "Windows x64"
16+
};
17+
318
/**
4-
* @param {import("@octokit/rest")} github
5-
* @param {Number} limit
19+
* @param {{ github: import('@octokit/rest'), context: any, limit: number, platform?: string }} options
620
*/
7-
module.exports = async function action({ github, context, limit }) {
21+
async function run({ github, context, limit, platform }) {
22+
const target = platform || DEFAULT_PLATFORM;
823
const commits = await github.rest.repos.listCommits({
924
owner: context.repo.owner,
1025
repo: context.repo.repo,
@@ -18,10 +33,13 @@ module.exports = async function action({ github, context, limit }) {
1833
console.log(commit.sha);
1934
try {
2035
const data = await fetchDataBySha(commit.sha);
21-
if (data?.size) {
36+
const size = data?.sizes?.[target] ?? data?.size;
37+
if (typeof size === "number") {
2238
baseCommit = commit;
23-
baseSize = data.size;
24-
console.log(`Commit ${commit.sha} has binary size: ${data.size}`);
39+
baseSize = size;
40+
console.log(
41+
`Commit ${commit.sha} has binary size (${target}): ${size}`
42+
);
2543
break;
2644
}
2745
} catch (e) {
@@ -35,48 +53,49 @@ module.exports = async function action({ github, context, limit }) {
3553
throw new Error(error);
3654
}
3755

38-
const headSize = fs.statSync(
39-
"./crates/node_binding/rspack.linux-x64-gnu.node"
40-
).size;
56+
const file = getBinaryPath(target);
57+
console.log(`Checking binary size for ${file}`);
58+
const headSize = fs.statSync(file).size;
4159

42-
console.log(`Base commit size: ${baseSize}`);
43-
console.log(`Head commit size: ${headSize}`);
44-
45-
const comment = compareBinarySize(headSize, baseSize, context, baseCommit);
46-
47-
try {
48-
await commentToPullRequest(github, context, comment);
49-
} catch (e) {
50-
console.error("Failed to comment on pull request:", e);
51-
}
60+
console.log(`Base commit size (${target}): ${baseSize}`);
61+
console.log(`Head commit size (${target}): ${headSize}`);
5262

5363
const increasedSize = headSize - baseSize;
54-
if (increasedSize > limit) {
55-
throw new Error(
56-
`Binary size increased by ${increasedSize} bytes, exceeding the limit of ${limit} bytes`
57-
);
58-
}
59-
};
64+
return {
65+
platform: target,
66+
baseSize,
67+
headSize,
68+
increasedSize,
69+
exceeded: increasedSize > limit,
70+
comment: compareBinarySize(headSize, baseSize, context, baseCommit)
71+
};
72+
}
73+
74+
module.exports = run;
75+
module.exports.commentToPullRequest = commentToPullRequest;
76+
module.exports.formatReport = formatReport;
77+
module.exports.getBinaryPath = getBinaryPath;
78+
module.exports.SIZE_LIMIT_HEADING = SIZE_LIMIT_HEADING;
6079

61-
async function commentToPullRequest(github, context, comment) {
80+
async function commentToPullRequest(github, context, body) {
6281
const { data: comments } = await github.rest.issues.listComments({
6382
owner: context.repo.owner,
6483
repo: context.repo.repo,
6584
issue_number: context.payload.number
6685
});
6786

68-
const prevComment = comments.filter(
87+
const prevComment = comments.find(
6988
comment =>
7089
comment.user.login === "github-actions[bot]" &&
7190
comment.body.startsWith(SIZE_LIMIT_HEADING)
72-
)[0];
91+
);
7392

7493
if (prevComment) {
7594
await github.rest.issues.updateComment({
7695
owner: context.repo.owner,
7796
repo: context.repo.repo,
7897
comment_id: prevComment.id,
79-
body: `${SIZE_LIMIT_HEADING}\n${comment}`
98+
body
8099
});
81100
return;
82101
}
@@ -85,8 +104,19 @@ async function commentToPullRequest(github, context, comment) {
85104
owner: context.repo.owner,
86105
repo: context.repo.repo,
87106
issue_number: context.payload.number,
88-
body: `${SIZE_LIMIT_HEADING}\n${comment}`
107+
body
108+
});
109+
}
110+
111+
function formatReport(entries) {
112+
const ordered = [...entries].sort((a, b) =>
113+
a.platform.localeCompare(b.platform)
114+
);
115+
const sections = ordered.map(entry => {
116+
const title = PLATFORM_LABELS[entry.platform] || entry.platform;
117+
return `### ${title}\n${entry.comment}`;
89118
});
119+
return `${SIZE_LIMIT_HEADING}\n\n${sections.join("\n\n")}`.trim();
90120
}
91121

92122
function fetchDataBySha(sha) {
@@ -95,8 +125,6 @@ function fetchDataBySha(sha) {
95125
return fetch(dataUrl).then(res => res.json());
96126
}
97127

98-
const SIZE_LIMIT_HEADING = "## 📦 Binary Size-limit";
99-
100128
const DATA_URL_BASE =
101129
"https://raw.githubusercontent.com/web-infra-dev/rspack-ecosystem-benchmark/data";
102130

@@ -118,6 +146,15 @@ function compareBinarySize(headSize, baseSize, context, baseCommit) {
118146
return `${info}🙈 Size remains the same at ${toHumanReadable(headSize)}`;
119147
}
120148

149+
function getBinaryPath(platform) {
150+
const target = platform || DEFAULT_PLATFORM;
151+
const filename = BINARY_PATHS[target];
152+
if (!filename) {
153+
throw new Error(`Unsupported platform: ${target}`);
154+
}
155+
return `./crates/node_binding/${filename}`;
156+
}
157+
121158
function toHumanReadable(size) {
122159
if (size < 1024) {
123160
return `${size}bytes`;

.github/workflows/size-limit.yml

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ permissions:
1010
jobs:
1111
size-limit:
1212
name: Binding Size Limit
13+
strategy:
14+
fail-fast: false
15+
matrix:
16+
include:
17+
- target: x86_64-unknown-linux-gnu
18+
id: linux
19+
- target: aarch64-apple-darwin
20+
id: mac
21+
- target: x86_64-pc-windows-msvc
22+
id: win
1323
runs-on: ${{ fromJSON(vars.LINUX_SELF_HOSTED_RUNNER_LABELS || '"ubuntu-22.04"') }}
1424
steps:
1525
- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
@@ -27,13 +37,11 @@ jobs:
2737
- name: Install Rust Toolchain
2838
uses: ./.github/actions/rustup
2939
with:
30-
key: x86_64-unknown-linux-gnu-release
31-
# don't need use cache in self-hosted windows; benefits of build with cargo build are wasted by cache restore
40+
key: ${{ matrix.target }}-release
3241
save-if: ${{ runner.environment != 'self-hosted' || runner.os != 'Windows' }}
3342

34-
# setup rust target for native runner
3543
- name: Setup Rust Target
36-
run: rustup target add x86_64-unknown-linux-gnu
44+
run: rustup target add ${{ matrix.target }}
3745

3846
- name: Run Cargo codegen
3947
run: cargo codegen
@@ -50,13 +58,76 @@ jobs:
5058
with:
5159
tool-cache: fals
5260

53-
- name: Build x86_64-unknown-linux-gnu native
61+
- name: Build ${{ matrix.target }} native
5462
run: |
55-
rustup target add x86_64-unknown-linux-gnu
56-
RUST_TARGET=x86_64-unknown-linux-gnu pnpm build:binding:release
63+
rustup target add ${{ matrix.target }}
64+
RUST_TARGET=${{ matrix.target }} pnpm build:binding:release
5765
5866
- name: Binary Size-limit
67+
id: size
5968
uses: ./.github/actions/binary-limit
6069
with:
6170
# 50k 50*1024
6271
size-threshold: 51200
72+
platform: ${{ matrix.target }}
73+
74+
- name: Save size result
75+
if: steps.size.outputs.result != ''
76+
shell: bash
77+
env:
78+
RESULT: ${{ steps.size.outputs.result }}
79+
run: |
80+
printf '%s' "$RESULT" > size-result.json
81+
82+
- name: Upload size result
83+
if: steps.size.outputs.result != ''
84+
uses: actions/upload-artifact@v4
85+
with:
86+
name: binary-size-${{ matrix.id }}
87+
path: size-result.json
88+
if-no-files-found: error
89+
90+
gather-results:
91+
name: Gather Binary Size Results
92+
needs: size-limit
93+
if: always()
94+
runs-on: ubuntu-22.04
95+
steps:
96+
- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
97+
98+
- name: Download size reports
99+
uses: actions/[email protected]
100+
with:
101+
pattern: binary-size-*
102+
merge-multiple: true
103+
path: size-results
104+
if-no-files-found: error
105+
106+
- name: Post combined report
107+
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd
108+
env:
109+
RESULTS_DIR: size-results
110+
with:
111+
script: |
112+
const fs = require("node:fs");
113+
const path = require("node:path");
114+
const action = require("./.github/actions/binary-limit/binary-limit-script.js");
115+
const dir = process.env.RESULTS_DIR;
116+
if (!fs.existsSync(dir)) {
117+
core.info("No size reports found");
118+
return;
119+
}
120+
const files = fs.readdirSync(dir).filter(name => name.endsWith(".json"));
121+
if (!files.length) {
122+
core.info("No size reports to process");
123+
return;
124+
}
125+
const entries = files.map(file => {
126+
const raw = fs.readFileSync(path.join(dir, file), "utf8");
127+
return JSON.parse(raw);
128+
});
129+
const body = action.formatReport(entries);
130+
await action.commentToPullRequest(github, context, body);
131+
if (entries.some(entry => entry.exceeded)) {
132+
core.setFailed("Binary size limit exceeded");
133+
}

crates/node_binding/scripts/build.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ async function build() {
8787
}
8888
if (values.profile === "release") {
8989
features.push("info-level");
90-
rustflags.push("-Cforce-unwind-tables=no");
90+
if (process.env.RUST_TARGET && !process.env.RUST_TARGET.startsWith("windows-msvc")) {
91+
rustflags.push("-Cforce-unwind-tables=no");
92+
}
9193
}
9294
if (features.length) {
9395
args.push("--features " + features.join(","));

0 commit comments

Comments
 (0)