From 82d94e43ac7eb14caeded4f5c7a55c9f8a9a2889 Mon Sep 17 00:00:00 2001 From: Arcturus Date: Tue, 8 Dec 2020 15:29:38 +0800 Subject: [PATCH] Refactor generator to use the metadata generated by autorest instead of analyzing output folders via `git diff` (#13830) * introduced metadata processor to replace the previous get packages function * refactor * some comment to satisfy the linter * adding more comment to satisfy the linter * refine and fix linter * add some comment to satisfy linter * gofmt * rename function to satisfy linter --- generate_options.json | 2 +- tools/generator/autorest/autorest.go | 184 ++++++------- tools/generator/autorest/format.go | 16 ++ tools/generator/autorest/metadata.go | 77 ++++++ tools/generator/autorest/model/metadata.go | 51 ++++ tools/generator/autorest/model/option.go | 59 ++++ tools/generator/autorest/model/options.go | 48 ++++ tools/generator/autorest/package.go | 117 -------- tools/generator/autorest/package_test.go | 257 ------------------ tools/generator/cmd/generation.go | 53 ++++ tools/generator/cmd/root.go | 136 ++++----- tools/generator/{model => pipeline}/model.go | 2 +- .../testdata/2020-10-29/foo/client.go | 1 - .../testdata/2020-10-29/foo/models.go | 1 - .../testdata/2020-10-29/foo/version.go | 1 - .../testdata/2020-10-30/foo/client.go | 1 - .../testdata/2020-10-30/foo/models.go | 1 - .../testdata/2020-10-30/foo/version.go | 1 - 18 files changed, 454 insertions(+), 554 deletions(-) create mode 100644 tools/generator/autorest/format.go create mode 100644 tools/generator/autorest/metadata.go create mode 100644 tools/generator/autorest/model/metadata.go create mode 100644 tools/generator/autorest/model/option.go create mode 100644 tools/generator/autorest/model/options.go delete mode 100644 tools/generator/autorest/package.go delete mode 100644 tools/generator/autorest/package_test.go create mode 100644 tools/generator/cmd/generation.go rename tools/generator/{model => pipeline}/model.go (99%) delete mode 100644 tools/generator/testdata/2020-10-29/foo/client.go delete mode 100644 tools/generator/testdata/2020-10-29/foo/models.go delete mode 100644 tools/generator/testdata/2020-10-29/foo/version.go delete mode 100644 tools/generator/testdata/2020-10-30/foo/client.go delete mode 100644 tools/generator/testdata/2020-10-30/foo/models.go delete mode 100644 tools/generator/testdata/2020-10-30/foo/version.go diff --git a/generate_options.json b/generate_options.json index 21a9151b7a43..8b459be91308 100644 --- a/generate_options.json +++ b/generate_options.json @@ -1,6 +1,6 @@ { "autorestArguments": [ - "--use=@microsoft.azure/autorest.go@~2.1.161", + "--use=@microsoft.azure/autorest.go@~2.1.162", "--go", "--verbose", "--go-sdk-folder=.", diff --git a/tools/generator/autorest/autorest.go b/tools/generator/autorest/autorest.go index 5be7aa57bf12..d33d92093191 100644 --- a/tools/generator/autorest/autorest.go +++ b/tools/generator/autorest/autorest.go @@ -1,136 +1,128 @@ package autorest import ( - "bufio" - "encoding/json" "fmt" "io" - "io/ioutil" - "log" - "os" "os/exec" - "strings" + + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" ) -// Task describes a generation task -type Task struct { - // AbsReadmeMd absolute path of the readme.md file to generate - AbsReadmeMd string +// Generator collects all the related context of an autorest generation +type Generator struct { + arguments []string + cmd *exec.Cmd } -// Execute executes the autorest task, and then invoke the after scripts -// the error returned will be TaskError -func (t *Task) Execute(options Options) error { - if err := t.executeAutorest(options.AutorestArguments); err != nil { - return err +// NewGeneratorFromOptions returns a new Generator with the given model.Options +func NewGeneratorFromOptions(o model.Options) *Generator { + return &Generator{ + arguments: o.Arguments(), } +} - if err := t.executeAfterScript(options.AfterScripts); err != nil { - return err - } +// 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()) + return g +} - return nil +// WithTag appends a tag option to the autorest argument list +func (g *Generator) WithTag(tag string) *Generator { + return g.WithOption(model.NewKeyValueOption("tag", tag)) +} + +// WithMultiAPI appends a multiapi flag to the autorest argument list +func (g *Generator) WithMultiAPI() *Generator { + return g.WithOption(model.NewFlagOption("multiapi")) } -func (t *Task) executeAutorest(options []string) error { - arguments := append(options, t.AbsReadmeMd) - c := exec.Command("autorest", arguments...) - log.Printf("Executing autorest with parameters: %+v", arguments) +// WithMetadataOutput appends a `metadata-output-folder` option to the autorest argument list +func (g *Generator) WithMetadataOutput(output string) *Generator { + return g.WithOption(model.NewKeyValueOption("metadata-output-folder", output)) +} - stdout, _ := c.StdoutPipe() - stderr, _ := c.StderrPipe() - errScanner := bufio.NewScanner(stderr) - errScanner.Split(bufio.ScanLines) - outScanner := bufio.NewScanner(stdout) - outScanner.Split(bufio.ScanLines) +// WithReadme appends a readme argument +func (g *Generator) WithReadme(readme string) *Generator { + return g.WithOption(model.NewArgument(readme)) +} - if err := c.Start(); err != nil { - return &TaskError{ - AbsReadmeMd: t.AbsReadmeMd, - Script: "autorest", - Message: err.Error(), +// Generate executes the autorest generation. The error will be of type *GenerateError +func (g *Generator) Generate() error { + g.buildCommand() + c := exec.Command("autorest", g.arguments...) + o, err := c.CombinedOutput() + if err != nil { + return &GenerateError{ + Arguments: g.arguments, + Message: string(o), } } + return nil +} - go func() { - for errScanner.Scan() { - line := errScanner.Text() - fmt.Fprintln(os.Stderr, "[AUTOREST] "+line) - } - }() - - go func() { - for outScanner.Scan() { - line := outScanner.Text() - fmt.Fprintln(os.Stdout, "[AUTOREST] "+line) - } - }() +func (g *Generator) buildCommand() { + if g.cmd != nil { + return + } + g.cmd = exec.Command("autorest", g.arguments...) +} - if err := c.Wait(); err != nil { - return &TaskError{ - AbsReadmeMd: t.AbsReadmeMd, - Script: "autorest", - Message: err.Error(), +// Start starts the generation +func (g *Generator) Start() error { + g.buildCommand() + if err := g.cmd.Start(); err != nil { + return &GenerateError{ + Arguments: g.arguments, + Message: err.Error(), } } return nil } -func (t *Task) executeAfterScript(afterScripts []string) error { - for _, script := range afterScripts { - log.Printf("Executing after scripts %s...", script) - arguments := strings.Split(script, " ") - c := exec.Command(arguments[0], arguments[1:]...) - output, err := c.CombinedOutput() - if err != nil { - return &TaskError{ - AbsReadmeMd: t.AbsReadmeMd, - Script: script, - Message: string(output), - } +// Wait waits for the generation to complete +func (g *Generator) Wait() error { + g.buildCommand() + if err := g.cmd.Wait(); err != nil { + return &GenerateError{ + Arguments: g.arguments, + Message: err.Error(), } } - return nil } -// Options describes the options used in an autorest task -type Options struct { - // AutorestArguments are the optional flags for the autorest tool - AutorestArguments []string - // AfterScripts are the scripts that need to be run after the SDK is generated - AfterScripts []string +// Run starts and waits the generation +func (g *Generator) Run() error { + g.buildCommand() + if err := g.cmd.Run(); err != nil { + return &GenerateError{ + Arguments: g.arguments, + Message: err.Error(), + } + } + return nil } -// NewOptionsFrom returns a new options from a io.Reader -func NewOptionsFrom(reader io.Reader) (*Options, error) { - b, err := ioutil.ReadAll(reader) - if err != nil { - return nil, err - } - var result Options - if err := json.Unmarshal(b, &result); err != nil { - return nil, err - } - return &result, nil +// StdoutPipe returns the stdout pipeline of the command +func (g *Generator) StdoutPipe() (io.ReadCloser, error) { + g.buildCommand() + return g.cmd.StdoutPipe() } -// String ... -func (o Options) String() string { - b, _ := json.MarshalIndent(o, "", " ") - return string(b) +// StderrPipe returns the stderr pipeline of the command +func (g *Generator) StderrPipe() (io.ReadCloser, error) { + g.buildCommand() + return g.cmd.StderrPipe() } -// TaskError the error returned during an autorest task -type TaskError struct { - // AbsReadmeMd relative path of the readme.md file to generate - AbsReadmeMd string - // Script the script running when the error is thrown - Script string - // Message the error message - Message string +// GenerateError ... +type GenerateError struct { + Arguments []string + Message string } -func (r *TaskError) Error() string { - return fmt.Sprintf("autorest task failed for readme file '%s' during '%s': %s", r.AbsReadmeMd, r.Script, r.Message) +// Error ... +func (e *GenerateError) Error() string { + return fmt.Sprintf("autorest error with arguments '%s': \n%s", e.Arguments, e.Message) } diff --git a/tools/generator/autorest/format.go b/tools/generator/autorest/format.go new file mode 100644 index 000000000000..caaa5d245df0 --- /dev/null +++ b/tools/generator/autorest/format.go @@ -0,0 +1,16 @@ +package autorest + +import ( + "fmt" + "os/exec" +) + +// FormatPackage formats the given package using gofmt +func FormatPackage(dir string) error { + c := exec.Command("gofmt", "-w", "-s", dir) + b, err := c.CombinedOutput() + if err != nil { + return fmt.Errorf(string(b)) + } + return nil +} diff --git a/tools/generator/autorest/metadata.go b/tools/generator/autorest/metadata.go new file mode 100644 index 000000000000..22bd2f00fd80 --- /dev/null +++ b/tools/generator/autorest/metadata.go @@ -0,0 +1,77 @@ +package autorest + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + + "github.com/Azure/azure-sdk-for-go/tools/generator/autorest/model" +) + +// MetadataProcessError ... +type MetadataProcessError struct { + MetadataLocation string + Errors []error +} + +// Error ... +func (e *MetadataProcessError) Error() string { + return fmt.Sprintf("total %d error(s) during processing metadata %s: %+v", len(e.Errors), e.MetadataLocation, e.Errors) +} + +func (e *MetadataProcessError) add(err error) { + e.Errors = append(e.Errors, err) +} + +// MetadataProcessor processes the metadata +type MetadataProcessor struct { + metadataOutputFolder string +} + +// NewMetadataProcessorFromLocation creates a new MetadataProcessor using the metadata output folder location +func NewMetadataProcessorFromLocation(metadataOutput string) *MetadataProcessor { + return &MetadataProcessor{ + metadataOutputFolder: metadataOutput, + } +} + +// Process returns the metadata result: a map from tag to Metadata, and an error if there is anything that could not be processed. +// the error returned must be of type *MetadataProcessError +func (p MetadataProcessor) Process() (map[string]model.Metadata, error) { + fi, err := ioutil.ReadDir(p.metadataOutputFolder) + if err != nil { + return nil, &MetadataProcessError{ + MetadataLocation: p.metadataOutputFolder, + Errors: []error{err}, + } + } + result := make(map[string]model.Metadata) + metadataErr := &MetadataProcessError{ + MetadataLocation: p.metadataOutputFolder, + } + for _, f := range fi { + // a metadata output must be a json file + if f.IsDir() || !strings.HasSuffix(f.Name(), ".json") { + continue + } + file, err := os.Open(filepath.Join(p.metadataOutputFolder, f.Name())) + if err != nil { + metadataErr.add(err) + continue + } + metadata, err := model.NewMetadataFrom(file) + if err != nil { + metadataErr.add(err) + continue + } + tag := strings.TrimSuffix(f.Name(), ".json") + result[tag] = metadata + } + + if len(metadataErr.Errors) != 0 { + return result, metadataErr + } + return result, nil +} diff --git a/tools/generator/autorest/model/metadata.go b/tools/generator/autorest/model/metadata.go new file mode 100644 index 000000000000..bd7cc3500457 --- /dev/null +++ b/tools/generator/autorest/model/metadata.go @@ -0,0 +1,51 @@ +package model + +import ( + "encoding/json" + "io" + "io/ioutil" +) + +// Metadata ... +type Metadata interface { + // SwaggerFiles returns the related swagger files in this tag + SwaggerFiles() []string + // PackagePath returns the output package path of this tag + PackagePath() string +} + +// NewMetadataFrom reads a new Metadata from a io.Reader +func NewMetadataFrom(reader io.Reader) (Metadata, error) { + b, err := ioutil.ReadAll(reader) + if err != nil { + return nil, err + } + var result localMetadata + if err := json.Unmarshal(b, &result); err != nil { + return nil, err + } + return &result, nil +} + +type localMetadata struct { + // InputFiles ... + InputFiles []string `json:"inputFiles"` + // OutputFolder ... + OutputFolder string `json:"outputFolder"` +} + +// SwaggerFiles ... +func (m localMetadata) SwaggerFiles() []string { + return m.InputFiles +} + +// PackagePath ... +func (m localMetadata) PackagePath() string { + return m.OutputFolder +} + +// String ... +func (m localMetadata) String() string { + b, _ := json.MarshalIndent(m, "", " ") + return string(b) +} diff --git a/tools/generator/autorest/model/option.go b/tools/generator/autorest/model/option.go new file mode 100644 index 000000000000..41c6b72690d6 --- /dev/null +++ b/tools/generator/autorest/model/option.go @@ -0,0 +1,59 @@ +package model + +import "fmt" + +// Option describes an option of a autorest command line +type Option interface { + // Format returns the actual option in string + Format() string +} + +type argument struct { + value string +} + +// Format ... +func (f argument) Format() string { + return f.value +} + +type flagOption struct { + value string +} + +// Format ... +func (f flagOption) Format() string { + return fmt.Sprintf("--%s", f.value) +} + +type keyValueOption struct { + key string + value string +} + +// Format ... +func (f keyValueOption) Format() string { + return fmt.Sprintf("--%s=%s", f.key, f.value) +} + +// NewArgument returns a new argument option (without "--") +func NewArgument(value string) Option { + return argument{ + value: value, + } +} + +// NewFlagOption returns a flag option (with "--", without value) +func NewFlagOption(flag string) Option { + return flagOption{ + value: flag, + } +} + +// NewKeyValueOption returns a key-value option like "--tag=something" +func NewKeyValueOption(key, value string) Option { + return keyValueOption{ + key: key, + value: value, + } +} diff --git a/tools/generator/autorest/model/options.go b/tools/generator/autorest/model/options.go new file mode 100644 index 000000000000..33436888852b --- /dev/null +++ b/tools/generator/autorest/model/options.go @@ -0,0 +1,48 @@ +package model + +import ( + "encoding/json" + "io" + "io/ioutil" +) + +// Options ... +type Options interface { + // Arguments returns the argument defined in this options + Arguments() []string + // CodeGeneratorVersion returns the code generator version defined in this options + CodeGeneratorVersion() string +} + +type localOptions struct { + AutorestArguments []string `json:"autorestArguments"` +} + +// 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 { + return nil, err + } + return &result, nil +} + +// Argument ... +func (o localOptions) Arguments() []string { + return o.AutorestArguments +} + +// String ... +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/package.go b/tools/generator/autorest/package.go deleted file mode 100644 index fdc9b22cc6ba..000000000000 --- a/tools/generator/autorest/package.go +++ /dev/null @@ -1,117 +0,0 @@ -package autorest - -import ( - "fmt" - "os" - "path/filepath" - "strings" -) - -// ChangedPackagesMap is a wrapper of a map of packages. The key is package names, and the value is the changed file list. -type ChangedPackagesMap map[string][]string - -func (c *ChangedPackagesMap) addFileToPackage(pkg, file string) { - pkg = strings.ReplaceAll(pkg, "\\", "/") - if _, ok := (*c)[pkg]; !ok { - (*c)[pkg] = []string{} - } - (*c)[pkg] = append((*c)[pkg], file) -} - -func (c *ChangedPackagesMap) String() string { - var r []string - for k, v := range *c { - r = append(r, fmt.Sprintf("%s: %+v", k, v)) - } - return strings.Join(r, "\n") -} - -// GetChangedPackages get the go SDK packages map from the given changed file list. -// the map returned has the package full path as key, and the changed files in the package as the value. -// This function identify the package by checking if a directory has both a `version.go` file and a `client.go` file. -func GetChangedPackages(changedFiles []string) (ChangedPackagesMap, error) { - changedFiles, err := ExpandChangedDirectories(changedFiles) - if err != nil { - return nil, err - } - r := ChangedPackagesMap{} - for _, file := range changedFiles { - fi, err := os.Stat(file) - isDir := false - if err != nil && !os.IsNotExist(err) { - return nil, err - } - if fi != nil { - isDir = fi.IsDir() - } - path := file - if !isDir { - path = filepath.Dir(file) - } - if IsValidPackage(path) { - r.addFileToPackage(path, file) - } - } - return r, nil -} - -// ExpandChangedDirectories expands every directory listed in the array to all its file -func ExpandChangedDirectories(changedFiles []string) ([]string, error) { - var result []string - for _, path := range changedFiles { - fi, err := os.Stat(path) - isDir := false - if err != nil && !os.IsNotExist(err) { - return nil, err - } - if fi != nil { - isDir = fi.IsDir() - } - if isDir { - siblings, err := getAllFiles(path) - if err != nil { - return nil, err - } - result = append(result, siblings...) - } else { - result = append(result, path) - } - } - - return result, nil -} - -func getAllFiles(root string) ([]string, error) { - var siblings []string - err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - if !info.IsDir() { - siblings = append(siblings, strings.ReplaceAll(path, "\\", "/")) - } - return nil - }) - return siblings, err -} - -const ( - clientGo = "client.go" - versionGo = "version.go" -) - -// IsValidPackage returns true when the given directory is a valid azure-sdk-for-go package, -// otherwise returns false (including the directory does not exist) -// The criteria of a valid azure-sdk-for-go package is that the directory must have a `client.go` and a `version.go` file -func IsValidPackage(dir string) bool { - client := filepath.Join(dir, clientGo) - version := filepath.Join(dir, versionGo) - // both the above files must exist to return true - if _, err := os.Stat(client); os.IsNotExist(err) { - return false - } - if _, err := os.Stat(version); os.IsNotExist(err) { - return false - } - return true -} diff --git a/tools/generator/autorest/package_test.go b/tools/generator/autorest/package_test.go deleted file mode 100644 index 3938ea315382..000000000000 --- a/tools/generator/autorest/package_test.go +++ /dev/null @@ -1,257 +0,0 @@ -package autorest - -import ( - "reflect" - "testing" -) - -func TestGetChangedPackages(t *testing.T) { - testData := []struct { - description string - changedFiles []string - expected map[string][]string - }{ - { - description: "one file changed in one package", - changedFiles: []string{ - "../testdata/2020-10-29/foo/models.go", - }, - expected: map[string][]string{ - "../testdata/2020-10-29/foo": { - "../testdata/2020-10-29/foo/models.go", - }, - }, - }, - { - description: "two files changed in one package", - changedFiles: []string{ - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - }, - expected: map[string][]string{ - "../testdata/2020-10-29/foo": { - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - }, - }, - }, - { - description: "multiple files changed in two packages", - changedFiles: []string{ - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - "../testdata/2020-10-30/foo/models.go", - }, - expected: map[string][]string{ - "../testdata/2020-10-29/foo": { - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - }, - "../testdata/2020-10-30/foo": { - "../testdata/2020-10-30/foo/models.go", - }, - }, - }, - { - description: "one directory untracked and one file changed in one package", - changedFiles: []string{ - "../testdata/2020-10-29/foo", - "../testdata/2020-10-30/foo/models.go", - }, - expected: map[string][]string{ - "../testdata/2020-10-29/foo": { - "../testdata/2020-10-29/foo/client.go", - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - }, - "../testdata/2020-10-30/foo": { - "../testdata/2020-10-30/foo/models.go", - }, - }, - }, - { - description: "two untracked directories", - changedFiles: []string{ - "../testdata/2020-10-29/foo", - "../testdata/2020-10-30/foo", - }, - expected: map[string][]string{ - "../testdata/2020-10-29/foo": { - "../testdata/2020-10-29/foo/client.go", - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - }, - "../testdata/2020-10-30/foo": { - "../testdata/2020-10-30/foo/client.go", - "../testdata/2020-10-30/foo/models.go", - "../testdata/2020-10-30/foo/version.go", - }, - }, - }, - { - description: "one untracked directory that contains one packages", - changedFiles: []string{ - "../testdata/2020-10-29", - }, - expected: map[string][]string{ - "../testdata/2020-10-29/foo": { - "../testdata/2020-10-29/foo/client.go", - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - }, - }, - }, - { - description: "one untracked directory that contains multiple packages", - changedFiles: []string{ - "../testdata/", - }, - expected: map[string][]string{ - "../testdata/2020-10-29/foo": { - "../testdata/2020-10-29/foo/client.go", - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - }, - "../testdata/2020-10-30/foo": { - "../testdata/2020-10-30/foo/client.go", - "../testdata/2020-10-30/foo/models.go", - "../testdata/2020-10-30/foo/version.go", - }, - }, - }, - } - - for _, c := range testData { - t.Logf("testing %s", c.description) - r, err := GetChangedPackages(c.changedFiles) - if err != nil { - t.Fatalf("unexpected error: %+v", err) - } - if !mapDeepEqual(r, c.expected) { - t.Fatalf("expected %+v, but got %+v", c.expected, r) - } - } -} - -func TestExpandChangedDirectories(t *testing.T) { - testData := []struct { - description string - input []string - expected []string - }{ - { - description: "only files", - input: []string{ - "../testdata/2020-10-29/foo/client.go", - }, - expected: []string{ - "../testdata/2020-10-29/foo/client.go", - }, - }, - { - description: "only directories", - input: []string{ - "../testdata/2020-10-29/foo", - }, - expected: []string{ - "../testdata/2020-10-29/foo/client.go", - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - }, - }, - { - description: "both directories and files", - input: []string{ - "../testdata/2020-10-29/foo", - "../testdata/2020-10-30/foo/models.go", - }, - expected: []string{ - "../testdata/2020-10-29/foo/client.go", - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - "../testdata/2020-10-30/foo/models.go", - }, - }, - { - description: "multiple hierarchy of directories but only one sub-directory", - input: []string{ - "../testdata/2020-10-29", - }, - expected: []string{ - "../testdata/2020-10-29/foo/client.go", - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - }, - }, - { - description: "multiple hierarchy of directories", - input: []string{ - "../testdata", - }, - expected: []string{ - "../testdata/2020-10-29/foo/client.go", - "../testdata/2020-10-29/foo/models.go", - "../testdata/2020-10-29/foo/version.go", - "../testdata/2020-10-30/foo/client.go", - "../testdata/2020-10-30/foo/models.go", - "../testdata/2020-10-30/foo/version.go", - }, - }, - } - - for _, c := range testData { - t.Logf("testing %s", c.description) - r, err := ExpandChangedDirectories(c.input) - if err != nil { - t.Fatalf("unexpected error: %+v", err) - } - if !reflect.DeepEqual(r, c.expected) { - t.Fatalf("expected %v but got %v", c.expected, r) - } - } -} - -// subsetOf return true if m2 is the subset of m1 (every key in m1 exists in m2 and the corresponding values are the same) -func subsetOf(m1, m2 map[string][]string) bool { - for k, v := range m1 { - if v2, ok := m2[k]; !ok || !reflect.DeepEqual(v, v2) { - return false - } - } - return true -} - -func mapDeepEqual(m1, m2 map[string][]string) bool { - return subsetOf(m1, m2) && subsetOf(m2, m1) -} - -func TestIsValidPackage(t *testing.T) { - testData := []struct { - input string - expected bool - }{ - { - input: "../../../services/compute/mgmt/2020-06-01/compute", - expected: true, - }, - { - input: "../../../services/compute/mgmt/2020-06-01", - expected: false, - }, - { - input: "../../../storage", - expected: false, - }, - { - input: "../../../profiles", - expected: false, - }, - } - - for _, c := range testData { - r := IsValidPackage(c.input) - if r != c.expected { - t.Fatalf("expected %v but got %v", c.expected, r) - } - } -} diff --git a/tools/generator/cmd/generation.go b/tools/generator/cmd/generation.go new file mode 100644 index 000000000000..3514bc2fe145 --- /dev/null +++ b/tools/generator/cmd/generation.go @@ -0,0 +1,53 @@ +package cmd + +import ( + "bufio" + "fmt" + "io" + "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 +} + +func (c autorestContext) generate() error { + g := autorest.NewGeneratorFromOptions(c.options).WithReadme(c.absReadme).WithMetadataOutput(c.metadataOutput) + + stdout, _ := g.StdoutPipe() + stderr, _ := g.StderrPipe() + defer stdout.Close() + defer stderr.Close() + outScanner := bufio.NewScanner(stdout) + outScanner.Split(bufio.ScanLines) + errScanner := bufio.NewScanner(stderr) + errScanner.Split(bufio.ScanLines) + + if err := g.Start(); err != nil { + return err + } + + go printWithPrefixTo(outScanner, os.Stdout, "[AUTOREST] ") + go printWithPrefixTo(errScanner, os.Stderr, "[AUTOREST] ") + + if err := g.Wait(); err != nil { + return err + } + + return nil +} + +func printWithPrefixTo(scanner *bufio.Scanner, writer io.Writer, prefix string) error { + for scanner.Scan() { + line := scanner.Text() + if _, err := fmt.Fprintln(writer, fmt.Sprintf("%s%s", prefix, line)); err != nil { + return err + } + } + return nil +} diff --git a/tools/generator/cmd/root.go b/tools/generator/cmd/root.go index ea47020a70f3..91292b568daa 100644 --- a/tools/generator/cmd/root.go +++ b/tools/generator/cmd/root.go @@ -4,14 +4,14 @@ import ( "fmt" "log" "os" - "os/exec" "path/filepath" "sort" "strings" "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/changelog" - "github.com/Azure/azure-sdk-for-go/tools/generator/model" + "github.com/Azure/azure-sdk-for-go/tools/generator/pipeline" "github.com/spf13/cobra" ) @@ -19,7 +19,8 @@ const ( defaultOptionPath = "generate_options.json" ) -// Command ... +// Command returns the command for the generator. Note that this command is designed to run in the root directory of +// azure-sdk-for-go. It might not work if you are running this tool in somewhere else func Command() *cobra.Command { rootCmd := &cobra.Command{ Use: "generator ", @@ -58,7 +59,15 @@ func execute(inputPath, outputPath string, flags Flags) error { return fmt.Errorf("cannot read generate input: %+v", err) } log.Printf("Generating using the following GenerateInput...\n%s", input.String()) - output, err := generate(input, flags.OptionPath) + cwd, err := os.Getwd() + if err != nil { + return err + } + ctx := generateContext{ + cwd: cwd, + optionPath: flags.OptionPath, + } + output, err := ctx.generate(input) if err != nil { return fmt.Errorf("cannot generate: %+v", err) } @@ -70,15 +79,15 @@ func execute(inputPath, outputPath string, flags Flags) error { return nil } -func readInputFrom(inputPath string) (*model.GenerateInput, error) { +func readInputFrom(inputPath string) (*pipeline.GenerateInput, error) { inputFile, err := os.Open(inputPath) if err != nil { return nil, err } - return model.NewGenerateInputFrom(inputFile) + return pipeline.NewGenerateInputFrom(inputFile) } -func writeOutputTo(outputPath string, output *model.GenerateOutput) error { +func writeOutputTo(outputPath string, output *pipeline.GenerateOutput) error { file, err := os.Create(outputPath) if err != nil { return err @@ -90,61 +99,81 @@ func writeOutputTo(outputPath string, output *model.GenerateOutput) error { return nil } +type generateContext struct { + cwd string + optionPath string +} + // TODO -- support dry run -func generate(input *model.GenerateInput, optionPath string) (*model.GenerateOutput, error) { +func (ctx generateContext) generate(input *pipeline.GenerateInput) (*pipeline.GenerateOutput, error) { if input.DryRun { return nil, fmt.Errorf("dry run not supported yet") } - log.Printf("Reading options from file '%s'...", optionPath) + log.Printf("Reading options from file '%s'...", ctx.optionPath) - optionFile, err := os.Open(optionPath) + optionFile, err := os.Open(ctx.optionPath) if err != nil { return nil, err } - options, err := autorest.NewOptionsFrom(optionFile) + options, err := model.NewOptionsFrom(optionFile) if err != nil { return nil, err } - log.Printf("Autorest options: \n%s", options.String()) + log.Printf("Autorest options: \n%+v", options) // iterate over all the readme - results := make([]model.PackageResult, 0) + results := make([]pipeline.PackageResult, 0) for _, readme := range input.RelatedReadmeMdFiles { + // TODO -- maintain a map from readme files to corresponding output folders, so that we could detect the situation that a package was deleted log.Printf("Processing readme '%s'...", readme) - task := autorest.Task{ - AbsReadmeMd: filepath.Join(input.SpecFolder, readme), + absReadme := filepath.Join(input.SpecFolder, readme) + // generate code + g := autorestContext{ + absReadme: absReadme, + metadataOutput: filepath.Dir(absReadme), + options: options, } - if err := task.Execute(*options); err != nil { + if err := g.generate(); err != nil { return nil, err } - // get changed file list - changedFiles, err := getChangedFiles() + // get the metadata map + m := autorest.NewMetadataProcessorFromLocation(g.metadataOutput) + metadataMap, err := m.Process() if err != nil { return nil, err } - log.Printf("Files changed in the SDK: %+v", changedFiles) - // get packages using the changed file list - // returns a map, key is package path, value is files that have changed - packages, err := autorest.GetChangedPackages(changedFiles) - if err != nil { - return nil, err + var packages []string + for _, metadata := range metadataMap { + // TODO -- first validate the output folder is valid + outputFolder := filepath.Clean(metadata.PackagePath()) + // first format the package + if err := autorest.FormatPackage(outputFolder); err != nil { + return nil, err + } + // get the package path - which is a relative path to the sdk root + packagePath, err := filepath.Rel(ctx.cwd, outputFolder) + if err != nil { + return nil, err + } + packages = append(packages, packagePath) } log.Printf("Packages changed: %+v", packages) // iterate over the changed packages - for p, files := range packages { - log.Printf("Getting package result for package '%s', changed files are: [%s]", p, strings.Join(files, ", ")) + for _, p := range packages { + p = normalizePath(p) + log.Printf("Getting package result for package '%s'", p) c, err := changelog.NewChangelogForPackage(p) if err != nil { return nil, err } content := c.ToMarkdown() breaking := c.HasBreakingChanges() - results = append(results, model.PackageResult{ + results = append(results, pipeline.PackageResult{ PackageName: getPackageIdentifier(p), Path: []string{p}, ReadmeMd: []string{readme}, - Changelog: &model.Changelog{ + Changelog: &pipeline.Changelog{ Content: &content, HasBreakingChange: &breaking, }, @@ -159,58 +188,13 @@ func generate(input *model.GenerateInput, optionPath string) (*model.GenerateOut return apiI > apiJ }) - return &model.GenerateOutput{ + return &pipeline.GenerateOutput{ Packages: results, }, nil } -func getChangedFiles() ([]string, error) { - var files []string - // get the file changed - changed, err := getDiffFiles() - if err != nil { - return nil, err - } - files = append(files, changed...) - // get the untracked files - untracked, err := getUntrackedFiles() - if err != nil { - return nil, err - } - files = append(files, untracked...) - return files, nil -} - -func getDiffFiles() ([]string, error) { - c := exec.Command("git", "diff", "--name-only") - output, err := c.Output() - if err != nil { - return nil, err - } - var files []string - for _, f := range strings.Split(string(output), "\n") { - f = strings.TrimSpace(f) - if f != "" { - files = append(files, f) - } - } - return files, nil -} - -func getUntrackedFiles() ([]string, error) { - c := exec.Command("git", "ls-files", "--other", "--exclude-standard") - output, err := c.Output() - if err != nil { - return nil, err - } - var files []string - for _, f := range strings.Split(string(output), "\n") { - f = strings.TrimSpace(f) - if f != "" { - files = append(files, f) - } - } - return files, nil +func normalizePath(path string) string { + return strings.ReplaceAll(path, "\\", "/") } func getPackageIdentifier(pkg string) string { diff --git a/tools/generator/model/model.go b/tools/generator/pipeline/model.go similarity index 99% rename from tools/generator/model/model.go rename to tools/generator/pipeline/model.go index 5fab36661246..f863c1450141 100644 --- a/tools/generator/model/model.go +++ b/tools/generator/pipeline/model.go @@ -1,4 +1,4 @@ -package model +package pipeline import ( "encoding/json" diff --git a/tools/generator/testdata/2020-10-29/foo/client.go b/tools/generator/testdata/2020-10-29/foo/client.go deleted file mode 100644 index f52652b1ba78..000000000000 --- a/tools/generator/testdata/2020-10-29/foo/client.go +++ /dev/null @@ -1 +0,0 @@ -package foo diff --git a/tools/generator/testdata/2020-10-29/foo/models.go b/tools/generator/testdata/2020-10-29/foo/models.go deleted file mode 100644 index f52652b1ba78..000000000000 --- a/tools/generator/testdata/2020-10-29/foo/models.go +++ /dev/null @@ -1 +0,0 @@ -package foo diff --git a/tools/generator/testdata/2020-10-29/foo/version.go b/tools/generator/testdata/2020-10-29/foo/version.go deleted file mode 100644 index f52652b1ba78..000000000000 --- a/tools/generator/testdata/2020-10-29/foo/version.go +++ /dev/null @@ -1 +0,0 @@ -package foo diff --git a/tools/generator/testdata/2020-10-30/foo/client.go b/tools/generator/testdata/2020-10-30/foo/client.go deleted file mode 100644 index f52652b1ba78..000000000000 --- a/tools/generator/testdata/2020-10-30/foo/client.go +++ /dev/null @@ -1 +0,0 @@ -package foo diff --git a/tools/generator/testdata/2020-10-30/foo/models.go b/tools/generator/testdata/2020-10-30/foo/models.go deleted file mode 100644 index f52652b1ba78..000000000000 --- a/tools/generator/testdata/2020-10-30/foo/models.go +++ /dev/null @@ -1 +0,0 @@ -package foo diff --git a/tools/generator/testdata/2020-10-30/foo/version.go b/tools/generator/testdata/2020-10-30/foo/version.go deleted file mode 100644 index f52652b1ba78..000000000000 --- a/tools/generator/testdata/2020-10-30/foo/version.go +++ /dev/null @@ -1 +0,0 @@ -package foo