Skip to content

validate local repository has the correct remote #7079

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
merged 2 commits into from
Nov 27, 2023

Conversation

tomerd
Copy link
Contributor

@tomerd tomerd commented Nov 10, 2023

motivation: in some edge cases, the local repository may be partially valid (the containing directory is a legit git repo, but the repository directory is not), or otherwise point to a remote different than the one expected

changes:

  • validate that the local repository remote aligns with the expected one, not just that the directory is a valid git repo
  • refactor validation flow to be more streamlined

@tomerd tomerd changed the title validate local repositry has the correct remote validate local repository has the correct remote Nov 10, 2023
@tomerd
Copy link
Contributor Author

tomerd commented Nov 10, 2023

@MaxDesiatov @neonichu @shahmishal this would now emit warnings in the edge case we ran into where the local repositories in .build became invalid

@@ -210,6 +210,11 @@ public struct GitRepositoryProvider: RepositoryProvider, Cancellable {
return result == ".git" || result == "." || result == directory.pathString
}

public func isValidDirectory(_ directory: Basics.AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
let remoteURL = try self.git.run(["-C", directory.pathString, "remote", "get-url", "origin"])
Copy link
Member

Choose a reason for hiding this comment

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

will the remote always be origin? (just wondering)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes iiuc, in this case it us the one that SwiftPM sets

motivation: in some edge cases, the local repository may be partially valid (the containing directory is a legit git repo, but the repository diretory is not), or otherwise point to a remote different than the one expected

changes:
* validate that the local reposiotry remote aligns with the expected one, not just that the directory is a valid git repo
* refactor validation flow to be more streamlined
@tomerd tomerd force-pushed the git-remote-validation branch from 9da2275 to 601fbd2 Compare November 10, 2023 22:49
Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Can we have a test for this? Maybe some preset fixture that reproduces what we've seen, or we could create a new corrupted git repository in a temp directory during the test?

@tomerd
Copy link
Contributor Author

tomerd commented Nov 11, 2023

yea i am planning to add a test

@tomerd tomerd self-assigned this Nov 13, 2023
@tomerd
Copy link
Contributor Author

tomerd commented Nov 14, 2023

@swift-ci test

@MaxDesiatov
Copy link
Contributor

@swift-ci test windows

@tomerd
Copy link
Contributor Author

tomerd commented Nov 17, 2023

@neonichu based on our discussion yesterday, is this good to go?

@neonichu
Copy link
Contributor

Yah, I think so.

@tomerd tomerd enabled auto-merge (squash) November 17, 2023 18:51
auto-merge was automatically disabled November 27, 2023 04:49

Base branch was modified

@tomerd tomerd merged commit 979c8dd into swiftlang:main Nov 27, 2023
tomerd added a commit to tomerd/swift-package-manager that referenced this pull request Nov 30, 2023
motivation: in some edge cases, the local repository may be partially
valid (the containing directory is a legit git repo, but the repository
directory is not), or otherwise point to a remote different than the one
expected

changes:
* validate that the local repository remote aligns with the expected
one, not just that the directory is a valid git repo
* refactor validation flow to be more streamlined
tomerd added a commit that referenced this pull request Dec 1, 2023
motivation: in some edge cases, the local repository may be partially
valid (the containing directory is a legit git repo, but the repository
directory is not), or otherwise point to a remote different than the one
expected

changes:
* validate that the local repository remote aligns with the expected
one, not just that the directory is a valid git repo
* refactor validation flow to be more streamlined
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.

4 participants