BlueBrain / hpc-coding-conventions

Apache License 2.0
8 stars 5 forks source link

Refactor management of third-parties utilities. #132

Closed tristan0x closed 2 years ago

tristan0x commented 2 years ago

This patch introduces:

They are meant to replace (most of) the CMake variables that were used to configure the formatting of C++/CMake files and static analysis of C++ files.

This change also removes a lot of Python and CMake code, not needed anymore.

Mostly fix #128 see touchdetector merge request 31

tristan0x commented 2 years ago

Posting my mid-review comments for now.

This is exciting, thank you! Ping me when you're done, I'd like to give it another once over if needed.

I am done! You can merge it and start playing with it in touchdetector, but my next working day is July 4th. I will be around until June 8th to provide support but after that don't count on me ✈️

1uc commented 2 years ago

I ran the newest version for TD. It fails with:

$ ./deps/hpc-coding-conventions/bin/format -n 
Traceback (most recent call last):
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/./deps/hpc-coding-conventions/bin/format", line 18, in <module>
    sys.exit(0 if main() == 0 else 1)
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/./deps/hpc-coding-conventions/bin/format", line 14, in main
    return BBPProject.run_task(task)
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 1041, in run_task
    project = BBPProject.from_config_files()
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 1335, in from_config_files
    cls.virtualenv().ensure_requirement("PyYAML>=5")
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 948, in virtualenv
    return BBPVEnv(source_dir().joinpath(".bbp-project-venv"))
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 53, in source_dir
    return git_dir.parent
AttributeError: 'str' object has no attribute 'parent'

This is likely related not converting a string to a path when switching to pathlib.

tristan0x commented 2 years ago

0dee53e

Sorry. Fixed there 041b6e5 (tested with touchdetector this time...)

1uc commented 2 years ago

I've tried to run it in TD. I'm still hitting an issue:

$ cd deps/hpc-coding-conventions/ && git rev-parse HEAD && cd -
6348cb78035f97bb6257591f787ac7d964a77ce2

$ module list
Currently Loaded Modulefiles:
  1) unstable               3) git/2.31.1             5) python/3.9.7
  2) spack                  4) cmake/3.21.4           6) py-virtualenv/16.7.6

$ python --version
Python 3.9.7

$ which python
~/touchdetector/.bbp-project-venv/bin/python

$ ./deps/hpc-coding-conventions/bin/format -n 
Traceback (most recent call last):
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/./deps/hpc-coding-conventions/bin/format", line 18, in <module>
    sys.exit(0 if main() == 0 else 1)
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/./deps/hpc-coding-conventions/bin/format", line 14, in main
    return BBPProject.run_task(task)
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 1051, in run_task
    num_errors = project.run_task_on_codebase(task, **vars(options))
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 1093, in run_task_on_codebase
    [tool.configure() for tool in tools]
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 1093, in <listcomp>
    [tool.configure() for tool in tools]
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 650, in configure
    self._path, self._version = self.find_tool_in_path()
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 675, in find_tool_in_path
    all_paths = [(p, self.find_version(p)) for p in paths]
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 675, in <listcomp>
    all_paths = [(p, self.find_version(p)) for p in paths]
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 705, in find_version
    return pkg_resources.get_distribution(pkg_name).version
  File "/gpfs/bbp.cscs.ch/ssd/apps/bsd/2022-01-10/stage_applications/install_gcc-11.2.0-skylake/py-setuptools-57.4.0-v4z3s5/lib/python3.9/site-packages/pkg_resources/__init__.py", line 466, in get_distribution
    dist = get_provider(dist)
  File "/gpfs/bbp.cscs.ch/ssd/apps/bsd/2022-01-10/stage_applications/install_gcc-11.2.0-skylake/py-setuptools-57.4.0-v4z3s5/lib/python3.9/site-packages/pkg_resources/__init__.py", line 342, in get_provider
    return working_set.find(moduleOrReq) or require(str(moduleOrReq))[0]
  File "/gpfs/bbp.cscs.ch/ssd/apps/bsd/2022-01-10/stage_applications/install_gcc-11.2.0-skylake/py-setuptools-57.4.0-v4z3s5/lib/python3.9/site-packages/pkg_resources/__init__.py", line 886, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/gpfs/bbp.cscs.ch/ssd/apps/bsd/2022-01-10/stage_applications/install_gcc-11.2.0-skylake/py-setuptools-57.4.0-v4z3s5/lib/python3.9/site-packages/pkg_resources/__init__.py", line 772, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'clang-format' distribution was not found and is required by the application

Let me provide context. AFAICT, it starts here:

            try:
                print(f" -- requirement = {self.requirement}" )
                self._path, self._version = self.find_tool_in_path()   # LINE 650
            except FileNotFoundError as e:
                print(f"requirement = {self.requirement}" )
                if self.requirement:
                    BBPProject.virtualenv().ensure_requirement(self.requirement)
                    self._path, self._version = self.find_tool_in_path(
                        [BBPProject.virtualenv().bin_dir]
                    )
                else:
                    raise e

            # NOTE: any error that isn't a FileNotFoundError will not
            # be caught here.
            logging.info( ... )

The trace show that a pkg_resources.DistributionNotFound is thrown. This happens when calling self.find_version(p) which calls pkg_resources.get_distribution(pkg_name).version (line 705).

Since this exception is caught it doesn't trigger the whole pip install workflow.

1uc commented 2 years ago

Let me disable auto-merge.

tristan0x commented 2 years ago

I've tried to run it in TD. I'm still hitting an issue:

$ cd deps/hpc-coding-conventions/ && git rev-parse HEAD && cd -
6348cb78035f97bb6257591f787ac7d964a77ce2

$ module list
Currently Loaded Modulefiles:
  1) unstable               3) git/2.31.1             5) python/3.9.7
  2) spack                  4) cmake/3.21.4           6) py-virtualenv/16.7.6

$ python --version
Python 3.9.7

$ which python
~/touchdetector/.bbp-project-venv/bin/python

$ ./deps/hpc-coding-conventions/bin/format -n 
Traceback (most recent call last):
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/./deps/hpc-coding-conventions/bin/format", line 18, in <module>
    sys.exit(0 if main() == 0 else 1)
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/./deps/hpc-coding-conventions/bin/format", line 14, in main
    return BBPProject.run_task(task)
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 1051, in run_task
    num_errors = project.run_task_on_codebase(task, **vars(options))
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 1093, in run_task_on_codebase
    [tool.configure() for tool in tools]
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 1093, in <listcomp>
    [tool.configure() for tool in tools]
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 650, in configure
    self._path, self._version = self.find_tool_in_path()
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 675, in find_tool_in_path
    all_paths = [(p, self.find_version(p)) for p in paths]
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 675, in <listcomp>
    all_paths = [(p, self.find_version(p)) for p in paths]
  File "/gpfs/bbp.cscs.ch/home/groshein/touchdetector/deps/hpc-coding-conventions/cpp/lib.py", line 705, in find_version
    return pkg_resources.get_distribution(pkg_name).version
  File "/gpfs/bbp.cscs.ch/ssd/apps/bsd/2022-01-10/stage_applications/install_gcc-11.2.0-skylake/py-setuptools-57.4.0-v4z3s5/lib/python3.9/site-packages/pkg_resources/__init__.py", line 466, in get_distribution
    dist = get_provider(dist)
  File "/gpfs/bbp.cscs.ch/ssd/apps/bsd/2022-01-10/stage_applications/install_gcc-11.2.0-skylake/py-setuptools-57.4.0-v4z3s5/lib/python3.9/site-packages/pkg_resources/__init__.py", line 342, in get_provider
    return working_set.find(moduleOrReq) or require(str(moduleOrReq))[0]
  File "/gpfs/bbp.cscs.ch/ssd/apps/bsd/2022-01-10/stage_applications/install_gcc-11.2.0-skylake/py-setuptools-57.4.0-v4z3s5/lib/python3.9/site-packages/pkg_resources/__init__.py", line 886, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/gpfs/bbp.cscs.ch/ssd/apps/bsd/2022-01-10/stage_applications/install_gcc-11.2.0-skylake/py-setuptools-57.4.0-v4z3s5/lib/python3.9/site-packages/pkg_resources/__init__.py", line 772, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'clang-format' distribution was not found and is required by the application

Let me provide context. AFAICT, it starts here:

            try:
                print(f" -- requirement = {self.requirement}" )
                self._path, self._version = self.find_tool_in_path()   # LINE 650
            except FileNotFoundError as e:
                print(f"requirement = {self.requirement}" )
                if self.requirement:
                    BBPProject.virtualenv().ensure_requirement(self.requirement)
                    self._path, self._version = self.find_tool_in_path(
                        [BBPProject.virtualenv().bin_dir]
                    )
                else:
                    raise e

            # NOTE: any error that isn't a FileNotFoundError will not
            # be caught here.
            logging.info( ... )

The trace show that a pkg_resources.DistributionNotFound is thrown. This happens when calling self.find_version(p) which calls pkg_resources.get_distribution(pkg_name).version (line 705).

Since this exception is caught it doesn't trigger the whole pip install workflow.

Thank you for the feedback @1uc It has been fixed in 1c710992ecfed3ae1a8bce0f3cde53ef823713b4

tristan0x commented 2 years ago

ready for review @1uc and @matz-e

tristan0x commented 2 years ago

Thanks for the review and testing @1uc I have just pushed your suggestion and tested it on the TouchDetector repository.