Skip to content

Commit

Permalink
fsx: fix regression that made fsxc misbehave
Browse files Browse the repository at this point in the history
In a geewallet's PR[1] I bisected the problem to this commit:
dd66eb3.

So I'm basically reverting that commit here, but I still don't
understand why it would cause such a regression. The problem
is that fsxc is not able to compile anymore certain scripts
and spits some errors that look to be warnings, such as:

```
Error: D:\a\geewallet\geewallet\scripts\bin\downloadUbuntuDeps.fsx.fs(14,1): error FS0020: The result of this expression has type 'int' and is implicitly ignored. Consider using
'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.
[D:\a\geewallet\geewallet\scripts\bin\downloadUbuntuDeps.fsx.fsproj]
```

[1] nblockchain/geewallet#289
  • Loading branch information
knocte committed Sep 3, 2024
1 parent e21c92b commit 8891a9f
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 18 deletions.
44 changes: 27 additions & 17 deletions fsx/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,13 @@ let SplitArgsIntoFsxcArgsAndUserArgs
|> List.ofArray
|> userArgsInternal FsxFsxNotFoundYet List.empty List.empty

let InjectBinSubfolderInPath(userScriptPath: string) =
let InjectBinSubfolderInPath(userScript: FileInfo) =
let userScriptPath = userScript.FullName

if not(userScriptPath.EndsWith ".fsx") then
failwithf "Assertion failed: %s should end with .fsx" userScriptPath
failwithf
"Assertion failed: %s should end with .fsx"
userScript.FullName

let binPath =
match userScriptPath.LastIndexOf Path.DirectorySeparatorChar with
Expand Down Expand Up @@ -149,23 +153,29 @@ match maybeUserScriptPath with
"Compilation of anything that is not an .fsx should have been rejected by fsx"
+ " and shouldn't have reached this point. Please report this bug."
)
| _ -> ()

let finalLaunch =
{
Command =
(InjectBinSubfolderInPath maybeUserScriptPath.Value)
.FullName
Arguments = String.Join(" ", userArgs)
}

let finalProc = Process.Execute(finalLaunch, Echo.OutputOnly)
// FIXME: fsx being an F# project instead of a launcher script means that, on
| Some userScriptPath ->
// FIXME: we shouldn't need to use FileInfo below, and in fact it looks
// dangerous because userScriptPath could be a relative path, but relative
// to what? to current dir or to folder where fsx/fsxc is? so I'm not
// sure what's going when creating the FileInfo instance here, but if we
// try to not use FileInfo then we cause a regression, see this PR:
// https://github.com/nblockchain/fsx/pull/44
let userScriptFile = FileInfo userScriptPath

let finalLaunch =
{
Command = (InjectBinSubfolderInPath userScriptFile).FullName
Arguments = String.Join(" ", userArgs)
}

let finalProc = Process.Execute(finalLaunch, Echo.OutputOnly)

// FIXME: fsx being an F# project instead of a launcher script means that, on
// Windows (and in Unix when installed via 'dotnet tool install fsx'), fsx will be running the user script as
// child process, which may make the memory gains of using fsx instead of fsi/fsharpi
// (as explained in the ReadMe.md file) not as prominent (while in Unix, i.e. Linux and macOS, when the
// tool is not installed via `dotnet tool install fsx`, they still are what ReadMe.md claims because we use a
// bash script which uses 'exec') // TODO: measure measure!
match finalProc.Result with
| Error(exitCode, _errOutput) -> Environment.Exit exitCode
| _ -> Environment.Exit 0
match finalProc.Result with
| Error(exitCode, _errOutput) -> Environment.Exit exitCode
| _ -> Environment.Exit 0
2 changes: 1 addition & 1 deletion version.config
Original file line number Diff line number Diff line change
@@ -1 +1 @@
BaseVersion=0.6.0
BaseVersion=0.6.1

0 comments on commit 8891a9f

Please sign in to comment.