CircleCI-Public / node-orb

An orb for working with Node.js on CircleCI
https://circleci.com/orbs/registry/orb/circleci/node
MIT License
52 stars 80 forks source link

fix: support cache computation on windows #157

Closed GerkinDev closed 2 years ago

GerkinDev commented 2 years ago

On windows runners, the cache keys fails to be computed because of the checksum function that can't find the file in /tmp. Indeed, it seems that bash /tmp is not the runner's C:\tmp directory.

See https://app.circleci.com/pipelines/github/KnodesCommunity/typedoc-plugins/693/workflows/897cb674-230e-46ae-99f5-b10d93f4b723/jobs/3872/parallel-runs/0/steps/0-106 for an example of failed step

KyleTryon commented 2 years ago

Thank you for this @GerkinDev, This looks like an excellent change. I just ran the tests and got hung up on the Shellcheck step. There is a suggestion for an alternative to the current if statement, would you mind making this change?

In ./src/scripts/packages/determine-lockfile.sh line 2: if ! [ -z $HOMEDRIVE ]; then ^-- SC2237: Use [ -n .. ] instead of ! [ -z .. ].

For more information: https://www.shellcheck.net/wiki/SC2237 -- Use [ -n .. ] instead of ! [ -z .... Exited with code exit status 1

GerkinDev commented 2 years ago

@KyleTryon fixed. Please note that I don't know how to properly test the changes, that were done mostly blind. A proper test on MacOS/Linux/Windows would be great

orb-publisher commented 2 years ago

Your development orb has been published. It will expire in 30 days. You can preview what this will look like on the CircleCI Orb Registry at the following link: https://circleci.com/developer/orbs/orb/circleci/node?version=dev:85c351a8b7b6c5fb94c89c0ed081155e2a9d66a6

orb-publisher commented 2 years ago

Your development orb has been published. It will expire in 30 days. You can preview what this will look like on the CircleCI Orb Registry at the following link: https://circleci.com/developer/orbs/orb/circleci/node?version=dev:f000823fa5e5cef7a7e3980e4f7b4e0ca80a2713

EricRibeiro commented 2 years ago

@KyleTryon fixed. Please note that I don't know how to properly test the changes, that were done mostly blind. A proper test on MacOS/Linux/Windows would be great

Thanks, @GerkinDev! Your changes are working great with the existing macOS and Linux tests. I included one for Windows here: https://github.com/CircleCI-Public/node-orb/pull/157/commits/4a4fca687a8aabec83b83637e236b93febe3ff18.

GerkinDev commented 1 year ago

Glad I can count on caching now :) thank you !

seanssel commented 11 months ago

Not sure if I'm missing something, but dependency caching doesn't seem to work for me on Windows runners (latest version, just running install-packages). It finds a cache at C:\Users\circleci\.npm, but there are always just a few bytes in there. Also seeing a bunch of (node:1272) MaxListenersExceededWarning: Possible EventEmitter memory leak detected during this step.

I know Windows isn't officially supported, but I'm curious how people are using this successfully.