Review public `Directory` API

We should make sure we're happy to support the public Directory API long term before releasing 2.0. Aspects to consider:

  • Do we want to keep the *paths: str approach (variable argument list for individual path components)? It can be convenient to avoid the need for string joining/splitting and eliminates the issue of OS-specific path separators. However, it's different from prevalent filesystem APIs, which may confuse plugin developers. And there are also cases where the API is less convenient (e.g., using paths specified in .bst files). If we decide to switch to a traditional path-based API, I'd always use / as path separator to match REAPI and avoid issues building e.g. Linux artifacts from Windows (using remote execution or Docker).
  • import_files(): Do we need the full complexity of the current API? filter_callback, report_written, FileListResult
  • import_single_file(): Do we want to support specifying a destination path? Maybe rename the method to import_file()
  • Are list_modified_paths() and mark_unmodified() the best approach for what compose needs?
  • Is set_deterministic_user() needed in the public API?
  • Directory.stat() returns a os.stat_result. Would it be better to define a custom, extensible data type instead? !1878 (comment 331809335)
  • Get rid of the undocumented dictionary returned from the CasBasedDirectory.walk() API
    • This is used in the frontend in order to implement bst artifact list-contents
    • To avoid redundancy and duplication, it would be suitable to use the same objects which Directory.stat() returns in this walk() API, perhaps a custom object with a bit more context than os.stat_result would be beneficial here, as we can add more context about the file, such as symbolic link targets.
    • Doing this part should result in better updated documentation of what is going on in the _frontend/cli.py and _frontend/widget.py portions which deal with formatting this data for the user.
  • Consider removing the mark_unmodified() API, as this sets the deterministic mtime only for the local filebased directory, but this is only relevant for builds and we only ever stage into CAS now
    • Note: deterministic mtime is now handled by not specifying any mtime in cas, and expecting a sandbox backend which stages files from CAS, like buildbox to use the deterministic mtime in place of any explicitly set mtime.
Edited by Tristan Van Berkom