-
Notifications
You must be signed in to change notification settings - Fork 280
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
Git notes #576
Git notes #576
Changes from 5 commits
abc1d74
41279e4
0871b0e
47dc0fc
fe13986
6df9faf
947058d
e99a62e
b64f024
0661bab
c27e838
73037a4
75e5356
16ef587
2352543
6e96e11
6228da7
2fea0a5
5bf9d5f
f4a1103
55898a7
16f980f
927cc49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
// | ||
// GTNote.h | ||
// ObjectiveGitFramework | ||
// | ||
// Created by Slava Karpenko on 5/16/2016. | ||
// | ||
// The MIT License | ||
// | ||
// Copyright (c) 2016 Wildbit LLC | ||
// | ||
// Permission is hereby granted, free of charge, to any person obtaining a copy | ||
// of this software and associated documentation files (the "Software"), to deal | ||
// in the Software without restriction, including without limitation the rights | ||
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
// copies of the Software, and to permit persons to whom the Software is | ||
// furnished to do so, subject to the following conditions: | ||
// | ||
// The above copyright notice and this permission notice shall be included in | ||
// all copies or substantial portions of the Software. | ||
// | ||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
// THE SOFTWARE. | ||
// | ||
|
||
#import <Foundation/Foundation.h> | ||
#import "git2/oid.h" | ||
|
||
@class GTSignature; | ||
@class GTRepository; | ||
@class GTOID; | ||
@class GTObject; | ||
|
||
NS_ASSUME_NONNULL_BEGIN | ||
|
||
@interface GTNote : NSObject {} | ||
|
||
/// The author of the note. | ||
@property (nonatomic, readonly, strong, nullable) GTSignature *author; | ||
|
||
/// The committer of the note. | ||
@property (nonatomic, readonly, strong, nullable) GTSignature *committer; | ||
|
||
/// Content of the note. | ||
@property (nonatomic, readonly, strong) NSString *note; | ||
|
||
@property (nonatomic, readonly, strong) GTObject *target; | ||
|
||
/// The underlying `git_note` object. | ||
- (git_note * _Nullable)git_note __attribute__((objc_returns_inner_pointer)); | ||
|
||
/// These initializers may fail if there's no note attached to the provided oid. | ||
- (nullable instancetype)initWithTargetOID:(GTOID*)oid repository:(GTRepository*)repository ref:(nullable NSString*)ref; | ||
- (nullable instancetype)initWithTargetGitOID:(git_oid*)oid repository:(git_repository *)repository ref:(const char* _Nullable)ref; | ||
|
||
@end | ||
|
||
NS_ASSUME_NONNULL_END | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
// | ||
// GTNote.m | ||
// ObjectiveGitFramework | ||
// | ||
// Created by Slava Karpenko on 16.05.16. | ||
// Copyright © 2016 Wildbit LLC. All rights reserved. | ||
// | ||
|
||
#import "GTNote.h" | ||
#import "NSError+Git.h" | ||
#import "GTSignature.h" | ||
#import "GTReference.h" | ||
#import "GTRepository.h" | ||
#import "NSString+Git.h" | ||
#import "GTOID.h" | ||
|
||
#import "git2/errors.h" | ||
#import "git2/notes.h" | ||
|
||
@interface GTNote () | ||
{ | ||
git_note *_note; | ||
} | ||
|
||
@end | ||
@implementation GTNote | ||
|
||
- (NSString *)description { | ||
return [NSString stringWithFormat:@"<%@: %p>", NSStringFromClass([self class]), self]; | ||
} | ||
|
||
#pragma mark API | ||
|
||
- (void)dealloc { | ||
if (_note != NULL) { | ||
git_note_free(_note); | ||
} | ||
} | ||
|
||
- (git_note *)git_note { | ||
return _note; | ||
} | ||
|
||
- (NSString*)note { | ||
return @(git_note_message(self.git_note)); | ||
} | ||
|
||
- (GTSignature *)author { | ||
return [[GTSignature alloc] initWithGitSignature:git_note_author(self.git_note)]; | ||
} | ||
|
||
- (GTSignature *)committer { | ||
return [[GTSignature alloc] initWithGitSignature:git_note_committer(self.git_note)]; | ||
} | ||
|
||
- (GTOID*)targetOID { | ||
return [GTOID oidWithGitOid:git_note_id(self.git_note)]; | ||
} | ||
|
||
- (instancetype)initWithTargetOID:(GTOID*)oid repository:(GTRepository*)repository ref:(NSString*)ref { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: |
||
return [self initWithTargetGitOID:(git_oid *)oid.git_oid repository:repository.git_repository ref:ref.UTF8String]; | ||
} | ||
|
||
- (instancetype)initWithTargetGitOID:(git_oid*)oid repository:(git_repository *)repository ref:(const char*)ref { | ||
if (self = [super init]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style : Explicit nil comparison + return early. |
||
int gitErr = git_note_read(&_note, repository, ref, oid); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: whitespace. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm blind. What's wrong with the whitespace above? Triple checked and don't see anything wrong. :\ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I meant the newline is unwarranted. |
||
if (gitErr != GIT_OK) | ||
return nil; // Cannot read the note, means it either doesn't exists for this object, this object is not found, or whatever else. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will silently swallow errors (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added error parameter to init calls to propagate errors, if needed. |
||
} | ||
|
||
return self; | ||
} | ||
|
||
@end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,8 @@ | |
|
||
#import "git2/errors.h" | ||
#import "git2/remote.h" | ||
#import "git2/notes.h" | ||
#import "git2/buffer.h" | ||
|
||
NSString *const GTRepositoryRemoteOptionsCredentialProvider = @"GTRepositoryRemoteOptionsCredentialProvider"; | ||
NSString *const GTRepositoryRemoteOptionsFetchPrune = @"GTRepositoryRemoteOptionsFetchPrune"; | ||
|
@@ -198,6 +200,28 @@ - (BOOL)pushBranches:(NSArray *)branches toRemote:(GTRemote *)remote withOptions | |
return [self pushRefspecs:refspecs toRemote:remote withOptions:options error:error progress:progressBlock]; | ||
} | ||
|
||
- (BOOL)pushNotes:(NSString *)noteRef toRemote:(GTRemote *)remote withOptions:(NSDictionary *)options error:(NSError **)error progress:(GTRemotePushTransferProgressBlock)progressBlock { | ||
NSParameterAssert(remote != nil); | ||
|
||
if (noteRef == nil) { | ||
git_buf default_ref_name = { 0 }; | ||
int gitErr = git_note_default_ref(&default_ref_name, self.git_repository); | ||
if (gitErr != GIT_OK) { | ||
git_buf_free(&default_ref_name); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're freeing on error, but not freeing on success here. |
||
|
||
if (error != NULL) { | ||
*error = [NSError git_errorFor:gitErr description:@"Unable to get default git notes reference name"]; | ||
} | ||
|
||
return NO; | ||
} | ||
|
||
noteRef = [NSString stringWithUTF8String:default_ref_name.ptr]; | ||
} | ||
|
||
return [self pushRefspecs:@[[NSString stringWithFormat:@"%@:%@", noteRef, noteRef]] toRemote:remote withOptions:options error:error progress:progressBlock]; | ||
} | ||
|
||
#pragma mark - Deletion (Public) | ||
- (BOOL)deleteBranch:(GTBranch *)branch fromRemote:(GTRemote *)remote withOptions:(NSDictionary *)options error:(NSError **)error { | ||
NSParameterAssert(branch != nil); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,7 @@ | |
@class GTTag; | ||
@class GTTree; | ||
@class GTRemote; | ||
@class GTNote; | ||
|
||
NS_ASSUME_NONNULL_BEGIN | ||
|
||
|
@@ -604,6 +605,48 @@ typedef NS_ENUM(NSInteger, GTRepositoryStateType) { | |
/// Returns YES if operation was successful, NO otherwise | ||
- (BOOL)cleanupStateWithError:(NSError **)error; | ||
|
||
/// Creates a new note in this repo (using a default notes reference, e.g. "refs/notes/commits") | ||
/// | ||
/// note - Note text. | ||
/// theTarget - Object (usually a commit) to which this note attaches to. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/attaches to/refers |
||
/// This object must belong to this repository. | ||
/// ref - Name for the notes reference in the repo, or nil for default ("refs/notes/commits") | ||
/// author - Signature of the author for this note, and | ||
/// of the note creation time | ||
/// committer - Signature of the committer for this note. | ||
/// overwrite - If set to YES, the note will be overwritten if it already exists. | ||
/// error - Will be filled with a NSError object in case of error. | ||
/// May be NULL. | ||
/// | ||
/// Returns the newly created note or nil on error. | ||
- (nullable GTNote *)createNote:(NSString *)note target:(GTObject *)theTarget ref:(nullable NSString *)ref author:(GTSignature *)author committer:(GTSignature *)committer overwriteIfExists:(BOOL)overwrite error:(NSError **)error; | ||
|
||
/// Removes a note attached to object in this repo (using a default notes reference, e.g. "refs/notes/commits") | ||
/// | ||
/// parentObject - Object (usually a commit) to which the note to be removed is attached to. | ||
/// This object must belong to this repository. | ||
/// ref - Name for the notes reference in the repo, or nil for default ("refs/notes/commits") | ||
/// author - Signature of the author for this note removal, and | ||
/// of the note removal time | ||
/// committer - Signature of the committer for this note removal. | ||
/// error - Will be filled with a NSError object in case of error. | ||
/// May be NULL. | ||
/// | ||
/// Returns the YES on success and NO on error. | ||
- (BOOL)removeNoteFromObject:(GTObject *)parentObject ref:(nullable NSString *)ref author:(GTSignature *)author committer:(GTSignature *)committer error:(NSError **)error; | ||
|
||
/// Enumerates through all stored notes in this repo (using a default notes reference, e.g. "refs/notes/commits") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the () part can be dropped, as it's not crucial in describing the method, and duplicates the explanation for the parameter below. |
||
/// | ||
/// error - Will be filled with a NSError object in case of error. | ||
/// May be null. | ||
/// ref - Name for the notes reference in the repo, or nil for default ("refs/notes/commits") | ||
/// block - A block to be called on each encountered note object. The block accepts | ||
/// a reference to `note`, an `object` that is annotated with the note. | ||
/// If the block sets `stop` to YES, the iterator is finished. | ||
/// | ||
/// Returns YES on overall success or NO on error of any kind. | ||
- (BOOL)enumerateNotesWithError:(NSError **)error ref:(nullable NSString *)ref usingBlock:(void (^)(GTNote *note, GTObject *object, BOOL *stop))block; | ||
|
||
@end | ||
|
||
NS_ASSUME_NONNULL_END |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,7 @@ | |
#import "NSError+Git.h" | ||
#import "NSString+Git.h" | ||
#import "GTRepository+References.h" | ||
#import "GTNote.h" | ||
|
||
#import "git2.h" | ||
|
||
|
@@ -919,4 +920,76 @@ - (BOOL)cleanupStateWithError:(NSError * _Nullable __autoreleasing *)error { | |
return YES; | ||
} | ||
|
||
#pragma mark Notes | ||
|
||
- (GTNote *)createNote:(NSString *)note target:(GTObject *)theTarget ref:(NSString *)ref author:(GTSignature *)author committer:(GTSignature *)committer overwriteIfExists:(BOOL)overwrite error:(NSError **)error { | ||
git_oid oid; | ||
|
||
int gitError = git_note_create(&oid, self.git_repository, ref.UTF8String, author.git_signature, committer.git_signature, theTarget.OID.git_oid, [note UTF8String], overwrite ? 1 : 0); | ||
if (gitError != GIT_OK) { | ||
if (error != NULL) *error = [NSError git_errorFor:gitError description:@"Failed to create a note in repository"]; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: whitespace. |
||
return nil; | ||
} | ||
|
||
return [[GTNote alloc] initWithTargetOID:theTarget.OID repository:self ref:ref]; | ||
} | ||
|
||
- (BOOL)removeNoteFromObject:(GTObject *)parentObject ref:(NSString *)ref author:(GTSignature *)author committer:(GTSignature *)committer error:(NSError **)error { | ||
int gitError = git_note_remove(self.git_repository, ref.UTF8String, author.git_signature, committer.git_signature, parentObject.OID.git_oid); | ||
if (gitError != GIT_OK) { | ||
if(error != NULL) *error = [NSError git_errorFor:gitError description:@"Failed to delete note from %@", parentObject]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: missing space after |
||
return NO; | ||
} | ||
|
||
return YES; | ||
} | ||
|
||
- (BOOL)enumerateNotesWithError:(NSError **)error ref:(NSString *)ref usingBlock:(void (^)(GTNote *note, GTObject *object, BOOL *stop))block | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: we don't do the opening brace thing. |
||
git_note_iterator* iter = NULL; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: |
||
|
||
int gitError = git_note_iterator_new(&iter, self.git_repository, ref.UTF8String); | ||
|
||
if (gitError != GIT_OK) | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: no newline before opening braces. |
||
if(error != NULL) *error = [NSError git_errorFor:gitError description:@"Failed to enumerate notes"]; | ||
return NO; | ||
} | ||
|
||
git_oid note_id; | ||
git_oid object_id; | ||
BOOL success = YES; | ||
|
||
while (git_note_next(¬e_id, &object_id, iter) == GIT_OK) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No error report on iteration failure. |
||
NSError* lookupErr = nil; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: out of place |
||
|
||
GTNote* note = [[GTNote alloc] initWithTargetGitOID:&object_id repository:self.git_repository ref:ref.UTF8String]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: Extraneous whitespace after = |
||
if (note == nil) { | ||
if (error != NULL) | ||
*error = [NSError git_errorFor:GIT_ENOTFOUND description:@"Cannot create note"]; | ||
success = NO; | ||
break; | ||
} | ||
|
||
GTObject* obj = [self lookUpObjectByGitOid:&object_id error:&lookupErr]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that I've hit the shallow repository problem, this kind of "helpful" lookup thing makes me nervous... See #590. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the records, I won't hold the PR up for that though 😉. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll see how this can be reworked to be in sync with #590. But I'll submit a different pull request for that one. |
||
if (obj == nil && lookupErr != nil) { | ||
if (error != NULL) | ||
*error = lookupErr; | ||
success = NO; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is more debatable, but (even though I think there are other places we use that idiom), this prevents us from progressing through failures. Arguably, the "Cocoa way" would be to pass the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree on that one. I haven't noticed |
||
break; | ||
} | ||
|
||
BOOL stop = NO; | ||
block(note, obj, &stop); | ||
if (stop) { | ||
break; | ||
} | ||
} | ||
|
||
git_note_iterator_free(iter); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can prettify with onExit and a straight There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is still valid 😉. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still still valid ? |
||
|
||
return success; | ||
} | ||
|
||
@end |
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.
_Nullable
?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.
Just to be clear, my nullability-fu is really not up-to-speed 😉. I'm just perplexed since it's in the designated initializer...
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 a remains of an experimental work-in-progress back in the iterations when it could be
NULL
. This is indeed no longer the case. Fixed.