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

manif_file relative path vs absolute - update to spec to support current gizmo behaviour #144

Open
tanant opened this issue May 31, 2021 · 1 comment

Comments

@tanant
Copy link

tanant commented May 31, 2021

I'm doing some testing with regards to Nuke 13/crypto and came across what I thought was an issue with the implementation, and while checking, it's actually it's spec-compliant but I feel the spec might need updating?

In 1.2.0 of the spec, it was changed so that that the manif_file is always relative to the EXR directory and should not start with "../"or "./", but provides the following python as sample processing which I believe is the same code in the reference gizmo

def resolve_manifest_paths(exr_path, sidecar_path):
    import os
    return os.path.join(os.path.dirname(exr_path), sidecar_path)

and

resolve_manifest_paths('/exr/abs/path', 'relative/path')  --> /exr/abs/path'/relative/path
resolve_manifest_paths('/exr/abs/path', '/relative/path') --> /relative/path

Obviously, this is a python implementation detail and it's not ported to the C++ binary, but the issue here is that we have files that don't always have the JSON sidecar deeper than the exr in the path hierarchy (so not 1.2.0 compliant), but the gizmo implementation never raised a concern so this has become part of the implementation detail that we're using (the sidecars are in their own separate directory at the same level, so you'd need to go ../sidecar or in our case, we write our files with absolute pathing)

Normally I'd update the source files rather than seek a spec update but this one feels like it might be the other way around.

@tanant
Copy link
Author

tanant commented Feb 23, 2022

For anyone reading this, to be clear (cause the issue isn't clear now rereading) - absolute pathing is how things were done as an implementation detail that would just happen to work with the reference python gizmo.

The core issue is that the JSON sidecar was not stored alongside the EXR, it was stored one level up in the hierarchy. To deal with this there are two possibilities (both not in spec currently)

  • support absolute pathing
  • support .. traversal

of the two, either would work but it's likely that absolute pathing is a Very Bad Idea for file portability, so I'd suggest the .. traversal if you asked me.

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

1 participant