Hi,
I have changed the subject to make it more obvious which aspects we are
discussing in this subthread:
Where (which platforms) and when (which build runs) to verify the ABI.
Sergei Golubchik wrote:
> Hi!
>
> On Aug 20, Joerg Bruehe wrote:
>> Hi Sergei, all !
>>
>>
>> Sergei Golubchik wrote:
>>> Hi!
>>> On Aug 18, Narayanan wrote:
>>>> I will change the rule to not run as part of the main build, basically
>>>> remove it from all-am. As long as it detects the changes
>>>> and generates the requisite noise, I am as happy as can be.
>>> ...
>>>> As long as it runs as part of pushbuild and someone helps me with doing
>>>> this, I am OK.
>>> But I'm not. I believe it must be run as a part of *every* build (if the
>>> prerequisites are met, of course).
>> Assuming "every build" is meant to include the release builds, I *very*
>> strongly disagree:
>> 1) This check is done on the source code, and its result should be
>> independent of the platform. Why do it more often than needed ?
>
> Taking into account the amount of #ifdef's in our code, I cannot believe
> that you can seriously insist on running the check on one platform only
> :)
>
> You know better than anybody else that running a build or a test suite
> on one platform is not enough. Why would that be enough for abi_check ?
I had assumed at least the modules that define the ABI are free of "#if"
and related constructs.
My approach was that the check should be run during development
(possibly on the developers' machines) and during pushbuild,
I had assumed that is sufficient coverage.
So you think we should try to run it on every platform, provided we can
make the compiler generate the check file ?
That is a possible approach, but it implies we should make the commands
(compiler options) configurable as much as possible (see below).
>
>> 2) If this check is done in release builds, it is simply too late.
>> Any changes in the API/ABI need to be caught in pushbuild, and then
>> either be rejected and fixed, or approved explicitly.
>> How should the build team react if the check fails ?
>
> How should the build team react if there's a compilation failure ?
> It's the same thing. And neither should happen in release build.
You have a point.
I had simply hoped our header files (for the API/ABI) are not so full of
confitional compiling, so fewer platforms would suffice.
>
>> 3) If this check is done in release builds, in all configurations, it is
>> just a waste of time and disk space.
>
> Waste of how much time and how much space ? It's far less than 1 per
> cent of either.
I have not taken any time measurements. VN, do you have any ?
>
> Anyway, the point is not to make release builds more expensive. The
> point is that it should be run on developer's boxes. For every WL, every
> bugfix, every source code change. If pushbuild detects it - it's too
> late, a bad changeset is already pushed. It's the same thing as
> requiring a developer to build the tree and run the test suite before
> pushing, although, in principle, pushbuild can do that. And doing it on
> one platform only should be enough, right ? :)
>
> As for release builds, the added cost is so minor that there's nothing
> to argue about, really.
In a release build, we do it six times (for the different configurations).
I would like to know the build times (compile + link, no tests) both
with and without the ABI check.
The answer should tell how important it is to avoid repeating the check
when building different configurations of the same version on the same
platform.
*If* the time needed is considerable, it might be good to have a way of
disabling the check (by not making it ".PHONY", so that a dummy output
file would let "make" not run it).
If it isn't, I will wthdraw my reservations.
>
>>>>> They have something called "diff", but I would not rely on it
>>>>> supporting "-w" (above):
>>>>> 1) Copy the (g)tar rule, so that we can provide a "gdiff" if needed.
>>>>> 2) Put it into the section where you checked for "gcc", there is no
> need
>>>>> to look for "diff" if we don't run the abi_check on that
> platform.
>>>> Will do!
>>> Is there a practical need to do that ? Any single host where we have gcc
>>> and diff -w doesn't work ?
>> We use gcc on OS X and (sometimes) on Solaris, and should not block us
>> from using it on other platforms as well.
>
> Right, but VN has this WL pushed in the team tree, and it's tested on
> all pushbuild platforms already. As far as I remember this includes
> Solaris and OS X with gcc.
It is in the team tree before having two reviews ? No comment.
The stronger you argue to "run it on as many platforms as possible",
the more important it gets to make the "diff" binary configurable.
Currently, "diff" output for the same file difference in the test suite
differs by platform. (A weird sentence, but you get the meaning.)
>
>>>>> IMO, there are two choices:
>>>>> a) Have "@CC@ -E -nostdinc -dI" hard-coded, like it is now (which
> means
>>>>> we can only do it on gcc platforms), or
>>>>> b) if it is "gcc", set some variable ("@ABI_CHECK@" ?) to the gcc
> line,
>>>>> otherwise set it to "no" or some other string.
>>>>> With approach a), it is the user's responsibility not to call "make
>>>>> abi_check" with a compiler that doesn't accept these options,
>>>>> with b), we (or some other user) could expand "configure.in" by
> adding
>>>>> settings for other compilers.
>>>>>
>>>>> As you can tell, I would prefer b), but I won't reject a).
>>>> Will work on b)
>>> Again, any practical need to do that ? Any compiler besides gcc that
>>> supports a functionality identical to "-E -nostdinc -dI" ? Or, at least,
>>> to "-E -nostdinc" ?
>> I didn't check. My proposal is not based on a currently perceived
>> possibility, rather on it being more flexible.
>
> A perceived possibility of a hypothetical compiler that supports
> -E -dI -nostdinc ? I'm afraid, it's over-engineering, adding the
> complexity without necessity.
Same argument as above: If you would like to cover all platforms, then
include the mechanism now, so that we can add other compilers if we
detect the need and the possibility (by adding a branch then, not
changing the general approach).
>
>> Serg,
>> to me your remarks appear to be somewhat contradictory:
>>
>> - On one side, you "believe it must be run as a part of *every* build
>> (if the prerequisites are met, of course)",
>> - while you also propose to check the "practical need" before doing more
>> general mechanisms.
>>
>> If the easy implementation ("gcc" only, "diff -w" assumed) is good enough,
>> why then in every build (including release builds) ?
>
> To have it run on developer's boxes, see above.
Seems I didn't put the question sufficiently clear, but it should be
covered by the above remarks.
Regards,
Jörg
--
Joerg Bruehe, MySQL Build Team, joerg@stripped (+49 30) 417 01 487
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschaeftsfuehrer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Haering Muenchen: HRB161028
