-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
WIP: API for self-hosted bitbucket instance #9134
base: main
Are you sure you want to change the base?
Conversation
Please rebase this on master branch, we will not add features to 1.x anymore. |
9f19c44
to
ce66a6f
Compare
|
||
public static function supports(IOInterface $io, Config $config, $url, $deep = false) | ||
{ | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really not great.. as it means it'll be used as fallback for any URL not matching any other driver. I am not sure how to best fix it tbh, but it's be nice to have a way to identify this correctly, or maybe we force people to use the bitbucket-server type instead of magic vcs type if they want to use this.. That'd be ok too IMO, then supports
is irrelevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tell me what you think about the following solution:
public static function supports(IOInterface $io, Config $config, $url, $deep = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A solution might be to introduce a bitbuket-server-domains
config setting, as already done to support Gitlab or Github Enterprise.
f074239
to
6086036
Compare
Basically, CPU usage has dropped, but the timing has increased (but it's probably due to BB Server configuration and VPN) |
|
4acbbe8
to
458b001
Compare
try { | ||
$http = Factory::createHttpDownloader($io, $config); | ||
$response = $http->get(sprintf('https://%s/rest/api/1.0/application-properties', $originUrl))->decodeJson(); | ||
} catch (\Exception $e) { | ||
$io->writeError($e->getMessage()); | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not really acceptable to do an http request for every repository to chek if it's bitbucket server or not. This should work with a bitbucket-domains
config value, similar to https://github.com/composer/composer/blob/master/src/Composer/Repository/Vcs/GitLabDriver.php#L527
protected $fallbackDriver; | ||
|
||
/** @var string|null if set either git or hg */ | ||
protected $vcsType; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bitbucket Server currently only supports support for git repositories. This can be removed
protected $branches; | ||
protected $infoCache = array(); | ||
protected $branchesUrl = ''; | ||
protected $tagsUrl = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$branchesUrl
and $tagsUrl
are unused and are not necessary. The Bitbucket Server API doesn't return those URLs so hard coding them as done in this PR should be ok.
protected $branchesUrl = ''; | ||
protected $tagsUrl = ''; | ||
protected $homeUrl = ''; | ||
protected $website = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$homeUrl
and $website
are never set. The Bitbucket Server API returns a self link which we can use and set as homepage in the Composer data.
} | ||
$this->parseCloneUrls($repoData['links']['clone']); | ||
|
||
$this->hasIssues = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is never set to true
. I don't think Bitbucket Server has issues support. The API docs also do not list anything related. This can be removed.
return $this->branches; | ||
} | ||
|
||
protected function attemptCloneFallback() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is currently unused. You can have a look at other drivers to see how this is done. Basically you will want to extend the getContents
method and handle cases where the credential doesn't have access to the API and then fallback to using git
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is then also the place where you can use the util class you created
protected function getRepoData() | ||
{ | ||
$resource = sprintf( | ||
'https://%s/rest/api/1.0/projects/%s/repos/%s', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All API URLs currently force HTTPS Bitbucket Server doesn't enforce HTTPS. I wonder whether we should handle this similar to the GitLabDriver where we detect the scheme from the repository URL?
I am interested in completing this PR. Is there anything I can help with? |
@beniamin I'd say send your own PR based on this one, taking @glaubinix's feedback into account above, then we can hopefully move this forward? |
|
By using Bitbucket Server API
composer install
"should" speed up a bitSome testing results (could be affected by bitbucket-server configuration)
without changes + without cache + install
without changes + without cache + update (without composer.lock)
with changes + without cache + install
with changes + without cache + update (without composer.lock)
Related #9100 #8388