[Maria-developers] [Merge] lp:~qu1j0t3/maria/solaris10-port into lp:maria
Toby Thain has proposed merging lp:~qu1j0t3/maria/solaris10-port into lp:maria. Requested reviews: Kristian Nielsen (knielsen) Added build scripts for 32 bit x86 architecture on Solaris. Renamed some scripts for consistency. Changed to dynamic linking of libgcc. -- https://code.launchpad.net/~qu1j0t3/maria/solaris10-port/+merge/6999 Your team Maria developers is subscribed to branch lp:maria.
Toby Thain <toby@telegraphics.com.au> writes:
Toby Thain has proposed merging lp:~qu1j0t3/maria/solaris10-port into lp:maria.
Added build scripts for 32 bit x86 architecture on Solaris. Renamed some scripts for consistency. Changed to dynamic linking of libgcc. -- https://code.launchpad.net/~qu1j0t3/maria/solaris10-port/+merge/6999 You are requested to review the proposed merge of lp:~qu1j0t3/maria/solaris10-port into lp:maria.
=== modified file 'BUILD/compile-solaris-amd64' --- BUILD/compile-solaris-amd64 2009-05-09 04:01:53 +0000 +++ BUILD/compile-solaris-amd64 2009-06-02 22:10:57 +0000 @@ -26,7 +26,7 @@ extra_flags="$amd64_cflags -D__sun -m64 -mtune=athlon64" extra_configs="$amd64_configs $max_configs --with-libevent"
-LDFLAGS="-lmtmalloc -static-libgcc" +LDFLAGS="-lmtmalloc -R/usr/sfw/lib/64" export LDFLAGS
. "$path/FINISH.sh"
I'm basically ok with these changes. However, I would like you to add some explaining comments in the commit message about why the changes are done, especially the above regarding -static-libgcc and -R/usr/sfw/lib. Why are the changes needed, and what do they do? (generally it is more important in comments to explain _why_ than to explain _what_; the code already shows what happens, but not why.) Eg. if I had to merge these changes against conflicting changes from MySQL upstream, I would have no clue about what to do to resolve the conflict. You might also want to add some comments in the new script files for the Forte C options if any of them are of special importance, it is up to you really, you are the one who knows what they mean. - Kristian. -- https://code.launchpad.net/~qu1j0t3/maria/solaris10-port/+merge/6999 Your team Maria developers is subscribed to branch lp:maria.
On 3-Jun-09, at 12:48 PM, Kristian Nielsen wrote:
Toby Thain <toby@telegraphics.com.au> writes:
Toby Thain has proposed merging lp:~qu1j0t3/maria/solaris10-port into lp:maria.
Added build scripts for 32 bit x86 architecture on Solaris. Renamed some scripts for consistency. Changed to dynamic linking of libgcc. -- https://code.launchpad.net/~qu1j0t3/maria/solaris10-port/+merge/6999 You are requested to review the proposed merge of lp:~qu1j0t3/ maria/solaris10-port into lp:maria.
=== modified file 'BUILD/compile-solaris-amd64' --- BUILD/compile-solaris-amd64 2009-05-09 04:01:53 +0000 +++ BUILD/compile-solaris-amd64 2009-06-02 22:10:57 +0000 @@ -26,7 +26,7 @@ extra_flags="$amd64_cflags -D__sun -m64 -mtune=athlon64" extra_configs="$amd64_configs $max_configs --with-libevent"
-LDFLAGS="-lmtmalloc -static-libgcc" +LDFLAGS="-lmtmalloc -R/usr/sfw/lib/64" export LDFLAGS
. "$path/FINISH.sh"
I'm basically ok with these changes.
However, I would like you to add some explaining comments in the commit message about why the changes are done, especially the above regarding -static-libgcc and -R/usr/sfw/lib. Why are the changes needed, and what do they do?
The -static-libgcc was a legacy of the original build scripts. -R (analogous to -L link time search path) is a Solaris mechanism to ensure a needed lib directory is searched at runtime. Background: Historically gcc has been available on Solaris through 3rd-party ports. In Solaris 10, gcc comes bundled, under /usr/sfw. If you use a 3rd party gcc then you cannot, of course, rely on the runtime library being available on all systems where the binaries might be running; which is perhaps why the old scripts linked libgcc in this way. But this doesn't apply if you are using the Solaris gcc, which is what I would recommend as a default procedure. (If a particular site wants to use source builds and 3rd party gcc then presumably the necessary libraries have been made available.) Personally I prefer to see this system library dynamically linked. One reason is that the application benefits from ordinary system patch maintenance.
(generally it is more important in comments to explain _why_ than to explain _what_; the code already shows what happens, but not why.)
Eg. if I had to merge these changes against conflicting changes from MySQL upstream, I would have no clue about what to do to resolve the conflict.
You might also want to add some comments in the new script files for the Forte C options if any of them are of special importance, it is up to you really, you are the one who knows what they mean.
These are cribbed from other Solaris builders who have benchmarked some benefit. I don't have my own benchmarks to justify them, but this should be done at some point (any recommended procedures? mysqlslap?) I guess we have 3 "less arbitrary" choices: 1. follow what Sun/MySQL use as best practice 2. build generically - without any platform performance options, until: 3. recommended options can be developed for Maria based on benchmarking --Toby
- Kristian. -- https://code.launchpad.net/~qu1j0t3/maria/solaris10-port/+merge/6999 Your team Maria developers is subscribed to branch lp:maria.
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Hi Toby, Toby Thain <toby@telegraphics.com.au> writes:
On 3-Jun-09, at 12:48 PM, Kristian Nielsen wrote:
Toby Thain <toby@telegraphics.com.au> writes:
Toby Thain has proposed merging lp:~qu1j0t3/maria/solaris10-port into lp:maria.
Added build scripts for 32 bit x86 architecture on Solaris. Renamed some scripts for consistency. Changed to dynamic linking of libgcc. I'm basically ok with these changes.
However, I would like you to add some explaining comments in the commit message about why the changes are done, especially the above regarding
The -static-libgcc was a legacy of the original build scripts. -R (analogous to -L link time search path) is a Solaris mechanism to ensure a needed lib directory is searched at runtime.
<cut> I'm sorry that this has been stalling. I have now re-committed your changes with a more verbose commit message that includes some of your explanations in the email. I have pushed the result to lp:maria (no code changes compared to your branch). Thanks for your efforts with this, and sorry for the delay. - Kristian.
On 7-Jul-09, at 7:23 AM, Kristian Nielsen wrote:
Hi Toby,
Toby Thain <toby@telegraphics.com.au> writes:
On 3-Jun-09, at 12:48 PM, Kristian Nielsen wrote:
Toby Thain <toby@telegraphics.com.au> writes:
Toby Thain has proposed merging lp:~qu1j0t3/maria/solaris10-port into lp:maria.
Added build scripts for 32 bit x86 architecture on Solaris. Renamed some scripts for consistency. Changed to dynamic linking of libgcc. I'm basically ok with these changes.
However, I would like you to add some explaining comments in the commit message about why the changes are done, especially the above regarding
The -static-libgcc was a legacy of the original build scripts. -R (analogous to -L link time search path) is a Solaris mechanism to ensure a needed lib directory is searched at runtime.
<cut>
I'm sorry that this has been stalling.
I have now re-committed your changes with a more verbose commit message that includes some of your explanations in the email. I have pushed the result to lp:maria (no code changes compared to your branch).
Thanks for your efforts with this, and sorry for the delay.
Hi Kristian, Not at all, I believe I was the hold-up, as you asked me to re-commit with the explanations in comments. Thanks for taking care of it! --Toby
- Kristian.
participants (2)
-
Kristian Nielsen
-
Toby Thain