Code review comment for lp:~michihenning/unity-scopes-api/abigail

> Okay, so I guess there are two options here:
>
> 1. If you want to generate a new dump as part of the vbuild process, why not
> just have a custom_command that depends on the libunity-scopes target and
> outputs the ABI dump, rather than dealing with the "run_always" bits?

Now I'm confused. I already have this command:

add_custom_command(
    OUTPUT ${abi_dump_file}
    COMMAND ${abi_dump_cmd}
    DEPENDS ${UNITY_SCOPES_LIB})

add_custom_target(gen-abi-dump DEPENDS ${abi_dump_file})

This generates the dump if the dump doesn't exist yet or is out of date with respect to the library.

The run always hack is necessary because, without it, a "make test" will correctly complain the first time but, if I have a failure the first time and then do "make test" again, I end up with nothing being done at all, because nothing is out of date, and the second run then incorrectly reports success when, in fact, the failure is still there.

> 2. Do you even need to dump the current ABI to an XML file as part of the
> test? Might it be quicker to just have abidiff compare the reference XML dump
> against the just-built .so file? That would reduce the number of moving parts
> in the test, and would likely be even faster.

I considered doing this, but decided not to. Any speed difference is completely unnoticeable because analysing the dump takes many times longer than uncompressing it. That's for a large dump. For a small dump, the difference is also unnoticeable because a small dump will uncompress very quickly.

And I figured it would be useful to leave the dump behind. When something goes wrong, and people want to dig into it, they don't have to copy and uncompress it manually, making sure that they use the correct arch subdirectory.

The dump file is also useful for reporting issues with abidiff, because I have everything I need (both dumps and the suppressions file) right there.

I've updated the clean target though to get rid of the base dump, so a "make clean" actually reclaims the disk space.

> I can understand having a helper script to generate a dump based on the just-
> built library, but it doesn't seem like a prerequisite for running the tests.

The helper script to create the dump is useful for generating the dump by hand (say, for debugging). It also avoids a needless dump on Vivid. Doing that sort of thing directly in cmake rapidly gets cumbersome. Using the helper script is cleaner, I think?