Add temporary directory helpers and a createDirectories write option

[ld-kerley]

(This work is extracted and refined from #2739)

This adds a small set of file system utilities for creating and removing temporary directories, along with an option for writeToXmlFile() to create the directory hierarchy of its target file. Together these let tools and tests stage
generated documents in scratch locations without pre-creating the directory structure themselves, which currently requires each caller to reimplement platform-specific temporary directory logic.

The following specific changes are included:

• Add FilePath::getSystemTemporaryDirectory(), returning the platform temporary directory. This is queried with GetTempPath on Windows, and with the TMPDIR, TMP, TEMP and TEMPDIR environment variables on POSIX,
  falling back to /tmp. The directory is not created, and its existence is not guaranteed.
• Add FilePath::createTemporaryDirectory(), creating a uniquely named directory under a given parent and defaulting to the system temporary directory, and throwing an Exception if the directory cannot be created. POSIX platforms use mkdtemp(), which generates the name, creates the directory atomically, and restricts access to the owner. Windows has no equivalent, so candidate names are passed to CreateDirectory, which fails rather than succeeding when the path already exists; only a name collision is retried, and any other error is reported.
• Add FilePath::removeDirectory(), removing a directory and optionally its contents, and returning whether the removal succeeded. A recursive removal classifies entries with lstat or the reparse point attribute, so that symbolic links within the directory are removed without deleting the contents of their targets.
• Add an XmlWriteOptions::createDirectories flag, causing writeToXmlFile() to create the parent directory hierarchy of the target file if it does not already exist. The flag defaults to false, so the behavior of existing code is unchanged.

New cases in MaterialXTest cover the default and explicit parent directories, the uniqueness and owner-only permissions of each created directory, the reporting of a creation failure under a read-only parent, directory removal with and without recursion, the preservation of a symbolic link's target during a recursive removal, and the round-tripping of a document written into a newly created hierarchy. The permission, read-only parent and symbolic link cases are guarded to POSIX. I have run the full test suite on macOS; the Windows code paths are exercised only by CI.

[lgritz]

Is there a reason why these aren't just implemented in terms of std::filesystem wherever possible? Are those inadequate in a way I have not previously appreciated?

[ld-kerley]

@lgritz I was considering exactly that while putting this together - but MaterialX is used in a whole bunch of different places javascript bindings via emscripten, and I wasn't 100% sure that std::filesystem is supported in all the places.

Also hoping to land this fairly easily, as a precursor to some other more important work - so didn't want to rock the boat too much.

Given you also flagged what I was thinking I'll file an issue and we can come back to std::filesystem conversion as a separate piece of work. In my very cursory investigation with Claude, it did notice that std::filesystem calls throw exceptions that are more specific that std::exception, but the MaterialX pattern for exceptions is to inherit from that. So I wonder if moving to std::filesystem might invalidate some client code where its catching the MaterialXException class.

[lgritz]

Don't let me derail you. In all honestly, I was thinking "gosh, I just minimally wrap std::filesystem for some of these... are there shortcomings I'm unaware of?"

I assume these are holdovers from the pre-C++17 days.

[jstone-lucasfilm]

Thanks for this contribution, @ld-kerley!

On the std::filesystem question that @lgritz raises above, it's worth noting that removeDirectory and getSystemTemporaryDirectory map directly to remove_all and temp_directory_path, but std::filesystem has no equivalent of mkdtemp, so the core logic in createTemporaryDirectory would be required either way. A follow-up issue for migrating File.cpp as a whole sounds like the right path forward.

Two suggestions from an initial review of the code:

• The read-only-parent test will fail when our test suite runs as root, since root bypasses file permission checks, allowing createTemporaryDirectory to succeed where the test expects a throw. This situation can occur in practice because Docker containers run as root by default, so packaging and CI environments often execute test suites as root. A geteuid() != 0 guard would make this case robust.
• Since writeToXmlFile never checks whether its stream opened, a caller who sets createDirectories and hits a permission failure receives a silent no-op. This issue predates the PR, but it may be worth throwing an exception when the stream fails to open, either here or in follow-up work.

Otherwise this looks good to me, and let's aim to include it in our v1.39.6 release.

[jstone-lucasfilm]

Thanks for the revision, @ld-kerley! The geteuid() guard and the new throw in writeToXmlFile both look good, and I'm happy to defer Python and JavaScript bindings for the new API to follow-up work.

One remaining request follows from the new throw. Three existing callers of writeToXmlFile are neither wrapped in this PR nor covered by an enclosing handler, so a failed write that was previously a silent no-op now escapes as an uncaught exception:

• MaterialXView/Viewer.cpp:2026, which writes the translated material on the T key. This call is made from within keyboard_event, so a read-only output path would terminate the viewer. Wrapping it in the same manner as the save-material path at line 660 would also correct the "Saved translated material" dialog, which currently appears regardless of whether the write succeeded.
• MaterialXView/Viewer.cpp:427, which writes DirectLightRig.mtlx when --saveGeneratedLights is set. This call is reached from the constructor, so a failure here would abort startup. The lighting block just below it demonstrates the pattern.
• MaterialXGenOsl/LibsToOso.cpp:506, which writes genoslnetwork_impl.mtlx. This call sits outside the per-nodedef try block, so it bypasses the tool's return 1 path. A catch that sets hasFailed = true would be consistent with the rest of the file, and this site could also serve as the first in-tree user of createDirectories = true.

With those three covered, let's aim to include this in our v1.39.6 release.

[ashwinbhat]

Hi, I have a question if MaterialX should own directory operations. In embedded/protected environments, the application has security, user-specific filesystem permissions, threading constraints, and cleanup rules that MaterialX might know about, esp. when the SDK is several layers below the application. Can we instead depend on caller to provide the paths etc?

[ld-kerley]

@ashwinbhat - how are directory operations different from file writing operations - from an embedded security perspective? MaterialXFormat already supports writing files - is that similarly a problem for security - and how are you handling that if thats the case?

I think we could scope this functionality more tightly to the OSL Network generator - if that makes it easier to remove from embedded environments that don't support that generator - but I do think this code needs to live on the MaterialX side somewhere. The eventual goal here is to allow the OSL Network generator to be able to on-the-fly code gen new OSL nodes from locally defined custom nodedef elements in the document.

[ashwinbhat]

In our usage we don't use any write apis from MaterialX, it's mainly read operations. Anytime we need to write we get the stream and write to filesystem higher up in the stack.
Just to clarify, since operations like recursive folder deletion, undeleted temp folders/files get flagged by security, I'm raising this as a concern not a blocker.

[ld-kerley]

Are there concrete compromises we can make here to make things easier from a security side of things for you - while still moving this forward?

would it help if we introduced a MATERIALX_READ_ONLY (name to be workshopped) cmake build mode where all the file write I/O was compiled out? - which would probably be easier to manage as a separate PR.

If feels like moving this code to be only inside the OSL Network generator would "hide" things - in a way that might make them less easily to audit? Or do you think this path would be more helpful?

[ashwinbhat]

Right now it looks like the main consumer of this will be OSL Network generator. I'm ok with isolating this to OSL Network generator instead of MaterialXFormat. We don't need to add ifdef on API headers, as it might create more issues with multiple MaterialX installs etc.

[ld-kerley]

How do you feel about that stylistically @jstone-lucasfilm ? I'm open to any approach that allows us to move forwards here.

If thats the route here - then it might make sense to just close this PR - and when I put up the final OSL Network parity PR - I'll include the code here (or some modified version of it) in the OSL Network source files.

The one downside to that approach might be - it feels less natural to unit test a generic file system feature if the code is hidden in the network generator - so we would likely just end up with the minimal set of features OSL Network generator needs - not the full interface I'm proposing here.

[jstone-lucasfilm]

Thanks for the revision, @ld-kerley! The three callers are now covered, the "Saved translated material" dialog correctly follows the write, and the Graph Editor wrapper is a welcome addition.

On the question of where this API should live, my recommendation would be to keep it in MaterialXFormat, for a few reasons:

• MaterialXFormat is already our file-system layer, providing FilePath::createDirectory, getFilesInDirectory, setCurrentPath, and writeToXmlFile, so directory removal and temporary-directory creation are natural extensions of its existing role. Placing file-system primitives inside of a shader generator would invert that layering, and MaterialXGenOsl already links MaterialXFormat.
• In OSL Network Generator updates #2739, the temporary directory is created within OslNetworkShaderGenerator.cpp rather than in the LibsToOso executable, so relocating the code would not remove it from the shipped library.
• As you note, a generic API in MaterialXFormat is where we can maintain the unit tests for uniqueness, owner-only permissions, symbolic-link safety, and read-only parents, and those tests are exactly the coverage that a security review would want to see.
• With C++17 as the current baseline for MaterialX, removeDirectory and getSystemTemporaryDirectory map directly to std::filesystem calls in the migration discussed above, so a small and well-tested API here is the easiest one to migrate later.

@ashwinbhat, thanks for raising this concern, and I think the substance of it belongs with #2739 rather than with this PR. The API here is opt-in, with the same footprint as our existing write functions for any host that only reads, and the implementation is deliberately conservative. The risk of undeleted temporary folders arises in the on-demand compile path of #2739, which creates a temporary directory without removing it, and I'd suggest we address that in the review of that PR, either by having the generator clean up after itself or by documenting oslTempOsoPath as the mechanism for hosts to direct output to a managed location.

One remaining request follows from my earlier note on LibsToOso. The output directory check at MaterialXGenOsl/LibsToOso.cpp:312 is conditioned on argSkipWritingMtlxDoc, so the directory is created only when the document is skipped, and never when it is written. This issue predates the PR, but setting createDirectories = true at the write site is a one-line fix that would also give this new option its first in-tree user.

With that change, I'd recommend that we include this in v1.39.6.