| Message ID | 20220808063843.3975130-9-alex@linutronix.de |
|---|---|
| State | Accepted, archived |
| Commit | 0b2f8da3ff9cbbb6fc2ab75fbe09ad1fe745c53b |
| Headers | show |
| Series | [01/45] rpm: update 4.17.0 -> 4.17.1 | expand |
On Mon, 2022-08-08 at 08:38 +0200, Alexander Kanavin wrote: > Drop handle-read-only-files.patch: read only files should > be handled by making them writeable explicitly. See > the upstream discussion: > https://github.com/NixOS/patchelf/pull/89 > > Signed-off-by: Alexander Kanavin <alex@linutronix.de> > --- > .../patchelf/handle-read-only-files.patch | 65 ------------------- > ...{patchelf_0.14.5.bb => patchelf_0.15.0.bb} | 6 +- > 2 files changed, 2 insertions(+), 69 deletions(-) > delete mode 100644 meta/recipes-devtools/patchelf/patchelf/handle-read-only-files.patch > rename meta/recipes-devtools/patchelf/{patchelf_0.14.5.bb => patchelf_0.15.0.bb} (79%) patchelf is used by uninative and you have no idea the world of pain you're pushing onto others by dropping this one. It isn't as simple as you think due to the places patchelf is used. I don't remember the details, I do remember thinking along the lines of this patch and then changing my mind though. Cheers, Richard
On Mon, 8 Aug 2022 at 09:43, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Mon, 2022-08-08 at 08:38 +0200, Alexander Kanavin wrote: > > Drop handle-read-only-files.patch: read only files should > > be handled by making them writeable explicitly. See > > the upstream discussion: > > https://github.com/NixOS/patchelf/pull/89 > > > > Signed-off-by: Alexander Kanavin <alex@linutronix.de> > > --- > > .../patchelf/handle-read-only-files.patch | 65 ------------------- > > ...{patchelf_0.14.5.bb => patchelf_0.15.0.bb} | 6 +- > > 2 files changed, 2 insertions(+), 69 deletions(-) > > delete mode 100644 meta/recipes-devtools/patchelf/patchelf/handle-read-only-files.patch > > rename meta/recipes-devtools/patchelf/{patchelf_0.14.5.bb => patchelf_0.15.0.bb} (79%) > > patchelf is used by uninative and you have no idea the world of pain > you're pushing onto others by dropping this one. It isn't as simple as > you think due to the places patchelf is used. I don't remember the > details, I do remember thinking along the lines of this patch and then > changing my mind though. Usage of patchelf in uninative seems to be contained in uninative_changeinterp, which can be tweaked to handle r/o files before invoking patchelf. Was it that all usages of it need to be tweaked similarly? In upgrades like these I only see what the autobuilder and the commit history and the patch header say. I can restore and rebase the patch, but it would benefit from a better explanation. Alex
On Mon, 8 Aug 2022 at 10:23, Alexander Kanavin via lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org> wrote: > Usage of patchelf in uninative seems to be contained in > uninative_changeinterp, which can be tweaked > to handle r/o files before invoking patchelf. Was it that all usages > of it need to be tweaked similarly? Ok, I couldnt find other usages of it that would require similar tweaking (icecc uses host patchelf if available, rust.inc/cargo.inc operate on only a few static filenames), so maybe it's ok to move what the patch does into uninative_changeinterp (add +r before invocation, restore original permissions after). Still better than carrying a rejected patch. Alex
On Mon, 2022-08-08 at 11:35 +0200, Alexander Kanavin wrote: > On Mon, 8 Aug 2022 at 10:23, Alexander Kanavin via > lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org> > wrote: > > Usage of patchelf in uninative seems to be contained in > > uninative_changeinterp, which can be tweaked > > to handle r/o files before invoking patchelf. Was it that all usages > > of it need to be tweaked similarly? > > Ok, I couldnt find other usages of it that would require similar > tweaking (icecc uses host patchelf if available, rust.inc/cargo.inc > operate on only a few static filenames), so maybe it's ok to move what > the patch does into uninative_changeinterp (add +r before invocation, > restore original permissions after). Still better than carrying a > rejected patch. It feels like there is something we're missing. I wish I could remember what :(. Note that the testing cycle for changing patchelf is a pain as we need to merge, create a uninative release and upgrade to before we have the full cycle and know any change is fully working. Cheers, Richard
diff --git a/meta/recipes-devtools/patchelf/patchelf/handle-read-only-files.patch b/meta/recipes-devtools/patchelf/patchelf/handle-read-only-files.patch deleted file mode 100644 index b755a263a4..0000000000 --- a/meta/recipes-devtools/patchelf/patchelf/handle-read-only-files.patch +++ /dev/null @@ -1,65 +0,0 @@ -From 682fb48c137b687477008b68863c2a0b73ed47d1 Mon Sep 17 00:00:00 2001 -From: Fabio Berton <fabio.berton@ossystems.com.br> -Date: Fri, 9 Sep 2016 16:00:42 -0300 -Subject: [PATCH] handle read-only files - -Patch from: -https://github.com/darealshinji/patchelf/commit/40e66392bc4b96e9b4eda496827d26348a503509 - -Upstream-Status: Denied [https://github.com/NixOS/patchelf/pull/89] - -Signed-off-by: Fabio Berton <fabio.berton@ossystems.com.br> - ---- - src/patchelf.cc | 16 +++++++++++++++- - 1 file changed, 15 insertions(+), 1 deletion(-) - -Index: git/src/patchelf.cc -=================================================================== ---- git.orig/src/patchelf.cc -+++ git/src/patchelf.cc -@@ -534,9 +534,19 @@ void ElfFile<ElfFileParamNames>::sortShd - - static void writeFile(const std::string & fileName, const FileContents & contents) - { -+ struct stat st; -+ int fd; -+ - debug("writing %s\n", fileName.c_str()); - -- int fd = open(fileName.c_str(), O_CREAT | O_TRUNC | O_WRONLY, 0777); -+ if (stat(fileName.c_str(), &st) != 0) -+ error("stat"); -+ -+ if (chmod(fileName.c_str(), 0600) != 0) -+ error("chmod"); -+ -+ fd = open(fileName.c_str(), O_CREAT | O_TRUNC | O_WRONLY, 0777); -+ - if (fd == -1) - error("open"); - -@@ -551,8 +561,6 @@ static void writeFile(const std::string - bytesWritten += portion; - } - -- if (close(fd) >= 0) -- return; - /* - * Just ignore EINTR; a retry loop is the wrong thing to do. - * -@@ -561,9 +569,11 @@ static void writeFile(const std::string - * http://utcc.utoronto.ca/~cks/space/blog/unix/CloseEINTR - * https://sites.google.com/site/michaelsafyan/software-engineering/checkforeintrwheninvokingclosethinkagain - */ -- if (errno == EINTR) -- return; -- error("close"); -+ if ((close(fd) < 0) && errno != EINTR) -+ error("close"); -+ -+ if (chmod(fileName.c_str(), st.st_mode) != 0) -+ error("chmod"); - } - - diff --git a/meta/recipes-devtools/patchelf/patchelf_0.14.5.bb b/meta/recipes-devtools/patchelf/patchelf_0.15.0.bb similarity index 79% rename from meta/recipes-devtools/patchelf/patchelf_0.14.5.bb rename to meta/recipes-devtools/patchelf/patchelf_0.15.0.bb index 0fa2c00f1d..26abde2ed5 100644 --- a/meta/recipes-devtools/patchelf/patchelf_0.14.5.bb +++ b/meta/recipes-devtools/patchelf/patchelf_0.15.0.bb @@ -4,10 +4,8 @@ HOMEPAGE = "https://github.com/NixOS/patchelf" LICENSE = "GPL-3.0-only" -SRC_URI = "git://github.com/NixOS/patchelf;protocol=https;branch=master \ - file://handle-read-only-files.patch \ - " -SRCREV = "a35054504293f9ff64539850d1ed0bfd2f5399f2" +SRC_URI = "git://github.com/NixOS/patchelf;protocol=https;branch=master" +SRCREV = "49008002562355b0e35075cbc1c42c645ff04e28" S = "${WORKDIR}/git"
Drop handle-read-only-files.patch: read only files should be handled by making them writeable explicitly. See the upstream discussion: https://github.com/NixOS/patchelf/pull/89 Signed-off-by: Alexander Kanavin <alex@linutronix.de> --- .../patchelf/handle-read-only-files.patch | 65 ------------------- ...{patchelf_0.14.5.bb => patchelf_0.15.0.bb} | 6 +- 2 files changed, 2 insertions(+), 69 deletions(-) delete mode 100644 meta/recipes-devtools/patchelf/patchelf/handle-read-only-files.patch rename meta/recipes-devtools/patchelf/{patchelf_0.14.5.bb => patchelf_0.15.0.bb} (79%)