-
Notifications
You must be signed in to change notification settings - Fork 34
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
Fix inconsistent path behaviors when running diffs #581
Conversation
Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>
@@ -99,6 +120,52 @@ func processSrc(ctx context.Context, c malcontent.Config, src, dest map[string]* | |||
} | |||
} | |||
|
|||
func processDest(ctx context.Context, c malcontent.Config, from, to map[string]*malcontent.FileReport, d *malcontent.DiffReport) { |
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.
I moved these functions around to make them easier to reference. They're coupled enough that jumping back and forth was a little painful.
if !behaviorExists(tb, fr.Behaviors) { | ||
tb.DiffAdded = true | ||
abs.Behaviors = append(abs.Behaviors, tb) | ||
continue |
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.
I also added this continue
.
Still seeing some odd diffs in the test data. Working through those now. |
Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>
@tstromberg -- let me know whether you think the updated test data is accurate. Most of the files we diff have entirely different names so the deleted/added behavior seems correct to me. Since we don't run the levenshtein distance for any non-JSON/SPDX files, we can't really expect files like |
Closes: #565
This PR should fix the behavior noted in the aforementioned Issue. The root cause was a couple of things, but it was made more obvious when scanning
/tmp
paths on macOS because this path is symlinked to/private/tmp
.Knowing this, I updated the code to evaluate symlinks and also made the behavior when scanning directories versus files consistent (the
/private/tmp
->/tmp
symlink behavior was making the relative path incorrect).Additionally, when passing in file paths specifically, the
fromPath
would match the scan path of files within the directory with the relative path product being.
. If both thesrc
anddest
reports contain the.
key, then it will appear as though completely unrelated files were actually modifications (which is incorrect).By checking if
fromPath
is a directory or not, we can set the value offromPath
such that the relative path between it andfiles.Value.Path
is essentially the filename which is stored as a key in the map we reference.As a last resort, I set
fromRoot
tofromPath
if the former evaluates to.
.A couple of examples with the fix:
Unrelated files in separate paths:
Same parent directory, related files, external scan:
Same parent directory, related files, internal scan:
Same parent directory, related files, internal scan, explicitly passing in files: