Skip to content
Peter Scheibel edited this page Aug 29, 2023 · 16 revisions

Notes from meeting

  • Harmen: What has changed since Windows support was added

    • e.g. decorators for path formatting
    • shutil seems ok with posix paths
    • and powershell
    • everywhere paths have been modified for windows, it was because it failed otherwise
  • John: we could make it so all paths have to be entered as posix paths

    • Harmen, how does .is_absolute work?
    • FWIW constructing PurePath on Linux gives a PurePosixPath, and c://root is not considered absolute
  • John: where can Windows-style paths come from

    • config
    • cli
    • concrete envs (external prefixes)
    • because there are limited places where a Windows-style path comes from, we could intercept all calls
      • e.g. raise an error and reject
      • or auto-convert
  • Harmen: Windows-paths in a YAML file are annoying

    • have to escape \ separators in some circumstances
    • Peter: we can't stop users from improperly escaping \ in YAML
  • John /opt/mnt/c refers to c:/ in WSL/Mingw/etc., could we autotranslate c:/ to /opt/mnt/c on Linux?

    • Peter: preference is fail on c:/
    • Harmen: preference is to let c:/ exist as c:/ on Linux
  • Massimiliano:

    • Let's say we want to auto-convert: how do we break up the changes into reasonably-sized chunks
    • E.g. llnl.util.filesystem is imported in ~150 files
      • John: have a PR that modifies filesystem: https://github.com/spack/spack/pull/36529
        • As an intermediary step, everything passing into .filesystem methods autoconverts pathlib objects to strings (i.e. what filesystem is used to)
    • Once 36529 is merged, we could for example auto-convert all incoming strings in config to pathlib objects and then llnl.util.filesystem will not get confused
      • Peter: there are potentially direct open calls that will get these
      • John: anything that accepts a path-like will accept a pathlib object
        • but in python 3.8 open doesn't (in API) accept path-like
        • 3.11 does
        • Or maybe 3.8 does too: point is we support 3.8 everywhere and we should make sure
    • Massimiliano: will auto-conversion via decorators potentially slow down a loop?
      • e.g. if we are doing something frequently that isn't interacting with the filesystem, like substring matching, it may be slowed down by the decorator
        • review of https://github.com/spack/spack/pull/36529 should take this into account
          • e.g. in that PR, pathlib_filter calls is_url which calls urlparse, is that expensive?
          • system_path_filter is a decorator already used in most filesystem methods and calls urlparse under the hood, so this in particular has been going on for a while without issues
          • also note that this decorator filter is temporary, and ultimately should be replaced with typehints in filesystem
      • Peter: operations that end up interacting with the filesystem or doing I/O are unlikely to be bottlenecked by decorator conversion
  • Massimiliano: can you modify all the caller sites (vs. filesystem method decorators)?

    • Say you have directory_layout module and a function there that accepts a path string
      • typehint that higher-level function to enforce pathlib
      • all calls to filesystem convert to str objects for now
      • finally, once all of the users of filesystem are converted, the filesystem module itself can be updated with pathlib typehints
    • Does decorator interfere with typehints?
    • e.g. let's say we have rename(src, dst), and it is refactored to use pathlib
      • rename(src: pathlib.path, dst: pathlib.path)

Pre-meeting notes

Pain points:

  • One thing that came up for me on https://github.com/spack/spack/pull/39398: pathlib.purepath will construct a PureWindowsPath on windows and a PurePosixPath on linux, which was not sufficient for me because a pathlib.PurePosixPath("c:/...") is not considered absolute
  • In general I think config can introduce posix-style paths into spack, which can form inconsistent paths when joined with relative paths constructed by spack. Consuming all config paths consistently has an appeal, which may require some form of validation to be attached to any config property that represents a path
  • Likewise, env variables may introduce paths and handling them consistently may be challenging
Clone this wiki locally