I added getPackageDir to utils on tickets/DM-2635 and included a simple unit test.

I chose to implement the function using environment variables, rather than using the productDir method that sconsUtils adds to eups. If standard eups included productDir as a method, or the function was clearly part of sconsUtils instead of having sconsUtils add it to eups, I would have chosen differently, because it makes me nervous to have two different implementations. But at least the implementation is trivial and the sconsUtils version will only be used inside of sconsUtils once DM-2636 is implemented.

After putting it up for review I realized there was a C++ wrapper as well. So I modified the code as follows:

• Rename the C++ wrapper from lsst::utils::eups::productDir to lsst::utils::getPackageDir and removed the extra version argument that was not actually usable. This function throws an exception if the package is not found.
• Made the Python code a SWIG wrapper of the C++ code. So now getPackageDir does throw an exception, which (having converted all the code in afw) I think is a marked improvement.

My apologies for the post-review-state changes. However, I think the results are significantly improved, and I've implemented all suggested changes where they still make sense.

Looks okay. How do the AFW patches relate to DM-2636? Do all the changes have to happen at once because the previous EUPS utility function has disappeared?

I changed afw on the same ticket because some C++ examples and tests were using lsst::utils::eups::packageDir, which has been replaced by lsst::utils::getPackageDir. While I was at it, I also updated the python code that was using eups.packageDir, though I could have waited until DM-2636 for that. afw is the only package I found that uses lsst::utils::eups::packageDir so I think this ticket is nicely self-contained. I will start a buildbot run shortly to see if I missed anything.

That's fine. I was a bit worried that splitting this task into 2 tickets would only work if the old interface remained until the second ticket was completed.

I have just noticed that diffimLib.py in ip_diffim (derived from diffimLib.i attempts to use guessSvnVersion in diffimlib.version. The entire version function seems anomalous. In fact, I'm suspicious of any code that is trying to deal with $HeadURL$ (pex_logging being the other package if you assume that sconsUtils has to have support for Svn).

Yup, that's a relic, and it's safe to just delete it. We now machine-generate a version.py as part of every sconsUtils-based build.