[workspace] Deprecate the tinyobjloader external (#19880)

Use vendored tinyobjloader_internal instead (using an ODR-safe hidden namespace, instead of -fvisibility=hidden; this avoids erroneously marking C++ standard library classes with hidden visibility). Drop the double_precision patch; we can use `defines = []` now that Drake has a proper implementation_deps graph. Move patch rationale comments to where they make sense (the patch files themselves), instead of the repository file. Remove stray patch metadata.
RobotLocomotion · Aug 1, 2023 · c3fee1d · c3fee1d
1 parent e75c6cc
commit c3fee1d
Show file tree

Hide file tree

Showing 15 changed files with 86 additions and 21 deletions.
diff --git a/geometry/BUILD.bazel b/geometry/BUILD.bazel
@@ -339,7 +339,7 @@ drake_cc_library(
     deps = [
         "//common:essential",
         "@fmt",
-        "@tinyobjloader",
+        "@tinyobjloader_internal//:tinyobjloader",
     ],
 )
 

diff --git a/geometry/proximity/BUILD.bazel b/geometry/proximity/BUILD.bazel
@@ -668,7 +668,7 @@ drake_cc_library(
     deps = [
         "//common:essential",
         "@fmt",
-        "@tinyobjloader",
+        "@tinyobjloader_internal//:tinyobjloader",
     ],
 )
 

diff --git a/geometry/render/BUILD.bazel b/geometry/render/BUILD.bazel
@@ -87,7 +87,7 @@ drake_cc_library(
         "//geometry:rgba",
     ],
     deps = [
-        "@tinyobjloader",
+        "@tinyobjloader_internal//:tinyobjloader",
     ],
 )
 
@@ -103,7 +103,7 @@ drake_cc_library(
         "//geometry:rgba",
     ],
     deps = [
-        "@tinyobjloader",
+        "@tinyobjloader_internal//:tinyobjloader",
     ],
 )
 

diff --git a/geometry/render_gl/BUILD.bazel b/geometry/render_gl/BUILD.bazel
@@ -177,7 +177,7 @@ drake_cc_library_ubuntu_only(
     ],
     deps = [
         "//common:essential",
-        "@tinyobjloader",
+        "@tinyobjloader_internal//:tinyobjloader",
     ],
 )
 

diff --git a/multibody/parsing/BUILD.bazel b/multibody/parsing/BUILD.bazel
@@ -140,7 +140,7 @@ drake_cc_library(
         "//multibody/plant",
         "//multibody/tree:geometry_spatial_inertia",
         "@fmt",
-        "@tinyobjloader",
+        "@tinyobjloader_internal//:tinyobjloader",
     ],
 )
 

diff --git a/tools/workspace/BUILD.bazel b/tools/workspace/BUILD.bazel
@@ -103,7 +103,7 @@ _DRAKE_EXTERNAL_PACKAGE_INSTALLS = ["@%s//:install" % p for p in [
     "statsjs",
     "stduuid_internal",
     "suitesparse_internal",
-    "tinyobjloader",
+    "tinyobjloader_internal",
     "tinyxml2_internal",
     "usockets",
     "uwebsockets",

diff --git a/tools/workspace/default.bzl b/tools/workspace/default.bzl
@@ -88,6 +88,7 @@ load("@drake//tools/workspace/stduuid_internal:repository.bzl", "stduuid_interna
 load("@drake//tools/workspace/styleguide:repository.bzl", "styleguide_repository")  # noqa
 load("@drake//tools/workspace/suitesparse_internal:repository.bzl", "suitesparse_internal_repository")  # noqa
 load("@drake//tools/workspace/tinyobjloader:repository.bzl", "tinyobjloader_repository")  # noqa
+load("@drake//tools/workspace/tinyobjloader_internal:repository.bzl", "tinyobjloader_internal_repository")  # noqa
 load("@drake//tools/workspace/tinyxml2_internal:repository.bzl", "tinyxml2_internal_repository")  # noqa
 load("@drake//tools/workspace/tomli_internal:repository.bzl", "tomli_internal_repository")  # noqa
 load("@drake//tools/workspace/typing_extensions_internal:repository.bzl", "typing_extensions_internal_repository")  # noqa
@@ -296,7 +297,11 @@ def add_default_repositories(excludes = [], mirrors = DEFAULT_MIRRORS):
     if "suitesparse_internal" not in excludes:
         suitesparse_internal_repository(name = "suitesparse_internal", mirrors = mirrors)  # noqa
     if "tinyobjloader" not in excludes:
+        # The @tinyobjloader external is deprecated in Drake's WORKSPACE and
+        # will be removed on or after 2023-11-01.
         tinyobjloader_repository(name = "tinyobjloader", mirrors = mirrors)
+    if "tinyobjloader_internal" not in excludes:
+        tinyobjloader_internal_repository(name = "tinyobjloader_internal", mirrors = mirrors)  # noqa
     if "tinyxml2_internal" not in excludes:
         tinyxml2_internal_repository(name = "tinyxml2_internal", mirrors = mirrors)  # noqa
     if "tomli_internal" not in excludes:

diff --git a/tools/workspace/tinyobjloader/package.BUILD.bazel b/tools/workspace/tinyobjloader/package.BUILD.bazel
@@ -18,6 +18,7 @@ cc_library(
     copts = ["-fvisibility=hidden"],
     includes = ["."],
     linkstatic = 1,
+    deprecation = "DRAKE DEPRECATED: The @tinyobjloader external is deprecated in Drake's WORKSPACE and will be removed on or after 2023-11-01.",  # noqa
 )
 
 install(

diff --git a/tools/workspace/tinyobjloader/patches/double_precision.patch b/tools/workspace/tinyobjloader/patches/double_precision.patch
@@ -1,3 +1,10 @@
+[tinyobjloader] Hard-code TINYOBJLOADER_USE_DOUBLE
+
+We select tinyobjloader's floating-point typedef using a patch file
+instead of `defines = []` because tinyobjloader is a private dependency
+of Drake and we don't want the definition to leak into all target that
+consume Drake.
+
 --- tiny_obj_loader.h.orig	2017-04-24 15:34:45.000000000 -0400
 +++ tiny_obj_loader.h	2019-06-19 15:57:29.479457051 -0400
 @@ -1,3 +1,4 @@

diff --git a/tools/workspace/tinyobjloader/repository.bzl b/tools/workspace/tinyobjloader/repository.bzl
@@ -3,6 +3,9 @@ load("@drake//tools/workspace:github.bzl", "github_archive")
 def tinyobjloader_repository(
         name,
         mirrors = None):
+    """The @tinyobjloader external is deprecated in Drake's WORKSPACE and will
+    be removed on or after 2023-11-01.
+    """
     github_archive(
         name = name,
         repository = "tinyobjloader/tinyobjloader",
@@ -11,16 +14,8 @@ def tinyobjloader_repository(
         build_file = ":package.BUILD.bazel",
         mirrors = mirrors,
         patches = [
-            # We select tinyobjloader's floating-point typedef using a patch
-            # file instead of `defines = []` because tinyobjloader is a private
-            # dependency of Drake and we don't want the definition to leak into
-            # all target that consume Drake.
             ":patches/double_precision.patch",
-            # We replace tinyobjloader's implementation of float parsing with a
-            # faster call to strtod_l.
-            ":patches/faster_float_parsing.patch",
-            # If only a diffuse texture is given (map_Kd) tinyobj modulates it
-            # to 60% grey. We prefer 100%.
-            ":patches/default_texture_color.patch",
+            "//tools/workspace/tinyobjloader_internal:patches/faster_float_parsing.patch",  # noqa
+            "//tools/workspace/tinyobjloader_internal:patches/default_texture_color.patch",  # noqa
         ],
     )
diff --git a/tools/workspace/tinyobjloader_internal/BUILD.bazel b/tools/workspace/tinyobjloader_internal/BUILD.bazel
@@ -0,0 +1,3 @@
+load("//tools/lint:lint.bzl", "add_lint_tests")
+
+add_lint_tests()
diff --git a/tools/workspace/tinyobjloader_internal/package.BUILD.bazel b/tools/workspace/tinyobjloader_internal/package.BUILD.bazel
@@ -0,0 +1,27 @@
+# -*- bazel -*-
+
+load(
+    "@drake//tools/install:install.bzl",
+    "install",
+)
+
+licenses(["notice"])  # MIT, ISC
+
+package(
+    default_visibility = ["//visibility:public"],
+)
+
+cc_library(
+    name = "tinyobjloader",
+    srcs = ["tiny_obj_loader.cc"],
+    hdrs = ["tiny_obj_loader.h"],
+    defines = ["TINYOBJLOADER_USE_DOUBLE=1"],
+    copts = ["-fvisibility=hidden"],
+    includes = ["."],
+    linkstatic = 1,
+)
+
+install(
+    name = "install",
+    docs = ["LICENSE"],
+)
diff --git a/...oader/patches/default_texture_color.patch → ...ernal/patches/default_texture_color.patch b/...oader/patches/default_texture_color.patch → ...ernal/patches/default_texture_color.patch
@@ -1,5 +1,10 @@
---- tiny_obj_loader.h.orig	2017-04-24 15:34:45.000000000 -0400
-+++ tiny_obj_loader.h	2019-06-19 15:57:29.479457051 -0400
+[tinyobjloader] Fix map_Kd default
+
+If only a diffuse texture is given (map_Kd) tinyobj modulates it to 60%
+grey. We prefer 100%.
+
+--- tiny_obj_loader.h
++++ tiny_obj_loader.h
 @@ -2297,12 +2297,13 @@ void LoadMtl(std::map<std::string, int> *material_map,
        ParseTextureNameAndOption(&(material.diffuse_texname),
                                  &(material.diffuse_texopt), token);

diff --git a/...loader/patches/faster_float_parsing.patch → ...ternal/patches/faster_float_parsing.patch b/...loader/patches/faster_float_parsing.patch → ...ternal/patches/faster_float_parsing.patch
@@ -1,5 +1,10 @@
---- tiny_obj_loader.h.orig	2021-04-08 18:57:52.000000000 +0900
-+++ tiny_obj_loader.h	2021-04-27 19:33:30.732647124 -0700
+[tinyobjloader] Improve float parsing speed
+
+We replace tinyobjloader's implementation of float parsing with a faster
+call to strtod_l.
+
+--- tiny_obj_loader.h
++++ tiny_obj_loader.h
 @@ -63,6 +63,14 @@
  #include <map>
  #include <string>

diff --git a/tools/workspace/tinyobjloader_internal/repository.bzl b/tools/workspace/tinyobjloader_internal/repository.bzl
@@ -0,0 +1,17 @@
+load("@drake//tools/workspace:github.bzl", "github_archive")
+
+def tinyobjloader_internal_repository(
+        name,
+        mirrors = None):
+    github_archive(
+        name = name,
+        repository = "tinyobjloader/tinyobjloader",
+        commit = "f5569db1ffb3b0222663ba38a7a9b3f6a461c469",
+        sha256 = "f0a0f5ee4e1c7cc573fba9044c1dbed2f547b3ff058840fc8f83c6b7b79f2391",  # noqa
+        build_file = ":package.BUILD.bazel",
+        mirrors = mirrors,
+        patches = [
+            ":patches/faster_float_parsing.patch",
+            ":patches/default_texture_color.patch",
+        ],
+    )
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		load("//tools/lint:lint.bzl", "add_lint_tests")

		add_lint_tests()