Merge ~liushuyu-011/ubuntu/+source/node-yarnpkg:ubuntu/devel into ubuntu/+source/node-yarnpkg:ubuntu/devel

Proposed by Zixing Liu
Status: Merged
Merge reported by: Robie Basak
Merged at revision: 659459fd772abcbe9739a579716d436e8584dd48
Proposed branch: ~liushuyu-011/ubuntu/+source/node-yarnpkg:ubuntu/devel
Merge into: ubuntu/+source/node-yarnpkg:ubuntu/devel
Diff against target: 200 lines (+156/-1)
5 files modified
debian/changelog (+12/-0)
debian/patches/disable-network-tests.patch (+139/-0)
debian/patches/fix-yarnpkg-libzip.patch (+1/-1)
debian/patches/series (+1/-0)
debian/rules (+3/-0)
Reviewer Review Type Date Requested Status
Lucas Kanashiro (community) Approve
Review via email: mp+467614@code.launchpad.net

Description of the change

This MP fixes the node-yarnpkg package after the Node.js 20 transition.

PPA build: https://launchpad.net/~liushuyu-011/+archive/ubuntu/misc/+build/28591489

To post a comment you must log in.
Revision history for this message
Zixing Liu (liushuyu-011) wrote :

Longer explanation of why the flags are needed:

Given the following C code:

```
int main(int argc, char *argv[])
{
  return 8;
}
```

Clang will generate the following WASM module (formatted using the flat WAST format for human reading):

```
__main_argc_argv: # @__main_argc_argv
        i32.const 8
        end_function
```

You will see the function entry was mangled by Clang/LLVM. In Debian Emscripten toolchain, the finalize tool will produce the following WASM module (formatted using canonical WAST format for complete structure):

```
(module
 (type $none_=>_none (func))
 (memory $0 256 256)
 (table $0 1 1 funcref)
 (export "a" (memory $0))
 (export "b" (func $0))
 (export "c" (table $0))
 (func $0
  (nop)
 )
)
```

As you can see the main function got DCE'd in the final binary.

In comparison, this is what the expected module should look like:

```
(module
 (type $0 (func))
 (type $1 (func (param i32) (result i32)))
 (type $2 (func (param i32 i32) (result i32)))
 (global $global$0 (mut i32) (i32.const 66560))
 (memory $0 258 258)
 (table $0 1 1 funcref)
 (export "a" (memory $0))
 (export "b" (func $0))
 (export "c" (func $2))
 (export "d" (func $1))
 (export "e" (table $0))
 (func $0
 )
 (func $1 (param $0 i32) (result i32)
  (global.set $global$0
   (local.tee $0
    (i32.and
     (i32.sub
      (global.get $global$0)
      (local.get $0)
     )
     (i32.const -16)
    )
   )
  )
  (local.get $0)
 )
 (func $2 (param $0 i32) (param $1 i32) (result i32)
  (i32.const 8)
 )
)
```

Revision history for this message
Zixing Liu (liushuyu-011) wrote :

The flags added are to force the finalizer tool to keep the entry points alive (which is automatic with the upstream emsdk binary).

Revision history for this message
Zixing Liu (liushuyu-011) wrote :

Okay, there was a network test failing during the build. I am unsure why this was not patched out in Debian

0476292... by Zixing Liu

d/p/disable-network-tests: add a patch to disable network tests

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

Thanks for this MP! I have a couple of comments for you:

- You forgot to run 'update-maintainer' to correctly set the Maintainers field.
- Could you forward the fix to Debian?

I did run 'update-maintainer' myself and I sponsored the upload for you. Thanks for the fix.

review: Approve
Revision history for this message
Zixing Liu (liushuyu-011) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 78548e2..3103d47 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,15 @@
6+node-yarnpkg (4.0.2+dfsg-2ubuntu1) oracular; urgency=medium
7+
8+ * d/rules: add a few LDFLAGS flags to workaround a bug in the Debian
9+ Emscripten toolchain. Also remove some flags to avoid wasm-ld crashing
10+ (LP: #2068769).
11+ * d/p/fix-yarnpkg-libzip.patch: modify the patch so that it will not add
12+ in unsupported Node.js options that got removed and stablized in Node.js 20
13+ * d/p/disable-network-tests.patch: remove a test that requires Internet
14+ to run properly
15+
16+ -- Zixing Liu <zixing.liu@canonical.com> Mon, 17 Jun 2024 12:52:18 -0600
17+
18 node-yarnpkg (4.0.2+dfsg-2) unstable; urgency=medium
19
20 * Team upload
21diff --git a/debian/patches/disable-network-tests.patch b/debian/patches/disable-network-tests.patch
22new file mode 100644
23index 0000000..961bacc
24--- /dev/null
25+++ b/debian/patches/disable-network-tests.patch
26@@ -0,0 +1,139 @@
27+Description: Disable network tests
28+ We can't reliably test those during the build as the build environment
29+ does not have Internet connection.
30+Author: Zixing Liu <zixing.liu@canonical.com>
31+Forwarded: not-needed
32+Last-Update: 2024-06-19
33+---
34+Index: node-yarnpkg/packages/plugin-compat/tests/patches.test.ts
35+===================================================================
36+--- node-yarnpkg.orig/packages/plugin-compat/tests/patches.test.ts
37++++ /dev/null
38+@@ -1,127 +0,0 @@
39+-import {Configuration, Descriptor, Project, ResolveOptions, ThrowReport, structUtils, Locator, Cache, LocatorHash} from '@yarnpkg/core';
40+-import {PortablePath, xfs, ppath} from '@yarnpkg/fslib';
41+-import NpmPlugin from '@yarnpkg/plugin-npm';
42+-import PatchPlugin from '@yarnpkg/plugin-patch';
43+-
44+-import CompatPlugin from '../sources/index';
45+-
46+-function getConfiguration(p: PortablePath) {
47+- return Configuration.create(p, p, new Map([
48+- [`@yarnpkg/plugin-compat`, CompatPlugin],
49+- [`@yarnpkg/plugin-npm`, NpmPlugin],
50+- [`@yarnpkg/plugin-patch`, PatchPlugin],
51+- ]));
52+-}
53+-
54+-async function createProject(configuration: Configuration, p: PortablePath, manifest: object = {}) {
55+- await xfs.writeFilePromise(ppath.join(p, `package.json`), JSON.stringify(manifest));
56+-
57+- return Project.find(configuration, p);
58+-}
59+-
60+-async function getDescriptorCandidates(descriptor: Descriptor) {
61+- return await xfs.mktempPromise(async dir => {
62+- const configuration = getConfiguration(dir);
63+- const {project} = await createProject(configuration, dir);
64+-
65+- const resolver = configuration.makeResolver();
66+- const resolveOptions: ResolveOptions = {project, resolver, report: new ThrowReport()};
67+-
68+- const normalizedDescriptor = configuration.normalizeDependency(descriptor);
69+- const candidates = await resolver.getCandidates(normalizedDescriptor, {}, resolveOptions);
70+-
71+- return candidates;
72+- });
73+-}
74+-
75+-/**
76+- * A Set used to keep track of the test candidates, so we only test each candidate once.
77+- */
78+-const testedCandidates: Set<LocatorHash> = new Set();
79+-
80+-async function testCandidate(locator: Locator) {
81+- if (testedCandidates.has(locator.locatorHash))
82+- return;
83+-
84+- testedCandidates.add(locator.locatorHash);
85+-
86+- await xfs.mktempPromise(async dir => {
87+- const configuration = getConfiguration(dir);
88+- const {project} = await createProject(configuration, dir, {
89+- dependencies: {
90+- [structUtils.stringifyIdent(locator)]: locator.reference,
91+- },
92+- });
93+- const cache = await Cache.find(configuration);
94+-
95+- await project.resolveEverything({
96+- cache,
97+- lockfileOnly: false,
98+- report: new ThrowReport(),
99+- });
100+-
101+- let error: Error | null = null;
102+-
103+- try {
104+- await project.fetchEverything({
105+- cache,
106+- report: new ThrowReport(),
107+- });
108+- } catch (e) {
109+- error = e;
110+- }
111+-
112+- if (error) {
113+- expect(error.message).not.toContain(`Cannot apply hunk`);
114+- }
115+- });
116+-}
117+-
118+-const TEST_TIMEOUT = 100000000;
119+-
120+-const TEST_RANGES: Array<[string, Array<string>]> = [
121+- [
122+- `fsevents`, [
123+- `^1`,
124+- `^2.1`,
125+- `latest`,
126+- ],
127+- ], [
128+- `resolve`, [
129+- `>=1.9`,
130+- `latest`,
131+- `next`,
132+- ],
133+- ], [
134+- `typescript`, [
135+- `>=3.2`,
136+- `latest`,
137+- `next`,
138+- ],
139+- ],
140+-];
141+-
142+-describe(`patches`, () => {
143+- describe.each(TEST_RANGES)(`%s`, (stringifiedIdent, ranges) => {
144+- const ident = structUtils.parseIdent(stringifiedIdent);
145+-
146+- it.each(ranges)(`should work with ${stringifiedIdent}@%s`, async range => {
147+- const descriptor = structUtils.makeDescriptor(ident, range);
148+- const candidates = await getDescriptorCandidates(descriptor);
149+-
150+- const errors = [];
151+-
152+- for (const candidate of candidates) {
153+- try {
154+- await testCandidate(candidate);
155+- } catch (error) {
156+- errors.push(`--- ${structUtils.stringifyLocator(candidate)} ---\n${error.stack}`);
157+- }
158+- }
159+-
160+- if (errors.length > 0) {
161+- throw new Error(errors.join(`\n`));
162+- }
163+- }, TEST_TIMEOUT);
164+- });
165+-});
166diff --git a/debian/patches/fix-yarnpkg-libzip.patch b/debian/patches/fix-yarnpkg-libzip.patch
167index 5c3e60a..6ac09fe 100644
168--- a/debian/patches/fix-yarnpkg-libzip.patch
169+++ b/debian/patches/fix-yarnpkg-libzip.patch
170@@ -90,7 +90,7 @@ Patch file for @yarnpkg/libzip
171 - -DZLIB_INCLUDE_DIR=/zlib/build/local/include \
172 + -DZLIB_LIBRARY="$ZLIB_ROOT/build/local/lib/libz.a" \
173 + -DZLIB_INCLUDE_DIR="$ZLIB_ROOT/build/local/include" \
174-+ -DCMAKE_CROSSCOMPILING_EMULATOR="/usr/bin/node;--experimental-wasm-threads;--no-experimental-fetch" \
175++ -DCMAKE_CROSSCOMPILING_EMULATOR="/usr/bin/node" \
176 ..
177
178 - docker run --rm \
179diff --git a/debian/patches/series b/debian/patches/series
180index 9f723e7..1ead677 100644
181--- a/debian/patches/series
182+++ b/debian/patches/series
183@@ -17,3 +17,4 @@ fix-yarnpkg-core.patch
184 fix-yarnpkg-pnp.patch
185 fix-arcanis-libzip.patch
186 fix-zlib-ng.patch
187+disable-network-tests.patch
188diff --git a/debian/rules b/debian/rules
189index d95588f..d683e79 100755
190--- a/debian/rules
191+++ b/debian/rules
192@@ -4,6 +4,9 @@
193 # Uncomment this to turn on verbose mode.
194 #export DH_VERBOSE=1
195
196+export DEB_LDFLAGS_MAINT_STRIP = -Wl,-Bsymbolic-functions
197+export DEB_LDFLAGS_MAINT_APPEND = -Wl,--export-if-defined=main -Wl,--export-if-defined=__main_argc_argv
198+
199 %:
200 dh $@
201

Subscribers

People subscribed via source and target branches

to all changes: