[MacPorts] #28276: Upgrade ARB 5.2 -> 5.2.1

[MacPorts]

#28276: Upgrade ARB 5.2 -> 5.2.1
----------------------------------+-----------------------------------------
 Reporter:  matt.cottrell@…       |       Owner:  macports-tickets@…                   
     Type:  update                |      Status:  new                                  
 Priority:  Normal                |   Milestone:                                       
Component:  ports                 |     Version:  1.9.2                                
 Keywords:  libpng                |        Port:  arb                                  
----------------------------------+-----------------------------------------

Comment(by matt.cottrell@…):

 Replying to [comment:5 ryandesign@…]:

 I'm now back home from my work in Antarctica and ready to move forward on
 fixing the ARB port.  All of the suggestions by ryandesign look good to
 me.  Let's start committing those changes.  I'll comment below on some
 specific items.

 > We really do need to base the build architecture on the
 ${configure.build_arch} variable, as Joshua said (or
 ${configure.universal_archs}, if building universal), and not on the
 operating system version. This is necessary because those are the
 architectures which MacPorts will record in the registry for this port
 when it is installed, and those are the architectures for which MacPorts
 will verify this port's dependencies are installed.
 >
 Yes, let's use he ${configure.build_arch} variable.

 > If you really do want to prevent installation on 32-bit systems, then
 "return -code error" in a pre-fetch block if build_arch is i386 or ppc.
 >
 > MacPorts is not "overly conservative" as you say; the build_arch is set
 to x86_64 by default in the macports.conf on 64-bit Snow Leopard systems,
 matching Apple's defaults for their gcc compiler. If build_arch is set to
 i386 in your macports.conf, it is either because your computer is not
 64-bit, or because you deliberately changed it, or because you upgraded
 from Tiger or Leopard and forgot to change it at that time; if the latter,
 please spend some time comparing your macports.conf with the
 macports.conf.default in the same directory, and adopt any changes from
 macports.conf.default that you don't specifically mean to change.
 >
 > Further problems with this port (not all related to the current patch):
 >
 > Your patch is still saying to link with libpng12.dylib; no such dylib is
 installed by any MacPorts port. Please patch the source so it can compile
 with libpng14.dylib.
 >
 Okay, I'll ask the ARB developers to update their code to libpng14.  To
 tide us over I hope we can rely on using libpng12 as described in "If you
 need a specific version, use, e.g., -lpng12 -lz." See the last few lines
 of  http://www.libpng.org/pub/png/src/libpng-1.2.x-to-1.4.x-summary.txt

 > It looks like it's still looking in /usr/X11 for includes and libaries
 (the Makefiles contain directives to do this) which is where it's finding
 libpng12.dylib; this should be changed to look in ${prefix}.

 You are right.  The libraries that were being liked in /usr/X11 can, in
 fact, be found under ${prefix}.  Let's do that.

 >
 > This port should use "notes", not a ui_msg in post-activate. This
 message should not hardcode /opt/local; it should use ${prefix}.
 >
 Good suggestion.  Let's go for it.

 > The Mac OS X SDK path should not be hardcoded into the port; it should
 be taken from the variable ${configure.sdkroot}. (This will be empty
 everywhere except PowerPC computers running Tiger, because that is the
 only time there is a need to use an SDK.)
 >

 Good point.

 > Are gsed and imake really library dependencies -- are they really used
 by this software at runtime? These are usually only build dependencies.
 Also not sure what arb uses sablotron and lynx for but I don't see it
 linking to sablotron's libraries, and lynx doesn't provide any.
 >
 Yes, gsed is used by ARB at runtime to view the help files.  You are
 probably right about the others not being used at runtime.  Let's fix
 that.

 > When I tried to install it, it complained that it could not find
 makedepend, so a build dependency on the makedepend port should be added.
 >
 Good point.  But something else might be going on here because many folks
 have installed this port successfully without the makedepend dependency.

 > The post-activate "system 'rm -rf !`find ...`'" call is strange and
 should probably either be moved to the destroot and modified to use fs-
 traverse, or "svn.method checkout" should be removed to let MacPorts use
 svn export instead, which wouldn't have any .svn directories to begin
 with.
 >
 Sorry for my naive use of svn.  Let's do it your way, which is much
 cleaner.

 > All the "system" calls in the post-destroot should be replaced with the
 equivalent native Tcl commands.
 >
 This is an excellent suggestion.   Probably a detail we can fix after the
 rest of the heavy lifting is done.

 > There's no need for post-configure and post-destroot blocks when you're
 already overriding the configure and destroot blocks; just put everything
 in there.
 >
 Sounds good to me.

 > It is odd to say "use_configure no" and then to define a configure
 block.
 >
 > The configure block reinplaces @@PREFIX@@ ''in the patchfile in the
 files directory''. A portfile must never do that. Clearly, the person who
 initially committed the arb_macsetup file (in r59128) first installed the
 port to ensure it worked, which replaced @@PREFIX@@ with the actual prefix
 in the patchfile, and then it was committed that way, already replaced.
 Instead, you must replace the string in files within the workpath only.
 >
 Yes, I certainly made of mess of that.  Glad you see how to fix it.

 > The repeated reinplaces for gsed in the configure block can be replaced
 with a single reinplace invocation.
 >
 Yes, much cleaner.

 > This port doesn't ensure it's UsingTheRightCompiler.
 >
 Okay, let's fix that.

 > Your patch purports to update the port to version 5.2.1 but you haven't
 changed the revision of the Subversion repository that's checked out, so I
 don't believe this is installing an updated version of the software at
 all.
 >
 Your are absolutely right.  My initial reason for revisiting the portfile
 was to fix the libpng12 vs libpng14 problem.  It's not an update to ARB
 itself.

 > This port fetches 45MB of files from the project's Subversion
 repository; this takes a long time and [ticket:16373 is not cached
 locally] so if I ever want to reinstall I have to download it all again.
 It would be better if we could fetch a normal compressed distfile, which
 our mirror servers could then automatically mirror as well. What would be
 the problem with using [http://download.arb-
 home.de/release/arb_5.2/arbsrc.tgz this arb 5.2 tarball]? If we do that,
 we will of course have to [wiki:PortfileRecipes#unversioned-distfiles set
 dist_subdir].
 >
 Great.  I glad you see how to fix this.  That always bothered me, but I
 didn't know how to solve that problem.

 > I'm attaching my proposed changes which address some of the above. It's
 all together as one diff, but when I commit it, I'll probably want to
 separate it into a dozen or more commits, to address several of the issues
 separately, so the history stays readable. I may take another look at this
 later, but I think that's enough for now.

 Great, if you have the time, I would really appreciate it if you could
 make those commits, little by little, and let's see how things go.

-- 
Ticket URL: <https://trac.macports.org/ticket/28276#comment:6>
MacPorts <http://www.macports.org/>
Ports system for Mac OS