From 8891a9fc2a389d49a723bdc72dd8078822eb5aa9 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Tue, 3 Sep 2024 12:31:38 +0800 Subject: [PATCH] fsx: fix regression that made fsxc misbehave In a geewallet's PR[1] I bisected the problem to this commit: dd66eb31d1d5321ee13afd53ff52b6f02e97179d. 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] https://github.com/nblockchain/geewallet/pull/289 --- fsx/Program.fs | 44 +++++++++++++++++++++++++++----------------- version.config | 2 +- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/fsx/Program.fs b/fsx/Program.fs index 6c618936..9d518519 100644 --- a/fsx/Program.fs +++ b/fsx/Program.fs @@ -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 @@ -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 diff --git a/version.config b/version.config index 73711746..9aa266be 100644 --- a/version.config +++ b/version.config @@ -1 +1 @@ -BaseVersion=0.6.0 \ No newline at end of file +BaseVersion=0.6.1