Skip to content

Use Path.GetFullPath to get the absolute path. #1033

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kmosegaard
Copy link
Contributor

Possible fix for #1030

@kmosegaard kmosegaard marked this pull request as draft March 3, 2023 14:24
@kmosegaard kmosegaard force-pushed the make_paths_absolute branch from ff63046 to ba6c7dc Compare March 19, 2023 08:36
@mgravell
Copy link
Member

I'd very much want to understand the full intent and scenarios here; if this is resolving down to the real file system, that's probably not ideal.

What problem are we trying to solve, and how does this fix address it?

@kmosegaard
Copy link
Contributor Author

The intent is to remove the .. from the paths given to the AdditionalFiles.

An example could be e.g.

    <AdditionalFiles Include="../../proto/base.proto" />
    <AdditionalFiles Include="../../proto/interface/rpc.proto" ImportPaths="../" />

These would be added to AdditionalFilesFileSystem as:

  • {repo dir}/src/App/../../proto/base.proto
  • {repo dir}/src/App/../../proto/interface/rpc.proto

rpc.proto relies on imported definitions from base.proto. An import path has to be added. If I'm not mistaken the generator tries to find base.proto (in the virtual file system) at the same path as rpc.proto. Next, it tries to go through the import paths.

As the example is presented, it will try to look for base.proto at {repo dir}/proto/ which is where it is located. In the look-up logic the path will look like this: {repo dir}/proto/base.proto. When trying to find the path in the virtual file system, it will end up doing this comparison: {repo dir}/src/App/../../proto/base.proto == {repo dir}/proto/base.proto which won't work.

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

Successfully merging this pull request may close these issues.

2 participants