Skip to content
Merged
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 29 additions & 21 deletions .github/actions/base-setup/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,34 +61,40 @@ runs:
restore-keys: |
${{ env.CACHE_PREFIX }}-pip-${{ env.PYTHON_VERSION }}

- name: Install node
uses: actions/setup-node@v6
with:
node-version: ${{ env.NODE_VERSION }}
- name: Detect package manager
id: detect-pm
shell: bash
run: |
if [ -f pnpm-lock.yaml ]; then
echo "manager=pnpm" >> $GITHUB_OUTPUT
elif [ -f yarn.lock ]; then
echo "manager=yarn" >> $GITHUB_OUTPUT
elif [ -f package-lock.json ]; then
echo "manager=npm" >> $GITHUB_OUTPUT
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Do we know of any repositories where the lock file is in a subdirectory?
  • What happens if there is no lock file, e.g. a repo does not include frontnend code? I think jupyter-ai repo is one example that falls in that category. Should we skip installing node in that case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know of any repositories where the lock file is in a subdirectory?

Yes there are some, for example https://github.com/voila-dashboards/voici/blob/main/python/voici-core/yarn.lock

What happens if there is no lock file, e.g. a repo does not include frontnend code? I think jupyter-ai repo is one example that falls in that category. Should we skip installing node in that case?

For Python monorepos probably it would make sense to not install Node. But not sure how disruptive this could be for those assuming Node was always available.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, just pushed a change to use the built-in hashFiles to more easily detect any lock files that may be located in subfolders.

Maybe we can look into skipping the setup-node separately.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can look into skipping the setup-node separately.

Opened #271


# Cache yarn
# We cannot use the builtin cache because it errors out with the files
# are not present.
- name: Get yarn cache directory path
id: yarn-cache-dir-path
- name: Install pnpm
if: steps.detect-pm.outputs.manager == 'pnpm'
uses: pnpm/action-setup@v4

- name: Enable corepack
shell: bash
run: |
# Enable corepack if the project defines packageManager
if [ -f package.json ]; then
export HAS_PKG_MANAGER=$(grep -e \"packageManager\"\: package.json)
HAS_PKG_MANAGER=$(grep -e \"packageManager\"\: package.json)
if [ ! -z "$HAS_PKG_MANAGER" ]; then corepack enable; fi
fi
CACHE_DIR=$(yarn config get cacheFolder)
[[ "$CACHE_DIR" == "undefined" ]] && CACHE_DIR=$(yarn cache dir)
echo "dir=$CACHE_DIR" >> $GITHUB_OUTPUT
- name: Cache yarn
uses: actions/cache@v4
id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`)

- name: Install node
uses: actions/setup-node@v6
with:
path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
key: ${{ env.CACHE_PREFIX }}-yarn-${{ env.NODE_VERSION }}-${{ hashFiles('**/yarn.lock') }}
restore-keys: |
${{ env.CACHE_PREFIX }}-yarn-${{ env.NODE_VERSION }}
node-version: ${{ env.NODE_VERSION }}
cache: ${{ steps.detect-pm.outputs.manager }}
cache-dependency-path: |
**/package-lock.json
**/yarn.lock
**/pnpm-lock.yaml

- name: Cache checked links
if: ${{ matrix.group == 'link_check' }}
Expand Down Expand Up @@ -147,5 +153,7 @@ runs:
echo "::group::piplist"
pip list || true
echo "::endgroup::"
yarn --version
node --version
npm --version || true
yarn --version || true
pnpm --version || true
Loading