KoffeinFlummi / armake2

Successor to armake written in Rust
GNU General Public License v2.0
50 stars 17 forks source link

Pretty error (or warning) in not found include folder #17

Open jonpas opened 5 years ago

jonpas commented 5 years ago

Right now, armake2 just prints the following when given an in-existing include folder (./inc does not exist in this example):

$ armake2 build addons/main addons/TST_main.pbo -i inc
error: Failed to build PBO:
Failed to parse config:
Failed to preprocess config:
Failed to preprocess include "script_component.hpp":
No such file or directory (os error 2)

First, actual missing include folder name is not printed.

Second, ideally this would just be a warning and folder gets skipped. Maybe in main to not constantly print it, but then programs using armake2 as library would have to implement same checks before passing it to eg. cmd_build.

Krzmbrzl commented 5 years ago

I am strongly against making this a warning instead if an error. Because in at least 90% of all cases the include directory is essential for further processing. So it would only forward the error to be thrown somewhere else. But in my opinion it is better to throw the error right where it occurs as it is then clearer what happened and why it happened. "Fail fast" is the principle that should be applied here

jonpas commented 5 years ago

What you are describing is a problem of "missing include", not a problem of "inexisting includefolder".

Above error gets thrown when you set a folder that doesn't exist as includefolder (argument -i), it's not even an error in includes themselves, it's a hard crash because armake2 can't canonicalize an inexisting folder path.

But error or warning, it should be something, not a panic.

On your "fail fast", that's not always favorable, especially when you want to see all errors/warnings to fix them faster without constant rebuilding. Not really applicable here, but generally to armake2 where possible, which is not in a lot of places because you'd just get a false chain.

Krzmbrzl commented 5 years ago

What you are describing is a problem of "missing include", not a problem of "inexisting includefolder".

No I really was referring to the inexisting include-folder (which would then lead to a missing include). Actually I was saying that if the non-existent folder wasn't treated as an error but a warning instead, armake would very likely throw an error because of a missing include afterwards. And because one tends to look at errors only if the occur, one could oversee the actual root of the problem.

It should be something, not a panic.

This is certainly correct.

especially when you want to see all errors/warnings to fix them faster without constant rebuilding

Agreed. But in a chain of interdependent processes it should be applied. Because an error in the beginning of the chain will most likely create "false-errors" further down the chain as it has to deal with invalid states. Different and independent chains on the other hand should not fail because another one has errored out.

jonpas commented 5 years ago

Agreed.

Maybe unrelated but something that would help HEMTT a great deal: return code separate into "PBO specific error" (syntax error, missing include, etc.) and "global error" (invalid argument like missing include folder, no space on disk etc). Would allow for better user experience as HEMTT could easily determine if it should continue building further PBOs or stop.