[cmake-developers] [PATCH] Discussion needed: FindBISON: refine VERBOSE feature of BISON_TARGET

[Eon Jeong]

A year passed so long to reply for a personal reason.

Adding to this list because I think it's better to review past
discussion on this issue.

So let's clearify:

1. Current [VERBOSE <file>] defines `--verbose`, so bison generates
foo.output from foo.y, (generating also foo.tab.c and foo.tab.h) then
cmake copies foo.output to <file>. It's intermediate-file-and-copy as
said. It's also problematic because using [VERBOSE <file>] forces the
report file to be copied to some other name.

2. Since 2007, bison has `--report-file <file>` option that specifies
the report file name instead of foo.output. so [REPORT_FILE <file>] is
suggested. No intermediate file is made; so using this breaks
behavioral backward compatibility. On the other hand, some build
systems are still using bison of version before 2007.

3. There are some candidates replacing current [VERBOSE <file>]:

3.1. No change for our FindBISON module. `--report-file <file>` option
can be used via COMPILE_FLAGS. Hint this in documentation.

3.2. [VERBOSE [<file>]]. This does not break backward compatibility,
and makes someone can choose the report file is copied or not.

3.4. Just [VERBOSE] and [REPORT_FILE <file>]. Independently, [VERBOSE]
inserts `--verbose`, [REPORT_FILE <file>] inserts `--report_file
<file>`. No more intermediate-file-and-copy, so breaking
compatibility, but the clearest solution.

3.5. [VERBOSE [<file>]] and [REPORT_FILE <file>]. In this case, <file>
argument of [VERBOSE <file>] can be either ignored or used (copying
the report file.) This does not break compatibility at all, but it's
dirty when [REPORT_FILE <file>] is cared for lower versions of bison
using `cmake -E copy` because [VERBOSE <file>] does the exact thing.

And many other solutions possible.

4. Issues are:

- Behavioral & syntactic compatibility of [VERBOSE <file>] between
before and after the patch.

- If we remove <file> from [VERBOSE <file>] so it becomes [VERBOSE],
syntactic compatibility breaks.

- If we add [REPORT_FILE <file>], it does not work on systems using
bison before 2007. We may care it with custom command using `move`, or
not.

These issues are about how CMake cares about the modules'
compatibility and clearness, so I need some direction.

I will make appropriate patch for the result.

Thanks,

Eon

2015-06-25 0:29 GMT+09:00 Brad King <brad.king at kitware.com>:
> On 06/24/2015 11:13 AM, 정언 wrote:
>> The copying command should be run right after each execution of bison.
>
> Yes.
>
>> However, add_custom_command never guarantees a running order of
>> internal commands. The documentation says, the commands can run
>> parallel so never ever put commands with implicit dependencies.
>
> I'm not sure what text from the documentation you're referencing,
> but I think that is warning about listing one output in multiple
> independent targets, e.g.
>
>  add_custom_command(OUTPUT out.txt ...)
>  add_custom_target(tgt1 DEPENDS out.txt)
>  add_custom_target(tgt2 DEPENDS out.txt)
>
> That is not the case here.
>
>> Again, we have two custom commands as a result of passing `VERBOSE` to
>> bison_target(). One runs bison, and the other runs cp.
>
> We only need one add_custom_command call.  It can list both outputs
> and have two COMMAND arguments.  Something like (untested):
>
>       if(...)
>         set(_maybe_copy_verbose_output ${BISON_TARGET_ARG_VERBOSE})
>         set(_maybe_copy_verbose_command
>           COMMAND ${CMAKE_COMMAND} -E copy ${BISON_TARGET_output_path}/${BISON_TARGET_output_name}.output ${BISON_TARGET_ARG_VERBOSE}
>           )
>       endif()
>
>       add_custom_command(OUTPUT ${BISON_TARGET_outputs} ${_maybe_copy_verbose_output}
>         ${BISON_TARGET_extraoutputs}
>         COMMAND ${BISON_EXECUTABLE} ${BISON_TARGET_cmdopt} -o ${BisonOutput} ${BisonInput}
>         ${_maybe_copy_verbose_command}
>         DEPENDS ${BisonInput}
>         COMMENT "[BISON][${Name}] Building parser with bison ${BISON_VERSION}"
>         WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
>
> When both COMMAND options are present they will run sequentially.
>
>> If VERBOSE behaved right, REPORT_FILE would not have any benefits over it.
>
> Wouldn't it be able to skip the intermediate file and copy step?
>
> -Brad
>



-- 
Eon Jeong <administrator at korea.ac.kr>
Seoul, Rep. of Korea
Undergrad @ Dept.of Computer Science and Communication Engineering, Korea Univ.