Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pnpm 7.20.0 and later should maybe log which config file is changed when doing "pnpm config set" #5877

Closed
stoyle opened this issue Jan 3, 2023 · 17 comments · Fixed by #6132

Comments

@stoyle
Copy link

stoyle commented Jan 3, 2023

So not really a bug, but still wanted to tell people about it, since this gave us quite a headache when deploying.

Long story short, pnpm 7.20.0 and later makes modifications to local .npmrc file when doing config set, and that may not always be what you want. For instance if overriding store default. E.g. pnpm config set store-dir /home/travis/.cache/.pnpm-store. Before 7.20.0 this was done in the global settings file, e.g. ˙~/.npmrc` on my machine.

We use travis for builds, and most of the build is outside docker. But the final artifact is inside docker. In order to only have relevant artifact we use docker COPYand .dockerignore to get the right things in. This means we copy over .npmrc. However when we run the pnpm config set store-dir /home/travis/.cache/.pnpm-store on the travis instance, the modified file is copied into docker.

We may have a non-standard setup. But we are many in our org which uses it, and we had some trouble debugging it.

I am not sure this is a great idea, up to the project of course. But logging which file is changed when doing a pnpm config set ... probably would have helped us debug this a lot faster.

Love pnpm, keep up the great work!

For reference, the errors we got were of the following sort (if people search for solutions):

Step 4/13 : RUN corepack pnpm install --prefer-frozen-lockfile --prod
---> Running in 4d42479840af
EACCES  EACCES: permission denied, mkdir '/home/travis'
@zkochan
Copy link
Member

zkochan commented Jan 4, 2023

I am sorry that it caused trouble.

The issue is that in the past we did not have a config command. pnpm was passing through the config command to npm. Only v7.20 got its own implementation of the pnpm config command. So, I guess I did not implement it the same way as npm config works. Probably it needs to be fixed.

If npm config set store-dir ... writes to the global config file, then npm should do the same. I guess this happens when there is no .npmrc in the current working directory.

@stoyle
Copy link
Author

stoyle commented Jan 4, 2023

Thanks for the quick reply @zkochan!

Yes, confirmed. Doing a npm config set store-dir foo puts it the value in my ~/.npmrc file not the .npmrc in the current directory. Tested with npm 8.19.2.

@tleunen
Copy link

tleunen commented Jan 10, 2023

Confirming what @stoyle is saying as well. Running pnpm config set does create a new .npmrc file in the current directory instead of writing in the global config.

@zkochan
Copy link
Member

zkochan commented Jan 10, 2023

I have tested with npm CLI. In my case it also created a .npmrc in the current dir

@tleunen
Copy link

tleunen commented Jan 10, 2023

Doesn't seem to do it with 8.5.5 for me. Maybe in npm v9?
I couldn't see any major changes in npm config which could explain that.

Although, since you're saying it's the normal behavior now given npm does that too (for you), I changed my call with pnpm to set the global option to make sure to update the global config instead of a new "local" one

@egg-r
Copy link

egg-r commented Jan 27, 2023

This issue sounds like what's causing me some grief with my Azure DevOps setup. We're using self-hosted agents using Microsoft's Packer templates for base image creation. Then I install pnpm globally via npm within that image. This worked well up until recently. Now the command pnpm config set store-dir $(pnpm_config_cache) is no longer setting the store location. I've tried adding --global as well, but it had not effect.

Versions used:
Node.js: 16.19.0
Npm: 8.19.3
Pnpm: 7.25.1

Example:
image
image

@marchy
Copy link

marchy commented Feb 5, 2023

@zkochan Wanted to say that our Azure Pipelines builds have also stopped working since this.

I have a thread going here with trying out different workarounds, but so far we haven't figured out anything that works.

Let me know if there is anything we should try out.

Our pipelines steps can be seen in the post (replicated here for ease):

steps:
  # PNPM (REF: https://pnpm.io/continuous-integration#azure-pipelines)
  - task: Cache@2
    inputs:
      key: 'pnpm | "$(Agent.OS)" | {Project Name}/pnpm-lock.yaml'
      path: '$(pnpm_config_cache)'
    displayName: Cache PNPM
  - script: |
      curl -f https://get.pnpm.io/v6.16.js | node - add --global pnpm@7
      pnpm config set store-dir $(pnpm_config_cache)
    displayName: "Setup PNPM"
  # NOTE: we are performing the following as part of the MSBuild task
  - script: |
        pnpm install --frozen-lockfile
        displayName: "pnpm install"
    workingDirectory: '{Project Dir}'
    displayName: "Install PNPM"

which is mostly as per the integrations steps, and confirmed works all the way up to v7.19.0

@gethari
Copy link

gethari commented Feb 20, 2023

@marchy I'm sorry if if my question is dumb. Is'nt the latest pnpm verson supposed to be v7 ? Why does the installation docs on CI point to download to this link curl -f https://get.pnpm.io/v6.16.js , which says v6.16 ??

@marchy
Copy link

marchy commented Feb 24, 2023

@gethari not dumb at all, it was purely because this was the command listed on the Continuous Integration docs page, and there was a question asked about this but it was apparently working regardless of the PNPM version specified.

HOWEVER... it seems the docs were just updated to now use 'corepack' instead – so this part of the puzzle at least is now moot.

- script: |
      corepack enable
      corepack prepare pnpm@latest-7 --activate
      pnpm config set store-dir $(pnpm_config_cache)
    displayName: "Setup PNPM"

The original issue is still not working however, getting the following message in the Install PNPM step: Content-addressable store is at: D:\.pnpm-store\v3 despite the config command setting it to a different path in the previous Cache PNPM step (pnpm config set store-dir D:\a\1/.pnpm-store)

  - task: Cache@2
    inputs:
      key: 'pnpm | "$(Agent.OS)" | MyProject/pnpm-lock.yaml'
      path: '$(pnpm_config_cache)'
    displayName: Cache PNPM
  - script: |
      corepack enable
      corepack prepare pnpm@latest-7 --activate
      pnpm config set store-dir $(pnpm_config_cache)
    displayName: "Setup PNPM"
  - script: |
      pnpm install --frozen-lockfile
      pnpm run build
    workingDirectory: 'MyProject'
    displayName: "Install PNPM"

@zkochan (!!) Unfortunately the prior workaround to use the 6.19.0 version (before the new config command was introduced) has now also stopped working. This has turned into a P1 issue as we cannot deploy our builds at all anymore – and the fallback to the old version is now not working anymore 🔥

Can we chase down why the pnpm config set store-dir command is not setting the store location / not getting picked up in the subsequent pnpm install command?

@zkochan
Copy link
Member

zkochan commented Feb 25, 2023

I don't understand why you cannot deploy without caching the store. Caching the store is not required. TBH, the job might be faster without caching the store.

@marchy
Copy link

marchy commented Feb 25, 2023

@zkochan is that an option? What would be the commands to do that?

There are no such instructions in the docs – and for sure, if it isn't necessary the build agent gets reset anyways between runs so there is no benefit to caching among runs

@zkochan
Copy link
Member

zkochan commented Feb 25, 2023

I assume you need to remove parts related to cache. Something like this

steps:
  - script: |
      corepack enable
      corepack prepare pnpm@latest-7 --activate
    displayName: "Setup pnpm"

  - script: |
      pnpm install
      pnpm run build
    displayName: "pnpm install and build"

@egg-r
Copy link

egg-r commented Feb 25, 2023

@zkochan this is still not working for me either in Azure DevOps, but in my case using Ubuntu runners. Same general steps. Caching is important to us as we're dealing with a bit of a large mono-repo for builds. I just switched this week to use corepack, but still having the same issue were Set is not updating the store path. This started when pnpm was updated to use it's own config setup, rather than using npm's in the background.

steps:
- bash: |
    : ${PNPM_CONFIG_CACHE:=$(Pipeline.Workspace)/.pnpm-store}
    : ${PNPM_CONFIG_NODE_LINKER:=isolated}

    echo "##vso[task.setvariable variable=PNPM_CONFIG_CACHE]$PNPM_CONFIG_CACHE"
    echo "##vso[task.setvariable variable=PNPM_CONFIG_NODE_LINKER]$PNPM_CONFIG_NODE_LINKER"
  displayName: Set up common PNPM build environment variable defaults

- bash: |
    : ${PNPM_VERSION:=latest}

    corepack enable
    corepack prepare pnpm@$PNPM_VERSION --activate
  displayName: Install Pnpm

- bash: pnpm config set store-dir $(PNPM_CONFIG_CACHE)
  displayName: Setup pnpm

- bash: pnpm config set node-linker $(PNPM_CONFIG_NODE_LINKER)
  displayName: Setup pnpm node-linker

- bash: pnpm install
  displayName: Install PNPM packages
  workingDirectory: $(PNPM_BUILD_CONTEXT_PATH)

image

image

@marchy
Copy link

marchy commented Feb 25, 2023

OMG that worked. So many days spent chasing this *head slap*
I completely assumed the PNPM cache was required for PNPM to work.

Really appreciate the super prompt replies @zkochan.
It shaved 15s off the build as well.

Could we please update the docs to mention that the caching is an optional step that is only useful for local build agents? (ie: not cloud agents which are ephemeral and clear out the file system image between builds anyways)

@zkochan
Copy link
Member

zkochan commented Feb 25, 2023

I am changing where pnpm config set writes the config: #6132

@marchy we can update the docs.

@egg-r I don't know why that doesn't work. Maybe try to use pnpm config --global set <key> <value>. Also, you can try to use npm config set instead. npm config set used to allow setting pnpm setting, so if you're not use the latest npm version, it might work.

@zkochan
Copy link
Member

zkochan commented Feb 25, 2023

@egg-r in my case the store is changed:

2023-02-25T16:07:01.6659225Z ##[section]Starting: pnpm install
2023-02-25T16:07:01.6666115Z ==============================================================================
2023-02-25T16:07:01.6666377Z Task         : Command line
2023-02-25T16:07:01.6666508Z Description  : Run a command line script using Bash on Linux and macOS and cmd.exe on Windows
2023-02-25T16:07:01.6666741Z Version      : 2.212.0
2023-02-25T16:07:01.6666866Z Author       : Microsoft Corporation
2023-02-25T16:07:01.6667032Z Help         : https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/command-line
2023-02-25T16:07:01.6667256Z ==============================================================================
2023-02-25T16:07:01.8148158Z Generating script.
2023-02-25T16:07:01.8156963Z ========================== Starting Command Output ===========================
2023-02-25T16:07:01.8178952Z [command]/usr/bin/bash --noprofile --norc /home/vsts/work/_temp/ef52fa48-192c-409b-b7ca-5818e428ee86.sh
2023-02-25T16:07:02.2565005Z /home/vsts/work/1/.pnpm-store
2023-02-25T16:07:02.7198276Z Lockfile is up to date, resolution step is skipped
2023-02-25T16:07:02.7297668Z Progress: resolved 1, reused 0, downloaded 0, added 0
2023-02-25T16:07:02.7369213Z Packages: +2
2023-02-25T16:07:02.7369728Z ++
2023-02-25T16:07:02.8562093Z Packages are hard linked from the content-addressable store to the virtual store.
2023-02-25T16:07:02.8562952Z   Content-addressable store is at: /home/vsts/work/1/.pnpm-store/v3
2023-02-25T16:07:02.8592501Z   Virtual store is at:             node_modules/.pnpm
2023-02-25T16:07:03.1622319Z 
2023-02-25T16:07:03.1624901Z dependencies:
2023-02-25T16:07:03.1629806Z + is-odd 0.1.0
2023-02-25T16:07:03.1641279Z 
2023-02-25T16:07:03.1644798Z Done in 806ms
2023-02-25T16:07:03.7304723Z Progress: resolved 2, reused 0, downloaded 2, added 2, done
2023-02-25T16:07:03.7474836Z ##[section]Finishing: pnpm install

@rulatir
Copy link

rulatir commented Aug 17, 2023

What exactly is "completed in #6132"? The original request was to implement logging of the excact destination where the changes go. Was that implemented?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants