Bug 32978 - 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass
Summary: 'npm install' fails in ktd on aarch64, giving unsupported architecture error ...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: Macintosh Mac OS
: P5 - low normal
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on: 32975
Blocks: 33615
  Show dependency treegraph
 
Reported: 2023-02-16 09:08 UTC by Paul Derscheid
Modified: 2023-12-28 20:44 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.04
Circulation function:


Attachments
Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass (74.40 KB, patch)
2023-02-16 13:11 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass (74.53 KB, patch)
2023-02-16 16:20 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass (74.59 KB, patch)
2023-02-16 16:51 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 32978: Replace node-sass with dart-sass (5.06 KB, patch)
2023-02-22 13:00 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 32978: Replace node-sass with dart-sass (5.06 KB, patch)
2023-02-22 13:01 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Derscheid 2023-02-16 09:08:18 UTC
The summary already says it, but you can't build Koha's dependencies on aarch64 because it breaks on node-sass which has native bindings that demand x86_64. 

Therefore we should switch to sass (aka dart-sass) which doesn't have this problem . For a possible migration path, see https://dev.to/ccreusat/migrating-from-node-sass-to-sass-dart-sass-with-npm-3g3c
Comment 1 Owen Leonard 2023-02-16 13:11:33 UTC
Created attachment 146729 [details] [review]
Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass

This patch adds replaces node-sass with dart-sass, updating our gulpfile
accordingly. Some corrections have been made to SCSS to fix warnings
raised during the build process.
Comment 2 Owen Leonard 2023-02-16 13:12:05 UTC
I'm not sure if this is really ready to go, but I wanted to share what I'd done so far.
Comment 3 Owen Leonard 2023-02-16 13:55:49 UTC
I should add that I only tested from inside KTD on an Intel Mac.
Comment 4 Paul Derscheid 2023-02-16 16:20:10 UTC
Created attachment 146744 [details] [review]
Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass

This patch adds replaces node-sass with dart-sass, updating our gulpfile
accordingly. Some corrections have been made to SCSS to fix warnings
raised during the build process.

Works like a charm. Great work oleonard!
Tested on Apple M1, e.g. aarch64.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 5 Martin Renvoize (ashimema) 2023-02-16 16:51:30 UTC
Created attachment 146745 [details] [review]
Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass

This patch adds replaces node-sass with dart-sass, updating our gulpfile
accordingly. Some corrections have been made to SCSS to fix warnings
raised during the build process.

Works like a charm. Great work oleonard!
Tested on Apple M1, e.g. aarch64.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2023-02-16 16:54:25 UTC
Working well in ktd on my linux box.  I ran a `yarn install --frozen-lockfile` to get the local dependancies installed and used over the ktd defaults.

It spits out a couple of errors:

warning " > bootstrap@4.6.2" has unmet peer dependency "jquery@1.9.1 - 3".
and 
warning " > bootstrap@4.6.2" has unmet peer dependency "popper.js@^1.16.1".

I think these are harmless.. I certailny couldn't spot any problems in running Koha.

QA scripts are also happy and running the builds outside of ktd also works (assuming you've got the right node versions pinned at 14)

I'm going to tentatively PQA here.. though more eyes are certainly welcome.
Comment 7 Jonathan Druart 2023-02-22 12:48:00 UTC
RM should generate the yarn.lock, I am getting changes when I run it locally.

(In reply to Martin Renvoize from comment #6)
> It spits out a couple of errors:
> 
> warning " > bootstrap@4.6.2" has unmet peer dependency "jquery@1.9.1 - 3".
> and 
> warning " > bootstrap@4.6.2" has unmet peer dependency "popper.js@^1.16.1".

This is on master as well.
Comment 8 Jonathan Druart 2023-02-22 12:53:08 UTC
And, just saying... but "Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass" is not a valid commit message. This is the description of the bug, not what the patch is actually doing.
Comment 9 Tomás Cohen Arazi (tcohen) 2023-02-22 13:00:49 UTC Comment hidden (obsolete)
Comment 10 Tomás Cohen Arazi (tcohen) 2023-02-22 13:01:10 UTC
Created attachment 147137 [details] [review]
Bug 32978: Replace node-sass with dart-sass

'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass.

This patch addresses this by replacing node-sass with dart-sass, updating our gulpfile
accordingly. Some corrections have been made to SCSS to fix warnings
raised during the build process.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Works like a charm. Great work oleonard! Tested on Apple M1, e.g. aarch64.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Edit: I removed some useless formatting changes in the gulpfile, and the
yarn.lock changes as well.
Comment 11 Tomás Cohen Arazi (tcohen) 2023-02-22 13:11:28 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 12 Jacob O'Mara 2023-02-24 11:33:22 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 13 Jacob O'Mara 2023-02-24 11:36:34 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.