jengelh/sssd
SHA256
1
0
Fork 1
forked from pool/sssd
Code Pull Requests Activity

Avoid globs, list specific packaged files #2

Manually merged
jengelh merged 1 commits from scabrero/sssd:master into master 2024-10-01 12:51:17 +02:00
Conversation 7 Commits 1 Files Changed 1 +4 -2
scabrero commented 2024-09-26 11:35:40 +02:00
Contributor
Copy Link

In preparation for samba 4.21 update.

Signed-off-by: Samuel Cabrero scabrero@suse.de

In preparation for samba 4.21 update. Signed-off-by: Samuel Cabrero <scabrero@suse.de>
jengelh commented 2024-09-26 23:14:17 +02:00
Owner
Copy Link

The rationale is insufficient. What is in samba 4.21 that makes these changes necessary?

The rationale is insufficient. What is in samba 4.21 that makes these changes necessary?
scabrero commented 2024-09-27 10:52:00 +02:00
Author
Contributor
Copy Link

The rationale is insufficient. What is in samba 4.21 that makes these changes necessary?

Samba no longer provides an independent libldb tarball, the library is now considered to be private by default but it provides a configure switch to expose it. Related to this the modules path changed from /usr/lib64/ldb2/modules/ldb/ to /usr/lib64/samba/ldb, causing the following build error:

[  103s] found conflict of sssd-2.9.5-0.x86_64 with sssd-winbind-idmap-2.9.5-0.x86_64:
[  103s]   - /usr/lib64/samba/ldb/memberof.so
> The rationale is insufficient. What is in samba 4.21 that makes these changes necessary? Samba no longer provides an independent `libldb` tarball, the library is now considered to be private by default but it provides a configure switch to expose it. Related to this the modules path changed from `/usr/lib64/ldb2/modules/ldb/` to `/usr/lib64/samba/ldb`, causing the following build error: ``` [ 103s] found conflict of sssd-2.9.5-0.x86_64 with sssd-winbind-idmap-2.9.5-0.x86_64: [ 103s] - /usr/lib64/samba/ldb/memberof.so ```
jengelh commented 2024-09-28 13:24:51 +02:00
Owner
Copy Link

The current filelist contains (rpm -ql):

$ rpm -qlvp binaries/sssd-2.9.5-227.1.x86_64.rpm | grep /ldb
drwxr-xr-x    2 root     root                        0 Jul 17 11:19 /usr/lib64/ldb2/modules/ldb
-rwxr-xr-x    1 root     root                    72128 Jul 17 11:19 /usr/lib64/ldb2/modules/ldb/memberof.so

$ rpm -qlvp binaries/sssd-winbind-idmap-2.9.5-227.1.x86_64.rpm | grep /samba/
drwxr-xr-x    2 root     root                        0 Jul 17 11:19 /usr/lib64/samba/idmap
-rwxr-xr-x    1 root     root                    14344 Jul 17 11:19 /usr/lib64/samba/idmap/sss.so

After your change, the filelist contains:

$ rpm -qlvp /run/ob/j0/.mount/.build.packages/RPMS/x86_64/sssd-2.9.5-0.x86_64.rpm | grep /ldb
-rwxr-xr-x    1 root     root                    72128 Sep 28 13:17 /usr/lib64/ldb2/modules/ldb/memberof.so

$ rpm -qlvp /run/ob/j0/.mount/.build.packages/RPMS/x86_64/sssd-winbind-idmap-2.9.5-0.x86_64.rpm | grep /samba/
-rwxr-xr-x    1 root     root                    14344 Sep 28 13:17 /usr/lib64/samba/idmap/sss.so

So the only immediate change you did is a removal of directories, which is bad in its own right.
The explicit spelling of .so files has the effect that, if those files were to be no longer produced by the sssd build, that we will know about their disappearance. But if sss.so / memberof.so is expected to be no longer produced, then why bother spelling it out instead of removing it altogether⸘

The current filelist contains (rpm -ql): ``` $ rpm -qlvp binaries/sssd-2.9.5-227.1.x86_64.rpm | grep /ldb drwxr-xr-x 2 root root 0 Jul 17 11:19 /usr/lib64/ldb2/modules/ldb -rwxr-xr-x 1 root root 72128 Jul 17 11:19 /usr/lib64/ldb2/modules/ldb/memberof.so $ rpm -qlvp binaries/sssd-winbind-idmap-2.9.5-227.1.x86_64.rpm | grep /samba/ drwxr-xr-x 2 root root 0 Jul 17 11:19 /usr/lib64/samba/idmap -rwxr-xr-x 1 root root 14344 Jul 17 11:19 /usr/lib64/samba/idmap/sss.so ``` After your change, the filelist contains: ``` $ rpm -qlvp /run/ob/j0/.mount/.build.packages/RPMS/x86_64/sssd-2.9.5-0.x86_64.rpm | grep /ldb -rwxr-xr-x 1 root root 72128 Sep 28 13:17 /usr/lib64/ldb2/modules/ldb/memberof.so $ rpm -qlvp /run/ob/j0/.mount/.build.packages/RPMS/x86_64/sssd-winbind-idmap-2.9.5-0.x86_64.rpm | grep /samba/ -rwxr-xr-x 1 root root 14344 Sep 28 13:17 /usr/lib64/samba/idmap/sss.so ``` So the only immediate change you did is a removal of directories, which is bad in its own right. The explicit spelling of .so files has the effect that, if those files were to be no longer produced by the sssd build, that we will know about their disappearance. But if sss.so / memberof.so is expected to be no longer produced, then why bother spelling it out instead of removing it altogether⸘
jengelh closed this pull request 2024-09-28 13:24:51 +02:00
scabrero commented 2024-09-30 12:40:34 +02:00
Author
Contributor
Copy Link

Hi @jengelh

I think there is a misunderstanding here. You have to build sssd with samba 4.21 in the same OBS project (I had to create a link with osc linkpac because branching packages with <scmsync> is no longer possible and AFAIK the GIT workflow isn't ready...).

After building with 4.21:

$ rpm -ql sssd-2.9.5-0.x86_64.rpm | grep memberof
/usr/lib64/samba/ldb/memberof.so

$ rpm -ql sssd-winbind-idmap-2.9.5-0.x86_64.rpm
/usr/lib64/samba/idmap/sss.so

Regarding the directories, from https://en.opensuse.org/openSUSE:Specfile_guidelines#Ownership

Directory ownership is a little more complex than file ownership. Although the rule of thumb is the same: own all the directories you create but none of the directories of packages you depend on, there are several instances where it is desirable for multiple packages to own a directory.

In the case of /usr/lib64/samba/ldb/memberof.so the directory /usr/lib64/samba/ldb is owned by libldb2 and sssd requires it, so the directory shouldn't be owned by sssd.

The case of /usr/lib64/samba/idmap/sss.so falls into the Common directory for unrelated packages exception. In this case sssd should also own /usr/lib64/samba.

Hi @jengelh I think there is a misunderstanding here. You have to build sssd with samba 4.21 in the same OBS project (I had to create a link with `osc linkpac` because branching packages with `<scmsync>` is no longer possible and AFAIK the GIT workflow isn't ready...). After building with 4.21: ``` $ rpm -ql sssd-2.9.5-0.x86_64.rpm | grep memberof /usr/lib64/samba/ldb/memberof.so $ rpm -ql sssd-winbind-idmap-2.9.5-0.x86_64.rpm /usr/lib64/samba/idmap/sss.so ``` Regarding the directories, from https://en.opensuse.org/openSUSE:Specfile_guidelines#Ownership ``` Directory ownership is a little more complex than file ownership. Although the rule of thumb is the same: own all the directories you create but none of the directories of packages you depend on, there are several instances where it is desirable for multiple packages to own a directory. ``` In the case of `/usr/lib64/samba/ldb/memberof.so` the directory `/usr/lib64/samba/ldb` is owned by `libldb2` and `sssd` requires it, so the directory shouldn't be owned by sssd. The case of `/usr/lib64/samba/idmap/sss.so` falls into the `Common directory for unrelated packages` exception. In this case sssd should also own `/usr/lib64/samba`.
scabrero reopened this pull request 2024-09-30 12:40:35 +02:00
jengelh commented 2024-09-30 17:27:20 +02:00
Owner
Copy Link

with samba 4.21 in the same OBS project

osc getbinaries home:scabrero:samba-4-21-udpate/samba openSUSE_Tumbleweed x86_64
mv binaries /tmp/samba
cd network/ldap/sssd && osc build -p /tmp/samba openSUSE_Tumbleweed

is exactly what I did. And this very local build succeeds already with the unmodified sssd.spec. The fact that the memberof.so file changed directories is automatically taken care of.

sssd.spec also does not depend on ldb's directory presence; which is to say, if libldb2 stopped shipping plugins tomorrow, no one would potentially own /usr/lib64/samba/ldb, and that's a good case for why sssd.spec ought to co-own it.

P.S.: Anyway, if you would like to indulge in a second opinion for the git workflow, do check out https://inai.de/linux/obs-with-git-scmsync where I have (just now) written down my rough take on the workflow.

>with samba 4.21 in the same OBS project ```sh osc getbinaries home:scabrero:samba-4-21-udpate/samba openSUSE_Tumbleweed x86_64 mv binaries /tmp/samba cd network/ldap/sssd && osc build -p /tmp/samba openSUSE_Tumbleweed ``` is exactly what I did. And this very local build succeeds already with the unmodified sssd.spec. The fact that the memberof.so file changed directories is automatically taken care of. sssd.spec also does not depend on ldb's directory presence; which is to say, if libldb2 stopped shipping plugins tomorrow, no one would potentially own /usr/lib64/samba/ldb, and that's a good case for why sssd.spec ought to co-own it. P.S.: Anyway, if you would like to indulge in a second opinion for the git workflow, do check out https://inai.de/linux/obs-with-git-scmsync where I have (just now) written down my rough take on the workflow.
scabrero commented 2024-10-01 11:55:13 +02:00
Author
Contributor
Copy Link

with samba 4.21 in the same OBS project

osc getbinaries home:scabrero:samba-4-21-udpate/samba openSUSE_Tumbleweed x86_64
mv binaries /tmp/samba
cd network/ldap/sssd && osc build -p /tmp/samba openSUSE_Tumbleweed

is exactly what I did. And this very local build succeeds already with the unmodified sssd.spec. The fact that the memberof.so file changed directories is automatically taken care of.

I tried and in my machine it fails in the same way OBS does.

sssd.spec also does not depend on ldb's directory presence; which is to say, if libldb2 stopped shipping plugins tomorrow, no one would potentially own /usr/lib64/samba/ldb, and that's a good case for why sssd.spec ought to co-own it.

Ok, I will push an update.

> >with samba 4.21 in the same OBS project > > ```sh > osc getbinaries home:scabrero:samba-4-21-udpate/samba openSUSE_Tumbleweed x86_64 > mv binaries /tmp/samba > cd network/ldap/sssd && osc build -p /tmp/samba openSUSE_Tumbleweed > ``` > > is exactly what I did. And this very local build succeeds already with the unmodified sssd.spec. The fact that the memberof.so file changed directories is automatically taken care of. I tried and in my machine it fails in the same way [OBS](https://build.opensuse.org/package/show/home:scabrero:samba-4-21-udpate/sssd) does. > > sssd.spec also does not depend on ldb's directory presence; which is to say, if libldb2 stopped shipping plugins tomorrow, no one would potentially own /usr/lib64/samba/ldb, and that's a good case for why sssd.spec ought to co-own it. > Ok, I will push an update.
jengelh commented 2024-10-01 12:12:40 +02:00
Owner
Copy Link

in my machine it fails in the same way OBS does

That means I must have made an error somewhere. Oh well, let me please take care of it now that I know where the error is ;-)

>in my machine it fails in the same way OBS does That means I must have made an error somewhere. Oh well, let me please take care of it now that I know where the error is ;-)
jengelh manually merged commit 3a2bee3ebf into master 2024-10-01 12:51:17 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jengelh/sssd#2
No description provided.
Delete Branch "scabrero/sssd:master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?