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

Binary right is not preserved with pnpm #1247

Open
gregberge opened this issue May 1, 2024 · 10 comments
Open

Binary right is not preserved with pnpm #1247

gregberge opened this issue May 1, 2024 · 10 comments

Comments

@gregberge
Copy link

Describe the bug

Using pnpm the binary right is not preserved. For example, the odiff package is broken.

By investigating I found that the underlying binary used in the package has lost its execution right after deploying on Heroku:

Fatal error: exception Unix.Unix_error(Unix.EACCES, "execve", "/app/node_modules/.pnpm/[email protected]/node_modules/odiff-bin/3/i/odiff-33ddbef3/bin/ODiffBin")

To Reproduce

  • Install odiff
  • Run a script on Heroku that use it

Versions (please complete the following information):

  • Heroku Stack: Heroku-22
  • Node Version: 20
  • NPM or Yarn Version: pnpm v9
  • Buildpack Version: v245

Additional context
Add any other context about the problem here.

@colincasey
Copy link
Contributor

Hi @gregberge, I'm having trouble reproducing this bug to investigate further. Can you provide a bit more information around how you're calling odiff?

As a quick test, I tried deploying an app using the following package.json file + some image files:

{
  "name": "issue-1247",
  "version": "0.0.0",
  "dependencies": {
    "odiff-bin": "2.6.1"
  },
  "engines": {
    "node": "20.x"
  },
  "scripts": {
    "build": "odiff tiger.jpg tiger-2.jpg output.jpg"
  },
  "packageManager": "[email protected]+sha256.0624e30eff866cdeb363b15061bdb7fd9425b17bc1bb42c22f5f4efdea21f6b3"
}

The command works though the build step fails because odiff reports that the images are different:

remote: -----> Build
remote:        Running build
remote:
remote:        > [email protected] build /tmp/build_6711e685
remote:        > odiff tiger.jpg tiger-2.jpg output.jpg
remote:
remote:        Failure! Images are different.
remote:        Different pixels: 7586 (1.137331%)
remote:         ELIFECYCLE  Command failed with exit code 22.

@haubey
Copy link

haubey commented May 2, 2024

We are also running into this issue with sentry/cli. It seems to work when the package is included in dependencies but doesn't work (EACCES) when added in devDependencies.

@colincasey
Copy link
Contributor

@haubey Can you provide details on how you're calling @sentry/cli?

@haubey
Copy link

haubey commented May 2, 2024

@colincasey sure thing.

Before pnpm was installed via corepack we had the following build script (pared down)

NODE_ENV=production
npm i -g [email protected]

pnpm i --prod --frozen-lockfile

set +e # Prevents a Sentry error from stalling the Heroku deployment

# Ensures that sourcemaps are only uploaded from Heroku
if [ -n "${SOURCE_VERSION}" ]; then
  pnpm i -g @sentry/cli
  sentry-cli releases new $SOURCE_VERSION
  sentry-cli releases  files $SOURCE_VERSION upload-sourcemaps ./packages/our-package/build
  sentry-cli releases finalize $SOURCE_VERSION
fi

# Prevents a sentry-cli failure from crashing the Heroku builds
exit 0

And it worked fine. With the codepack change we removed npm i -g [email protected] and added it as an engine in our package.json instead. At that point the pnpm i -g @sentry/cli call started not being runnable. We then moved it to dependencies and it worked, then devDependencies and it failed. However we're also experiencing some issues around our pnpm cache with the move to engines/packageManager so I can't say for certain what the exact issue we're experiencing is. Once we nail it down, I will be sure to follow up here!

@gregberge
Copy link
Author

Gently bumping this one. @colincasey any news on that?

@colincasey
Copy link
Contributor

@gregberge the only update here is that I also tried to reproduce the issue using the information provided by @haubey. The results from that are summarized below:

Package Declared In Command Build time Run time
@sentry/cli dependencies sentry-cli -V Success Success
@sentry/cli devDependencies sentry-cli -V Success Error - Not found

Note

The failure at run time when it's installed as a dev dependency is expected since, by default, dev dependencies are pruned at the end of the build.

At no point did I observe any EACCES issues calling the @sentry/cli binary installed by pnpm.

To investigate this further, it would help if you could provide some information around how you're calling odiff or share a minimal reproduction of the issue.

@dzuelke
Copy link
Contributor

dzuelke commented Sep 10, 2024

Do y'all by any chance have node_modules checked into Git, @gregberge and @haubey ?

@dzuelke
Copy link
Contributor

dzuelke commented Sep 10, 2024

Actually, could it be pnpm/pnpm#6285, or more specifically this: aws/aws-pdk#322 (comment)?

It appears PNPM does this intentionally for portability reasons and requires executables to either be listed in bin or publishConfig.executableFiles in package.json.

@haubey
Copy link

haubey commented Sep 10, 2024

@dzuelke we do not have node_modules checked into git. For now we've turned caching off and things are generally working for us, though some services like Fontawesome charge for bandwidth so it would still be good to figure out a more cache-able solution.

@colincasey
Copy link
Contributor

I've been able to make a bit of headway here in terms of a minimal reproduction on the Heroku platform. I'm going to record my notes here.

Reproducing

  1. Create a new folder containing the following package.json:

    {
      "name": "eaccess-test",
      "private": true,
      "scripts": {
        "build": "sentry-cli --version"
      },
      "packageManager": "[email protected]+sha512.3f9e23f20bdbf7a27b87b804383f1dafdb5cb35cdc40fce590aff2215255446ff595878ee4f33429e6a0e7c3882b1ae926514f6fea6a5ba75e52f87bfc2592e7",
      "dependencies": {
        "@sentry/cli": "1.77.3"
      },
      "engines": {
        "node": "18.20.2"
      }
    }
  2. Setup the remaining files (e.g.; pnpm-lock.yaml, .gitignore):

    pnpm install
    echo "node_modules" > .gitignore
  3. Create the Heroku app and deploy it:

    heroku create
    git add .
    git commit -m "test eaccess app"
    git push
  4. Redeploy the app:

    git commit --allow-empty -m "trigger build" 
    git push 

Observations

  • Builds using a fresh cache will succeed

  • Builds using a restored cache will encounter the EACCESS error

  • The @sentry/cli module uses a postinstall script to fetch an appropriate binary for the target architecture (the odiff-bin package mentioned in this issue also does something similar)

  • The permissions of the downloaded sentry-cli binary are as follows:

    Cache State Permissions in
    node_modules/.pnpm
    Permissions in
    pnpm store
    Fresh -rwx------ -rw-------
    Restored -rw------- -rw-------

I haven't been able to reproduce the above in a local environment (yet). Only on the Heroku platform. This situation has made isolating a root cause for this behavior more difficult and time consuming.

Workarounds

Either of the following workarounds will prevent the EACCESS error by forcing the postinstall hook to execute.

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

No branches or pull requests

4 participants