atlas: incorrect compiler checking code

[ryandesign (Ryan Carsten Schmidt)]

The atlas port includes this code in the global part of the portfile:

# Chose the right flags for GCC and Clang compilers

if {${build_arch} == "i386" || ${build_arch} == "x86_64"} {

    # General flags
    # GCC 
    set gcc_flags   "-fomit-frame-pointer -mfpmath=sse -O3\
                     -fno-schedule-insns2 -fPIC"

    # Clang 
    if {${use_clang} == "32" || ${use_clang} == "XCode" } {

        pre-fetch {        
            ui_warn "Compiling Atlas with this version of clang is\
                    likely *NOT* to work. Please use clang-3.3 or\
                    higher."
        }
        set clang_flags "-O3 -fomit-frame-pointer -fPIC"
    } else {

        # Clang 3.3 – Use loop and straight vectorizer
        set clang_flags "-O3 -fomit-frame-pointer -fPIC\
                         -fvectorize -fslp-vectorize"

    }

There are two problems with this.

First, if compiling atlas with clang 3.2 will not work, then the +mpclang32 variant should be removed and macports-clang-3.2 should be added to compiler.blacklist (along with any other compilers with which atlas will not work). If compiling atlas with some versions of Xcode clang will not work, then the specific versions of Xcode clang with which it will not work should be blacklisted using the compiler_blacklist_versions portgroup as mentioned in #38768.

Second, the more important issue is that the message will never actually be printed, because MacPorts does not execute code in the order in which you think it does. First, MacPorts executes all code in the global part of the portfile (i.e. code that's not within a phase or a variant). Then afterward, it executes the code in any selected variants. And within variants is where you're defining the use_clang variable. So at the point where the above code runs, use_clang is always empty (except when no compiler variant was selected and have_avx is yes, in which case use_clang is set to 33), so the message never prints. This also means the clang_flags aren't set the way you want them to be, for some clang versions.

The same goes for the gcc_version variable. It's initialized to 0 in the global part of the portfile, then changed in the gcc4x variants, but it's checked in the global part of the portfile, before it's been set, so it will always be 0 when the following code runs (except when no compiler variant was selected and have_avx is no, in which case gcc_version is set to 47):

# Threading
# If we run on a mono-processor machine (PPC), then disable threading

if {![catch {sysctl hw.logicalcpu} result] && $result == 1} {

    configure.args-append    -t 0
    set no_threads  1
} else {

    set no_threads  0

    # Threading relies on OpenMP if gcc compilers are selected
    if {${gcc_version} != 0} {

        configure.args-append   -Si omp 1 
    }
}

This means -Si omp 1 won't be added to the configure.args in all of the situations where you want it to be. (Specifically, it won't be, if the user selects a gcc4x variant.)

---

One solution could be to set the variables in the global part of the portfile instead of within a variant. For example, instead of doing this:

variant gcc48               conflicts   gcc46 gcc47 \
                                        clang mpclang32 mpclang33 \
                            description {build using macports-gcc-4.8} {

    depends_build-append    port:gcc48
    configure.compiler      macports-gcc-4.8

    set gcc_version         48
    set use_clang           ""
}

Do this:

variant gcc48               conflicts   gcc46 gcc47 \
                                        clang mpclang32 mpclang33 \
                            description {build using macports-gcc-4.8} {

    depends_build-append    port:gcc48
    configure.compiler      macports-gcc-4.8
}
if {[variant_isset gcc48]} {
    set gcc_version         48
    set use_clang           ""
}

You'll need to increase the port's revision when you make these changes so that things get rebuilt in the way you want them to be built.

[cooljeanius (Eric Gallager)]

Vince should make the atlas port openmaintainer; the tickets against it seem to be piling up lately...

[cooljeanius (Eric Gallager)]

Cc Me!

[ryandesign (Ryan Carsten Schmidt)]

atlas is a complicated port with very specific needs that most ports don't have, so I'm fine with filtering tickets though Vince so that he maintains a cohesive understanding of how the port works.

[larryv (Lawrence Velázquez)]

Conversely, Vince is probably the only person who understands all the repercussions of making changes to the port—even small ones.

He just overhauled the port completely, so the spate of new issues is not surprising.

[ryandesign (Ryan Carsten Schmidt)]

Right, so I think it's reasonable for Vince to be in charge of it, and to be free to veto any suggestions that don't fit with his vision for the port.

Also, Vince emailed me the rewritten portfile a month ago for review, but alas I didn't have time to review it. So now we just need to deal with the problems as we find them, as usual with any port.

[mf2k (Frank Schima)]

Cc Me!

[Veence (Vincent)]

Sorry, I was both away and kind of overburdened lately. Clang3.2 seems to be perfectly acceptable as a compiler as long as you don’t try to use AVX asm snippets. But it leads to inferior results w/r to gcc4.7. It may be acceptable for ppc. So I decided that rather to blindly blacklist it, it was best simply to warn the user they might run into trouble, and let them proceed.

Thanks for point out the basic defects of the Portfile. I’m going to patch it right away.

[Veence (Vincent)]

Committed in r105207. Should work now.

[Veence (Vincent)]

Besides, I perfectly agree that Atlas could be set under openmaintener. The purpose of the rewriting of the Portfile was precisely to make it more legible and easier to figure out…

[ryandesign (Ryan Carsten Schmidt)]

Before r105207, the port chose some wrong settings. Would all of those wrong settings have produced compile errors and halted the build? Or would some of them have been accepted and just resulted in atlas being compiled differently than intended? If the latter, the revision should be increased. But maybe you should first verify that it actually builds for you with most of the variants; it doesn't build for me or some other users, for which other tickets have been filed.

[Veence (Vincent)]

That’s exactly what I intended to do: try some extensive tests before bumping the revision. I’ll hopefully have some spare time next week to clean up all this mess (at least, I hope ! :)) Thanks Ryan for your help.

[Veence (Vincent)]

Everything seems to work fine now. I close this bug.