[Rpm-maint] [rpm-software-management/rpm] Failed to delete files during the rmbuild execution (Issue #2380)
When Non-privileged user was used to build the "rpm" package, the file deletion failed during the final rmbuild cleanup phase. The problem is caused by the lack of write permission on the testing directory. ``` ... [ 209s] Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/abuild/rpmbuild/BUILDROOT/rpm-4.18.0-1.oe1.x86_64 [ 209s] Wrote: /home/abuild/rpmbuild/SRPMS/rpm-4.18.0-1.oe1.src.rpm [ 209s] Wrote: /home/abuild/rpmbuild/RPMS/x86_64/rpm-plugin-systemd-inhibit-4.18.0-1.oe1.x86_64.rpm [ 209s] Wrote: /home/abuild/rpmbuild/RPMS/x86_64/rpm-build-4.18.0-1.oe1.x86_64.rpm [ 209s] Wrote: /home/abuild/rpmbuild/RPMS/x86_64/python3-rpm-4.18.0-1.oe1.x86_64.rpm [ 209s] Wrote: /home/abuild/rpmbuild/RPMS/x86_64/rpm-devel-4.18.0-1.oe1.x86_64.rpm [ 209s] Wrote: /home/abuild/rpmbuild/RPMS/x86_64/rpm-libs-4.18.0-1.oe1.x86_64.rpm [ 209s] Wrote: /home/abuild/rpmbuild/RPMS/x86_64/rpm-4.18.0-1.oe1.x86_64.rpm [ 209s] Wrote: /home/abuild/rpmbuild/RPMS/x86_64/rpm-debugsource-4.18.0-1.oe1.x86_64.rpm [ 210s] Wrote: /home/abuild/rpmbuild/RPMS/x86_64/rpm-debuginfo-4.18.0-1.oe1.x86_64.rpm [ 210s] Wrote: /home/abuild/rpmbuild/RPMS/noarch/rpm-help-4.18.0-1.oe1.noarch.rpm [ 210s] Executing(%clean): /bin/sh -e /var/tmp/rpm-tmp.2fUx5e [ 210s] + umask 022 [ 210s] + cd /home/abuild/rpmbuild/BUILD [ 210s] + cd rpm-4.18.0 [ 210s] + /usr/bin/rm -rf /home/abuild/rpmbuild/BUILDROOT/rpm-4.18.0-1.oe1.x86_64 [ 210s] + RPM_EC=0 [ 210s] ++ jobs -p [ 210s] + exit 0 [ 210s] Executing(rmbuild): /bin/sh -e /var/tmp/rpm-tmp.jaZtVz [ 210s] + umask 022 [ 210s] + cd /home/abuild/rpmbuild/BUILD [ 210s] + rm -rf rpm-4.18.0 rpm-4.18.0.gemspec [ 210s] rm: cannot remove 'rpm-4.18.0/tests/testing/usr/share/locale/br/LC_MESSAGES/rpm.mo': Permission denied [ 210s] rm: cannot remove 'rpm-4.18.0/tests/testing/usr/share/locale/ko/LC_MESSAGES/rpm.mo': Permission denied [ 210s] rm: cannot remove 'rpm-4.18.0/tests/testing/usr/share/locale/uk/LC_MESSAGES/rpm.mo': Permission denied ... ``` -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2380 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Failed to delete files during the rmbuild execution (Issue #2380)
It is confirmed that this problem is caused by b34333fa021c0ee7215714eeef96d1a2843ea08e. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2380#issuecomment-1418443920 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Failed to delete files during the rmbuild execution (Issue #2380)
No, that commit is not the problem, it's just the messenger. See https://github.com/rpm-software-management/rpm/issues/1382 -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2380#issuecomment-1418551808 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Failed to delete files during the rmbuild execution (Issue #2380)
Closed #2380 as completed. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2380#event-8443809541 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Enhance requires with version information from the build root. (PR #2372)
@pmatilai commented on this pull request. > +/* + * Rather than re-implement path searching for shared objects, use + * dlmopen(). This will still perform initialization and finalization + * functions, which isn't necessarily safe, so do that in a separate + * process. + */ +static char *getLibtoolVerFromShLink(const char *filename) +{ +char dest[PATH_MAX]; +int pipefd[2]; +pid_t cpid; + +if (pipe(pipefd) == -1) { + return NULL; // Should this be a fatal error instead? +} +cpid = fork(); Why the fork? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2372#pullrequestreview-1284594584 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Enhance requires with version information from the build root. (PR #2372)
@gordonmessmer commented on this pull request. > +/* + * Rather than re-implement path searching for shared objects, use + * dlmopen(). This will still perform initialization and finalization + * functions, which isn't necessarily safe, so do that in a separate + * process. + */ +static char *getLibtoolVerFromShLink(const char *filename) +{ +char dest[PATH_MAX]; +int pipefd[2]; +pid_t cpid; + +if (pipe(pipefd) == -1) { + return NULL; // Should this be a fatal error instead? +} +cpid = fork(); When dlopen() loads a library, it runs functions with the __attribute__((constructor)) function attribute, and when they are closed, it runs functions with __attribute__((destructor)). This isn't always safe. Some libraries (like gobject) do no support being opened and closed, and they'll result in a SEGV if they're opened more than once. That will happen if elfdeps examines a shared object that is linked to gobject, and then later another one (or libgobject itself). However, if we fork and then open only one shared object and then exit, we don't cause that problem. dlopen() isn't a perfect mechanism for resolving the name of a shared object to its path for that reason, but the alternatives are either parsing the output of ldd (which seems an awful lot like forking and dlopen() an object with more steps) or re-implementing the search algorithm in glibc/elf/dl-load.c:_dl_map_object(), and I dislike those options more, personally. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2372#discussion_r1096987384 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Enhance requires with version information from the build root. (PR #2372)
@pmatilai commented on this pull request. > +/* + * Rather than re-implement path searching for shared objects, use + * dlmopen(). This will still perform initialization and finalization + * functions, which isn't necessarily safe, so do that in a separate + * process. + */ +static char *getLibtoolVerFromShLink(const char *filename) +{ +char dest[PATH_MAX]; +int pipefd[2]; +pid_t cpid; + +if (pipe(pipefd) == -1) { + return NULL; // Should this be a fatal error instead? +} +cpid = fork(); Ack, I suspected it would be something like that. If we need a fork then we need a fork, but please add a comment about that. That said, within rpm, the dependency generators like elfdeps always get called one file at a time, so the problematic case never happens in the actual use-case. But for testing purposes, it is handy to be able to pass multiple files at once. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2372#discussion_r1097002768 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Enhance requires with version information from the build root. (PR #2372)
@gordonmessmer commented on this pull request. > +/* + * Rather than re-implement path searching for shared objects, use + * dlmopen(). This will still perform initialization and finalization + * functions, which isn't necessarily safe, so do that in a separate + * process. + */ +static char *getLibtoolVerFromShLink(const char *filename) +{ +char dest[PATH_MAX]; +int pipefd[2]; +pid_t cpid; + +if (pipe(pipefd) == -1) { + return NULL; // Should this be a fatal error instead? +} +cpid = fork(); There is a brief comment before getLibtoolVerFromShLink, but I can move it or expand it with more detail, at your preference. Note: The problem does occur when processing a single file, because this new process is calling dlopen() on each object in the dynamic table of the file being processed. So, if a file is linked to libsoup3, for example, elfdeps will dlopen() libsoup and then later dlopen() libgobject. Each handle is passed to dlinfo() and then the resulting linkmap->l_name is processed. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2372#discussion_r1097010807 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Enhance requires with version information from the build root. (PR #2372)
@pmatilai commented on this pull request. > +/* + * Rather than re-implement path searching for shared objects, use + * dlmopen(). This will still perform initialization and finalization + * functions, which isn't necessarily safe, so do that in a separate + * process. + */ +static char *getLibtoolVerFromShLink(const char *filename) +{ +char dest[PATH_MAX]; +int pipefd[2]; +pid_t cpid; + +if (pipe(pipefd) == -1) { + return NULL; // Should this be a fatal error instead? +} +cpid = fork(); Oh, I seem to have missed the comment indeed. The comment still doesn't explain *why* it is unsafe to do the processing in a single process, but actually the better place for the long explanation (basically what you just said here) is in the commit message. And okay, it does sound like we do need the fork then. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2372#discussion_r1097021319 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Enhance requires with version information from the build root. (PR #2372)
@pmatilai commented on this pull request. > + * a copy of the version number. + */ +static char *getLibtoolVer(const char *filename) +{ +const char *so; +char dest[PATH_MAX]; +int destsize = 0; +int found_digit = 0, found_dot = 0; + +destsize = readlink(filename, dest, PATH_MAX); +if (destsize > 0) { + dest[destsize] = 0; + filename = dest; +} +// Start from the end of the string. Verify that it ends with +// numbers and dots, preceded by ".so.". On a purely stylistical nit, `//` comments do not belong in rpm coding style. Always use `/* ... */` and for multiline comments, ``` /* * ... * ... */ ``` /me realizes this isn't even explained in contributing.md, sigh... -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2372#pullrequestreview-1284678640 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Enhance requires with version information from the build root. (PR #2372)
@gordonmessmer commented on this pull request. > + * a copy of the version number. + */ +static char *getLibtoolVer(const char *filename) +{ +const char *so; +char dest[PATH_MAX]; +int destsize = 0; +int found_digit = 0, found_dot = 0; + +destsize = readlink(filename, dest, PATH_MAX); +if (destsize > 0) { + dest[destsize] = 0; + filename = dest; +} +// Start from the end of the string. Verify that it ends with +// numbers and dots, preceded by ".so.". I'll follow up on each of those items tomorrow. Thanks for the feedback! -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2372#discussion_r1097034900 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Failed to delete files during the rmbuild execution (Issue #2380)
> No, that commit is not the problem, it's just the messenger. > > See #1382 https://github.com/rpm-software-management/rpm/commit/b34333fa021c0ee7215714eeef96d1a2843ea08e changed the default behavior. Now users running `rpmbuild -ba *spec` will automatically remove the build directory, so many other packages which doesn't have `make clean` in `%check` may fail to build rpm. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/2380#issuecomment-1418647208 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Simplify + clean up rpmio thread enablement code (PR #2379)
Merged #2379 into master. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2379#event-8444504213 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Enhance requires with version information from the build root. (PR #2372)
@pmatilai commented on this pull request. > @@ -542,6 +542,12 @@ Supplements: (%{name} = %{version}-%{release} and > langpacks-%{1})\ # Use internal dependency generator rather than external helpers? %_use_internal_dependency_generator1 +# +# Generate minimum versions for ELF libraries that don't provide +# versioned symbol? +#%_provide_fallback_versions --libtool-version-fallback +#%_require_fallback_versions --libtool-version-fallback I think these ought to have elf-prefixes (ie %_elf_provide...), just to be on the safe side. Historically rpm has been terribly sloppy wrt namespaces and we can no longer afford to do that. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2372#pullrequestreview-1284696569 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint