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

DSDLDefinition: do not resolve symlink #113

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

filippobrizzi
Copy link

Remove path resolution for files.
To make pydsdl work with Bazel we need to allow it to work with symlinks as Bazel works with sandbox environments.

Remove path resolution
@pavel-kirienko
Copy link
Member

Have you ascertained that this change alone is sufficient to make it work with Bazel? Symlink resolution is not essential for PyDSDL, but path canonicalization is, so that should be introduced instead.

@filippobrizzi
Copy link
Author

@pavel-kirienko to be honest, I didn't do a deep investigation of every possible Bazel use case, but just the minimal changes to make my specific case work:
Run nnvg tool to generate C++ code from dsdl files.

Also, if you will ever be interested in adding Bazel support to dsdl C++ code generation, let me know as I can share some Bazel code that can serve as starting point.

@pavel-kirienko
Copy link
Member

@thirtytwobits is this going to break anything on your side?

@thirtytwobits
Copy link
Member

Yes. This was added to fix #111
I'm not sure why it doesn't work with symlinks?

@thirtytwobits
Copy link
Member

@filippobrizzi , can you provide an example for how this code fails when the path includes a symlink?

@thirtytwobits
Copy link
Member

Is this really about symlinks or are we asking to allow relative paths to be canonical?

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.

3 participants