Skip to content

Call bsc.exe directly, when no binaryPath is set in settings #529

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 1, 2022
Merged

Call bsc.exe directly, when no binaryPath is set in settings #529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef9251a
Call bsc.exe directly, when no binaryPath is set in settings
fhammerschmidt Jul 27, 2022
6f574d2
Base bsc.exe lookup on rescript binary lookup
fhammerschmidt Jul 31, 2022
ccd5130
Add changes according review
fhammerschmidt Jul 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions server/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ let platformDir =
// See https://microsoft.github.io/language-server-protocol/specification Abstract Message
// version is fixed to 2.0
export let jsonrpcVersion = "2.0";
export let platformPath = path.join("rescript", platformDir);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: check that platformDir computation is in sync with the compiler.

Copy link
Member Author

@fhammerschmidt fhammerschmidt Jul 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite, in rescript-vscode the platformDir also allows for "linuxarm64", the compiler does not:
https://github.com/rescript-lang/rescript-compiler/blob/master/bsc#L6-L12
https://github.com/rescript-lang/rescript-compiler/blob/master/scripts/bin_path.js#L8-L11

Edit: Or technically also "win32arm64"? Shall I adapt it to be the same as the compiler?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case the compiler is a little behind and could be updated.

Copy link
Member Author

@fhammerschmidt fhammerschmidt Jul 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is the logic in rescript-vscode correct? The compiler has no directories for win32arm64 or linuxarm64, and neither do they get built in CI?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be kind of future-proof, if one wants to build from source.
But does not matter much either way, as this is rarely done I guess.

export let nodeModulesPlatformPath = path.join("node_modules", platformPath);
export let bscExeName = "bsc.exe";
export let bscNativeReScriptPartialPath = path.join(
"node_modules",
"rescript",
platformDir,
"bsc.exe"
nodeModulesPlatformPath,
bscExeName
);

export let analysisDevPath = path.join(
Expand Down
43 changes: 28 additions & 15 deletions server/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,16 @@ let codeActionsFromDiagnostics: codeActions.filesCodeActions = {};
// will be properly defined later depending on the mode (stdio/node-rpc)
let send: (msg: p.Message) => void = (_) => {};

let findBinaryDirPathFromProjectRoot = (
// Check if the rescript binary is available at node_modules/.bin/rescript,
// otherwise recursively check parent directories for it.
let findBinaryPathFromProjectRoot = (
directory: p.DocumentUri // This must be a directory and not a file!
): null | p.DocumentUri => {
let binaryDirPath = path.join(directory, c.nodeModulesBinDir);
let binaryPath = path.join(binaryDirPath, c.rescriptBinName);

if (fs.existsSync(binaryPath)) {
return binaryDirPath;
return binaryPath;
}

let parentDir = path.dirname(directory);
Expand All @@ -95,19 +97,26 @@ let findBinaryDirPathFromProjectRoot = (
return null;
}

return findBinaryDirPathFromProjectRoot(parentDir);
return findBinaryPathFromProjectRoot(parentDir);
};

let getBinaryDirPath = (projectRootPath: p.DocumentUri) =>
extensionConfiguration.binaryPath == null
? findBinaryDirPathFromProjectRoot(projectRootPath)
: extensionConfiguration.binaryPath;

let findRescriptBinary = (projectRootPath: p.DocumentUri) =>
utils.findRescriptBinary(getBinaryDirPath(projectRootPath));

let findBscBinary = (projectRootPath: p.DocumentUri) =>
utils.findBscBinary(getBinaryDirPath(projectRootPath));
extensionConfiguration.binaryPath == null
? findBinaryPathFromProjectRoot(projectRootPath)
: utils.findRescriptBinary(extensionConfiguration.binaryPath);

let findBscBinary = (projectRootPath: p.DocumentUri) => {
let rescriptBinaryPath = findRescriptBinary(projectRootPath);
if (rescriptBinaryPath !== null) {
return path.join(
path.dirname(rescriptBinaryPath),
"..",
c.platformPath,
c.bscExeName
);
}
return null;
};

interface CreateInterfaceRequestParams {
uri: string;
Expand Down Expand Up @@ -312,9 +321,13 @@ let openedFile = (fileUri: string, fileContent: string) => {
method: "window/showMessage",
params: {
type: p.MessageType.Error,
message: `Can't find ReScript binary in the directory ${getBinaryDirPath(
projectRootPath
)}`,
message:
extensionConfiguration.binaryPath == null
? `Can't find ReScript binary in ${path.join(
projectRootPath,
c.nodeModulesBinDir
)} or parent directories. Did you install it? It's required to use "rescript" > 9.1`
: `Can't find ReScript binary in the directory ${extensionConfiguration.binaryPath}`,
},
};
send(request);
Expand Down