fix: account for analysis started in repo sub-dirs #504
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change accounts for the possibility that dependency files are specified or detected at levels lower than the root git directory. This is the case when analysis is started in a sub-directory (e.g., monorepos). When determining the base dependencies to make sure only the newly added dependencies are considered, a temporary git worktree is created with the common ancestor commit. That worktree is of the **entire** git repo at that commit and therefore the relative path to dependency files specified/detected at lower levels in the path are not correct. The effect is an error in parsing the file due to it not existing (at the expected path). The fallback when this happens is to assume that all current dependencies are new and to analyze them (e.g., the same as if the `--all-deps` option was provided). So, in that sense, the bug is an "over-analysis" compared to what was expected (fail secure/closed). This change creates a path for the previous dependency file that is relative to the git root directory instead of the current working directory. Other changes made include: * Add `CIBase` class prerequisite * Must be operating within the context of a git repository * Remove `Depfile` class `git` prerequisite as dead code * The `Depfile` class does not directly use `git` * Format and refactor throughout
cd-work
approved these changes
Nov 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change accounts for the possibility that dependency files are specified or detected at levels lower than the root git directory. This is the case when analysis is started in a sub-directory (e.g., monorepos).
When determining the base dependencies to make sure only the newly added dependencies are considered, a temporary git worktree is created with the common ancestor commit. That worktree is of the entire git repo at that commit and therefore the relative path to dependency files specified/detected at lower levels in the path are not correct. The effect is an error in parsing the file due to it not existing (at the expected path).
The fallback when this happens is to assume that all current dependencies are new and to analyze them (e.g., the same as if the
--all-depsoption was provided). So, in that sense, the bug is an "over-analysis" compared to what was expected (fail secure/closed).This change creates a path for the previous dependency file that is relative to the git root directory instead of the current working directory.
Other changes made include:
CIBaseclass prerequisiteDepfileclassgitprerequisite as dead codeDepfileclass does not directly usegitTesting
The changes in this PR are available for testing with the
maxrake/phylum-ci:git_depthDocker image found on Docker Hub.This is what it looks like with the current released version of
phylum-ci. Thephylum-dev/isildurs_banemonorepo is used for testing. A branch was created there and several dependency files were updated and committed. In this example, thecargo_workspacesub-directory was updated such that theCargo.lockthere has new dependencies. Analysis is started from within that sub-directory.Details (click to expand...)
This is what the same sequence looks like now, using the changes from this PR.
Notice that there is no error and only the newly added/changed dependencies are used (7 instead of 9).
Details (click to expand...)
This is what it looks like when trying to operate outside the context of a git repository:
Details (click to expand...)