diff --git a/.github/workflows/eicshell.yml b/.github/workflows/eicshell.yml index 15247740b..b9ba956cd 100644 --- a/.github/workflows/eicshell.yml +++ b/.github/workflows/eicshell.yml @@ -24,6 +24,7 @@ jobs: cd $GITHUB_WORKSPACE/build cmake .. \ -DCMAKE_INSTALL_PREFIX=$GITHUB_WORKSPACE \ + -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_CXX_STANDARD=20 \ -DUSE_PYTHON=ON \ -DUSE_ROOT=ON \ @@ -40,23 +41,26 @@ jobs: cd $GITHUB_WORKSPACE/build make -j4 install - - name: Examine dynamic linking - run: | - export JANA_HOME=$GITHUB_WORKSPACE - export JANA_PLUGIN_PATH=$JANA_HOME/plugins - export LD_LIBRARY_PATH=$JANA_HOME/lib:$JANA_HOME/lib/JANA/plugins:$LD_LIBRARY_PATH - echo "ldd bin/jana" - ldd bin/jana - echo "ldd bin/libJANA.so" - ldd lib/libJANA.so - echo "bin/jana rpath" - objdump -x bin/jana | grep PATH - echo "lib/libJANA.so rpath" - objdump -x lib/libJANA.so | grep PATH - echo "lib/JANA/plugins/TimesliceExample.so" - objdump -x lib/JANA/plugins/TimesliceExample.so | grep PATH - echo "bin/PodioExample" - objdump -x bin/PodioExample | grep PATH + - name: Examine JANA2 dynamic linking + uses: eic/run-cvmfs-osg-eic-shell@main + with: + platform-release: "jug_xl:nightly" + run: | + export JANA_HOME=$GITHUB_WORKSPACE + export JANA_PLUGIN_PATH=$JANA_HOME/plugins + export LD_LIBRARY_PATH=$JANA_HOME/lib:$JANA_HOME/lib/JANA/plugins:$LD_LIBRARY_PATH + echo "ldd bin/jana" + ldd bin/jana + echo "ldd bin/libJANA.so" + ldd lib/libJANA.so + echo "bin/jana rpath" + objdump -x bin/jana | grep PATH + echo "lib/libJANA.so rpath" + objdump -x lib/libJANA.so | grep PATH + echo "lib/JANA/plugins/TimesliceExample.so" + objdump -x lib/JANA/plugins/TimesliceExample.so | grep PATH + echo "bin/PodioExample" + objdump -x bin/PodioExample | grep PATH - name: Run JTest plugin with 100 events uses: eic/run-cvmfs-osg-eic-shell@main @@ -115,8 +119,24 @@ jobs: cd .. git clone https://github.com/eic/EICrecon cd EICrecon - cmake -S . -B build -DJANA_DIR=$JANA_HOME/lib/cmake/JANA + cmake -S . -B build -DJANA_DIR=$JANA_HOME/lib/JANA/cmake -DCMAKE_BUILD_TYPE=Debug cmake --build build --target install -- -j8 + + - name: Examine EICrecon dynamic linking + uses: eic/run-cvmfs-osg-eic-shell@main + with: + platform-release: "jug_xl:nightly" + run: | + export JANA_HOME=$GITHUB_WORKSPACE + export JANA_PLUGIN_PATH=$JANA_HOME/plugins + export LD_LIBRARY_PATH=${GITHUB_WORKSPACE}/../EICrecon/lib:${GITHUB_WORKSPACE}/../EICrecon/lib/EICrecon/plugins:${JANA_HOME}/lib:${JANA_HOME}/lib/JANA/plugins:$LD_LIBRARY_PATH + cd ../EICrecon + echo "ldd bin/eicrecon" + ldd bin/eicrecon + echo "ldd lib/libreco.so" + ldd lib/libreco.so + echo "ldd lib/EICrecon/plugins/tracking.so" + ldd lib/EICrecon/plugins/tracking.so - name: Generate EICrecon input data uses: eic/run-cvmfs-osg-eic-shell@main @@ -136,9 +156,8 @@ jobs: platform-release: "jug_xl:nightly" setup: "/opt/detector/epic-main/bin/thisepic.sh" run: | - echo "--- Running EICrecon ---" export JANA_HOME=$GITHUB_WORKSPACE export JANA_PLUGIN_PATH=$GITHUB_WORKSPACE/../EICrecon/lib/EICrecon/plugins - export LD_LIBRARY_PATH=$GITHUB_WORKSPACE/../EICrecon/lib:$JANA_HOME/lib:$JANA_HOME/lib/JANA/plugins:$LD_LIBRARY_PATH + export LD_LIBRARY_PATH=${GITHUB_WORKSPACE}/../EICrecon/lib:${GITHUB_WORKSPACE}/../EICrecon/lib/EICrecon/plugins:${JANA_HOME}/lib:${JANA_HOME}/lib/JANA/plugins:$LD_LIBRARY_PATH ../EICrecon/bin/eicrecon sim_e_1GeV_20GeV_craterlake.edm4hep.root diff --git a/src/libraries/JANA/JEventProcessor.h b/src/libraries/JANA/JEventProcessor.h index dcf3c989e..bebe7d039 100644 --- a/src/libraries/JANA/JEventProcessor.h +++ b/src/libraries/JANA/JEventProcessor.h @@ -112,9 +112,9 @@ class JEventProcessor : public jana::omni::JComponent, } - void Summarize(JComponentSummary& summary) { + void Summarize(JComponentSummary& summary) const override { auto* result = new JComponentSummary::Component( - JComponentSummary::ComponentType::Processor, GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); + "Processor", GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); for (const auto* input : m_inputs) { size_t subinput_count = input->names.size(); diff --git a/src/libraries/JANA/JEventSource.h b/src/libraries/JANA/JEventSource.h index f4b757d05..5e1161cbe 100644 --- a/src/libraries/JANA/JEventSource.h +++ b/src/libraries/JANA/JEventSource.h @@ -373,10 +373,10 @@ class JEventSource : public jana::omni::JComponent, public jana::omni::JHasOutpu } } - void Summarize(JComponentSummary& summary) { + void Summarize(JComponentSummary& summary) const override { auto* result = new JComponentSummary::Component( - JComponentSummary::ComponentType::Source, GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); + "Source", GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); for (const auto* output : m_outputs) { size_t suboutput_count = output->collection_names.size(); diff --git a/src/libraries/JANA/JEventUnfolder.h b/src/libraries/JANA/JEventUnfolder.h index 5503a1c10..6c456373e 100644 --- a/src/libraries/JANA/JEventUnfolder.h +++ b/src/libraries/JANA/JEventUnfolder.h @@ -169,9 +169,9 @@ class JEventUnfolder : public jana::omni::JComponent, } } - void Summarize(JComponentSummary& summary) override { + void Summarize(JComponentSummary& summary) const override { auto* us = new JComponentSummary::Component( - JComponentSummary::ComponentType::Unfolder, GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); + "Unfolder", GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); for (const auto* input : m_inputs) { size_t subinput_count = input->names.size(); diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 1abd2764c..998c35b31 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -47,10 +47,10 @@ void JFactory::DoInit() { } } -void JFactory::Summarize(JComponentSummary& summary) { +void JFactory::Summarize(JComponentSummary& summary) const { auto fs = new JComponentSummary::Component( - JComponentSummary::ComponentType::Factory, + "Factory", GetPrefix(), GetTypeName(), GetLevel(), diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index bccf74678..a896331bf 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -174,7 +174,7 @@ class JFactory : public jana::omni::JComponent { /// type of object contained. In order to access these objects when all you have is a JFactory*, use JFactory::GetAs(). virtual void Create(const std::shared_ptr& event); void DoInit(); - void Summarize(JComponentSummary& summary); + void Summarize(JComponentSummary& summary) const override; virtual void Set(const std::vector &data) = 0; diff --git a/src/libraries/JANA/JFactoryGenerator.h b/src/libraries/JANA/JFactoryGenerator.h index c6e2d5f78..8ac3f0ab6 100644 --- a/src/libraries/JANA/JFactoryGenerator.h +++ b/src/libraries/JANA/JFactoryGenerator.h @@ -57,7 +57,7 @@ class JFactoryGeneratorT : public JFactoryGenerator { // Otherwise, use whatever tag the factory may have set for itself. factory->SetTag(m_tag); } - factory->SetFactoryName(JTypeInfo::demangle()); + factory->SetTypeName(JTypeInfo::demangle()); factory->SetPluginName(GetPluginName()); factory->SetApplication(GetApplication()); factory->SetLogger(GetApplication()->template GetService()->get_logger(factory->GetPrefix())); diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index 5d801b18d..3b29258dc 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -5,7 +5,6 @@ #include #include -#include "JApplication.h" #include "JFactorySet.h" #include "JFactory.h" #include "JMultifactory.h" @@ -22,32 +21,11 @@ JFactorySet::JFactorySet(void) //--------------------------------- // JFactorySet (Constructor) //--------------------------------- -JFactorySet::JFactorySet(const std::vector& aFactoryGenerators) +JFactorySet::JFactorySet(const std::vector& generators) { - //Add all factories from all factory generators - for(auto sGenerator : aFactoryGenerators){ - - // Generate the factories into a temporary JFactorySet. - JFactorySet myset; - sGenerator->GenerateFactories( &myset ); - - // Merge factories from temporary JFactorySet into this one. Any that - // already exist here will leave the duplicates in the temporary set - // where they will be destroyed by its destructor as it falls out of scope. - Merge( myset ); - } -} - -JFactorySet::JFactorySet(JFactoryGenerator* source_gen, const std::vector& default_gens) { - - if (source_gen != nullptr) source_gen->GenerateFactories(this); - for (auto gen : default_gens) { - JFactorySet temp_set; - gen->GenerateFactories(&temp_set); - Merge(temp_set); // Factories which are shadowed stay in temp_set; others are removed - for (auto straggler : temp_set.GetAllFactories()) { - LOG << "Factory '" << straggler->GetFactoryName() << "' overriden, will be excluded from event." << LOG_END; - } + // Add all factories from all factory generators + for(auto generator : generators){ + generator->GenerateFactories(this); } } @@ -64,8 +42,6 @@ JFactorySet::~JFactorySet() } // Now that the factories are deleted, nothing can call the multifactories so it is safe to delete them as well for (auto* mf : mMultifactories) { delete mf; } - - } //--------------------------------- @@ -88,8 +64,20 @@ bool JFactorySet::Add(JFactory* aFactory) if (typed_result != std::end(mFactories) || untyped_result != std::end(mFactoriesFromString)) { // Factory is duplicate. Since this almost certainly indicates a user error, and // the caller will not be able to do anything about it anyway, throw an exception. - throw JException("JFactorySet::Add failed because factory is duplicate"); - // return false; + // We show the user which factory is causing this problem, including both plugin names + std::string other_plugin_name; + if (typed_result != std::end(mFactories)) { + other_plugin_name = typed_result->second->GetPluginName(); + } + else { + other_plugin_name = untyped_result->second->GetPluginName(); + } + auto ex = JException("Attempted to add duplicate factories"); + ex.function_name = "JFactorySet::Add"; + ex.instance_name = aFactory->GetPrefix(); + ex.type_name = aFactory->GetTypeName(); + ex.plugin_name = aFactory->GetPluginName() + ", " + other_plugin_name; + throw ex; } mFactories[typed_key] = aFactory; @@ -153,51 +141,6 @@ std::vector JFactorySet::GetAllMultifactories() const { return results; } -//--------------------------------- -// Merge -//--------------------------------- -void JFactorySet::Merge(JFactorySet &aFactorySet) -{ - /// Merge any factories in the specified JFactorySet into this - /// one. Any factories which don't have the same type and tag as one - /// already in this set will be transferred and this JFactorySet - /// will take ownership of them. Ones that have a type and tag - /// that matches one already in this set will be left in the - /// original JFactorySet. Thus, all factories left in the JFactorySet - /// passed into this method upon return from it can be considered - /// duplicates. It will be left to the caller to delete those. - - JFactorySet tmpSet; // keep track of duplicates to copy back into aFactorySet - for( auto pair : aFactorySet.mFactories ){ - auto factory = pair.second; - - auto typed_key = std::make_pair(factory->GetObjectType(), factory->GetTag()); - auto untyped_key = std::make_pair(factory->GetObjectName(), factory->GetTag()); - - auto typed_result = mFactories.find(typed_key); - auto untyped_result = mFactoriesFromString.find(untyped_key); - - if (typed_result != std::end(mFactories) || untyped_result != std::end(mFactoriesFromString)) { - // Factory is duplicate. Return to caller just in case - tmpSet.mFactories[pair.first] = factory; - } - else { - mFactories[typed_key] = factory; - mFactoriesFromString[untyped_key] = factory; - } - } - - // Copy duplicates back to aFactorySet - aFactorySet.mFactories.swap( tmpSet.mFactories ); - tmpSet.mFactories.clear(); // prevent ~JFactorySet from deleting any factories - - // Move ownership of multifactory pointers over. - for (auto* mf : aFactorySet.mMultifactories) { - mMultifactories.push_back(mf); - } - aFactorySet.mMultifactories.clear(); -} - //--------------------------------- // Print //--------------------------------- diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index deade7168..5a67939fb 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -21,14 +21,12 @@ class JMultifactory; class JFactorySet : public JResettable { public: - JFactorySet(void); + JFactorySet(); JFactorySet(const std::vector& aFactoryGenerators); - JFactorySet(JFactoryGenerator* source_gen, const std::vector& default_gens); virtual ~JFactorySet(); bool Add(JFactory* aFactory); bool Add(JMultifactory* multifactory); - void Merge(JFactorySet &aFactorySet); void Print(void) const; void Release(void); diff --git a/src/libraries/JANA/JMultifactory.cc b/src/libraries/JANA/JMultifactory.cc index a36f4c66d..e6da04fc1 100644 --- a/src/libraries/JANA/JMultifactory.cc +++ b/src/libraries/JANA/JMultifactory.cc @@ -74,15 +74,15 @@ void JMultifactory::DoInit() { } } -void JMultifactory::Summarize(JComponentSummary& summary) { +void JMultifactory::Summarize(JComponentSummary& summary) const { auto mfs = new JComponentSummary::Component( - JComponentSummary::ComponentType::Factory, + "Multifactory", GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); - for (auto* helper : GetHelpers()->GetAllFactories()) { + for (auto* helper : mHelpers.GetAllFactories()) { auto coll = new JComponentSummary::Collection( helper->GetTag(), helper->GetFactoryName(), diff --git a/src/libraries/JANA/JMultifactory.h b/src/libraries/JANA/JMultifactory.h index 6ca466506..d5b7e27d6 100644 --- a/src/libraries/JANA/JMultifactory.h +++ b/src/libraries/JANA/JMultifactory.h @@ -4,6 +4,7 @@ #pragma once +#include "JANA/Utils/JTypeInfo.h" #include #include #include @@ -33,6 +34,9 @@ class JMultifactoryHelper : public JFactoryT{ // Alternatively, we could move all the JMultiFactoryHelper functionality into JFactoryT directly JMultifactory* GetMultifactory() { return mMultiFactory; } + + // Helpers do not produce any summary information + void Summarize(JComponentSummary&) const override { } }; @@ -45,6 +49,7 @@ class JMultifactoryHelperPodio : public JFactoryPodioT{ public: JMultifactoryHelperPodio(JMultifactory* parent) : mMultiFactory(parent) {} + virtual ~JMultifactoryHelperPodio() = default; // This does NOT own mMultiFactory; the enclosing JFactorySet does @@ -54,6 +59,9 @@ class JMultifactoryHelperPodio : public JFactoryPodioT{ // Alternatively, we could move all of the JMultiFactoryHelper functionality into JFactoryT directly JMultifactory* GetMultifactory() { return mMultiFactory; } + + // Helpers do not produce any summary information + void Summarize(JComponentSummary&) const override { } }; #endif // JANA2_HAVE_PODIO @@ -67,10 +75,6 @@ class JMultifactory : public jana::omni::JComponent, // However, don't worry about a Status variable. Every time Execute() gets called, so does Process(). // The JMultifactoryHelpers will control calls to Execute(). - std::string mTag; // JMultifactories each get their own name - // This can be used for parameter and collection name prefixing, though at a higher level - std::string mFactoryName; // So we can propagate this to the JMultifactoryHelpers, so we can have useful error messages - #if JANA2_HAVE_PODIO bool mNeedPodio = false; // Whether we need to retrieve the podio::Frame podio::Frame* mPodioFrame = nullptr; // To provide the podio::Frame to SetPodioData, SetCollection @@ -128,10 +132,13 @@ class JMultifactory : public jana::omni::JComponent, // in place. This method is only supposed to be called by JFactorySet::Add(JMultifactory). // These are set by JFactoryGeneratorT (just like JFactories) and get propagated to each of the JMultifactoryHelpers - void SetTag(std::string tag) { mTag = std::move(tag); } - void SetFactoryName(std::string factoryName) { mFactoryName = std::move(factoryName); } + void SetTag(std::string tag) { SetPrefix(tag); } + + void SetFactoryName(std::string factoryName) { + SetTypeName(factoryName); + } - void Summarize(JComponentSummary& summary) override; + void Summarize(JComponentSummary& summary) const override; }; @@ -141,7 +148,7 @@ void JMultifactory::DeclareOutput(std::string tag, bool owns_data) { JFactory* helper = new JMultifactoryHelper(this); if (!owns_data) helper->SetFactoryFlag(JFactory::JFactory_Flags_t::NOT_OBJECT_OWNER); helper->SetPluginName(m_plugin_name); - helper->SetFactoryName(mFactoryName); + helper->SetFactoryName(GetTypeName()+"::Helper<" + JTypeInfo::demangle() + ">"); helper->SetTag(std::move(tag)); helper->SetLevel(GetLevel()); mHelpers.SetLevel(GetLevel()); @@ -180,7 +187,7 @@ void JMultifactory::DeclarePodioOutput(std::string tag, bool owns_data) { helper->SetTag(std::move(tag)); helper->SetPluginName(m_plugin_name); - helper->SetFactoryName(mFactoryName); + helper->SetFactoryName(GetTypeName() + "::Helper<" + JTypeInfo::demangle() + ">"); helper->SetLevel(GetLevel()); mHelpers.SetLevel(GetLevel()); mHelpers.Add(helper); diff --git a/src/libraries/JANA/Omni/JComponentFwd.h b/src/libraries/JANA/Omni/JComponentFwd.h index f4a23b7eb..be8da4871 100644 --- a/src/libraries/JANA/Omni/JComponentFwd.h +++ b/src/libraries/JANA/Omni/JComponentFwd.h @@ -69,9 +69,9 @@ struct JComponent { // --------------------- // Meant to be called by JANA // --------------------- - std::string GetPrefix() { return m_prefix.empty() ? m_type_name : m_prefix; } + std::string GetPrefix() const { return m_prefix.empty() ? m_type_name : m_prefix; } - JEventLevel GetLevel() { return m_level; } + JEventLevel GetLevel() const { return m_level; } std::string GetLoggerName() const { return m_prefix.empty() ? m_type_name : m_prefix; } @@ -81,7 +81,7 @@ struct JComponent { std::string GetTypeName() const { return m_type_name; } - virtual void Summarize(JComponentSummary&) {}; + virtual void Summarize(JComponentSummary&) const {}; Status GetStatus() const { std::lock_guard lock(m_mutex); diff --git a/src/libraries/JANA/Omni/JOmniFactory.h b/src/libraries/JANA/Omni/JOmniFactory.h index a705ce460..ab94ca0b7 100644 --- a/src/libraries/JANA/Omni/JOmniFactory.h +++ b/src/libraries/JANA/Omni/JOmniFactory.h @@ -202,7 +202,6 @@ class JOmniFactory : public JMultifactory, public jana::omni::JHasInputs { std::vector input_collection_levels, std::vector output_collection_names ) { - // TODO: NWB: JMultiFactory::GetTag,SetTag are not currently usable m_prefix = (this->GetPluginName().empty()) ? tag : this->GetPluginName() + ":" + tag; m_level = level; @@ -326,10 +325,10 @@ class JOmniFactory : public JMultifactory, public jana::omni::JHasInputs { /// Generate summary for UI, inspector - void Summarize(JComponentSummary& summary) override { + void Summarize(JComponentSummary& summary) const override { auto* mfs = new JComponentSummary::Component( - JComponentSummary::ComponentType::Factory, GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); + "OmniFactory", GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); for (const auto* input : m_inputs) { size_t subinput_count = input->names.size(); diff --git a/src/libraries/JANA/Omni/JOmniFactoryGeneratorT.h b/src/libraries/JANA/Omni/JOmniFactoryGeneratorT.h index 44e3e5bba..a8320fd3f 100644 --- a/src/libraries/JANA/Omni/JOmniFactoryGeneratorT.h +++ b/src/libraries/JANA/Omni/JOmniFactoryGeneratorT.h @@ -99,14 +99,10 @@ class JOmniFactoryGeneratorT : public JFactoryGenerator { for (const auto& wiring : m_typed_wirings) { - FactoryT *factory = new FactoryT; factory->SetApplication(GetApplication()); factory->SetPluginName(this->GetPluginName()); - factory->SetFactoryName(JTypeInfo::demangle()); - // factory->SetTag(wiring.m_tag); - // We do NOT want to do this because JMF will use the tag to suffix the collection names - // TODO: NWB: Change this in JANA + factory->SetTypeName(JTypeInfo::demangle()); factory->config() = wiring.configs; // Set up all of the wiring prereqs so that Init() can do its thing diff --git a/src/libraries/JANA/Services/JParameterManager.cc b/src/libraries/JANA/Services/JParameterManager.cc index 8323b8b9b..f61759693 100644 --- a/src/libraries/JANA/Services/JParameterManager.cc +++ b/src/libraries/JANA/Services/JParameterManager.cc @@ -189,12 +189,12 @@ void JParameterManager::PrintParameters(int verbosity, int strictness) { // Print table JTablePrinter table; - table.AddColumn("Name"); + table.AddColumn("Name", JTablePrinter::Justify::Left, 20); if (warnings_present) { table.AddColumn("Warnings"); // IsDeprecated column } - table.AddColumn("Value", JTablePrinter::Justify::Left, 20); - table.AddColumn("Default", JTablePrinter::Justify::Left, 20); + table.AddColumn("Value", JTablePrinter::Justify::Left, 25); + table.AddColumn("Default", JTablePrinter::Justify::Left, 25); table.AddColumn("Description", JTablePrinter::Justify::Left, 50); for (JParameter* p: params_to_print) { diff --git a/src/libraries/JANA/Services/JPluginLoader.cc b/src/libraries/JANA/Services/JPluginLoader.cc index e9a5545b6..5ab20ae2c 100644 --- a/src/libraries/JANA/Services/JPluginLoader.cc +++ b/src/libraries/JANA/Services/JPluginLoader.cc @@ -14,14 +14,13 @@ #include #include #include -#include #include class JApplication; void JPluginLoader::Init() { - m_params->SetDefaultParameter("plugins", m_plugins_to_include, "Comma-separated list of plugins to load."); + m_params->SetDefaultParameter("plugins", m_plugins_from_parameter, "Comma-separated list of plugins to load."); m_params->SetDefaultParameter("plugins_to_ignore", m_plugins_to_exclude, "Comma-separated list of plugins to NOT load, even if they are specified in 'plugins'."); m_params->SetDefaultParameter("jana:plugin_path", m_plugin_paths_str, "Colon-separated list of paths to search for plugins"); m_params->SetDefaultParameter("jana:debug_plugin_loading", m_verbose, "Trace the plugin search path and display any loading errors"); @@ -36,27 +35,19 @@ void JPluginLoader::Init() { void JPluginLoader::add_plugin(std::string plugin_name) { - /// Add the specified plugin to the list of plugins to be - /// attached. This only records the name. The plugin is not - /// actually attached until AttachPlugins() is called (typically - /// from Initialize() which is called from Run()). - /// This will check if the plugin already exists in the list - /// of plugins to attach and will not add it a second time - /// if it is already there. This may be important if the order - /// of plugins is important. It is left to the user to handle - /// in those cases. + /// Add the specified plugin to the list of plugins to be attached. This only records + /// the plugin name. The plugin is not actually attached until AttachPlugins() is called during + /// JApplication::Initialize(). Note that any plugins specified by the 'plugins' parameter + /// are loaded _after_ any plugins specified via 'add_plugin'. This lets the maintainers + /// of a project specify a minimum set of plugins that always get loaded, and enforce that + /// their loading order. This also prevents users from clobbering the minimum plugin set, unless + /// they explicitly intend to, in which case they should use the 'plugins_to_ignore' parameter. /// - /// @param plugin_name name of the plugin. Do not include the - /// ".so" or ".dylib" suffix in the name. - /// The path to the plugin will be searched - /// from the JANA_PLUGIN_PATH envar. + /// @param plugin_name name of the plugin. Do not include the ".so" or ".dylib" suffix in the name. + /// The path to the plugin will be searched first from the 'jana:plugin_path' parameter, + /// followed by the JANA_PLUGIN_PATH envar, followed by the JANA install prefix. /// - for (std::string& n : m_plugins_to_include) { - if (n == plugin_name) { - return; - } - } - m_plugins_to_include.push_back(plugin_name); + m_all_plugins_requested.push(plugin_name); } void JPluginLoader::resolve_plugin_paths() { @@ -116,47 +107,42 @@ void JPluginLoader::attach_plugins(JComponentManager* jcm) { // Loop over all requested plugins as per the `plugins` parameter. Note that this vector may grow as // plugins themselves request additional plugins. These get appended to the back of `m_plugins_to_include`. - for (int i=0; i Found" << std::endl; - path = user_plugin; - } - else { - // User entered an invalid path; `path` variable stays enpty - paths_checked << " " << user_plugin << " => Not found" << std::endl; - } - } - else { - path = find_first_valid_path(name, paths_checked); - // User didn't provide a path, so we have to search - // If no valid paths found, `path` variable stays empty + if (m_plugin_index.find(name) != m_plugin_index.end()) { + // Make sure each plugin is only loaded once (whether provided as a path or not) + LOG_DEBUG(m_logger) << "Ignoring already-loaded plugin `" << name << "`" << LOG_END; + continue; } - if (path.empty()) { + std::string path = find_first_valid_path(name, paths_checked); + // User didn't provide a path, so we have to search + // If no valid paths found, `path` variable stays empty - LOG_ERROR(m_logger) << "Couldn't find plugin '" << name << "'\n" << - " Make sure that JANA_HOME and/or JANA_PLUGIN_PATH environment variables are set correctly.\n" - << - " Paths checked:\n" << paths_checked.str() << LOG_END; + if (path.empty()) { + LOG_ERROR(m_logger) + << "Couldn't find plugin '" << name << "'\n" + << " Make sure that the plugin search path is correctly set using the 'jana:plugin_path' parameter or the JANA_PLUGIN_PATH environment variable.\n" + << " Paths checked:\n" << paths_checked.str() << LOG_END; throw JException("Couldn't find plugin '%s'", name.c_str()); } @@ -261,29 +247,12 @@ JPlugin::~JPlugin() { LOG_DEBUG(m_logger) << "Unloaded plugin \"" << m_name << "\"" << LOG_END; } - -bool JPluginLoader::is_path(const std::string user_plugin) { - return user_plugin.find('/') != -1; -} - -std::string JPluginLoader::normalize_name(const std::string user_plugin) { - if (user_plugin.substr(user_plugin.size() - 3) == ".so") { - return user_plugin.substr(0, user_plugin.size() - 3); - } - return user_plugin; -} - -std::string JPluginLoader::extract_name_from_path(const std::string user_plugin) { - // Ideally we would just do this, but we want to be C++17 compatible - // return std::filesystem::path(filesystem_path).filename().stem().string(); - - size_t pos_begin = user_plugin.find_last_of('/'); - if (pos_begin == std::string::npos) pos_begin = 0; +bool JPluginLoader::is_valid_plugin_name(const std::string& plugin_name) const { - size_t pos_end = user_plugin.find_last_of('.'); - //if (pos_end == std::string::npos) pos_end = filesystem_path.size(); - - return user_plugin.substr(pos_begin+1, pos_end-pos_begin-1); + if (plugin_name.size() > 2 && plugin_name.substr(plugin_name.size() - 3) == ".so") return false; + if (plugin_name.size() > 5 && plugin_name.substr(plugin_name.size() - 6) == ".dylib") return false; + if (plugin_name.find('/') != std::string::npos) return false; + return true; } bool ends_with(const std::string& s, char c) { @@ -292,7 +261,7 @@ bool ends_with(const std::string& s, char c) { return s.back() == c; } -std::string JPluginLoader::make_path_from_name(std::string name, const std::string& path_prefix) { +std::string JPluginLoader::make_path_from_name(const std::string& name, const std::string& path_prefix) const { std::ostringstream oss; oss << path_prefix; if (!ends_with(path_prefix, '/')) { @@ -303,12 +272,12 @@ std::string JPluginLoader::make_path_from_name(std::string name, const std::stri return oss.str(); } -std::string JPluginLoader::find_first_valid_path(const std::string& name, std::ostringstream& debug_log) { +std::string JPluginLoader::find_first_valid_path(const std::string& name, std::ostringstream& debug_log) const { for (const std::string& path_prefix : m_plugin_paths) { auto path = make_path_from_name(name, path_prefix); - if (validate_path(path)) { + if (is_valid_path(path)) { debug_log << " " << path << " => Found" << std::endl; return path; } @@ -320,7 +289,7 @@ std::string JPluginLoader::find_first_valid_path(const std::string& name, std::o } -bool JPluginLoader::validate_path(const std::string& path) { +bool JPluginLoader::is_valid_path(const std::string& path) const { return (access(path.c_str(), F_OK) != -1); } diff --git a/src/libraries/JANA/Services/JPluginLoader.h b/src/libraries/JANA/Services/JPluginLoader.h index e5689c9a1..3707292dd 100644 --- a/src/libraries/JANA/Services/JPluginLoader.h +++ b/src/libraries/JANA/Services/JPluginLoader.h @@ -12,6 +12,7 @@ #include #include #include +#include class JComponentManager; @@ -45,17 +46,16 @@ class JPluginLoader : public JService { void attach_plugin(std::string name, std::string path); void resolve_plugin_paths(); - bool is_path(const std::string user_plugin); - std::string normalize_name(const std::string user_plugin); - std::string extract_name_from_path(const std::string user_plugin); - std::string find_first_valid_path(const std::string& name, std::ostringstream& debug_log); - std::string make_path_from_name(std::string name, const std::string& path_prefix); - bool validate_path(const std::string& path); + bool is_valid_plugin_name(const std::string& plugin_name) const; + bool is_valid_path(const std::string& path) const; + std::string find_first_valid_path(const std::string& name, std::ostringstream& debug_log) const; + std::string make_path_from_name(const std::string& name, const std::string& path_prefix) const; private: Service m_params {this}; - std::vector m_plugins_to_include; + std::queue m_all_plugins_requested; + std::vector m_plugins_from_parameter; std::vector m_plugins_to_exclude; std::vector m_plugin_paths; std::string m_plugin_paths_str; diff --git a/src/libraries/JANA/Status/JComponentSummary.cc b/src/libraries/JANA/Status/JComponentSummary.cc index 6f6bfafed..2f77bdd3d 100644 --- a/src/libraries/JANA/Status/JComponentSummary.cc +++ b/src/libraries/JANA/Status/JComponentSummary.cc @@ -3,7 +3,6 @@ // Subject to the terms in the LICENSE file found in the top-level directory. #include -#include #include #include "JComponentSummary.h" @@ -114,23 +113,16 @@ void PrintComponentTable(std::ostream& os, const JComponentSummary& cs) { JTablePrinter comp_table; - comp_table.AddColumn("Base"); - comp_table.AddColumn("Type"); - comp_table.AddColumn("Prefix"); - comp_table.AddColumn("Level"); - comp_table.AddColumn("Plugin"); + comp_table.AddColumn("Base", JTablePrinter::Justify::Left, 12); + comp_table.AddColumn("Type", JTablePrinter::Justify::Left, 40); + comp_table.AddColumn("Prefix", JTablePrinter::Justify::Left, 40); + comp_table.AddColumn("Level", JTablePrinter::Justify::Left, 12); + comp_table.AddColumn("Plugin", JTablePrinter::Justify::Left, 16); os << " Component Summary" << std::endl; for (const auto* comp : cs.GetAllComponents()) { - switch(comp->GetComponentType()) { - case JComponentSummary::ComponentType::Source: comp_table | "Source"; break; - case JComponentSummary::ComponentType::Processor: comp_table | "Processor"; break; - case JComponentSummary::ComponentType::Factory: comp_table | "Factory"; break; - case JComponentSummary::ComponentType::Unfolder: comp_table | "Unfolder"; break; - case JComponentSummary::ComponentType::Folder: comp_table | "Folder"; break; - } - comp_table | comp->GetTypeName() | comp->GetPrefix() | comp->GetLevel() | comp->GetPluginName(); + comp_table | comp->GetBaseName() | comp->GetTypeName() | comp->GetPrefix() | comp->GetLevel() | comp->GetPluginName(); } os << comp_table; } diff --git a/src/libraries/JANA/Status/JComponentSummary.h b/src/libraries/JANA/Status/JComponentSummary.h index f1c3d8624..fd185b9e5 100644 --- a/src/libraries/JANA/Status/JComponentSummary.h +++ b/src/libraries/JANA/Status/JComponentSummary.h @@ -8,7 +8,6 @@ #include #include #include -#include // Keeping this around for backwards compatibility only @@ -23,14 +22,13 @@ struct JFactorySummary { class JComponentSummary { public: - enum class ComponentType { Source, Processor, Factory, Unfolder, Folder }; class Collection; class Component { friend class JComponentSummary; - ComponentType m_component_type; + std::string m_base_name; std::string m_prefix; std::string m_type_name; JEventLevel m_level; @@ -39,11 +37,11 @@ class JComponentSummary { std::vector m_outputs; public: - Component(ComponentType component_type, std::string prefix, std::string type_name, JEventLevel level, std::string plugin_name) - : m_component_type(component_type), m_prefix(prefix), m_type_name(type_name), m_level(level), m_plugin_name(plugin_name) {} + Component(std::string base_name, std::string prefix, std::string type_name, JEventLevel level, std::string plugin_name) + : m_base_name(base_name), m_prefix(prefix), m_type_name(type_name), m_level(level), m_plugin_name(plugin_name) {} void AddInput(Collection* input) { m_inputs.push_back(input); } void AddOutput(Collection* output) { m_outputs.push_back(output); } - ComponentType GetComponentType() const { return m_component_type; } + std::string GetBaseName() const { return m_base_name; } std::string GetPrefix() const { return m_prefix; } std::string GetTypeName() const { return m_type_name; } JEventLevel GetLevel() const { return m_level; } diff --git a/src/programs/unit_tests/Topology/MultiLevelTopologyTests.cc b/src/programs/unit_tests/Topology/MultiLevelTopologyTests.cc index 650c8b561..c9bf633c6 100644 --- a/src/programs/unit_tests/Topology/MultiLevelTopologyTests.cc +++ b/src/programs/unit_tests/Topology/MultiLevelTopologyTests.cc @@ -1,8 +1,11 @@ -#include +#include "MultiLevelTopologyTests.h" +#include "JANA/JException.h" + #include #include -#include "MultiLevelTopologyTests.h" +namespace jana { +namespace timeslice_tests { enum class Level { None, Timeslice, Event, Subevent }; enum class ComponentType { None, Source, Filter, Map, Split, Merge, Reduce }; @@ -442,7 +445,13 @@ TEST_CASE("TimeslicesTests") { app.Add(new JFactoryGeneratorT); app.Add(new JFactoryGeneratorT); app.SetTicker(true); - app.Run(); + try { + app.Run(); + } + catch (JException& e) { + std::cout << e << std::endl; + throw e; + } } diff --git a/src/programs/unit_tests/Topology/MultiLevelTopologyTests.h b/src/programs/unit_tests/Topology/MultiLevelTopologyTests.h index 2798c88da..851dc6ef0 100644 --- a/src/programs/unit_tests/Topology/MultiLevelTopologyTests.h +++ b/src/programs/unit_tests/Topology/MultiLevelTopologyTests.h @@ -4,6 +4,7 @@ #include #include #include +#include namespace jana { namespace timeslice_tests { @@ -94,9 +95,9 @@ struct MyEventProcessor : public JEventProcessor { void Process(const JEvent& event) override { process_called_count++; - // TODO: Trigger cluster factory - // TODO: Validate that the clusters make sense jout << "MyEventProcessor: Processing " << event.GetEventNumber() << jendl; + auto clusters = event.Get("evt"); + // TODO: Validate that the clusters make sense REQUIRE(init_called_count == 1); REQUIRE(finish_called_count == 0); } @@ -116,6 +117,7 @@ struct MyProtoClusterFactory : public JFactoryT { MyProtoClusterFactory() { SetLevel(JEventLevel::Timeslice); + SetTag("ts"); } void Init() override { @@ -142,6 +144,7 @@ struct MyClusterFactory : public JFactoryT { MyClusterFactory() { SetLevel(JEventLevel::PhysicsEvent); + SetTag("evt"); } void Init() override { @@ -159,4 +162,5 @@ struct MyClusterFactory : public JFactoryT { } }; - +} // namespace timeslice_tests +} // namespace jana