Run install and pack when fetching git dependencies with a prepare script #3553

Volune · 2017-06-01T16:50:44Z

Implement a new behaviour in GitFetcher to fix #2875 and match the new behaviour of npm 5 regarding git dependencies:

If the git repository's package.json does not contain a prepare script, no change of behaviour
If the git repository's package.json contains a prepare script:
- clone the repository in a new temporary folder
- run install command in that folder, excluding the prepublish script
- run pack command and use the content of the generated tarball as result of GitFetcher
- remove the temporary folder

(@zkat please let me know if you think it differs from npm)

At the moment, I open the PR mostly for review and comments. The PR still lacks documentation of the new behaviour.

Some implementation details:

There is a new boolean config.gitDependency to disable the prepublish script
The tarball generated if there is a prepare script uses the pack code. That means all the files are in a package/ folder. If generated from git archive, there are no package/ folder. To differentiate during the tarball extraction, when I generate a tarball from the pack command, I add a parameter in the header of the first file.
The tarballMirrorPath filename does not have extension. The tarballCachePath filename has extension tgz, but the content does not seem gzipped. I did not change these points.
The git archive may be run multiple times for one fetch. I replicated this behaviour, running the pack command multiple times. I did so to ensure a better error handling with minimal work. I think both cases can be optimised.

I added only one test. I'm thinking of improving it to ensure the prepublish script is not run.
I'm not sure what other tests are worth adding, while staying in the scope of this PR. (I don't want to fix all git-fetcher issues here).
By the way, if yarn supports a "git+file://" protocol, that would be useful for testing.

zkat · 2017-06-02T00:23:36Z

The algorithm specified seems to be right. Just make sure all the relevant lifecycle scripts are being run in the same order and such.

Volune · 2017-06-12T02:18:00Z

Tests updated, and I think they pass. (The master branch builds are failing, so I don't know how confident I should be about the tests)
Can someone point me to the documentation I need to update?

bestander · 2017-06-16T00:42:48Z

package.json

@@ -37,6 +37,7 @@
    "strip-bom": "^3.0.0",
    "tar-fs": "^1.15.1",
    "tar-stream": "^1.5.2",
+    "test-js-git-repo": "https://github.com/Volune/test-js-git-repo",


not on npm?

Is this dependency needed?

bestander · 2017-06-16T00:54:22Z

src/cli/commands/install.js

@@ -842,7 +846,9 @@ export async function wrapLifecycle(config: Config, flags: Object, factory: () =
  await config.executeLifecycleScript('postinstall');

  if (!config.production) {
-    await config.executeLifecycleScript('prepublish');
+    if (!config.gitDependency) {


I think it is better to name it more generic disablePrepublish

bestander · 2017-06-16T00:56:15Z

Thanks, @Volune, this looks legit and well thought through.

…ript.

Volune · 2017-06-20T02:37:04Z

I cleaned the code according to comments, and rebased, so tests are passing now. Let me know if anything is missing.

bestander · 2017-06-20T21:03:39Z

Great work, @Volune.
I think this feature is under documented now and many people might want to use it.
Do you think it will be possible for you to write a short blog post here https://github.com/yarnpkg/website/tree/master/_posts?

Volune · 2017-06-22T02:41:20Z

I'll look into it. It will take some time, I'm not used to writing.

sammarks · 2017-06-25T01:55:24Z

@bestander @Volune just checked out the new feature, and I'm wondering what your recommendation is for the following setup given the new functionality:

I have package-a that depends on package-b (through a git URL as package-b is not published to a registry).
package-a and package-b both use Babel (listed in devDependencies in both).
package-b's prepare script is: babel --optional runtime src -d dist

When installing the dependencies of package-a, I get an error message: babel: command not found

When I change to the temporary directory where the build is supposed to be happening before packing, it appears the node_modules/.bin directory is not created (but the dependencies are installed), which causes the reference to babel to be not found (since we can't expect babel to be installed globally).

Here's the output just for reference:

[1/4] Resolving packages...
[2/4] Fetching packages...
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
[1/2] ⠄ ngrok
[2/2] ⠄ fsevents
[-/2] ⠄ waiting...
[-/2] ⠄ waiting...
warning Error running install script for optional dependency: "/Users/sammarks/Library/Caches/Yarn/v1/.tmp/a339a22b5c259ad43ee07022e7413f6b.9abc356b9eaa1ddf2c84e3bec8f94758bb49eaf4.prepare/node_modules/fsevents: Command failed.\nExit code: 1\nCommand: sh\nArguments: -c node install\nDirectory: /Users/sammarks/Library/Caches/Yarn/v1/.tmp/a339a22b5c259ad43ee07022e7413f6b.9abc356b9eaa1ddf2c84e3bec8f94758bb49eaf4.prepare/node_modules/fsevents\nOutput:\nevents.js:160\n      throw er; // Unhandled 'error' event\n      ^\n\nError: spawn node-pre-gyp ENOENT\n    at exports._errnoException (util.js:1022:11)\n    at Process.ChildProcess._handle.onexit (internal/child_process.js:193:32)\n    $ babel --optional runtime src -d dist
sh: babel: command not found
error Command failed with exit code 127.
info Visit https://yarnpkg.com/en/docs/cli/upgrade for documentation about this command.

Could this be related to #2053?

I thought best to continue the discussion on this PR since it's recent and relevant, but I can create a new issue to house the discussion if you'd like.

Volune · 2017-06-26T04:17:18Z

@sammarks I can reproduce.
An option binLinks: true is missing when installing dependencies

I will prepare a PR with a fix and improved tests.

@bestander Do you or another contributor with good knowledge of the binLinks option know if it can have undesired side effects?

…rnpkg#3553

ramasilveyra · 2017-06-26T17:53:57Z

Hello! I just published a tool to create git tags for packages with build steps and monorepos packages https://github.com/ramasilveyra/gitpkg, my intention is to create very lightweight git tags that don't delay the install command and also be able to publish the gitpkg packages to the same repo.

Please if you want let me know what do you think about this approach.

Thanks!

sammarks · 2017-06-26T18:12:39Z

This is pretty cool @ramasilveyra! Looks like a nice alternative until we can nail down this issue.

bestander · 2017-06-26T18:14:37Z

@ramasilveyra that looks exciting.
What can we do to make it work better with Yarn and Lerna?

ramasilveyra · 2017-06-26T18:44:28Z

@sammarks Sadly they are not the same 🙁 the approach choose by npm and yarn is to run the build scripts when you install the git dep, this could increase a lot the installation time. And it doesn't work with lerna git tags.

gitpkg publish at this moment does:

Read and validate package.json.
Run prepublish scripts.
Prepare package: npm pack and untar tarball to temp dir.
Upload package: create git tag from temp dir and push to resolved gitpkg registry.
Run postpublish scripts.

The end result is a git tag with only the files needed, look at this https://github.com/ramasilveyra/public-registry/tree/gitpkg%401.0.0-beta.0-gitpkg

@bestander I believe that there is a lot of wins if the functionality of gitpkg gets integrated in yarn, npm and lerna/workspaces.

On the yarn and npm side:
- publishConfig.registry: "publishConfig":{"registry":"http://my-internal-registry.local"} maybe it would be cool to check if that entry is a git url and if it is the publish command will create the git tag and push it to that repo.
- semver ranges support: for example that the dep ramasilveyra/public-registry#gitpkg:^1.0.0-beta.0 (note the :^ chars) resolves to the git tag [email protected] on [email protected]:ramasilveyra/public-registry.git.
On the lerna and workspaces:
- That the bootstrap command creates the symlinks for the git tags deps too.

Is this something for a RFC? Is there a common place to discuss this ideas to get implemented on npm and yarn?

bestander · 2017-06-26T18:49:58Z

@ramasilveyra, yes an RFC would be the right way to go.
Please consider a multistep approach, there are a lot of things that are not the best in publish experience and we are going to work on revamping it a lot, so big changes may be hard to consider against multiple other changes.

…rnpkg#3553

(#3723)

masaeedu · 2017-10-06T19:33:01Z

@Volune Doing yarn add masaeedu/ava doesn't seem to result in the prepare script being executed with yarn 1.1.0. Is this feature perhaps part of a later milestone?

rwjblue · 2017-10-31T21:01:34Z

@masaeedu - I was just looking into a similar issue, in my case it turned out that the logic added in this PR only applies to git: references and not github: references. Once I changed my package.json to reference it via git: the prepare was properly being invoked.

Volune · 2017-11-10T04:55:15Z

@masaeedu @rwjblue
Yarn has a different behavior for "hosted git repositories"
I started a discussion a while ago regarding this issue on yarnpkg/rfcs#79, but it then happened that I didn't have time to take care of it. The idea is to use the same algorithm to install "hosted git repositories" and "git repositories"
I can't promise I'll have time to work on it soon.

When a prepare script is defined, npm and yarn will build uppy automatically if it's being depended on through a git URL. This is useful when downstream projects don't want to wait for the next release to be tagged. See npm/npm#3055 (comment) See also yarnpkg/yarn#3553

…rnpkg/yarn#3553 (#3723)

Volune force-pushed the master branch from 46c5075 to e6a1c98 Compare June 3, 2017 03:28

bestander reviewed Jun 16, 2017

View reviewed changes

Volune force-pushed the master branch from e6a1c98 to c71dfe9 Compare June 16, 2017 07:05

Run install and pack when fetching git dependencies with a prepare sc…

9bd94d8

…ript.

Volune force-pushed the master branch from c71dfe9 to 9bd94d8 Compare June 19, 2017 09:36

bestander merged commit 5839c36 into yarnpkg:master Jun 20, 2017

Volune added a commit to Volune/yarn that referenced this pull request Jun 26, 2017

Fix ".bin" not linked when preparing a git dependency, reported in ya…

d54ee89

…rnpkg#3553

Volune mentioned this pull request Jun 26, 2017

Fix ".bin" not linked when preparing a git dependency #3723

Merged

Volune added a commit to Volune/yarn that referenced this pull request Jun 27, 2017

Fix ".bin" not linked when preparing a git dependency, reported in ya…

ed280db

…rnpkg#3553

bestander pushed a commit that referenced this pull request Jun 28, 2017

Fix ".bin" not linked when preparing a git dependency, reported in #3553

08c36fa

(#3723)

bestander mentioned this pull request Jun 29, 2017

auto-build-rfc yarnpkg/rfcs#38

Closed

bb010g mentioned this pull request Aug 7, 2017

Support setting Request encoding for response apiaryio/dredd#836

Closed

3 tasks

jasonkarns mentioned this pull request Nov 19, 2017

WIP - remove dist from git testdouble/testdouble.js#299

Merged

rtsao mentioned this pull request Jan 16, 2018

Lifecycle scripts for hosted git and local dependencies not run on install #5235

Closed

jfirebaugh mentioned this pull request Jul 13, 2019

Run the prepare script of git dependencies even if NODE_ENV=production #7398

Merged

lovelypuppy0607 added a commit to lovelypuppy0607/yarn that referenced this pull request May 11, 2023

Fix ".bin" not linked when preparing a git dependency, reported in ya…

f04040f

…rnpkg/yarn#3553 (#3723)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run install and pack when fetching git dependencies with a prepare script #3553

Run install and pack when fetching git dependencies with a prepare script #3553

Volune commented Jun 1, 2017

zkat commented Jun 2, 2017

Volune commented Jun 12, 2017

bestander Jun 16, 2017

bestander Jun 16, 2017

bestander Jun 16, 2017

bestander commented Jun 16, 2017

Volune commented Jun 20, 2017

bestander commented Jun 20, 2017

Volune commented Jun 22, 2017

sammarks commented Jun 25, 2017

Volune commented Jun 26, 2017

ramasilveyra commented Jun 26, 2017 •

edited

Loading

sammarks commented Jun 26, 2017

bestander commented Jun 26, 2017

ramasilveyra commented Jun 26, 2017 •

edited

Loading

bestander commented Jun 26, 2017

masaeedu commented Oct 6, 2017

rwjblue commented Oct 31, 2017

Volune commented Nov 10, 2017

Run install and pack when fetching git dependencies with a prepare script #3553

Run install and pack when fetching git dependencies with a prepare script #3553

Conversation

Volune commented Jun 1, 2017

zkat commented Jun 2, 2017

Volune commented Jun 12, 2017

bestander Jun 16, 2017

Choose a reason for hiding this comment

bestander Jun 16, 2017

Choose a reason for hiding this comment

bestander Jun 16, 2017

Choose a reason for hiding this comment

bestander commented Jun 16, 2017

Volune commented Jun 20, 2017

bestander commented Jun 20, 2017

Volune commented Jun 22, 2017

sammarks commented Jun 25, 2017

Volune commented Jun 26, 2017

ramasilveyra commented Jun 26, 2017 • edited Loading

sammarks commented Jun 26, 2017

bestander commented Jun 26, 2017

ramasilveyra commented Jun 26, 2017 • edited Loading

bestander commented Jun 26, 2017

masaeedu commented Oct 6, 2017

rwjblue commented Oct 31, 2017

Volune commented Nov 10, 2017

ramasilveyra commented Jun 26, 2017 •

edited

Loading

ramasilveyra commented Jun 26, 2017 •

edited

Loading