fix(core): ensure proper install for git dependency using Yarn classic and npm (#4403)

* fix(core): ensure proper install for git dependency with npm prepack

* refactor: don't use variables outside of the test

* test: expect on the part that rejects

* chore: versions

* fix(core): ensure proper install for git dependency with prepack

* fix(core): ensure proper install for git dependency with prepack

* fix(core): ensure proper install for git dependency with prepack

* fix(core): ensure proper install for git dependency with prepack

* fix(core): ensure proper install for git dependency with prepack

* fix(core): ensure proper install for git dependency with prepack

* fix(core): ensure proper install for git dependency with prepack

* chore: style

Co-authored-by: merceyz <[email protected]>

Author: colincasey

.yarn/versions/df71c502.yml

@@ -0,0 +1,33 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/core": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-exec"
+  - "@yarnpkg/plugin-file"
+  - "@yarnpkg/plugin-git"
+  - "@yarnpkg/plugin-github"
+  - "@yarnpkg/plugin-http"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-link"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-pnpm"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/builder"
+  - "@yarnpkg/doctor"
+  - "@yarnpkg/nm"
+  - "@yarnpkg/pnpify"
+  - "@yarnpkg/sdks"

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/HEAD

@@ -0,0 +1 @@
+ref: refs/heads/master

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/config

@@ -0,0 +1,6 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = true
+	ignorecase = true
+	precomposeunicode = true

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/description

@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/hooks/post-update

@@ -0,0 +1,8 @@
+#!/bin/sh
+#
+# An example hook script to prepare a packed repository for use over
+# dumb transports.
+#
+# To enable this hook, rename this file to "post-update".
+
+exec git update-server-info

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/info/exclude

@@ -0,0 +1,6 @@
+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/info/refs

@@ -0,0 +1 @@
+8d338e539dda97c94b22ceb7dacba3753189b548	refs/heads/master

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/03/0257e4e87a536fe9b3c90a6240d5b2462aa1cc

@@ -0,0 +1,2 @@
+x
��Q
+�0��S�,I4I�<ɚ�5Ԩ���7^�_�xLXsN��" m��VF���AJ�q�b�����I���Z�_�@�;}�N\é���i�;?@Y��2�V��VJQm�2�y˖�	�f+�ax��0����A�

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/03/a7dd4d44b65bd459efbeb058fa24ecf01fbb5f

@@ -0,0 +1,2 @@
+x
]��� �=��K/Bh��|��H��mcb��B��Mfg��
+�w�|�0^F�@\ʎ�|�,c�h�S��)��ա�V��g�|�\������b�(�QR�ys�~�a�8��7��?o1=L7��=�+�_��v�����lf_c�9�

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/05/c1e8b5f3dacd39234db4778843119bfe7359e1

@@ -0,0 +1,3 @@
+x
��A
+�0E��)�-�$�J)x�I2�R��颷�^���y��ť��i��$n���-
+�w&u�u���3.�Ψ�7�+x�9��)4��Ɉ�D�)�.�I��d�l�/�8Cϻ|�O�⩞���t�򀦵9g5�5�:�Y�ϻ���n�r|C���[2L�

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/8d/338e539dda97c94b22ceb7dacba3753189b548

@@ -0,0 +1,2 @@
+x
��Q� D��{
(lb�IOˢ�Ғ?���
+~��%o2��27�=��2bУ�)F�sF��Lhe����F�E/�%�>ڬS��q�&E��F+�Ev�o(>��n0��y�)���+q��%�����h��,g���������Rԍk�4ޛ�
3�KL

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/95/fad9bf2b1ca94f0054c4adb68f8bc23b0eb024

@@ -0,0 +1,2 @@
+x
U̱
+�0�qg�ⶤP��EW���(isb$�����M�R\�~߿��C�ۮr��
�>�����|x$P�k�~�sw2���ߊ�����>�Y��Le�i��A3kxbQ��H:Ylp�8�W!�ɱ\�5��l?h�A

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/ad/3d74b1406d3978c109b8be28a0be96b45ea2bc

@@ -0,0 +1 @@
+x
-˽@0`u�¤��Px}�"�_��w䚝�vo6~��~���q',��3�v�� ���_/��&PT��e_�q)B�t҅���>Mw�
n���8��>'0

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/b0/f4580fd4a6c9b1b63458dbec2c6dd6dae4429d

@@ -0,0 +1,3 @@
+x
U��
+�0E]��K
+���J)�W.����#F�$&���nZ|���9��Z��e>k��p(ڝTx���P���C�N�%�������V�y ���C h���|��j�Ӣ����zb�w�&T�X�x��##';��ت`q�3mP�ao�#�V8�{M|4��/��+�P�

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/b1/0e3b99a0734870b04b3a1c471645b6fba5cc3c

@@ -0,0 +1 @@
+x
]���0���V�.4J*&�x���D��Q\�J���7�K��w�t���ԇO<��řwtţ�"&�u��U0�~v(�����&{���YX�/�gTc}��O	r̻C�c�'�©��P��G�$��M[���Y�-������nw����Ȧ���?

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/be/f26554f462b4ab8ed76f5ccdda2af79fadefe2

@@ -0,0 +1,5 @@
+P pack-deb3ab14c86f9987f651a205edb2adf1fee1f0bc.pack
+P pack-4832094f546294dfed5d785d57d7c87052827240.pack
+P pack-46f82cca8c5d7f3facc0cde372bbca42b3128a7f.pack
+P pack-bd0d366d37ae3fd802844fff1b8e5328bc2294c5.pack
+

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/d2/280ae1948b354ce62895125640f104029fcd92

@@ -0,0 +1,2 @@
+# pack-refs with: peeled fully-peeled sorted 
+8d338e539dda97c94b22ceb7dacba3753189b548 refs/heads/master

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/da/680dda2950e2a984d7d2003da76afba748c474

@@ -0,0 +1 @@
+8d338e539dda97c94b22ceb7dacba3753189b548

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/e2/5a7932ead0c2586b0079ab4dcb7627a6f97fde

@@ -124,7 +124,15 @@ describe(`Protocols`, () => {
           },
         },
         async ({path, run, source}) => {
-          await run(`install`);
+          await expect(run(`install`, {
+            env: {
+              // if this is set then yarn 1 will be executed as if `--production` was passed during the install
+              // but `yarn-1-project` requires dev dependencies to be present so this is a good way to
+              // verify that yarn isn't throw off by this when handling the clone, install, and pack process
+              // for git dependencies (see: https://classic.yarnpkg.com/lang/en/docs/cli/install/#toc-yarn-install-production-true-false)
+              NODE_ENV: `production`,
+            },
+          })).resolves.toBeTruthy();
 
           await expect(source(`require('yarn-1-project')`)).resolves.toMatch(/\byarn\/1\.[0-9]+/);
         },
@@ -149,6 +157,38 @@ describe(`Protocols`, () => {
       45000,
     );
 
+    test(
+      `it should guarantee that all dependencies will be installed when using npm to setup npm repositories`,
+      makeTemporaryEnv(
+        {
+          dependencies: {
+            [`npm-has-prepack`]: startPackageServer().then(url => `${url}/repositories/npm-has-prepack.git`),
+          },
+        },
+        async ({path, run, source}) => {
+          await expect(run(`install`, {
+            env: {
+              // if this is set then npm will be executed as if `--omit=dev` was passed during the install
+              // but `has-prepack-npm` requires dev dependencies to be present so this is a good way to
+              // verify that yarn isn't throw off by this when handling the clone, install, and pack process
+              // for git dependencies (see: https://docs.npmjs.com/cli/v8/using-npm/config#omit)
+              NODE_ENV: `production`,
+
+              // same for NPM_CONFIG_PRODUCTION which acts just like the `--production` flat during install step
+              // (see: https://docs.npmjs.com/cli/v8/using-npm/config#environment-variables, https://docs.npmjs.com/cli/v8/using-npm/config#production)
+              NPM_CONFIG_PRODUCTION: `true`,
+              npm_config_production: `true`,
+
+              // also force npm to use the package server as the registry so that the `has-bin-entry` dependency can be resolved
+              NPM_CONFIG_REGISTRY: await startPackageServer(),
+            },
+          })).resolves.toBeTruthy();
+          await expect(source(`require('npm-has-prepack')`)).resolves.toEqual(42);
+        },
+      ),
+      45000,
+    );
+
     test(
       `it should support installing specific workspaces from npm repositories`,
       makeTemporaryEnv(

packages/acceptance-tests/pkg-tests-fixtures/repositories/npm-has-prepack.git/objects/f1/16b60b454737055ce23504bb1cee6977d4b518

@@ -240,6 +240,9 @@ export async function prepareExternalProject(cwd: PortablePath, outputPath: Port
 
             stdout.write(`\n`);
 
+            // Remove environment variables that limit the install to just production dependencies
+            delete env.NODE_ENV;
+
             // Run an install; we can't avoid it unless we inspect the
             // package.json, which I don't want to do to keep the codebase
             // clean (even if it has a slight perf cost when cloning v1 repos)
@@ -323,6 +326,11 @@ export async function prepareExternalProject(cwd: PortablePath, outputPath: Port
             // one instead
             delete env.npm_config_user_agent;
 
+            // Remove environment variables that limit the install to just production dependencies
+            delete env.npm_config_production;
+            delete env.NPM_CONFIG_PRODUCTION;
+            delete env.NODE_ENV;
+
             // We can't use `npm ci` because some projects don't have npm
             // lockfiles that are up-to-date. Hopefully npm won't decide
             // to change the versions randomly.