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

Discussion on how to handle --config-path option of the analyze command after pull-request 401 #417

Closed
tromai opened this issue Aug 15, 2023 · 3 comments
Assignees
Labels
cli related to the Command-line Interface

Comments

@tromai
Copy link
Member

tromai commented Aug 15, 2023

As part of #401, the --config-path CLI option of Macaron has been agreed that it is not important for Macaron at the moment because of the following reasons:

  • In our Sphinx documentation, we don't have any examples for using this --config-path option.
  • The content of this config file is a duplication of others CLI options we have to specify the main component (e.g. repo-path or branch or digest). The adding of -purl in feat: add purl as a CLI options #401, we also decide that's not worth adding -purl option to the config file because it would lead to potential conflicting values between the input yaml config file and other options.

Therefore, it's decided to find a way to potentially remove the involvement of --config-path with the existing design of Macaron. There are multiple options, each with its own pros/cons. This ticket is created for the discussion on what is the best option to proceed.

  1. Remove the --config-path option completely. Because this feature is not likely to be known and used by the users of Macaron (as far as we know), we don't need to go through a "Deprecated" process for it.
  • Pros: remove any entangle with the current development. This make sure that this feature won't be in our way. It's reasonable to do this as we are still < v1.0 so it's possible for these changes.
  • Cons: what if there is need for this feature in the future, especially that the --config-path has support for pinning the dependencies version of the main software component that Macaron analyzes.
  1. Leave the --config-path as it is and ignore it as we move on with the development (need to make sure that its existence won't be conflicted with any existing or future CLI options).
  • Pros: We could still keep this feature for future usage
  • Cons: It could get in our way when we change the interface of Macaron.
  1. Move the --config-path to a separated option/command as a "light-weight" SBOM option where the user could define the specific dependencies by themselves.
  • However, I think that this could still require a lot of effort to move and maintain it without bringing any major benefits (as far as it is now).

When making any changes to this option, these are the aspects that we need to care about:

Please feel free to give your opinions and suggestions about this.

@tromai tromai added this to the Release v0.3.0 milestone Aug 15, 2023
@tromai tromai changed the title Add notices that the --config-path of the analyze command is going to be depreated soon. Discussion on how to handle --config-path option of the analyze command after #401 Aug 15, 2023
@nathanwn
Copy link
Member

On option 1: Removing --config-path completely.
I would go for this option. If there is a need for it in the future (like you are alluding to in option 3), we will need to revise the YAML schema and rethink the implementation for that specific need anyway. Leaving it around without actual use will create unnecessary noise in the command-line help menu and potentially interfere with the implementation of other things. I think fixing integration tests and documentation should not be a major issue.

@nathanwn nathanwn added the cli related to the Command-line Interface label Aug 21, 2023
@tromai tromai changed the title Discussion on how to handle --config-path option of the analyze command after #401 Discussion on how to handle --config-path option of the analyze command after pull-request 401 Aug 22, 2023
@tromai
Copy link
Member Author

tromai commented Sep 6, 2023

With further discuss, we have decided to not remove this feature completely because the YAML input file (provided through --config-path) allows us to pin the exact commit of the dependencies to run the analysis on. This is used in various places in our integration tests and we don't want to remove them.

Instead, this functionality will be "relocated" to a different place and have adjustments to its purpose.

  • The input yaml config file will be called a "repofinder-input" where it provides the repo-finder with the mapping between a software component (PURL) and its source definition
  • Each source definition is the combination of repo-path, branch and the specific commit's digest.
  • These mappings will support the repo-finder in looking for the source definition of the dependencies.

Furthermore, the schema of this file should be adjusted to that we use the new data model with PURL (instead of the old one repo-based one).

@tromai
Copy link
Member Author

tromai commented Aug 29, 2024

We have re-visited this matter and decide to remove --config-path completely.
It's now safe to do so because our improvements on repo, commit finder and dependency resolver ensure that we can pin the exact version of the software component through the input PURL, and Macaron would reliably resolve to the correct version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to the Command-line Interface
Projects
None yet
Development

No branches or pull requests

3 participants