Fix variable reference for PR_REF in workflow

[glegendre01]

No description provided.

[coyotte508]

here too?

[glegendre01]

not needed, base.ref is main.

[coyotte508]

unless you do a PR on top of another PR?

[coyotte508]

(ok to merge after this is is fixed, for me)

[ydshieh]

unless you do a PR on top of another PR?

In this case, the workflow won't be trigger in our repository (if the the base PR is also in a forked repository), or if the base PR is a branch in our repository, there should be no security issue as the branch is created by a team member (if we assume we could trust team members for branch name).

But I personally go for avoiding any {{ }} and use env variables.

I don't push the change, but here it is

(with the help of AI, need review if you take it)

name: Test
on:
  pull_request:
  push:
    branches:
      - main

jobs:
  node:
    runs-on: ubuntu-latest
    timeout-minutes: 10

    steps:
      - uses: actions/checkout@v3
        with:
          # Could use something like rmacklin/fetch-through-merge-base@v0 instead when the repo gets bigger
          fetch-depth: 0
      - name: "Extracting the merge base into 'SINCE'"
        id: since
        env: 
          PR_REF: ${{ github.event.pull_request.head.ref }}
          PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
          GITHUB_SHA: ${{ github.sha }}
        run: |
          if [ -z "$PR_REF" ]
          then
            echo "SINCE=${GITHUB_SHA}^1" >> $GITHUB_OUTPUT
          else
            echo "SINCE=$(git merge-base origin/${PR_BASE_REF} ${GITHUB_SHA})" >> $GITHUB_OUTPUT
          fi

      - run: npm install -g corepack@latest && corepack enable

      - uses: actions/setup-node@v3
        with:
          node-version: "20"
          cache: "pnpm"
          cache-dependency-path: "**/pnpm-lock.yaml"
      - name: Install and build
        env:
          SINCE_REF: ${{ steps.since.outputs.SINCE }}
        run: |
          pnpm install --frozen-lockfile --filter .
          pnpm install --frozen-lockfile --filter ...["${SINCE_REF}"]...
          pnpm --filter ...["${SINCE_REF}"]... build

      - name: Test
        env:
          SINCE_REF: ${{ steps.since.outputs.SINCE }}
          HF_TOKEN: ${{ secrets.HF_TOKEN }}
        run: pnpm --filter ...["${SINCE_REF}"] test

  browser:
    runs-on: ubuntu-latest
    timeout-minutes: 10

    steps:
      - uses: actions/checkout@v3
        with:
          # Could use something like rmacklin/fetch-through-merge-base@v0 instead when the repo gets bigger
          fetch-depth: 0
      - name: "Extracting the merge base into 'SINCE'"
        id: since
        env: 
          PR_REF: ${{ github.event.pull_request.head.ref }}
          PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
          GITHUB_SHA: ${{ github.sha }}
        run: |
          if [ -z "$PR_REF" ]
          then
            echo "SINCE=${GITHUB_SHA}^1" >> $GITHUB_OUTPUT
          else
            echo "SINCE=$(git merge-base origin/${PR_BASE_REF} ${GITHUB_SHA})" >> $GITHUB_OUTPUT
          fi

      - run: google-chrome --version

      - run: npm install -g corepack@latest && corepack enable

      - uses: actions/setup-node@v3
        with:
          node-version: "22"
          cache: "pnpm"
          cache-dependency-path: "**/pnpm-lock.yaml"
      - name: Install and build
        env:
          SINCE_REF: ${{ steps.since.outputs.SINCE }}
        run: |
          pnpm install --frozen-lockfile --filter .
          pnpm install --frozen-lockfile --filter ...["${SINCE_REF}"]...
          pnpm --filter ...["${SINCE_REF}"]... build

      - name: Test in browser
        env:
          SINCE_REF: ${{ steps.since.outputs.SINCE }}
          HF_TOKEN: ${{ secrets.HF_TOKEN }}
        run: pnpm --filter ...["${SINCE_REF}"] test:browser

  e2e:
    runs-on: ubuntu-latest
    timeout-minutes: 10

    steps:
      - uses: actions/checkout@v3

      - run: npm install -g corepack@latest && corepack enable

      - uses: actions/setup-node@v3
        with:
          node-version: "20"
          cache: "pnpm"
          cache-dependency-path: "**/pnpm-lock.yaml"

      - name: E2E - start mock npm registry
        run: |
          npm i -g verdaccio verdaccio-memory verdaccio-auth-memory
          echo >> .npmrc
          echo "//localhost:4874/:_authToken=1FlYaQkIdHsIztXCHmqu9g==" >> .npmrc
          npx verdaccio --listen 4874 --config e2e/mock-registry-config.yaml &

      - name: E2E test - publish packages to mock repo
        working-directory: e2e
        run: |
          sleep 3
          pnpm i --filter root --filter inference... --filter hub... --filter tasks-gen --frozen-lockfile
          pnpm --filter inference --filter hub --filter tasks --filter jinja publish --force --no-git-checks --registry http://localhost:4874/

      - name: E2E test - test yarn install
        working-directory: e2e/ts
        run: |
          npm i -g yarn --force
          # yarn now looks at root package.json for the package manager...
          mv ../../package.json ../../package.json.bk
          yarn install --registry http://localhost:4874/
          mv ../../package.json.bk ../../package.json

      - name: E2E test - typescript node project
        working-directory: e2e/ts
        env:
          HF_TOKEN: ${{ secrets.HF_TOKEN }}
        run: |
          pnpm i --ignore-workspace --registry http://localhost:4874/
          pnpm start

      - name: E2E test - svelte app build
        working-directory: e2e/svelte
        run: |
          pnpm i --ignore-workspace --registry http://localhost:4874/
          pnpm build
          pnpm run test:browser

      - uses: denoland/setup-deno@v1
        with:
          deno-version: vx.x.x
      - name: E2E test - deno import from npm
        working-directory: e2e/deno
        env:
          NPM_CONFIG_REGISTRY: http://localhost:4874/
          HF_TOKEN: ${{ secrets.HF_TOKEN }}
        run: deno run --allow-read --allow-net --allow-env=HF_TOKEN index.ts

[ydshieh]

LGTM, although I personally would change any usage (( ... }} to env. variables.

[coyotte508]

here, maybe it'll fix the lint problem we've had (lint job succeeding before merging)

cc @Wauplin for viz

[coyotte508]

needs an approval from @huggingface/security before it can be merged

[ydshieh]

I leave this for you two. For security issue we intent to fix, this current version is good.

[ydshieh]

LGTM

[coyotte508]

merged via git since I couldn't do it on github interface