Skip to content

Add a customized GitRepositoryError and funnel all Git operations thr ough a method that throws one of those if the operation fails #2987

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

Merged

Conversation

abertelrud
Copy link
Contributor

This makes sure that Git failures clients can distinguish from other Process errors (so failure handling can be customized), and that they all have a location. At present the location for an error during a cloning operation is the remote URL, while the operation for an operation on a local clone is the path of the local clone.

We might want to extend this in the future so that local clones also keep track of the URL, but this will require the GitRepository to keep a copy of the associated RepositorySpecifier of its origin remote. This would in turn require a change to the Repository protocol so that all methods take a RepositorySpecificier, and it would require changes to all the clients. So that should be a separate PR, if we do it.

This is a re-opening of a PR (#2958) that was reverted as part of reverting a ToolsSupportCore regression, with the only change being to remove the use of the ToolSupportCore parameter that got reverted. It is not needed for the correct functioning of this PR.

@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud self-assigned this Oct 20, 2020
}
}

public func checkout(newBranch: String) throws {
precondition(isWorkingRepo, "This operation should run in a working repo.")
precondition(isWorkingRepo, "This operation is only value in a working repository")
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean: This operation is only valid in a working repository ?
valid

Copy link
Contributor Author

@abertelrud abertelrud Oct 20, 2020

Choose a reason for hiding this comment

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

Yes indeed I did. I was trying to improve the wording on each of these messages. Thanks for catching it!

…ough a method that throws one of those if the operation fails. This makes sure that Git failures clients can distinguish from other Process errors (so failure handling can be customized), and that they all have a location.
@abertelrud abertelrud force-pushed the eng/git-repository-invocation-cleanup branch from 852a2e7 to da6000e Compare October 20, 2020 17:32
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Member

@federicobucchi federicobucchi left a comment

Choose a reason for hiding this comment

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

Thank you!

@abertelrud
Copy link
Contributor Author

Thanks for reviewing!

@abertelrud abertelrud merged commit a5824b1 into swiftlang:main Oct 20, 2020
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.

2 participants