[GitHub] [camel-k] lburgazzoli commented on a change in pull request #1777: Inspect all transitive dependencies of an integration

[GitBox <gi...@apache.org>]

lburgazzoli commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r511794988



##########
File path: pkg/cmd/inspect.go
##########
@@ -32,36 +36,62 @@ import (
 	"github.com/spf13/cobra"
 )
 
+var acceptedDependencyTypes = []string{"bom", "camel", "camel-k", "camel-quarkus", "mvn", "github"}
+
+const (
+	defaultWorkspaceDirectoryName    = "workspace"
+	defaultDependenciesDirectoryName = "dependencies"
+)
+
 func newCmdInspect(rootCmdOptions *RootCmdOptions) (*cobra.Command, *inspectCmdOptions) {
 	options := inspectCmdOptions{
 		RootCmdOptions: rootCmdOptions,
 	}
 
 	cmd := cobra.Command{
-		Use:     "inspect [files to inspect]",
-		Short:   "Generate dependencies list the given integration files.",
-		Long:    `Compute and emit the dependencies for a list of integration files.`,
+		Use:   "inspect [files to inspect]",
+		Short: "Generate dependencies list given integration files.",
+		Long: `Output dependencies for a list of integration files. By default this command returns the
+top level dependencies only. When --all-dependencies is enabled, the transitive dependencies
+will be generated by calling Maven and then copied into the directory pointed to by the
+--dependencies-directory flag.`,
 		PreRunE: decode(&options),
 		RunE: func(_ *cobra.Command, args []string) error {
 			if err := options.validate(args); err != nil {
 				return err
 			}
+			if err := options.initialize(args); err != nil {
+				return err
+			}
 			if err := options.run(args); err != nil {
 				fmt.Println(err.Error())
 			}
 
 			return nil
 		},
+		Annotations: map[string]string{
+			offlineCommandLabel: "true",
+		},
 	}
 
+	cmd.Flags().Bool("all-dependencies", false, "Compute transitive dependencies and move them to directory pointed to by the --dependencies-directory flag.")
+	cmd.Flags().StringArrayP("additional-dependencies", "d", nil, `Comma-separated lists of additional top-level dependencies with the format:

Review comment:
       I don't think they necessarily be copmma separated but you can also repeat the flag

##########
File path: pkg/builder/builder_steps.go
##########
@@ -140,7 +141,17 @@ func generateProjectSettings(ctx *Context) error {
 
 func injectDependencies(ctx *Context) error {
 	// Add dependencies from build
-	for _, d := range ctx.Build.Dependencies {
+	return InjectDependenciesCommon(&ctx.Maven.Project, ctx.Build.Dependencies, ctx.Catalog)
+}
+
+// InjectDependenciesCommon --
+func InjectDependenciesCommon(

Review comment:
       We should avoid this `*Common` suffix, I'd rather move those functions to a dedicated package or rethink the names used for the steps

##########
File path: pkg/cmd/inspect.go
##########
@@ -32,36 +36,62 @@ import (
 	"github.com/spf13/cobra"
 )
 
+var acceptedDependencyTypes = []string{"bom", "camel", "camel-k", "camel-quarkus", "mvn", "github"}
+
+const (
+	defaultWorkspaceDirectoryName    = "workspace"
+	defaultDependenciesDirectoryName = "dependencies"
+)
+
 func newCmdInspect(rootCmdOptions *RootCmdOptions) (*cobra.Command, *inspectCmdOptions) {
 	options := inspectCmdOptions{
 		RootCmdOptions: rootCmdOptions,
 	}
 
 	cmd := cobra.Command{
-		Use:     "inspect [files to inspect]",
-		Short:   "Generate dependencies list the given integration files.",
-		Long:    `Compute and emit the dependencies for a list of integration files.`,
+		Use:   "inspect [files to inspect]",
+		Short: "Generate dependencies list given integration files.",
+		Long: `Output dependencies for a list of integration files. By default this command returns the
+top level dependencies only. When --all-dependencies is enabled, the transitive dependencies
+will be generated by calling Maven and then copied into the directory pointed to by the
+--dependencies-directory flag.`,
 		PreRunE: decode(&options),
 		RunE: func(_ *cobra.Command, args []string) error {
 			if err := options.validate(args); err != nil {
 				return err
 			}
+			if err := options.initialize(args); err != nil {
+				return err
+			}
 			if err := options.run(args); err != nil {
 				fmt.Println(err.Error())
 			}
 
 			return nil
 		},
+		Annotations: map[string]string{
+			offlineCommandLabel: "true",
+		},
 	}
 
+	cmd.Flags().Bool("all-dependencies", false, "Compute transitive dependencies and move them to directory pointed to by the --dependencies-directory flag.")
+	cmd.Flags().StringArrayP("additional-dependencies", "d", nil, `Comma-separated lists of additional top-level dependencies with the format:
+<type>:<dependency-name>
+where <type> is one of {`+strings.Join(acceptedDependencyTypes, "|")+`}.`)
+	cmd.Flags().String("workspace", "", "Workspace directory. Default: <kamel-invocation-directory>/workspace")

Review comment:
       I'm not very sure we want to expose this "workspace" concept to the user as in general camel-k hides all the project related stuffs

##########
File path: pkg/cmd/inspect.go
##########
@@ -32,36 +36,62 @@ import (
 	"github.com/spf13/cobra"
 )
 
+var acceptedDependencyTypes = []string{"bom", "camel", "camel-k", "camel-quarkus", "mvn", "github"}
+
+const (
+	defaultWorkspaceDirectoryName    = "workspace"
+	defaultDependenciesDirectoryName = "dependencies"
+)
+
 func newCmdInspect(rootCmdOptions *RootCmdOptions) (*cobra.Command, *inspectCmdOptions) {
 	options := inspectCmdOptions{
 		RootCmdOptions: rootCmdOptions,
 	}
 
 	cmd := cobra.Command{
-		Use:     "inspect [files to inspect]",
-		Short:   "Generate dependencies list the given integration files.",
-		Long:    `Compute and emit the dependencies for a list of integration files.`,
+		Use:   "inspect [files to inspect]",
+		Short: "Generate dependencies list given integration files.",
+		Long: `Output dependencies for a list of integration files. By default this command returns the
+top level dependencies only. When --all-dependencies is enabled, the transitive dependencies
+will be generated by calling Maven and then copied into the directory pointed to by the
+--dependencies-directory flag.`,
 		PreRunE: decode(&options),
 		RunE: func(_ *cobra.Command, args []string) error {
 			if err := options.validate(args); err != nil {
 				return err
 			}
+			if err := options.initialize(args); err != nil {
+				return err
+			}
 			if err := options.run(args); err != nil {
 				fmt.Println(err.Error())
 			}
 
 			return nil
 		},
+		Annotations: map[string]string{
+			offlineCommandLabel: "true",
+		},
 	}
 
+	cmd.Flags().Bool("all-dependencies", false, "Compute transitive dependencies and move them to directory pointed to by the --dependencies-directory flag.")
+	cmd.Flags().StringArrayP("additional-dependencies", "d", nil, `Comma-separated lists of additional top-level dependencies with the format:

Review comment:
       maybe it is better to keep it as `--dependency` as it is for the run command

##########
File path: pkg/cmd/inspect.go
##########
@@ -86,32 +116,110 @@ func (command *inspectCmdOptions) validate(args []string) error {
 		}
 	}
 
+	// Validate list of additional dependencies i.e. make sure that each dependency has
+	// a valid type.
+	if command.AdditionalDependencies != nil {
+		for _, additionalDependency := range command.AdditionalDependencies {
+			additionalDependencies := strings.Split(additionalDependency, ",")
+
+			for _, dependency := range additionalDependencies {
+				dependencyComponents := strings.Split(dependency, ":")
+
+				TypeIsValid := false
+				for _, dependencyType := range acceptedDependencyTypes {
+					if dependencyType == dependencyComponents[0] {
+						TypeIsValid = true
+					}
+				}
+
+				if !TypeIsValid {
+					return errors.New("Unexpected type for user-provided dependency: " + dependency + ", check command usage for valid format.")
+				}
+			}
+		}
+	}
+
+	// If provided, ensure that that the dependencies directory exists.
+	if command.DependenciesDirectory != "" {
+		dependenciesDirectoryExists, err := util.DirectoryExists(command.DependenciesDirectory)
+		// Report any error.
+		if err != nil {
+			return err
+		}
+
+		// Signal file not found.
+		if !dependenciesDirectoryExists {
+			return errors.New("input file " + command.DependenciesDirectory + " file does not exist")
+		}
+	}
+
+	return nil
+}
+
+func (command *inspectCmdOptions) initialize(args []string) error {
+	// If --all-dependencies flag is set the workspace and dependencies directories need to have
+	// valid values. If not provided on the command line, the values need to be initialized with
+	// their default values.
+	if command.AllDependencies {
+		// Create workspace directory to hold all intermediate files.
+		err := createAndSetWorkspaceDirectory(command)
+		if err != nil {
+			return err
+		}
+
+		// Move the integration dependecies to the dependencies directory.
+		err = createAndSetDependenciesDirectory(command)
+		if err != nil {
+			return err
+		}
+	}
 	return nil
 }
 
 func (command *inspectCmdOptions) run(args []string) error {
-	// Attempt to reuse existing Camel catalog if one is present.
-	catalog, err := camel.MainCatalog()
+	// Fetch existing catalog or create new one if one does not already exist.
+	catalog, err := createCamelCatalog()
+
+	// Output top-level dependencies.
+	outputTopLevel := !command.AllDependencies

Review comment:
       I think transitive dependencies should not be handled in a different manner than the top level one so they should be included in the output as well.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org