From 2ab4f0dc438cf91f48e6cb96ee7b9c1689a0afc8 Mon Sep 17 00:00:00 2001 From: Marcin Bilski Date: Sat, 23 Oct 2021 23:05:11 +0200 Subject: [PATCH 1/9] Experimental way to expose tasks, adding them to global scope. --- features/expose.feature | 70 +++++++++++++++++++++++++++++++++++++++++ pkg/oyafile/imports.go | 11 ++++++- pkg/oyafile/oyafile.go | 23 ++++++++------ pkg/oyafile/parsing.go | 13 ++++++++ pkg/task/mocks.go | 13 ++++++++ pkg/task/name.go | 11 +++++++ pkg/task/name_test.go | 23 ++++++++++++++ pkg/task/table.go | 25 ++++++++++++--- pkg/task/table_test.go | 24 ++++++++++++++ todo.org | 5 +++ 10 files changed, 203 insertions(+), 15 deletions(-) create mode 100644 features/expose.feature create mode 100644 pkg/task/mocks.go create mode 100644 pkg/task/table_test.go diff --git a/features/expose.feature b/features/expose.feature new file mode 100644 index 0000000..6a2f8f4 --- /dev/null +++ b/features/expose.feature @@ -0,0 +1,70 @@ +Feature: Exposing imported package tasks so they can be invoked without the alias. + +Background: + Given I'm in project dir + + +Scenario: Expose tasks + Given file ./Oyafile containing + """ + Project: main + """ + And file ./project1/Oyafile containing + """ + Values: + foo: project1 + + echo: | + echo "project1" + """ + And file ./project2/Oyafile containing + """ + Import: + p: /project1 + + Expose: p + """ + And I'm in the ./project2 dir + When I run "oya run echo" + Then the command succeeds + And the command outputs + """ + project1 + + """ + +@current +Scenario: Never overwrite existing an task when exposing tasks + Given file ./Oyafile containing + """ + Project: main + """ + And file ./project1/Oyafile containing + """ + Values: + foo: project1 + + echo: | + echo "project1" + """ + And file ./project2/Oyafile containing + """ + Import: + p: /project1 + + Expose: p + + echo: | + echo "project2" + """ + And I'm in the ./project2 dir + When I run "oya run echo" + Then the command succeeds + And the command outputs + """ + project2 + + """ + + +# TODO: Show as aliases when listing tasks. diff --git a/pkg/oyafile/imports.go b/pkg/oyafile/imports.go index a186895..ad74e20 100644 --- a/pkg/oyafile/imports.go +++ b/pkg/oyafile/imports.go @@ -36,11 +36,13 @@ func (oyafile *Oyafile) resolveImports(loader PackLoader) error { if err != nil { return err } + + // TODO(bilus): Extract function. err = oyafile.Values.UpdateScopeAt(string(alias), func(scope template.Scope) template.Scope { // Values in the main Oyafile overwrite values in the pack Oyafile. merged := packOyafile.Values.Merge(scope) - // Task is keeping a pointed to the scope. + // Task is keeping a pointer to the scope. packOyafile.Values.Replace(merged) return merged }, false) @@ -51,6 +53,13 @@ func (oyafile *Oyafile) resolveImports(loader PackLoader) error { oyafile.Tasks.ImportTasks(alias, packOyafile.Tasks) } + return oyafile.expose() +} + +func (oyafile *Oyafile) expose() error { + for _, alias := range oyafile.ExposedAliases { + oyafile.Tasks.Expose(alias) + } return nil } diff --git a/pkg/oyafile/oyafile.go b/pkg/oyafile/oyafile.go index f8ccbdf..efc40ac 100644 --- a/pkg/oyafile/oyafile.go +++ b/pkg/oyafile/oyafile.go @@ -27,16 +27,19 @@ type PackReference struct { type PackReplacements map[types.ImportPath]string type Oyafile struct { - Dir string - Path string - RootDir string - Shell string - Imports map[types.Alias]types.ImportPath - Tasks task.Table - Values template.Scope - Project string // Project is set for root Oyafile. - Ignore []string // Ignore contains directory exclusion rules. - Requires []PackReference + Dir string + Path string + RootDir string + Shell string + Imports map[types.Alias]types.ImportPath + Tasks task.Table + // ExposedAliases contains a list of imported packs' aliases that will + // be exposed to global scope. + ExposedAliases []types.Alias + Values template.Scope + Project string // Project is set for root Oyafile. + Ignore []string // Ignore contains directory exclusion rules. + Requires []PackReference // Replacements map packs to local paths relative to project root directory for development based on the Replace: directive. Replacements PackReplacements IsBuilt bool diff --git a/pkg/oyafile/parsing.go b/pkg/oyafile/parsing.go index dfe29fe..777ef8e 100644 --- a/pkg/oyafile/parsing.go +++ b/pkg/oyafile/parsing.go @@ -13,6 +13,7 @@ import ( ) const StmtImport = "Import" +const StmtExpose = "Expose" const StmtValues = "Values" const StmtProject = "Project" const StmtIgnore = "Ignore" @@ -80,6 +81,8 @@ func parseStatement(name string, value interface{}, o *Oyafile) error { return parseRequire(value, o) case StmtReplace: return parseReplace(value, o) + case StmtExpose: + return parseExpose(value, o) default: taskName := task.Name(name) @@ -244,3 +247,13 @@ func parseReplace(value interface{}, o *Oyafile) error { return nil } + +func parseExpose(value interface{}, o *Oyafile) error { + aliasStr, ok := value.(string) + if !ok { + return fmt.Errorf("expected an import alias") + } + alias := types.Alias(aliasStr) + o.ExposedAliases = []types.Alias{alias} + return nil +} diff --git a/pkg/task/mocks.go b/pkg/task/mocks.go new file mode 100644 index 0000000..3a89b6f --- /dev/null +++ b/pkg/task/mocks.go @@ -0,0 +1,13 @@ +package task + +import ( + "io" + + "github.com/tooploox/oya/pkg/template" +) + +type MockTask struct{} + +func (MockTask) Exec(workDir string, args []string, scope template.Scope, stdout, stderr io.Writer) error { + return nil +} diff --git a/pkg/task/name.go b/pkg/task/name.go index d783eff..aa13196 100644 --- a/pkg/task/name.go +++ b/pkg/task/name.go @@ -23,6 +23,17 @@ func (n Name) Split() (types.Alias, string) { } } +func (n Name) IsAliased(alias types.Alias) bool { + // TODO(bilus): Turn Name into a a struct. + actualAlias, _ := n.Split() + return actualAlias == alias +} + +func (n Name) Unaliased() Name { + _, s := n.Split() + return Name(s) +} + func (n Name) IsBuiltIn() bool { firstChar := string(n)[0:1] return firstChar == strings.ToUpper(firstChar) diff --git a/pkg/task/name_test.go b/pkg/task/name_test.go index e7afbe9..a2c4f34 100644 --- a/pkg/task/name_test.go +++ b/pkg/task/name_test.go @@ -28,3 +28,26 @@ func TestName_Split_WithNestedAlias(t *testing.T) { tu.AssertEqual(t, types.Alias("foo.bar"), alias) tu.AssertEqual(t, "zoo", task) } + +func TestName_IsAliased(t *testing.T) { + testCases := []struct { + name task.Name + alias types.Alias + isAliased bool + }{ + {task.Name("name"), types.Alias("alias"), false}, + {task.Name("name").Aliased("alias"), types.Alias("alias"), true}, + {task.Name("name").Aliased("otherAlias"), types.Alias("alias"), false}, + } + + for _, tc := range testCases { + tu.AssertEqual(t, tc.isAliased, tc.name.IsAliased(tc.alias)) + } +} + +func TestName_Unaliased(t *testing.T) { + globalName := task.Name("name") + aliasedName := globalName.Aliased(types.Alias("alias")) + tu.AssertEqual(t, globalName, aliasedName.Unaliased()) + tu.AssertEqual(t, globalName, globalName.Unaliased()) +} diff --git a/pkg/task/table.go b/pkg/task/table.go index 0adbd8c..6b6237d 100644 --- a/pkg/task/table.go +++ b/pkg/task/table.go @@ -1,8 +1,6 @@ package task import ( - "fmt" - "github.com/tooploox/oya/pkg/types" ) @@ -34,12 +32,31 @@ func (tt Table) AddDoc(taskName Name, s string) { } func (tt Table) ImportTasks(alias types.Alias, other Table) { - for key, t := range other.tasks { + for name, t := range other.tasks { // TODO: Detect if task already set. - tt.AddTask(Name(fmt.Sprintf("%v.%v", alias, key)), t) + tt.AddTask(Name(name).Aliased(alias), t) + } +} + +// Expose copies tasks under an alias to global scope (without the alias) +// never overriding the existing global tasks. +func (tt Table) Expose(alias types.Alias) { + for name, task := range tt.tasks { + if name.IsAliased(alias) { + globalName := name.Unaliased() + _, ok := tt.LookupTask(globalName) + if !ok { + tt.addTaskWithMeta(globalName, task, tt.meta[name]) + } + } } } +func (tt Table) addTaskWithMeta(name Name, task Task, meta Meta) { + tt.tasks[name] = task + tt.meta[name] = meta +} + func (tt Table) ForEach(f func(taskName Name, task Task, meta Meta) error) error { for taskName, task := range tt.tasks { meta := tt.meta[taskName] diff --git a/pkg/task/table_test.go b/pkg/task/table_test.go new file mode 100644 index 0000000..5591386 --- /dev/null +++ b/pkg/task/table_test.go @@ -0,0 +1,24 @@ +package task_test + +import ( + "testing" + + "github.com/tooploox/oya/pkg/task" + "github.com/tooploox/oya/pkg/types" + tu "github.com/tooploox/oya/testutil" +) + +func TestTable_Expose(t *testing.T) { + tt := task.NewTable() + + globalName := task.Name("task") + aliasedName := globalName.Aliased("alias") + tt.AddTask(aliasedName, task.MockTask{}) + + tt.Expose(types.Alias("alias")) + + _, ok := tt.LookupTask(aliasedName) + tu.AssertTrue(t, ok, "Aliased task not found") + _, ok = tt.LookupTask(globalName) + tu.AssertTrue(t, ok, "Global task not found") +} diff --git a/todo.org b/todo.org index 81231d8..ab4a7b2 100644 --- a/todo.org +++ b/todo.org @@ -59,6 +59,11 @@ - State "CANCELLED" from "TODO" [2019-01-25 Fri 10:34] \\ It's enough to have "install" task by convention in packs and then oya tasks will show it. :END: +* TODO Experiment: exposing tasks +** TODO PoC +** TODO Flag for go import +** TODO Multiple packages? +** TODO Transient exposure (package exposing another package) * TODO Vendoring is only partially implemented ** TODO Simplify oya get/vendor (based on Import statements) TBD **** Just use Import From db73c24ebaabeff014288dc3e31e2897a4ec5ba7 Mon Sep 17 00:00:00 2001 From: Marcin Bilski Date: Sun, 24 Oct 2021 21:42:15 +0200 Subject: [PATCH 2/9] Add a `--expose` flag to the `oya import` command. --- cmd/import.go | 7 +++- cmd/internal/import.go | 12 ++++++- features/import.feature | 42 +++++++++++++++++++++++ pkg/project/packs.go | 8 +++-- pkg/raw/expose.go | 31 +++++++++++++++++ pkg/raw/fixtures/Expose_FirstTime/Oyafile | 3 ++ pkg/raw/imports.go | 25 ++++++-------- pkg/raw/imports_test.go | 36 +++++++++++++++++++ pkg/raw/patching.go | 18 +++++----- pkg/raw/requires.go | 5 +-- pkg/raw/requires_test.go | 14 ++++++++ 11 files changed, 170 insertions(+), 31 deletions(-) create mode 100644 pkg/raw/expose.go create mode 100644 pkg/raw/fixtures/Expose_FirstTime/Oyafile diff --git a/cmd/import.go b/cmd/import.go index 18346c3..c41a9c2 100644 --- a/cmd/import.go +++ b/cmd/import.go @@ -20,11 +20,16 @@ var importCmd = &cobra.Command{ if err != nil { return err } - return internal.Import(cwd, args[0], alias, cmd.OutOrStdout(), cmd.OutOrStderr()) + expose, err := cmd.Flags().GetBool("expose") + if err != nil { + return err + } + return internal.Import(cwd, args[0], alias, expose, cmd.OutOrStdout(), cmd.OutOrStderr()) }, } func init() { rootCmd.AddCommand(importCmd) importCmd.Flags().StringP("alias", "a", "", "Import pack under alias name") + importCmd.Flags().BoolP("expose", "e", false, "Expose imported tasks") } diff --git a/cmd/internal/import.go b/cmd/internal/import.go index bc25331..37741f6 100644 --- a/cmd/internal/import.go +++ b/cmd/internal/import.go @@ -10,7 +10,7 @@ import ( "github.com/tooploox/oya/pkg/raw" ) -func Import(workDir, uri, alias string, stdout, stderr io.Writer) error { +func Import(workDir, uri, alias string, exposeTasks bool, stdout, stderr io.Writer) error { if alias == "" { uriArr := strings.Split(uri, "/") alias = strcase.ToLowerCamel(uriArr[len(uriArr)-1]) @@ -37,5 +37,15 @@ func Import(workDir, uri, alias string, stdout, stderr io.Writer) error { return err } + if exposeTasks { + if err := raw.Expose(alias); err != nil { + return err + } + } + + if err := raw.ApplyChanges(); err != nil { + return err + } + return proj.InstallPacks() } diff --git a/features/import.feature b/features/import.feature index e76c9e4..0535816 100644 --- a/features/import.feature +++ b/features/import.feature @@ -138,3 +138,45 @@ Scenario: Import a pack with alias from a parameter pack3_5: github.com/tooploox/oya-fixtures/pack3-and-a-half """ + +@current +Scenario: Import a pack and expose it + Given file ./Oyafile containing + """ + Project: project + """ + When I run "oya import github.com/tooploox/oya-fixtures/pack3-and-a-half --expose" + Then the command succeeds + And file ./Oyafile contains + """ + Project: project + Require: + github.com/tooploox/oya-fixtures/pack3-and-a-half: v1.1.0 + Expose: pack3AndAHalf + Import: + pack3AndAHalf: github.com/tooploox/oya-fixtures/pack3-and-a-half + + """ + +@current +Scenario: Try to expose two packs + Given file ./Oyafile containing + """ + Project: project + """ + When I run "oya import github.com/tooploox/oya-fixtures/pack1 --expose" + And I run "oya import github.com/tooploox/oya-fixtures/pack2 --expose" + Then the command fails with error matching + """ + .*already exposed.* + """ + And file ./Oyafile contains + """ + Project: project + Require: + github.com/tooploox/oya-fixtures/pack1: v1.1.1 + Expose: pack1 + Import: + pack1: github.com/tooploox/oya-fixtures/pack1 + + """ diff --git a/pkg/project/packs.go b/pkg/project/packs.go index 5cf177d..090783a 100644 --- a/pkg/project/packs.go +++ b/pkg/project/packs.go @@ -19,10 +19,14 @@ func (p *Project) Require(pack pack.Pack) error { return ErrNoOyafile{Path: p.RootDir} } - err = raw.AddRequire(pack) - if err != nil { + if err := raw.AddRequire(pack); err != nil { return err } + + if err := raw.ApplyChanges(); err != nil { + return err + } + p.invalidateOyafileCache(p.RootDir) p.dependencies = nil // Force reload. diff --git a/pkg/raw/expose.go b/pkg/raw/expose.go new file mode 100644 index 0000000..cc5ccaa --- /dev/null +++ b/pkg/raw/expose.go @@ -0,0 +1,31 @@ +package raw + +import ( + "errors" + "fmt" + "regexp" +) + +var exposeStmt = "Expose: %s" +var exposeRegexp = regexp.MustCompile("(?m)^Expose:.+$") + +func (raw *Oyafile) Expose(alias string) error { + if raw.isAlreadyExposed() { + return errors.New("an import is already exposed") + } + return raw.expose(alias) +} + +func (raw *Oyafile) expose(alias string) error { + stmt := fmt.Sprintf(exposeStmt, alias) + if updated, err := raw.insertAfter(projectRegexp, stmt); err != nil || updated { + return err // nil if updated + } + return raw.prepend( + stmt, + ) +} + +func (raw *Oyafile) isAlreadyExposed() bool { + return raw.matches(exposeRegexp) +} diff --git a/pkg/raw/fixtures/Expose_FirstTime/Oyafile b/pkg/raw/fixtures/Expose_FirstTime/Oyafile new file mode 100644 index 0000000..ef09ea5 --- /dev/null +++ b/pkg/raw/fixtures/Expose_FirstTime/Oyafile @@ -0,0 +1,3 @@ +Project: Expose_FirstTime +Import: + foo: github.com/tooploox/foo diff --git a/pkg/raw/imports.go b/pkg/raw/imports.go index ab285af..0d50ef2 100644 --- a/pkg/raw/imports.go +++ b/pkg/raw/imports.go @@ -7,23 +7,18 @@ import ( "github.com/pkg/errors" ) -var importKey = "Import:" -var projectKey = "Project:" +var importStmt = "Import:" +var projectStmt = "Project:" var uriVal = " %s: %s" -var importRegexp = regexp.MustCompile("(?m)^" + importKey + "$") -var projectRegexp = regexp.MustCompile("^" + projectKey) +var importRegexp = regexp.MustCompile("(?m)^" + importStmt + "$") +var projectRegexp = regexp.MustCompile("^" + projectStmt) func (raw *Oyafile) AddImport(alias, uri string) error { if gotIt := raw.isAlreadyImported(uri); gotIt { return errors.Errorf("Pack already imported: %v", uri) } - err := raw.addImport(alias, uri) - if err != nil { - return err - } - - return raw.write() + return raw.addImport(alias, uri) } func (raw *Oyafile) addImport(alias, uri string) error { @@ -31,18 +26,20 @@ func (raw *Oyafile) addImport(alias, uri string) error { if updated, err := raw.insertAfter(importRegexp, uriStr); err != nil || updated { return err // nil if updated } - if updated, err := raw.insertAfter(projectRegexp, importKey, uriStr); err != nil || updated { + if updated, err := raw.insertAfter(projectRegexp, importStmt, uriStr); err != nil || updated { return err // nil if updated } - return raw.prepend( - importKey, + importStmt, uriStr, ) } func (raw *Oyafile) isAlreadyImported(uri string) bool { - // BUG(bilus): This is slightly brittle because it will match any line that ends with the uri. It might be a good idea to add a raw.(*Oyafile).matchesWithin function along the lines of how insertBeforeWithin is written. + // BUG(bilus): This is slightly brittle because it will match any line that + // ends with the uri. It might be a good idea to add a + // raw.(*Oyafile).matchesWithin function along the lines of how + // insertBeforeWithin is written. rx := regexp.MustCompile("(?m)" + uri + "$") return raw.matches(rx) } diff --git a/pkg/raw/imports_test.go b/pkg/raw/imports_test.go index 3b0e4f6..b0c9d36 100644 --- a/pkg/raw/imports_test.go +++ b/pkg/raw/imports_test.go @@ -25,6 +25,8 @@ func TestOyafile_AddImport_NoImport(t *testing.T) { err = raw.AddImport("foo", "github.com/tooploox/foo") tu.AssertNoErr(t, err, "Error adding import") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddImport Import: foo: github.com/tooploox/foo @@ -47,6 +49,8 @@ func TestOyafile_AddImport_ToExisting(t *testing.T) { err = raw.AddImport("bar", "github.com/tooploox/bar") tu.AssertNoErr(t, err, "Error adding import") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddImport_ToExisting Import: @@ -71,6 +75,8 @@ func TestOyafile_AddImport_MoreKeys(t *testing.T) { err = raw.AddImport("bar", "github.com/tooploox/bar") tu.AssertNoErr(t, err, "Error adding import") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddImport_MoreKeys Import: bar: github.com/tooploox/bar @@ -98,6 +104,8 @@ func TestOyafile_AddImport_TwoPacks(t *testing.T) { err = raw.AddImport("bar", "github.com/tooploox/bar") tu.AssertNoErr(t, err, "Error adding import") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddImport Import: bar: github.com/tooploox/bar @@ -121,6 +129,8 @@ func TestOyafile_AddImport_AlreadyImported(t *testing.T) { err = raw.AddImport("foo", "github.com/tooploox/foo") tu.AssertErr(t, err, "Expected an error trying to import the same pack twice") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddImport_ToExisting Import: @@ -128,3 +138,29 @@ Import: ` tu.AssertFileContains(t, oyafilePath, expectedContent) } + +func TestOyafile_Expose_FirstTime(t *testing.T) { + outputDir, err := ioutil.TempDir("", "oya") + tu.AssertNoErr(t, err, "Error creating temporary output dir") + defer os.RemoveAll(outputDir) + + oyafilePath := filepath.Join(outputDir, "Oyafile") + tu.MustCopyFile(t, "./fixtures/Expose_FirstTime/Oyafile", oyafilePath) + + raw, found, err := raw.Load(oyafilePath, oyafilePath) + tu.AssertNoErr(t, err, "Error loading raw Oyafile") + tu.AssertTrue(t, found, "No Oyafile found") + + err = raw.Expose("foo") + tu.AssertNoErr(t, err, "Error adding import") + + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + + expectedContent := `Project: Expose_FirstTime +Expose: foo +Import: + foo: github.com/tooploox/foo +` + tu.AssertFileContains(t, oyafilePath, expectedContent) + +} diff --git a/pkg/raw/patching.go b/pkg/raw/patching.go index fb4d801..f16909c 100644 --- a/pkg/raw/patching.go +++ b/pkg/raw/patching.go @@ -10,6 +10,15 @@ import ( "strings" ) +// ApplyChanges flushes in-memory Oyafile contents to disk. +func (raw *Oyafile) ApplyChanges() error { + info, err := os.Stat(raw.Path) + if err != nil { + return err + } + return ioutil.WriteFile(raw.Path, raw.oyafileContents, info.Mode()) +} + // flatMap maps each line of Oyafile to possibly multiple lines flattening the result. Does not write to the file. func (raw *Oyafile) flatMap(f func(line string) []string) error { scanner := bufio.NewScanner(bytes.NewReader(raw.oyafileContents)) @@ -152,15 +161,6 @@ func (raw *Oyafile) matches(rx *regexp.Regexp) bool { return rx.MatchString(string(raw.oyafileContents)) } -// write flushes in-memory Oyafile contents to disk. -func (raw *Oyafile) write() error { - info, err := os.Stat(raw.Path) - if err != nil { - return err - } - return ioutil.WriteFile(raw.Path, raw.oyafileContents, info.Mode()) -} - func indent(lines []string, level int) []string { indented := make([]string, len(lines)) for i, line := range lines { diff --git a/pkg/raw/requires.go b/pkg/raw/requires.go index 01838fd..3522869 100644 --- a/pkg/raw/requires.go +++ b/pkg/raw/requires.go @@ -30,10 +30,7 @@ var defaultIndent = 2 // AddRequire adds a Require: entry for the pack. func (raw *Oyafile) AddRequire(pack Pack) error { - if err := raw.addRequire(pack); err != nil { - return err - } - return raw.write() + return raw.addRequire(pack) } // addRequire adds a Require: entry for a pack using the following algorithm: diff --git a/pkg/raw/requires_test.go b/pkg/raw/requires_test.go index ea624e4..b407149 100644 --- a/pkg/raw/requires_test.go +++ b/pkg/raw/requires_test.go @@ -26,6 +26,8 @@ func TestOyafile_AddRequire_NoRequire(t *testing.T) { err = raw.AddRequire(pack) tu.AssertNoErr(t, err, "Error adding require") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddRequire Require: github.com/tooploox/foo: v1.0.0 @@ -49,6 +51,8 @@ func TestOyafile_AddRequire_EmptyRequire(t *testing.T) { err = raw.AddRequire(pack) tu.AssertNoErr(t, err, "Error adding require") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddRequire_EmptyRequire Require: @@ -73,6 +77,8 @@ func TestOyafile_AddRequire_ExistingRequire(t *testing.T) { err = raw.AddRequire(pack) tu.AssertNoErr(t, err, "Error adding require") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddRequire_ExistingRequire Require: @@ -98,6 +104,8 @@ func TestOyafile_AddRequire_SameVersion(t *testing.T) { err = raw.AddRequire(pack) tu.AssertNoErr(t, err, "Error adding require") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddRequire_ExistingRequire Require: @@ -122,6 +130,8 @@ func TestOyafile_AddRequire_DifferentVersion(t *testing.T) { err = raw.AddRequire(pack) tu.AssertNoErr(t, err, "Error adding require") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddRequire_ExistingRequire Require: @@ -146,6 +156,8 @@ func TestOyafile_AddRequire_MoreKeys(t *testing.T) { err = raw.AddRequire(pack) tu.AssertNoErr(t, err, "Error adding require") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddRequire_MoreKeys Require: github.com/tooploox/bar: v1.1.0 @@ -172,6 +184,8 @@ func TestOyafile_AddRequire_DifferentVersionMoreKeys(t *testing.T) { err = raw.AddRequire(pack) tu.AssertNoErr(t, err, "Error adding require") + tu.AssertNoErr(t, raw.ApplyChanges(), "Error applying changes") + expectedContent := `Project: AddRequire_ExistingMoreKeys # It's before Require on purpose; we want to test whether matching From e849b94d4d9cc4e0b521a2b2aa8d90ee93b1f367 Mon Sep 17 00:00:00 2001 From: Marcin Bilski Date: Sun, 24 Oct 2021 21:45:46 +0200 Subject: [PATCH 3/9] Update the changelog. --- CHANGELOG.md | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d470d7..7d6aa42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,29 @@ - Ensure non-zero exit code from a command in Oya tasks, including sub-commands, propagates to the shell invoking `oya run`, even without `set -e`. - -### Added + +### Added - REPL, helping build scripts interactively with access to values in .oya files and auto-completion, started using `oya repl`, an example session: - + oya run repl $ echo ${Oya[someValue]} foobar - + +- Added `Expose` statement, pulling imported tasks into global scope, for example: + + Project: someproject + + Import: + bar: github.com/foo/bar + + Expose: bar + + With this command the imported tasks are available both under the `bar` alias + (e.g. `oya run bar.doSomething`) as well as without it (e.g. `oya run + doSomething`). + + The `oya import` command now has a flag, to expose the import, for example: + + oya import github.com/foo/bar --expose From c5414166e07f231ebdcf15cb7e13ce57f2a5a706 Mon Sep 17 00:00:00 2001 From: Marcin Bilski Date: Sun, 24 Oct 2021 21:46:29 +0200 Subject: [PATCH 4/9] Update the todo. --- todo.org | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/todo.org b/todo.org index ab4a7b2..346552b 100644 --- a/todo.org +++ b/todo.org @@ -60,9 +60,11 @@ It's enough to have "install" task by convention in packs and then oya tasks will show it. :END: * TODO Experiment: exposing tasks -** TODO PoC -** TODO Flag for go import -** TODO Multiple packages? +** DONE PoC +** DONE Flag for go import +** DONE Multiple packages? +Not for now. +** TODO Better handling by oya tasks ** TODO Transient exposure (package exposing another package) * TODO Vendoring is only partially implemented ** TODO Simplify oya get/vendor (based on Import statements) TBD From 16c60c1656647fd093c297e2e2907edb30d1b83c Mon Sep 17 00:00:00 2001 From: Marcin Bilski Date: Tue, 26 Oct 2021 10:56:19 +0200 Subject: [PATCH 5/9] Extract & improve task list formatting (`oya tasks`). --- cmd/internal/printers/tasks.go | 132 ++++++++++++++++++++++++ cmd/internal/printers/tasks_test.go | 153 ++++++++++++++++++++++++++++ cmd/internal/tasks.go | 29 +----- features/tasks.feature | 37 ++++--- todo.org | 3 + 5 files changed, 315 insertions(+), 39 deletions(-) create mode 100644 cmd/internal/printers/tasks.go create mode 100644 cmd/internal/printers/tasks_test.go diff --git a/cmd/internal/printers/tasks.go b/cmd/internal/printers/tasks.go new file mode 100644 index 0000000..019e77b --- /dev/null +++ b/cmd/internal/printers/tasks.go @@ -0,0 +1,132 @@ +package printers + +import ( + "fmt" + "io" + "path" + "path/filepath" + "sort" + "strings" + + "github.com/tooploox/oya/pkg/task" + "github.com/tooploox/oya/pkg/types" +) + +type taskInfo struct { + taskName task.Name + alias types.Alias + bareTaskName string + meta task.Meta + relOyafilePath string +} + +type TaskList struct { + workDir string + tasks []taskInfo +} + +func NewTaskList(workDir string) *TaskList { + return &TaskList{ + workDir: workDir, + } +} + +func (p *TaskList) AddTask(taskName task.Name, meta task.Meta, oyafilePath string) error { + alias, bareTaskName := taskName.Split() + relPath, err := filepath.Rel(p.workDir, oyafilePath) + if err != nil { + return err + } + + p.tasks = append(p.tasks, taskInfo{taskName, alias, bareTaskName, meta, relPath}) + return nil +} + +func (p *TaskList) Print(w io.Writer) { + sortTasks(p.tasks) + + printTask := p.taskPrinter() + + lastRelPath := "" + first := true + for _, t := range p.tasks { + if t.relOyafilePath != lastRelPath { + if !first { + fmt.Fprintln(w) + } else { + first = false + } + + fmt.Fprintf(w, "# in ./%s\n", t.relOyafilePath) + } + printTask(w, t.taskName, t.meta) + lastRelPath = t.relOyafilePath + } +} + +func (p *TaskList) taskPrinter() func(io.Writer, task.Name, task.Meta) { + docOffset := maxTaskWidth(p.tasks) + return func(w io.Writer, taskName task.Name, meta task.Meta) { + fmt.Fprintf(w, "oya run %s", taskName) + if len(meta.Doc) > 0 { + padding := strings.Repeat(" ", docOffset-len(taskName)) + fmt.Fprintf(w, "%s # %s", padding, meta.Doc) + } + fmt.Fprintln(w) + } +} + +func maxTaskWidth(tasks []taskInfo) int { + w := 0 + for _, t := range tasks { + l := len(string(t.taskName)) + if l > w { + w = l + } + } + return w +} + +func isParentPath(p1, p2 string) bool { + relPath, _ := filepath.Rel(p2, p1) + return strings.Contains(relPath, "../") +} + +func sortTasks(tasks []taskInfo) { + sort.SliceStable(tasks, func(i, j int) bool { + lt := tasks[i] + rt := tasks[j] + + ldir := path.Dir(lt.relOyafilePath) + rdir := path.Dir(rt.relOyafilePath) + + // Top-level tasks go before tasks in subdirectories. + if isParentPath(ldir, rdir) { + return true + } + if isParentPath(rdir, ldir) { + return false + } + + if rdir == ldir { + if len(lt.alias) == 0 && len(rt.alias) != 0 { + return true + } + if len(lt.alias) != 0 && len(rt.alias) == 0 { + return false + } + // Tasks w/o alias before tasks with alias, + // sort aliases alphabetically. + if lt.alias < rt.alias { + return true + } + + // Sort tasks alphabetically. + if lt.bareTaskName < rt.bareTaskName { + return true + } + } + + return false + }) +} diff --git a/cmd/internal/printers/tasks_test.go b/cmd/internal/printers/tasks_test.go new file mode 100644 index 0000000..587bffc --- /dev/null +++ b/cmd/internal/printers/tasks_test.go @@ -0,0 +1,153 @@ +package printers_test + +import ( + "strings" + "testing" + + "github.com/tooploox/oya/cmd/internal/printers" + "github.com/tooploox/oya/pkg/task" + "github.com/tooploox/oya/pkg/types" + tu "github.com/tooploox/oya/testutil" +) + +type mockWriter struct { + bs []byte +} + +func (w *mockWriter) Write(p []byte) (n int, err error) { + w.bs = append(w.bs, p...) + return len(p), nil +} + +func (w *mockWriter) Lines() []string { + if len(w.bs) == 0 { + return []string{} + } + return strings.Split(string(w.bs), "\n") +} + +func TestTaskList(t *testing.T) { + type taskDef struct { + name task.Name + meta task.Meta + oyafilePath string + } + testCases := []struct { + desc string + workDir string + tasks []taskDef + expectedOutput []string + }{ + { + desc: "no tasks", + workDir: "/project/", + tasks: nil, + expectedOutput: []string{}, + }, + { + desc: "one global task", + workDir: "/project/", + tasks: []taskDef{ + {task.Name("task1"), task.Meta{}, "/project/Oyafile"}, + }, + expectedOutput: []string{ + "# in ./Oyafile", + "oya run task1", + "", + }, + }, + { + desc: "global tasks before imported tasks", + workDir: "/project/", + tasks: []taskDef{ + {task.Name("task1"), task.Meta{}, "/project/Oyafile"}, + {task.Name("othertask"), task.Meta{}, "/project/Oyafile"}, + {task.Name("task2").Aliased(types.Alias("pack")), task.Meta{}, "/project/Oyafile"}, + {task.Name("task3").Aliased(types.Alias("pack")), task.Meta{}, "/project/Oyafile"}, + }, + expectedOutput: []string{ + "# in ./Oyafile", + "oya run othertask", + "oya run task1", + "oya run pack.task2", + "oya run pack.task3", + "", + }, + }, + { + desc: "top-level tasks before tasks in subdirectories", + workDir: "/project/", + tasks: []taskDef{ + {task.Name("task1"), task.Meta{}, "/project/Oyafile"}, + {task.Name("otherTask"), task.Meta{}, "/project/Oyafile"}, + {task.Name("aTask"), task.Meta{}, "/project/subdir/Oyafile"}, + {task.Name("task3"), task.Meta{}, "/project/subdir/Oyafile"}, + }, + expectedOutput: []string{ + "# in ./Oyafile", + "oya run otherTask", + "oya run task1", + "", + "# in ./subdir/Oyafile", + "oya run aTask", + "oya run task3", + "", + }, + }, + { + desc: "sort aliases and task names alphabetically", + workDir: "/project/", + tasks: []taskDef{ + {task.Name("yyy"), task.Meta{}, "/project/Oyafile"}, + {task.Name("zzz"), task.Meta{}, "/project/Oyafile"}, + {task.Name("aaa").Aliased(types.Alias("BBB")), task.Meta{}, "/project/Oyafile"}, + {task.Name("ddd").Aliased(types.Alias("AAA")), task.Meta{}, "/project/Oyafile"}, + {task.Name("ccc").Aliased(types.Alias("AAA")), task.Meta{}, "/project/Oyafile"}, + }, + expectedOutput: []string{ + "# in ./Oyafile", + "oya run yyy", + "oya run zzz", + "oya run AAA.ccc", + "oya run AAA.ddd", + "oya run BBB.aaa", + "", + }, + }, + { + desc: "task descriptions are aligned", + workDir: "/project/", + tasks: []taskDef{ + {task.Name("y"), task.Meta{Doc: "a description"}, "/project/Oyafile"}, + {task.Name("zzz"), task.Meta{Doc: "another description"}, "/project/Oyafile"}, + {task.Name("aaa").Aliased(types.Alias("BBB")), task.Meta{Doc: "task aa"}, "/project/Oyafile"}, + {task.Name("ddddd").Aliased(types.Alias("AAA")), task.Meta{}, "/project/Oyafile"}, + {task.Name("ccc").Aliased(types.Alias("AAA")), task.Meta{}, "/project/Oyafile"}, + }, + expectedOutput: []string{ + "# in ./Oyafile", + "oya run y # a description", + "oya run zzz # another description", + "oya run AAA.ccc", + "oya run AAA.ddddd", + "oya run BBB.aaa # task aa", + "", + }, + }, + // NEXT: Exposed + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + tl := printers.NewTaskList(tc.workDir) + for _, task := range tc.tasks { + tu.AssertNoErr(t, tl.AddTask(task.name, task.meta, task.oyafilePath), "Adding task failed") + + } + w := mockWriter{} + tl.Print(&w) + tu.AssertObjectsEqual(t, tc.expectedOutput, w.Lines()) + + }) + } +} diff --git a/cmd/internal/tasks.go b/cmd/internal/tasks.go index 4b34b54..9f9c39a 100644 --- a/cmd/internal/tasks.go +++ b/cmd/internal/tasks.go @@ -1,11 +1,10 @@ package internal import ( - "fmt" "io" - "path/filepath" "text/tabwriter" + "github.com/tooploox/oya/cmd/internal/printers" "github.com/tooploox/oya/pkg/project" "github.com/tooploox/oya/pkg/task" ) @@ -28,32 +27,12 @@ func Tasks(workDir string, recurse, changeset bool, stdout, stderr io.Writer) er return err } - first := true - + printer := printers.NewTaskList(workDir) err = p.ForEachTask(workDir, recurse, changeset, false, // No built-ins. func(i int, oyafilePath string, taskName task.Name, task task.Task, meta task.Meta) error { - if i == 0 { - relPath, err := filepath.Rel(workDir, oyafilePath) - if err != nil { - return err - } - if !first { - fmt.Fprintln(w) - } else { - first = false - } - - fmt.Fprintf(w, "# in ./%s\n", relPath) - } - - if len(meta.Doc) > 0 { - fmt.Fprintf(w, "oya run %s\t# %s\n", taskName, meta.Doc) - } else { - fmt.Fprintf(w, "oya run %s\t\n", taskName) - } - - return nil + return printer.AddTask(taskName, meta, oyafilePath) }) + printer.Print(w) w.Flush() return err } diff --git a/features/tasks.feature b/features/tasks.feature index 2f3bd1a..0ddd90e 100644 --- a/features/tasks.feature +++ b/features/tasks.feature @@ -15,7 +15,7 @@ Scenario: Single Oyafile And the command outputs text matching """ # in ./Oyafile - oya run build\s+ + oya run build """ @@ -32,7 +32,7 @@ Scenario: Show only user-defined And the command outputs text matching """ # in ./Oyafile - oya run build\s+ + oya run build """ @@ -53,10 +53,11 @@ Scenario: Subdirectories are not recursed by default And the command outputs text matching """ # in ./Oyafile - oya run build\s+ + oya run build """ +@current Scenario: Subdirectories can be recursed Given file ./Oyafile containing """ @@ -64,7 +65,12 @@ Scenario: Subdirectories can be recursed build: | echo "Done" """ - And file ./subdir1/Oyafile containing + And file ./subdir/Oyafile containing + """ + build: | + echo "Done" + """ + And file ./another_subdir/Oyafile containing """ build: | echo "Done" @@ -74,10 +80,13 @@ Scenario: Subdirectories can be recursed And the command outputs text matching """ # in ./Oyafile - oya run build\s+ + oya run build - # in ./subdir1/Oyafile - oya run build\s+ + # in ./subdir/Oyafile + oya run build + + # in ./another_subdir/Oyafile + oya run build """ @@ -96,7 +105,7 @@ Scenario: Docstring prints And the command outputs text matching """ # in ./Oyafile - oya run build # Build it.* + oya run build # Build it.* """ @@ -124,11 +133,11 @@ Scenario: Doc strings are properly aligned And the command outputs text matching """ # in ./Oyafile - oya run build # Build it.* - oya run testAll # Run all tests.* + oya run build # Build it.* + oya run testAll # Run all tests.* # in ./subdir1/Oyafile - oya run foo # Do foo + oya run foo # Do foo """ @@ -157,7 +166,7 @@ Scenario: Parent dir tasks are not listed And the command outputs text matching """ # in ./Oyafile - oya run foo # Do foo + oya run foo # Do foo """ @@ -185,7 +194,7 @@ Scenario: Imported packs tasks are listed And the command outputs text matching """ # in ./Oyafile - oya run foo.packTask\s+ - oya run test\s+ + oya run test + oya run foo.packTask """ diff --git a/todo.org b/todo.org index 346552b..6d53bfb 100644 --- a/todo.org +++ b/todo.org @@ -65,6 +65,9 @@ ** DONE Multiple packages? Not for now. ** TODO Better handling by oya tasks +*** TODO Show global tasks first +*** TODO Group aliased tasks +*** TODO Show global tasks pointing to exposed tasks ** TODO Transient exposure (package exposing another package) * TODO Vendoring is only partially implemented ** TODO Simplify oya get/vendor (based on Import statements) TBD From 4fd9f27f80d8e38c5966da70290bcf5ea70dba92 Mon Sep 17 00:00:00 2001 From: Marcin Bilski Date: Wed, 27 Oct 2021 01:04:10 +0200 Subject: [PATCH 6/9] Show diff when matching command output in tests for easier debugging. --- go.mod | 1 + go.sum | 2 ++ oya_test.go | 6 ++++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index ff00108..3d865de 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/Masterminds/goutils v1.1.0 // indirect github.com/Masterminds/semver v1.4.2 // indirect github.com/Masterminds/sprig v2.18.0+incompatible + github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883 github.com/blang/semver v3.5.1+incompatible github.com/c-bata/go-prompt v0.2.4-0.20200321140817-d043be076398 github.com/cucumber/godog v0.12.2 diff --git a/go.sum b/go.sum index a8ca81f..86b753f 100644 --- a/go.sum +++ b/go.sum @@ -50,6 +50,8 @@ github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7 h1:uSoVVbwJiQipAclBb github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7/go.mod h1:6zEj6s6u/ghQa61ZWa/C2Aw3RkjiTBOix7dkqa1VLIs= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= +github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883 h1:bvNMNQO63//z+xNgfBlViaCIJKLlCJ6/fmUseuG0wVQ= +github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= diff --git a/oya_test.go b/oya_test.go index 58eea44..30e2a14 100644 --- a/oya_test.go +++ b/oya_test.go @@ -11,6 +11,7 @@ import ( "runtime" "strings" + "github.com/andreyvit/diff" "github.com/cucumber/godog" "github.com/pkg/errors" log "github.com/sirupsen/logrus" @@ -281,7 +282,8 @@ func (c *SuiteContext) theCommandFailsWithErrorMatching(errMsg *gherkin.DocStrin func (c *SuiteContext) theCommandOutputs(expected *gherkin.DocString) error { actual := c.stdout.String() if actual != expected.Content { - return fmt.Errorf("unexpected %v; expected: %q", actual, expected.Content) + return fmt.Errorf("unexpected %v; expected: %v; diff: %v", + actual, expected.Content, diff.LineDiff(actual, expected.Content)) } return nil } @@ -290,7 +292,7 @@ func (c *SuiteContext) theCommandOutputsTextMatching(expected *gherkin.DocString actual := c.stdout.String() rx := regexp.MustCompile(expected.Content) if !rx.MatchString(actual) { - return fmt.Errorf("unexpected %v; expected to match: %q", actual, expected.Content) + return fmt.Errorf("unexpected %v; expected to match: %v", actual, expected.Content) } return nil } From 3596cce45e1fee8967e26bebf86e738ebee5322f Mon Sep 17 00:00:00 2001 From: Marcin Bilski Date: Wed, 27 Oct 2021 01:05:12 +0200 Subject: [PATCH 7/9] Show information about exposed tasks when listing all tasks. --- cmd/internal/printers/tasks.go | 12 +++++++++-- cmd/internal/printers/tasks_test.go | 15 +++++++++++++- features/expose.feature | 31 ++++++++++++++++++++++++++++- pkg/task/meta.go | 14 +++++++++++++ pkg/task/table.go | 11 +++++++--- todo.org | 12 +++++++---- 6 files changed, 84 insertions(+), 11 deletions(-) diff --git a/cmd/internal/printers/tasks.go b/cmd/internal/printers/tasks.go index 019e77b..4ae5ec8 100644 --- a/cmd/internal/printers/tasks.go +++ b/cmd/internal/printers/tasks.go @@ -68,9 +68,17 @@ func (p *TaskList) taskPrinter() func(io.Writer, task.Name, task.Meta) { docOffset := maxTaskWidth(p.tasks) return func(w io.Writer, taskName task.Name, meta task.Meta) { fmt.Fprintf(w, "oya run %s", taskName) - if len(meta.Doc) > 0 { + exposed := meta.IsTaskExposed(taskName) + if len(meta.Doc) > 0 || exposed { padding := strings.Repeat(" ", docOffset-len(taskName)) - fmt.Fprintf(w, "%s # %s", padding, meta.Doc) + fmt.Fprintf(w, "%s #", padding) + if len(meta.Doc) > 0 { + fmt.Fprintf(w, " %s", meta.Doc) + } + + if exposed { + fmt.Fprintf(w, " (%s)", string(meta.OriginalTaskName)) + } } fmt.Fprintln(w) } diff --git a/cmd/internal/printers/tasks_test.go b/cmd/internal/printers/tasks_test.go index 587bffc..8685a2e 100644 --- a/cmd/internal/printers/tasks_test.go +++ b/cmd/internal/printers/tasks_test.go @@ -134,7 +134,20 @@ func TestTaskList(t *testing.T) { "", }, }, - // NEXT: Exposed + { + desc: "exposed tasks", + workDir: "/project/", + tasks: []taskDef{ + {task.Name("y"), task.Meta{Doc: "a description", OriginalTaskName: task.Name("y").Aliased("somepack")}, "/project/Oyafile"}, + {task.Name("z"), task.Meta{OriginalTaskName: task.Name("z").Aliased("somepack")}, "/project/Oyafile"}, + }, + expectedOutput: []string{ + "# in ./Oyafile", + "oya run y # a description (somepack.y)", + "oya run z # (somepack.z)", + "", + }, + }, } for _, tc := range testCases { diff --git a/features/expose.feature b/features/expose.feature index 6a2f8f4..bfeaae6 100644 --- a/features/expose.feature +++ b/features/expose.feature @@ -66,5 +66,34 @@ Scenario: Never overwrite existing an task when exposing tasks """ +@current +Scenario: Show task as exposed when listing tasks + Given file ./Oyafile containing + """ + Project: main + """ + And file ./project1/Oyafile containing + """ + Values: + foo: project1 -# TODO: Show as aliases when listing tasks. + echo: | + echo "project1" + """ + And file ./project2/Oyafile containing + """ + Import: + p: /project1 + + Expose: p + """ + And I'm in the ./project2 dir + When I run "oya tasks" + Then the command succeeds + And the command outputs + """ + # in ./Oyafile + oya run echo # (p.echo) + oya run p.echo + + """ diff --git a/pkg/task/meta.go b/pkg/task/meta.go index 33b142c..4f83274 100644 --- a/pkg/task/meta.go +++ b/pkg/task/meta.go @@ -2,4 +2,18 @@ package task type Meta struct { Doc string + + // OriginalTaskName, when different from the actual name associated with the + // task, contains the name of the original task from an imported package + // after it has been exposed, becoming accessible in the global scope as + // taskName. For example, task name is "deploy" while originalTaskName = + // "docker.deploy". + // + // For regular tasks it should either be equal to the task's actual name + // or left empty. + OriginalTaskName Name +} + +func (meta Meta) IsTaskExposed(taskName Name) bool { + return taskName != meta.OriginalTaskName && len(meta.OriginalTaskName) > 0 } diff --git a/pkg/task/table.go b/pkg/task/table.go index 6b6237d..e62f038 100644 --- a/pkg/task/table.go +++ b/pkg/task/table.go @@ -27,7 +27,8 @@ func (tt Table) AddTask(name Name, task Task) { func (tt Table) AddDoc(taskName Name, s string) { tt.meta[taskName] = Meta{ - Doc: s, + Doc: s, + OriginalTaskName: taskName, } } @@ -46,7 +47,9 @@ func (tt Table) Expose(alias types.Alias) { globalName := name.Unaliased() _, ok := tt.LookupTask(globalName) if !ok { - tt.addTaskWithMeta(globalName, task, tt.meta[name]) + meta := tt.meta[name] + meta.OriginalTaskName = name + tt.addTaskWithMeta(globalName, task, meta) } } } @@ -57,6 +60,7 @@ func (tt Table) addTaskWithMeta(name Name, task Task, meta Meta) { tt.meta[name] = meta } +// ForEach invokes the callback for each task. func (tt Table) ForEach(f func(taskName Name, task Task, meta Meta) error) error { for taskName, task := range tt.tasks { meta := tt.meta[taskName] @@ -67,13 +71,14 @@ func (tt Table) ForEach(f func(taskName Name, task Task, meta Meta) error) error return nil } +// ForEachSorted invokes the callback for each task, the tasks sorted alphabetically. func (tt Table) ForEachSorted(f func(taskName Name, task Task, meta Meta) error) error { taskNames := make([]Name, 0, len(tt.tasks)) for taskName := range tt.tasks { taskNames = append(taskNames, taskName) } - Sort(taskNames) + for _, taskName := range taskNames { task := tt.tasks[taskName] meta := tt.meta[taskName] diff --git a/todo.org b/todo.org index 6d53bfb..aea89f6 100644 --- a/todo.org +++ b/todo.org @@ -64,10 +64,14 @@ ** DONE Flag for go import ** DONE Multiple packages? Not for now. -** TODO Better handling by oya tasks -*** TODO Show global tasks first -*** TODO Group aliased tasks -*** TODO Show global tasks pointing to exposed tasks +** DONE Better handling by oya tasks +*** DONE Show global tasks first +*** DONE Group aliased tasks +*** DONE Show global tasks pointing to exposed tasks +Use Meta.OriginalTaskName +**** DONE Add to .feature +**** DONE Add to tasks_test.go + ** TODO Transient exposure (package exposing another package) * TODO Vendoring is only partially implemented ** TODO Simplify oya get/vendor (based on Import statements) TBD From 3cab0dd9aac8245b8538e02e9a779a0939a97d4a Mon Sep 17 00:00:00 2001 From: Marcin Bilski Date: Mon, 1 Nov 2021 01:20:47 +0100 Subject: [PATCH 8/9] Fix Replace not working in packs. --- CHANGELOG.md | 2 ++ features/development.feature | 38 ++++++++++++++++++++++++++++++++++++ pkg/deptree/internal/reqs.go | 10 ++++++++-- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d6aa42..e47dce4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ - Ensure non-zero exit code from a command in Oya tasks, including sub-commands, propagates to the shell invoking `oya run`, even without `set -e`. +- Fix `Replace` directive having no effect when used in imported packs. + ### Added - REPL, helping build scripts interactively with access to values in .oya files diff --git a/features/development.feature b/features/development.feature index ab47a60..e428eb9 100644 --- a/features/development.feature +++ b/features/development.feature @@ -55,3 +55,41 @@ Scenario: With local require oya doesn't attempt to lookup requirements remotely """ When I run "oya run foo.version" Then the command succeeds + +Scenario: Replace directive works in packs + Given file ./Oyafile containing + """ + Project: project + + Require: + github.com/tooploox/does-not-exist: v1.0.0 + + Replace: + github.com/tooploox/does-not-exist: /tmp/pack1 + + Import: + foo: github.com/tooploox/does-not-exist + """ + And file /tmp/pack1/Oyafile containing + """ + Project: pack1 + + Require: + github.com/tooploox/does-not-exist2: v1.0.0 + + Replace: + github.com/tooploox/does-not-exist2: /tmp/pack2 + + Import: + foo: github.com/tooploox/does-not-exist2 + + version: echo 1.0.0 + """ + And file /tmp/pack2/Oyafile containing + """ + Project: pack2 + + version: echo 1.0.0 + """ + When I run "oya run foo.version" + Then the command succeeds diff --git a/pkg/deptree/internal/reqs.go b/pkg/deptree/internal/reqs.go index 6747806..e37fa38 100644 --- a/pkg/deptree/internal/reqs.go +++ b/pkg/deptree/internal/reqs.go @@ -134,16 +134,22 @@ func (r *Reqs) remoteReqs(p pack.Pack) ([]pack.Pack, error) { return l.Reqs(p.Version()) } +// TODO(bilus): The same logic is in resolvePackReferences in packs.go func toPacks(references []oyafile.PackReference) ([]pack.Pack, error) { packs := make([]pack.Pack, len(references)) for i, reference := range references { - repo, err := repo.Open(reference.ImportPath) + l, err := repo.Open(reference.ImportPath) if err != nil { return nil, err } - if packs[i], err = repo.Version(reference.Version); err != nil { + pack, err := l.Version(reference.Version) + if err != nil { return nil, err } + if len(reference.ReplacementPath) > 0 { + pack = pack.LocalReplacement(reference.ReplacementPath) + } + packs[i] = pack } return packs, nil } From 880adbfb2771fabecd1aba8887edae4c67f44dca Mon Sep 17 00:00:00 2001 From: Marcin Bilski Date: Mon, 1 Nov 2021 01:29:00 +0100 Subject: [PATCH 9/9] Add a test, clean up. --- features/expose.feature | 103 +++++++++++++++++++++++++++++++++++++++- features/import.feature | 2 - features/tasks.feature | 1 - todo.org | 4 +- 4 files changed, 103 insertions(+), 7 deletions(-) diff --git a/features/expose.feature b/features/expose.feature index bfeaae6..efc0b14 100644 --- a/features/expose.feature +++ b/features/expose.feature @@ -33,7 +33,6 @@ Scenario: Expose tasks """ -@current Scenario: Never overwrite existing an task when exposing tasks Given file ./Oyafile containing """ @@ -66,7 +65,6 @@ Scenario: Never overwrite existing an task when exposing tasks """ -@current Scenario: Show task as exposed when listing tasks Given file ./Oyafile containing """ @@ -97,3 +95,104 @@ Scenario: Show task as exposed when listing tasks oya run p.echo """ + +Scenario: Tasks exposed in imported subdirs carry over when imported + Given file ./Oyafile containing + """ + Project: main + """ + And file ./project1/Oyafile containing + """ + Values: + foo: project1 + + echo: | + echo "project1" + """ + And file ./project2/Oyafile containing + """ + Import: + p1: /project1 + + Expose: p1 + """ + And file ./project3/Oyafile containing + """ + Import: + p2: /project2 + + Expose: p2 + """ + And I'm in the ./project3 dir + When I run "oya run echo" + Then the command succeeds + And the command outputs + """ + project1 + + """ + +Scenario: Tasks exposed in imported packs carry over when imported + Given file ./Oyafile containing + """ + Project: main + + Require: + github.com/tooploox/pack3: v1.0.0 + + # Use replace so we don't have to host it on github. + Replace: + github.com/tooploox/pack3: /tmp/pack3 + + Import: + pack3: github.com/tooploox/pack3 + + Expose: pack3 + """ + And file /tmp/pack1/Oyafile containing + """ + Project: github.com/tooploox/pack1 + + echo: | + echo "pack1" + """ + And file /tmp/pack2/Oyafile containing + """ + Project: github.com/tooploox/pack2 + + Require: + github.com/tooploox/pack1: v1.0.0 + + # Use replace so we don't have to host it on github. + Replace: + github.com/tooploox/pack1: /tmp/pack1 + + Import: + p1: github.com/tooploox/pack1 + + Expose: p1 + """ + And file /tmp/pack3/Oyafile containing + """ + Project: github.com/tooploox/pack3 + + Require: + github.com/tooploox/pack2: v1.0.0 + + # Use replace so we don't have to host it on github. + Replace: + github.com/tooploox/pack2: /tmp/pack2 + + Import: + p2: github.com/tooploox/pack2 + + Expose: p2 + """ + And I'm in the ./ dir + When I run "oya run echo" + Then the command succeeds + And the command outputs + """ + pack1 + + """ diff --git a/features/import.feature b/features/import.feature index 0535816..c597ed4 100644 --- a/features/import.feature +++ b/features/import.feature @@ -139,7 +139,6 @@ Scenario: Import a pack with alias from a parameter """ -@current Scenario: Import a pack and expose it Given file ./Oyafile containing """ @@ -158,7 +157,6 @@ Scenario: Import a pack and expose it """ -@current Scenario: Try to expose two packs Given file ./Oyafile containing """ diff --git a/features/tasks.feature b/features/tasks.feature index 0ddd90e..5052c10 100644 --- a/features/tasks.feature +++ b/features/tasks.feature @@ -57,7 +57,6 @@ Scenario: Subdirectories are not recursed by default """ -@current Scenario: Subdirectories can be recursed Given file ./Oyafile containing """ diff --git a/todo.org b/todo.org index aea89f6..14e4914 100644 --- a/todo.org +++ b/todo.org @@ -59,7 +59,7 @@ - State "CANCELLED" from "TODO" [2019-01-25 Fri 10:34] \\ It's enough to have "install" task by convention in packs and then oya tasks will show it. :END: -* TODO Experiment: exposing tasks +* DONE Experiment: exposing tasks ** DONE PoC ** DONE Flag for go import ** DONE Multiple packages? @@ -72,7 +72,7 @@ Use Meta.OriginalTaskName **** DONE Add to .feature **** DONE Add to tasks_test.go -** TODO Transient exposure (package exposing another package) +** DONE Transient exposure (package exposing another package) * TODO Vendoring is only partially implemented ** TODO Simplify oya get/vendor (based on Import statements) TBD **** Just use Import