-
Notifications
You must be signed in to change notification settings - Fork 117
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
Include repository license in package when available #920
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import ( | |
"os" | ||
"path/filepath" | ||
|
||
"github.com/magefile/mage/sh" | ||
"github.com/pkg/errors" | ||
|
||
"github.com/elastic/package-spec/code/go/pkg/validator" | ||
|
@@ -19,6 +20,7 @@ import ( | |
) | ||
|
||
const builtPackagesFolder = "packages" | ||
const licenseTextFileName = "LICENSE.txt" | ||
|
||
type BuildOptions struct { | ||
PackageRoot string | ||
|
@@ -157,6 +159,12 @@ func BuildPackage(options BuildOptions) (string, error) { | |
return "", errors.Wrap(err, "copying package contents failed") | ||
} | ||
|
||
logger.Debug("Copy license file if needed") | ||
err = copyLicenseTextFile(filepath.Join(destinationDir, licenseTextFileName)) | ||
if err != nil { | ||
return "", errors.Wrap(err, "copying license text file") | ||
} | ||
|
||
logger.Debug("Encode dashboards") | ||
err = encodeDashboards(destinationDir) | ||
if err != nil { | ||
|
@@ -233,7 +241,53 @@ func signZippedPackage(options BuildOptions, zippedPackagePath string) error { | |
return nil | ||
} | ||
|
||
func copyLicenseTextFile(licensePath string) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess that you don't check if the license content has changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean? The file is copied as is (as any other file in the package). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this content in the README is generated and has to be kept on sync with the fields definitions. Nothing is generated in the license files, they are copied as they are, as other files in packages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok 👍 |
||
_, err := os.Stat(licensePath) | ||
if err == nil { | ||
logger.Debug("License file in the package will be used") | ||
return nil | ||
} | ||
|
||
sourceLicensePath, err := findRepositoryLicense() | ||
if errors.Is(err, os.ErrNotExist) { | ||
logger.Debug("No license text file is included in package") | ||
return nil | ||
} | ||
if err != nil { | ||
return errors.Wrap(err, "failure while looking for license in repository") | ||
} | ||
|
||
logger.Infof("License text found in %q will be included in package", sourceLicensePath) | ||
err = sh.Copy(licensePath, sourceLicensePath) | ||
if err != nil { | ||
return errors.Wrap(err, "can't copy license from repository") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func createBuildDirectory(dirs ...string) (string, error) { | ||
dir, err := findRepositoryRootDirectory() | ||
if errors.Is(err, os.ErrNotExist) { | ||
return "", errors.New("package can be only built inside of a Git repository (.git folder is used as reference point)") | ||
} | ||
Comment on lines
+270
to
+273
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it change the implementation logic or is it just refactoring? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just refactoring, this is basically a copy of what there was in the loop looking for |
||
if err != nil { | ||
return "", err | ||
} | ||
|
||
p := []string{dir, "build"} | ||
if len(dirs) > 0 { | ||
p = append(p, dirs...) | ||
} | ||
buildDir := filepath.Join(p...) | ||
err = os.MkdirAll(buildDir, 0755) | ||
if err != nil { | ||
return "", errors.Wrapf(err, "mkdir failed (path: %s)", buildDir) | ||
} | ||
return buildDir, nil | ||
} | ||
|
||
func findRepositoryRootDirectory() (string, error) { | ||
workDir, err := os.Getwd() | ||
if err != nil { | ||
return "", errors.Wrap(err, "locating working directory failed") | ||
|
@@ -244,22 +298,29 @@ func createBuildDirectory(dirs ...string) (string, error) { | |
path := filepath.Join(dir, ".git") | ||
fileInfo, err := os.Stat(path) | ||
if err == nil && fileInfo.IsDir() { | ||
p := []string{dir, "build"} | ||
if len(dirs) > 0 { | ||
p = append(p, dirs...) | ||
} | ||
buildDir := filepath.Join(p...) | ||
err = os.MkdirAll(buildDir, 0755) | ||
if err != nil { | ||
return "", errors.Wrapf(err, "mkdir failed (path: %s)", buildDir) | ||
} | ||
return buildDir, nil | ||
return dir, nil | ||
} | ||
|
||
if dir == "/" { | ||
break | ||
} | ||
dir = filepath.Dir(dir) | ||
} | ||
return "", errors.New("package can be only built inside of a Git repository (.git folder is used as reference point)") | ||
|
||
return "", os.ErrNotExist | ||
} | ||
|
||
func findRepositoryLicense() (string, error) { | ||
dir, err := findRepositoryRootDirectory() | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
sourceLicensePath := filepath.Join(dir, licenseTextFileName) | ||
_, err = os.Stat(sourceLicensePath) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
return sourceLicensePath, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please leave
logger.Debugf
here for consistency with other steps (clear target, encode dashboards, resolve fields).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it does different things depending on the situation, so it will log two messages for the same thing, do we still want the log message here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, message added, now it looks like this:
When copied from the repo:
When already available in the package: