-
Notifications
You must be signed in to change notification settings - Fork 19
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
Correct paths windows backslashes and spawn shell param. #112
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,10 +19,9 @@ import Data.Monoid (guard, power) | |
import Data.Newtype (over2, unwrap) | ||
import Data.Number.Format as Number | ||
import Data.Ord.Down (Down(..)) | ||
import Data.Posix.Signal (Signal(..)) | ||
import Data.Set (Set) | ||
import Data.Set as Set | ||
import Data.String (Pattern(..)) | ||
import Data.String (Pattern(..), Replacement(..)) | ||
import Data.String as String | ||
import Data.String.CodeUnits as SCU | ||
import Data.Traversable (traverse) | ||
|
@@ -36,9 +35,11 @@ import Effect.Class.Console as Console | |
import Effect.Now (now) | ||
import Effect.Ref (Ref) | ||
import Effect.Ref as Ref | ||
import Node.ChildProcess (Exit(..), StdIOBehaviour(..), defaultSpawnOptions) | ||
import Node.ChildProcess (exitH) | ||
import Node.ChildProcess as ChildProcess | ||
import Node.ChildProcess.Types (Exit(..), enableShell, pipe, shareStream, stringSignal) | ||
import Node.Encoding (Encoding(..)) | ||
import Node.EventEmitter (on_) | ||
import Node.FS.Aff (writeTextFile) | ||
import Node.FS.Aff as FS | ||
import Node.FS.Perms as Perms | ||
|
@@ -47,7 +48,9 @@ import Node.FS.Stream (createReadStream, createWriteStream) | |
import Node.Path (FilePath) | ||
import Node.Path as Path | ||
import Node.Process as Process | ||
import Node.Stream (errorH, finishH) | ||
import Node.Stream as Stream | ||
import Node.UnsafeChildProcess.Unsafe as UnsafeChildProcess | ||
import PureScript.Backend.Optimizer.Codegen.EcmaScript (codegenModule, esModulePath) | ||
import PureScript.Backend.Optimizer.Codegen.EcmaScript.Builder (basicBuildMain, externalDirectivesFromFile) | ||
import PureScript.Backend.Optimizer.Codegen.EcmaScript.Foreign (esForeignSemantics) | ||
|
@@ -223,7 +226,7 @@ type BuildState = | |
} | ||
|
||
main :: FilePath -> Effect Unit | ||
main cliRoot = | ||
main cliRoot = do | ||
parseArgs >>= case _ of | ||
Left err -> | ||
Console.error $ ArgParser.printArgError err | ||
|
@@ -341,8 +344,11 @@ main cliRoot = | |
, "--outfile=" <> bundleArgs.targetFile | ||
, "--bundle" | ||
] | ||
if shouldInvokeMain then do | ||
spawnFromParentWithStdin "esbuild" esBuildArgs $ Just $ "import { main } from '" <> entryPath <> "'; main();" | ||
fixBackSlash = String.replaceAll (Pattern "\\") (Replacement "\\\\") | ||
if shouldInvokeMain then | ||
spawnFromParentWithStdin "esbuild" esBuildArgs | ||
$ Just | ||
$ "import { main } from '" <> fixBackSlash entryPath <> "'; main();" | ||
else | ||
spawnFromParentWithStdin "esbuild" (Array.snoc esBuildArgs entryPath) Nothing | ||
|
||
|
@@ -354,19 +360,17 @@ copyFile from to = do | |
makeAff \k -> do | ||
src <- createReadStream from | ||
dst <- createWriteStream to | ||
res <- Stream.pipe src dst | ||
Stream.onError src (k <<< Left) | ||
Stream.onError dst (k <<< Left) | ||
Stream.onError res (k <<< Left) | ||
Stream.onFinish res (k (Right unit)) | ||
Stream.pipe src dst | ||
src # on_ errorH (k <<< Left) | ||
dst # on_ errorH (k <<< Left) | ||
dst # on_ finishH (k (Right unit)) | ||
pure $ effectCanceler do | ||
Stream.destroy res | ||
Stream.destroy dst | ||
Stream.destroy src | ||
|
||
writeString :: forall r. Stream.Writable r -> String -> Aff Unit | ||
writeString stream str = makeAff \k -> do | ||
_ <- Stream.writeString stream UTF8 str (k <<< maybe (Right unit) Left) | ||
_ <- Stream.writeString' stream UTF8 str (k <<< maybe (Right unit) Left) | ||
pure nonCanceler | ||
|
||
mkdirp :: FilePath -> Aff Unit | ||
|
@@ -377,24 +381,26 @@ esModulePackageJson = """{"type": "module"}""" | |
|
||
spawnFromParentWithStdin :: String -> Array String -> Maybe String -> Aff Unit | ||
spawnFromParentWithStdin command args input = makeAff \k -> do | ||
childProc <- ChildProcess.spawn command args defaultSpawnOptions | ||
{ stdio = | ||
[ Just $ Pipe | ||
, Just $ ShareStream (unsafeCoerce Process.stdout) | ||
, Just $ ShareStream (unsafeCoerce Process.stderr) | ||
-- To preserve colors in stdout need to pass it into spawn's stdio. | ||
childProc <- unsafeCoerce <$> UnsafeChildProcess.spawn' command args | ||
{ stdio: | ||
[ pipe | ||
, shareStream Process.stdout | ||
, shareStream Process.stderr | ||
] | ||
, shell: enableShell | ||
} | ||
for_ input \inp -> do | ||
_ <- Stream.writeString (ChildProcess.stdin childProc) UTF8 inp mempty | ||
Stream.end (ChildProcess.stdin childProc) mempty | ||
ChildProcess.onExit childProc case _ of | ||
_ <- Stream.writeString (ChildProcess.stdin childProc) UTF8 inp | ||
Stream.end (ChildProcess.stdin childProc) | ||
childProc # on_ exitH case _ of | ||
Normally code | ||
| code > 0 -> Process.exit code | ||
| code > 0 -> Process.exit' code | ||
| otherwise -> k (Right unit) | ||
BySignal _ -> | ||
Process.exit 1 | ||
Process.exit' 1 | ||
pure $ effectCanceler do | ||
ChildProcess.kill SIGABRT childProc | ||
void $ ChildProcess.kill' (stringSignal "SIGABRT") childProc | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this use https://pursuit.purescript.org/packages/purescript-node-child-process/11.1.0/docs/Node.ChildProcess#v:killSignal to avoid the string? These functions are largely undocumented so I'm not really sure what the difference is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missed this functions. Seems it it does the same just converts Signal to string and passes it to kill'. |
||
|
||
timeDiff :: Instant -> Instant -> Milliseconds | ||
timeDiff = over2 Milliseconds (flip (-)) `on` Instant.unInstant | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,42 +4,45 @@ import Prelude | |
|
||
import Data.Either (Either(..)) | ||
import Data.Maybe (Maybe(..)) | ||
import Data.Posix.Signal (Signal(..)) | ||
import Effect (Effect) | ||
import Effect.Aff (Aff, Error, effectCanceler, error, makeAff, throwError) | ||
import Effect.Class (liftEffect) | ||
import Node.Buffer (Buffer, freeze) | ||
import Node.Buffer.Immutable as ImmutableBuffer | ||
import Node.ChildProcess (ChildProcess, ExecResult, Exit(..), defaultExecOptions) | ||
import Node.ChildProcess (ChildProcess, ExecResult, exitH) | ||
import Node.ChildProcess as ChildProcess | ||
import Node.ChildProcess.Types (Exit(..), stringSignal) | ||
import Node.Encoding (Encoding(..)) | ||
import Node.EventEmitter (on_) | ||
import Node.FS.Aff as FS | ||
import Node.FS.Perms (mkPerms) | ||
import Node.FS.Perms as Perms | ||
import Node.FS.Stats as Stats | ||
import Node.FS.Stream (createReadStream, createWriteStream) | ||
import Node.Path (FilePath) | ||
import Node.Process as Process | ||
import Node.Stream (errorH, finishH) | ||
import Node.Stream as Stream | ||
|
||
spawnFromParent :: String -> Array String -> Aff Unit | ||
spawnFromParent command args = makeAff \k -> do | ||
childProc <- spawnImpl command args | ||
ChildProcess.onExit childProc case _ of | ||
childProc # on_ exitH case _ of | ||
Normally code | ||
| code > 0 -> Process.exit code | ||
| code > 0 -> Process.exit' code | ||
| otherwise -> k (Right unit) | ||
BySignal _ -> | ||
Process.exit 1 | ||
Process.exit' 1 | ||
pure $ effectCanceler do | ||
ChildProcess.kill SIGABRT childProc | ||
void $ ChildProcess.kill' (stringSignal "SIGABRT") childProc | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question here regarding |
||
|
||
execWithStdin :: String -> String -> Aff ExecResult | ||
execWithStdin command input = makeAff \k -> do | ||
childProc <- ChildProcess.exec command defaultExecOptions (k <<< pure) | ||
_ <- Stream.writeString (ChildProcess.stdin childProc) UTF8 input mempty | ||
Stream.end (ChildProcess.stdin childProc) mempty | ||
pure $ effectCanceler $ ChildProcess.kill SIGABRT childProc | ||
childProc <- ChildProcess.exec' command identity (k <<< pure) | ||
_ <- Stream.writeString' (ChildProcess.stdin childProc) UTF8 input mempty | ||
--Stream.end (ChildProcess.stdin childProc) mempty | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this comment be removed? |
||
(ChildProcess.stdin childProc) # on_ finishH mempty | ||
pure $ effectCanceler $ void $ ChildProcess.kill' (stringSignal "SIGABRT") childProc | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
bufferToUTF8 :: Buffer -> Aff String | ||
bufferToUTF8 = liftEffect <<< map (ImmutableBuffer.toString UTF8) <<< freeze | ||
|
@@ -62,15 +65,14 @@ copyFile from to = do | |
stats <- FS.stat from | ||
unless (Stats.isFile stats) do | ||
throwError $ error $ "Not a file: " <> from | ||
--FS.copyFile from to | ||
makeAff \k -> do | ||
src <- createReadStream from | ||
dst <- createWriteStream to | ||
res <- Stream.pipe src dst | ||
Stream.onError src (k <<< Left) | ||
Stream.onError dst (k <<< Left) | ||
Stream.onError res (k <<< Left) | ||
Stream.onFinish res (k (Right unit)) | ||
Stream.pipe src dst | ||
src # on_ errorH (k <<< Left) | ||
dst # on_ errorH (k <<< Left) | ||
dst # on_ finishH (k (Right unit)) | ||
pure $ effectCanceler do | ||
Stream.destroy res | ||
Stream.destroy dst | ||
Stream.destroy src |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the unsafeCoerce needed here? Presumably the
map
is unnecessary?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because below used
ChildProcess.stdin
and other functions for safe ChildProcess, if to useunsafeStdin
it returns Nullable Writtable, it should be handled, map can be ommited yes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you suppose it is more correct not ot use unsafeCoerce here, I can fix this too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.