compiler-explorer / compiler-explorer

Run compilers interactively from your web browser and interact with the assembly
https://godbolt.org/
BSD 2-Clause "Simplified" License
15.73k stars 1.67k forks source link

[BUG]: Missing auto-disabling for various more recent compilers #4325

Open jrtc27 opened 1 year ago

jrtc27 commented 1 year ago

Describe the bug

For most tools, Compiler Explorer will attempt to stat it and, if it doesn't exist, disable that tool, with the following kind of log message:

warn: Unable to stat gfortran compiler binary:  ENOENT: no such file or directory, stat '/usr/bin/gfortran' {"code":"ENOENT","errno":-2,"path":"/usr/bin/gfortran","stack":"Error: ENOENT: no such file or directory, stat '/usr/bin/gfortran'","syscall":"stat"}

However, for a handful of tools, this does not end up happening when they don't exist, and they end up instead giving the following errors:

error: Unable to resolve 'todo-default-path'
error: Unable to resolve 'circle'
error: Unable to resolve 'cppfront'
error: Unable to resolve 'hlsl_clang_default'
error: Unable to resolve 'hook'
error: Unable to resolve 'ocamlopt'

(Note that todo-default-path is the entertaining default name for Carbon, overridden to /.../carbon-explorer in the Amazon config files)

Whilst I don't particularly care about the errors in a log file, this has the side-effect of Compiler Explorer not disabling them automatically and so they end up as selectable in the web UI, only to give an error like the following in the Output tab when used:

Internal Compiler Explorer error: Error: spawn todo-default-path ENOENT
    at Process.ChildProcess._handle.onexit (node:internal/child_process:285:19)
    at onErrorNT (node:internal/child_process:485:16)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)
Compiler returned: -1

These tools should behave like the rest and be disabled in this case, not enabled and broken.

Steps to reproduce

  1. Run Compiler Explorer in an environment without any of the mentioned tools (and no config to override them to something that does exist)

Expected behavior

The tools are disabled automatically like most others

Reproduction link

Not applicable

Screenshots

Not applicable

Operating System

No response

Browser version

No response

jrtc27 commented 1 year ago

Probably https://github.com/compiler-explorer/compiler-explorer/blob/f276f73dab8e82c775e9db52505ec0c2a4655f0f/lib/handlers/compile.ts#L180 should have a return null; after it like the absolute path case? Though I don't understand why one's a warning and one's an error, though, nor do I understand why compiler.exe is permitted to be false-y either.

mattgodbolt commented 1 year ago

(Note that todo-default-path is the entertaining default name for Carbon, overridden to /.../carbon-explorer in the Amazon config files)

😊 oops that me....

jrtc27 commented 1 year ago

Probably

https://github.com/compiler-explorer/compiler-explorer/blob/f276f73dab8e82c775e9db52505ec0c2a4655f0f/lib/handlers/compile.ts#L180

should have a return null; after it like the absolute path case? Though I don't understand why one's a warning and one's an error, though, nor do I understand why compiler.exe is permitted to be false-y either.

Does anyone have any opinions on whether this is the right fix to make or something else should be done instead? As things stand the inconsistency is odd and this bug needs fixing one way or another.

AbrilRBS commented 1 year ago

Let me get back to you on this this weekend, sorry this issue fell thru the cracks :/

jrtc27 commented 1 year ago

Gentle ping as it's past the weekend in case you forgot about this

AbrilRBS commented 1 year ago

Indeed, I was short on time and couldn't get to this :( Thanks for the heads-up!

jrtc27 commented 1 year ago

Ping? I'm running our instance with https://github.com/CTSRD-CHERI/cheri-docker-images/commit/99c280bb044d0e87c5bcb48f1ce7d6069e5534d7 which resolves this in our environment but don't like carrying patches indefinitely where possible (currently the only other one we have is to add .chericap as a recognised directive).

AbrilRBS commented 1 year ago

Hi! Sorry about the radio silence, life has been a bit busy for me lately. After looking at it a bit more, returning null for cases like this should be ok, so we would be happy to accept your patch as a PR :)

Again, sorry for the extreme delay dealing with this :/