Skip to content

Commit

Permalink
drop unvendoring hunk; order definition before use
Browse files Browse the repository at this point in the history
While modern protobuf already sets PROTOBUF_USE_DLLS in the
library interface, this does not seem to work as intended yet
  • Loading branch information
h-vetinari committed Feb 21, 2024
1 parent 9a21672 commit 67b630e
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 31 deletions.
2 changes: 0 additions & 2 deletions recipe/build-lib.bat
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ cmake -G "Ninja" ^
-DCMAKE_PREFIX_PATH=%LIBRARY_PREFIX%;%LIBRARY_BIN%;%LIBRARY_LIB% ^
-DCMAKE_INCLUDE_PATH=%LIBRARY_INC% ^
-DCMAKE_INSTALL_PREFIX=%LIBRARY_PREFIX% ^
-DCMAKE_INSTALL_LIBDIR="lib" ^
-DCMAKE_INSTALL_INCLUDEDIR="include" ^
-Dprotobuf_BUILD_SHARED_LIBS=OFF ^
-DSPM_ENABLE_SHARED=OFF ^
-DSPM_ABSL_PROVIDER="package" ^
Expand Down
2 changes: 0 additions & 2 deletions recipe/build-lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ fi
cmake -G "Ninja" \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_INSTALL_PREFIX=$PREFIX \
-DCMAKE_INSTALL_LIBDIR="lib" \
-DCMAKE_INSTALL_INCLUDEDIR="include" \
-DCMAKE_AR="${AR}" \
-DSPM_ENABLE_SHARED=ON \
-DSPM_ENABLE_TCMALLOC=OFF \
Expand Down
7 changes: 5 additions & 2 deletions recipe/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ source:
patches:
# trying to build both static & shared build seems to break on OSX
- patches/0001-do-not-mix-static-shared-builds.patch
# unvendor abseil & protobuf-lite
- patches/0002-do-not-build-vendored-abseil-libprotobuf-lite.patch
# set PROTOBUF_USE_DLLS
- patches/0002-ensure-we-set-PROTOBUF_USE_DLLS-when-using-our-own-p.patch
# ensure python bindings link to correct libs on windows
- patches/0003-point-to-our-libs-headers-for-windows-in-setup.py.patch
# install pkg-config metadata also on windows
Expand All @@ -20,6 +20,9 @@ source:
- patches/0005-create-and-install-CMake-metadata.patch
# fix abseil setup on windows
- patches/0006-also-link-to-static-absl_flags_-on-windows.patch
# backport of https://github.com/google/sentencepiece/pull/979:
# avoid having to specify CMAKE_INSTALL_{LIB,INCLUDE}DIR due to wrong order
- patches/0007-move-setting-of-default-CMAKE_INSTALL_-BIN-INCLUDE-L.patch

build:
number: 0
Expand Down
2 changes: 1 addition & 1 deletion recipe/patches/0001-do-not-mix-static-shared-builds.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 2fe3e37744c810590e631c01fb57133080fc5f46 Mon Sep 17 00:00:00 2001
From: "H. Vetinari" <[email protected]>
Date: Thu, 2 Dec 2021 08:39:53 +1100
Subject: [PATCH 1/6] do not mix static & shared builds
Subject: [PATCH 1/7] do not mix static & shared builds

---
src/CMakeLists.txt | 20 ++++++++++----------
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
From ab5c20be4a987d1cd6d2e472634f5e1b9c11211f Mon Sep 17 00:00:00 2001
From 185e8cd8603d188cccdb6f170a60d2984211b70c Mon Sep 17 00:00:00 2001
From: "H. Vetinari" <[email protected]>
Date: Thu, 2 Dec 2021 10:05:12 +1100
Subject: [PATCH 2/6] do not build vendored abseil & libprotobuf-lite
Subject: [PATCH 2/7] ensure we set PROTOBUF_USE_DLLS when using our own
protobuf

ensure we can use shared builds of libprotobuf also on windows
---
src/CMakeLists.txt | 5 +++++
third_party/CMakeLists.txt | 5 +----
2 files changed, 6 insertions(+), 4 deletions(-)
src/CMakeLists.txt | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index fbdf238..2b8aefa 100644
Expand All @@ -25,13 +24,3 @@ index fbdf238..2b8aefa 100644
include_directories(${Protobuf_INCLUDE_DIRS})
protobuf_generate_cpp(SPM_PROTO_SRCS SPM_PROTO_HDRS sentencepiece.proto)
protobuf_generate_cpp(SPM_MODEL_PROTO_SRCS SPM_MODEL_PROTO_HDRS sentencepiece_model.proto)
diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt
index d00ecba..3096702 100644
--- a/third_party/CMakeLists.txt
+++ b/third_party/CMakeLists.txt
@@ -1,4 +1 @@
-include_directories(absl/strings darts_clone esaxx protobuf-lite)
-
-
-
+include_directories(darts_clone esaxx)
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 428ac3758e24b1aeedec8e0568d128e2097d1646 Mon Sep 17 00:00:00 2001
From a285dbb0bb469256fb43f483e398cc0f028cd2c8 Mon Sep 17 00:00:00 2001
From: "H. Vetinari" <[email protected]>
Date: Sun, 11 Dec 2022 01:09:03 +1100
Subject: [PATCH 3/6] point to our libs / headers for windows in setup.py
Subject: [PATCH 3/7] point to our libs / headers for windows in setup.py

also do not risk building against bundled libs, nor
setting /MT for the MSVC static runtime libs
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From f6ec12fe46a666940007cb205e715ee6f3916e97 Mon Sep 17 00:00:00 2001
From d1afa1e1983080ce57443aca9afac359199f115b Mon Sep 17 00:00:00 2001
From: "H. Vetinari" <[email protected]>
Date: Mon, 12 Dec 2022 14:36:45 +1100
Subject: [PATCH 4/6] also install pkg-config files on windows
Subject: [PATCH 4/7] also install pkg-config files on windows

---
CMakeLists.txt | 4 +---
Expand Down
4 changes: 2 additions & 2 deletions recipe/patches/0005-create-and-install-CMake-metadata.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 366a1080b048a43452bdfb43968c5f2a44acdcf4 Mon Sep 17 00:00:00 2001
From c963db4cb65affe315ae197b482557715908c3da Mon Sep 17 00:00:00 2001
From: "H. Vetinari" <[email protected]>
Date: Wed, 18 Jan 2023 19:44:15 +1100
Subject: [PATCH 5/6] create and install CMake metadata
Subject: [PATCH 5/7] create and install CMake metadata

---
CMakeLists.txt | 10 ++++++++++
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 64f7608bd0757387cee221df7014c9bdbe78585b Mon Sep 17 00:00:00 2001
From 2a09741805ff7c3fad70b41e2c07b0520d9f77a9 Mon Sep 17 00:00:00 2001
From: "H. Vetinari" <[email protected]>
Date: Tue, 20 Feb 2024 17:43:23 +1100
Subject: [PATCH 6/6] also link to static absl_flags_* on windows
Subject: [PATCH 6/7] also link to static absl_flags_* on windows

---
python/setup.py | 10 +++++++++-
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
From aa7701e2fc3bc5997cc07e44c647dfe3261c9e49 Mon Sep 17 00:00:00 2001
From: "H. Vetinari" <[email protected]>
Date: Tue, 20 Feb 2024 18:43:25 +1100
Subject: [PATCH 7/7] move setting of default
CMAKE_INSTALL_{BIN,INCLUDE,LIB}DIR before first use

also unify spelling of CMAKE_INSTALL_INCLUDEDIR following GNUInstallDirs
defaults, see also CMake docs:
https://cmake.org/cmake/help/latest/command/install.html
---
CMakeLists.txt | 24 ++++++++++++------------
src/CMakeLists.txt | 4 ++--
2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 56830cf..03f1589 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -57,6 +57,18 @@ if((CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND
string(APPEND CMAKE_CXX_FLAGS " -fmacro-prefix-map=${CMAKE_SOURCE_DIR}/=''")
endif()

+if (NOT DEFINED CMAKE_INSTALL_BINDIR)
+ set(CMAKE_INSTALL_BINDIR bin)
+endif()
+
+if (NOT DEFINED CMAKE_INSTALL_LIBDIR)
+ set(CMAKE_INSTALL_LIBDIR lib)
+endif()
+
+if (NOT DEFINED CMAKE_INSTALL_INCLUDEDIR)
+ set(CMAKE_INSTALL_INCLUDEDIR include)
+endif()
+
if (UNIX)
include(GNUInstallDirs)
set(prefix ${CMAKE_INSTALL_PREFIX})
@@ -103,18 +115,6 @@ if (APPLE)
endif()
endif()

-if (NOT DEFINED CMAKE_INSTALL_BINDIR)
- set(CMAKE_INSTALL_BINDIR bin)
-endif()
-
-if (NOT DEFINED CMAKE_INSTALL_LIBDIR)
- set(CMAKE_INSTALL_LIBDIR lib)
-endif()
-
-if (NOT DEFINED CMAKE_INSTALL_INCDIR)
- set(CMAKE_INSTALL_INCDIR include)
-endif()
-
# SPDX-License-Identifier: (MIT OR CC0-1.0)
# Copyright 2020 Jan Tojnar
# https://github.com/jtojnar/cmake-snips
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index eed204f..a612357 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -331,9 +331,9 @@ install(EXPORT sentencepieceTargets
DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/sentencepiece")

install(FILES sentencepiece_trainer.h sentencepiece_processor.h
- DESTINATION ${CMAKE_INSTALL_INCDIR})
+ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
if (NOT SPM_PROTOBUF_PROVIDER STREQUAL "internal")
- install(FILES ${SPM_PROTO_HDRS} DESTINATION ${CMAKE_INSTALL_INCDIR})
+ install(FILES ${SPM_PROTO_HDRS} DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
endif()

file(TO_NATIVE_PATH "${PROJECT_SOURCE_DIR}/data" data_dir)

0 comments on commit 67b630e

Please sign in to comment.