8000
Skip to content

Disable shallow clones for all repos except those from github.com - #1393

Merged
sheerun merged 1 commit into
bower:masterfrom
Lukeas14:disable-shallow-clone-except-github
Sep 7, 2014
Merged

Disable shallow clones for all repos except those from github.com#1393
sheerun merged 1 commit into
bower:masterfrom
Lukeas14:disable-shallow-clone-except-github

Conversation

@Lukeas14
@Lukeas14 Lukeas14 commented Jul 8, 2014
Copy link
Copy Markdown

This PR disables shallow clones for all repos except those with github.com in the hostname. It does this by checking the hostname using a regex in the getOrgRepoPair method before setting the "--depth 1" flag.

This is an update to the PR #1389 (Add --full-depth flag to 'bower install'). @sheerun mentioned we should disable all shallow clones instead of creating a new flag.

Note: The getOrgRepoPair method was moved from the GitHubResolver to the GitRemoteResolver so that it could be accessed by the GitRemoteResolver. Any module that called getOrgRepoPair was refactored to reflect the move.

@sheerun
sheerun commented Jul 8, 2014
Copy link
Copy Markdown
Contributor

I think it would be better to var GitHubResolver = require('./GitHubResolver'); in GitRemoteResolver, and leave getOrgRepoPair in GitHubResolver.

Otherwise seems OK.

@Lukeas14
Lukeas14 commented Jul 8, 2014
Copy link
Copy Markdown
Author

I tried that at first but since GitHubResolver inherits GitRemoteResolver you get an error (Object prototype may only be an Object or null) when trying to var GitHubResolver = require('./GitHubResolver');

Can you think of another way to access methods in GitHubResolver from GitRemoteResolver?

@sheerun
sheerun commented Jul 8, 2014
Copy link
Copy Markdown
Contributor

Or even better: create this._shallowClone = false variable in GitRemote resolver and override it to true in GitHubResolver which inherits from GitRemoteResolver.

Then only check _shallowClone variable and not call getOrgRepoPair at all in GitRemoteResolver

@Lukeas14
Lukeas14 commented Jul 8, 2014
Copy link
Copy Markdown
Author

Sounds good. I'll try that.

@Lukeas14
Lukeas14 commented Jul 8, 2014
Copy link
Copy Markdown
Author

@sheerun Updated.

@sheerun
sheerun commented Jul 8, 2014
Copy link
Copy Markdown
Contributor

Looks better :) @satazor Any doubts?

@sindresorhus
Copy link
Copy Markdown
Contributor

👎 Everyone shouldn't be punished just because GitHub Enterprise is broken.

@sheerun
sheerun 8000 commented Jul 9, 2014
Copy link
Copy Markdown
Contributor

@sindresorhus This is 1.3.x fix. We can introduce more granular control in 1.4.x.

@sheerun
sheerun commented Jul 9, 2014
Copy link
Copy Markdown
Contributor

Btw. by introducing shallow clones bower punished every server not supporting it.

@benschwarz
Copy link
Copy Markdown
Member

I tend to agree with @sindresorhus's sentiments, but shallow clone will just not work for any host using dumb http transport. It'd be nice to get a clear picture / plan together for how we're going to address this as a whole, before this PR lands.

sheerun added a commit that referenced this pull request Sep 7, 2014
…ithub

Disable shallow clones for all repos except those from github.com
@sheerun
sheerun merged commit 5eed363 into bower:master Sep 7, 2014
@Lukeas14
Copy link
Copy Markdown
Author

Thanks guys!

@nwinkler
nwinkler commented Oct 7, 2014
Copy link
Copy Markdown
Contributor

Is there going to be a real fix for this at one point? We're using a private repo (not GitHub) that supports shallow clones, and our build time has increased significantly since this change.

As part of our build/continuous integration process, we're cloning several projects into a local directory, and then run bower install and a couple of bower link commands to set up the projects. Between Bower version 1.3.9 and 1.3.10, the build time has increased by a factor of 4, due to this change. Previously, our local Bower dependencies were checked out using shallow cloning, and now they're being downloaded every time we run a clean build.

Is there an easy way to disable this change per repo? I would like to get our build fast and lean again.

@sheerun
sheerun commented Oct 7, 2014
Copy link
Copy Markdown
Contributor

@nwinkler Could you create new issue? Maybe we'll introduce new configuration in next minor like:

{
  "hosts": {
    "github.myservice.com": {
      "shallow_clone": true
    }
  }
}

or

{
  "shallow_clone": "github.myservice.com"
}

@nwinkler
nwinkler commented Oct 7, 2014
Copy link
Copy Markdown
Contributor

Thanks - that makes sense - I've created #1558 for this.

@nwinkler
nwinkler commented Oct 8, 2014
Copy link
Copy Markdown
Contributor

I've created PR #1559 to provide a config option per host for this in .bowerrc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

0