Skip to content
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 deps in static report bulk option #324

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

eemcmullan
Copy link
Collaborator

Copy link
Member

@aufi aufi left a comment

Choose a reason for hiding this comment

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

Thanks for a quick fix on the ticket!

@@ -1660,7 +1664,7 @@ func (a *analyzeCommand) moveResults() error {
if err != nil {
return err
}
err = copyFileContents(depsPath, fmt.Sprintf("%s.%s", analysisLogFilePath, a.inputShortName()))
err = copyFileContents(depsPath, fmt.Sprintf("%s.%s", depsPath, a.inputShortName()))
Copy link
Member

Choose a reason for hiding this comment

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

👍 Thanks

for i := range outputDeps {
_, depErr := os.Stat(outputDeps[i])
for i := range depFiles {
_, depErr := os.Stat(depFiles[i])
Copy link
Member

@aufi aufi Aug 26, 2024

Choose a reason for hiding this comment

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

I think this can end up in different lenghts of outputAnalyses and depFiles arrays (when user called --bulk to the same output directory for analysis with and without dependencies, which is not recommended, but there no hard check for it).

(defining those array together on lines 1585-1586 / 1587-1588 should have been ensuring those array will be the same length).

if (hasJava || hasGo) && a.mode == string(provider.FullAnalysisMode) && len(a.providersMap) == 1 {
staticReportArgs = append(staticReportArgs,
fmt.Sprintf("--deps-output-list=%s", DepsOutputMountPath))
if !a.bulk {
Copy link
Member

Choose a reason for hiding this comment

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

👍

_, hasGo := a.providersMap[goProvider]
if (hasJava || hasGo) && a.mode == string(provider.FullAnalysisMode) && len(a.providersMap) == 1 {
staticReportArgs = append(staticReportArgs,
fmt.Sprintf("--deps-output-list=%s", DepsOutputMountPath))
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about the DepsOutputMountPath here (instead of outputDeps), but that was introduced in #291 which I've seen already merged, so didn't pay much attention to it, will check better next time.

@eemcmullan eemcmullan merged commit 65dc385 into konveyor:main Aug 27, 2024
3 checks passed
eemcmullan added a commit that referenced this pull request Aug 28, 2024
* remove first prov container last

Signed-off-by: Emily McMullan <[email protected]>

* handle cleanup for interrupt

Signed-off-by: Emily McMullan <[email protected]>

* include more detailed container names

Signed-off-by: Emily McMullan <[email protected]>

* fix deps in static report bulk option

Signed-off-by: Emily McMullan <[email protected]>

---------

Signed-off-by: Emily McMullan <[email protected]>
@eemcmullan eemcmullan deleted the bulk-deps branch September 17, 2024 13:44
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.

2 participants