From c48f22b72d6ca3d4146c6c4ce51a044432721e57 Mon Sep 17 00:00:00 2001 From: arcturusZhang Date: Thu, 8 Apr 2021 14:51:19 +0800 Subject: [PATCH 01/13] export one function --- tools/generator/autorest/changelog.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/generator/autorest/changelog.go b/tools/generator/autorest/changelog.go index 4e2a1e613354..0b056f89f3cf 100644 --- a/tools/generator/autorest/changelog.go +++ b/tools/generator/autorest/changelog.go @@ -127,7 +127,7 @@ func (p *ChangelogProcessor) GenerateChangelog(packagePath, tag string) (*Change if err != nil { return nil, fmt.Errorf("failed to get exports from package '%s' in the sdk '%s': %+v", packageName, p.ctx.SDKRoot(), err) } - r, err := getChangelogForPackage(lhs, rhs) + r, err := GetChangelogForPackage(lhs, rhs) if err != nil { return nil, fmt.Errorf("failed to generate changelog for package '%s': %+v", packagePath, err) } @@ -157,7 +157,8 @@ func getExportsForPackage(dir string) (*exports.Content, error) { return &exp, nil } -func getChangelogForPackage(lhs, rhs *exports.Content) (*model.Changelog, error) { +// GetChangelogForPackage generates the changelog report with the given two Contents +func GetChangelogForPackage(lhs, rhs *exports.Content) (*model.Changelog, error) { if lhs == nil && rhs == nil { return nil, fmt.Errorf("this package does not exist even after the generation, this should never happen") } From d2bbd0b2af28b0b34885561f97edc23d7105c0c7 Mon Sep 17 00:00:00 2001 From: arcturusZhang Date: Thu, 8 Apr 2021 17:50:42 +0800 Subject: [PATCH 02/13] no longer backup the repo, instead using readContent to get the content before generation --- tools/apidiff/report/report.go | 15 ++-- tools/generator/autorest/changelog.go | 82 +++++++------------ .../generator/autorest/generationMetadata.go | 6 +- tools/generator/cmd/clean.go | 23 +++++- tools/generator/cmd/metadata.go | 43 +++++----- tools/generator/cmd/root.go | 54 ++++++++++-- tools/pkgchk/track1/packages.go | 4 +- 7 files changed, 134 insertions(+), 93 deletions(-) diff --git a/tools/apidiff/report/report.go b/tools/apidiff/report/report.go index d79e893d55ad..b174dd3eb83e 100644 --- a/tools/apidiff/report/report.go +++ b/tools/apidiff/report/report.go @@ -120,6 +120,7 @@ func (p Package) writeNewContent(md *markdown.Writer) { if !p.HasAdditiveChanges() { return } + md.WriteTopLevelHeader("Additive Changes") writeConsts(p.AdditiveChanges.Consts, "New Constants", md) writeFuncs(p.AdditiveChanges.Funcs, "New Funcs", md) writeStructs(p.AdditiveChanges, "New Structs", "New Struct Fields", md) @@ -140,7 +141,7 @@ func writeSigChanges(bc *BreakingChanges, md *markdown.Writer) { if len(bc.Consts) == 0 && len(bc.Funcs) == 0 && len(bc.Structs) == 0 { return } - md.WriteTopLevelHeader("Signature Changes") + md.WriteHeader("Signature Changes") if len(bc.Consts) > 0 { items := make([]string, len(bc.Consts)) i := 0 @@ -149,7 +150,7 @@ func writeSigChanges(bc *BreakingChanges, md *markdown.Writer) { i++ } sort.Strings(items) - md.WriteHeader("Const Types") + md.WriteSubheader("Const Types") for _, item := range items { md.WriteLine(item) } @@ -163,7 +164,7 @@ func writeSigChanges(bc *BreakingChanges, md *markdown.Writer) { i++ } sort.Strings(items) - md.WriteHeader("Funcs") + md.WriteSubheader("Funcs") for _, item := range items { // now add params/returns info changes := bc.Funcs[item] @@ -184,7 +185,7 @@ func writeSigChanges(bc *BreakingChanges, md *markdown.Writer) { } } sort.Strings(items) - md.WriteHeader("Struct Fields") + md.WriteSubheader("Struct Fields") for _, item := range items { md.WriteLine(item) } @@ -245,16 +246,16 @@ func writeStructs(content *delta.Content, sheader1, sheader2 string, md *markdow if len(content.Structs) == 0 { return } - md.WriteTopLevelHeader("Struct Changes") + md.WriteHeader("Struct Changes") if len(content.CompleteStructs) > 0 { - md.WriteHeader(sheader1) + md.WriteSubheader(sheader1) for _, s := range content.CompleteStructs { md.WriteLine(fmt.Sprintf("1. %s", s)) } } modified := content.GetModifiedStructs() if len(modified) > 0 { - md.WriteHeader(sheader2) + md.WriteSubheader(sheader2) items := make([]string, 0, len(content.Structs)-len(content.CompleteStructs)) for s, f := range modified { for _, af := range f.AnonymousFields { diff --git a/tools/generator/autorest/changelog.go b/tools/generator/autorest/changelog.go index 0b056f89f3cf..e4b3d2b4ae5e 100644 --- a/tools/generator/autorest/changelog.go +++ b/tools/generator/autorest/changelog.go @@ -2,23 +2,19 @@ package autorest import ( "fmt" + "github.com/Azure/azure-sdk-for-go/tools/generator/utils" "os" "path/filepath" - "strings" "github.com/Azure/azure-sdk-for-go/tools/apidiff/exports" "github.com/Azure/azure-sdk-for-go/tools/apidiff/report" "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" - "github.com/Azure/azure-sdk-for-go/tools/generator/utils" ) // ChangelogContext describes all necessary data that would be needed in the processing of changelogs type ChangelogContext interface { SDKRoot() string - SDKCloneRoot() string - SpecRoot() string - SpecCommitHash() string - CodeGenVersion() string + RepoContent() map[string]exports.Content } // ChangelogProcessor processes the metadata and output changelog with the desired format @@ -35,30 +31,11 @@ func NewChangelogProcessorFromContext(ctx ChangelogContext) *ChangelogProcessor } } -// WithLocation adds the information of the metadata-output-folder -func (p *ChangelogProcessor) WithLocation(metadataLocation string) *ChangelogProcessor { - p.metadataLocation = metadataLocation - return p -} - -// WithReadme adds the information of the path of readme.md file. This path could be relative or absolute. -func (p *ChangelogProcessor) WithReadme(readme string) *ChangelogProcessor { - // make sure the readme here is a relative path to the root of spec - readme = utils.NormalizePath(readme) - root := utils.NormalizePath(p.ctx.SpecRoot()) - if filepath.IsAbs(readme) { - readme = strings.TrimPrefix(readme, root) - } - p.readme = readme - return p -} - // ChangelogResult describes the result of the generated changelog for one package type ChangelogResult struct { - PackageName string - PackagePath string - GenerationMetadata GenerationMetadata - Changelog model.Changelog + PackageName string + PackageFullPath string + Changelog model.Changelog } // ChangelogProcessError describes the errors during the processing @@ -93,14 +70,14 @@ func (b *changelogErrorBuilder) build() error { func (p *ChangelogProcessor) Process(metadataMap map[string]model.Metadata) ([]ChangelogResult, error) { builder := changelogErrorBuilder{} var results []ChangelogResult - for tag, metadata := range metadataMap { + for _, metadata := range metadataMap { outputFolder := filepath.Clean(metadata.PackagePath()) // format the package before getting the changelog if err := FormatPackage(outputFolder); err != nil { builder.add(err) continue } - result, err := p.GenerateChangelog(outputFolder, tag) + result, err := p.GenerateChangelog(outputFolder) if err != nil { builder.add(err) continue @@ -111,39 +88,42 @@ func (p *ChangelogProcessor) Process(metadataMap map[string]model.Metadata) ([]C } // GenerateChangelog generates a changelog for one package -func (p *ChangelogProcessor) GenerateChangelog(packagePath, tag string) (*ChangelogResult, error) { - // use the relative path to the sdk root as package name - packageName, err := filepath.Rel(p.ctx.SDKRoot(), packagePath) +func (p *ChangelogProcessor) GenerateChangelog(packageFullPath string) (*ChangelogResult, error) { + relativePackagePath, err := p.getRelativePackagePath(packageFullPath) if err != nil { return nil, err } - // normalize the package name - packageName = utils.NormalizePath(packageName) - lhs, err := getExportsForPackage(filepath.Join(p.ctx.SDKCloneRoot(), packageName)) - if err != nil { - return nil, fmt.Errorf("failed to get exports from package '%s' in the clone '%s': %+v", packageName, p.ctx.SDKCloneRoot(), err) - } - rhs, err := getExportsForPackage(packagePath) + lhs := p.getLHSContent(relativePackagePath) + rhs, err := getExportsForPackage(packageFullPath) if err != nil { - return nil, fmt.Errorf("failed to get exports from package '%s' in the sdk '%s': %+v", packageName, p.ctx.SDKRoot(), err) + return nil, fmt.Errorf("failed to get exports from package '%s' in the sdk '%s': %+v", relativePackagePath, p.ctx.SDKRoot(), err) } r, err := GetChangelogForPackage(lhs, rhs) if err != nil { - return nil, fmt.Errorf("failed to generate changelog for package '%s': %+v", packagePath, err) + return nil, fmt.Errorf("failed to generate changelog for package '%s': %+v", relativePackagePath, err) } return &ChangelogResult{ - PackageName: packageName, - PackagePath: packagePath, - GenerationMetadata: GenerationMetadata{ - CommitHash: p.ctx.SpecCommitHash(), - Readme: p.readme, - Tag: tag, - CodeGenVersion: p.ctx.CodeGenVersion(), - }, - Changelog: *r, + PackageName: relativePackagePath, + PackageFullPath: packageFullPath, + Changelog: *r, }, nil } +func (p *ChangelogProcessor) getRelativePackagePath(fullPath string) (string, error) { + relative, err := filepath.Rel(p.ctx.SDKRoot(), fullPath) + if err != nil { + return "", err + } + return utils.NormalizePath(relative), nil +} + +func (p *ChangelogProcessor) getLHSContent(relativePackagePath string) *exports.Content { + if v, ok := p.ctx.RepoContent()[relativePackagePath]; ok { + return &v + } + return nil +} + func getExportsForPackage(dir string) (*exports.Content, error) { // The function exports.Get does not handle the circumstance that the package does not exist // therefore we have to check if it exists and if not exit early to ensure we do not return an error diff --git a/tools/generator/autorest/generationMetadata.go b/tools/generator/autorest/generationMetadata.go index 5a2af7f84bae..8d7ee5ca19d9 100644 --- a/tools/generator/autorest/generationMetadata.go +++ b/tools/generator/autorest/generationMetadata.go @@ -82,7 +82,11 @@ func GetGenerationMetadata(pkg string) (*GenerationMetadata, error) { return nil, fmt.Errorf("cannot open file %s: %+v", changelogPath, err) } defer file.Close() - return Parse(file) + metadata, err := Parse(file) + if err != nil { + return nil, fmt.Errorf("cannot parse metadata in '%s': %+v", pkg, err) + } + return metadata, nil } func parseFirstLine(line string) (*GenerationMetadata, error) { diff --git a/tools/generator/cmd/clean.go b/tools/generator/cmd/clean.go index a41b53eb01dd..422e9a6473ec 100644 --- a/tools/generator/cmd/clean.go +++ b/tools/generator/cmd/clean.go @@ -4,6 +4,8 @@ import ( "fmt" "log" "os" + "path/filepath" + "strings" "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" ) @@ -29,11 +31,30 @@ func (ctx *cleanUpContext) clean() (readmePackageOutputMap, error) { return nil, fmt.Errorf("cannot remove package '%s': %+v", p.outputFolder, err) } removedPackages.add(readme, p) + + // recursively remove all its parent if this directory is empty after the deletion + if err := removeEmptyParents(filepath.Dir(p.outputFolder)); err != nil { + return nil, err + } } } return removedPackages, nil } +func removeEmptyParents(parent string) error { + fi, err := os.ReadDir(parent) + if err != nil { + return err + } + if len(fi) == 0 { + if err := os.RemoveAll(parent); err != nil { + return err + } + return removeEmptyParents(filepath.Dir(parent)) + } + return nil +} + func summaryReadmePackageOutputMap(root string) (readmePackageOutputMap, error) { // first we list all the go sdk track 1 packages m, err := autorest.CollectGenerationMetadata(root) @@ -42,7 +63,7 @@ func summaryReadmePackageOutputMap(root string) (readmePackageOutputMap, error) } result := readmePackageOutputMap{} for pkg, metadata := range m { - result.add(metadata.Readme, packageOutput{ + result.add(strings.TrimPrefix(metadata.Readme, "/"), packageOutput{ tag: metadata.Tag, outputFolder: pkg, }) diff --git a/tools/generator/cmd/metadata.go b/tools/generator/cmd/metadata.go index e8ddffab5971..07d15dd5f6cd 100644 --- a/tools/generator/cmd/metadata.go +++ b/tools/generator/cmd/metadata.go @@ -6,6 +6,8 @@ import ( "path/filepath" "strings" + "github.com/Azure/azure-sdk-for-go/tools/apidiff/exports" + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" "github.com/Azure/azure-sdk-for-go/tools/generator/validate" @@ -19,26 +21,16 @@ type changelogContext struct { codeGenVer string readme string removedPackages []packageOutput + + repoContent map[string]exports.Content } func (ctx changelogContext) SDKRoot() string { return ctx.sdkRoot } -func (ctx changelogContext) SDKCloneRoot() string { - return ctx.clnRoot -} - -func (ctx changelogContext) SpecRoot() string { - return ctx.specRoot -} - -func (ctx changelogContext) SpecCommitHash() string { - return ctx.commitHash -} - -func (ctx changelogContext) CodeGenVersion() string { - return ctx.codeGenVer +func (ctx changelogContext) RepoContent() map[string]exports.Content { + return ctx.repoContent } func (ctx changelogContext) process(metadataLocation string) ([]autorest.ChangelogResult, error) { @@ -53,7 +45,7 @@ func (ctx changelogContext) process(metadataLocation string) ([]autorest.Changel return nil, err } // generate the changelogs - p := autorest.NewChangelogProcessorFromContext(ctx).WithLocation(metadataLocation).WithReadme(ctx.readme) + p := autorest.NewChangelogProcessorFromContext(ctx) changelogResults, err := p.Process(metadataMap) if err != nil { return nil, err @@ -71,19 +63,28 @@ func (ctx changelogContext) process(metadataLocation string) ([]autorest.Changel // this package has been regenerated continue } - result, err := p.GenerateChangelog(rp.outputFolder, rp.tag) + result, err := p.GenerateChangelog(rp.outputFolder) if err != nil { return nil, err } removedResults = append(removedResults, *result) } changelogResults = append(changelogResults, removedResults...) - return changelogResults, nil + + // omit the packages not in services directory + var results []autorest.ChangelogResult + for _, result := range changelogResults { + if strings.HasPrefix(result.PackageName, "services/") { + results = append(results, result) + } + } + + return results, nil } func contains(array []autorest.ChangelogResult, item string) bool { for _, r := range array { - if r.PackagePath == item { + if r.PackageFullPath == item { return true } } @@ -91,10 +92,8 @@ func contains(array []autorest.ChangelogResult, item string) bool { } func writeChangelogFile(result autorest.ChangelogResult) error { - fileContent := fmt.Sprintf(`%s - -%s`, result.GenerationMetadata.String(), result.Changelog.ToMarkdown()) - changelogFile, err := os.Create(filepath.Join(result.PackagePath, autorest.ChangelogFilename)) + fileContent := result.Changelog.ToMarkdown() + changelogFile, err := os.Create(filepath.Join(result.PackageFullPath, autorest.ChangelogFilename)) if err != nil { return err } diff --git a/tools/generator/cmd/root.go b/tools/generator/cmd/root.go index 374a5e9601ba..64cf9cac4b82 100644 --- a/tools/generator/cmd/root.go +++ b/tools/generator/cmd/root.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/Azure/azure-sdk-for-go/tools/apidiff/exports" "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" "github.com/Azure/azure-sdk-for-go/tools/generator/pipeline" "github.com/Azure/azure-sdk-for-go/tools/generator/utils" @@ -66,17 +67,18 @@ func execute(inputPath, outputPath string, flags Flags) error { if err != nil { return err } - log.Printf("Backuping azure-sdk-for-go to temp directory...") - backupRoot, err := backupSDKRepository(cwd) - if err != nil { - return err - } - defer eraseBackup(backupRoot) - log.Printf("Finished backuping to '%s'", backupRoot) + + // we no longer need to back up the repo + //log.Printf("Backuping azure-sdk-for-go to temp directory...") + //backupRoot, err := backupSDKRepository(cwd) + //if err != nil { + // return err + //} + //defer eraseBackup(backupRoot) + //log.Printf("Finished backuping to '%s'", backupRoot) ctx := generateContext{ sdkRoot: utils.NormalizePath(cwd), - clnRoot: backupRoot, specRoot: input.SpecFolder, commitHash: input.HeadSha, optionPath: flags.OptionPath, @@ -118,6 +120,8 @@ type generateContext struct { specRoot string commitHash string optionPath string + + repoContent map[string]exports.Content } // TODO -- support dry run @@ -125,7 +129,11 @@ func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.Ge if input.DryRun { return nil, fmt.Errorf("dry run not supported yet") } - log.Printf("Reading options from file '%s'...", ctx.optionPath) + + log.Printf("Reading packages in azure-sdk-for-go...") + if err := ctx.readRepoContent(); err != nil { + return nil, err + } // now we summary all the metadata in sdk log.Printf("Cleaning up all the packages related with the following readme files: [%s]", strings.Join(input.RelatedReadmeMdFiles, ", ")) @@ -143,6 +151,7 @@ func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.Ge } log.Printf("The following %d package(s) have been cleaned up: [%s]", len(removedPackagePaths), strings.Join(removedPackagePaths, ", ")) + log.Printf("Reading options from file '%s'...", ctx.optionPath) optionFile, err := os.Open(ctx.optionPath) if err != nil { return nil, err @@ -178,6 +187,7 @@ func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.Ge codeGenVer: options.CodeGeneratorVersion(), readme: readme, removedPackages: removedPackages[readme], + repoContent: ctx.repoContent, } log.Printf("Processing metadata generated in readme '%s'...", readme) packages, err := m.process(g.metadataOutput) @@ -217,6 +227,32 @@ func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.Ge }, errorBuilder.build() } +func (ctx *generateContext) readRepoContent() error { + ctx.repoContent = make(map[string]exports.Content) + pkgs, err := track1.List(filepath.Join(ctx.sdkRoot, "services")) + if err != nil { + return fmt.Errorf("failed to list track 1 packages: %+v", err) + } + + for _, pkg := range pkgs { + relativePath, err := filepath.Rel(ctx.sdkRoot, pkg.FullPath()) + if err != nil { + return err + } + relativePath = utils.NormalizePath(relativePath) + if _, ok := ctx.repoContent[relativePath]; ok { + return fmt.Errorf("duplicate package: %s", pkg.Path()) + } + exp, err := exports.Get(pkg.FullPath()) + if err != nil { + return err + } + ctx.repoContent[relativePath] = exp + } + + return nil +} + type generateErrorBuilder struct { errors []error } diff --git a/tools/pkgchk/track1/packages.go b/tools/pkgchk/track1/packages.go index 8dbca8bb63d9..129de805d7d6 100644 --- a/tools/pkgchk/track1/packages.go +++ b/tools/pkgchk/track1/packages.go @@ -95,10 +95,10 @@ func List(root string) ([]Package, error) { return err } if len(packages) < 1 { - return fmt.Errorf("did not find any packages which is unexpected") + return fmt.Errorf("did not find any packages in '%s' which is unexpected", path) } if len(packages) > 1 { - return fmt.Errorf("found more than one package which is unexpected") + return fmt.Errorf("found more than one package in '%s' which is unexpected", path) } pkgName := "" for _, pkg := range packages { From 6b2955e204f0edd744deac5cadd6dc5bf11667b8 Mon Sep 17 00:00:00 2001 From: ArcturusZhang Date: Tue, 13 Apr 2021 18:23:59 +0800 Subject: [PATCH 03/13] using metadata instead of using changelog --- tools/generator/autorest/changelog.go | 11 ++- .../generator/autorest/generationMetadata.go | 86 +++---------------- .../autorest/generationMetadata_test.go | 39 --------- tools/generator/cmd/metadata.go | 13 ++- 4 files changed, 31 insertions(+), 118 deletions(-) delete mode 100644 tools/generator/autorest/generationMetadata_test.go diff --git a/tools/generator/autorest/changelog.go b/tools/generator/autorest/changelog.go index fc4c7c2d517a..a0a0460d571c 100644 --- a/tools/generator/autorest/changelog.go +++ b/tools/generator/autorest/changelog.go @@ -5,13 +5,13 @@ package autorest import ( "fmt" - "github.com/Azure/azure-sdk-for-go/tools/generator/utils" "os" "path/filepath" "github.com/Azure/azure-sdk-for-go/tools/apidiff/exports" "github.com/Azure/azure-sdk-for-go/tools/apidiff/report" "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" + "github.com/Azure/azure-sdk-for-go/tools/generator/utils" ) // ChangelogContext describes all necessary data that would be needed in the processing of changelogs @@ -163,3 +163,12 @@ func GetChangelogForPackage(lhs, rhs *exports.Content) (*model.Changelog, error) Modified: &p, }, nil } + +func (r ChangelogResult) Write() string { + return r.Changelog.ToMarkdown() +} + +const ( + // ChangelogFilename ... + ChangelogFilename = "CHANGELOG.md" +) diff --git a/tools/generator/autorest/generationMetadata.go b/tools/generator/autorest/generationMetadata.go index 3cc1b5f45b05..e9fdaddd6bf4 100644 --- a/tools/generator/autorest/generationMetadata.go +++ b/tools/generator/autorest/generationMetadata.go @@ -4,24 +4,16 @@ package autorest import ( + "encoding/json" "fmt" - "io" "io/ioutil" "log" "os" "path/filepath" - "regexp" - "strings" "github.com/Azure/azure-sdk-for-go/tools/pkgchk/track1" ) -// GeneratedFrom gives the information of the generation metadata, including the commit hash that this package is generated from, -// the readme path, and the tag -func GeneratedFrom(commitHash, readme, tag string) string { - return fmt.Sprintf("Generated from https://github.com/Azure/azure-rest-api-specs/tree/%s/%s tag: `%s`", commitHash, readme, tag) -} - // GenerationMetadata contains all the metadata that has been used when generating a track 1 package type GenerationMetadata struct { AutorestVersion string `json:"autorest,omitempty"` @@ -34,36 +26,6 @@ type GenerationMetadata struct { AdditionalProperties map[string]interface{} `json:"additional_properties,omitempty"` } -// String ... -func (m GenerationMetadata) String() string { - return fmt.Sprintf(`%s - -Code generator %s -`, GeneratedFrom(m.CommitHash, m.Readme, m.Tag), m.CodeGenVersion) -} - -// Parse parses the metadata info stored in a changelog with certain format into the GenerationMetadata struct -func Parse(reader io.Reader) (*GenerationMetadata, error) { - b, err := ioutil.ReadAll(reader) - if err != nil { - return nil, err - } - lines := strings.Split(string(b), "\n") - if len(lines) < 3 { - return nil, fmt.Errorf("expecting at least 3 lines from changelog, but only get %d line(s)", len(lines)) - } - // parse the first line to get readme, tag and commit hash - m, err := parseFirstLine(strings.TrimSpace(lines[0])) - if err != nil { - return nil, err - } - m.CodeGenVersion, err = parseThirdLine(strings.TrimSpace(lines[2])) - if err != nil { - return nil, err - } - return m, nil -} - // CollectGenerationMetadata iterates every track 1 go sdk package under root, and collect all the GenerationMetadata into a map // using relative path of the package as keys func CollectGenerationMetadata(root string) (map[string]GenerationMetadata, error) { @@ -86,50 +48,26 @@ func CollectGenerationMetadata(root string) (map[string]GenerationMetadata, erro // GetGenerationMetadata gets the GenerationMetadata in one specific package func GetGenerationMetadata(pkg track1.Package) (*GenerationMetadata, error) { - changelogPath := filepath.Join(pkg.FullPath(), ChangelogFilename) - if _, err := os.Stat(changelogPath); os.IsNotExist(err) { - log.Printf("Package '%s' does not have a changelog file", pkg.Path()) + metadataFilepath := filepath.Join(pkg.FullPath(), MetadataFilename) + if _, err := os.Stat(metadataFilepath); os.IsNotExist(err) { + log.Printf("Package '%s' does not have a metadata file", pkg.Path()) return nil, nil } - file, err := os.Open(changelogPath) - if err != nil { - return nil, fmt.Errorf("cannot open file %s: %+v", changelogPath, err) - } - defer file.Close() - metadata, err := Parse(file) + b, err := ioutil.ReadFile(metadataFilepath) if err != nil { - return nil, fmt.Errorf("cannot parse metadata in '%s': %+v", pkg, err) + return nil, fmt.Errorf("cannot read file %s: %+v", metadataFilepath, err) } - return metadata, nil -} -func parseFirstLine(line string) (*GenerationMetadata, error) { - matches := firstLineRegex.FindStringSubmatch(line) - if len(matches) < 4 { - return nil, fmt.Errorf("expecting 4 matches for line '%s', but only get the following matches: [%s]", line, strings.Join(matches, ", ")) + var metadata GenerationMetadata + if err := json.Unmarshal(b, &metadata); err != nil { + return nil, fmt.Errorf("cannot unmarshal metadata: %+v", err) } - return &GenerationMetadata{ - CommitHash: matches[1], - Readme: matches[2], - Tag: matches[3], - }, nil -} -func parseThirdLine(line string) (string, error) { - matches := thirdLineRegex.FindStringSubmatch(line) - if len(matches) < 2 { - return "", fmt.Errorf("expecting 2 matches for line '%s', but only get the following matches: [%s]", line, strings.Join(matches, ", ")) - } - return matches[1], nil + return &metadata, nil } -var ( - firstLineRegex = regexp.MustCompile("^Generated from https://github\\.com/Azure/azure-rest-api-specs/tree/([0-9a-f]+)/(.+) tag: `(.+)`$") - thirdLineRegex = regexp.MustCompile(`^Code generator (\S+)$`) -) - const ( - // ChangelogFilename ... - ChangelogFilename = "CHANGELOG.md" + // MetadataFilename + MetadataFilename = "_meta.json" ) diff --git a/tools/generator/autorest/generationMetadata_test.go b/tools/generator/autorest/generationMetadata_test.go deleted file mode 100644 index 2d6091ecfdd1..000000000000 --- a/tools/generator/autorest/generationMetadata_test.go +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. See License.txt in the project root for license information. - -package autorest_test - -import ( - "strings" - "testing" - - "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" -) - -func TestParse(t *testing.T) { - tests := []struct { - changelog string - expected autorest.GenerationMetadata - }{ - { - changelog: "Generated from https://github.com/Azure/azure-rest-api-specs/tree/3c764635e7d442b3e74caf593029fcd440b3ef82/specification/compute/resource-manager/readme.md tag: `package-2020-06-30`\n\nCode generator @microsoft.azure/autorest.go@2.1.168\n", - expected: autorest.GenerationMetadata{ - CommitHash: "3c764635e7d442b3e74caf593029fcd440b3ef82", - Readme: "specification/compute/resource-manager/readme.md", - Tag: "package-2020-06-30", - CodeGenVersion: "@microsoft.azure/autorest.go@2.1.168", - }, - }, - } - - for _, c := range tests { - reader := strings.NewReader(c.changelog) - m, err := autorest.Parse(reader) - if err != nil { - t.Fatalf("unexpected error: %+v", err) - } - if m.String() != c.expected.String() { - t.Fatalf("expect %+v, but got %+v", c.expected, *m) - } - } -} diff --git a/tools/generator/cmd/metadata.go b/tools/generator/cmd/metadata.go index 5c56e6ea4c5d..9fb4687ced6a 100644 --- a/tools/generator/cmd/metadata.go +++ b/tools/generator/cmd/metadata.go @@ -55,7 +55,7 @@ func (ctx changelogContext) process(metadataLocation string) ([]autorest.Changel } for _, result := range changelogResults { // we need to write the changelog file to the corresponding package here - if err := writeChangelogFile(result); err != nil { + if err := WriteChangelogFile(result); err != nil { return nil, err } } @@ -94,14 +94,19 @@ func contains(array []autorest.ChangelogResult, item string) bool { return false } -func writeChangelogFile(result autorest.ChangelogResult) error { - // TODO -- format of changelog - fileContent := result.Changelog.ToMarkdown() +func WriteChangelogFile(result autorest.ChangelogResult) error { + fileContent := result.Write() + changelogFile, err := os.Create(filepath.Join(result.PackageFullPath, autorest.ChangelogFilename)) if err != nil { return err } defer changelogFile.Close() + + fileContent = fmt.Sprintf(`# Unreleased content + +%s`, fileContent) + if _, err := changelogFile.WriteString(fileContent); err != nil { return err } From 8badab85f6dd7de2b2c6efde5baab3306fa14068 Mon Sep 17 00:00:00 2001 From: ArcturusZhang Date: Wed, 14 Apr 2021 08:49:11 +0800 Subject: [PATCH 04/13] update --- tools/generator/autorest/autorest.go | 4 +++ tools/generator/cmd/generation.go | 16 +++++++---- tools/generator/cmd/metadata.go | 41 +++++++++++++++++++++++----- tools/generator/cmd/root.go | 24 ++++++++++++---- 4 files changed, 68 insertions(+), 17 deletions(-) diff --git a/tools/generator/autorest/autorest.go b/tools/generator/autorest/autorest.go index 9bad2eb3de56..894c18dc88f9 100644 --- a/tools/generator/autorest/autorest.go +++ b/tools/generator/autorest/autorest.go @@ -70,6 +70,10 @@ func (g *Generator) buildCommand() { g.cmd = exec.Command("autorest", g.arguments...) } +func (g *Generator) Arguments() []string { + return g.arguments +} + // Start starts the generation func (g *Generator) Start() error { g.buildCommand() diff --git a/tools/generator/cmd/generation.go b/tools/generator/cmd/generation.go index 645dc2dc2968..076213beb760 100644 --- a/tools/generator/cmd/generation.go +++ b/tools/generator/cmd/generation.go @@ -17,13 +17,15 @@ type autorestContext struct { absReadme string metadataOutput string options model.Options + + g *autorest.Generator } func (c autorestContext) generate() error { - g := autorest.NewGeneratorFromOptions(c.options).WithReadme(c.absReadme).WithMetadataOutput(c.metadataOutput) + c.g = autorest.NewGeneratorFromOptions(c.options).WithReadme(c.absReadme).WithMetadataOutput(c.metadataOutput) - stdout, _ := g.StdoutPipe() - stderr, _ := g.StderrPipe() + stdout, _ := c.g.StdoutPipe() + stderr, _ := c.g.StderrPipe() defer stdout.Close() defer stderr.Close() outScanner := bufio.NewScanner(stdout) @@ -31,20 +33,24 @@ func (c autorestContext) generate() error { errScanner := bufio.NewScanner(stderr) errScanner.Split(bufio.ScanLines) - if err := g.Start(); err != nil { + if err := c.g.Start(); err != nil { return err } go printWithPrefixTo(outScanner, os.Stdout, "[AUTOREST] ") go printWithPrefixTo(errScanner, os.Stderr, "[AUTOREST] ") - if err := g.Wait(); err != nil { + if err := c.g.Wait(); err != nil { return err } return nil } +func (c autorestContext) autorestArguments() []string { + return c.g.Arguments() +} + func printWithPrefixTo(scanner *bufio.Scanner, writer io.Writer, prefix string) error { for scanner.Scan() { line := scanner.Text() diff --git a/tools/generator/cmd/metadata.go b/tools/generator/cmd/metadata.go index 9fb4687ced6a..ba122bb5143b 100644 --- a/tools/generator/cmd/metadata.go +++ b/tools/generator/cmd/metadata.go @@ -4,6 +4,7 @@ package cmd import ( + "encoding/json" "fmt" "os" "path/filepath" @@ -17,15 +18,16 @@ import ( ) type changelogContext struct { - sdkRoot string - clnRoot string - specRoot string - commitHash string - codeGenVer string - readme string + sdkRoot string + readme string + removedPackages []packageOutput repoContent map[string]exports.Content + + commonMetadata autorest.GenerationMetadata + + autorestArguments []string } func (ctx changelogContext) SDKRoot() string { @@ -58,7 +60,14 @@ func (ctx changelogContext) process(metadataLocation string) ([]autorest.Changel if err := WriteChangelogFile(result); err != nil { return nil, err } + // we need to write the generation metadata to the corresponding package here + metadata := ctx.commonMetadata + metadata.Tag = "" // TODO -- find how to populate tag here + if err := WriteGenerationMetadata(result.PackageFullPath, metadata); err != nil { + return nil, err + } } + // iterate over the removed packages, generate changelogs for them as well var removedResults []autorest.ChangelogResult for _, rp := range ctx.removedPackages { @@ -94,9 +103,27 @@ func contains(array []autorest.ChangelogResult, item string) bool { return false } +func WriteGenerationMetadata(path string, metadata autorest.GenerationMetadata) error { + b, err := json.MarshalIndent(metadata, "", " ") + if err != nil { + return fmt.Errorf("cannot marshal metadata: %+v", err) + } + + metadataFile, err := os.Create(filepath.Join(path, autorest.MetadataFilename)) + if err != nil { + return err + } + defer metadataFile.Close() + + if _, err := metadataFile.Write(b); err != nil { + return err + } + return nil +} + func WriteChangelogFile(result autorest.ChangelogResult) error { fileContent := result.Write() - + changelogFile, err := os.Create(filepath.Join(result.PackageFullPath, autorest.ChangelogFilename)) if err != nil { return err diff --git a/tools/generator/cmd/root.go b/tools/generator/cmd/root.go index 878530017389..ae3331a4e6f9 100644 --- a/tools/generator/cmd/root.go +++ b/tools/generator/cmd/root.go @@ -13,6 +13,8 @@ import ( "strings" "time" + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" + "github.com/Azure/azure-sdk-for-go/tools/apidiff/exports" "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" "github.com/Azure/azure-sdk-for-go/tools/generator/pipeline" @@ -184,13 +186,20 @@ func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.Ge } m := changelogContext{ sdkRoot: ctx.sdkRoot, - clnRoot: ctx.clnRoot, - specRoot: ctx.specRoot, - commitHash: ctx.commitHash, - codeGenVer: options.CodeGeneratorVersion(), readme: readme, removedPackages: removedPackages[readme], - repoContent: ctx.repoContent, + commonMetadata: autorest.GenerationMetadata{ + CommitHash: ctx.commitHash, + Readme: readme, + CodeGenVersion: options.CodeGeneratorVersion(), + RepositoryURL: "https://github.com/Azure/azure-rest-api-specs.git", + AutorestCommand: fmt.Sprintf("autorest %s", strings.Join(g.autorestArguments(), " ")), // TODO -- find a way to normalize the readme path and go-sdk-folder path + AdditionalProperties: map[string]interface{}{ + "additional_options": additionalOptions(g.autorestArguments()), + }, + }, + repoContent: ctx.repoContent, + autorestArguments: g.autorestArguments(), } log.Printf("Processing metadata generated in readme '%s'...", readme) packages, err := m.process(g.metadataOutput) @@ -329,3 +338,8 @@ func loadExceptions(exceptFile string) (map[string]bool, error) { return exceptions, nil } + +// additionalOptions removes flags that may change over scenarios +func additionalOptions(arguments []string) []string { + return arguments +} From bf0b1fb0feb3462e41508e2f2865c36a2243b294 Mon Sep 17 00:00:00 2001 From: ArcturusZhang Date: Wed, 14 Apr 2021 14:57:53 +0800 Subject: [PATCH 05/13] add new features for sdk automation --- tools/generator/autorest/changelog.go | 8 +- tools/generator/autorest/model/changelog.go | 101 +++++++++++++------- tools/generator/cmd/metadata.go | 11 +-- tools/generator/cmd/root.go | 10 +- tools/generator/pipeline/model.go | 6 +- 5 files changed, 90 insertions(+), 46 deletions(-) diff --git a/tools/generator/autorest/changelog.go b/tools/generator/autorest/changelog.go index a0a0460d571c..f05aa983c381 100644 --- a/tools/generator/autorest/changelog.go +++ b/tools/generator/autorest/changelog.go @@ -5,6 +5,7 @@ package autorest import ( "fmt" + "io" "os" "path/filepath" @@ -164,10 +165,15 @@ func GetChangelogForPackage(lhs, rhs *exports.Content) (*model.Changelog, error) }, nil } -func (r ChangelogResult) Write() string { +func (r ChangelogResult) ToMarkdown() string { return r.Changelog.ToMarkdown() } +func (r ChangelogResult) Write(writer io.Writer) error { + _, err := writer.Write([]byte(r.ToMarkdown())) + return err +} + const ( // ChangelogFilename ... ChangelogFilename = "CHANGELOG.md" diff --git a/tools/generator/autorest/model/changelog.go b/tools/generator/autorest/model/changelog.go index b9e78a77f64b..9052a0755522 100644 --- a/tools/generator/autorest/model/changelog.go +++ b/tools/generator/autorest/model/changelog.go @@ -54,23 +54,40 @@ func (c Changelog) ToCompactMarkdown() string { return writeChangelogForPackage(c.Modified) } +func (c Changelog) GetBreakingChangeItems() []string { + if c.Modified == nil { + return nil + } + return getBreakingChanges(c.Modified.BreakingChanges) +} + func writeChangelogForPackage(r *report.Package) string { if r == nil || r.IsEmpty() { return "No exported changes" } md := &markdown.Writer{} + // write breaking changes - writeBreakingChanges(r.BreakingChanges, md) + md.WriteHeader("Breaking Changes") + for _, item := range getBreakingChanges(r.BreakingChanges) { + md.WriteLine(item) + } + // write additional changes - writeNewContent(r.AdditiveChanges, md) + md.WriteHeader("New Content") + for _, item := range getNewContents(r.AdditiveChanges) { + md.WriteLine(item) + } - writeSummaries(r.BreakingChanges, r.AdditiveChanges, md) + md.EmptyLine() + summaries := getSummaries(r.BreakingChanges, r.AdditiveChanges) + md.WriteLine(summaries) return md.String() } -func writeSummaries(breaking *report.BreakingChanges, additive *delta.Content, md *markdown.Writer) { +func getSummaries(breaking *report.BreakingChanges, additive *delta.Content) string { bc := 0 if breaking != nil { bc = breaking.Count() @@ -79,19 +96,21 @@ func writeSummaries(breaking *report.BreakingChanges, additive *delta.Content, m if additive != nil { ac = additive.Count() } - md.WriteLine("") - md.WriteLine(fmt.Sprintf("Total %d breaking change(s), %d additive change(s).", bc, ac)) + + return fmt.Sprintf("Total %d breaking change(s), %d additive change(s).", bc, ac) } -func writeNewContent(c *delta.Content, md *markdown.Writer) { +func getNewContents(c *delta.Content) []string { if c == nil || c.IsEmpty() { - return + return nil } - md.WriteHeader("New Content") + + var items []string + if len(c.Consts) > 0 { for k := range c.Consts { line := fmt.Sprintf("- New const `%s`", k) - md.WriteLine(line) + items = append(items, line) } } if len(c.Funcs) > 0 { @@ -108,13 +127,13 @@ func writeNewContent(c *delta.Content, md *markdown.Writer) { } } line := fmt.Sprintf("- New function `%s(%s) %s`", k, params, returns) - md.WriteLine(line) + items = append(items, line) } } if len(c.CompleteStructs) > 0 { for _, v := range c.CompleteStructs { line := fmt.Sprintf("- New struct `%s`", v) - md.WriteLine(line) + items = append(items, line) } } if len(c.Structs) > 0 { @@ -122,34 +141,46 @@ func writeNewContent(c *delta.Content, md *markdown.Writer) { for s, f := range modified { for _, af := range f.AnonymousFields { line := fmt.Sprintf("- New anonymous field `%s` in struct `%s`", af, s) - md.WriteLine(line) + items = append(items, line) } for f := range f.Fields { line := fmt.Sprintf("- New field `%s` in struct `%s`", f, s) - md.WriteLine(line) + items = append(items, line) } } } + + return items } -func writeBreakingChanges(b *report.BreakingChanges, md *markdown.Writer) { +func getBreakingChanges(b *report.BreakingChanges) []string { if b == nil || b.IsEmpty() { - return + return nil } - md.WriteHeader("Breaking Changes") - writeSignatureChanges(b, md) - writeRemovedContent(b.Removed, md) + + var items []string + + // get signature changes + items = append(items, getSignatureChangeItems(b)...) + + // get removed content + items = append(items, getRemovedContent(b.Removed)...) + + return items } -func writeSignatureChanges(b *report.BreakingChanges, md *markdown.Writer) { +func getSignatureChangeItems(b *report.BreakingChanges) []string { if b.IsEmpty() { - return + return nil } + + var items []string + // write const changes if len(b.Consts) > 0 { for k, v := range b.Consts { line := fmt.Sprintf("- Const `%s` type has been changed from `%s` to `%s`", k, v.From, v.To) - md.WriteLine(line) + items = append(items, line) } // TODO -- sort? } @@ -158,11 +189,11 @@ func writeSignatureChanges(b *report.BreakingChanges, md *markdown.Writer) { for k, v := range b.Funcs { if v.Params != nil { line := fmt.Sprintf("- Function `%s` parameter(s) have been changed from `(%s)` to `(%s)`", k, v.Params.From, v.Params.To) - md.WriteLine(line) + items = append(items, line) } if v.Returns != nil { line := fmt.Sprintf("- Function `%s` return value(s) have been changed from `(%s)` to `(%s)`", k, v.Returns.From, v.Returns.To) - md.WriteLine(line) + items = append(items, line) } } } @@ -171,36 +202,40 @@ func writeSignatureChanges(b *report.BreakingChanges, md *markdown.Writer) { for k, v := range b.Structs { for f, d := range v.Fields { line := fmt.Sprintf("- Type of `%s.%s` has been changed from `%s` to `%s`", k, f, d.From, d.To) - md.WriteLine(line) + items = append(items, line) } } } // interfaces are skipped, which are identical to some of the functions + + return items } -func writeRemovedContent(removed *delta.Content, md *markdown.Writer) { +func getRemovedContent(removed *delta.Content) []string { if removed == nil { - return + return nil } + + var items []string // write constants if len(removed.Consts) > 0 { for k := range removed.Consts { line := fmt.Sprintf("- Const `%s` has been removed", k) - md.WriteLine(line) + items = append(items, line) } } // write functions if len(removed.Funcs) > 0 { for k := range removed.Funcs { line := fmt.Sprintf("- Function `%s` has been removed", k) - md.WriteLine(line) + items = append(items, line) } } // write complete struct removal if len(removed.CompleteStructs) > 0 { for _, v := range removed.CompleteStructs { line := fmt.Sprintf("- Struct `%s` has been removed", v) - md.WriteLine(line) + items = append(items, line) } } // write struct modification (some fields are removed) @@ -209,12 +244,14 @@ func writeRemovedContent(removed *delta.Content, md *markdown.Writer) { for s, f := range modified { for _, af := range f.AnonymousFields { line := fmt.Sprintf("- Field `%s` of struct `%s` has been removed", af, s) - md.WriteLine(line) + items = append(items, line) } for f := range f.Fields { line := fmt.Sprintf("- Field `%s` of struct `%s` has been removed", f, s) - md.WriteLine(line) + items = append(items, line) } } } + + return items } diff --git a/tools/generator/cmd/metadata.go b/tools/generator/cmd/metadata.go index ba122bb5143b..d8a5f9f09f07 100644 --- a/tools/generator/cmd/metadata.go +++ b/tools/generator/cmd/metadata.go @@ -122,22 +122,19 @@ func WriteGenerationMetadata(path string, metadata autorest.GenerationMetadata) } func WriteChangelogFile(result autorest.ChangelogResult) error { - fileContent := result.Write() - changelogFile, err := os.Create(filepath.Join(result.PackageFullPath, autorest.ChangelogFilename)) if err != nil { return err } defer changelogFile.Close() - fileContent = fmt.Sprintf(`# Unreleased content - -%s`, fileContent) + if _, err := changelogFile.WriteString(`# Unreleased Content - if _, err := changelogFile.WriteString(fileContent); err != nil { +`); err != nil { return err } - return nil + + return result.Write(changelogFile) } func (ctx changelogContext) validateMetadata(metadataMap map[string]model.Metadata) error { diff --git a/tools/generator/cmd/root.go b/tools/generator/cmd/root.go index ae3331a4e6f9..e6c2f822fe50 100644 --- a/tools/generator/cmd/root.go +++ b/tools/generator/cmd/root.go @@ -13,14 +13,14 @@ import ( "strings" "time" - "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" - "github.com/Azure/azure-sdk-for-go/tools/apidiff/exports" + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" "github.com/Azure/azure-sdk-for-go/tools/generator/pipeline" "github.com/Azure/azure-sdk-for-go/tools/generator/utils" "github.com/Azure/azure-sdk-for-go/tools/internal/ioext" "github.com/Azure/azure-sdk-for-go/tools/pkgchk/track1" + "github.com/Azure/azure-sdk-for-go/version" "github.com/spf13/cobra" ) @@ -215,12 +215,14 @@ func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.Ge content := p.Changelog.ToCompactMarkdown() breaking := p.Changelog.HasBreakingChanges() set.add(pipeline.PackageResult{ + Version: version.Number, PackageName: getPackageIdentifier(p.PackageName), Path: []string{p.PackageName}, ReadmeMd: []string{readme}, Changelog: &pipeline.Changelog{ - Content: &content, - HasBreakingChange: &breaking, + Content: &content, + HasBreakingChange: &breaking, + BreakingChangeItems: p.Changelog.GetBreakingChangeItems(), }, }) } diff --git a/tools/generator/pipeline/model.go b/tools/generator/pipeline/model.go index bb2af1585af2..2f6d44da13eb 100644 --- a/tools/generator/pipeline/model.go +++ b/tools/generator/pipeline/model.go @@ -74,6 +74,7 @@ func (o GenerateOutput) WriteTo(writer io.Writer) (int64, error) { // PackageResult ... type PackageResult struct { + Version string `json:"version,omitempty"` PackageName string `json:"packageName,omitempty"` Path []string `json:"path,omitempty"` ReadmeMd []string `json:"readmeMd,omitempty"` @@ -84,8 +85,9 @@ type PackageResult struct { // Changelog ... type Changelog struct { - Content *string `json:"content,omitempty"` - HasBreakingChange *bool `json:"hasBreakingChange,omitempty"` + Content *string `json:"content,omitempty"` + HasBreakingChange *bool `json:"hasBreakingChange,omitempty"` + BreakingChangeItems []string `json:"breakingChangeItems,omitempty"` } // InstallInstructionScriptOutput ... From fff96f98b955f46d50acb061d7491551ac65c2c7 Mon Sep 17 00:00:00 2001 From: ArcturusZhang Date: Wed, 14 Apr 2021 17:22:45 +0800 Subject: [PATCH 06/13] update for metadata --- tools/generator/autorest/changelog.go | 8 ++-- .../generator/autorest/generationMetadata.go | 22 ++++++++++- tools/generator/cmd/clean.go | 6 +-- tools/generator/cmd/generation.go | 19 +++------- tools/generator/cmd/metadata.go | 37 ++++++++++++++++++- tools/generator/cmd/root.go | 18 ++------- 6 files changed, 73 insertions(+), 37 deletions(-) diff --git a/tools/generator/autorest/changelog.go b/tools/generator/autorest/changelog.go index f05aa983c381..419893b3d53e 100644 --- a/tools/generator/autorest/changelog.go +++ b/tools/generator/autorest/changelog.go @@ -37,6 +37,7 @@ func NewChangelogProcessorFromContext(ctx ChangelogContext) *ChangelogProcessor // ChangelogResult describes the result of the generated changelog for one package type ChangelogResult struct { + Tag string PackageName string PackageFullPath string Changelog model.Changelog @@ -74,14 +75,14 @@ func (b *changelogErrorBuilder) build() error { func (p *ChangelogProcessor) Process(metadataMap map[string]model.Metadata) ([]ChangelogResult, error) { builder := changelogErrorBuilder{} var results []ChangelogResult - for _, metadata := range metadataMap { + for tag, metadata := range metadataMap { outputFolder := filepath.Clean(metadata.PackagePath()) // format the package before getting the changelog if err := FormatPackage(outputFolder); err != nil { builder.add(err) continue } - result, err := p.GenerateChangelog(outputFolder) + result, err := p.GenerateChangelog(outputFolder, tag) if err != nil { builder.add(err) continue @@ -92,7 +93,7 @@ func (p *ChangelogProcessor) Process(metadataMap map[string]model.Metadata) ([]C } // GenerateChangelog generates a changelog for one package -func (p *ChangelogProcessor) GenerateChangelog(packageFullPath string) (*ChangelogResult, error) { +func (p *ChangelogProcessor) GenerateChangelog(packageFullPath, tag string) (*ChangelogResult, error) { relativePackagePath, err := p.getRelativePackagePath(packageFullPath) if err != nil { return nil, err @@ -107,6 +108,7 @@ func (p *ChangelogProcessor) GenerateChangelog(packageFullPath string) (*Changel return nil, fmt.Errorf("failed to generate changelog for package '%s': %+v", relativePackagePath, err) } return &ChangelogResult{ + Tag: tag, PackageName: relativePackagePath, PackageFullPath: packageFullPath, Changelog: *r, diff --git a/tools/generator/autorest/generationMetadata.go b/tools/generator/autorest/generationMetadata.go index b465dc111363..b18d9c9e4566 100644 --- a/tools/generator/autorest/generationMetadata.go +++ b/tools/generator/autorest/generationMetadata.go @@ -10,22 +10,36 @@ import ( "log" "os" "path/filepath" + "strings" "github.com/Azure/azure-sdk-for-go/tools/pkgchk/track1" ) // GenerationMetadata contains all the metadata that has been used when generating a track 1 package type GenerationMetadata struct { + // AutorestVersion is the version of autorest.core AutorestVersion string `json:"autorest,omitempty"` + // CommitHash is the commit hash of azure-rest-api-specs from which this SDK package is generated CommitHash string `json:"commit,omitempty"` + // Readme is the normalized path of the readme file from which this SDK package is generated. It should be in this pattern: /_/azure-rest-api-specs/{relative_path} Readme string `json:"readme,omitempty"` + // Tag is the tag from which this SDK package is generated Tag string `json:"tag,omitempty"` + // CodeGenVersion is the version of autorest.go using when this package is generated CodeGenVersion string `json:"use,omitempty"` + // RepositoryURL is the URL of the azure-rest-api-specs. This should always be a constant "https://github.com/Azure/azure-rest-api-specs.git" RepositoryURL string `json:"repository_url,omitempty"` + // AutorestCommand is the full command that generates this package AutorestCommand string `json:"autorest_command,omitempty"` + // AdditionalProperties is a map of addition information in this metadata AdditionalProperties map[string]interface{} `json:"additional_properties,omitempty"` } +// RelativeReadme returns the relative readme path +func (meta *GenerationMetadata) RelativeReadme() string { + return strings.TrimPrefix(meta.Readme, NormalizedSpecRoot) +} + // CollectGenerationMetadata iterates every track 1 go sdk package under root, and collect all the GenerationMetadata into a map // using relative path of the package as keys func CollectGenerationMetadata(root string) (map[string]GenerationMetadata, error) { @@ -68,6 +82,12 @@ func GetGenerationMetadata(pkg track1.Package) (*GenerationMetadata, error) { } const ( - // MetadataFilename + // NormalizedSpecRoot this is the prefix for readme + NormalizedSpecRoot = "/_/azure-rest-api-specs/" + + // NormalizedSDKRoot this is the prefix for readme + NormalizedSDKRoot = "/_/azure-sdk-for-go/" + + // MetadataFilename ... MetadataFilename = "_meta.json" ) diff --git a/tools/generator/cmd/clean.go b/tools/generator/cmd/clean.go index 8affeb729107..9bfad65747af 100644 --- a/tools/generator/cmd/clean.go +++ b/tools/generator/cmd/clean.go @@ -5,12 +5,10 @@ package cmd import ( "fmt" + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" "log" "os" "path/filepath" - "strings" - - "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" ) type cleanUpContext struct { @@ -66,7 +64,7 @@ func summaryReadmePackageOutputMap(root string) (readmePackageOutputMap, error) } result := readmePackageOutputMap{} for pkg, metadata := range m { - result.add(strings.TrimPrefix(metadata.Readme, "/"), packageOutput{ + result.add(metadata.RelativeReadme(), packageOutput{ tag: metadata.Tag, outputFolder: pkg, }) diff --git a/tools/generator/cmd/generation.go b/tools/generator/cmd/generation.go index 076213beb760..c9691ef3a1fe 100644 --- a/tools/generator/cmd/generation.go +++ b/tools/generator/cmd/generation.go @@ -10,22 +10,15 @@ import ( "os" "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" - "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" ) type autorestContext struct { - absReadme string - metadataOutput string - options model.Options - - g *autorest.Generator + generator *autorest.Generator } func (c autorestContext) generate() error { - c.g = autorest.NewGeneratorFromOptions(c.options).WithReadme(c.absReadme).WithMetadataOutput(c.metadataOutput) - - stdout, _ := c.g.StdoutPipe() - stderr, _ := c.g.StderrPipe() + stdout, _ := c.generator.StdoutPipe() + stderr, _ := c.generator.StderrPipe() defer stdout.Close() defer stderr.Close() outScanner := bufio.NewScanner(stdout) @@ -33,14 +26,14 @@ func (c autorestContext) generate() error { errScanner := bufio.NewScanner(stderr) errScanner.Split(bufio.ScanLines) - if err := c.g.Start(); err != nil { + if err := c.generator.Start(); err != nil { return err } go printWithPrefixTo(outScanner, os.Stdout, "[AUTOREST] ") go printWithPrefixTo(errScanner, os.Stderr, "[AUTOREST] ") - if err := c.g.Wait(); err != nil { + if err := c.generator.Wait(); err != nil { return err } @@ -48,7 +41,7 @@ func (c autorestContext) generate() error { } func (c autorestContext) autorestArguments() []string { - return c.g.Arguments() + return c.generator.Arguments() } func printWithPrefixTo(scanner *bufio.Scanner, writer io.Writer, prefix string) error { diff --git a/tools/generator/cmd/metadata.go b/tools/generator/cmd/metadata.go index d8a5f9f09f07..a6a9f5ae3078 100644 --- a/tools/generator/cmd/metadata.go +++ b/tools/generator/cmd/metadata.go @@ -62,7 +62,13 @@ func (ctx changelogContext) process(metadataLocation string) ([]autorest.Changel } // we need to write the generation metadata to the corresponding package here metadata := ctx.commonMetadata - metadata.Tag = "" // TODO -- find how to populate tag here + metadata.Tag = result.Tag + options := additionalOptions(ctx.autorestArguments) + metadata.AdditionalProperties = map[string]interface{}{ + "additional_options": strings.Join(options, " "), + } + metadata.AutorestCommand = fmt.Sprintf("autorest --tag=%s --go-sdk-folder=/_/azure-sdk-for-go %s /_/azure-rest-api-specs/%s", + result.Tag, strings.Join(options, " "), utils.NormalizePath(ctx.readme)) if err := WriteGenerationMetadata(result.PackageFullPath, metadata); err != nil { return nil, err } @@ -75,7 +81,7 @@ func (ctx changelogContext) process(metadataLocation string) ([]autorest.Changel // this package has been regenerated continue } - result, err := p.GenerateChangelog(rp.outputFolder) + result, err := p.GenerateChangelog(rp.outputFolder, "") if err != nil { return nil, err } @@ -183,3 +189,30 @@ func (b *validationErrorBuilder) build() error { } return fmt.Errorf("validation failed in readme '%s' with %d error(s): \n%s", b.readme, len(b.errors), strings.Join(messages, "\n")) } + +// additionalOptions removes flags that may change over scenarios +func additionalOptions(arguments []string) []string { + var transformed []string + for _, argument := range arguments { + // remove the readme path + if !strings.HasPrefix(argument, "--") { + continue + } + // remove the go-sdk-folder + if strings.HasPrefix(argument, "--go-sdk-folder") { + continue + } + if strings.HasPrefix(argument, "--use") { + continue + } + if strings.HasPrefix(argument, "--metadata-output-folder") { + continue + } + // remove multiapi + if argument == "--multiapi" { + continue + } + transformed = append(transformed, argument) + } + return transformed +} diff --git a/tools/generator/cmd/root.go b/tools/generator/cmd/root.go index e6c2f822fe50..27e4de840a0c 100644 --- a/tools/generator/cmd/root.go +++ b/tools/generator/cmd/root.go @@ -174,11 +174,10 @@ func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.Ge for _, readme := range input.RelatedReadmeMdFiles { log.Printf("Processing readme '%s'...", readme) absReadme := filepath.Join(input.SpecFolder, readme) + metadataOutput := filepath.Dir(absReadme) // generate code g := autorestContext{ - absReadme: absReadme, - metadataOutput: filepath.Dir(absReadme), - options: options, + generator: autorest.NewGeneratorFromOptions(options).WithReadme(absReadme).WithMetadataOutput(metadataOutput), } if err := g.generate(); err != nil { errorBuilder.add(fmt.Errorf("cannot generate readme '%s': %+v", readme, err)) @@ -190,19 +189,15 @@ func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.Ge removedPackages: removedPackages[readme], commonMetadata: autorest.GenerationMetadata{ CommitHash: ctx.commitHash, - Readme: readme, + Readme: autorest.NormalizedSpecRoot + utils.NormalizePath(readme), CodeGenVersion: options.CodeGeneratorVersion(), RepositoryURL: "https://github.com/Azure/azure-rest-api-specs.git", - AutorestCommand: fmt.Sprintf("autorest %s", strings.Join(g.autorestArguments(), " ")), // TODO -- find a way to normalize the readme path and go-sdk-folder path - AdditionalProperties: map[string]interface{}{ - "additional_options": additionalOptions(g.autorestArguments()), - }, }, repoContent: ctx.repoContent, autorestArguments: g.autorestArguments(), } log.Printf("Processing metadata generated in readme '%s'...", readme) - packages, err := m.process(g.metadataOutput) + packages, err := m.process(metadataOutput) if err != nil { errorBuilder.add(fmt.Errorf("cannot process metadata for readme '%s': %+v", readme, err)) continue @@ -340,8 +335,3 @@ func loadExceptions(exceptFile string) (map[string]bool, error) { return exceptions, nil } - -// additionalOptions removes flags that may change over scenarios -func additionalOptions(arguments []string) []string { - return arguments -} From bd9e3ab5db3fcd75e16f92cf8221813277d230a8 Mon Sep 17 00:00:00 2001 From: ArcturusZhang Date: Wed, 14 Apr 2021 18:54:24 +0800 Subject: [PATCH 07/13] remove call from go 1.16 --- tools/generator/cmd/clean.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/generator/cmd/clean.go b/tools/generator/cmd/clean.go index 9bfad65747af..be9d80dd26f0 100644 --- a/tools/generator/cmd/clean.go +++ b/tools/generator/cmd/clean.go @@ -5,10 +5,12 @@ package cmd import ( "fmt" - "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" + "io/ioutil" "log" "os" "path/filepath" + + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest" ) type cleanUpContext struct { @@ -43,7 +45,7 @@ func (ctx *cleanUpContext) clean() (readmePackageOutputMap, error) { } func removeEmptyParents(parent string) error { - fi, err := os.ReadDir(parent) + fi, err := ioutil.ReadDir(parent) if err != nil { return err } From 35218926bd518686e9bc4b9f1c066fae6003bafb Mon Sep 17 00:00:00 2001 From: arcturusZhang Date: Thu, 15 Apr 2021 13:44:44 +0800 Subject: [PATCH 08/13] fix a minor flau --- tools/generator/cmd/metadata.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/generator/cmd/metadata.go b/tools/generator/cmd/metadata.go index a6a9f5ae3078..2243129cae71 100644 --- a/tools/generator/cmd/metadata.go +++ b/tools/generator/cmd/metadata.go @@ -81,7 +81,7 @@ func (ctx changelogContext) process(metadataLocation string) ([]autorest.Changel // this package has been regenerated continue } - result, err := p.GenerateChangelog(rp.outputFolder, "") + result, err := p.GenerateChangelog(rp.outputFolder, rp.tag) if err != nil { return nil, err } From 93f6a3b8ad6f1f3106ee6f0ae08d7f11ceda1dad Mon Sep 17 00:00:00 2001 From: ArcturusZhang Date: Fri, 16 Apr 2021 11:08:08 +0800 Subject: [PATCH 09/13] make the generationmetadata more strong typed --- .../generator/autorest/generationMetadata.go | 20 +++++++++++-------- tools/generator/cmd/metadata.go | 4 ++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/tools/generator/autorest/generationMetadata.go b/tools/generator/autorest/generationMetadata.go index b18d9c9e4566..7c2767a2b7f4 100644 --- a/tools/generator/autorest/generationMetadata.go +++ b/tools/generator/autorest/generationMetadata.go @@ -18,21 +18,25 @@ import ( // GenerationMetadata contains all the metadata that has been used when generating a track 1 package type GenerationMetadata struct { // AutorestVersion is the version of autorest.core - AutorestVersion string `json:"autorest,omitempty"` + AutorestVersion string `json:"autorest,omitempty"` // CommitHash is the commit hash of azure-rest-api-specs from which this SDK package is generated - CommitHash string `json:"commit,omitempty"` + CommitHash string `json:"commit,omitempty"` // Readme is the normalized path of the readme file from which this SDK package is generated. It should be in this pattern: /_/azure-rest-api-specs/{relative_path} - Readme string `json:"readme,omitempty"` + Readme string `json:"readme,omitempty"` // Tag is the tag from which this SDK package is generated - Tag string `json:"tag,omitempty"` + Tag string `json:"tag,omitempty"` // CodeGenVersion is the version of autorest.go using when this package is generated - CodeGenVersion string `json:"use,omitempty"` + CodeGenVersion string `json:"use,omitempty"` // RepositoryURL is the URL of the azure-rest-api-specs. This should always be a constant "https://github.com/Azure/azure-rest-api-specs.git" - RepositoryURL string `json:"repository_url,omitempty"` + RepositoryURL string `json:"repository_url,omitempty"` // AutorestCommand is the full command that generates this package - AutorestCommand string `json:"autorest_command,omitempty"` + AutorestCommand string `json:"autorest_command,omitempty"` // AdditionalProperties is a map of addition information in this metadata - AdditionalProperties map[string]interface{} `json:"additional_properties,omitempty"` + AdditionalProperties GenerationMetadataAdditionalProperties `json:"additional_properties,omitempty"` +} + +type GenerationMetadataAdditionalProperties struct { + AdditionalOptions string `json:"additional_options,omitempty"` } // RelativeReadme returns the relative readme path diff --git a/tools/generator/cmd/metadata.go b/tools/generator/cmd/metadata.go index 2243129cae71..895e3b51d25c 100644 --- a/tools/generator/cmd/metadata.go +++ b/tools/generator/cmd/metadata.go @@ -64,8 +64,8 @@ func (ctx changelogContext) process(metadataLocation string) ([]autorest.Changel metadata := ctx.commonMetadata metadata.Tag = result.Tag options := additionalOptions(ctx.autorestArguments) - metadata.AdditionalProperties = map[string]interface{}{ - "additional_options": strings.Join(options, " "), + metadata.AdditionalProperties = autorest.GenerationMetadataAdditionalProperties{ + AdditionalOptions: strings.Join(options, " "), } metadata.AutorestCommand = fmt.Sprintf("autorest --tag=%s --go-sdk-folder=/_/azure-sdk-for-go %s /_/azure-rest-api-specs/%s", result.Tag, strings.Join(options, " "), utils.NormalizePath(ctx.readme)) From 1d7aa97f617692d149f0059a38fb845251a9f83b Mon Sep 17 00:00:00 2001 From: arcturusZhang Date: Fri, 16 Apr 2021 17:25:41 +0800 Subject: [PATCH 10/13] refined options --- tools/generator/autorest/autorest.go | 21 +++-- .../generator/autorest/generationMetadata.go | 33 +++++++ tools/generator/autorest/model/option.go | 92 ++++++++++++++++++- tools/generator/autorest/model/option_test.go | 38 ++++++++ tools/generator/autorest/model/options.go | 46 +++++++--- .../generator/autorest/model/options_test.go | 51 ++++++++++ tools/generator/cmd/generation.go | 3 +- tools/generator/cmd/metadata.go | 35 +------ 8 files changed, 267 insertions(+), 52 deletions(-) create mode 100644 tools/generator/autorest/model/option_test.go create mode 100644 tools/generator/autorest/model/options_test.go diff --git a/tools/generator/autorest/autorest.go b/tools/generator/autorest/autorest.go index 894c18dc88f9..270597cc0506 100644 --- a/tools/generator/autorest/autorest.go +++ b/tools/generator/autorest/autorest.go @@ -7,13 +7,14 @@ import ( "fmt" "io" "os/exec" + "strings" "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" ) // Generator collects all the related context of an autorest generation type Generator struct { - arguments []string + arguments []model.Option cmd *exec.Cmd } @@ -26,7 +27,7 @@ func NewGeneratorFromOptions(o model.Options) *Generator { // WithOption appends an model.Option to the argument list of the autorest generation func (g *Generator) WithOption(option model.Option) *Generator { - g.arguments = append(g.arguments, option.Format()) + g.arguments = append(g.arguments, option) return g } @@ -67,10 +68,14 @@ func (g *Generator) buildCommand() { if g.cmd != nil { return } - g.cmd = exec.Command("autorest", g.arguments...) + arguments := make([]string, len(g.arguments)) + for i, o := range g.arguments { + arguments[i] = o.Format() + } + g.cmd = exec.Command("autorest", arguments...) } -func (g *Generator) Arguments() []string { +func (g *Generator) Arguments() []model.Option { return g.arguments } @@ -124,11 +129,15 @@ func (g *Generator) StderrPipe() (io.ReadCloser, error) { // GenerateError ... type GenerateError struct { - Arguments []string + Arguments []model.Option Message string } // Error ... func (e *GenerateError) Error() string { - return fmt.Sprintf("autorest error with arguments '%s': \n%s", e.Arguments, e.Message) + arguments := make([]string, len(e.Arguments)) + for i, o := range e.Arguments { + arguments[i] = o.Format() + } + return fmt.Sprintf("autorest error with arguments '%s': \n%s", strings.Join(arguments, ", "), e.Message) } diff --git a/tools/generator/autorest/generationMetadata.go b/tools/generator/autorest/generationMetadata.go index 7c2767a2b7f4..70948ba933d7 100644 --- a/tools/generator/autorest/generationMetadata.go +++ b/tools/generator/autorest/generationMetadata.go @@ -6,6 +6,7 @@ package autorest import ( "encoding/json" "fmt" + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" "io/ioutil" "log" "os" @@ -85,6 +86,38 @@ func GetGenerationMetadata(pkg track1.Package) (*GenerationMetadata, error) { return &metadata, nil } +// AdditionalOptions removes flags that may change over scenarios +func AdditionalOptions(arguments []model.Option) []model.Option { + var transformed []model.Option + for _, argument := range arguments { + switch o := argument.(type) { + case model.ArgumentOption: // omit the readme path argument + continue + case model.FlagOption: + if o.Flag() == "multiapi" { // omit the multiapi flag or use + continue + } + case model.KeyValueOption: + // omit go-sdk-folder, use, tag and metadata-output-folder + if o.Key() == "go-sdk-folder" || o.Key() == "use" || o.Key() == "tag" || o.Key() == "metadata-output-folder" { + continue + } + } + transformed = append(transformed, argument) + } + return transformed +} + +// AdditionalOptionsToString removes flags that may change over scenarios and cast them to strings +func AdditionalOptionsToString(arguments []model.Option) []string { + transformed := AdditionalOptions(arguments) + result := make([]string, len(transformed)) + for i, o := range transformed { + result[i] = o.Format() + } + return result +} + const ( // NormalizedSpecRoot this is the prefix for readme NormalizedSpecRoot = "/_/azure-rest-api-specs/" diff --git a/tools/generator/autorest/model/option.go b/tools/generator/autorest/model/option.go index eb80cd171ff1..d5588638fa9a 100644 --- a/tools/generator/autorest/model/option.go +++ b/tools/generator/autorest/model/option.go @@ -3,42 +3,132 @@ package model -import "fmt" +import ( + "fmt" + "regexp" + "strings" +) + +type OptionType string + +const ( + Argument OptionType = "Argument" + Flag OptionType = "Flag" + KeyValue OptionType = "KeyValue" +) // Option describes an option of a autorest command line type Option interface { + Type() OptionType // Format returns the actual option in string Format() string } +type ArgumentOption interface { + Option + Argument() string +} + +type FlagOption interface { + Option + Flag() string +} + +type KeyValueOption interface { + Option + Key() string + Value() string +} + +// NewOption returns an Option from a formatted string. We should hold this result using this function: input == result.Format() +func NewOption(input string) (Option, error) { + if strings.HasPrefix(input, "--") { + // this option is either a flag or a key value pair option + return parseFlagOrKeyValuePair(strings.TrimPrefix(input, "--")) + } + // this should be an argument + return argument{value: input}, nil +} + +var flagRegex = regexp.MustCompile(`^[a-zA-Z]`) + +func parseFlagOrKeyValuePair(input string) (Option, error) { + if !flagRegex.MatchString(input) { + return nil, fmt.Errorf("cannot parse flag '%s', a flag or option should only start with letters", input) + } + segments := strings.Split(input, "=") + if len(segments) <= 1 { + // this should be a flag + return flagOption{value: input}, nil + } + return keyValueOption{ + key: segments[0], + value: strings.Join(segments[1:], "="), + }, nil +} + type argument struct { value string } +func (f argument) Type() OptionType { + return Argument +} + // Format ... func (f argument) Format() string { return f.value } +func (f argument) Argument() string { + return f.value +} + +var _ ArgumentOption = (*argument)(nil) + type flagOption struct { value string } +func (f flagOption) Type() OptionType { + return Flag +} + // Format ... func (f flagOption) Format() string { return fmt.Sprintf("--%s", f.value) } +func (f flagOption) Flag() string { + return f.value +} + +var _ FlagOption = (*flagOption)(nil) + type keyValueOption struct { key string value string } +func (f keyValueOption) Type() OptionType { + return KeyValue +} + // Format ... func (f keyValueOption) Format() string { return fmt.Sprintf("--%s=%s", f.key, f.value) } +func (f keyValueOption) Key() string { + return f.key +} + +func (f keyValueOption) Value() string { + return f.value +} + +var _ KeyValueOption = (*keyValueOption)(nil) + // NewArgument returns a new argument option (without "--") func NewArgument(value string) Option { return argument{ diff --git a/tools/generator/autorest/model/option_test.go b/tools/generator/autorest/model/option_test.go new file mode 100644 index 000000000000..b9b26e912c49 --- /dev/null +++ b/tools/generator/autorest/model/option_test.go @@ -0,0 +1,38 @@ +package model_test + +import ( + "reflect" + "testing" + + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" +) + +func TestNewOption(t *testing.T) { + testcase := []struct { + input string + expected model.Option + }{ + { + input: "specification/compute/resource-manager/readme.md", + expected: model.NewArgument("specification/compute/resource-manager/readme.md"), + }, + { + input: "--multiapi", + expected: model.NewFlagOption("multiapi"), + }, + { + input: "--tag=package-2020-01-01", + expected: model.NewKeyValueOption("tag", "package-2020-01-01"), + }, + } + + for _, c := range testcase { + o, err := model.NewOption(c.input) + if err != nil { + t.Fatalf("unexpected error: %+v", err) + } + if !reflect.DeepEqual(o, c.expected) { + t.Fatalf("expecting %+v, but got %+v", c.expected, o) + } + } +} diff --git a/tools/generator/autorest/model/options.go b/tools/generator/autorest/model/options.go index 2db54858ca6c..836c85c22913 100644 --- a/tools/generator/autorest/model/options.go +++ b/tools/generator/autorest/model/options.go @@ -12,31 +12,56 @@ import ( // Options ... type Options interface { // Arguments returns the argument defined in this options - Arguments() []string + Arguments() []Option // CodeGeneratorVersion returns the code generator version defined in this options CodeGeneratorVersion() string } -type localOptions struct { +type generateOptions struct { AutorestArguments []string `json:"autorestArguments"` } +type localOptions struct { + arguments []Option + codeGenVersion string +} + // NewOptionsFrom returns a new instance of Options from the given io.Reader func NewOptionsFrom(reader io.Reader) (Options, error) { b, err := ioutil.ReadAll(reader) if err != nil { return nil, err } - var result localOptions - if err := json.Unmarshal(b, &result); err != nil { + var raw generateOptions + if err := json.Unmarshal(b, &raw); err != nil { return nil, err } - return &result, nil + var options []Option + var codeGenVersion string + for _, r := range raw.AutorestArguments { + o, err := NewOption(r) + if err != nil { + return nil, err + } + options = append(options, o) + if v, ok := o.(KeyValueOption); ok && v.Key() == "use" { + codeGenVersion = v.Value() + } + } + return &localOptions{ + arguments: options, + codeGenVersion: codeGenVersion, + }, nil } -// Argument ... -func (o localOptions) Arguments() []string { - return o.AutorestArguments +// Arguments ... +func (o localOptions) Arguments() []Option { + return o.arguments +} + +// CodeGeneratorVersion ... +func (o localOptions) CodeGeneratorVersion() string { + return o.codeGenVersion } // String ... @@ -44,8 +69,3 @@ func (o localOptions) String() string { b, _ := json.MarshalIndent(o, "", " ") return string(b) } - -// CodeGeneratorVersion ... -func (o localOptions) CodeGeneratorVersion() string { - return "" -} diff --git a/tools/generator/autorest/model/options_test.go b/tools/generator/autorest/model/options_test.go new file mode 100644 index 000000000000..bd6dbadca0eb --- /dev/null +++ b/tools/generator/autorest/model/options_test.go @@ -0,0 +1,51 @@ +package model_test + +import ( + "reflect" + "strings" + "testing" + + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" +) + +func TestNewOptionsFrom(t *testing.T) { + testdata := []struct { + input string + expected []model.Option + }{ + { + input: `{ + "autorestArguments": [ + "--use=@microsoft.azure/autorest.go@2.1.178", + "--go", + "--verbose", + "--go-sdk-folder=.", + "--multiapi", + "--use-onever", + "--version=V2", + "--go.license-header=MICROSOFT_MIT_NO_VERSION" + ] +}`, + expected: []model.Option{ + model.NewKeyValueOption("use", "@microsoft.azure/autorest.go@2.1.178"), + model.NewFlagOption("go"), + model.NewFlagOption("verbose"), + model.NewKeyValueOption("go-sdk-folder", "."), + model.NewFlagOption("multiapi"), + model.NewFlagOption("use-onever"), + model.NewKeyValueOption("version", "V2"), + model.NewKeyValueOption("go.license-header", "MICROSOFT_MIT_NO_VERSION"), + }, + }, + } + + for _, c := range testdata { + options, err := model.NewOptionsFrom(strings.NewReader(c.input)) + if err != nil { + t.Fatalf("unexpected error: %+v", err) + } + if !reflect.DeepEqual(options.Arguments(), c.expected) { + t.Fatalf("expected %+v, but got %+v", c.expected, options.Arguments()) + } + } +} diff --git a/tools/generator/cmd/generation.go b/tools/generator/cmd/generation.go index c9691ef3a1fe..6fe3efdbca8d 100644 --- a/tools/generator/cmd/generation.go +++ b/tools/generator/cmd/generation.go @@ -6,6 +6,7 @@ package cmd import ( "bufio" "fmt" + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" "io" "os" @@ -40,7 +41,7 @@ func (c autorestContext) generate() error { return nil } -func (c autorestContext) autorestArguments() []string { +func (c autorestContext) autorestArguments() []model.Option { return c.generator.Arguments() } diff --git a/tools/generator/cmd/metadata.go b/tools/generator/cmd/metadata.go index 895e3b51d25c..5e09ae560b26 100644 --- a/tools/generator/cmd/metadata.go +++ b/tools/generator/cmd/metadata.go @@ -27,7 +27,7 @@ type changelogContext struct { commonMetadata autorest.GenerationMetadata - autorestArguments []string + autorestArguments []model.Option } func (ctx changelogContext) SDKRoot() string { @@ -63,12 +63,12 @@ func (ctx changelogContext) process(metadataLocation string) ([]autorest.Changel // we need to write the generation metadata to the corresponding package here metadata := ctx.commonMetadata metadata.Tag = result.Tag - options := additionalOptions(ctx.autorestArguments) + options := autorest.AdditionalOptionsToString(ctx.autorestArguments) metadata.AdditionalProperties = autorest.GenerationMetadataAdditionalProperties{ AdditionalOptions: strings.Join(options, " "), } - metadata.AutorestCommand = fmt.Sprintf("autorest --tag=%s --go-sdk-folder=/_/azure-sdk-for-go %s /_/azure-rest-api-specs/%s", - result.Tag, strings.Join(options, " "), utils.NormalizePath(ctx.readme)) + metadata.AutorestCommand = fmt.Sprintf("autorest --use=%s --tag=%s --go-sdk-folder=/_/azure-sdk-for-go %s /_/azure-rest-api-specs/%s", + metadata.CodeGenVersion, result.Tag, strings.Join(options, " "), utils.NormalizePath(ctx.readme)) if err := WriteGenerationMetadata(result.PackageFullPath, metadata); err != nil { return nil, err } @@ -189,30 +189,3 @@ func (b *validationErrorBuilder) build() error { } return fmt.Errorf("validation failed in readme '%s' with %d error(s): \n%s", b.readme, len(b.errors), strings.Join(messages, "\n")) } - -// additionalOptions removes flags that may change over scenarios -func additionalOptions(arguments []string) []string { - var transformed []string - for _, argument := range arguments { - // remove the readme path - if !strings.HasPrefix(argument, "--") { - continue - } - // remove the go-sdk-folder - if strings.HasPrefix(argument, "--go-sdk-folder") { - continue - } - if strings.HasPrefix(argument, "--use") { - continue - } - if strings.HasPrefix(argument, "--metadata-output-folder") { - continue - } - // remove multiapi - if argument == "--multiapi" { - continue - } - transformed = append(transformed, argument) - } - return transformed -} From 22680c1b9410fecbd67cefd3d33333625907a412 Mon Sep 17 00:00:00 2001 From: ArcturusZhang Date: Fri, 16 Apr 2021 18:16:34 +0800 Subject: [PATCH 11/13] goimports --- tools/generator/autorest/generationMetadata.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/generator/autorest/generationMetadata.go b/tools/generator/autorest/generationMetadata.go index 70948ba933d7..82b4135d0a0f 100644 --- a/tools/generator/autorest/generationMetadata.go +++ b/tools/generator/autorest/generationMetadata.go @@ -6,13 +6,13 @@ package autorest import ( "encoding/json" "fmt" - "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" "io/ioutil" "log" "os" "path/filepath" "strings" + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" "github.com/Azure/azure-sdk-for-go/tools/pkgchk/track1" ) From 1294bf92d58c15bbf9b132b4afec53b25909507b Mon Sep 17 00:00:00 2001 From: arcturusZhang Date: Sat, 17 Apr 2021 10:19:20 +0800 Subject: [PATCH 12/13] if meta.json file does not exist, we need to return nil instead of empty to indicate this is special --- tools/generator/autorest/generationMetadata.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/generator/autorest/generationMetadata.go b/tools/generator/autorest/generationMetadata.go index 82b4135d0a0f..7529316d020e 100644 --- a/tools/generator/autorest/generationMetadata.go +++ b/tools/generator/autorest/generationMetadata.go @@ -58,7 +58,9 @@ func CollectGenerationMetadata(root string) (map[string]GenerationMetadata, erro if err != nil { return nil, err } - result[pkg.FullPath()] = *m + if m != nil { + result[pkg.FullPath()] = *m + } } return result, nil } @@ -70,7 +72,7 @@ func GetGenerationMetadata(pkg track1.Package) (*GenerationMetadata, error) { // some classical package might not have a changelog, therefore we need to identify whether the changelog file exist or not if _, err := os.Stat(metadataFilepath); os.IsNotExist(err) { log.Printf("package '%s' does not have a metadata file", pkg.Path()) - return &GenerationMetadata{}, nil + return nil, nil } b, err := ioutil.ReadFile(metadataFilepath) From b9412dfc037a490e8a0793fac593155269d46747 Mon Sep 17 00:00:00 2001 From: ArcturusZhang Date: Mon, 19 Apr 2021 15:25:54 +0800 Subject: [PATCH 13/13] resolve CI failures --- tools/generator/autorest/autorest.go | 1 + tools/generator/autorest/changelog.go | 2 ++ tools/generator/autorest/generationMetadata.go | 1 + tools/generator/autorest/model/changelog.go | 1 + tools/generator/autorest/model/option.go | 10 +++++++++- tools/generator/cmd/metadata.go | 2 ++ tools/generator/cmd/root.go | 8 ++++---- 7 files changed, 20 insertions(+), 5 deletions(-) diff --git a/tools/generator/autorest/autorest.go b/tools/generator/autorest/autorest.go index 270597cc0506..b2cdc28f030f 100644 --- a/tools/generator/autorest/autorest.go +++ b/tools/generator/autorest/autorest.go @@ -75,6 +75,7 @@ func (g *Generator) buildCommand() { g.cmd = exec.Command("autorest", arguments...) } +// Arguments returns the arguments which are using in the autorest command ('autorest' itself excluded) func (g *Generator) Arguments() []model.Option { return g.arguments } diff --git a/tools/generator/autorest/changelog.go b/tools/generator/autorest/changelog.go index 419893b3d53e..a45f5a31d49b 100644 --- a/tools/generator/autorest/changelog.go +++ b/tools/generator/autorest/changelog.go @@ -167,10 +167,12 @@ func GetChangelogForPackage(lhs, rhs *exports.Content) (*model.Changelog, error) }, nil } +// ToMarkdown convert this ChangelogResult to markdown string func (r ChangelogResult) ToMarkdown() string { return r.Changelog.ToMarkdown() } +// Write writes this ChangelogResult to the specified writer in markdown format func (r ChangelogResult) Write(writer io.Writer) error { _, err := writer.Write([]byte(r.ToMarkdown())) return err diff --git a/tools/generator/autorest/generationMetadata.go b/tools/generator/autorest/generationMetadata.go index 7529316d020e..ed68896d35a2 100644 --- a/tools/generator/autorest/generationMetadata.go +++ b/tools/generator/autorest/generationMetadata.go @@ -36,6 +36,7 @@ type GenerationMetadata struct { AdditionalProperties GenerationMetadataAdditionalProperties `json:"additional_properties,omitempty"` } +// GenerationMetadataAdditionalProperties contains all the additional options other than go-sdk-foler, tag, multiapi, use or the readme path type GenerationMetadataAdditionalProperties struct { AdditionalOptions string `json:"additional_options,omitempty"` } diff --git a/tools/generator/autorest/model/changelog.go b/tools/generator/autorest/model/changelog.go index 9052a0755522..9b02e8834aff 100644 --- a/tools/generator/autorest/model/changelog.go +++ b/tools/generator/autorest/model/changelog.go @@ -54,6 +54,7 @@ func (c Changelog) ToCompactMarkdown() string { return writeChangelogForPackage(c.Modified) } +// GetBreakingChangeItems returns an array of the breaking change items func (c Changelog) GetBreakingChangeItems() []string { if c.Modified == nil { return nil diff --git a/tools/generator/autorest/model/option.go b/tools/generator/autorest/model/option.go index d5588638fa9a..2fad34262ebb 100644 --- a/tools/generator/autorest/model/option.go +++ b/tools/generator/autorest/model/option.go @@ -9,31 +9,39 @@ import ( "strings" ) +// OptionType describes the type of the option, possible values are 'Argument', 'Flag' or 'KeyValue' type OptionType string const ( + // Argument ... Argument OptionType = "Argument" - Flag OptionType = "Flag" + // Flag ... + Flag OptionType = "Flag" + // KeyValue ... KeyValue OptionType = "KeyValue" ) // Option describes an option of a autorest command line type Option interface { + // Type returns the type of this option Type() OptionType // Format returns the actual option in string Format() string } +// ArgumentOption is an option not starting with '--' or '-' type ArgumentOption interface { Option Argument() string } +// FlagOption is an option that starts with '--' but do not have a value type FlagOption interface { Option Flag() string } +// KeyValueOption is an option that starts with '--' and have a value type KeyValueOption interface { Option Key() string diff --git a/tools/generator/cmd/metadata.go b/tools/generator/cmd/metadata.go index 5e09ae560b26..e9507d6a2724 100644 --- a/tools/generator/cmd/metadata.go +++ b/tools/generator/cmd/metadata.go @@ -109,6 +109,7 @@ func contains(array []autorest.ChangelogResult, item string) bool { return false } +// WriteGenerationMetadata writes the metadata to _meta.json file func WriteGenerationMetadata(path string, metadata autorest.GenerationMetadata) error { b, err := json.MarshalIndent(metadata, "", " ") if err != nil { @@ -127,6 +128,7 @@ func WriteGenerationMetadata(path string, metadata autorest.GenerationMetadata) return nil } +// WriteChangelogFile writes the changelog to CHANGELOG.md func WriteChangelogFile(result autorest.ChangelogResult) error { changelogFile, err := os.Create(filepath.Join(result.PackageFullPath, autorest.ChangelogFilename)) if err != nil { diff --git a/tools/generator/cmd/root.go b/tools/generator/cmd/root.go index 27e4de840a0c..6485f52fa324 100644 --- a/tools/generator/cmd/root.go +++ b/tools/generator/cmd/root.go @@ -188,10 +188,10 @@ func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.Ge readme: readme, removedPackages: removedPackages[readme], commonMetadata: autorest.GenerationMetadata{ - CommitHash: ctx.commitHash, - Readme: autorest.NormalizedSpecRoot + utils.NormalizePath(readme), - CodeGenVersion: options.CodeGeneratorVersion(), - RepositoryURL: "https://github.com/Azure/azure-rest-api-specs.git", + CommitHash: ctx.commitHash, + Readme: autorest.NormalizedSpecRoot + utils.NormalizePath(readme), + CodeGenVersion: options.CodeGeneratorVersion(), + RepositoryURL: "https://github.com/Azure/azure-rest-api-specs.git", }, repoContent: ctx.repoContent, autorestArguments: g.autorestArguments(),