1 |
Hi! |
2 |
|
3 |
On Tue, 30 Oct 2018 08:18:58 +0100 Michał Górny wrote: |
4 |
> On Mon, 2018-10-29 at 03:57 +0300, Andrew Savchenko wrote: |
5 |
> > On Sun, 28 Oct 2018 19:29:28 +0100 Michał Górny wrote: |
6 |
> > > On Sun, 2018-10-28 at 01:38 +0300, Andrew Savchenko wrote: |
7 |
> > > > Hi all! |
8 |
> > > > |
9 |
> > > > The only blocker for EAPI 7 update is eutils inheritance, but it |
10 |
> > > > seems to be not used within the current eclass code, probably a |
11 |
> > > > remnant from older days. So it is removed. |
12 |
> > > > |
13 |
> > > > Looks like no other EAPI 7 specific changes needed. |
14 |
> > > > |
15 |
> > > |
16 |
> > > Please use -U99999 to include more context to the patches. |
17 |
> |
18 |
> I'm going to include a few 'easy cleanup' comments since EAPI 7 |
19 |
> is a good opportunity to improve the eclass. I'm going to skip horribly |
20 |
> bad design decisions since I suppose nobody cares. |
21 |
|
22 |
Should we really mix EAPI bump with full code review? |
23 |
|
24 |
This eclass is small, so no harm here. But for larger eclasses |
25 |
(hello java-*.eclass) this will hinder updates considerably. I |
26 |
prefer to fix something rather than to fix nothing while |
27 |
frustrating in attempt to fix everything at once. |
28 |
|
29 |
Also this make git history review harder as fixes for independent |
30 |
issues will be mixed together. |
31 |
|
32 |
So I kindly ask you for future updates (from everyone, not just |
33 |
me) focus on review of the proposed changes instead of reviewing |
34 |
full code. Thank you for understanding. |
35 |
|
36 |
> > # @FUNCTION: fortran_int64_abi_fflags |
37 |
> > # @DESCRIPTION: |
38 |
> > # Return the Fortran compiler flag to enable 64 bit integers for |
39 |
> > # array indices |
40 |
> > # @CODE |
41 |
> > fortran_int64_abi_fflags() { |
42 |
> > debug-print-function ${FUNCNAME} "${@}" |
43 |
> > |
44 |
> > _FC=$(tc-getFC) |
45 |
> |
46 |
> Any reason not to make it local? |
47 |
|
48 |
Fixed. |
49 |
|
50 |
> > # @FUNCTION: _fortran_write_testsuite |
51 |
> > # @INTERNAL |
52 |
> > # @DESCRIPTION: |
53 |
> > # writes fortran test code |
54 |
> > _fortran_write_testsuite() { |
55 |
> > debug-print-function ${FUNCNAME} "${@}" |
56 |
> > |
57 |
> > local filebase=${T}/test-fortran |
58 |
> > |
59 |
> > # f77 code |
60 |
> > cat <<- EOF > "${filebase}.f" |
61 |
> |
62 |
> || die |
63 |
|
64 |
Done. |
65 |
|
66 |
> > end |
67 |
> > EOF |
68 |
> > |
69 |
> > # f90/95 code |
70 |
> > cat <<- EOF > "${filebase}.f90" |
71 |
> |
72 |
> || die |
73 |
|
74 |
Done. |
75 |
|
76 |
> > end |
77 |
> |
78 |
> Also, why different indentation? |
79 |
|
80 |
I prefer not to touch it. Fortran compilers are quite picky with |
81 |
leading spaces or tabs. |
82 |
|
83 |
> > EOF |
84 |
> > |
85 |
> > # f2003 code |
86 |
> > cat <<- EOF > "${filebase}.f03" |
87 |
> |
88 |
> || die |
89 |
|
90 |
Done. |
91 |
|
92 |
> > # @FUNCTION: _fortran-has-openmp |
93 |
> > # @RETURN: return code of the compiler |
94 |
> > # @INTERNAL |
95 |
> > # @DESCRIPTION: |
96 |
> > # See if the fortran supports OpenMP. |
97 |
> > _fortran-has-openmp() { |
98 |
> > debug-print-function ${FUNCNAME} "${@}" |
99 |
> > |
100 |
> > local flag |
101 |
> > local filebase=${T}/test-fc-openmp |
102 |
> > local fcode=${filebase}.f |
103 |
> > local ret |
104 |
> > local _fc=$(tc-getFC) |
105 |
> > |
106 |
> > cat <<- EOF > "${fcode}" |
107 |
> |
108 |
> || die |
109 |
|
110 |
Done. |
111 |
|
112 |
> > for flag in -fopenmp -xopenmp -openmp -mp -omp -qsmp=omp; do |
113 |
> > ${_fc} ${flag} "${fcode}" -o "${fcode}.x" \ |
114 |
> > &>> "${T}"/_fortran_compile_test.log |
115 |
> > ret=$? |
116 |
> > (( ${ret} )) || break |
117 |
> |
118 |
> This (( ... )) is unreadable at best; please replace it with clear |
119 |
> condition. |
120 |
|
121 |
Fixed. ret variable is not needed at all. |
122 |
|
123 |
> > # @FUNCTION: _fortran_die_msg |
124 |
> > # @INTERNAL |
125 |
> > # @DESCRIPTION: |
126 |
> > # Detailed description how to handle fortran support |
127 |
> > _fortran_die_msg() { |
128 |
> > debug-print-function ${FUNCNAME} "${@}" |
129 |
> > |
130 |
> > echo |
131 |
> |
132 |
> Don't mix echo with eerror. |
133 |
|
134 |
Done. |
135 |
|
136 |
> > # @FUNCTION: _fortran-2_pkg_setup |
137 |
> > # @INTERNAL |
138 |
> > # @DESCRIPTION: |
139 |
> > # _The_ fortran-2_pkg_setup() code |
140 |
> > _fortran-2_pkg_setup() { |
141 |
> > for _f_use in ${FORTRAN_NEEDED}; do |
142 |
> > case ${_f_use} in |
143 |
> > always) |
144 |
> > _fortran_test_function && break |
145 |
> > ;; |
146 |
> > no) |
147 |
> > einfo "Forcing fortran support off" |
148 |
> > break |
149 |
> > ;; |
150 |
> > *) |
151 |
> > if use ${_f_use}; then |
152 |
> > _fortran_test_function && break |
153 |
> > else |
154 |
> > unset FC |
155 |
> > unset F77 |
156 |
> > fi |
157 |
> |
158 |
> This contradicts the dependency atoms. |
159 |
> |
160 |
> If FORTRAN_NEEDED="foo bar", you'll get: |
161 |
> |
162 |
> DEP="foo? ( virtual/fortran ) bar? ( virtual/fortran )" |
163 |
> |
164 |
> However, with USE="foo -bar" this will first set the compiler |
165 |
> for USE=foo, then reset it for USE=bar. |
166 |
|
167 |
Ok, now both case and for will break immediately if fortran |
168 |
compiler is found and passed tests. |
169 |
|
170 |
The updated full v2 patch will be sent as a separate e-mail. |
171 |
|
172 |
Best regards, |
173 |
Andrew Savchenko |