Reindent all files following clang-format and ruff update

mentioned in merge request !418 (merged)

added 7 commits

• 5d957988...f2c36828 - 2 commits from branch master
• 0cadf9af - Remove .clang-format from git ignore list
• 4885bca9 - Add .clang-format file
• a61bb0d1 - Update .pre-commit-config.yaml
• 8bf45ad1 - Add ruff.toml
• b0f58a28 - Reindent all files following clang-format and ruff update

Compare with previous version

added 1 commit

• 3f3195f5 - Reindent all files following clang-format and ruff update

Compare with previous version

added 1 commit

• 5b3f46c5 - Reindent all files following clang-format and ruff update

Compare with previous version

@clemenci @sponce The pipeline is failing here as it is still trying to use the old configuration (clang-format 8 etc.)

Everything now works fine for me locally.

col ~/LHCb/stack/Master/Panoptes > pre-commit run --all-files
test inputs use TestFileDB...............................................Passed
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
clang-format.............................................................Passed
clang-format (non-standard extensions)...............(no files to check)Skipped
ruff (isort).............................................................Passed
ruff (PyConf)........................................(no files to check)Skipped
ruff-format..............................................................Passed

Do I need to do something here to update the pipeline to get that to use the new pre-commit settings ?

added 7 commits

• 188fc8c8 - Remove .clang-format from git ignore list
• 3a201b6e - Add .clang-format file
• 129a55eb - Update .pre-commit-config.yaml
• f91adf6d - Add ruff.toml
• 0329463e - update .gitlab-ci.yml
• ecfb095f - Sequence to reconstruct and monitor simulated data from RICH in DD4HEP
• 2a22f211 - Reindent all files following clang-format and ruff update

Compare with previous version

resolved all threads

added 5 commits

• 2a22f211...7e596c8b - 4 commits from branch master
• f18d0c06 - Reindent all files following clang-format and ruff update

Compare with previous version

/ci-test

Started integration test build. Once done, check the results or the comparison to a reference build.

@sponce It looks like you have merged something today that is causing new build warnings in LHCb. See e.g.

https://lhcb-nightlies.web.cern.ch/nightly/lhcb-master-mr/12242/LHCb/x86_64_v2-el9-gcc13-opt/build

Do you have any idea what this might be ?

The warnings started with this ci test

https://lhcb-nightlies.web.cern.ch/nightly/lhcb-master-mr/12238/

The one just before does not have them.

Comparing the checkout logs the build without warnings has

HEAD is now at e8e10fd2ab Merge branch 'use-pre-commit' into 'master'

Whereas the one with the new warnings is

HEAD is now at 38c4819487 Merge branch 'cleanup-particleid' into 'master'

So the problem is being caused by something between these two commits

I've seen it 30mn ago, and my best guess for the moment is that it's due to the reindentation. I've just managed to reproduce it locally. I'm trying to find out

Yeah, I was wondering if the ruff linting has changed when something gets imported in python, such that its now importing it when it should not in the dd4hep builds ?

My fear is that the order of imports was important and ruff does change that. But I have no clue where to look for the moment

At least I can confirm for sure that it's the commit of the reindentation now

It's happening with a couple package builds, and happens when genconfuser.py is being run to generate the configurable database for that package. That should narrow it down somewhat as to where the problem can be.

Try here

https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Det/DDDB/python/DDDB/Configuration.py?ref_type=heads#L29

The fact the warning is for the 4 imports tried there in the try/except block is I think not a coincidence.

See the change here LHCb!4884 (comment 8931367) I suspect the reordering there could be the reason ?

Try convincing ruff to retain the original order of the imports, that I suspect was important, i.e.

from DetCond.Configuration import CondDB
from Configurables import (XmlCnvSvc, XmlParserSvc,
                           EntityResolverDispatcher, GitEntityResolver)

ruff has inverted these two lines which I think is why we now see the warnings from the 4 entities in the second import.

Or perhaps @clemenci has an idea why the order matters here (as this relates to the old CondDB stuff). Ideally the order should not matter...

Anyway, as this has nothing to do with the MR which otherwise looks fine I'm going to merge...

One thing I am not sure I really get here is why this try/catch block is being used at all. Isn’t what it is doing is (indirectly) checking if the build is for DD4Hep or DetDesc ? If so would it not be better to just use the UseDD4Hep flag check instead ?

added ci-test-triggered label

• [2025-01-15 15:56] Validation started with lhcb-master-mr#12242

resolved all threads

approved this merge request

mentioned in commit fef28709

merged

mentioned in merge request LHCb!4884 (merged)