@richard67 do you know if the drone server runs on a 32bit linux?

you can check with node, add to your script https://nodejs.org/api/process.html#process_process_arch
then should be visible in the log.

but I doubt that it is 32, no one should use it nowadays for linux

@Fedik it should be a shell or env variables problem (the script actually has x86 32bit binaries). But it uses some bash command (that probably is the one failing here):

#!/bin/sh

# This script drives the standalone sass_embedded package, which bundles together a
# Dart executable and a snapshot of sass_embedded.

follow_links() {
  file="$1"
  while [ -h "$file" ]; do
    # On Mac OS, readlink -f doesn't work.
    file="$(readlink "$file")"
  done
  echo "$file"
}

# Unlike $0, $BASH_SOURCE points to the absolute path of this file.
path=`dirname "$(follow_links "$0")"`
exec "$path/src/dart" "$path/src/dart-sass-embedded.snapshot" "$@"

@dgrammatiko The npm build fails in drone, see https://ci.joomla.org/joomla/joomla-cms/42050/1/7 :

Processing Vendor file: media/vendor/webcomponentsjs/js/webcomponents-bundle.js
events.js:292
throw er; // Unhandled 'error' event

@richard67 I know the error is spawn /********/src/node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded ENOENT basically the file doesn't exist. Spawn is heavily dependant on a proper export of the paths node, node_modules, etc. I really can't debug this remotely

I really can't debug this remotely

You mean it works for you locally?

Yes, I wouldn't really create a PR with non-working code...

Yes, I wouldn't really create a PR with non-working code...

@dgrammatiko I haven't assumed that. Here it works too locally on Ubuntu. Other thing: I think you should change your testing instructions from "npm install" to "npm ci".

I think you should change your testing instructions from "npm install" to "npm ci".

Done but it's also worthless if the CI is failing, thus the PR can't be merged

Done but it's also worthless if the CI is failing, thus the PR can't be merged

Well, we should try to fix that. Regarding 32 or 64 bit of our testing environment I have no idea, but it is not sooooo uncommon to still have 32 bit compilations running on 64 bit systems.

@Hackwar Do you know if our Drone runs on 32 or 64 bit Linux?

I think the error comes from their postinstall script:

> sass-embedded@1.0.0-beta.0 postinstall /********/src/node_modules/sass-embedded
11	> node ./download-compiler-for-end-user.js
12	
13	Downloading dart-sass-embedded release asset.
14	Unzipping dart-sass-embedded release asset to ./dist/lib/src/vendor.

The last line indicates that the script is attempting to d/l the executable on a relative (to the node_modules/sass_...) and obviously there's a path conflict there.

@dgrammatiko So here it works on Linux and fails on Windows with the same error as we have in drone.

@dgrammatiko Node version on my Linux where it works is v12.13.0, on my Windows where it fails it's v14.15.5. So it's maybe related to node versions?

@richard67 might be but it works on my v14: node -v v14.16.1

@dgrammatiko I think I have found the mistake in their build script. Will see if I can make PR there and let you know when done.

@dgrammatiko sass/embedded-host-node#45 ... let's see what happens.

@dgrammatiko https://github.com/sass/embedded-host-node/releases/tag/1.0.0-beta.1 ?

@dgrammatiko Hmm, seems that was not all. Drone is still failing. I've restarted it to be sure, but I'm not optimistic.

I've spent the last hour trying to build it locally but without any success

Here "npm ci" works fine on Linux (like before) and Windows (now after the last change for the update). I have no idea what goes wrong with the "npm" step in Drone.

our CI uses Debian buster 64bit base systems with docker images. Depending on the task we have our own images inherited from upstream or directly the main docker images provided by the relevant software package in this case NPM and we use node:14-alpine for running npm.

So you should be able to reproduce the problem locally if you use the node:14-alpine docker image.

I've converted this to draft till the drone issue is solved (either upstream or here)

@dgrammatiko I still have no idea why npm ci fails in drone, here it works fine on Linux and Windows. And I have no knowledge or experience with Docker yet. Do you have the knowledge how to use that node:14-alpine image? As far as I know it will run "npm ci" on the Joomla branch where before another docker container has done the "composer install" (which is required because our build script uses some composer dependency).

Do you have the knowledge how to use that node:14-alpine image?

I haven't used it before but I'm planning to give it a go locally in the coming days and see where it fails. It might not be a problem on our side...

@dgrammatiko I've managed to set up docker on my Linux VM and use that image to run npm, and now I can reproduce the error. So I can debug that later after work.

@dgrammatiko I've found out something, but I don't know if it is a valid fix.

Update: Forget it, I was wrong.

@richard67 sounds legit. I mean if they want to keep it asynchronous they have to await till the process has finished (with an event) before proceeding in their logic. So basically it's a race condition and by using the synchronous version of spawn you ensure the order of execution. Hope this makes some sense

I was wrong, I had a typo, "spawnSynch" instead of "spawnSync". Without the typo it doesn't work, I get errors. Strange that with the typo it seemed to work.

@richard67 with the typo you were able to compile scss to css?

with the typo you were able to compile scss to css?

@dgrammatiko At least it seemed so, no error messages were shown, and it is going through the files. Weird.

@richard67 fwiw can you use https://www.npmjs.com/package/promisify-child-process instead of the core child-process
and await on that spawn? If this is a race condition then this should fix it. Probably you need to use also https://www.npmjs.com/package/patch-package to patch their package

Scrap that. Can you try this code:

export class EmbeddedCompiler {
  constructor() {
    this.process = spawn(
      resolve(
        __dirname,
        `../vendor/dart-sass-embedded/dart-sass-embedded${
          process.platform === 'win32' ? '.bat' : ''
        }`
      ),
      {
        windowsHide: true,
      }
    );

    /** The child process's exit event. */
    this.exit$ = new Observable<number | null>(observer => {
      this.process.on('exit', code => observer.next(code));
    });

    /** The buffers emitted by the child process's stdout. */
    this.stdout$ = new Observable<Buffer>(observer => {
      this.process.stdout.on('data', buffer => observer.next(buffer));
    }).pipe(takeUntil(this.exit$));

    /** The buffers emitted by the child process's stderr. */
    this.stderr$ = new Observable<Buffer>(observer => {
      this.process.stderr.on('data', buffer => observer.next(buffer));
    }).pipe(takeUntil(this.exit$));
  }

  /** Writes `buffer` to the child process's stdin. */
  writeStdin(buffer: Buffer): void {
    this.process.stdin.write(buffer);
  }

  /** Kills the child process, cleaning up all associated Observables. */
  close() {
    this.process.stdin.end();
  }
}

@dgrammatiko Should your code be the complete content of file node_modules/sass-embedded/dist/lib/src/embedded/compiler.js? Or only a part of it? Or shall I put it elsewhere? When using it for the complete file content, I get a SyntaxError: Unexpected token 'export'.

It should replace https://github.com/sass/embedded-host-node/blob/661fa06465e8cae1ecc518e11dc6dae24dccb573/lib/src/embedded/compiler.ts#L14-L51

@dgrammatiko

  writeStdin(buffer: Buffer): void {
                   ^

SyntaxError: Unexpected token ':'

@dgrammatiko Your code is for the ".ts" file, but I need to modify the ".js".

@richard67 can you run the script from the docker's shell: ./node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded?

add echo "$path/src/dart" before the last line
What's the path there?

Also can you try changing the line file="$(readlink "$file")" to file="$(readlink -- "$file")"

@richard67 can you run the script from the docker's shell: ./node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded?

@dgrammatiko It's not a script, it's a binary on Linux.

If I pass it as command to docker, it can't be executed because it is not found:

richard@vmkubu02:~/lamp/public_html/joomla-cms-4.0-dev$ docker run -ti -v `pwd`:/home -w /home node:14-alpine /home/node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded
/usr/local/bin/docker-entrypoint.sh: exec: line 8: /home/node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded: not found
richard@vmkubu02:~/lamp/public_html/joomla-cms-4.0-dev$

If I pass in the same way an ls -al command for that file to the docker, it shows the file:

richard@vmkubu02:~/lamp/public_html/joomla-cms-4.0-dev$ docker run -ti -v `pwd`:/home -w /home node:14-alpine ls -al /home/node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded
-rwxr-xr-x    1 root     xfs        9240040 Feb 25 00:00 /home/node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded
richard@vmkubu02:~/lamp/public_html/joomla-cms-4.0-dev$

add echo "$path/src/dart" before the last line
What's the path there?

Where should I add that, in which file?

Also can you try changing the line file="$(readlink "$file")" to file="$(readlink -- "$file")"

Where should I do that change, in which file?

Where should I add that, in which file?
Where should I do that change, in which file?

Both are pointing in the same file: node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded

If I pass in the same way an ls -al command for that file to the docker, it shows the file:

Are we hitting the weirdness of symlinks on the containers here?

I still don't get it. node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded is a binary file here, so there is no file="$(readlink "$file")" in that file I could change, and I still don't get where I have to change that.

I don't have a symlink here anywhere below node_modules/sass-embedded. I only have a symlink ./node_modules/sass-embedded/node_modules/.bin/semver pointing to ../semver/bin/semver, but I think that's something else.

Yeah there's no symlink but I have

@dgrammatiko I think it needs to sign the changed .drone.yml.

I think it needs to sign the changed .drone.yml.

Sure but it's not a symlink problem. Btw which of all the node-alpine packages are you using?

„14-alpine“ with exactly that name.

But I‘ve also tried „12-alpine“ and „15-alpine“ (the numbers are the node versions) with the same result, so it‘s not a problem with a particular version.

I ran npm ci three times with an without the patch on a Macbook Pro. The real time difference was about 7 seconds in 90 seconds. Without patch:

1:30, 1:31, 1:30

Timer: Build finished in 79371 ms

added 942 packages, and audited 943 packages in 2m

2 packages are looking for funding
run npm fund for details

found 0 vulnerabilities

=== With patch:

1:25, 1:23, 1:22
Timer: Build finished in 70086 ms
Timer: Build finished in 68411 ms

added 949 packages, and audited 950 packages in 1m

8 vulnerabilities (1 moderate, 7 high)

To address issues that do not require attention, run:
npm audit fix

To address all issues (including breaking changes), run:
npm audit fix --force

Run npm audit for details.

== did npm audit fix --force then npm ci with the following error:

events.js:292
throw er; // Unhandled 'error' event
^

Error: spawn /Users/ceford/Sites/j4test/node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded ENOENT
at Process.ChildProcess._handle.onexit (internal/child_process.js:269:19)
at onErrorNT (internal/child_process.js:465:16)
at processTicksAndRejections (internal/process/task_queues.js:80:21)
Emitted 'error' event on ChildProcess instance at:
at Process.ChildProcess._handle.onexit (internal/child_process.js:275:12)
at onErrorNT (internal/child_process.js:465:16)
at processTicksAndRejections (internal/process/task_queues.js:80:21) {
errno: -2,
code: 'ENOENT',
syscall: 'spawn /Users/ceford/Sites/j4test/node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded',
path: '/Users/ceford/Sites/j4test/node_modules/sass-embedded/dist/lib/src/vendor/dart-sass-embedded/dart-sass-embedded',
spawnargs: []
}
npm ERR! code 1
npm ERR! path /Users/ceford/Sites/j4test
npm ERR! command failed
npm ERR! command sh -c node build/build.js --prepare

npm ERR! A complete log of this run can be found in:
npm ERR! /Users/ceford/.npm/_logs/2021-05-08T13_11_58_321Z-debug.log

Something is off in your comment

before:added 942 packages, and audited 943 packages in 2m
after: added 949 packages, and audited 950 packages in 1m

@HLeithner could you approve the drone changes here?

@dgrammatiko approved, are you planing other changes to the drone file?

@dgrammatiko failed at https://ci.joomla.org/joomla/joomla-cms/45361/1/7

This PR will never work correctly in the Drone CI if the Node container doesn't have the git app (apt-get install git) and since I have no clue how the whole CI was assembled I can't fix it...

Anyone interested to move this forward could copy the changes from here and patch the missing git from the Node Alpine container