Skip to content
This repository has been archived by the owner on Jul 3, 2019. It is now read-only.

git support #69

Merged
merged 1 commit into from
Mar 17, 2017
Merged

git support #69

merged 1 commit into from
Mar 17, 2017

Conversation

zkat
Copy link
Owner

@zkat zkat commented Mar 14, 2017

Fixes: #4

This PR implements a baseline solution to #4, and is not, by far, a complete one. Things will be significantly slower than other types until better caching and fallback support is added. The goal here, though, is purely to get it to work.

The good news here is that once this is implemented, pacote will have baseline support for all dependency types and we can start going all-in on integration with npm proper.

TODO

  • git manifest fetching (with shasum, directories, and npm-shrinkwrap)
  • use spawn instead of execFile to avoid maxBuffer exceeded error with large git.revs requests (like, uh, npm/npm <_<;;)
  • implement tarball handler (this is basically done but needs to be hooked up to manifest's work
  • manifest caching support
  • fast-path manifest fetching for public hosted git deps
  • fallback cascade for hosted git types only when the default representation is shortcut (npm does git: then git+ssh: then git+https until one succeeds or they all fail). Explicit URIs should be left intact and used as-is.
  • Filter out /^GIT/ variables except GIT_ASKPASS and override --template.
  • tar repos using a callback or something, so it's npm-compatible packing

@zkat zkat added this to the 1.0.0 milestone Mar 14, 2017
@zkat zkat mentioned this pull request Mar 14, 2017
5 tasks
@iarna
Copy link
Collaborator

iarna commented Mar 14, 2017

Possibly unintentional differences between this and add-remote-git.js:

  • This doesn't pass in --template to override any local user config around that.
  • This isn't filtering the GIT_ environment variables. The list we use is:
  'GIT_ASKPASS',
  'GIT_EXEC_PATH',
  'GIT_PROXY_COMMAND',
  'GIT_SSH',
  'GIT_SSH_COMMAND',
  'GIT_SSL_CAINFO',
  'GIT_SSL_NO_VERIFY'

@zkat
Copy link
Owner Author

zkat commented Mar 14, 2017

@iarna I'm intentionally not filtering git variables on the pacote end. There's a gitEnv config that I added with this PR. My stance is that this filtering is best left to client apps because they can pick what their UI is for their users from there. It's not stuff that really changes how package downloads happen.

I don't really understand what --template is doing there. I thought it was just something for the persistent mirrors we were keeping. I looked around for git template stuff and my casual searches didn't really clarify stuff so I moved on. What's it actually do for npm?... It seems to just point to .npm/_git-remotes/_templates? :\

@zkat
Copy link
Owner Author

zkat commented Mar 14, 2017

...ugh, but then the default state of pacote will be incompatible with standard npm behavior. We're already gonna have one such wart in the npm.pack logic having to be injected. Let's go with "maximally useful defaults" I guess. You've convinced me 😭

@iarna
Copy link
Collaborator

iarna commented Mar 14, 2017

So, templates are a thing where you can say "any time I create a new repository either via git init or git clone use these files in the module's .git directory". It let's you do things like have default git hooks that follow you around. (For example.)

We need to override this because the user's git hooks or other config can do, well, anything, and having them act that way on package sources is surprising.

@zkat
Copy link
Owner Author

zkat commented Mar 17, 2017

I wanna get this merged in sooner rather than later, so I'm moving that over to #70 and merging this one for now, so I can start doing more integration work before having the whole test suite working.

@zkat zkat merged commit 6d7eaf5 into latest Mar 17, 2017
@zkat zkat deleted the zkat/git branch March 17, 2017 08:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants