Incorrect Parsing of `-with-bear` and `-bear-output-dir` in `AllwmakeParseArguments`

Summary

-with-bear accepts the optional argument -bear-output-dir to set the compile_commands.json output directory. -with-bear -bear-output-dir=/dev/null, for example, produces

$ ./Allwmake -with-bear -bear-output-dir=/dev/null
Use bear 3.1.5
Output = /Users/ali/OpenFOAM/OpenFOAM-v2412/build/darwin64ClangDPInt64Debug/compile_commands.json

Error encountered:
    unknown option: '-bear-output-dir=/dev/null'

See 'wmake -help' for usage
 or 'wmake -help-full' for extended usage
...

It seems that AllwmakeParseArguments does not parse -bear-output-dir immediately following -with-bear. For example, sh -x ./Allwmake -j -s -with-bear -bear-output-dir=/dev/null results in

...
++ exec wmake -with-bear -all -j -s -bear-output-dir=/dev/null
...

Here's a list of files that might be related to this issue:

• $WM_DIR/scripts/AllwmakeParseArguments
• $WM_DIR/scripts/wmake-with-bear

Steps to reproduce

1. Run ./Allwmake -with-bear -bear-output-dir=$HOME

Example case

What is the current bug behaviour?

Using -bear-output-dir with -with-bear causes the path to be ignored and an error to be emitted.

What is the expected correct behavior?

compile_commands.json is stored in the requested directory with no error.

Relevant logs and/or images

Environment information

• OpenFOAM version : v2412 (also v2312)
• Operating system : macOS (Sonoma 14.7)
• Hardware info :
• Compiler : clang

Possible fixes

Here's the diff of $WM_DIR/scripts/AllwmakeParseArguments

diff --git a/wmake/scripts/AllwmakeParseArguments b/wmake/scripts/AllwmakeParseArguments
index 63a7501a30..4e8148105d 100644
--- a/wmake/scripts/AllwmakeParseArguments
+++ b/wmake/scripts/AllwmakeParseArguments
@@ -88,7 +88,7 @@ USAGE
 #
 #------------------------------------------------------------------------------
 
-unset wmakeOpt_frontend wmakeOpt_nonRecursive
+unset wmakeOpt_frontend wmakeOpt_withBearOutputDir wmakeOpt_nonRecursive
 unset wmakeOpt_debug wmakeOpt_log wmakeOpt_openmp wmakeOpt_strict wmakeOpt_queue
 
 for arg in "$@"
@@ -104,9 +104,23 @@ do
     -with-bear)
         # Use 'bear' as frontend wrapper to wmake
         wmakeOpt_frontend="-with-bear"
+
+        # Check if the next argument is `-bear-output-dir=*`
+        if [ "${1#-bear-output-dir=}" != "$1" ]; then
+            wmakeOpt_withBearOutputDir="$1"
+
+            # Remove `-bear-output-dir=*` from the list of arguments
+            shift
+        fi
+
         continue    # Argument handled, remove it
         ;;
 
+    -bear-output-dir=*)
+        # It is already handled by `-with-bear`
+        continue
+        ;;
+
     -no-recurs* | -fromWmake)
         # Avoid recursion (eg, if called from wmake)
         wmakeOpt_nonRecursive=true
@@ -214,13 +228,13 @@ if [ -z "$wmakeOpt_nonRecursive" ]
 then
     if [ -z "$wmakeOpt_log" ]
     then
-        exec wmake $wmakeOpt_frontend -all \
+        exec wmake $wmakeOpt_frontend "$wmakeOpt_withBearOutputDir" -all \
             $wmakeOpt_debug $wmakeOpt_strict $wmakeOpt_queue $wmakeOpt_openmp $*
         exit $? # Unneeded, but just in case something went wrong
     else
         echo "Logging wmake -all output to '$wmakeOpt_log'" 1>&2
         echo 1>&2
-        exec wmake $wmakeOpt_frontend -all \
+        exec wmake $wmakeOpt_frontend "$wmakeOpt_withBearOutputDir" -all \
             $wmakeOpt_debug $wmakeOpt_strict $wmakeOpt_queue $wmakeOpt_openmp $* 2>&1 | \
             /usr/bin/tee $wmakeOpt_log
         # Need to cleanup after the tee
@@ -245,9 +259,11 @@ fi
 # Cleanup local variables and functions
 #------------------------------------------------------------------------------
 
-unset wmakeOpt_frontend wmakeOpt_nonRecursive
+unset wmakeOpt_frontend wmakeOpt_withBearOutputDir wmakeOpt_nonRecursive
 unset wmakeOpt_debug wmakeOpt_log wmakeOpt_openmp wmakeOpt_strict wmakeOpt_queue
 unset -f usage
 
 
 #------------------------------------------------------------------------------
+
+# vim: ft=sh

Could you please test with this type of patch instead?

with-bear-output.diff

assigned to @mark

Thank you @mark. I've just tested it and it causes the following error:

$ ./Allwmake -with-bear -bear-output-dir=$HOME

Error encountered:
    unknown option: '-with-bear -bear-output-dir=/Users/ali'

See 'wmake -help' for usage
 or 'wmake -help-full' for extended usage

It considers both options as a single option. This is probably because of quoting here:

@@ -214,13 +220,13 @@ if [ -z "$wmakeOpt_nonRecursive" ]
 then
     if [ -z "$wmakeOpt_log" ]
     then
-        exec wmake $wmakeOpt_frontend -all \
+        exec wmake "$wmakeOpt_frontend" -all \
             $wmakeOpt_debug $wmakeOpt_strict $wmakeOpt_queue $wmakeOpt_openmp $*
         exit $? # Unneeded, but just in case something went wrong
     else
         echo "Logging wmake -all output to '$wmakeOpt_log'" 1>&2
         echo 1>&2
-        exec wmake $wmakeOpt_frontend -all \
+        exec wmake "$wmakeOpt_frontend" -all \
             $wmakeOpt_debug $wmakeOpt_strict $wmakeOpt_queue $wmakeOpt_openmp $* 2>&1 | \
             /usr/bin/tee $wmakeOpt_log
         # Need to cleanup after the tee

Removing them resolves the problem so long as the directory path does not contain any whitespace.

That's what I thought about after firing off the patch.

-bear-output-dir=... requires -with-bear. If -with-bear is not specified, wmake will not recognize -bear-output-dir=....

$ wmake -bear-output-dir=helloworld

Error encountered:
    unknown option: '-bear-output-dir=helloworld'

See 'wmake -help' for usage
 or 'wmake -help-full' for extended usage

Therefore, we should not parse it independently of -with-bear.

This is correct behaviour. The initial '-with-bear' trigger sends wmake to a different backend to get more options. This similar to what we do for wmake -check-dir and wmake -build-info dispatching.

Yes, there's no problem with how wmake works. However, what I said there is that your initial patch adds -with-bear even if the user hasn't passed it.

+     -bear-output-dir=*)
+        # Use 'bear' as frontend wrapper to wmake
+        wmakeOpt_frontend="-with-bear $arg"
+        continue    # Argument handled, remove it
+        ;;

Slightly more correct, I think we first need to check if the user has passed -with-bear and then take care of -bear-output-dir.

Here's a new diff.

with-bear-outupt.reverseila.diff

This one addresses an issue with checking the argument for -bear-output-dir=... in my previous diff.

-        if [ "${1#-bear-output-dir=}" != "$1" ]; then
+        if expr "$1" : '-bear-output-dir=.*' >/dev/null

I couldn't find a way to

1. not introduce a new variable (wmake_bearOutputDir).
2. prevent -bear-output-dir from being re-added to the options list within the loop, except ignoring it with continue.

Alternatively, under -bear-output-dir=*) we can check if wmakeOpt_frontend variable has already been assigned. If so, that means that -with-bear is parsed. Therefore, there's no need to check if -bear-output-dir=... is passed right after -with-bear. The user can then pass -bear-output-dir=... anywhere.

...
    -with-bear)
        # Use 'bear' as frontend wrapper to wmake
        wmakeOpt_frontend="-with-bear"
        continue    # Argument handled, remove it
        ;;

    -bear-output-dir=*)
        if [ -n "$wmakeOpt_frontend" ]
        then
            wmakeOpt_bearOutputDir="${arg#*=}"
            wmakeOpt_frontend="$wmakeOpt_frontend -bear-output-dir="
        fi
        continue    # Argument handled, remove it
        ;;
...

with-bear-outupt.reverseila.2.diff

This is similar to what I thought. with-bear-output_02.diff

But you do know that you can also simply just use wmake -with-bear -bear-output-dir=xxx -all -s -j directly? Or does that miss out something else?

This patch looks very good. It also handles the case where the user might have passed -bear-output-dir=... first.

But you do know that you can also simply just use wmake -with-bear -bear-output-dir=xxx -all -s -j directly?

I am not sure if I understood your question. I looked at the $WM_DIR/wmake, and I think the difference between AllwmakeParseArgument and wmake is that the former now takes care of misordering of the frontend arguments, whereas the latter is not forgiving about that. That means the user has to order frontend arguments properly when they decide to use wmake ... -all.

If you have anything in mind that might go wrong with this patch please let me know to test it.

I've just tested the patch by recompiling OpenFOAM with

$ wmake -with-bear -bear-output-dir=$PWD -all -s -j

(without wclean -a). It didn't emit any error.

I also tested this with a directory path that have a space in it, and it didn't handle that. I can't think of any other way to handle that except capturing the directory name in a variable, similar to the way I did in my second patch.

mentioned in commit 09e04003

I've made the change using the extra variable to capture the argument. Seems to work with spaces ok.

closed

added community label