[prev in list] [next in list] [prev in thread] [next in thread] 

List:       macports-tickets
Subject:    Re: [MacPorts] #37844: Port submission: leveldb
From:       MacPorts <noreply () macports ! org>
Date:       2013-01-31 13:47:27
Message-ID: 067.80984c0a3f08ddab341ed064d41bc9f8 () macports ! org
[Download RAW message or body]

#37844: Port submission: leveldb
-------------------------+--------------------------------
  Reporter:  dw@…        |      Owner:  macports-tickets@…
      Type:  submission  |     Status:  new
  Priority:  Normal      |  Milestone:
 Component:  ports       |    Version:
Resolution:              |   Keywords:
      Port:              |
-------------------------+--------------------------------

Comment (by dw@…):

 Thanks for taking the time to reply to a noob.. :P~


 >  * The distname line can be removed; that's the default.
 >  * "configure {}" should be replaced with "use_configure no"; see "port
 lint"

 Fixed.


 >  * You're hardcoding the list of architectures to use in the universal
 variant, and not setting any architecture when not using the universal
 variant; you need to always set architectures, and use the list of
 architectures the user requested. The [get_canonical_archflags] procedure
 is useful for this. Ensure you define at least an empty universal variant
 ("variant universal {}") before invoking [get_canonical_archflags] (and in
 this portfile I don't think you'll need more than an empty universal
 variant).
 >  * In the universal variant, the second build.env statement overrides
 the first. If you want to append to the environment rather than
 overwriting it, use build.env-append, but note that you can set multiple
 environment variables in a single build.env invocation, which is usually
 preferable.


 >  * The snappy variant only adds a dependency; it does nothing to ensure
 that snappy will ''not'' be used if the snappy variant is ''not''
 selected. Is there a reason why snappy needs to be a variant? Couldn't
 snappy support just be enabled all the time?

 In this case, leveldb's build_detect_platform notices the libraries and
 enables them, so all required is to ensure they exist. I concur that just
 removing the variant makes more sense: snappy is a motivating feature of
 LevelDB and it can still be disabled at runtime. The alternative is
 battling the simplistic build_detect_platform into ignoring it.


 >  * In destroot, you don't need to create the lib directory; MacPorts
 automatically creates all the directories of the standard mtree for you.

 Removed.


 >  * What's the "platform darwin" block all about? Why is INSTALL_PATH set
 to ${prefix}/lib only on Darwin? What will happen on other OSes?

 Blindly copying examples from other Portfiles. :(  I've moved it to the
 single remaining build.end-append statement. It controls -rpath via
 build_detect_platform.


 >  * +universal should usually not be a default variant, unless there are
 extenuating circumstances.

 Fixed.


 Please find a revised Portfile attached.

-- 
Ticket URL: <https://trac.macports.org/ticket/37844#comment:3>
MacPorts <http://www.macports.org/>
Ports system for Mac OS
_______________________________________________
macports-tickets mailing list
macports-tickets@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-tickets

[prev in list] [next in list] [prev in thread] [next in thread]