Opened 14 years ago

Closed 14 years ago

#25562 closed update (fixed)

miriad 4.1.3.20100512 - update to version 4.1.3.20100706

Reported by: pkgw (Peter Williams) Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version: 1.9.1
Keywords: haspatch maintainer Cc: ryandesign (Ryan Carsten Schmidt)
Port: miriad

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

Attached are changes to the MIRIAD MacPort to update it to the newest upstream release and make other correctness fixes.

For readability, I've separated the changes into five cumulative patches. Their effects are:

  1. Update to newest MIRIAD upstream release
  2. Remove unnecessary manual --prefix option to configure
  3. Explicitly disable building of docs unless requested with +docs
  4. Implement telescope choice with a default-on variant +ata
  5. Allow the user to use custom compilers, if he/she wants to live dangerously, via the gcc_select mechanism.

I suspect the last one might be a bit controversial. MIRIAD is a heavily numerical piece of software, and compiler choice can make a large difference in its performance. Several users have wanted to use proprietary compilers to build MIRIAD to take advantage of their better numerical optimizations. The people who are interested in doing this are aware that this is not a configuration that can be generically supported by myself or MacPorts.

Attachments (6)

miriad-1.diff (819 bytes) - added by pkgw (Peter Williams) 14 years ago.
Update to new upstream release 20100706
miriad-2.diff (719 bytes) - added by pkgw (Peter Williams) 14 years ago.
Remove superfluous --prefix argument to configure
miriad-3.diff (950 bytes) - added by pkgw (Peter Williams) 14 years ago.
Explicitly disable docs unless given +docs
miriad-4.diff (3.1 KB) - added by pkgw (Peter Williams) 14 years ago.
Implement default telescope choice as default +ata variant
miriad-5.diff (2.1 KB) - added by pkgw (Peter Williams) 14 years ago.
Allow user to use a nonstandard compiler if they really want to
miriad-5-ryandesign.diff (2.3 KB) - added by ryandesign (Ryan Carsten Schmidt) 14 years ago.
proposed revised patch

Download all attachments as: .zip

Change History (19)

Changed 14 years ago by pkgw (Peter Williams)

Attachment: miriad-1.diff added

Update to new upstream release 20100706

Changed 14 years ago by pkgw (Peter Williams)

Attachment: miriad-2.diff added

Remove superfluous --prefix argument to configure

Changed 14 years ago by pkgw (Peter Williams)

Attachment: miriad-3.diff added

Explicitly disable docs unless given +docs

Changed 14 years ago by pkgw (Peter Williams)

Attachment: miriad-4.diff added

Implement default telescope choice as default +ata variant

comment:1 Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

Description: modified (diff)

comment:2 in reply to:  description Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to peter@…:

  1. Remove unnecessary manual --prefix option to configure

r69571

comment:3 in reply to:  description Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to peter@…:

  1. Update to newest MIRIAD upstream release
  1. Explicitly disable building of docs unless requested with +docs

r69572. I also added a second checksum type (sha160).

comment:4 in reply to:  description ; Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to peter@…:

  1. Implement telescope choice with a default-on variant +ata

There are two problems with this patch, and they're both the same problem: you're specifying default variants even if the user has already requested a conflicting other variant.

First, let's take the gcc variants. Before, gcc44 was the default variant only if gcc43 was not already selected. This was the correct strategy. After your changes, a user requesting the gcc43 variant will receive this error:

$ sudo port install miriad +gcc43
Password:
Error: miriad: Variant gcc43 conflicts with gcc44
Error: Unable to open port: Error evaluating variants

The solution is to not change how the gcc44 default variant is specified; leave it exactly how it was.

The second problem is the telescope variants. If the user requests the carma variant, this error will appear:

$ sudo port install miriad +carma
Error: miriad: Variant ata conflicts with carma
Error: Unable to open port: Error evaluating variants
To report a bug, see <http://guide.macports.org/#project.tickets>

The solution is to model the ata variant default specification on the way the gcc variant default specification was being done: make ata the default variant only if a conflicting telescope variant (i.e. carma) has not already been specified.

What I'm implying here but should state explicitly is that default variants accumulate, they don't overwrite one another.

Committed a version that doesn't have these problems in r69573.

comment:5 in reply to:  description ; Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

Cc: ryandesign@… added

Replying to peter@…:

  1. Allow the user to use custom compilers, if he/she wants to live dangerously, via the gcc_select mechanism.


I suspect the last one might be a bit controversial. MIRIAD is a heavily numerical piece of software, and compiler choice can make a large difference in its performance. Several users have wanted to use proprietary compilers to build MIRIAD to take advantage of their better numerical optimizations. The people who are interested in doing this are aware that this is not a configuration that can be generically supported by myself or MacPorts.

You're right, I find this controversial, and am not committing it yet. You say this is meant to let the user use any compiler, yet you restrict it to run ${prefix}/bin/gcc. This implies you're not letting the user run any compiler installed anywhere after all; you're only letting them run a compiler that can be selected with the MacPorts gcc_select tool. In that case, why not just offer variants for each compiler MacPorts gcc_select can select?

comment:6 in reply to:  4 ; Changed 14 years ago by pkgw (Peter Williams)

Replying to ryandesign@…:

There are two problems with this patch, and they're both the same problem: you're specifying default variants even if the user has already requested a conflicting other variant.

OK, thanks for committing an improved version.

In case you're interested in some context from a non-core Portfile author, I had misunderstood what the default_variants directive did. Going from guide.macports.org, I thought that it was a sort of global variable that specified what variants would be used if the user didn't specify anything on the commandline. Based on your change and a peek at portutil.tcl, my understanding is that it's a directive that enables the specified variants when executed in the portfile code. (I had wondered why the original compiler-selection code was written the way it was; now that makes sense.) I'd say that default_variants would be better named "enable_variants" or something, but obviously there are backwards-compatibility issues related to that.

comment:7 in reply to:  5 ; Changed 14 years ago by pkgw (Peter Williams)

Replying to ryandesign@…:

You're right, I find this controversial, and am not committing it yet. You say this is meant to let the user use any compiler, yet you restrict it to run ${prefix}/bin/gcc. This implies you're not letting the user run any compiler installed anywhere after all; you're only letting them run a compiler that can be selected with the MacPorts gcc_select tool. In that case, why not just offer variants for each compiler MacPorts gcc_select can select?

Because my intention was to have some special instructions for these power users that had them write their own gcc_select data files in ${prefix}/etc/select/gcc that pointed to their special compilers.

This was the only solution I could come up with given the constraints that 1) users can only input information to the Portfile in binary form by selection/deselection of variants 2) I don't know where the users will install their fancy compilers or what names they'll have. The gcc_select mechanism allows the port to be fed the locations of the fancy compilers implicitly, by the symlinks it creates. We can't just say CC=cc and let $PATH do the work, I think, because it doesn't seem like you can override the $PATH used to build ports, which is an entirely reasonable thing if that is indeed the case.

comment:8 in reply to:  6 Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to peter@…:

In case you're interested in some context from a non-core Portfile author, I had misunderstood what the default_variants directive did. Going from guide.macports.org, I thought that it was a sort of global variable that specified what variants would be used if the user didn't specify anything on the commandline. Based on your change and a peek at portutil.tcl, my understanding is that it's a directive that enables the specified variants when executed in the portfile code. (I had wondered why the original compiler-selection code was written the way it was; now that makes sense.) I'd say that default_variants would be better named "enable_variants" or something, but obviously there are backwards-compatibility issues related to that.

I initially (years ago) also misunderstood how default_variants works. I have not looked at the base code for this, but I had assumed it was a variable, like configure.args and most other portfile statements, and that setting it would overwrite what was there before. After all, in order to append to any other variable, you use the -append procedure (i.e. "configure.args-append"). But no, just calling "default_variants +foo" on its own causes the variant +foo to be appended to the list of default variants. Its design is a bit strange compared to the rest of MacPorts.

Note that renaming it to "enable_variants" would not be ideal, because with default_variants you can also disable variants. For example, "default_variants +x11 -universal" enables the x11 variant by default and disables the universal variant by default. Since all variants are disabled anyway to start with, the only purpose for this that I see would be to defeat variants a user might have specified in their variants.conf, but I don't know why a port author would want to do such a thing. Note that what's written in default_variants is only a default; the user is still free to override those defaults at the command line (e.g. with "sudo port install someport -x11"). We do also have the variant_set and variant_unset procedures available, which the user cannot override at the command line, but these have thus far not been used much in portfiles.

The MacPorts Guide could certainly use some improvements to make this and other things clearer. Unlike the Hitchhiker's Guide to the Galaxy, our Guide is unfortunately not definitive, not yet anyway. Patches for the documentation are appreciated.

comment:9 in reply to:  7 ; Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to peter@…:

my intention was to have some special instructions for these power users that had them write their own gcc_select data files in ${prefix}/etc/select/gcc that pointed to their special compilers.

This was the only solution I could come up with given the constraints that 1) users can only input information to the Portfile in binary form by selection/deselection of variants 2) I don't know where the users will install their fancy compilers or what names they'll have. The gcc_select mechanism allows the port to be fed the locations of the fancy compilers implicitly, by the symlinks it creates. We can't just say CC=cc and let $PATH do the work, I think, because it doesn't seem like you can override the $PATH used to build ports, which is an entirely reasonable thing if that is indeed the case.

Users can actually change the $PATH MacPorts uses (it's the binpath variable in macports.conf), but doing so is probably not a super idea; your gcc_select idea is probably the cleaner of the two. And as the author of such convoluted portfiles as oracle-instantclient and minivmac, I'm hardly in a position to complain.

Changed 14 years ago by pkgw (Peter Williams)

Attachment: miriad-5.diff added

Allow user to use a nonstandard compiler if they really want to

comment:10 in reply to:  9 Changed 14 years ago by pkgw (Peter Williams)

Replying to ryandesign@…:

Users can actually change the $PATH MacPorts uses (it's the binpath variable in macports.conf), but doing so is probably not a super idea; your gcc_select idea is probably the cleaner of the two. And as the author of such convoluted portfiles as oracle-instantclient and minivmac, I'm hardly in a position to complain.

I've updated patch #5 to handle the variant selection correctly (I think). How does it look now? It's definitely not elegant, but it is the most reasonable thing that I can think of given the constraints.

Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

Attachment: miriad-5-ryandesign.diff added

proposed revised patch

comment:11 Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

I think it would be simpler if the variant code were within the actual variants. See what you think of this. Additional changes I made: renamed gccselect variant to gcc_select to match the name of the program, and made the compiler ports library (not build) dependencies (because the programs do link with libraries provided by the compiler port).

comment:12 Changed 14 years ago by pkgw (Peter Williams)

That looks good to me. I'd say that the bit that enables "default_variants +gcc44" typifies what's suboptimal about the semantics of that command, but it does seem like the best option.

comment:13 in reply to:  12 Changed 14 years ago by ryandesign (Ryan Carsten Schmidt)

Resolution: fixed
Status: newclosed

Replying to peter@…:

That looks good to me.

Committed in r69773.

I'd say that the bit that enables "default_variants +gcc44" typifies what's suboptimal about the semantics of that command, but it does seem like the best option.

Yes, but at least this is how other ports handle default_variants, so even if the syntax is wordy, it is the same wordiness portfile authors should already be familiar with when implementing this feature.

Note: See TracTickets for help on using tickets.