Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PSA integration changed? #693

Closed
ntwilson opened this issue Oct 16, 2020 · 10 comments · Fixed by #740
Closed

PSA integration changed? #693

ntwilson opened this issue Oct 16, 2020 · 10 comments · Fixed by #740

Comments

@ntwilson
Copy link
Contributor

It seems the psa integration isn't working for me anymore. My build script uses spago build --purs-args "--stash". This works using spago 0.14.0. As soon as I update to 0.15.2, 0.15.3, or 0.16.0, I get the error:

> spago build --purs-args "--stash"

[info] Installation complete.
Invalid option `--stash'

Usage: purs.bin COMMAND
  The PureScript compiler and tools
[error] Failed to build.

Which I'm pretty sure means it's not finding psa anymore and trying to use purs.bin directly. Is this a regression, or did something change about the integration with psa? I have psa installed globally and local to the project. I also have spago installed globally and local to the project. I've tried this with psa versions 0.7.3 and 0.8.0.

OS: Windows 10
spago version: 0.15.2-0.16.0
purescript version: 0.13.6
psa version: 0.7.3-0.8.0

Thanks!

@f-f
Copy link
Member

f-f commented Oct 19, 2020

We had a big refactoring leading to 0.15, so this might be related to #634

A couple of questions:

  • which WSL version are you using?
  • what's the output of spago -V build --purs-args "--stash"?

@ntwilson
Copy link
Contributor Author

welllllll, to be honest, I was using PowerShell and not going through WSL at all.
If I load up WSL (Ubuntu 20.04 LTS (GNU/Linux 4.4.0-19041-Microsoft x86_64)) and try spago 0.16.0, it works!! So it must be a Windows-specific thing.
If I run spago -V build --purs-args "--stash" on my Windows side, I get

2020-10-19 12:07:49.097912: [debug] Running `getGlobalCacheDir`
@(src\Spago\RunEnv.hs:44:7)
2020-10-19 12:07:49.132616: [debug] Transformed config is the same as the read one, not overwriting it
@(src\Spago\Config.hs:364:14)
2020-10-19 12:07:49.212327: [debug] Ensuring that the package set is frozen
@(src\Spago\PackageSet.hs:283:3)
2020-10-19 12:07:49.259202: [debug] Running `spago build`
@(src\Spago\Build.hs:58:3)
2020-10-19 12:07:49.265185: [debug] Getting transitive deps
@(src\Spago\Packages.hs:129:3)
2020-10-19 12:07:49.278151: [debug] Running `fetchPackages`
@(src\Spago\FetchPackage.hs:41:3)
2020-10-19 12:07:49.286130: [debug] Checking if `purs` is up to date
@(src\Spago\PackageSet.hs:191:3)
2020-10-19 12:07:49.361928: [debug] Compiling with "purs.cmd"
@(src\Spago\Purs.hs:29:3)
2020-10-19 12:07:49.368919: [debug] Running command: `purs.cmd compile --stash ".spago/aff/v5.1.2/src/**/*.purs" ".spago/aff-promise/v2.1.0/src/**/*.purs" ".spago/affjax/v10.0.0/src/**/*.purs" ".spago/ansi/v5.0.0/src/**/*.purs" ".spago/argonaut/v6.0.0/src/**/*.purs" ".spago/argonaut-codecs/v6.0.2/src/**/*.purs" ".spago/argonaut-core/v5.0.2/src/**/*.purs" ".spago/argonaut-traversals/v7.0.0/src/**/*.purs" ".spago/arraybuffer-types/v2.0.0/src/**/*.purs" ".spago/arrays/v5.3.1/src/**/*.purs" ".spago/avar/v3.0.0/src/**/*.purs" ".spago/bifunctors/v4.0.0/src/**/*.purs" ".spago/browser-cookies/v0.0.1/src/**/*.purs" ".spago/catenable-lists/v5.0.1/src/**/*.purs" ".spago/console/v4.4.0/src/**/*.purs" ".spago/const/v4.1.0/src/**/*.purs" ".spago/contravariant/v4.0.1/src/**/*.purs" ".spago/control/v4.2.0/src/**/*.purs" ".spago/datetime/v4.1.1/src/**/*.purs" ".spago/datetime-iso/v4.0.0/src/**/*.purs" ".spago/debug/v4.0.0/src/**/*.purs" ".spago/distributive/v4.0.0/src/**/*.purs" ".spago/effect/v2.0.1/src/**/*.purs" ".spago/either/v4.1.1/src/**/*.purs" ".spago/enums/v4.0.1/src/**/*.purs" ".spago/exceptions/v4.0.0/src/**/*.purs" ".spago/exists/v4.0.0/src/**/*.purs" ".spago/fixed-points/v5.1.0/src/**/*.purs" ".spago/foldable-traversable/v4.1.1/src/**/*.purs" ".spago/foreign/v5.0.0/src/**/*.purs" ".spago/foreign-object/v2.0.3/src/**/*.purs" ".spago/fork/v4.0.0/src/**/*.purs" ".spago/form-urlencoded/v5.0.0/src/**/*.purs" ".spago/formatters/v4.0.1/src/**/*.purs" ".spago/free/v5.2.0/src/**/*.purs" ".spago/functions/v4.0.0/src/**/*.purs" ".spago/functors/v3.1.1/src/**/*.purs" ".spago/gen/v2.1.1/src/**/*.purs" ".spago/generics-rep/v6.1.1/src/**/*.purs" ".spago/globals/v4.1.0/src/**/*.purs" ".spago/http-methods/v4.0.2/src/**/*.purs" ".spago/identity/v4.1.0/src/**/*.purs" ".spago/indexed-monad/v1.2.0/src/**/*.purs" ".spago/integers/v4.0.0/src/**/*.purs" ".spago/invariant/v4.1.0/src/**/*.purs" ".spago/js-date/v6.0.0/src/**/*.purs" ".spago/js-timers/v4.0.1/src/**/*.purs" ".spago/lazy/v4.0.0/src/**/*.purs" ".spago/lcg/v2.0.0/src/**/*.purs"
 ".spago/lists/v5.4.1/src/**/*.purs" ".spago/math/v2.1.1/src/**/*.purs" ".spago/maybe/v4.0.1/src/**/*.purs" ".spago/media-types/v4.0.1/src/**/*.purs" ".spago/mmorph/v5.1.0/src/**/*.purs" ".spago/newtype/v3.0.0/src/**/*.purs" ".spago/node-buffer/v6.0.0/src/**/*.purs" ".spago/node-fs/v5.0.1/src/**/*.purs" ".spago/node-path/v3.0.0/src/**/*.purs" ".spago/node-streams/v4.0.1/src/**/*.purs" ".spago/nonempty/v5.0.0/src/**/*.purs" ".spago/now/v4.0.0/src/**/*.purs" ".spago/nullable/v4.1.1/src/**/*.purs" ".spago/numbers/v7.0.0/src/**/*.purs" ".spago/options/v5.0.0/src/**/*.purs" ".spago/ordered-collections/v1.6.1/src/**/*.purs" ".spago/orders/v4.0.0/src/**/*.purs" ".spago/parallel/v4.0.0/src/**/*.purs" ".spago/parsing/v5.0.3/src/**/*.purs" ".spago/partial/v2.0.1/src/**/*.purs" ".spago/pipes/v6.0.0/src/**/*.purs" ".spago/prelude/v4.1.1/src/**/*.purs" ".spago/profunctor/v4.1.0/src/**/*.purs" ".spago/profunctor-lenses/v6.3.0/src/**/*.purs" ".spago/proxy/v3.0.0/src/**/*.purs" ".spago/psci-support/v4.0.0/src/**/*.purs" ".spago/quickcheck/v6.1.0/src/**/*.purs" ".spago/random/v4.0.0/src/**/*.purs" ".spago/react-basic/v13.0.0/src/**/*.purs" ".spago/react-basic-hooks/v5.0.0/src/**/*.purs" ".spago/record/v2.0.2/src/**/*.purs" ".spago/refs/v4.1.0/src/**/*.purs" ".spago/remotedata/v4.2.0/src/**/*.purs" ".spago/spec/v4.0.1/src/**/*.purs" ".spago/spec-discovery/v4.0.0/src/**/*.purs" ".spago/spec-quickcheck/v3.1.0/src/**/*.purs" ".spago/st/v4.1.1/src/**/*.purs" ".spago/string-parsers/v5.0.1/src/**/*.purs" ".spago/strings/v4.0.1/src/**/*.purs" ".spago/stringutils/v0.0.10/src/**/*.purs" ".spago/tailrec/v4.1.1/src/**/*.purs" ".spago/transformers/v4.2.0/src/**/*.purs" ".spago/tuples/v5.1.0/src/**/*.purs" ".spago/type-equality/v3.0.0/src/**/*.purs" ".spago/typelevel-prelude/v5.0.2/src/**/*.purs" ".spago/unfoldable/v4.1.0/src/**/*.purs" ".spago/unicode/v4.0.1/src/**/*.purs" ".spago/unsafe-coerce/v4.0.0/src/**/*.purs" ".spago/unsafe-reference/v3.0.1/src/**/*.purs" ".spago/validation/v4.2.0/src/**/*.purs" ".spago/web-dom/v4.0.1/src/**/*.purs" ".spago/web-events/v2.0.1/src/**/*.purs" ".spago/web-file/v2.3.0/src/**/*.purs" ".spago/web-html/v2.3.0/src/**/*.purs" ".spago/web-storage/v3.0.0/src/**/*.purs" ".spago/web-xhr/v3.0.2/src/**/*.purs" "src/**/*.purs"`
@(src\Spago\Purs.hs:121:3)
Invalid option `--stash'

Usage: purs.bin COMMAND
  The PureScript compiler and tools
2020-10-19 12:07:49.612664: [error] Failed to build.
@(src\Spago\Prelude.hs:127:13)

@f-f f-f added the windows label Oct 19, 2020
@f-f
Copy link
Member

f-f commented Oct 19, 2020

@ntwilson I see, it looks like psa cannot be found, and now I recall that the logic around that changed (quite ironically to make Spago work on my non-WSL Windows installation). This is the current logic that decides if psa should be used:

spago/src/Spago/RunEnv.hs

Lines 116 to 130 in 9fe8bfa

getPurs :: HasLogFunc env => UsePsa -> RIO env Text
getPurs usePsa = do
-- first we decide if we _want_ to use psa, then if we _can_
pursCandidate <- case usePsa of
NoPsa -> pure "purs"
UsePsa -> findExecutable "psa" >>= \case
Just _ -> pure "psa"
Nothing -> pure "purs"
-- We first try this for Windows
case OS.buildOS of
OS.Windows -> do
findExecutable (pursCandidate <> ".cmd") >>= \case
Just _ -> pure (Text.pack pursCandidate <> ".cmd")
Nothing -> findExecutableOrDie pursCandidate
_ -> findExecutableOrDie pursCandidate

Now, the place where we decide if we should use psa is the findExecutable function, which basically just calls the same function in the directory library:

spago/src/Spago/Prelude.hs

Lines 228 to 237 in 9fe8bfa

-- | Return the full path of the executable we're trying to call,
-- or die trying
findExecutableOrDie :: HasLogFunc env => String -> RIO env Text
findExecutableOrDie cmd = do
Directory.findExecutable cmd >>= \case
Nothing -> die [ "Executable was not found in path: " <> displayShow cmd ]
-- Note: we ignore the path and just return the input because the one we get
-- here is absolute, and Windows doesn't seem to be able to deal with that.
-- See: https://github.com/purescript/spago/issues/635
Just _path -> pure $ Text.pack cmd

I am not sure if we can do anything at all about this. @stkb any thoughts on this? 🤔

@kim366
Copy link

kim366 commented Dec 3, 2020

It would be nice if spago could fetch PSA from a per-project install with npx psa. Although detection would probably involve looking at the package.json. I just don't want to add PSA flags in the npm run script and in the future just having it output "Build failed".

The --purs-binary from #252 would actually work fine for this as well

@ntwilson
Copy link
Contributor Author

ntwilson commented Feb 2, 2021

@f-f, can I ask how your non-WSL Windows installation is setup? Perhaps I can mirror the same thing. I'd love to have the latest spago changes, but right now I'm stuck on 0.14.0 because I can't switch to WSL.

@f-f
Copy link
Member

f-f commented Feb 2, 2021

@ntwilson it's just a vanilla Windows 10 where I installed the tooling with npm install purs spago

@ntwilson
Copy link
Contributor Author

ntwilson commented Feb 2, 2021

Do you have PSA installed at all, and are you able to use PSA with spago?

@f-f
Copy link
Member

f-f commented Feb 2, 2021

I don't use psa at all. I can try installing it and see if I can replicate

@ntwilson
Copy link
Contributor Author

ntwilson commented Feb 2, 2021

Oh, sorry, I think I just misunderstood your comment above.

to make Spago work on my non-WSL Windows installation

I thought you meant to make it work with PSA, and I was curious how you did it. If there's anything I can do to try to resolve this, I'm happy to help, but I'll confess I've got very little familiarity with Haskell + Windows + file systems, so I'd potentially need some guidance on all the edge cases we're trying to support.

@stkb
Copy link
Contributor

stkb commented Feb 3, 2021

I can repro and will look into this.

Update: PR will be incoming.

@f-f f-f added the bug label Feb 3, 2021
stkb added a commit to stkb/spago that referenced this issue Feb 3, 2021
In `getPurs` we were just using the standard `Directory.findExecutable`
to look for `psa`, forgetting to check for `psa.cmd` too. In windows
this was looking for `psa.exe`, which doesn't exist.

To help prevent this happening again, a self-written `findExecutable`
function in `Prelude` is added, instead of just exporting
`Directory.findExecutable`. This new version will always first check
for a `.cmd` version of the executable name on Windows.
stkb added a commit to stkb/spago that referenced this issue Feb 3, 2021
In `getPurs` we were just using the standard `Directory.findExecutable`
to look for `psa`, forgetting to check for `psa.cmd` too. In windows
this was looking for `psa.exe`, which doesn't exist.

To help prevent this happening again, a self-written `findExecutable`
function in `Prelude` is added, instead of just exporting
`Directory.findExecutable`. This new version will always first check
for a `.cmd` version of the executable name on Windows.
stkb added a commit to stkb/spago that referenced this issue Feb 3, 2021
In `getPurs` we were just using the standard `Directory.findExecutable`
to look for `psa`, forgetting to check for `psa.cmd` too. In windows
this was looking for `psa.exe`, which doesn't exist.

To help prevent this happening again, a self-written `findExecutable`
function in `Prelude` is added, instead of just exporting
`Directory.findExecutable`. This new version will always first check
for a `.cmd` version of the executable name on Windows.
stkb added a commit to stkb/spago that referenced this issue Feb 3, 2021
In `getPurs` we were just using the standard `Directory.findExecutable`
to look for `psa`, forgetting to check for `psa.cmd` too. In windows
this was looking for `psa.exe`, which doesn't exist.

To help prevent this happening again, a self-written `findExecutable`
function in `Prelude` is added, instead of just exporting
`Directory.findExecutable`. This new version will always first check
for a `.cmd` version of the executable name on Windows.
@f-f f-f closed this as completed in #740 Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants