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

Question: Can InferImportPaths be supported? #315

Closed
FGYFFFF opened this issue Jun 11, 2024 · 8 comments
Closed

Question: Can InferImportPaths be supported? #315

FGYFFFF opened this issue Jun 11, 2024 · 8 comments

Comments

@FGYFFFF
Copy link

FGYFFFF commented Jun 11, 2024

I'm working on a problem, but I don't have much of an idea.

protobuf
├── api.proto
└── hello
    └── hello.proto

hello.proto imports api.proto. But I can't get 'import path' for api.proto, so how can I to infer api.proto?

thanks~

@jhump
Copy link
Member

jhump commented Jun 11, 2024

Inferring import paths is rather error-prone and brittle. I'd recommend instead setting the import paths explicitly.

For that example, I think you want "protobuf" as the single import path and then compile "hello/hello.proto". It should successfully find both files.

An alternative would be to provide both "protobuf" and "protobuf/hello" as import paths and then compile "hello.proto", but that is a bad practice because that file (hello.proto) actually has two possible paths -- both "hello.proto" and "hello/hello.proto", which is not good. Ideally, a file has exactly one way to refer to it so you don't end up with the same file being referring to via multiple paths in import statements. See this doc for more context.

@FGYFFFF
Copy link
Author

FGYFFFF commented Jun 12, 2024

Thank you for your answer. We have a historical problem with our system where all proto files are stored under a repo, but no “importPath” is stored, so I'm trying to come up with an algorithm to automatically search for importPath.

Would it be a problem if I use all directories of the current repo as “importPath”? Maybe there might be files with the same name.

@jhump
Copy link
Member

jhump commented Jun 12, 2024

Maybe there might be files with the same name.

That would definitely be a problem.

Instead of using all directories, you could do a fast-scan of every file just to read its imports. Then you could create an index that maps those import paths to all possible files (files whose path suffix matches the import path). From that you should be able to (1) enumerate exactly the directories you need (the prefixes in that index) and (2) detect possible conflicts (where there's more than one match to an import path, so it's not clear what the import path should be since it's not clear which file should be imported). If you do discover conflicts, you'll have to come up with some way to fix them or them to perhaps come up with a rubric for eliminating some of the choices. Sorry that I can't be more clear than that -- it's certainly not a simple process, but should hopefully be feasible.

@FGYFFFF
Copy link
Author

FGYFFFF commented Jun 13, 2024

@jhump Thanks for the reply, I will make a try.

@FGYFFFF FGYFFFF closed this as completed Jun 13, 2024
@FGYFFFF FGYFFFF reopened this Jun 13, 2024
@FGYFFFF
Copy link
Author

FGYFFFF commented Jun 13, 2024

image I have implemented the auto search algorithm and can generate the code correctly. However, I would like to get the absolute path to the file from the `compile result` , and it seems that the information is not saved at the moment. Is there currently a way to get the absolute path to the proto file.

@FGYFFFF
Copy link
Author

FGYFFFF commented Jun 13, 2024

I think that if it can correctly identify the proto file, it can get the corresponding absolute path

@jhump
Copy link
Member

jhump commented Jun 13, 2024

@FGYFFFF, the compiler has and thus the result types have no concept of "absolute path". That is all handled by the protocompile.Resolver.

Currently, the compiler only knows about the partial path, and the source resolver's accessor function only knows about the full path. It's the protocompile.SourceResolver that handles iterating through import paths, trying each one. In order to learn what happens in that "middle man", I see two options:

  1. Create your type that implements Resolver. It merely delegates to a SourceResolver, but it sets the accesor function to a custom function. That way it can see both the original requested path, and the custom accessor function can see the full path. On successful resolution, you can then store that in a map or something.
  2. Fork the SourceResolver. It's not a very large or complicated type. In its loop, where it tries the import paths, on successful resolution, have it store the import path that was used in a map or something.

These would make the "full path" available via the resolver: your code would need to ask the resolver what the full path is for a given file, and it could be looked up from a map.

@FGYFFFF
Copy link
Author

FGYFFFF commented Jun 14, 2024

@jhump Thanks. I have solve the problem by implementing Resolver. Thank you very much for your answer.

@FGYFFFF FGYFFFF closed this as completed Jun 14, 2024
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

2 participants