Re: [Rpm-maint] [rpm-software-management/rpm] Enhance requires with version information from the build root. (PR #2372)

2023-02-05 Thread Panu Matilainen
@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


Re: [Rpm-maint] [rpm-software-management/rpm] Simplify + clean up rpmio thread enablement code (PR #2379)

2023-02-05 Thread Panu Matilainen
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] Failed to delete files during the rmbuild execution (Issue #2380)

2023-02-05 Thread licunlong
> 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] Enhance requires with version information from the build root. (PR #2372)

2023-02-05 Thread Gordon Messmer
@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] Enhance requires with version information from the build root. (PR #2372)

2023-02-05 Thread Panu Matilainen
@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)

2023-02-05 Thread Panu Matilainen
@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)

2023-02-05 Thread Gordon Messmer
@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)

2023-02-05 Thread Panu Matilainen
@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)

2023-02-05 Thread Gordon Messmer
@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)

2023-02-05 Thread Panu Matilainen
@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] Failed to delete files during the rmbuild execution (Issue #2380)

2023-02-05 Thread Panu Matilainen
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] Failed to delete files during the rmbuild execution (Issue #2380)

2023-02-05 Thread Panu Matilainen
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)

2023-02-05 Thread xujing
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


[Rpm-maint] [rpm-software-management/rpm] Failed to delete files during the rmbuild execution (Issue #2380)

2023-02-05 Thread xujing
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