Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Sep 7, 2014

Conversation

@Lukeas14
Copy link

@Lukeas14 Lukeas14 commented Jul 8, 2014

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
Copy link
Contributor

@sheerun sheerun commented Jul 8, 2014

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

Otherwise seems OK.

@Lukeas14
Copy link
Author

@Lukeas14 Lukeas14 commented Jul 8, 2014

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
Copy link
Contributor

@sheerun sheerun commented Jul 8, 2014

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
Copy link
Author

@Lukeas14 Lukeas14 commented Jul 8, 2014

Sounds good. I'll try that.

@Lukeas14
Copy link
Author

@Lukeas14 Lukeas14 commented Jul 8, 2014

@sheerun Updated.

@sheerun
Copy link
Contributor

@sheerun sheerun commented Jul 8, 2014

Looks better :) @satazor Any doubts?

@sindresorhus
Copy link
Contributor

@sindresorhus sindresorhus commented Jul 9, 2014

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

@sheerun
Copy link
Contributor

@sheerun sheerun commented Jul 9, 2014

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

@sheerun
Copy link
Contributor

@sheerun sheerun commented Jul 9, 2014

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

@benschwarz
Copy link
Member

@benschwarz benschwarz commented Jul 9, 2014

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
1 check passed
1 check passed
@vladikoff
continuous-integration/travis-ci The Travis CI build passed
Details
@Lukeas14
Copy link
Author

@Lukeas14 Lukeas14 commented Sep 18, 2014

Thanks guys!

@nwinkler
Copy link
Contributor

@nwinkler nwinkler commented Oct 7, 2014

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
Copy link
Contributor

@sheerun sheerun commented Oct 7, 2014

@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
Copy link
Contributor

@nwinkler nwinkler commented Oct 7, 2014

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

@nwinkler
Copy link
Contributor

@nwinkler nwinkler commented Oct 8, 2014

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
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants