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

generator: --include doesn't support external package(s) ? #179

Open
iurii-ssv opened this issue Nov 2, 2024 · 4 comments
Open

generator: --include doesn't support external package(s) ? #179

iurii-ssv opened this issue Nov 2, 2024 · 4 comments

Comments

@iurii-ssv
Copy link

iurii-ssv commented Nov 2, 2024

Perhaps I'm using it wrong (there isn't a proper example for it though), but when doing something like

import (
    spectypes "github.com/ssvlabs/ssv-spec/types"
)

//go:generate sszgen -path ./shares.go --objs storageShare --include github.com/ssvlabs/ssv-spec/types

type storageShare struct {
	OperatorID spectypes.OperatorID // uint64 essentially
}

I get the following error running sszgen:

[ERR]: stat github.com/ssvlabs/ssv-spec/types: no such file or directory
shares.go:18: running "sszgen": exit status 1

I've tried different ways to reference this external package (using alias and whatnot) but result is the same.


Additionally, if I try embedding - the error would be long the following lines:

[ERR]: failed to encode storageShare: embed type expects a typed object in same package but *ast.SelectorExpr found
shares.go:18: running "sszgen": exit status 1

which kind of suggest that external packages aren't supported at all (if they are supported, why not for embedding).

@iurii-ssv iurii-ssv changed the title --include doesn't support external package(s) ? generator: --include doesn't support external package(s) ? Nov 3, 2024
@ferranbt
Copy link
Owner

ferranbt commented Nov 4, 2024

For include to work you have to use the absolute path for github.com/ssvlabs/ssv-spec/types. The code generator does not resolve paths itself.

@iurii-ssv
Copy link
Author

For include to work you have to use the absolute path for github.com/ssvlabs/ssv-spec/types. The code generator does not resolve paths itself.

Hmm, well, but that doesn't really make much sense in terms of portability - local path on my machine would be different from the one my colleague uses (and I think it also might change for new package version). So it pretty much means generator can't be used with external packages, right ?

@mcdee
Copy link
Contributor

mcdee commented Nov 4, 2024

Bear in mind that includes can be relative, so if you use the standard go location for packages your include can be something like ../../repo/package/... and it will work.

@iurii-ssv
Copy link
Author

iurii-ssv commented Nov 4, 2024

Bear in mind that includes can be relative, so if you use the standard go location for packages your include can be something like ../../repo/package/... and it will work.

Right, but that's still probably "a hacky way to do it" at best - because, like I mentioned above:

  • different version of the same package might have different "local" file path (meaning once I upgrade to a newer version of some package generator was using external type from - generator will no longer find it, and I'd need to manually resolve it again)
  • people might not "use defaults" when it comes to where golang (and it's tooling) was/is installed
  • go mod tooling might change where they store project dependencies - it is an implementation detail after all
  • ...

To me it seems the best work-around at the moment would be to redefine those external types in the package ssz generator runs in (and manually type-convert those). As opposed to "make it work today" and try to debug why it doesn't work month/years later (or explaining to somebody why it doesn't work on his machine).

As a related side-note,

from what I understand for package-local types you still need to do something along the lines of adding an include like this:

... --include ./types.go,../types/signer.go

which is also not ideal in terms of code upgrades - for example, if I move struct definition to another file generator won't find it (and we won't even know our script for generating encodings broke until somebody runs it again). That is just to say that package-local types could also benefit from some general-purpose path-independent dependency resolution mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants