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

Inconsistency constructing settings & scan results paths #87

Closed
deadmoose opened this issue Feb 7, 2025 · 1 comment · Fixed by #94
Closed

Inconsistency constructing settings & scan results paths #87

deadmoose opened this issue Feb 7, 2025 · 1 comment · Fixed by #94
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@deadmoose
Copy link
Collaborator

In the UI, there are two separate handlers deriving scan settings & results paths based on the scanroot when it changes:

In addition to being two copies of the same string building, it's doing that without worrying about the platform-local path separators (e.g. on windows you get a path mixing forward & backslashes such as C:\foo\bar\baz/.scanoss/results.json)

From there, in the UI the output location is immutable, but users ARE able to update the settings file location.

When a scan is performed, the values are pulled from the UI elements & built into the string array passed to ScanStream, and then it calls SetScanRoot bridging over to go-land:

In THAT, it does its own job of setting results & settings paths based on the scanroot. That DOES use golang's path/filepath, so they use proper path separators.

However, as mentioned above, the settings file location may have been modified in the UI, and this will stomp on it so it doesn't persist to the next time you open the scan dialog (although as mentioned above, opening the dialog ALSO stomps on whatever custom values may have been there)

This all leads to a series of closely entangled issues:

  • There are 3 separate implementation of the logic deriving the results + settings paths based on scanroot which need to kept in sync
  • The 2 javascript implementations fail to handle platform-native path separators
  • Despite the UI being mutable for the settings file location, it's almost immutable because it's frequently stomped on
  • The scan results path is theoretically mutable via a flag (--input) but effectively immutable because the UI always stomps on it before it's ever used
@deadmoose
Copy link
Collaborator Author

Oh, and the other-other closely related issue worth keeping in mind while digging into this:

Currently the ordering is:

  1. Call ScanStream with args built based on values from the UI
  2. Update the config object with the values from the UI

Upstream here, that's fine because ScanStream only cares about the string slice of flags to pass along to the python tool. I've got a forked scan implementation that does read things from the config object, so I've had to flip those two lines. (Which also demonstrates how ephemeral the UI's settings file field is)

@matiasdaloia matiasdaloia self-assigned this Feb 12, 2025
@matiasdaloia matiasdaloia added enhancement New feature or request bug Something isn't working labels Feb 12, 2025
@matiasdaloia matiasdaloia linked a pull request Feb 19, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants