feat: generate stubs for FTL modules #1106

matt2e · 2024-03-18T04:53:32Z

ftl build and ftl dev now accept a list of external library directories which need stubs generated of the ftl modules it uses

eg:

ftl dev build examples/go --external examples/go/lib

This example finds all go modules in the go folder as well as treating go/lib as an external library

common/moduleconfig/moduleconfig.go

matt2e · 2024-03-20T05:32:50Z

buildengine/engine.go

-		gatherShemas(moduleSchemas, modules, modules[dep], out)
+		if dep != "builtin" {
+			depModule, ok := e.projects[dep]
+			// TODO: should we be gathering schemas from dependencies without a project?


Need to ask someone about this

matt2e · 2024-03-21T01:21:48Z

buildengine/project.go

+// It is used to:
+// - build the dependency graph
+// - map changes in the file system to the project
+type ProjectKey string


I'm a bit iffy on how wise it was to do it like this. It extends to a Project's Dependencies []ProjectKey.

I imagine one day we'd want to make a ModuleConfig.Module into a ModuleName type?
Projects can only depend on Modules (not ExternalLibraries), but they are all accessed via buildengine's map[ProjectKey]Project, and declaring all dependencies as ProjectKeys lessens the amount of recasting

Up to you, but having it in Dependencies definitely doesn't make sense as an external lib can't be a dependency.

Weren't you getting rid of ProjectKey?

buildengine/project.go

buildengine/build.go

buildengine/discover.go

buildengine/project.go

alecthomas · 2024-03-21T01:54:49Z

buildengine/project.go

+// It is used to:
+// - build the dependency graph
+// - map changes in the file system to the project
+type ProjectKey string


Up to you, but having it in Dependencies definitely doesn't make sense as an external lib can't be a dependency.

alecthomas · 2024-03-21T01:57:36Z

buildengine/topological.go

@@ -8,34 +8,37 @@ import (

 // TopologicalSort returns a sequence of groups of modules in topological order
 // that may be built in parallel.
-func TopologicalSort(graph map[string][]string) [][]string {
-	modulesByName := map[string]bool{}
+func TopologicalSort(graph map[ProjectKey][]ProjectKey) [][]ProjectKey {


Similar to above, ProjectKey doesn't make semantic sense here, and doesn't ensure enough type safety. I'd probably either back it out or split the types even further:

//sumtype:decl type ProjectKey interface { projectKey() } type ModuleKey string func (ModuleKey) projectKey() {} type ExternalLibraryKey string func (ExternalLibraryKey) projectKey() {}

This allows either to be used, or only one.

I'll back out ProjectKey somewhat, i think that's the best thing for now.
I was also thinking along the lines of having separate types, but figured to do a fully thought through ModuleKey/ModuleName type would be a lot farther reaching, and beyond the current scope of what I wanted to change.

alecthomas

LGTM but please add some tests.

alecthomas · 2024-03-21T03:19:09Z

buildengine/discover_test.go

 	"testing"

 	"github.com/alecthomas/assert/v2"

 	"github.com/TBD54566975/ftl/common/moduleconfig"
-	"github.com/TBD54566975/ftl/internal/log"
 )

 func TestDiscoverModules(t *testing.T) {


Add a test that includes an external library that imports one of the existing ftl modules in testdata.

alecthomas · 2024-03-21T03:21:20Z

buildengine/project.go

+// It is used to:
+// - build the dependency graph
+// - map changes in the file system to the project
+type ProjectKey string


Weren't you getting rid of ProjectKey?

matt2e · 2024-03-21T04:23:36Z

@alecthomas I kept ProjectKey but just for getting projects via the key (engine.projects map[ProjectKey]Project{}) but removed the other uses when we really meant module name (dependencies, maps of schemas etc).

# Conflicts: # buildengine/discover_test.go

alecthomas · 2024-03-21T06:53:32Z

buildengine/testdata/projects/libkotlin/src/main/kotlin/lib/Lib.kt

@@ -0,0 +1,11 @@
+package lib


alecthomas mentioned this pull request Mar 18, 2024

Dashboard #728

Open

matt2e commented Mar 19, 2024

View reviewed changes

common/moduleconfig/moduleconfig.go Outdated Show resolved Hide resolved

matt2e force-pushed the matt2e/stub-ftl-modules branch from 83f55fe to 198e0e2 Compare March 20, 2024 02:48

matt2e commented Mar 20, 2024

View reviewed changes

matt2e force-pushed the matt2e/stub-ftl-modules branch from 0e2e55b to 6aa1e3b Compare March 20, 2024 05:42

matt2e commented Mar 21, 2024

View reviewed changes

buildengine/project.go Show resolved Hide resolved

alecthomas reviewed Mar 21, 2024

View reviewed changes

matt2e marked this pull request as ready for review March 21, 2024 03:07

matt2e requested review from dhanji, wesbillman and worstell as code owners March 21, 2024 03:07

alecthomas approved these changes Mar 21, 2024

View reviewed changes

matt2e force-pushed the matt2e/stub-ftl-modules branch 2 times, most recently from c7f19e6 to df13243 Compare March 21, 2024 03:27

matt2e and others added 14 commits March 21, 2024 15:56

first go at generating stubs for go module

2177ca9

chore(autofmt): Automated formatting

cdda2a1

works in build cmd with --external

c16e536

extractDependencies has no need to be public

0ceafe1

works with dev + live updates

13892d7

simplify engine changes

b1ef386

simplify watch.go

a87fc59

Simplify FTLModule name

8fa048a

refactor: external libs are models with buildengine.Module

ab7a492

refactor: Project as interface which Module/ExternalLib implement

cb9e86d

chore(autofmt): Automated formatting

25d6d71

separate out project discovery code

ceaff99

refactor: add ProjectConfig, ProjectKey

37b0edf

fix tests

dc50681

# Conflicts: # buildengine/discover_test.go

matt2e added 12 commits March 21, 2024 16:05

remove todo

b93c102

build handles libs and modules

ff41a1b

generate stubs for kotlin external libraries

7437c46

fix lint warning

a68832c

lint fixes

17ad15a

project is a sumtype

6e5e68c

remove ProjectKeyForModuleName()

3fe62f9

clean up

4de50b2

reduce use of ProjectKey

34c49bb

Remove unneeded changes to topological.go

270cb34

seal Project

e8e1c09

improved testing: discovery + deps

c279d52

# Conflicts: # buildengine/discover_test.go

matt2e force-pushed the matt2e/stub-ftl-modules branch 2 times, most recently from 545c008 to 30108a4 Compare March 21, 2024 05:14

fix rebase

372c565

matt2e force-pushed the matt2e/stub-ftl-modules branch from 30108a4 to 372c565 Compare March 21, 2024 05:21

matt2e merged commit 5eb0b3f into main Mar 21, 2024
11 checks passed

matt2e deleted the matt2e/stub-ftl-modules branch March 21, 2024 05:24

alecthomas reviewed Mar 21, 2024

View reviewed changes

buildengine/testdata/projects/libkotlin/src/main/kotlin/lib/Lib.kt

@@ -0,0 +1,11 @@

package lib

Copy link

Collaborator

alecthomas Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: generate stubs for FTL modules #1106

feat: generate stubs for FTL modules #1106

matt2e commented Mar 18, 2024 •

edited

Loading

matt2e Mar 20, 2024

matt2e Mar 21, 2024

alecthomas Mar 21, 2024

alecthomas Mar 21, 2024

alecthomas Mar 21, 2024

alecthomas Mar 21, 2024

matt2e Mar 21, 2024

alecthomas left a comment

alecthomas Mar 21, 2024

alecthomas Mar 21, 2024

matt2e commented Mar 21, 2024

alecthomas Mar 21, 2024

feat: generate stubs for FTL modules #1106

feat: generate stubs for FTL modules #1106

Conversation

matt2e commented Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alecthomas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matt2e commented Mar 21, 2024

Choose a reason for hiding this comment

matt2e commented Mar 18, 2024 •

edited

Loading