Bug 32978

Summary: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass
Product: Koha Reporter: Paul Derscheid <paul.derscheid>
Component: Architecture, internals, and plumbingAssignee: Owen Leonard <oleonard>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: dcook, jacob.omara, jonathan.druart, martin.renvoize, tomascohen
Version: Main   
Hardware: Macintosh   
OS: Mac OS   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33615
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
Bug Depends on: 32975    
Bug Blocks: 33615    
Attachments: Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass
Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass
Bug 32978: 'npm install' fails in ktd on aarch64, giving unsupported architecture error for node-sass
Bug 32978: Replace node-sass with dart-sass
Bug 32978: Replace node-sass with dart-sass

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 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 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 2023-02-22 13:00:49 UTC Comment hidden (obsolete)
Comment 10 Tomás Cohen Arazi 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 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.