Move all PCLConfigPlugins sources to plugins
#40245
Conversation
This is a follow up to #39649. In that PR, some shared code in the `plugins` directory of CondCore/PCLConfigPlugins got factored out and was put into a new header file that was placed in the `interface` directory. That resulted in some inconsistencies in the build configuration, because the `interface` and `src` directories actually didn't correspond to a normal library, but also to a plugins library because of the `<flags EDM_PLUGIN="1"/>` in the BuildFile! Hence, one plugin library included the header files from another plugin library, which should not be done. In particular, it confused my unused dependency checker, because you can't declare another plugins library as a dependency. That forced us to declare the dependencies of the `CondCore/PCLConfigPlugins` plugins library also in `CondCore/PCLConfigPlugins`, which caused false positives in the unused-dependency checks. This commit fixes the situation by moving the header file into the `plugins` directory. To avoid confusing a plugin library with a regular library in the future, the plugin library that was defined in the top-level of `CondCore/PCLConfigPlugins` got also moved to be another build target in the `plugins` subdirectory, in a way such that the compiled library name doesn't change.
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40245/33299
|
|
A new Pull Request was created by @guitargeek (Jonas Rembser) for master. It involves the following packages:
@malbouis, @cmsbuild, @saumyaphor4252, @ggovi, @francescobrivio, @tvami can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
test parameters:
|
|
Thanks @mmusich! Seems the bot hasn't started the tests yet. Maybe you got to change the parameters and ask for the tests to start in two separate comment? |
|
I am not asking the bot to test anything, that's intentional. |
|
Ah alright, thanks for clarifying! Just wanted to make sure |
|
@cmsbuild , please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a447e3/29494/summary.html Comparison SummarySummary:
|
|
+db
|
cmsbuild
added
db-approved
fully-signed
and removed
db-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
This is a follow up to #39649.
In that PR, some shared code in the
pluginsdirectory of CondCore/PCLConfigPlugins got factored out and was put into a new header file that was placed in theinterfacedirectory.That resulted in some inconsistencies in the build configuration, because the
interfaceandsrcdirectories actually didn't correspond to a normal library, but also to a plugins library because of the<flags EDM_PLUGIN="1"/>in the BuildFile!Hence, one plugin library included the header files from another plugin library, which should not be done. In particular, it confused my unused dependency checker, because you can't declare another plugins library as a dependency. That forced us to declare the dependencies of the
CondCore/PCLConfigPluginsplugins library also inCondCore/PCLConfigPlugins, which caused false positives in the unused-dependency checks.This commit fixes the situation by moving the header file into the
pluginsdirectory. To avoid confusing a plugin library with a regular library in the future, the plugin library that was defined in the top-level ofCondCore/PCLConfigPluginsgot also moved to be another build target in thepluginssubdirectory, in a way such that the compiled library name doesn't change.