Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

build safe convenience wrapper for git_remote_ls#1218

Merged
implausible merged 9 commits intonodegit:masternodegit/nodegit:masterfrom
implausible:feature/git_remote_lsimplausible/nodegit:feature/git_remote_lsCopy head branch name to clipboard
Feb 23, 2017
Merged

build safe convenience wrapper for git_remote_ls#1218
implausible merged 9 commits intonodegit:masternodegit/nodegit:masterfrom
implausible:feature/git_remote_lsimplausible/nodegit:feature/git_remote_lsCopy head branch name to clipboard

Conversation

@implausible
Copy link
Member

Documentation for git_remote_ls shows that for it to work, a connection must be established to a remote first. When a subsequent connection is made, retrieved remote heads are destroyed and replaced with a new set.

This manual template locks the remote during execution of 3 actions (git_remote_connect, git_remote_ls, git_remote_disconnect) and copies all the data retrieved from git_remote_ls into memory for nodegit consumption.

The other contributing factor for using a manual template here is git_remote_ls has the only triple pointer use case in libgit2, so writing additional handlers for one case is overkill, especially when considering the volatility of the memory being retrieved. Complications that will exist if there is ever a good reason to build triple pointer handling into our generation system will be handling the bounds of the retrieved double pointer, as there will probably be a corresponding size argument.

baton->proxy_opts,
baton->custom_headers
);
baton->error_code = git_remote_connect(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of connecting (and disconnecting) as part of ReferenceList, it could be just a wrapper over git_remote_ls, and that way the user can use it with an already connected remote.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done and doner @srajko.

}
else if (baton->error_code < 0)
{
Local<v8::Object> err = Nan::Error("Method next has thrown an error.")->ToObject();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Method next has thrown an error." seems like the wrong error here.

dest->local = src->local;
git_oid_cpy(&dest->oid, &src->oid);
git_oid_cpy(&dest->loid, &src->loid);
dest->name = strdup(src->name);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a null-check here like you have below wouldn't hurt

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's always supposed to be defined, but you're right, we should err on the side of safety 🤦‍♂️.

lib/remote.js Outdated
* Lists advertised references from a remote
*
* @async
* @return {Promise<Array<RemoteHead>>} a list of the remote heads currently
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"currently available" might be a little off - isn't it more like what the remote had available when the connection was established?

@implausible implausible merged commit 5c3b406 into nodegit:master Feb 23, 2017
@implausible implausible deleted the feature/git_remote_ls branch February 23, 2017 03:06
@danwoodbury
Copy link

is there a way to use this but for a specific branch?

@mklueh
Copy link

mklueh commented Nov 25, 2017

Can sb please tell me how to use this? I can´t find it in the documentation

Just want to get a list of all remote branches from remote without fetching anything like "git ls-remote git@...."

@webbrandon
Copy link

@mklueh Looking at the unit test helps :) Looks like a function on the response:

return this.repository.getRemote("origin")
      .then(function(remote) {
        return remote.referenceList();
})

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.