Opened 12 years ago

Closed 12 years ago

#35018 closed update (fixed)

metis: update to 5.0.2

Reported by: zan@… Owned by: ryandesign (Ryan Carsten Schmidt)
Priority: Normal Milestone:
Component: ports Version: 2.1.1
Keywords: haspatch Cc: skymoo (Adam Mercer), neverpanic (Clemens Lang)
Port: metis

Description (last modified by ryandesign (Ryan Carsten Schmidt))

Updated Metis to version 5.0.2. Related ports SuiteSparse (updated) and scipy (patched).

Solves ticket #32788.

Tested with Apple gcc-4.2, llvm-gcc-4.2, clang, and Macports gcc-4.5 on Snow Leopard 10.6.8, 32/64 bit.

Attachments (1)

Portfile.diff (4.7 KB) - added by zan@… 12 years ago.
Unified diff of Portfile

Download all attachments as: .zip

Change History (7)

Changed 12 years ago by zan@…

Attachment: Portfile.diff added

Unified diff of Portfile

comment:1 Changed 12 years ago by jmroot (Joshua Root)

Cc: jmr@… removed

comment:2 Changed 12 years ago by neverpanic (Clemens Lang)

When generating future patch files, please try to separate functional from whitespace changes. Not doing so makes the patch harder to read than necessary.

What purpose does setting configure.universal_args to {} have? Do you want to disable building +universal? In that case you should use universal_variant no and best, add a comment why +universal doesn't work.

comment:3 Changed 12 years ago by neverpanic (Clemens Lang)

Cc: cal@… added

Cc Me!

comment:4 Changed 12 years ago by ryandesign (Ryan Carsten Schmidt)

Description: modified (diff)

comment:5 Changed 12 years ago by ryandesign (Ryan Carsten Schmidt)

Owner: changed from macports-tickets@… to ryandesign@…
Status: newassigned

Thanks for the update!

Clemens: Clearing configure.pre_args and configure.universal_args is fine and necessary for some non-autoconf configure scripts; this configure script isn't even a configure script—it's make—so clearing those variables is right and good in this situation.

Often in these cases we'd still need to manually handle -arch flags, like the previous version of the port did. But cmake appears to be handling it correctly based only on the environment variables MacPorts passes at configure time. So everything's fine there.

Clearing variables is usually done by listing the variable name with no value after it, not by listing the value {}.

Setting build.cmd and destroot.cmd to /usr/bin/make is not necessary because that is the default. And instead of clearing build.args and build.pre_args, usually you would clear build.target—like the previous version of the Portfile did. I see however that it's having problems with the "-w" argument that MacPorts base now adds to build.pre_args. I'm not real clear on why "-w" was added to the default build.pre_args (documentation for "-w" says you usually don't need to add it manually), nor why "-w" is getting translated to "w" somewhere within metis' build system. But the "-w" argument appears to be the only problem, and setting (build|destroot).pre_args back to just (build|destroot).target, like it was in older versions of MacPorts, works.

You've added our standard modeline, which is good, but while you've made some whitespace changes, the whitespace does not conform to what the modeline says. The standard modeline should only be added if the port's whitespace already conforms to it, or if in the same commit you make the whitespace conform. In particular this means using spaces, at four spaces per indent, instead of tabs.

You removed a comment which explained why we were taking the unusual step of deleting -L${prefix}/lib from configure.ldflags. That comment should be retained, if indeed that line is still needed and still performs that function. Rather, I guess since the build system has totally changed (to cmake) that this line isn't necessary anymore and can be removed along with the comment.

The openmpi variant you added needs to conflict with the universal variant, because the openmpi port does not have a universal variant. I can't test this variant further because I can't build openmpi (#34302).

comment:6 Changed 12 years ago by ryandesign (Ryan Carsten Schmidt)

Resolution: fixed
Status: assignedclosed

It appears we don't need cc=${configure.cc} in configure.args either; it already picks up the correct compiler from the CC environment variable MacPorts sets, and furthermore then supports ccache if the user has enabled it in macports.conf (like I have).

I committed the update in r94816, with all the new and changed lines conforming to our preferred whitespace style. In r94817 I added the modeline and converted the remaining lines to the new whitespace style.

One undesirable consequence of this update is that we now only have a static library instead of a dynamic library. I've filed ticket #35041 for this.

Note: See TracTickets for help on using tickets.