You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@skywalking.apache.org by ke...@apache.org on 2022/06/29 07:38:48 UTC

[skywalking-eyes] branch main updated: Add support for multiple licenses in the header config section (#118)

This is an automated email from the ASF dual-hosted git repository.

kezhenxu94 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/skywalking-eyes.git


The following commit(s) were added to refs/heads/main by this push:
     new c343ca4  Add support for multiple licenses in the header config section (#118)
c343ca4 is described below

commit c343ca474d69d2aabfded07455baef4d8819a58b
Author: Dave Tucker <da...@dtucker.co.uk>
AuthorDate: Wed Jun 29 08:38:44 2022 +0100

    Add support for multiple licenses in the header config section (#118)
---
 README.md                                         | 11 ++-
 commands/deps_check.go                            | 18 ++++-
 commands/deps_resolve.go                          | 10 ++-
 commands/header_check.go                          | 36 +++++----
 commands/header_fix.go                            | 34 ++++-----
 commands/root.go                                  |  3 +-
 pkg/config/config.go                              | 90 +++++++++++++++++++----
 pkg/review/header.go                              | 13 ++--
 test/config_test.go                               | 25 +++++--
 test/testdata/multiple_license_test/pkg/a/main.go | 15 ++++
 test/testdata/multiple_license_test/pkg/b/main.go |  5 ++
 test/testdata/test-multiple.yaml                  | 14 ++++
 12 files changed, 209 insertions(+), 65 deletions(-)

diff --git a/README.md b/README.md
index 405d1d0..79343d2 100644
--- a/README.md
+++ b/README.md
@@ -774,7 +774,16 @@ dependency: # <15>
       version: dependency-version # the same format as <19>
 ```
 
-1. The `header` section is configurations for source codes license header.
+1. The `header` section is configurations for source codes license header. If you have mutliple modules or packages in your project that have differing licenses, this section may contain a list of licenses:
+```yaml
+header:
+  - license:
+    spdx-id: Apache-2.0
+    path: "/path/to/module/a"
+  - license:
+    spdx-id: MPL-2.0
+    path: "/path/to/module/b"
+```
 2. The [SPDX ID](https://spdx.org/licenses/) of the license, it’s convenient when your license is standard SPDX license, so that you can simply specify this identifier without copying the whole license `content` or `pattern`. This will be used as the content when `fix` command needs to insert a license header.
 3. The copyright owner to replace the `[owner]` in the `SPDX-ID` license template.
 4. If you are not using the standard license text, you can paste your license text here, this will be used as the content when `fix` command needs to insert a license header, if both `license` and `SPDX-ID` are specified, `license` wins.
diff --git a/commands/deps_check.go b/commands/deps_check.go
index d739e7e..8073d0b 100644
--- a/commands/deps_check.go
+++ b/commands/deps_check.go
@@ -18,8 +18,11 @@
 package commands
 
 import (
+	"fmt"
+
 	"github.com/spf13/cobra"
 
+	"github.com/apache/skywalking-eyes/internal/logger"
 	"github.com/apache/skywalking-eyes/pkg/deps"
 )
 
@@ -28,6 +31,19 @@ var DepsCheckCommand = &cobra.Command{
 	Aliases: []string{"c"},
 	Long:    "resolves and check license compatibility in all dependencies of a module and their transitive dependencies",
 	RunE: func(cmd *cobra.Command, args []string) error {
-		return deps.Check(Config.Header.License.SpdxID, &Config.Deps)
+		var errors []error
+		configDeps := Config.Dependencies()
+		for _, header := range Config.Headers() {
+			if err := deps.Check(header.License.SpdxID, configDeps); err != nil {
+				errors = append(errors, err)
+			}
+		}
+		if len(errors) > 0 {
+			for _, err := range errors {
+				logger.Log.Error(err)
+			}
+			return fmt.Errorf("one or more errors occurred checking license compatibility")
+		}
+		return nil
 	},
 }
diff --git a/commands/deps_resolve.go b/commands/deps_resolve.go
index 4c8e197..cc4d00c 100644
--- a/commands/deps_resolve.go
+++ b/commands/deps_resolve.go
@@ -77,7 +77,8 @@ var DepsResolveCommand = &cobra.Command{
 	RunE: func(cmd *cobra.Command, args []string) error {
 		report := deps.Report{}
 
-		if err := deps.Resolve(&Config.Deps, &report); err != nil {
+		configDeps := Config.Dependencies()
+		if err := deps.Resolve(configDeps, &report); err != nil {
 			return err
 		}
 
@@ -132,7 +133,12 @@ func writeSummary(rep *deps.Report) error {
 		return err
 	}
 	defer file.Close()
-	summary, err := deps.GenerateSummary(summaryTpl, &Config.Header, rep)
+
+	headers := Config.Headers()
+	if len(headers) > 1 {
+		return fmt.Errorf("unable to write summary as multiple licenses were provided in configuration")
+	}
+	summary, err := deps.GenerateSummary(summaryTpl, headers[0], rep)
 	if err != nil {
 		return err
 	}
diff --git a/commands/header_check.go b/commands/header_check.go
index 7d99d6b..19f0e97 100644
--- a/commands/header_check.go
+++ b/commands/header_check.go
@@ -33,28 +33,34 @@ var CheckCommand = &cobra.Command{
 	Aliases: []string{"c"},
 	Long:    "check command walks the specified paths recursively and checks if the specified files have the license header in the config file.",
 	RunE: func(cmd *cobra.Command, args []string) error {
-		var result header.Result
+		hasErrors := false
+		for _, h := range Config.Headers() {
+			var result header.Result
 
-		if len(args) > 0 {
-			logger.Log.Debugln("Overriding paths with command line args.")
-			Config.Header.Paths = args
-		}
+			if len(args) > 0 {
+				logger.Log.Debugln("Overriding paths with command line args.")
+				h.Paths = args
+			}
 
-		if err := header.Check(&Config.Header, &result); err != nil {
-			return err
-		}
+			if err := header.Check(h, &result); err != nil {
+				return err
+			}
 
-		logger.Log.Infoln(result.String())
+			logger.Log.Infoln(result.String())
 
-		writeSummaryQuietly(&result)
+			writeSummaryQuietly(&result)
 
-		if result.HasFailure() {
-			if err := review.Header(&result, &Config); err != nil {
-				logger.Log.Warnln("Failed to create review comments", err)
+			if result.HasFailure() {
+				if err := review.Header(&result, h); err != nil {
+					logger.Log.Warnln("Failed to create review comments", err)
+				}
+				hasErrors = true
+				logger.Log.Error(result.Error())
 			}
-			return result.Error()
 		}
-
+		if hasErrors {
+			return fmt.Errorf("one or more files does not have a valid license header")
+		}
 		return nil
 	},
 }
diff --git a/commands/header_fix.go b/commands/header_fix.go
index 7473706..1e35889 100644
--- a/commands/header_fix.go
+++ b/commands/header_fix.go
@@ -32,30 +32,30 @@ var FixCommand = &cobra.Command{
 	Aliases: []string{"f"},
 	Long:    "fix command walks the specified paths recursively and fix the license header if the specified files don't have the license header.",
 	RunE: func(cmd *cobra.Command, args []string) error {
-		var result header.Result
 		var errors []string
-		var files []string
-
-		if len(args) > 0 {
-			files = args
-		} else if err := header.Check(&Config.Header, &result); err != nil {
-			return err
-		} else {
-			files = result.Failure
-		}
-
-		for _, file := range files {
-			if err := header.Fix(file, &Config.Header, &result); err != nil {
-				errors = append(errors, err.Error())
+		for _, h := range Config.Headers() {
+			var result header.Result
+			var files []string
+
+			if len(args) > 0 {
+				files = args
+			} else if err := header.Check(h, &result); err != nil {
+				return err
+			} else {
+				files = result.Failure
 			}
-		}
 
-		logger.Log.Infoln(result.String())
+			for _, file := range files {
+				if err := header.Fix(file, h, &result); err != nil {
+					errors = append(errors, err.Error())
+				}
+			}
 
+			logger.Log.Infoln(result.String())
+		}
 		if len(errors) > 0 {
 			return fmt.Errorf("%s", strings.Join(errors, "\n"))
 		}
-
 		return nil
 	},
 }
diff --git a/commands/root.go b/commands/root.go
index 92cbe8d..6f74a91 100644
--- a/commands/root.go
+++ b/commands/root.go
@@ -44,7 +44,8 @@ var root = &cobra.Command{
 		}
 		logger.Log.SetLevel(level)
 
-		return Config.Parse(configFile)
+		Config, err = config.NewConfigFromFile(configFile)
+		return err
 	},
 	Version: version,
 }
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 1d552a7..1d822df 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -28,37 +28,97 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-type Config struct {
+type V1 struct {
 	Header header.ConfigHeader `yaml:"header"`
 	Deps   deps.ConfigDeps     `yaml:"dependency"`
 }
 
-// Parse reads and parses the header check configurations in config file.
-func (config *Config) Parse(file string) (err error) {
+func ParseV1(filename string, bytes []byte) (*V1, error) {
+	var config V1
+	if err := yaml.Unmarshal(bytes, &config); err != nil {
+		return nil, err
+	}
+
+	if err := config.Header.Finalize(); err != nil {
+		return nil, err
+	}
+
+	if err := config.Deps.Finalize(filename); err != nil {
+		return nil, err
+	}
+	return &config, nil
+}
+
+func (config *V1) Headers() []*header.ConfigHeader {
+	return []*header.ConfigHeader{&config.Header}
+}
+
+func (config *V1) Dependencies() *deps.ConfigDeps {
+	return &config.Deps
+}
+
+type V2 struct {
+	Header []*header.ConfigHeader `yaml:"header"`
+	Deps   deps.ConfigDeps        `yaml:"dependency"`
+}
+
+func ParseV2(filename string, bytes []byte) (*V2, error) {
+	var config V2
+	if err := yaml.Unmarshal(bytes, &config); err != nil {
+		return nil, err
+	}
+
+	for _, header := range config.Header {
+		if err := header.Finalize(); err != nil {
+			return nil, err
+		}
+	}
+
+	if err := config.Deps.Finalize(filename); err != nil {
+		return nil, err
+	}
+
+	return &config, nil
+}
+
+func (config *V2) Headers() []*header.ConfigHeader {
+	return config.Header
+}
+
+func (config *V2) Dependencies() *deps.ConfigDeps {
+	return &config.Deps
+}
+
+type Config interface {
+	Headers() []*header.ConfigHeader
+	Dependencies() *deps.ConfigDeps
+}
+
+func NewConfigFromFile(filename string) (Config, error) {
+	var err error
 	var bytes []byte
 
 	// attempt to read configuration from specified file
-	logger.Log.Infoln("Loading configuration from file:", file)
+	logger.Log.Infoln("Loading configuration from file:", filename)
 
-	if bytes, err = os.ReadFile(file); err != nil && !os.IsNotExist(err) {
-		return err
+	if bytes, err = os.ReadFile(filename); err != nil && !os.IsNotExist(err) {
+		return nil, err
 	}
 
 	if os.IsNotExist(err) {
-		logger.Log.Infof("Config file %s does not exist, using the default config", file)
+		logger.Log.Infof("Config file %s does not exist, using the default config", filename)
 
 		if bytes, err = assets.Asset("default-config.yaml"); err != nil {
-			return err
+			return nil, err
 		}
 	}
 
-	if err := yaml.Unmarshal(bytes, config); err != nil {
-		return err
+	var config Config
+	if config, err = ParseV2(filename, bytes); err == nil {
+		return config, nil
 	}
-
-	if err := config.Header.Finalize(); err != nil {
-		return err
+	if config, err = ParseV1(filename, bytes); err != nil {
+		return nil, err
 	}
-
-	return config.Deps.Finalize(file)
+	return config, nil
 }
diff --git a/pkg/review/header.go b/pkg/review/header.go
index 139c51e..55d6b5d 100644
--- a/pkg/review/header.go
+++ b/pkg/review/header.go
@@ -33,7 +33,6 @@ import (
 
 	"github.com/apache/skywalking-eyes/internal/logger"
 	comments2 "github.com/apache/skywalking-eyes/pkg/comments"
-	config2 "github.com/apache/skywalking-eyes/pkg/config"
 	header2 "github.com/apache/skywalking-eyes/pkg/header"
 )
 
@@ -109,10 +108,10 @@ func Init() {
 }
 
 // Header reviews the license header, including suggestions on the pull request and an overview of the checks.
-func Header(result *header2.Result, config *config2.Config) error {
+func Header(result *header2.Result, config *header2.ConfigHeader) error {
 	ghOnce.Do(Init)
 
-	if !result.HasFailure() || !IsPR() || gh == nil || config.Header.Comment == header2.Never {
+	if !result.HasFailure() || !IsPR() || gh == nil || config.Comment == header2.Never {
 		return nil
 	}
 
@@ -150,7 +149,7 @@ func Header(result *header2.Result, config *config2.Config) error {
 				logger.Log.Warnln("Failed to determine the comment style of file:", changedFile.GetFilename())
 				continue
 			}
-			header, err := header2.GenerateLicenseHeader(style, &config.Header)
+			header, err := header2.GenerateLicenseHeader(style, config)
 			if err != nil {
 				logger.Log.Warnln("Failed to generate comment header:", changedFile.GetFilename())
 				continue
@@ -173,7 +172,7 @@ func Header(result *header2.Result, config *config2.Config) error {
 	return tryReview(result, config, comments)
 }
 
-func tryReview(result *header2.Result, config *config2.Config, comments []*github.DraftReviewComment) error {
+func tryReview(result *header2.Result, config *header2.ConfigHeader, comments []*github.DraftReviewComment) error {
 	tryBestEffortToComment := func() error {
 		if err := doReview(result, comments); err != nil {
 			logger.Log.Warnln("Failed to create review comment, fallback to a plain comment:", err)
@@ -183,11 +182,11 @@ func tryReview(result *header2.Result, config *config2.Config, comments []*githu
 		return nil
 	}
 
-	if config.Header.Comment == header2.Always {
+	if config.Comment == header2.Always {
 		if err := tryBestEffortToComment(); err != nil {
 			return err
 		}
-	} else if config.Header.Comment == header2.OnFailure && len(comments) > 0 {
+	} else if config.Comment == header2.OnFailure && len(comments) > 0 {
 		if err := tryBestEffortToComment(); err != nil {
 			return err
 		}
diff --git a/test/config_test.go b/test/config_test.go
index 13c4297..22608a6 100644
--- a/test/config_test.go
+++ b/test/config_test.go
@@ -26,8 +26,9 @@ import (
 )
 
 func TestConfigHeaderSpdxASF(t *testing.T) {
-	c := config2.Config{}
-	if err := c.Parse("./testdata/test-spdx-asf.yaml"); err != nil {
+	var c config2.Config
+	var err error
+	if c, err = config2.NewConfigFromFile("./testdata/test-spdx-asf.yaml"); err != nil {
 		t.Error("unexpected error", err)
 	}
 
@@ -48,14 +49,15 @@ KIND, either express or implied.  See the License for the
 specific language governing permissions and limitations
 under the License.
 `
-	if actual := c.Header.GetLicenseContent(); actual != expected {
+	if actual := c.Headers()[0].GetLicenseContent(); actual != expected {
 		t.Errorf("Actual: \n%v\nExpected: \n%v", actual, expected)
 	}
 }
 
 func TestConfigHeaderSpdxPlain(t *testing.T) {
-	c := config2.Config{}
-	if err := c.Parse("./testdata/test-spdx.yaml"); err != nil {
+	var c config2.Config
+	var err error
+	if c, err = config2.NewConfigFromFile("./testdata/test-spdx.yaml"); err != nil {
 		t.Error("unexpected error", err)
 	}
 
@@ -73,7 +75,18 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 `
-	if actual := c.Header.GetLicenseContent(); actual != expected {
+	if actual := c.Headers()[0].GetLicenseContent(); actual != expected {
 		t.Errorf("Actual: \n%v\nExpected: \n%v", actual, expected)
 	}
 }
+
+func TestConfigMultipleHeaders(t *testing.T) {
+	var c config2.Config
+	var err error
+	if c, err = config2.NewConfigFromFile("./testdata/test-multiple.yaml"); err != nil {
+		t.Error("unexpected error", err)
+	}
+	if len(c.Headers()) != 2 {
+		t.Errorf("Expected 2 header sections in the config. Actual %d", len(c.Headers()))
+	}
+}
diff --git a/test/testdata/multiple_license_test/pkg/a/main.go b/test/testdata/multiple_license_test/pkg/a/main.go
new file mode 100644
index 0000000..54ae0cb
--- /dev/null
+++ b/test/testdata/multiple_license_test/pkg/a/main.go
@@ -0,0 +1,15 @@
+// Copyright 2022 The Project Contributors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
diff --git a/test/testdata/multiple_license_test/pkg/b/main.go b/test/testdata/multiple_license_test/pkg/b/main.go
new file mode 100644
index 0000000..36def0f
--- /dev/null
+++ b/test/testdata/multiple_license_test/pkg/b/main.go
@@ -0,0 +1,5 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+package main
diff --git a/test/testdata/test-multiple.yaml b/test/testdata/test-multiple.yaml
new file mode 100644
index 0000000..13f16da
--- /dev/null
+++ b/test/testdata/test-multiple.yaml
@@ -0,0 +1,14 @@
+header:
+  - license:
+      spdx-id: Apache-2.0
+      copyright-owner: The Project Contributors
+
+    paths:
+      - 'test/testdata/multiple_license_test/pkg/a'
+
+  - license:
+      spdx-id: MPL-2.0
+      copyright-owner: The Project Contributors
+
+    paths:
+      - 'test/testdata/multiple_license_test/pkg/b'