Skip to content

Commit

Permalink
Fix path check for exported include paths
Browse files Browse the repository at this point in the history
In the early days, catkin_lint was unable to do sophisticated path name
analysis, which is why some checks ignored absolute paths as a
workaround.

As the workaround is no longer required, fix the check for valid
INCLUDE_DIRS.

This commit also supersedes the proposed solutions in #111 and #112.

Fixes #110
  • Loading branch information
roehling committed Sep 20, 2024
1 parent eedd8cb commit 4351b1b
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 17 deletions.
7 changes: 5 additions & 2 deletions src/catkin_lint/checks/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ def on_catkin_package(info, cmd, args):
if ext_includes:
info.report(ERROR, "EXTERNAL_INCLUDE_PATH")
info.export_libs |= set(opts["LIBRARIES"])
info.export_includes |= set([d for d in includes if not os.path.isabs(d)])
info.export_includes |= set(includes)
info.export_packages |= set(opts["CATKIN_DEPENDS"])
info.export_targets |= set(opts["EXPORTED_TARGETS"])

Expand Down Expand Up @@ -423,8 +423,11 @@ def on_final(info):
for incl in info.export_includes - info.build_includes:
info.report(WARNING, "UNUSED_INCLUDE_PATH", path=incl, file_location=info.location_of("catkin_package"))
for incl in info.export_includes:
if not info.is_existing_path(incl, check=os.path.isdir, require_source_folder=True):
pc = info.path_class(incl)
if pc == PathClass.SOURCE and not info.is_existing_path(incl, check=os.path.isdir):
info.report(ERROR, "MISSING_INCLUDE_PATH", path=incl, file_location=info.location_of("catkin_package"))
elif pc == PathClass.BINARY:
info.report(ERROR, "BUILD_INCLUDE_PATH", path=incl, file_location=info.location_of("catkin_package"))
includes = info.build_includes | info.export_includes
for d1 in includes:
if not posixpath.isabs(d1):
Expand Down
8 changes: 8 additions & 0 deletions src/catkin_lint/diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,14 @@
You have listed an invalid include path in the INCLUDE_DIRS stanza of the
catkin_package() command.
"""),
"BUILD_INCLUDE_PATH":
("catkin_package() exports build include path",
"""\
You listed a build path below ${CMAKE_BINARY_DIR} in the INCLUDE_DIRS stanza
of your catkin_package() call. These paths are only available for develspace
builds and will be missing for installed packages.
"""
),
"EXTERNAL_INCLUDE_PATH":
("catkin_package() exports non-package include path",
"""\
Expand Down
30 changes: 16 additions & 14 deletions src/catkin_lint/linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,26 +215,28 @@ def is_existing_path(self, path, check=os.path.exists, require_source_folder=Fal
if check(os.path.normpath(os.path.join(self.path, self.subdir, tmp))):
return True
tmp = posixpath.normpath(posixpath.join(self.var["CMAKE_CURRENT_SOURCE_DIR"], path.replace(os.path.sep, "/")))
print(path, tmp, PathConstants.PACKAGE_BINARY)
if tmp.startswith(PathConstants.PACKAGE_SOURCE):
if not require_source_folder and not posixpath.isabs(path) and tmp[len(PathConstants.PACKAGE_SOURCE) + 1:] in self.generated_files:
return True
if not require_source_folder and tmp in self.generated_files:
return True
return check(os.path.join(self.path, os.path.normpath(tmp[len(PathConstants.PACKAGE_SOURCE) + 1:])))
if not require_source_folder and tmp.startswith(PathConstants.PACKAGE_BINARY):
return tmp[len(PathConstants.PACKAGE_BINARY) + 1:] in self.generated_files
if not require_source_folder and tmp in self.generated_files:
return True
if not require_source_folder and tmp.startswith(PathConstants.CATKIN_DEVEL):
s = tmp[len(PathConstants.CATKIN_DEVEL) + 1:]
for t in ["include", "lib", "share", "bin"]:
if s.startswith(t):
return True
if not require_source_folder and tmp.startswith(PathConstants.CATKIN_INSTALL):
s = tmp[len(PathConstants.CATKIN_INSTALL) + 1:]
for t in ["include", "lib", "share", "bin"]:
if s.startswith(t):
return True
if not require_source_folder:
if tmp.startswith(PathConstants.PACKAGE_BINARY):
return tmp[len(PathConstants.PACKAGE_BINARY) + 1:] in self.generated_files
if tmp.startswith(PathConstants.CATKIN_DEVEL):
s = tmp[len(PathConstants.CATKIN_DEVEL) + 1:]
for t in ["include", "lib", "share", "bin"]:
if s.startswith(t):
return True
if tmp.startswith(PathConstants.CATKIN_INSTALL):
s = tmp[len(PathConstants.CATKIN_INSTALL) + 1:]
for t in ["include", "lib", "share", "bin"]:
if s.startswith(t):
return True
if tmp in self.generated_files:
return True
return tmp.startswith(PathConstants.DISCOVERED_PATH) and discovered_path_ok

def is_internal_path(self, path):
Expand Down
2 changes: 1 addition & 1 deletion test/test_checks_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ def test_exports(self):
)
""",
checks=cc.exports)
self.assertEqual([], result)
self.assertEqual(["BUILD_INCLUDE_PATH"], result)

result = mock_lint(env, pkg,
"""
Expand Down

0 comments on commit 4351b1b

Please sign in to comment.