diff mbox series

[meta-oe] srecord: fix install prefix

Message ID 20240102191222.833181-1-iillyyaa@gmail.com
State Superseded
Headers show
Series [meta-oe] srecord: fix install prefix | expand

Commit Message

Ilya A. Kriveshko Jan. 2, 2024, 7:12 p.m. UTC
srecord's CMakeLists.txt was unconditionally setting CMAKE_INSTALL_PREFIX
for non-WIN32 builds, which caused it to ignore OE-supplied prefix
that contained the sysroot portion of the path.  Fixed by removing the
offending line.

Signed-off-by: Ilya A. Kriveshko <iillyyaa@gmail.com>
---
 ...rnally-supplied-CMAKE_INSTALL_PREFIX.patch | 27 +++++++++++++++++++
 .../recipes-support/srecord/srecord_1.65.0.bb |  3 ++-
 2 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 meta-oe/recipes-support/srecord/files/0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch

Comments

Khem Raj Jan. 2, 2024, 7:29 p.m. UTC | #1
On Tue, Jan 2, 2024 at 11:12 AM Ilya A. Kriveshko <iillyyaa@gmail.com> wrote:
>
> srecord's CMakeLists.txt was unconditionally setting CMAKE_INSTALL_PREFIX
> for non-WIN32 builds, which caused it to ignore OE-supplied prefix
> that contained the sysroot portion of the path.  Fixed by removing the
> offending line.
>
> Signed-off-by: Ilya A. Kriveshko <iillyyaa@gmail.com>
> ---
>  ...rnally-supplied-CMAKE_INSTALL_PREFIX.patch | 27 +++++++++++++++++++
>  .../recipes-support/srecord/srecord_1.65.0.bb |  3 ++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
>  create mode 100644 meta-oe/recipes-support/srecord/files/0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch
>
> diff --git a/meta-oe/recipes-support/srecord/files/0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch b/meta-oe/recipes-support/srecord/files/0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch
> new file mode 100644
> index 000000000..19b5c88f6
> --- /dev/null
> +++ b/meta-oe/recipes-support/srecord/files/0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch
> @@ -0,0 +1,27 @@
> +From 023b3de19a9c04858f34143d5d9b32bd0287f6d7 Mon Sep 17 00:00:00 2001
> +From: "Ilya A. Kriveshko" <iillyyaa@gmail.com>
> +Date: Fri, 29 Dec 2023 20:48:37 +0000
> +Subject: [PATCH] Allow externally supplied CMAKE_INSTALL_PREFIX
> +
> +CMAKE_INSTALL_PREFIX variable is supposed to be left as a default,
> +or overwritten by the packager.  In particular, OE-provided prefix
> +includes the sysroots path and should be used.
> +

I think if we can change the logic to check for CMAKE_INSTALL_PREFIX
before setting it to /usr here
could work here as well as become worth upstreaming too.

> +Upstream-Status: Inappropriate [OE-specific]
> +
> +---
> + CMakeLists.txt | 1 -
> + 1 file changed, 1 deletion(-)
> +
> +diff --git a/CMakeLists.txt b/CMakeLists.txt
> +index 74b8108..f149917 100644
> +--- a/CMakeLists.txt
> ++++ b/CMakeLists.txt
> +@@ -32,7 +32,6 @@ include(GNUInstallDirs)
> + # FHS compliant paths for Linux installation
> + if(NOT WIN32 AND CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
> + #  set(CMAKE_INSTALL_PREFIX "/opt/${PROJECT_NAME}")
> +-  set(CMAKE_INSTALL_PREFIX "/usr")
> + endif()
> +
> + # Pull in the rest of the pieces
> diff --git a/meta-oe/recipes-support/srecord/srecord_1.65.0.bb b/meta-oe/recipes-support/srecord/srecord_1.65.0.bb
> index 06ce48e65..6887f629e 100644
> --- a/meta-oe/recipes-support/srecord/srecord_1.65.0.bb
> +++ b/meta-oe/recipes-support/srecord/srecord_1.65.0.bb
> @@ -4,11 +4,12 @@ LICENSE = "GPL-3.0-or-later & LGPL-3.0-or-later"
>  LIC_FILES_CHKSUM = "file://LICENSE;md5=d32239bcb673463ab874e80d47fae504"
>
>  SRC_URI = " \
>      https://sourceforge.net/projects/${BPN}/files/srecord/${@oe.utils.trim_version('${PV}', 2)}/${BP}-Source.tar.gz \
>      file://0001-Disable-doxygen.patch \
> -    file://0001-cmake-Do-not-try-to-compute-library-dependencies-dur.patch"
> +    file://0001-cmake-Do-not-try-to-compute-library-dependencies-dur.patch \
> +    file://0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch"
>  SRC_URI[sha256sum] = "81c3d07cf15ce50441f43a82cefd0ac32767c535b5291bcc41bd2311d1337644"
>  S = "${WORKDIR}/${BP}-Source"
>
>  UPSTREAM_CHECK_URI = "https://sourceforge.net/projects/srecord/files/releases"
>
> --
> 2.39.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#108015): https://lists.openembedded.org/g/openembedded-devel/message/108015
> Mute This Topic: https://lists.openembedded.org/mt/103487925/1997914
> Group Owner: openembedded-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-devel/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Ilya A. Kriveshko Jan. 2, 2024, 8:31 p.m. UTC | #2
On Tue, Jan 2, 2024 at 2:29 PM Khem Raj <raj.khem@gmail.com> wrote:

> I think if we can change the logic to check for CMAKE_INSTALL_PREFIX
> before setting it to /usr here
> could work here as well as become worth upstreaming too.
>

OK, thanks for the suggestion.  I previously struggled to write that check,
since cmake defines CMAKE_INSTALL_PREFIX by default.
However, with your prodding, I found:
https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT.html

I will re-create the patch with this, and make an upstream submission.
diff mbox series

Patch

diff --git a/meta-oe/recipes-support/srecord/files/0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch b/meta-oe/recipes-support/srecord/files/0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch
new file mode 100644
index 000000000..19b5c88f6
--- /dev/null
+++ b/meta-oe/recipes-support/srecord/files/0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch
@@ -0,0 +1,27 @@ 
+From 023b3de19a9c04858f34143d5d9b32bd0287f6d7 Mon Sep 17 00:00:00 2001
+From: "Ilya A. Kriveshko" <iillyyaa@gmail.com>
+Date: Fri, 29 Dec 2023 20:48:37 +0000
+Subject: [PATCH] Allow externally supplied CMAKE_INSTALL_PREFIX
+
+CMAKE_INSTALL_PREFIX variable is supposed to be left as a default,
+or overwritten by the packager.  In particular, OE-provided prefix
+includes the sysroots path and should be used.
+
+Upstream-Status: Inappropriate [OE-specific]
+
+---
+ CMakeLists.txt | 1 -
+ 1 file changed, 1 deletion(-)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 74b8108..f149917 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -32,7 +32,6 @@ include(GNUInstallDirs)
+ # FHS compliant paths for Linux installation
+ if(NOT WIN32 AND CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+ #  set(CMAKE_INSTALL_PREFIX "/opt/${PROJECT_NAME}")
+-  set(CMAKE_INSTALL_PREFIX "/usr")
+ endif()
+ 
+ # Pull in the rest of the pieces
diff --git a/meta-oe/recipes-support/srecord/srecord_1.65.0.bb b/meta-oe/recipes-support/srecord/srecord_1.65.0.bb
index 06ce48e65..6887f629e 100644
--- a/meta-oe/recipes-support/srecord/srecord_1.65.0.bb
+++ b/meta-oe/recipes-support/srecord/srecord_1.65.0.bb
@@ -4,11 +4,12 @@  LICENSE = "GPL-3.0-or-later & LGPL-3.0-or-later"
 LIC_FILES_CHKSUM = "file://LICENSE;md5=d32239bcb673463ab874e80d47fae504"
 
 SRC_URI = " \
     https://sourceforge.net/projects/${BPN}/files/srecord/${@oe.utils.trim_version('${PV}', 2)}/${BP}-Source.tar.gz \
     file://0001-Disable-doxygen.patch \
-    file://0001-cmake-Do-not-try-to-compute-library-dependencies-dur.patch"
+    file://0001-cmake-Do-not-try-to-compute-library-dependencies-dur.patch \
+    file://0001-allow-externally-supplied-CMAKE_INSTALL_PREFIX.patch"
 SRC_URI[sha256sum] = "81c3d07cf15ce50441f43a82cefd0ac32767c535b5291bcc41bd2311d1337644"
 S = "${WORKDIR}/${BP}-Source"
 
 UPSTREAM_CHECK_URI = "https://sourceforge.net/projects/srecord/files/releases"