Skip to content

Wrong use of configuration fortran.linter.compilerPath in file linter-provider.ts #492

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

Closed
maroony opened this issue May 25, 2022 · 6 comments · Fixed by #503
Closed

Wrong use of configuration fortran.linter.compilerPath in file linter-provider.ts #492

maroony opened this issue May 25, 2022 · 6 comments · Fixed by #503

Comments

@maroony
Copy link

maroony commented May 25, 2022

Describe the bug
When setting this option like

"fortran.linter.compilerPath": "/usr/bin/gfortran-9",

the function getLinterExecutable() in linter-provider.ts will return this compiler path:

using linter: gfortran located in: /usr/bin/gfortran-9/gfortran

This is wrong! It has to be /usr/bin/gfortran-9. The linting feature isn't working any more.

Build info (please complete the following information):

  • OS: Remote Development on Linux (SLES 12)
  • Extension Version v3.0.2022042917
  • Visual Studio Code Version 1.67.2
@maroony maroony added the bug label May 25, 2022
@gnikit gnikit removed the bug label May 25, 2022
@maroony maroony changed the title Wrong use of configuration fortran.linter.compilerPath linter-provider.ts Wrong use of configuration fortran.linter.compilerPath in file linter-provider.ts May 25, 2022
@gnikit
Copy link
Member

gnikit commented May 25, 2022

This is not really a bug, the path is meant to be the rootpath of the binary, i.e. /usr/bin . The option fortran.linter.compiler is what determines the compiler to be used. Together the 2 become /usr/bin/gfortran.

There are 2 reasons why we don't allow you to just give the an arbitrary binary path:

  1. It is a massive security risk to allow an extension to execute arbitrary programs on your computer (especially upon file modification)
  2. There is no easy way to identify which compiler you are using i.e. gfortran or ifort (or flang and lfortran in the future)

Number 2. can be addressed by allowing you to specify the full path in fortran.linter.compilerPath and using fortran.linter.compiler to interpret the diagnostics served by the compiler. This however can lead to people using for example an Intel compiler for linting but parsing its diagnostics with gfortran, which would obviously not work.

@gnikit
Copy link
Member

gnikit commented May 25, 2022

I obviously understand how this might be a bummer since not all gfortran versions have a binary alias without the version appended at the end

@maroony
Copy link
Author

maroony commented May 25, 2022

In the documentation a full path is used:

The linter executable is assumed to be found in the PATH. In order to use a different executable or if the executable can't be found in the PATH you can point the extension to another linter with the fortran.linter.compilerPath option.

{
"fortran.linter.compilerPath": "/opt/oneapi/compiler/2022.0.2/linux/bin/intel64/ifort"
}

@maroony
Copy link
Author

maroony commented May 25, 2022

When speaking of a path, usually the full string with the binary should be set. Otherwise speak of compilerDir or compilerFolder.

Compare this to maven.executable.path for example.

@gnikit
Copy link
Member

gnikit commented May 25, 2022

in the README you are right it should not have ifort appended in the end of that path. I will fix it. In general, I think that I might just remove the tricks with the compiler paths and leave it up to the user to not pass unsafe binaries to the linter.

@maroony
Copy link
Author

maroony commented May 30, 2022

That's exactly what I think: Those tricks with the compiler path (here: directory) and the name are way too complicated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants