Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't rebuild proto files if nothing in the proto dir has changed #1285

Closed
fadeev opened this issue Jun 22, 2021 · 10 comments
Closed

Don't rebuild proto files if nothing in the proto dir has changed #1285

fadeev opened this issue Jun 22, 2021 · 10 comments
Assignees
Labels
type:feat To implement new feature.
Milestone

Comments

@fadeev
Copy link
Contributor

fadeev commented Jun 22, 2021

Rebuilding proto files takes time, would be great if we could skip the process for when only Go files are changed.

@fedekunze
Copy link

ideally building proto files should be disabled and should only run by passing a flag. Otherwise starting the chain takes way longer than necessary

@fadeev
Copy link
Contributor Author

fadeev commented Jul 6, 2021

We're adding a separate command that will generate go from proto: starport generate proto-go, so it can be run manually. Maybe adding starport chain serve --skip-proto makes sense. I would still keep make serve build everything by default, because it's what new developers want: build everything for me with a single command.

@fadeev fadeev added this to the v0.18 milestone Jul 13, 2021
@ilgooz
Copy link
Member

ilgooz commented Jul 13, 2021

  • Let's try to omit go mod download and check how much time we cut from there.

For proto:

  • Calculate a hash from go.mod + proto files in the project, including third party ones defined in the config.yml + isRebuildAll enabled.
  • And calculate a hash from the contents and paths of generated files.

If these two hashes are the same between c build executions or when c serve triggered by a file change, automatically skip generating code from proto for go and others enabled through the config.yml.

@fadeev fadeev assigned Pantani and ilgooz and unassigned Pantani Sep 6, 2021
@fadeev fadeev removed this from the v0.18 milestone Sep 14, 2021
@ilgooz ilgooz added this to the v0.20.0 milestone Jan 11, 2022
@ilgooz ilgooz assigned ivanovpetr and unassigned ilgooz Jan 11, 2022
@ilgooz ilgooz added priority/high type:feat To implement new feature. labels Jan 11, 2022
@ilgooz
Copy link
Member

ilgooz commented Jan 17, 2022

Our last decisions about this future:

  • When a proto is changed under the proto/ dir of a chain; we'll check which module has changed and only do code generation for that module.
  • When a proto file is changed in one of the third party dirs defined in config.yml: we'll do code generation for all of the modules of the chain (for entire proto/ dir of the chain).
  • When the go.mod of the chain has changed; we'll do code generation for the modules imported in go.mod and for the entire proto/ dir of the chain.

also see #1997.

@ilgooz
Copy link
Member

ilgooz commented Jan 17, 2022

One more thing to add to this, we need to break the cache per module basis if the generated go files modified/deleted under x/ or when generated Dart/JS/Vuex code is modified/deleted for a module. cc @ivanovpetr

@ivanovpetr
Copy link
Contributor

@ilgooz You mean we need to regenerate all proto outputs (*.pb.go, vuex, dart) if some of them was deleted/modified? And outputs also should be tracked for every module separately.

@ilgooz
Copy link
Member

ilgooz commented Jan 20, 2022

Yes but separately for *.pb.go, vuex, dart.

If *.pb.go changed for a module, we only re-generate *.pb.go for that module.
If vuex changed for a module, we only re-generate vuex for that module.
If dart changed for a module, we only re-generate dart for that module.

By mean changed, yes it means deleted or modified.

@ilgooz
Copy link
Member

ilgooz commented Jan 25, 2022

Hey @ivanovpetr we need to prioritize improvements for the network cmd so, I put this issue to hold and remove it from the board. If you already have some existent code for it, you can create a draft PR and link to the issue so we can revisit later.

@ilgooz
Copy link
Member

ilgooz commented Apr 18, 2022

closing in favor of #2383.

@ilgooz ilgooz closed this as completed Apr 18, 2022
@ilgooz
Copy link
Member

ilgooz commented Jun 14, 2022

@fedekunze the latest release v0.22.1 now has an advanced caching system, works pretty fast now and it'll rebuild the modified proto packages (SDK modules) only.

You can observe the change in all Ignite CLI commands that requires to build from proto files, e.g.: chain serve/build/generate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feat To implement new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants