npm should init/update submodules when installing from git url

[vincentcr]

it would be nice if installing from a git url expanded the git submodules before running the install scripts.

[kuebk]

Yea, that would be cool. +1.

[jbeard4]

Still cool. +1

[enriclluelles]

@isaacs would you merge a PR that implemented this?

[isaacs]

Sure.

[kumavis]

👍 +1

--recursive or --recurse-submodules

[alexeyraspopov]

Maybe we can just add --recursive arg in this line. Should it work?

[kumavis]

@alexeyraspopov might as well submit a PR, this issue has been hanging around for awhile

[guybrush]

--recursive will not work since npm is currently doing git archive <git-ref> --format=tar --prefix=package/

see #3343

but maybe i can do a PR after some other npm-cache related PR/issues are resolved (#4667, #4824, #4965). maybe we can checkout the revision + update submodules + tar the whole directory OR just copy the files without taring (not sure about that at all).

just now i tried to install "bud":"git+https://github.com/indutny/bud.git#master" and it didnt work :/

[gagern]

Since 00ecb61 for #6400, we no longer use git archive. So a simple call to git submodule update --init --recursive would be feasible these days, and I filed #11094 for this. But perhaps that's not the end of the story. I have two issues I'd like to discuss.

One thing is caching. The current handling of git remotes re-uses local mirrors of remote repos to store relevant data. Should we leverage this mechanism for submodules as well? If so, we can't let a single git invocation do all the work, but need to do a lot more work in npm. We'd have to iterate over submodules and to recurse into them ourselves. We'd have to look at how git submodule is implemented and duplicate much of that: list submodules, read their configuration to identify repository url, cache the corresponding remotes as we do for top level modules, then initialize them and update them using --reference to pull data from the cache. Git itself apparently intrends to switch submodule implementation to c, and the helper introduced in the first step of that change would make life easier if it were to stay. But since we can't rely on git 2.7 and even then probably can't rely on this internal helper remaining available in future versions, I don't see a way around doing all that stuff ourselves for the near future. So is the benefit really worth all this effort?

The other thing is configurability. Usually a submodule is considered a dependency which we need, so updating and initializing is the right thing to do. But what if a project uses submodules for the web site with all its huge videos? Or the repository of huge test cases? Or some company-internal style checking tool which is not available to other users? In short, there may be reasons for packages to better not initialize their submodules automatically. And the choice is likely per package, not per user. Should we introduce a package.json option to control this? Should it be opt-out or opt-in? Seeing how most people may expect their submodules to “just work” I'd say opt-out, perhaps using an array of glob patterns or something like that.

[othiym23]

1. Given the amount of concern within the npm community around the CLI's performance, unless it can be shown that there's a way to do all this without caching repos that's as fast or faster as with caching, it's a very tough sell. As much as possible, when adding features to the CLI, I'd like performance to be a ratchet.
2. As a general principle, every time we add new configuration to deal with edge cases, it makes the CLI harder to use and contributes to its (already forbiddingly steep) learning curve. If the CLI is going to support submodules in Git dependencies, it should be something similar to what's already in your PR, and follow the happy path. If there are concerns about this breaking existing Git dependencies, it can be included as part of a semver-major release. It would be worth exploring whether some of these edge cases could be handled purely on the Git side.
3. The CLI team has never nailed down a minimum supported version of Git, but given that Ubuntu LTS ships git@1.9, depending on anything newer is going to result in more support issues from confused users (I think there's probably a plurality of npm users who have no idea that it uses the installed git binaries instead of some kind of bundled functionality).

[gagern]

@othiym23 Did I get you right?

1. So we should add caching, even if it's a lot of code.
2. No config option, to keep things simple.
3. Support a broad range of git versions.

If there are concerns about this breaking existing Git dependencies, it can be included as part of a semver-major release.

Theoretically it would be conceivable that some people depend on submodules not being initialized. E.g. they might have some machinery to do so automatically, and that machinery might break if the repo is already initialized by npm. Unlikely, but possible.

Seeing the many references to this bug here tells me that people need this feature. So if the next major release is imminent, or you'd be willing to bump the major version for this, that's fine. Otherwise I'd say that practically the lack of submodule support is a bug, not something to rely on, so submodule initialization fixes that bug and doesn't need a major version bump.

It would be worth exploring whether some of these edge cases could be handled purely on the Git side.

Well, if people have npm module A, and submodules B and C which are unfit for npm installs, then we could tell them to instead create a master module D which uses A, B, C as submodules, so that A itself refers to B and C via relative paths outside its module, not via submodules. Is this the kind of git-based solution you had in mind?

[othiym23]

So we should add caching, even if it's a lot of code.

I would say the requirement is that this feature shouldnt cause a significant performance hit. It's not important how that requirement gets satisfied. Git dependencies are already significantly slower to install than plain dependencies, especially once we land your patch to build git deps before caching them (which is a much-needed change, thank you).

1. No config option, to keep things simple.
2. Support a broad range of git versions.

Yes.

Otherwise I'd say that practically the lack of submodule support is a bug, not something to rely on, so submodule initialization fixes that bug and doesn't need a major version bump.

If breaking existing use cases is a concern, there's no alternative but to bump major. Semver is pretty unambiguous here. Right now I don't have an opinion on whether it's a significant concern, because we lack evidence one way or the other.

There are a few other small but breaking changes queued up for npm@4, but I wouldn't call it imminent, given the other priorities of the team.