Skip to content

Commit

Permalink
refactor: cleanup tmp folders & change third party includes (#3696)
Browse files Browse the repository at this point in the history
* fix: implement third party includes using previous semantics

Use `thirdModuleIncludes` instead of defining a new type to store
modules and includes to be consistent with `appIncludes` and to avoid
changing existing code to work with a new type.

* feat: add tmp dir cleanup support to cosmos generator

* chore: restore Go version to `1.21` and remove toolchain

Currently CI is giving an error because is not properly configured to
support the new Go 1.21 features for the mod file and we don't have a
consensus yet on how to configure the toolchain and Go versions.

* chore: go mod tidy

* ci: change integration tests to use Go version from mod file

This is required to avoid downloading a newer Go toolchain which will
fail because GOSUMDB is disabled to fix the timeout issues because of
the repository size.

* ci: disable GOTOOLCHAIN when running integration tests

This is required because toolchain won't work when GOSUMDB is disabled.

* ci: change integration tests to use the stable go version

The stable Go version should be used to avoid keeping updating the
workflow config when a new patch version is released. This is to make
sure that Go doesn't try to download a new toolchain version.

* fix: correct CI linting issues

* ci: remove redundant GH workflow cache step

Go setup action caches since `v4`

* fix: disable go setup GH action cache

Action for `golangci-lint` already have caching functionality so Go setup
must be disabled to avoid caching issues.

* chore: add thitd party includes only when available

* fix: correct directory remove

Co-authored-by: Danilo Pantani <[email protected]>
  • Loading branch information
jeronimoalbi and Pantani authored Oct 17, 2023
1 parent e32b646 commit 19c2666
Show file tree
Hide file tree
Showing 13 changed files with 86 additions and 67 deletions.
17 changes: 6 additions & 11 deletions .github/workflows/test-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,17 @@ jobs:
go.sum
**/testdata/**
- uses: actions/cache@v3
if: env.GIT_DIFF
with:
path: |
~/.cache/go-build
~/go/pkg/mod
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-go-
- uses: actions/setup-go@v4
if: env.GIT_DIFF
with:
go-version: '1.21'
go-version: 'stable'

- name: Run Integration Tests
if: env.GIT_DIFF
run: GOSUMDB=off go test -v -timeout 120m ./integration/${{ matrix.test-path }}
env:
GOTOOLCHAIN: local+path
GOSUMDB: off
run: go test -v -timeout 120m ./integration/${{ matrix.test-path }}

status:
runs-on: ubuntu-latest
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/test-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ jobs:
**/*.go
go.mod
go.sum
- uses: actions/setup-go@v4
if: env.GIT_DIFF
with:
go-version-file: go.mod
cache: false

- uses: golangci/golangci-lint-action@v3
if: env.GIT_DIFF
with:
Expand Down
2 changes: 1 addition & 1 deletion ignite/pkg/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (c Cache[T]) Put(key string, value T) error {
func (c Cache[T]) Get(key string) (val T, err error) {
db, err := openDB(c.storage.storagePath)
if err != nil {
return
return val, err
}
defer db.Close()

Expand Down
10 changes: 5 additions & 5 deletions ignite/pkg/cosmosanalysis/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,25 +220,25 @@ func newUnexpectedTypeErr(n any) error {
func findBasicManagerRegistrations(n ast.Node, pkgDir string, fileImports map[string]string) (packages []string, err error) {
callExprType, ok := n.(*ast.CallExpr)
if !ok {
return
return packages, err
}

selectorExprType, ok := callExprType.Fun.(*ast.SelectorExpr)
if !ok {
return
return packages, err
}

identExprType, ok := selectorExprType.X.(*ast.Ident)
if !ok {
return
return packages, err
}
basicModulePkgName := findBasicManagerPkgName(fileImports)
if basicModulePkgName == "" {
// cosmos-sdk/types/module is not imported in this file, skip
return
return packages, err
}
if identExprType.Name != basicModulePkgName || selectorExprType.Sel.Name != "NewBasicManager" {
return
return packages, err
}

// Node "n" defines the call to NewBasicManager, let's loop on its args to discover modules
Expand Down
2 changes: 1 addition & 1 deletion ignite/pkg/cosmosbuf/buf.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func (b Buf) Generate(
// Cleanup deletes temporary files and directories.
func (b Buf) Cleanup() error {
if b.sdkProtoDir != "" {
return os.Remove(b.sdkProtoDir)
return os.RemoveAll(b.sdkProtoDir)
}
return nil
}
Expand Down
61 changes: 34 additions & 27 deletions ignite/pkg/cosmosgen/cosmosgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cosmosgen

import (
"context"
"os"
"path/filepath"
"strings"

Expand Down Expand Up @@ -36,12 +37,6 @@ type generateOptions struct {
specOut string
}

// TODO add WithInstall.
type ModuleIncludes struct {
Includes []string
Modules []module.Module
}

// ModulePathFunc defines a function type that returns a path based on a Cosmos SDK module.
type ModulePathFunc func(module.Module) string

Expand Down Expand Up @@ -110,18 +105,27 @@ func IncludeDirs(dirs []string) Option {

// generator generates code for sdk and sdk apps.
type generator struct {
ctx context.Context
buf cosmosbuf.Buf
cacheStorage cache.Storage
appPath string
protoDir string
gomodPath string
o *generateOptions
sdkImport string
deps []gomodule.Version
appModules []module.Module
appIncludes []string
thirdModules map[string]ModuleIncludes // app dependency-modules/includes pair.
ctx context.Context
buf cosmosbuf.Buf
cacheStorage cache.Storage
appPath string
protoDir string
gomodPath string
o *generateOptions
sdkImport string
deps []gomodule.Version
appModules []module.Module
appIncludes []string
thirdModules map[string][]module.Module
thirdModuleIncludes map[string][]string
tmpDirs []string
}

func (g *generator) cleanup() {
// Remove temporary directories created during generation
for _, path := range g.tmpDirs {
_ = os.RemoveAll(path)
}
}

// Generate generates code from protoDir of an SDK app residing at appPath with given options.
Expand All @@ -135,15 +139,18 @@ func Generate(ctx context.Context, cacheStorage cache.Storage, appPath, protoDir
defer b.Cleanup()

g := &generator{
ctx: ctx,
buf: b,
appPath: appPath,
protoDir: protoDir,
gomodPath: gomodPath,
o: &generateOptions{},
thirdModules: make(map[string]ModuleIncludes),
cacheStorage: cacheStorage,
}
ctx: ctx,
buf: b,
appPath: appPath,
protoDir: protoDir,
gomodPath: gomodPath,
o: &generateOptions{},
thirdModules: make(map[string][]module.Module),
thirdModuleIncludes: make(map[string][]string),
cacheStorage: cacheStorage,
}

defer g.cleanup()

for _, apply := range options {
apply(g.o)
Expand Down
36 changes: 24 additions & 12 deletions ignite/pkg/cosmosgen/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ var protocGlobalInclude = xfilepath.List(
)

type ModulesInPath struct {
Path string
ModuleIncludes ModuleIncludes
Path string
Modules []module.Module
Includes []string
}

func (g *generator) setup() (err error) {
Expand Down Expand Up @@ -117,28 +118,38 @@ func (g *generator) setup() (err error) {
if err != nil {
return err
}
includes := []string{}
if len(modules) > 0 {

includes, err = g.resolveIncludes(path) // For versioning issues, we do dependency/includes resolution per module
var includes []string
if len(modules) > 0 {
// For versioning issues, we do dependency/includes resolution per module
includes, err = g.resolveIncludes(path)
if err != nil {
return err
}
}

modulesInPath = ModulesInPath{
Path: path, // Each go module
ModuleIncludes: ModuleIncludes{ // Has a set of sdk modules and a set of includes to build them
Includes: includes,
Modules: modules,
},
Path: path,
Modules: modules,
Includes: includes,
}

if err := moduleCache.Put(cacheKey, modulesInPath); err != nil {
return err
}
}

g.thirdModules[modulesInPath.Path] = modulesInPath.ModuleIncludes
g.thirdModules[modulesInPath.Path] = append(
g.thirdModules[modulesInPath.Path],
modulesInPath.Modules...,
)

if modulesInPath.Includes != nil {
g.thirdModuleIncludes[modulesInPath.Path] = append(
g.thirdModuleIncludes[modulesInPath.Path],
modulesInPath.Includes...,
)
}
}

return nil
Expand Down Expand Up @@ -200,9 +211,10 @@ func (g *generator) generateBufIncludeFolder(modpath string) (string, error) {
return "", err
}

g.tmpDirs = append(g.tmpDirs, protoPath)

err = g.buf.Export(g.ctx, modpath, protoPath)
if err != nil {
os.Remove(protoPath)
return "", err
}
return protoPath, nil
Expand Down
2 changes: 1 addition & 1 deletion ignite/pkg/cosmosgen/generate_composables.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (g *generator) generateComposables(frontendType string) error {
}

for _, modules := range g.thirdModules {
data.Modules = append(data.Modules, modules.Modules...)
data.Modules = append(data.Modules, modules...)
}

vsg := newComposablesGenerator(g, frontendType)
Expand Down
2 changes: 1 addition & 1 deletion ignite/pkg/cosmosgen/generate_openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func (g *generator) generateOpenAPISpec() error {
}

for src, modules := range g.thirdModules {
if err := add(src, modules.Modules); err != nil {
if err := add(src, modules); err != nil {
return err
}
}
Expand Down
7 changes: 4 additions & 3 deletions ignite/pkg/cosmosgen/generate_typescript.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ func (g *generator) generateTS() error {
// custom modules losing the registration of the third party
// modules when the root templates are re-generated.
for _, modules := range g.thirdModules {
data.Modules = append(data.Modules, modules.Modules...)
for _, m := range modules.Modules {
data.Modules = append(data.Modules, modules...)
for _, m := range modules {
if strings.HasPrefix(m.Pkg.Name, "interchain_security.ccv.consumer") {
data.IsConsumerChain = true
}
Expand Down Expand Up @@ -140,7 +140,8 @@ func (g *tsGenerator) generateModuleTemplates() error {
// is available and not generated it would lead to the registration of a new not generated
// 3rd party module.
for sourcePath, modules := range g.g.thirdModules {
add(sourcePath, modules.Modules, append(g.g.appIncludes, modules.Includes...))
includes := g.g.thirdModuleIncludes[sourcePath]
add(sourcePath, modules, append(g.g.appIncludes, includes...))
}

return gg.Wait()
Expand Down
2 changes: 1 addition & 1 deletion ignite/pkg/cosmosgen/generate_vuex.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (g *generator) generateVuex() error {
}

for _, modules := range g.thirdModules {
data.Modules = append(data.Modules, modules.Modules...)
data.Modules = append(data.Modules, modules...)
}

vsg := newVuexGenerator(g)
Expand Down
4 changes: 2 additions & 2 deletions ignite/pkg/nodetime/programs/ts-proto/tsproto.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func BinaryPath() (path string, cleanup func(), err error) {
// Create binary for the TypeScript protobuf generator
command, cleanupBin, err := nodetime.Command(nodetime.CommandTSProto)
if err != nil {
return
return path, cleanup, err
}

defer func() {
Expand All @@ -39,7 +39,7 @@ func BinaryPath() (path string, cleanup func(), err error) {
// test overwriting the generator script while it is being run in a separate test process.
tmpDir, err := os.MkdirTemp("", "ts_proto_plugin")
if err != nil {
return
return path, cleanup, err
}

cleanupScriptDir := func() { os.RemoveAll(tmpDir) }
Expand Down
5 changes: 3 additions & 2 deletions ignite/pkg/placeholder/error.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package placeholder

import (
"errors"
"fmt"
"strings"

Expand All @@ -18,8 +19,8 @@ type MissingPlaceholdersError struct {

// Is true if both errors have the same list of missing placeholders.
func (e *MissingPlaceholdersError) Is(err error) bool {
other, ok := err.(*MissingPlaceholdersError) //nolint:errorlint
if !ok {
var other *MissingPlaceholdersError
if !errors.As(err, &other) {
return false
}
if len(other.missing) != len(e.missing) {
Expand Down

0 comments on commit 19c2666

Please sign in to comment.