Message ID | 20240102191222.833181-1-iillyyaa@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [meta-oe] srecord: fix install prefix | expand |
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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 --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"
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