-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
Fixed exception on repeated run of clone_repo_to #180
Conversation
Codecov Report
@@ Coverage Diff @@
## master #180 +/- ##
=======================================
Coverage 98.32% 98.32%
=======================================
Files 15 15
Lines 1015 1017 +2
=======================================
+ Hits 998 1000 +2
Misses 17 17
|
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.
Thanks for the PR!
I was thinking, can you do this check in clone_remote_repo
? In Line 149. What I had in mind is that when cloning the repo we check whether it already exists.
Yep, done. It makes more sense there and makes the code actually also shorter :) Thank you for pointing me to the correct place for the change. |
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.
Looking great! Thanks for the work!
Just a nit comment, will accept anyway!
pydriller/repository.py
Outdated
repo_folder = os.path.join(tmp_folder, self._get_repo_name_from_url(repo)) | ||
if os.path.isdir(repo_folder): | ||
logger.info("Reusing folder %s for %s", repo_folder, repo) | ||
else: | ||
logger.info("Cloning %s in temporary folder %s", repo, repo_folder) | ||
Repo.clone_from(url=repo, to_path=repo_folder) |
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.
Oh beautiful ❤️
pydriller/repository.py
Outdated
clone_folder = self._clone_folder() | ||
local_path_repo = self._clone_remote_repo(clone_folder, path_repo) |
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.
Can you undo this change? Thanks!
clone_folder = self._clone_folder() | |
local_path_repo = self._clone_remote_repo(clone_folder, path_repo) | |
local_path_repo = self._clone_remote_repo(self._clone_folder(), path_repo) |
Yes, I reverted the creation of the |
Awesome thank you for the PR! |
Hej, in this PR I modified the code so that repeated executions of the following form will work:
In its current implementation, the above code works for the first instantiation of
Repository
, where the repository is correctly cloned into the given directory. However, the second instantiation ofRepository
panics with an exception similar to the following. In essence, it does not work since Pydriller tries to clone the repository again.Since I did not expect such a behaviour and since I think it is practical when running repeated analysis on a repository, I implemented this fix with test.
To me, it is especially useful during exploratory analyzes of large repositories for which I do not want to repeatedly write code that checks if I already cloned a repo with another script.