Failed to add dependencies. The `dependencies` field wasn't a List of Strings.

[wclr]

I get the following warning when doing spago install some-package

Failed to add dependencies. The dependencies field wasn't a List of Strings.

Spago installs (downloads) the package but, but it doesn't modify the list of packages.

The spago.dhall is:

let package1 = ./packages/package1/spago.dhall

in  { name = "monorepo"
    , dependencies =
          [ "aff"
          , "affjax"
          ]
        # package1.dependencies
    , packages = ./packages.dhall
    , sources =
      [ "packages/**/src/**/*.purs"
      ]
    }

Though I can add the package to the list manually, and it will work, I wonder why spago doesn't support this kind of dependencies config while installation, it is a legal dhall.

[JordanMartinez]

Spago expects the dhall expression in your spago.dhall file to be very simple. So, it doesn't account for things like ListAppend which is what that # means. When it comes across that value, it crashes because there isn't a case for it in the pattern match.

[f-f]

@JordanMartinez I believe this is going to be fixed by your work in #811 right?

[JordanMartinez]

In this particular case, no, because #811 only updates a ListLit if it's on the rightmost side. I can update it to work on the first ListLit it finds by traversing left to right. However, I think we should also have a pass that "cleans up" an expression like ["foo"] # ["bar"] # target.deps to ["foo", "bar"] # target.deps in case one ever uses two ListLits with a ListAppend

[JordanMartinez]

I've updated #811 to add dependencies to the first ListLit it finds. If it doesn't find any (e.g. target1.dependencies # target2.dependencies), then it'll add one to the right end.

[JordanMartinez]

Actually, this issue is a bit more involved when we start dealing with ListAppends. Right now, we only deal with a single ListLit. So, we can take that ListLit's dependencies value and append it to the new packages we're installing, sort it, remove duplicates, and write it back out. In other words, we never need to check whether each package we're trying to install has already been installed elsewhere because there's only one place it can be installed.

[JordanMartinez]

All in all, fixing this issue requires the following work.

1. Figure out what are all the packages already in the dependencies key either directly (i.e. ListLit/ListAppend) or indirectly (i.e. Let binding).
2. Filter out already-added packages from the list of packages the user is requesting Spago install.
3. If the list of packages to install is not empty, append them to the first ListLit we find or add a new ListLit to the end of the list expression if a ListLit doesn't exist.

One question I have after working on this. Does it matter if we ultimately pass the same source glob multiple times into purs?

[wclr]

Does it matter if we ultimately pass the same source glob multiple times into purs?

Why do this?

[JordanMartinez]

It matters because it determines how much bookkeeping I need to do when modifying the raw Dhall AST. Enabling more of Dhall's features in our parsing means accounting for more things that can go wrong, such has installing the same package multiple times.

If we can pass the same globs to PureScript multiple times, then it means I don't need to worry as much about whether the same package has been installed multiple times. For example, if I have package foo installed 3 times in spago.dhall, its source globs (I assume) would be passed 3 times into the purs compile command.

If we can't, then we'll need to ensure we don't pass the same source globs multiple times to purs. I guess this can be solved in two ways:

1. Ensure we never modify the raw AST in such a way that it does install the same package multiple times
2. Deduplicate source globs before calling purs compile.

[f-f]

@JordanMartinez if we change the dependencies from [PackageName] to Set PackageName then we should be set?

https://github.com/purescript/spago/blob/7728ce42a1c66968cce7a72dbb6ab05ea0904d9f/src/Spago/Types.hs#L186-L193

[JordanMartinez]

@f-f That would work!

[f-f]

In progress work for this in #815

[JordanMartinez]

Actually, I fixed this issue separately in #818, which doesn't include any target stuff.

[f-f]

Excellent! :tada:

[f-f]

The new Spago has a simpler configuration format, so this should not be possible anymore