jurplel / install-qt-action

Install Qt on your Github Actions workflows with just one simple action
MIT License
459 stars 78 forks source link

CI (Windows): got unexpected output when executing "npm run build-setup-python" #116

Closed pzhlkj6612 closed 2 years ago

pzhlkj6612 commented 2 years ago

What happened

On the Windows runner, we got two errors (TS7016 and TS2322) during the execution of the "npm run build" step. We can view the logs in test (windows-2019, 5.9.0) or test (windows-2019, 5.15.1):

> install-qt-action@0.0.0 build-setup-python D:\a\install-qt-action\install-qt-action
> npm install --prefix node_modules/setup-python && npm run build --prefix node_modules/setup-python

+ install-qt-action@0.0.0
added 1 package from 1 contributor in 0.14s

> setup-python@2.0.1 build D:\a\install-qt-action\install-qt-action\node_modules\setup-python
> tsc

Error: src/find-python.ts(4,25): error TS7016: Could not find a declaration file for module 'semver'. 'D:/a/install-qt-action/install-qt-action/node_modules/semver/index.js' implicitly has an 'any' type.
  Try `npm install @types/semver` if it exists or add a new declaration (.d.ts) file containing `declare module 'semver';`
Error: src/install-python.ts(39,5): error TS2322: Type '{ [key: string]: string | undefined; } | { LD_LIBRARY_PATH: string; }' is not assignable to type '{ [key: string]: string; } | undefined'.
  Type '{ [key: string]: string | undefined; }' is not assignable to type '{ [key: string]: string; }'.
    Index signatures are incompatible.
      Type 'string | undefined' is not assignable to type 'string'.
        Type 'undefined' is not assignable to type 'string'.
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! setup-python@2.0.1 build: `tsc`
npm ERR! Exit status 2
npm ERR! 
npm ERR! Failed at the setup-python@2.0.1 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\npm\cache\_logs\...-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! install-qt-action@0.0.0 build-setup-python: `npm install --prefix node_modules/setup-python && npm run build --prefix node_modules/setup-python`
npm ERR! Exit status 2
npm ERR! 
npm ERR! Failed at the install-qt-action@0.0.0 build-setup-python script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\npm\cache\_logs\...-debug.log

> install-qt-action@0.0.0 build D:\a\install-qt-action\install-qt-action
> tsc

Essentially, two npm scripts were executed in this step:

Code in the workflow step npm script
npm run build-setup-python npm install --prefix node_modules/setup-python && npm run build --prefix node_modules/setup-python
npm run build tsc

According to the output, we know that the errors occurred when executing npm run build --prefix node_modules/setup-python . However, the workflow was not interrupted at all, and the subsequent call to tsc was successful. In other words, the imported library "node_modules/setup-python/lib/find-python.js" was built successfully.

pzhlkj6612 commented 2 years ago

Why was the CI workflow not interrupted?

npm exited with error code 2:

...

npm ERR! install-qt-action@0.0.0 build-setup-python: `npm install --prefix node_modules/setup-python && npm run build --prefix node_modules/setup-python`
npm ERR! Exit status 2

We executed those commands in pwsh on Windows:

Using a specific shell

Supported platform shell parameter Description Command run internally
Windows pwsh This is the default shell used on Windows. The PowerShell Core. GitHub appends the extension .ps1 to your script name. If your self-hosted Windows runner does not have PowerShell Core installed, then PowerShell Desktop is used instead. pwsh -command ". '{0}'".

PowerShell, currently, doesn't have the ability to handle the error from native commands (executables). Therefore, if we don't check either $? or $LastExitCode after executing commands with npm, the script will continue to execute to the end.

This is easy to fix. But obviously, once it is fixed, the CI workflow will fail.

See also windows - How to stop a PowerShell script on the first error? - Stack Overflow.

ddalcino commented 2 years ago

There are two errors here.

Error: src/find-python.ts(4,25): error TS7016: Could not find a declaration file for module 'semver'. 'D:/a/install-qt-action/install-qt-action/node_modules/semver/index.js' implicitly has an 'any' type. Try npm install @types/semver if it exists or add a new declaration (.d.ts) file containing declare module 'semver';

This one could be a side effect of the way that this action installs setup-python. The setup-python action includes @types/semver as a devDependency, but it appears as if install-qt-action does not have access to it. If you add @types/semver to install-qt-action/package.json, this problem goes away.

Error: src/install-python.ts(39,5): error TS2322: Type '{ [key: string]: string | undefined; } | { LD_LIBRARY_PATH: string; }' is not assignable to type '{ [key: string]: string; } | undefined'. Type '{ [key: string]: string | undefined; }' is not assignable to type '{ [key: string]: string; }'. Index signatures are incompatible. Type 'string | undefined' is not assignable to type 'string'. Type 'undefined' is not assignable to type 'string'.

This one's a little tougher. This is happening because the typescript compiler has detected a type mismatch. setup-python is trying to assign process.env, which is of type { [key: string]: string | undefined; } to options.env, which must have the type { [key: string]: string; } | undefined. In other words, the values attached to the keys in process.env are allowed to be undefined, but in ExecOptions.env, these values must be strings.

I think the solution here is to filter out all of the undefined values in process.env before assigning them to options.env.

I have attempted a solution, and you can see a working build here: https://github.com/ddalcino/install-qt-action/runs/3970883740?check_suite_focus=true#step:5:1 I don't think you'll want to use my solution just yet though; I have modified install-qt-action/package.json to pull a specific commit hash from my personal fork of setup-python. I will try to file a PR with the setup-python maintainers, and see where that gets us.

Edit: most of this comment is irrelevant: see https://github.com/jurplel/install-qt-action/issues/116#issuecomment-949834321 below.

ddalcino commented 2 years ago

After some further thought, I don't think that modifying setup-python is necessary. I tried another build in which install-qt-action uses the same versions of three devDependencies used by setup-python:

    "@types/node": "^12.12.31",
    "@types/semver": "^7.1.0",
    "typescript": "^3.8.3"

This build completes successfully on all 3 platforms.

I think that this change is a much better solution than modifying setup-python, so I will file the PR here instead.

ddalcino commented 2 years ago

PowerShell, currently, doesn't have the ability to handle the error from native commands (executables). Therefore, if we don't check either $? or $LastExitCode after executing commands with npm, the script will continue to execute to the end.

Do we have to use powershell for this step? Bash is available on Windows Github-hosted runners; we could specify shell: bash in this step and get the "fail on errors" behavior by default. Is there a good reason not to do this?

ddalcino commented 2 years ago

Is there any movement on this issue? I opened #123 more than a month ago, and it fixes the root cause of this issue. I also opened #122, which will fix the error on Windows builds that occurs when running dir on this line: https://github.com/jurplel/install-qt-action/blob/10cbdcd020ebd47297c2c40d37d610fed13f7d93/.github/workflows/test.yml#L39

@jurplel, please note that if you run the workflow for either #122 or #123 right now, they will fail because the test.yml workflow specifies a version of QtIFW which is no longer available. You could fix that the same way #118 did, if you're not ready to use #119 yet.

pzhlkj6612 commented 2 years ago

For early adopters of PowerShell (pwsh):

In PowerShell 7.3.0-preview.1, set $PSNativeCommandUseErrorActionPreference to $true and $ErrorActionPreference to 'Stop' can stop script execution when native command error occurs (behaves like set -e in Bash shell).

Test code:

& {
    $PSNativeCommandUseErrorActionPreference = $true;
    $ErrorActionPreference = 'Stop';
    ping.exe bbb;
    ping.exe aaa;
}

Please see PowerShell/PowerShell#3415 for details.