Uploaded image for project: 'tools.deps'
  1. TDEPS-74

Problem with local deps with local deps

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Completed
    • Labels:
      None

      Description

      Local deps can refer to other local deps with either absolute or relative paths. Absolute paths are unambiguous but not portable across members of a team or when pulling down a monorepo git dep with internal local deps.

      Repro:

      a/
        deps.edn
      b/
        deps.edn
        libs/
          c/deps.edn
       
          
      # in a
      b {:local/root "../b"}
      # in b
      c {:local/root "./libs/c"}
      
      clj -Spath
      src:
      local-deps-bug/a/../b/src
      local-deps-bug/a/libs/c/src
      
      # but should be
      
      local-deps-bug/b/libs/c/src
      
      # or
      
      local-deps-bug/../b/libs/c/src
      

      See sample repo - https://github.com/niquola/local-deps-bug

      Cause: The main problem here is that when using relative paths in the deps.edn, they have to be relative to something. The current assumption made is that they are relative to the current directory (which is wrong once you traverse to a dep).

      Places where relative paths can appear:

      • :local/root - used in a deps.edn local dep to refer to the directory containing the manifest file of the dep. Should be relative to the current deps.edn file.
      • :deps/root - used in a deps.edn git dep to refer to a subdirectory relative to the git working tree root
      • :paths - relative to the location of the manifest file

      Additionally, some environments (like Cursive) do not have a meaningful "current directory" so relying on relative paths at all is prone to error.

      Proposed: While traversing and resolving dependencies, keep track of a directory context. Allow the starting context to be set explicitly on invocation (scripts can default to current directory). It might be convenient for this context to have stack-like properties (as you traverse to a dep you want to push a new directory on the stack etc), however, since the dependency traversal is iterative, not recursive, this may actually not be necessary. There are several possible places to put this context:

      • The config map. Currently the config is basically static. Putting it in the config means we will be modifying it and making it context-dependent, which could increase chance of mistakes.
      • Explicit parameter. Could be passed around in addition to the config. Probably affects dozens of call sites, so messy breaking change.
      • Dynamic var. While I am rarely a fan of implicit state, this may actually be a good place for it. Works well for per-thread (might be useful if we parallelize), has stack properties if needed. Minimizes param changes. With some standard helper functions could minimize the code that uses it.
      • Other ambient state (var/atom, threadlocal, etc) - same implicit downsides as dynamic
        vars and not as good in other ways.

        Attachments

          Activity

            People

            • Assignee:
              alexmiller Alex Miller
              Reporter:
              alex+import import
            • Votes:
              7 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: