-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: add command to run repo and commit finder without analysis #827
base: staging
Are you sure you want to change the base?
Conversation
…finding and commit finding for a PURL, or commit finding for a PURL and repo Signed-off-by: Ben Selwyn-Smith <benselwynsmith@googlemail.com>
Signed-off-by: Ben Selwyn-Smith <benselwynsmith@googlemail.com>
Signed-off-by: Ben Selwyn-Smith <benselwynsmith@googlemail.com>
Signed-off-by: Ben Selwyn-Smith <benselwynsmith@googlemail.com>
|
||
analyzer = Analyzer(global_config.output_path, global_config.build_log_path) | ||
repo_dir = os.path.join(analyzer.output_path, analyzer.GIT_REPOS_DIR) | ||
git_obj = analyzer.prepare_repo(repo_dir, found_repo, "", "", purl) |
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 again at the implementation of this prepare_repo
function, it only uses self
at a few locations to run static methods:
macaron/src/macaron/slsa_analyzer/analyzer.py
Line 867 in 2d3a64a
git_service = self.get_git_service(resolved_remote_path) macaron/src/macaron/slsa_analyzer/analyzer.py
Line 878 in 2d3a64a
resolved_local_path = self._resolve_local_path(self.local_repos_path, repo_path) macaron/src/macaron/slsa_analyzer/analyzer.py
Line 913 in 2d3a64a
git_service = self.get_git_service(origin_remote_url)
Do you think it would be worth while to refactor this method to another module to avoid having to silent the pylint cyclic-import
error?
I tried to run the
I believe we don't support Maven PURL without a version now - macaron/src/macaron/repo_finder/repo_finder_java.py Lines 53 to 56 in 44dbf0a
However, the debug message doesn't show up if I ran it without the verbose flag. I believe we should let the user of the |
I also think it's great to add an integration test case for the new command too (may be we can revisit this after we settle on how to return the results to the user). |
Signed-off-by: Ben Selwyn-Smith <benselwynsmith@googlemail.com>
@@ -218,3 +213,158 @@ def find_source(purl_string: str, repo: str | None) -> bool: | |||
logger.info("%s/commit/%s", found_repo, digest) | |||
|
|||
return True | |||
|
|||
|
|||
def prepare_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.
Thanks for making the refactoring. However, I think prepare_repo
, get_git_service
and get_local_repos_path
can be moved to a new package within src/macaron/
(for example, prepare_repo
is used both within analyzer.py
and repo_finder.py
).
We could name this new package something like "repo_utils" or something like that 🤔
@behnazh-w I would love to know what you think about this.
This pull request adds a new command
find-source
that requires a PURL, and optionally accepts a repository path as input. If no repository path is provided, the command will call the Repo Finder and Commit Finder to find the repository and commit of the provided PURL. If a repository is provided as input, only the Commit Finder will be called.Closes #781