Merge lp:~qu1j0t3/maria/solaris10-port into lp:~maria-captains/maria/5.1-converting

Proposed by Toby Thain
Status: Rejected
Rejected by: Sergei Golubchik
Proposed branch: lp:~qu1j0t3/maria/solaris10-port
Merge into: lp:~maria-captains/maria/5.1-converting
Diff against target: None lines
To merge this branch: bzr merge lp:~qu1j0t3/maria/solaris10-port
Reviewer Review Type Date Requested Status
Kristian Nielsen Pending
Review via email: mp+6999@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Toby Thain (qu1j0t3) wrote :

Added build scripts for 32 bit x86 architecture on Solaris. Renamed some scripts for consistency. Changed to dynamic linking of libgcc.

Revision history for this message
Kristian Nielsen (knielsen) wrote :

Toby Thain <email address hidden> 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.

Revision history for this message
Toby Thain (qu1j0t3) wrote :
Download full text (3.6 KiB)

On 3-Jun-09, at 12:48 PM, Kristian Nielsen wrote:

> Toby Thain <email address hidden> 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...

Read more...

Revision history for this message
Sergei Golubchik (sergii) wrote :

proposal for the old lp:~maria-captains/maria/5.1-converting tree.
it the proposal is still relevant, please resubmit for the current tree

Unmerged revisions

2708. By Toby Thain

Dynamically link to Solaris' libgcc. Add scripts for 32-bit builds.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'BUILD/compile-solaris-amd64'
2--- BUILD/compile-solaris-amd64 2009-05-09 04:01:53 +0000
3+++ BUILD/compile-solaris-amd64 2009-06-02 22:10:57 +0000
4@@ -26,7 +26,7 @@
5 extra_flags="$amd64_cflags -D__sun -m64 -mtune=athlon64"
6 extra_configs="$amd64_configs $max_configs --with-libevent"
7
8-LDFLAGS="-lmtmalloc -static-libgcc"
9+LDFLAGS="-lmtmalloc -R/usr/sfw/lib/64"
10 export LDFLAGS
11
12 . "$path/FINISH.sh"
13
14=== modified file 'BUILD/compile-solaris-amd64-debug'
15--- BUILD/compile-solaris-amd64-debug 2009-05-09 04:01:53 +0000
16+++ BUILD/compile-solaris-amd64-debug 2009-06-02 22:10:57 +0000
17@@ -5,7 +5,7 @@
18 extra_flags="$amd64_cflags -D__sun -m64 -mtune=athlon64 $debug_cflags"
19 extra_configs="$amd64_configs $debug_configs $max_configs --with-libevent"
20
21-LDFLAGS="-lmtmalloc -static-libgcc"
22+LDFLAGS="-lmtmalloc -R/usr/sfw/lib/64"
23 export LDFLAGS
24
25 . "$path/FINISH.sh"
26
27=== added file 'BUILD/compile-solaris-amd64-debug-forte'
28--- BUILD/compile-solaris-amd64-debug-forte 1970-01-01 00:00:00 +0000
29+++ BUILD/compile-solaris-amd64-debug-forte 2009-06-02 22:10:57 +0000
30@@ -0,0 +1,27 @@
31+#!/bin/sh
32+
33+path=`dirname $0`
34+. "$path/SETUP.sh"
35+
36+# Take only #define options - the others are gcc specific.
37+# (real fix is for SETUP.sh not to put gcc specific options in $debug_cflags)
38+DEFS=""
39+for F in $debug_cflags ; do
40+ expr "$F" : "^-D" && DEFS="$DEFS $F"
41+done
42+debug_cflags="-O0 -g $DEFS"
43+
44+extra_flags="-m64 -mt -D_FORTEC_ -xlibmopt -fns=no $debug_cflags"
45+extra_configs="$max_configs --with-libevent $debug_configs"
46+
47+warnings=""
48+c_warnings=""
49+cxx_warnings=""
50+base_cxxflags="-noex"
51+
52+CC=cc
53+CFLAGS="-xstrconst"
54+CXX=CC
55+LDFLAGS="-lmtmalloc"
56+
57+. "$path/FINISH.sh"
58
59=== removed file 'BUILD/compile-solaris-amd64-forte-debug'
60--- BUILD/compile-solaris-amd64-forte-debug 2009-05-09 04:01:53 +0000
61+++ BUILD/compile-solaris-amd64-forte-debug 1970-01-01 00:00:00 +0000
62@@ -1,27 +0,0 @@
63-#!/bin/sh
64-
65-path=`dirname $0`
66-. "$path/SETUP.sh"
67-
68-# Take only #define options - the others are gcc specific.
69-# (real fix is for SETUP.sh not to put gcc specific options in $debug_cflags)
70-DEFS=""
71-for F in $debug_cflags ; do
72- expr "$F" : "^-D" && DEFS="$DEFS $F"
73-done
74-debug_cflags="-O0 -g $DEFS"
75-
76-extra_flags="-m64 -mt -D_FORTEC_ -xlibmopt -fns=no $debug_cflags"
77-extra_configs="$max_configs --with-libevent $debug_configs"
78-
79-warnings=""
80-c_warnings=""
81-cxx_warnings=""
82-base_cxxflags="-noex"
83-
84-CC=cc
85-CFLAGS="-xstrconst"
86-CXX=CC
87-LDFLAGS="-lmtmalloc"
88-
89-. "$path/FINISH.sh"
90
91=== added file 'BUILD/compile-solaris-x86-32'
92--- BUILD/compile-solaris-x86-32 1970-01-01 00:00:00 +0000
93+++ BUILD/compile-solaris-x86-32 2009-06-02 22:10:57 +0000
94@@ -0,0 +1,11 @@
95+#!/bin/sh
96+
97+path=`dirname $0`
98+. "$path/SETUP.sh"
99+extra_flags="-D__sun -m32"
100+extra_configs="$max_configs --with-libevent"
101+
102+LDFLAGS="-lmtmalloc -R/usr/sfw/lib"
103+export LDFLAGS
104+
105+. "$path/FINISH.sh"
106
107=== added file 'BUILD/compile-solaris-x86-32-debug'
108--- BUILD/compile-solaris-x86-32-debug 1970-01-01 00:00:00 +0000
109+++ BUILD/compile-solaris-x86-32-debug 2009-06-02 22:10:57 +0000
110@@ -0,0 +1,11 @@
111+#!/bin/sh
112+
113+path=`dirname $0`
114+. "$path/SETUP.sh"
115+extra_flags="-D__sun -m32 $debug_cflags"
116+extra_configs="$max_configs --with-libevent $debug_configs"
117+
118+LDFLAGS="-lmtmalloc -R/usr/sfw/lib"
119+export LDFLAGS
120+
121+. "$path/FINISH.sh"
122
123=== added file 'BUILD/compile-solaris-x86-32-debug-forte'
124--- BUILD/compile-solaris-x86-32-debug-forte 1970-01-01 00:00:00 +0000
125+++ BUILD/compile-solaris-x86-32-debug-forte 2009-06-02 22:10:57 +0000
126@@ -0,0 +1,27 @@
127+#!/bin/sh
128+
129+path=`dirname $0`
130+. "$path/SETUP.sh"
131+
132+# Take only #define options - the others are gcc specific.
133+# (real fix is for SETUP.sh not to put gcc specific options in $debug_cflags)
134+DEFS=""
135+for F in $debug_cflags ; do
136+ expr "$F" : "^-D" && DEFS="$DEFS $F"
137+done
138+debug_cflags="-O0 -g $DEFS"
139+
140+extra_flags="-m32 -mt -D_FORTEC_ -xbuiltin=%all -xlibmil -xlibmopt -fns=no -xprefetch=auto -xprefetch_level=3 $debug_cflags"
141+extra_configs="$max_configs --with-libevent $debug_configs"
142+
143+warnings=""
144+c_warnings=""
145+cxx_warnings=""
146+base_cxxflags="-noex"
147+
148+CC=cc
149+CFLAGS="-xstrconst"
150+CXX=CC
151+LDFLAGS="-lmtmalloc"
152+
153+. "$path/FINISH.sh"
154
155=== added file 'BUILD/compile-solaris-x86-forte-32'
156--- BUILD/compile-solaris-x86-forte-32 1970-01-01 00:00:00 +0000
157+++ BUILD/compile-solaris-x86-forte-32 2009-06-02 22:10:57 +0000
158@@ -0,0 +1,19 @@
159+#!/bin/sh
160+
161+path=`dirname $0`
162+. "$path/SETUP.sh"
163+
164+extra_flags="-m32 -mt -D_FORTEC_ -xbuiltin=%all -xlibmil -xlibmopt -fns=no -xprefetch=auto -xprefetch_level=3"
165+extra_configs="$max_configs --with-libevent"
166+
167+warnings=""
168+c_warnings=""
169+cxx_warnings=""
170+base_cxxflags="-noex"
171+
172+CC=cc
173+CFLAGS="-xstrconst"
174+CXX=CC
175+LDFLAGS="-lmtmalloc"
176+
177+. "$path/FINISH.sh"

Subscribers

People subscribed via source and target branches