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

Relative paths in flow configurations are handled in respect to CWD instead of file location. #640

Open
kboronski-ant opened this issue Sep 27, 2022 · 2 comments
Labels
Bug Something isn't working f4pga (python) Python package

Comments

@kboronski-ant
Copy link
Collaborator

First described in #554

This behviour forces the user to run f4pga with PWD being equal to the location of flow.json. Otherwise, the paths in flow.json would be resolved incorrectly.

@umarcor umarcor added the f4pga (python) Python package label Sep 30, 2022
@kboronski-ant kboronski-ant added the Bug Something isn't working label Oct 4, 2022
@suksham11
Copy link

can you give me suggestions how can i contribute on this

@kboronski-ant
Copy link
Collaborator Author

@suksham11 Hi, thanks for your interest in this project.
There are several sources from which f4pga takes paths.

  1. Platform definitions (f4pga/flows/platforms.yml) - those will be values (dependencies cannot be specified there).
  2. Flow configuration - these will be dependencies provided by the user and maybe overrides for values that were previously defined in platform definition (unlikely).
  3. Paths that are generated during dependency resolution - we probably don't need to worry about those, at least if every path at this point is resolved to an absolute path as it should be.

If you found the terminology here a bit confusing, the "Fundamental concepts" section of docs might be helpful.

Be sure to also check developer notes


Handling it just for dependencies provided by the user should be a good starting point and a valuable contribution on its own.

1) is out of reach as for now and so is the "value" part of 2).

The reason why values are a problem is because they are untyped, so we don't know whether they are a path or not - it's up to a module to decide how to parse a value. I would love to see it change as it would greatly improve error reporting, but it's easier said than done. An alternative approach, one I'd like a bit more would be to allow dependencies to be specified in platform definitions. In that case we would probably want to exclude them from modification tracking mechanism (f4pga/flows/cache.py), unless overriden by user.

There are places in the in which Path.resolve() is being used to convert paths to absolute paths. Those should be modified (at least some of them) by introducing a bit more sophisticated logic that would take into account the origin of the path. If it comes from a flow configuration file, then it should be resolved as a path relative to the location of that file. However, if it was passed through CLI, it should be treated as if it was relative to PWD. Of course the origin would have to be tracked somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working f4pga (python) Python package
Projects
None yet
Development

No branches or pull requests

3 participants