-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use "best matching" EDAlias in module dependence checks when placing ConditionalTask modules in Path #39409
Use "best matching" EDAlias in module dependence checks when placing ConditionalTask modules in Path #39409
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39409/32120 ERROR: Build errors found during clang-tidy run.
|
457dcf7
to
cf5e630
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39409/32121
|
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages:
@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
@Dr15Jones please check |
-1 Failed Tests: UnitTests Unit TestsI found errors in the following unit tests: ---> test test-das-selected-lumis had ERRORS Comparison SummarySummary:
|
The unit test failure is
and is unrelated to this PR. |
7000528
to
5b46812
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39409/32292
|
Pull request #39409 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again. |
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3d54ae/27820/summary.html Comparison SummarySummary:
|
+1 |
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) |
type bug-fix |
+1 |
PR description:
This PR fixes the problem reported in #36938 (comment). If the
EDAlias
has wildcards for one module, and more specific aliases for another module, we should use the one that has the most constrained match. There is one ambiguous case left, for which an exception is thrown. I hope we won't encounter that situation in practice.The problem was encountered with a
SwitchProducer
having theEDAlias
case, but I think the problem could happen also withoutSwitchProducer
.PR validation:
Framework unit test passed in 12_4_8 (where I developed this change originally)