Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380

[Joerg Bruehe]

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 ?
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 ?
3) If this check is done in release builds, in all configurations, it is
    just a waste of time and disk space.

> 
>>> As regards .PHONY:
>>> A Makefile target should be declared .PHONY if it doesn't correspond to a 
>>> file, that's what I took from "info make" (node "Phony targets").
>> If it has to be run as a separate target, it does not need to be included 
>> as part of .PHONY also.
> 
> It still does. Otherwise a file 'abi_check' will prevent this rule from
> running.

ACK.  OTOH, this could be the way to prevent the check from running in 
release builds: "touch abi_check" in our tools.

> 
>>> 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.
I don't see us losing anything by checking for "gdiff", and the rule is 
simple.

> 
>>> 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.


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) ?


Jörg

-- 
Joerg Bruehe,  MySQL Build Team,  joerg@stripped   ([phone]
Sun Microsystems GmbH,   Sonnenallee 1,   D-85551 Kirchheim-Heimstetten
Geschaeftsfuehrer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Haering     Muenchen: HRB161028