Opened 13 years ago

Closed 13 years ago

#28276 closed update (fixed)

arb: Upgrade to 5.2

Reported by: matt.cottrell@… Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version: 1.9.2
Keywords: libpng Cc: mf2k (Frank Schima),ryandesign (Ryan Carsten Schmidt)
Port: arb

Description

This is an upgrade of ARB 5.2 to 5.2.1.

ARB was broken by the MacPorts upgrade from libpng 1.2.44 to 1.4.5 three weeks ago as identified by ticket #28238. The solution for now is to explicitly specify in the makefile -l png12, which lives in /usr/X11R6/lib/libpng12.dylib.

I'm including fixes to some minor issues as well.

Attachments (20)

arb_intro.fig.diff (5.1 KB) - added by matt.cottrell@… 13 years ago.
New patch file arb_intro.fig.diff
arb_macsetup.diff (2.9 KB) - added by matt.cottrell@… 13 years ago.
Patch file to tweak the arb_macsetup script
Portfile.diff (3.3 KB) - added by matt.cottrell@… 13 years ago.
Updated Portfile.diff now includes 64-bit for darwin 10
patch-ARB-makefile.diff (2.2 KB) - added by matt.cottrell@… 13 years ago.
No longer linking against libpng in /usr/X11R6
arb-5.2-ryandesign.diff (22.2 KB) - added by ryandesign (Ryan Carsten Schmidt) 13 years ago.
proposed changes (incomplete)
arb-5.2-libpng14.dylib-patched.diff (23.9 KB) - added by matt.cottrell@… 13 years ago.
ARB source now patched for libpng14.dylib
arb-5.2 consolidated config.env to build.env.diff (24.0 KB) - added by matt.cottrell@… 13 years ago.
config.env -> build.env
arb-5.2-build_arch-x86_64-only.diff (24.7 KB) - added by matt.cottrell@… 13 years ago.
Allow build_arch x86_64 only
Portfile-OS-and-hardware-bitness.diff (12.2 KB) - added by matt.cottrell@… 13 years ago.
Build 32- or 64-bit (Portfile vs current Portfile in the repository)
patch-ARB-glpng.diff (1.0 KB) - added by matt.cottrell@… 13 years ago.
Patch provided by the official ARB developers - solves crashes with earlier patch from Matt Cottrell
patch-ARB-makefile.2.diff (2.5 KB) - added by matt.cottrell@… 13 years ago.
Revised Makefile patch needed to compile against png.h (v14) provided by MacPorts
Portfile.2.diff (7.3 KB) - added by matt.cottrell@… 13 years ago.
Added dependency xorig-libXaw
arb_macsetup.2.diff (2.7 KB) - added by matt.cottrell@… 13 years ago.
Fixes a problem with creating xmodmap file
patch-ARB-makefile.3.diff (2.5 KB) - added by matt.cottrell@… 13 years ago.
Fixes the libpng 1.4 runtime error (look in PREFIX for XINCLUDES)
Portfile.3.diff (533 bytes) - added by matt.cottrell@… 13 years ago.
Added depends xorg-libXaw
Portfile.4.diff (1.8 KB) - added by matt.cottrell@… 13 years ago.
Fetch a normal compressed distfile
Portfile.5.diff (4.1 KB) - added by matt.cottrell@… 13 years ago.
Don't clobber the config file -> post-activate
patch-ARB-DARWIN.pl.diff (793 bytes) - added by matt.cottrell@… 13 years ago.
Response to perl5.12.3 update
arb-proposed-5.2-update.diff (11.9 KB) - added by ryandesign (Ryan Carsten Schmidt) 13 years ago.
proposed patch to update to 5.2
arb_macsetup.3.diff (3.5 KB) - added by matt.cottrell@… 13 years ago.
Corrects capitalization of PREFIX

Download all attachments as: .zip

Change History (62)

Changed 13 years ago by matt.cottrell@…

Attachment: arb_intro.fig.diff added

New patch file arb_intro.fig.diff

Changed 13 years ago by matt.cottrell@…

Attachment: arb_macsetup.diff added

Patch file to tweak the arb_macsetup script

comment:1 Changed 13 years ago by matt.cottrell@…

Just a heads up that I am now at Palmer Station, Antarctica and will be departing tomorrow (Saturday) morning. I won't have an internet connection on the ship during the four day crossing of the Drake Passage, but I'll be back in the States on the 11th.

Changed 13 years ago by matt.cottrell@…

Attachment: Portfile.diff added

Updated Portfile.diff now includes 64-bit for darwin 10

comment:2 Changed 13 years ago by jmroot (Joshua Root)

Linking against the libpng in /usr/X11R6 is not acceptable. Please update the code for compatibility with libpng14 if possible, otherwise create a libpng12 port if it's really impossible. Also, you should not choose whether to build 64-bit based solely on the OS version. Look at $build_arch.

comment:3 in reply to:  2 Changed 13 years ago by matt.cottrell@…

Replying to jmr@…:

Linking against the libpng in /usr/X11R6 is not acceptable. Please update the code for compatibility with libpng14 if possible, otherwise create a libpng12 port if it's really impossible. Also, you should not choose whether to build 64-bit based solely on the OS version. Look at $build_arch.

That libpng12 vs libpng14 summary was very useful. No longer linking against libpng in /usr/X11R6. See updated patch-ARB-makefile.diff.

Changed 13 years ago by matt.cottrell@…

Attachment: patch-ARB-makefile.diff added

No longer linking against libpng in /usr/X11R6

comment:4 in reply to:  2 Changed 13 years ago by matt.cottrell@…

Replying to jmr@…:

Linking against the libpng in /usr/X11R6 is not acceptable. Please update the code for compatibility with libpng14 if possible, otherwise create a libpng12 port if it's really impossible. Also, you should not choose whether to build 64-bit based solely on the OS version. Look at $build_arch.

I would prefer to stick with my current approach, choosing 64-bit vs 32-bit based on the OS version. The build_arch variable appears to be overly conservative. On both of my machines running OS 10.6 (Intel Xeon CPU X5482 @ 3.20GHz and Intel Core2 Duo CPU P8700 @ 2.53GHz) build_arch is set to i386 in the macports.conf file. 64-bit ARB runs just fine on both of them.

Truth be told, the only useful version of ARB is 64-bit because modern DNA sequences databases are so large. Users would be better served if ARB would not install if a combination of hardware and OS was detected that absolutely cannot run 64-bit. Any insight on accomplishing that?

Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

Attachment: arb-5.2-ryandesign.diff added

proposed changes (incomplete)

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

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.

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.

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}.

This port should use "notes", not a ui_msg in post-activate. This message should not hardcode /opt/local; it should use ${prefix}.

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.)

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.

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.

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.

All the "system" calls in the post-destroot should be replaced with the equivalent native Tcl commands.

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.

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.

The repeated reinplaces for gsed in the configure block can be replaced with a single reinplace invocation.

This port doesn't ensure it's UsingTheRightCompiler.

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.

This port fetches 45MB of files from the project's Subversion repository; this takes a long time and 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 this arb 5.2 tarball? If we do that, we will of course have to set dist_subdir.

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.

comment:6 in reply to:  5 Changed 13 years ago by matt.cottrell@…

Replying to 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 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 this arb 5.2 tarball? If we do that, we will of course have to 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.

Changed 13 years ago by matt.cottrell@…

ARB source now patched for libpng14.dylib

comment:7 Changed 13 years ago by matt.cottrell@…

Patching the ARB source for libpng14.dylib compatibility was not as difficult as I thought it might be. Please take a look and give it a try.

comment:8 Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

Summary: Upgrade ARB 5.2 -> 5.2.1arb: Upgrade to 5.2

Here's a start:

It occurs to me that the configure.env you're setting isn't used anywhere other than in setting build.env, so why don't we consolidate all those environment variables in build.env?

Changed 13 years ago by matt.cottrell@…

config.env -> build.env

comment:9 in reply to:  8 ; Changed 13 years ago by matt.cottrell@…

config.env now consolidated into build.env

Changed 13 years ago by matt.cottrell@…

Allow build_arch x86_64 only

comment:10 in reply to:  9 ; Changed 13 years ago by matt.cottrell@…

Allow build_arch x86_64 only. Prompt the user to confirm that build_arch is set properly in macports.conf. User may have forgotten to update build_arch after upgrading the OS to a 64-bit version.

comment:11 in reply to:  10 ; Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

  • r75953: consolidate configure.env into build.env

Replying to matt.cottrell@…:

Allow build_arch x86_64 only. Prompt the user to confirm that build_arch is set properly in macports.conf. User may have forgotten to update build_arch after upgrading the OS to a 64-bit version.

I don't think we want that. What if the user's system really is 32-bit only -- as in, PowerPC G4 or Intel Core (not Core 2)? What if the user is on Leopard or Tiger, where 32-bit building is the default? If this software will not work well as 32-bit, you could write a ui_warn at pre-configure time alerting the user to this fact, but I would not prevent the build if the software will technically work 32-bit. If you like, you can tailor the message based on whether the user's computer supports 64-bit. MacPorts uses sysctl hw.cpu64bit_capable to determine this; if it prints "1" the CPU is 64-bit capable; if it shows "0", then it's not. If it's "1", your message could advise the user to change build_arch; if it's "0", you could tell the user to buy a new computer. :) In any case, do not hardcode /opt/local; use ${prefix}.

As I work to commit these individual pieces, evaluating your monolithic diffs against the original portfile becomes more difficult. For future proposed changes, please supply a diff against the current port in the repository making only those changes.

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

  • r75955: add libpng 1.4 patch and libpng dependency (I did not apply your proposed changing of #include <png.h> to #include <libpng14/png.h> because it seems unnecessary; png.h is a symlink to libpng14/png.h)
  • r75956: consolidate post-destroot into destroot
  • r75957: use file attributes instead of system "chmod"

Two of the things being made writeable (formerly using system "chmod", now file attributes) are directories; fine; you want regular users to be able to put files into them. The third is a file called arb_tcp.dat, which lib/Makefile copies from arb_tcp_org.dat. This feels like a config file that the user is expected to edit; is that right? If so, we should not register the file to the port. Currently, any changes the user makes to this file are lost when they upgrade the port. Let me know and I can make these changes.

comment:13 in reply to:  11 ; Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

MacPorts uses sysctl hw.cpu64bit_capable to determine this; if it prints "1" the CPU is 64-bit capable; if it shows "0", then it's not. If it's "1", your message could advise the user to change build_arch; if it's "0", you could tell the user to buy a new computer. :)

Sorry, what portfile keyword returns "1" or "0" based on the 64-bit capability of the hardware? I searched the portfiles for "sysctl hw.cpu64bit_capable" and I can't find any examples of portfiles using that syntax.

comment:14 in reply to:  12 Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

Two of the things being made writeable (formerly using system "chmod", now file attributes) are directories; fine; you want regular users to be able to put files into them. The third is a file called arb_tcp.dat, which lib/Makefile copies from arb_tcp_org.dat. This feels like a config file that the user is expected to edit; is that right? If so, we should not register the file to the port. Currently, any changes the user makes to this file are lost when they upgrade the port. Let me know and I can make these changes.

That's right, arb_tcp.dat is a config file that will be edited by the user. We need to preserve its contents and not overwrite it when the port is upgraded. Likewise the files pts and macros should not be overwritten upon upgrading the port.

Thanks for catching that one.

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

Replying to matt.cottrell@…:

Sorry, what portfile keyword returns "1" or "0" based on the 64-bit capability of the hardware? I searched the portfiles for "sysctl hw.cpu64bit_capable" and I can't find any examples of portfiles using that syntax.

No port keyword does that, but the sysctl Tcl procedure (which is a wrapper around the sysctl shell command) does. gtk2, gtk3 and gprolog are the ports I found that are using this, and MacPorts base does as well.

Be aware that sysctl hw.cpu64bit_capable was introduced with Leopard; it will generate an error message on Tiger.

    if {${os.platform} == "darwin" && ${os.major} >= 9} {
        if {[sysctl hw.cpu64bit_capable] == 1} {
            ui_msg "64-bit capable"
        } else {
            ui_msg "not 64-bit capable"
        }
    } else {
        ui_msg "unable to determine 64-bit capability"
    }

On Tiger, if build_arch is a 32-bit arch, just assume 64-bit capability is not available; support for 64-bit building was spotty on Tiger anyway.

Replying to matt.cottrell@…:

That's right, arb_tcp.dat is a config file that will be edited by the user. We need to preserve its contents and not overwrite it when the port is upgraded. Likewise the files pts and macros should not be overwritten upon upgrading the port.

pts and macros are directories, not files. The pts directory is empty as installed, so nothing need be done there. The macros directory contains some files. Is the user going to be modifying those, or just adding their own files in the directory? If the latter, nothing need be done; MacPorts will not overwrite or remove files it doesn't know about.

comment:16 in reply to:  15 ; Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

pts and macros are directories, not files. The pts directory is empty as installed, so nothing need be done there. The macros directory contains some files. Is the user going to be modifying those, or just adding their own files in the directory? If the latter, nothing need be done; MacPorts will not overwrite or remove files it doesn't know about.

The user would be adding their own files to the macros directory not modifying the existing files, so nothing needs to be done.

Changed 13 years ago by matt.cottrell@…

Build 32- or 64-bit (Portfile vs current Portfile in the repository)

comment:17 in reply to:  16 Changed 13 years ago by matt.cottrell@…

Updated the Portfile to detect 32- vs 64-bit build. Think I got it right this time, maybe Warns user if running 32-bit OS on 64-bit hardware, continues to build the 32-bit version and suggests that user inspect build_arch in macports.conf.

Changed 13 years ago by matt.cottrell@…

Attachment: patch-ARB-glpng.diff added

Patch provided by the official ARB developers - solves crashes with earlier patch from Matt Cottrell

Changed 13 years ago by matt.cottrell@…

Attachment: patch-ARB-makefile.2.diff added

Revised Makefile patch needed to compile against png.h (v14) provided by MacPorts

comment:18 Changed 13 years ago by matt.cottrell@…

Testing ARB built with the latest patches I discovered some crashes. I've added two files (patch-ARB-glpng.diff and patch-ARB-makefile.2.diff) that solve those issues. The glpng patch was provided by the official ARB developer (thanks Ralf!) and the makefile patch is a to make ARB compile against the png.h (v14) provided by MacPorts.

comment:19 Changed 13 years ago by matt.cottrell@…

If we can resolve the last two ARB port issues it should't require any more work.

  1. Detecting 32- vs 64-bit build.
  2. libpng14 patch provided by the ARB developers

Two diffs uploaded in the last two days propose solutions.

Changed 13 years ago by matt.cottrell@…

Attachment: Portfile.2.diff added

Added dependency xorig-libXaw

Changed 13 years ago by matt.cottrell@…

Attachment: arb_macsetup.2.diff added

Fixes a problem with creating xmodmap file

comment:20 Changed 13 years ago by matt.cottrell@…

Additional testing revealed a few glitches in the ARB port. Uploaded files Portfile.2.diff and arb_macsetup.2.diff address them. One was a missing dependency and the other was a problem writing the .xmodmap file.

comment:21 Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

  • r76092: use upstream version of libpng 1.4 patch

comment:22 in reply to:  21 Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

  • r76092: use upstream version of libpng 1.4 patch

Thanks for committing the upstream patch, but the following runtime error occurs:

libpng warning: Application was compiled with png.h from libpng-1.2.35
libpng warning: Application  is  running with png.c from libpng-1.4.5
libpng warning: Incompatible libpng version in application and library

Please consider the following patch to the makefile, which is needed to compile against png.h (v14) provided by MacPorts

patch-ARB-makefile.2.diff   (2.5 KB) - added by matt.cottrell@me.com 4 days ago.
Revised Makefile patch needed to compile against png.h (v14) provided by MacPorts

Additional items that still need consideration include:

Portfile.2.diff   (7.3 KB) - added by matt.cottrell@me.com 23 hours ago.
Added dependency xorig-libXaw

and

arb_macsetup.2.diff   (2.7 KB) - added by matt.cottrell@me.com 23 hours ago.
Fixes a problem with creating xmodmap file

Changed 13 years ago by matt.cottrell@…

Attachment: patch-ARB-makefile.3.diff added

Fixes the libpng 1.4 runtime error (look in PREFIX for XINCLUDES)

comment:23 Changed 13 years ago by matt.cottrell@…

Attached patch-ARB-makefile.3.diff, which fixes the libpng 1.4 runtime error by looking for XINCLUDES in PREFIX (line 11 of the diff)

comment:24 Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

  • r76340: committed your libpng fix patch, with these changes:
    • removed "-I$(OSX_SDK)/usr/X11/include" from XINCLUDES definition on darwin; on all OSes we want X11 software to come from PREFIX
    • changed XHOME from /usr/X11R6 to PREFIX for the same reason
    • changed XMKMF to always be PREFIX/bin/xmkmf, not just on darwin, for the same reason
    • removed the hunk about OSX_SDK since we're already handling that with a reinplace in the portfile, and this doesn't relate to the task of fixing libpng
    • incremented the port revision

Hope this looks ok. I did not know how to reproduce the runtime libpng error you encountered, so I could not verify that these changes fix it.

Changed 13 years ago by matt.cottrell@…

Attachment: Portfile.3.diff added

Added depends xorg-libXaw

comment:25 in reply to:  24 Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

Hope this looks ok. I did not know how to reproduce the runtime libpng error you encountered, so I could not verify that these changes fix it.

Unable to test fix to runtime libpng error. ARB build fails with missing library (ld: library not found for -lXaw). Attached Portfile.3.diff fixes this and adds some longstanding missing depends needed for printing from ARB (xfig and gv)

After we fix this please stay tuned for more Portfile patches to come. Taking them one at a time.

comment:26 Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

  • r76365: added missing dependencies

comment:27 in reply to:  26 ; Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

  • r76365: added missing dependencies

ARB now builds without errors and passes the libpng 1.4 test

Changed 13 years ago by matt.cottrell@…

Attachment: Portfile.4.diff added

Fetch a normal compressed distfile

comment:28 in reply to:  27 Changed 13 years ago by matt.cottrell@…

Added Portfile.4.diff to stop using svn and fetch a normal compressed distfile

comment:29 Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

This doesn't work -- your patch introduces the checksums of arb 5.2 but leaves the version and download location of 5.1 so the checksums don't match.

We will want to update to 5.2 at the same time, I assume including your changes for the arb_macsetup script and notes.

However, before we update to 5.2, I'd like to fix the config file issue (arb_tcp.dat should not be installed, or at least should not be registered to the port); you probably should add something to the notes that explains to users either that they need to copy the sample config file, or if you're going to do that for them in the port, then explain to the user that they may need to keep that file updated w.r.t. the sample config file when the port is updated.

Changed 13 years ago by matt.cottrell@…

Attachment: Portfile.5.diff added

Don't clobber the config file -> post-activate

comment:30 in reply to:  29 ; Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

However, before we update to 5.2, I'd like to fix the config file issue...

Uploaded Portfile.5.diff which uses post-activate to avoid clobbering the config file. Useful information for the user about these config files added to the notes.

comment:31 in reply to:  30 ; Changed 13 years ago by matt.cottrell@…

Replying to matt.cottrell@…:

Replying to ryandesign@…:

However, before we update to 5.2, I'd like to fix the config file issue...

Ryan,

I've got a suggestion for the arb port using post-activate to avoid clobbering the config file. Could you take a look at my Portfile.5.diff? I'd appreciate your input.

Thanks, Matt

comment:32 Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

I have looked at it, and would like to commit it, with a number of changes, but I can't build arb anymore, perhaps due to the perl5.12 upgrade. Can you still build arb after upgrading to perl5.12?

comment:33 in reply to:  32 ; Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

I have looked at it, and would like to commit it, with a number of changes, but I can't build arb anymore, perhaps due to the perl5.12 upgrade. Can you still build arb after upgrading to perl5.12?

I'll try arb with perl5.12 and get back to you.

Changed 13 years ago by matt.cottrell@…

Attachment: patch-ARB-DARWIN.pl.diff added

Response to perl5.12.3 update

comment:34 in reply to:  33 ; Changed 13 years ago by matt.cottrell@…

Replying to matt.cottrell@…:

Replying to ryandesign@…:

I'll try arb with perl5.12 and get back to you.

Uploaded patch-ARB-DARWIN.pl.diff in response to perl5.12.3 update. Now arb builds again.

BTW, must be a better way of patching DARWIN.pl than my brittle approach that breaks when perl gets upgraded.

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

Replying to matt.cottrell@…:

I've got a suggestion for the arb port using post-activate to avoid clobbering the config file. Could you take a look at my Portfile.5.diff? I'd appreciate your input.

  • r76562: Committed it with these changes:
    • No need to add a pre-destroot block; I just added these lines to the beginning of the existing destroot block
    • In the post-activate block, you were copying the defaults from worksrcdir, but by post-activate time, the worksrcdir might have already been deleted; copy from the right place in ${prefix} instead
    • In post-activate, I removed the foreach loop over arb_tcp.dat macros, since files and directories need different handling. The file can be simply installed with the right permissions using "xinstall -m 777", whereas for the directory, I presumed that your intention was that the user be able to modify the files in it (not just modify the directory itself) so I added an fs-traverse block to traverse every item in the directory and make it writable too.
    • In post-activate, the several lines that deal with creating the pts directory can be much more simply accomplished using "xinstall -d" and "destroot.keepdirs", so I kept the "destroot.keepdirs" that was already there and added the "xinstall" to install it with the desired permissions
    • Changed the comment in post-activate to more accurately reflect what it's doing.

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

Replying to matt.cottrell@…:

Uploaded patch-ARB-DARWIN.pl.diff in response to perl5.12.3 update. Now arb builds again.

BTW, must be a better way of patching DARWIN.pl than my brittle approach that breaks when perl gets upgraded.

Took a bit of research, but I figured it out:

  • r76563: don't hardcode path to libperl dir

comment:37 Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

I think next up is the actual update to 5.2 (and that might be the last change needed?). I've attached what I think should be in that commit. It:

  • updates to 5.2
  • fetches from a proper distfile, removing all the previous Subversion-related tricks
  • adds your arb_intro.fig patch
  • updates your arb_macsetup and notes
  • makes imake a build dependency instead of a library dependency, and adds makedepend build dependency; in fact I can't find where arb uses imake, but it complains if these aren't there:
    The following tools are missing:
        /opt/local/bin/xmkmf, makedepend
    These tools are vital to compile ARB, so
    please ensure that these tools are installed and that either
    - their installation directory is in PATH or
    - they are linked into the PATH
    

Does this patch look ok to commit?

Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

proposed patch to update to 5.2

comment:38 in reply to:  37 Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

I think next up is the actual update to 5.2 (and that might be the last change needed?). I've attached what I think should be in that commit. It:

  • updates to 5.2
  • fetches from a proper distfile, removing all the previous Subversion-related tricks
  • adds your arb_intro.fig patch
  • updates your arb_macsetup and notes
  • makes imake a build dependency instead of a library dependency, and adds makedepend build dependency; in fact I can't find where arb uses imake, but it complains if these aren't there:
    The following tools are missing:
        /opt/local/bin/xmkmf, makedepend
    These tools are vital to compile ARB, so
    please ensure that these tools are installed and that either
    - their installation directory is in PATH or
    - they are linked into the PATH
    

Does this patch look ok to commit?

Thanks for all the heavy lifting. I'll test build and runtime functionality of the proposed 5.2 update and get back to you soon.

Changed 13 years ago by matt.cottrell@…

Attachment: arb_macsetup.3.diff added

Corrects capitalization of PREFIX

comment:39 in reply to:  37 Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

I think next up is the actual update to 5.2 (and that might be the last change needed?). I've attached what I think should be in that commit. It: Does this patch look ok to commit?

One minor problem with capitalization of PREFIX in arb_macsetup needed attention. I uploaded arb_macsetup.3.diff to address this issue. Other than that your proposed-5.2 update builds and runs without any errors. Please commit.

comment:40 Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

  • r76579: move imake to a build dependency; add makedepend build dependency
  • r76580: fix capitalization of PREFIX in arb_macsetup
  • r76582: update to 5.2, use a distfile instead of fetching from Subversion, add .xmodmap setup

comment:41 in reply to:  40 Changed 13 years ago by matt.cottrell@…

Replying to ryandesign@…:

  • r76579: move imake to a build dependency; add makedepend build dependency
  • r76580: fix capitalization of PREFIX in arb_macsetup
  • r76582: update to 5.2, use a distfile instead of fetching from Subversion, add .xmodmap setup

ARB 5.2 now builds and runs without any errors. We're done. Pleasure doing business with you.

On behalf of the many scientists, graduate students and postdocs who rely on ARB for their research - thanks much.

comment:42 Changed 13 years ago by ryandesign (Ryan Carsten Schmidt)

Resolution: fixed
Status: newclosed

Thanks for working with me on it. Glad we got it all updated to your satisfaction. And sorry it took so long.

Please do send any further updates you may have, in separate tickets. If you can keep each change small and self-contained, that should make it easier to evaluate and commit quickly.

Note: See TracTickets for help on using tickets.