List:Commits« Previous MessageNext Message »
From:Sergei Golubchik Date:August 20 2008 11:56am
Subject:Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380
View as plain text  
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 ?

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

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

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.

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

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

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

Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Engineer/Server Architect
/_/  /_/\_, /___/\___\_\___/  Sun Microsystems GmbH, HRB München 161028
       <___/                  Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Häring
Thread
bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380Narayanan V13 Aug
  • Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380Joerg Bruehe13 Aug
    • Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380Narayanan14 Aug
      • Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380Joerg Bruehe15 Aug
        • Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380Narayanan18 Aug
          • Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380Sergei Golubchik18 Aug
            • Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380Joerg Bruehe20 Aug
              • Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380Sergei Golubchik20 Aug
                • Re: WL#4380 ABI check: Where and when to run it ?Joerg Bruehe26 Aug
                  • Re: WL#4380 ABI check: Where and when to run it ?Sergei Golubchik26 Aug
                  • Re: WL#4380 ABI check: Where and when to run it ?Narayanan27 Aug
                    • Re: WL#4380 ABI check: Where and when to run it ?Joerg Bruehe27 Aug
          • Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380Joerg Bruehe20 Aug
            • Re: bzr commit into mysql-5.1 branch (v.narayanan:2678) WL#4380V Narayanan21 Aug
              • Re: WL#4380 ABI check: Implementation detailsJoerg Bruehe26 Aug
                • Re: WL#4380 ABI check: Implementation detailsSergei Golubchik26 Aug
                  • Re: WL#4380 ABI check: Implementation detailsJoerg Bruehe26 Aug
                    • Re: WL#4380 ABI check: Implementation detailsSergei Golubchik26 Aug
                      • Re: WL#4380 ABI check: Implementation detailsJoerg Bruehe26 Aug
                        • Re: WL#4380 ABI check: Implementation detailsNarayanan27 Aug
                          • Re: WL#4380 ABI check: Implementation detailsJoerg Bruehe27 Aug
                            • Re: WL#4380 ABI check: Implementation detailsNarayanan27 Aug
                • Re: WL#4380 ABI check: Implementation detailsNarayanan27 Aug