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: strapproach (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.bstfiles). 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 toimport_file() - Are
list_modified_paths()andmark_unmodified()the best approach for whatcomposeneeds? - Is
set_deterministic_user()needed in the public API? -
Directory.stat()returns aos.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 thiswalk()API, perhaps a custom object with a bit more context thanos.stat_resultwould 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.pyand_frontend/widget.pyportions which deal with formatting this data for the user.
- This is used in the frontend in order to implement
- 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
buildboxto use the deterministic mtime in place of any explicitly set mtime.
- Note: deterministic mtime is now handled by not specifying any mtime in cas, and expecting a sandbox backend which stages files from CAS, like
Edited by Tristan Van Berkom