yarnpkg / berry

📦🐈 Active development trunk for Yarn ⚒
https://yarnpkg.com
BSD 2-Clause "Simplified" License
7.45k stars 1.11k forks source link

[Bug?]: Yarn removes executable permissions when doing a partial install #6551

Open JavaScriptBach opened 1 month ago

JavaScriptBach commented 1 month ago

Self-service

Describe the bug

When I run yarn install from scratch, everything installs correctly. When I then upgrade a package (in this case "mocha"), Yarn removes executable permissions, making the binary unrunnable.

To reproduce

https://github.com/JavaScriptBach/yarn-permissions-bug

Environment

System: OS: macOS 14.7 CPU: (10) arm64 Apple M1 Pro Binaries: Node: 18.14.0 - /private/var/folders/kv/7d2gvknd4mv1vdlrk9srf0r00000gn/T/xfs-0a868b6a/node Yarn: 4.5.0 - /private/var/folders/kv/7d2gvknd4mv1vdlrk9srf0r00000gn/T/xfs-0a868b6a/yarn npm: 9.3.1 - ~/.nvm/versions/node/v18.14.0/bin/npm

Additional context

No response

larixer commented 1 month ago

Bug confirmed - it happens when the bin entry points to a file in package tarball without executable permission and the package is updated. As for the fix, it requires a bit of effort. Currently BinSymlinkMap type used by the nm linker has the structure:

Map() {
  '<...>/yarn-permissions-bug' => Map() {
    'mocha' => '<...>/yarn-permissions-bug/node_modules/mocha/bin/mocha.js',
    '_mocha' => '<...>/yarn-permissions-bug/node_modules/mocha/bin/_mocha',
    ...
  }
}

On update the nm linker "knows" that <...>/yarn-permissions-bug/node_modules/mocha is going to be updated, however, it is difficult to write a fast code that removes entries from BinSymlinkMap structure. To make the fast removal possible BinSymlinkMap should be changed to something like:

Map() {
  '<...>/yarn-permissions-bug/node_modules/mocha' => Map() {
    'mocha' => Map() { 
        'binDirRoot': '<...>/yarn-permissions-bug',
        'target' : './bin/mocha.js',
    },
    '_mocha' => Map() { 
        'binDirRoot': '<...>/yarn-permissions-bug',
        'target' : './bin/_mocha',
    }
  }
}
larixer commented 1 month ago

binDirRoot can be omitted, it can be computed from the package location: <...>/yarn-permissions-bug/node_modules/mocha should have their executable bin entries in: <...>/yarn-permissions-bug/.bin/.... So new BinSymlinkMap structure might look like this:

Map() {
  '<...>/yarn-permissions-bug/node_modules/mocha' => Map() {
    'mocha' => './bin/mocha.js',
    '_mocha' =>  './bin/_mocha',
  }
}

When nm linker tries to remove '<...>/yarn-permissions-bug/node_modules/mocha' the corresponding entry should be removed from prevBinSymlinks, meaning that previous bin symlinks have "invalid" entries for this location that should not be kept.