Project

General

Profile

Bug #7172

Changes to ArtMake and cetbuildtools break g-2 builds

Added by Adam Lyon almost 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
Build System
Target version:
Start date:
10/16/2014
Due date:
% Done:

100%

Estimated time:
Spent time:
Occurs In:
Scope:
Internal
Experiment:
g-2
SSI Package:
cetbuildtools
Duration:

Description

Since the beginning of time, g-2 shared objects have, after "lib", the product name, followed by subdirectory names if necessary, followed by the library name, followed by .so or .dylib.

With art v1_12_02 and cetbuildtools v4_02_02 (possibly a version or two earlier), the product name is stripped out of the shared object name. For example, we used to have...

libgm2artexamples_DataObjects.dylib libgm2artexamples_Lesson1_ProduceMyLittleDatum_module.dylib
libgm2artexamples_DataObjects_dict.dylib libgm2artexamples_Lesson2_makeHits_module.dylib
libgm2artexamples_DataObjects_map.dylib libgm2artexamples_Lesson2_makeRotatedHits_module.dylib
libgm2artexamples_HitAndTrackObjects.dylib

Instead we now get,
libDataObjects.dylib libDataObjects_map.dylib libHitAndTrackObjects_dict.dylib
libDataObjects_dict.dylib libHitAndTrackObjects.dylib libHitAndTrackObjects_map.dylib

I feel strongly that the product name in the shared object name is important to reduce conflicts and improve sanity. When code in a different product needs to link against a library, it's very convenient to see what product that library is coming in from. Without the product name in the shared object name, that information is lost.

Lynn suggested a work around to manually specify all shared object names in the art_make( ... ) calls to put the product name back in. Apparently she did this to all of the LBNE code.

But I find this work around to not be feasible. Having to manually specify the shared object names will only lead to errors and will be an enormous migration. I also don't see how having to set things manually that were previously automatic is acceptable.

SO, I have simple patches (see two attached files) to three cmake macros that restore the old behavior if MRB_PROJECT is set to gm2. Kinda ugly, but I need this fixed.

The affected files are,
$ART_DIR/Modules/ArtMake.cmake
$CETBUILDTOOLS_DIR/Modules/BasicPlugin.cmake
$CETBUILDTOOLS_DIR/Modules/BuildDictionary.cmake

cetbuildtools.patch (1.31 KB) cetbuildtools.patch Adam Lyon, 10/17/2014 12:51 AM
art.patch (656 Bytes) art.patch Adam Lyon, 10/17/2014 12:51 AM

History

#1 Updated by Lynn Garren almost 5 years ago

As I explained, gm2 has been using this feature in an unintended manner. The top level checked out directory was never intended to be part of the library name. The top level directory name could be anything depending on how you check out the code. That this top level directory name was included in the library name with mrb was an oversight that we fixed. It really never affected LArSoft since they migrated from a SRT build environment and we used explicit naming to keep the old names.

If these patches implement using the product name as specified in ups/product_deps, that will work.

#2 Updated by Lynn Garren almost 5 years ago

  • Status changed from New to Assigned
  • Assignee set to Lynn Garren
  • % Done changed from 0 to 40
  • SSI Package cetbuildtools added
  • SSI Package deleted ()

We have agreed on a solution. art_make and cet_make will recognize a new directive: USE_PRODUCT_NAME. If this directive is present, the product name is prepended to the calculated library name.

In addition, it will be possible to set a cmake variable which directs art_make to pass USE_PRODUCT_NAME to the underlying macros. Since art_make also calculates library names, it will prepend the product name if this as yet unnamed variable is set. We must worry about names of libraries, plugins, and dictionaries.

At this time, changes have been committed to cetbuildtools for cet_make and some old code has been improved while we were having a look.

#3 Updated by Lynn Garren almost 5 years ago

  • % Done changed from 40 to 60

cetbuildtools v4_03_00 is now available.

  • new feature: basic_plugin, build_dictionary, and cet_make accept a USE_PRODUCT_NAME option
    • This option will prepend the product name to any calculated library name
    • This option may be useful if the recommended product structure myprod/myprod/<source code> is NOT used.

Necessary fixes to art/Modules are still in progress.

#4 Updated by Lynn Garren almost 5 years ago

  • Status changed from Assigned to Feedback
  • % Done changed from 60 to 90

Everything should now be in place.

art_make, art_make_library, art_dictionary, and simple_plugin now accept USE_PRODUCT_NAME.
In addition you may define the boolean ART_MAKE_PREPEND_PRODUCT_NAME cmake variable. If this variable is true, art_make will set USE_PRODUCT_NAME to true. You may define this variable in any CMakeLists.txt file or with, for instance, cmake -DPREPEND_PRODUCT_NAME=TRUE ...

Note that USE_PRODUCT_NAME and specifying an explicit library name are mutually exclusive. The macros will throw a fatal error if this occurs.

This is the art hotfix release v1_12_03, which is now tagged. Please test v1_12_03, which is not the same as the head of develop.

We will not build this release until you have verified that it works as expected.

#5 Updated by Adam Lyon almost 5 years ago

Finally trying this -- it doesn't work. Unfortunately, setting USE_PRODUCT_NAME adds underscores to the library name, which then runs afoul of the new underscore checker in basic_plugin. Somehow, you also have to set ALLOW_UNDERSCORE when USE_PRODUCT_NAME is set. I'm looking to see how to do this. :-(

#6 Updated by Adam Lyon almost 5 years ago

Where in basic_plugin does it handle USE_PRODUCT_NAME? I don't see it in the argument list (I see BP_USE_PRODUCT_NAME, but I don't see where that could get set).

#7 Updated by Adam Lyon almost 5 years ago

I commented out the part of basic_plugin to check for underscores so I could get past that. But there are now bigger problems. Basic_plugin is adding the product name and underscore to the front of the source file. Which then, of course, it can't find. I'm giving up on this for tonight. :-( :-(

#8 Updated by Lynn Garren almost 5 years ago

  • Status changed from Feedback to Assigned
  • % Done changed from 90 to 80

Thanks Adam. It will be easier to debug now that I'm setup to test with the g-2 code.

#9 Updated by Lynn Garren almost 5 years ago

  • Status changed from Assigned to Feedback

Please test again with cetbuildtools v4_03_01 and the latest revision of art/Modules/ArtMake.cmake. This should solve all problems.

#10 Updated by Adam Lyon almost 5 years ago

I think it works now. Thanks! I'm getting an error building a dictionary. Let's see if that has some relation to cetbuildtool. -- A

#11 Updated by Lynn Garren almost 5 years ago

install_headers also needs to add the product name to the install path. We wonder if there is anything else that needs this feature?

#12 Updated by Adam Lyon almost 5 years ago

  • Status changed from Feedback to Assigned

I've changed the ticket to assigned (from Feedback).

So if you look at $CETBUILDTOOLS_DIR/Modules/InstallSource.cmake...

Look in install_source and install_headers macros.

g-2 seems to activate the PACKAGE_TOP_DIRECTORY If. But that "then" removes the product name from the include directory. If I copy the else in its place then I get the right result. E.g. I use STRING" "\\1" CURRENT_SUBDIR "${CMAKE_CURRENT_SOURCE_DIR}" ) and I get include files in the right place.

I don't know what this will do to other people.

#13 Updated by Lynn Garren almost 5 years ago

  • Status changed from Assigned to Feedback

Please try cetbuildtools v4_03_02.

#14 Updated by Lynn Garren almost 5 years ago

  • Status changed from Feedback to Assigned

cetbuildtools v4_03_02 behaves as expected. There are no further issues. This will be part of the next art release and distribution.

#15 Updated by Lynn Garren almost 5 years ago

  • Status changed from Assigned to Resolved

#16 Updated by Christopher Green almost 5 years ago

  • Target version set to 1.12.04
  • % Done changed from 80 to 100

#17 Updated by Lynn Garren almost 5 years ago

  • Status changed from Resolved to Closed


Also available in: Atom PDF