AcademySoftwareFoundation / OpenColorIO

A color management framework for visual effects and animation.
https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
1.79k stars 454 forks source link

Paths checked for absolute-ness in GetAbsoluteSearchPaths before EnvExpand is called #409

Closed acjones closed 5 months ago

acjones commented 8 years ago

When resolving a relative path in Context::resolveFileLocation, GetAbsoluteSearchPaths splits the search_path string into a map, adding the working directory to any relative-looking paths in the map. resolveFileLocation then calls EnvExpand on each item in the map, and checks for a file's existence. GetAbsoluteSearchPaths probably needs to either call EnvExpand itself, or leave the absolute-ness check for after EnvExpand is called. The second options seems likely to be faster, since the absolute-ness check is likely to be cheaper than EnvExpand.

A related issue is that Windows is sensitive to the ":" character. Using environment tokens is actually a work-around for this, but it doesn't really work in the search path for the reasons above.

My current work-around is to use absolute paths explicitly for files that need to specify the drive root. In the search paths, I'm making liberal use of "../../../../" to allow paths above the OCIO folder to be relative. This works, but is inelegant.

hodoulp commented 7 years ago

Concerning windows and the delimiter: Instead of using a string containing one or several relative or absolute paths, why not having a list of paths. By doing so the problem with the delimiter disappears by itself (i.e. no more method to parse the string). Anyway, any delimiter choice will hit the parsing problem as most of the charater set could be used in file path.

acjones commented 6 years ago

For me, the delimiter issue is fairly minor compared to the absoluteness issue. Right now there's seemingly no way to specify an absolute search path in Windows. Trying to detect absolute vs relative paths before expanding environment variables seems like it's just a run-of-the mill bug that can be fixed, regardless of how the paths are separated into a list.

Allowing a yaml list is an interesting idea, and would certainly be a little cleaner. That said, does it make any sense to allow a config to support something like: "$SEARCH_PATHS" that might resolve to a string containing the delimiter itself -- sort of like how $PATH works in a shell? Switching to an explicit list might make doing that more difficult down the road. I think being able to condense the search paths to a single pipeline variable, and keep that out of the config would be really helpful, so I'm hesitant to do anything that would make that more difficult down the road. (This should probably become its own issue, but it's relevant to the idea of yaml lists of paths).

There are some other good ways to get around the ":" vs ";" delimiter issue with the OSes. A definition of the delimiter character itself, etc. Or just using ";" instead of ":" if it's detected anywhere in the string. I think a pretty standard thing to do is to just detect the OS and use ";" on Windows. I'm sure there are some reasonably canonical implementations for cross-platform search path parsing out there that we could reference whenever this gets revisited.

devAtTheBoat commented 6 years ago

I'm having this exactly same issue. Is there any way to resolve it?

The search_path key in our ocio config looks like this: search_path: $ROOTFOLDER/$JOB/vfx/sequences/$SEQUENCE/$LINK/luts

This resolves OK in unix environment, where our ROOTFOLDER env is /raid/ but in the windows machines where the $ROOTFOLDER is "X:\raid\", OCIO thinks it's a relative path and prepends the config.ocio location directory to the search_path.

is there any workaround?

Thanks!

acjones commented 6 years ago

It's been a little while since I messed with this, but my workaround was to use relative paths. Not ideal at all, but it seemed to be the only way to get arbitrary paths with OCIO. I can't promise that's the only way, but I did spend a while looking at it before I gave up and went with that approach. Basically, instead of using ${ROOTFOLDER} you'd need to use something like python's os.path.relpath to get a relative version of ${ROOTFOLDER}.

Of course the other way to fix it would be to author a patch, but depending on where you need OCIO support it may not be much help until the patch is merged and released as part of VFX Platform.

IIRC, there's a different code path for LUTs that are specified from a colorspace or look definition. So that might be something to try if you don't need to search multiple folders for your LUT. In my OCIO config I have some colorspace definitions that use absolute paths, with the root defined in a variable, and it's been working.

dbr commented 6 years ago

I think having the search_path as a YAML list is a good idea.

The search_path key could accept both a string and a list, e.g both of these could coexist:

search_path: ["C:/path/to/luts", "C:/path/to/more/luts"]
# or
search_path: "/path/to/luts:/path/to/more/luts"

One unrelated benefit is you can format YAML lists more nicely - e.g we have quite a long search_path for various fallbacks, and it can be quite hard to read, whereas the YAML list could be formatted in a nicer fashion:

search_path:
  - /path/to/luts
  - /path/to/more/luts
  - etc

That said, does it make any sense to allow a config to support something like: "$SEARCH_PATHS" that might resolve to a string containing the delimiter itself -- sort of like how $PATH works in a shell? [...] I think being able to condense the search paths to a single pipeline variable, and keep that out of the config would be really helpful,

If there is a compelling reason for this, I think this would be better done by adding an $OCIO_SEARCH_PATH environment variable to OCIO which overrides the search_path parameter (similar to $OCIO_ACTIVE_VEIWS etc), rather than trying to support this in a cross-platform manner in the config itself. This could be parsed with per-platform delimiters without much contention since it wouldn't be in a shareable-config-file (one of the goals of the config.ocio format)

hodoulp commented 6 years ago

As I mentioned, I support the YAML list proposal.

Just to mention that the env. variable (for paths) proposals add some challenge for timeline-based tools where several 'shots' are used (with potentially different env. variables) and potentially coming from different machines/OSs. Render farms and Cloud could also add some challenges.

On the other hand, the UNC paths for search_path are working fine. Perhaps it could be an acceptable workaround for short-term. search_path: \\1.2.3.4\public\luts

BernardLefebvre commented 5 years ago

I tried this and it worked on Windows. I'll add a test in an upcoming PR. context->setStringVar("ROOTFOLDER", "c:/"); context->setSearchPath("${ROOTFOLDER}/folder1:${ROOTFOLDER}/folder2"); context->resolveFileLocation("file.test");

hodoulp commented 5 years ago

The pull request #712 provides a support for multiple platform specific paths i.e. several Windows c:/xxx type of paths.

p0las commented 4 years ago

on windows you can use "DOS device paths".

The Windows operating system has a unified object model that points to all resources, including files. These object paths are accessible from the console window and are exposed to the Win32 layer through a special folder of symbolic links that legacy DOS and UNC paths are mapped to. This special folder is accessed via the DOS device path syntax, which is one of:

\.\C:\Test\Foo.txt \?\C:\Test\Foo.txt

search_path: \\?\$LUTS works fine as absolute path (just make sure the env var contains backslashes)