Skip to content

Clone fetch auth #203

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions Classes/GTRepository.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,39 @@ typedef void (^GTRepositoryStatusBlock)(NSURL *fileURL, GTRepositoryFileStatus s
// returns nil (and fills the error parameter) if an error occurred, or a GTRepository object if successful.
+ (id)cloneFromURL:(NSURL *)originURL toWorkingDirectory:(NSURL *)workdirURL barely:(BOOL)barely withCheckout:(BOOL)withCheckout error:(NSError **)error transferProgressBlock:(void (^)(const git_transfer_progress *))transferProgressBlock checkoutProgressBlock:(void (^)(NSString *path, NSUInteger completedSteps, NSUInteger totalSteps))checkoutProgressBlock;

// Clone a repository with plain authentication
//
// originURL - The URL to clone from.
// workdirURL - A URL to the desired working directory on the local machine.
// barely - If YES, create a bare clone
// withCheckout - if NO, don't checkout the remote HEAD
// error - A pointer to fill in case of trouble.
// transferProgressBlock - This block is called with network transfer updates.
// checkoutProgressBlock - This block is called with checkout updates (if withCheckout is YES).
// username - Username.
// password - Password.
//
// returns nil (and fills the error parameter) if an error occurred, or a GTRepository object if successful.
Copy link
Member

Choose a reason for hiding this comment

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

Could you note which parameters are required vs. optional and add the associated NSParameterAsserts?

+ (id)cloneFromURL:(NSURL *)originURL toWorkingDirectory:(NSURL *)workdirURL barely:(BOOL)barely withCheckout:(BOOL)withCheckout error:(NSError **)error transferProgressBlock:(void (^)(const git_transfer_progress *))transferProgressBlock checkoutProgressBlock:(void (^)(NSString *path, NSUInteger completedSteps, NSUInteger totalSteps))checkoutProgressBlock asUser:(NSString*)username withPassword:(NSString*)password;

// Fetch from remote.
//
// remote - Remote name. "origin" if not provided.
// error(out) - If not NULL, set to any error that occurs.
//
// Returns true if fetched.
- (bool)fetchFromRemote:(NSString*)name transferProgressBlock:(void (^)(const git_transfer_progress *))transferProgressBlock error:(NSError **)error;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use BOOL.


// Fetch changes from remote. With authentication.
//
// remote - Remote name. "origin" if not provided.
// error(out) - If not NULL, set to any error that occurs.
// username - User name, can be null.
// password - Password, can be null.
//
// Returns true if fetched.
- (bool)fetchFromRemote:(NSString*)name transferProgressBlock:(void (^)(const git_transfer_progress *))transferProgressBlock error:(NSError **)error asUser:(NSString*)username withPassword:(NSString*)password;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use BOOL.


// Helper for getting the sha1 has of a raw object
//
// data - the data to compute a sha1 hash for
Expand Down
113 changes: 111 additions & 2 deletions Classes/GTRepository.m
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@

@interface GTRepository ()
@property (nonatomic, assign, readonly) git_repository *git_repository;

// Cache plain credentials with respect to url authentication callback.
+ (void)cacheCredentialsForCallBack:(NSString*)url username:(NSString*)username password:(NSString*)password;
Copy link
Member

Choose a reason for hiding this comment

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

Style: URL should always be capitalized since it's an abbreviation.

Any reason these can't be NSURLs?

+ (NSArray *)credentialsForUrl:(NSString*)url;
+ (void)forgetCredentialsForUrl:(NSString*)url;
Copy link
Member

Choose a reason for hiding this comment

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

From how these APIs are used currently, it seems like a block-based API would be more foolproof.

[GTRepository cacheCredentials... withBlock:^{
    // do stuff with them
}];

Copy link
Author

Choose a reason for hiding this comment

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

agree, will check this out


@end

@implementation GTRepository
Expand All @@ -79,6 +85,38 @@ - (void)dealloc {
}
}

#pragma mark Plain authentication

static NSMutableDictionary *credentials;

+ (void)cacheCredentialsForCallBack:(NSString*)url username:(NSString*)username password:(NSString*)password {
if (!credentials) {
Copy link
Member

Choose a reason for hiding this comment

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

We've been working hard to make objective-git thread safe and unfortunately this is not. Could you fix that up please?

Copy link
Author

Choose a reason for hiding this comment

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

actually, my implementation is also not thread safe :(
as for now - I am using it in ios app which is single threaded by design (I mean app allows only one git operation at a time)

after this particular commit will be done, I'll give a try...

credentials = [NSMutableDictionary dictionary];
}
NSArray *creds = [NSArray arrayWithObjects:username, password, nil];
[credentials setObject:creds forKey:url];
}

+ (NSArray *)credentialsForUrl:(NSString*)url {
return [credentials objectForKey:url];
}

+ (void)forgetCredentialsForUrl:(NSString*)url {
[credentials removeObjectForKey:url];
}

static int cred_acquire_cb(git_cred **out, const char *url, const char *username_from_url, unsigned int allowed_types, void *payload) {
NSString *_url = [NSString stringWithUTF8String:url];
Copy link
Member

Choose a reason for hiding this comment

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

Style: underscore prefix is reserved for instance variables. Use boxing @() instead of +stringWithUTF8String:.

NSArray *creds = [GTRepository credentialsForUrl:_url];
if (creds) {
const char* username = [[creds objectAtIndex:0] UTF8String];
Copy link
Member

Choose a reason for hiding this comment

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

Style: Index subscripting instead of -objectAtIndex:

const char* password = [[creds objectAtIndex:1] UTF8String];
return git_cred_userpass_plaintext_new(out, username, password);
} else {
return git_cred_userpass_plaintext_new(out, "", "");
}
}

#pragma mark API

+ (BOOL)isAGitDirectory:(NSURL *)directory {
Expand Down Expand Up @@ -154,7 +192,7 @@ static int transferProgressCallback(const git_transfer_progress *progress, void
return 0;
}

+ (id)cloneFromURL:(NSURL *)originURL toWorkingDirectory:(NSURL *)workdirURL barely:(BOOL)barely withCheckout:(BOOL)withCheckout error:(NSError **)error transferProgressBlock:(void (^)(const git_transfer_progress *))transferProgressBlock checkoutProgressBlock:(void (^)(NSString *path, NSUInteger completedSteps, NSUInteger totalSteps))checkoutProgressBlock {
+ (id)cloneFromURL:(NSURL *)originURL toWorkingDirectory:(NSURL *)workdirURL barely:(BOOL)barely withCheckout:(BOOL)withCheckout error:(NSError **)error transferProgressBlock:(void (^)(const git_transfer_progress *))transferProgressBlock checkoutProgressBlock:(void (^)(NSString *path, NSUInteger completedSteps, NSUInteger totalSteps))checkoutProgressBlock asUser:(NSString*)username withPassword:(NSString*)password {

git_clone_options cloneOptions = GIT_CLONE_OPTIONS_INIT;
if (barely) {
Expand All @@ -172,18 +210,89 @@ + (id)cloneFromURL:(NSURL *)originURL toWorkingDirectory:(NSURL *)workdirURL bar
cloneOptions.fetch_progress_cb = transferProgressCallback;
cloneOptions.fetch_progress_payload = (__bridge void *)transferProgressBlock;

if (([username length] > 0)&&([password length] > 0)) {
Copy link
Member

Choose a reason for hiding this comment

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

Style: dot syntax those methods and give the && some breathing space.

// Auth might fail in case of redirects.
[GTRepository cacheCredentialsForCallBack:[originURL absoluteString] username:username password:password];
Copy link
Member

Choose a reason for hiding this comment

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

Style: dot syntax on absoluteString.

cloneOptions.cred_acquire_cb = cred_acquire_cb;
}

const char *remoteURL = originURL.absoluteString.UTF8String;
const char *workingDirectoryPath = workdirURL.path.UTF8String;
git_repository *repository;
int gitError = git_clone(&repository, remoteURL, workingDirectoryPath, &cloneOptions);

[GTRepository forgetCredentialsForUrl:[originURL absoluteString]];

if (gitError < GIT_OK) {
if (error != NULL) *error = [NSError git_errorFor:gitError withAdditionalDescription:@"Failed to clone repository."];
return nil;
}

Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary whitespace introduced here.

return [[self alloc] initWithGitRepository:repository];

Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary whitespace introduced here.

}

+ (id)cloneFromURL:(NSURL *)originURL toWorkingDirectory:(NSURL *)workdirURL barely:(BOOL)barely withCheckout:(BOOL)withCheckout error:(NSError **)error transferProgressBlock:(void (^)(const git_transfer_progress *))transferProgressBlock checkoutProgressBlock:(void (^)(NSString *path, NSUInteger completedSteps, NSUInteger totalSteps))checkoutProgressBlock {

return [GTRepository cloneFromURL:originURL toWorkingDirectory:workdirURL barely:barely withCheckout:withCheckout error:error transferProgressBlock:transferProgressBlock checkoutProgressBlock:checkoutProgressBlock asUser:nil withPassword:nil];

}

- (bool)fetchFromRemote:(NSString*)name transferProgressBlock:(void (^)(const git_transfer_progress *))transferProgressBlock error:(NSError **)error {

return [self fetchFromRemote:name transferProgressBlock:transferProgressBlock error:error asUser:nil withPassword:nil];

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Use BOOL, and extraneous whitespace in method body.


- (bool)fetchFromRemote:(NSString*)name transferProgressBlock:(void (^)(const git_transfer_progress *))transferProgressBlock error:(NSError **)error asUser:(NSString*)username withPassword:(NSString*)password {
git_remote *remote = NULL;
const char *cname;
if (name) {
cname = [name UTF8String];
} else {
cname = [@"origin" UTF8String];
}

int gitError = git_remote_load(&remote, self.git_repository, cname);
if (gitError < GIT_OK) {
if (error != NULL) *error = [NSError git_errorFor:gitError withAdditionalDescription:@"Failed to fetch."];
return false;
}

NSString *_url = [NSString stringWithUTF8String:git_remote_url(remote)];
if (([username length] > 0)&&([password length] > 0)) {
// Auth might fail in case of redirects.
[GTRepository cacheCredentialsForCallBack:_url username:username password:password];
git_remote_set_cred_acquire_cb(remote, cred_acquire_cb, NULL);
}

gitError = git_remote_connect(remote, GIT_DIRECTION_FETCH);
if (gitError < GIT_OK) {
[GTRepository forgetCredentialsForUrl:_url];
if (error != NULL) *error = [NSError git_errorFor:gitError withAdditionalDescription:@"Failed to fetch."];
return false;
}

gitError = git_remote_download(remote, transferProgressCallback, (__bridge void *)transferProgressBlock);
if (gitError < GIT_OK) {
[GTRepository forgetCredentialsForUrl:_url];
if (error != NULL) *error = [NSError git_errorFor:gitError withAdditionalDescription:@"Failed to fetch."];
return false;
}

gitError = git_remote_update_tips(remote);
if (gitError < GIT_OK) {
[GTRepository forgetCredentialsForUrl:_url];
if (error != NULL) *error = [NSError git_errorFor:gitError withAdditionalDescription:@"Failed to fetch."];
return false;
}

[GTRepository forgetCredentialsForUrl:_url];
git_remote_disconnect(remote);
git_remote_free(remote);

return true;
}

+ (NSString *)hash:(NSString *)data objectType:(GTObjectType)type error:(NSError **)error {
git_oid oid;
Expand Down