Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable RPM sysusers integration #1367

Merged

Conversation

nikromen
Copy link
Collaborator

@nikromen nikromen commented Apr 28, 2024

@nikromen nikromen marked this pull request as draft April 28, 2024 15:16
@xsuchy
Copy link
Member

xsuchy commented Apr 28, 2024

what about %files section?

@nikromen nikromen force-pushed the provice-groups-via-sysusers branch from b347681 to 2b1b941 Compare July 22, 2024 14:55
@xsuchy
Copy link
Member

xsuchy commented Jul 23, 2024

I think this still miss a few things: BuildRequires and %pre macro. See https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation

@xsuchy
Copy link
Member

xsuchy commented Jul 23, 2024

Additionally, it would be nice if you amend https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation and for package that provides the user do s/mock/mock-core-configs/.

@praiskup
Copy link
Member

Additionally, it would be nice if you amend https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation and for package that provides the user do s/mock/mock-core-configs/.

.. or mock-filesystem.

@nikromen nikromen force-pushed the provice-groups-via-sysusers branch 3 times, most recently from 7968df6 to d348272 Compare July 23, 2024 11:51
@nikromen nikromen changed the title Revert "revert sysusers setting [RHBZ#1740545]" Enable RPM sysusers integration Jul 23, 2024
@nikromen nikromen force-pushed the provice-groups-via-sysusers branch from d348272 to 4c8cd88 Compare July 23, 2024 14:45
@xsuchy
Copy link
Member

xsuchy commented Jul 23, 2024

Why you removed that line from %files section?

@nikromen nikromen force-pushed the provice-groups-via-sysusers branch from 4c8cd88 to 353c12a Compare July 23, 2024 14:57
@nikromen
Copy link
Collaborator Author

nikromen commented Jul 23, 2024

The %sysusers_create_compat macro will generate something like this:

# generated from mock.conf.
getent group 'mock' >/dev/null || groupadd -r 'mock' || :
getent passwd 'mock' >/dev/null || \
    useradd -r -g 'mock' -d '/run/mock' -s '/sbin/nologin' -c 'Runs Uid '\''N'\'' some text' 'mock' || :

so the line in files is no longer needed

@xsuchy
Copy link
Member

xsuchy commented Jul 23, 2024

I meant:

%files
%{_sysusersdir}/mock.conf

But you added it back with latest update. It is ok then.

mock/mock.spec Outdated Show resolved Hide resolved
mock/mock.spec Outdated Show resolved Hide resolved
@praiskup
Copy link
Member

I tried to install on UBI8:

  Installing       : mock-5.6.post1-1.git.3754.50829bf.el8.noarch                                                                                                                                                                     52/53 
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root

  Installing       : mock-core-configs-40.6-1.git.3754.50829bf.el8.noarch                                                                                                                                                             53/53 
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
warning: group mock does not exist - using root
...

@nikromen nikromen force-pushed the provice-groups-via-sysusers branch from 353c12a to ab1e9a5 Compare July 24, 2024 14:27
mock/mock.spec Outdated Show resolved Hide resolved
mock/mock.spec Outdated Show resolved Hide resolved
mock/mock.spec Outdated
@@ -17,7 +16,8 @@ License: GPL-2.0-or-later
# cd mock
# git reset --hard %%{name}-%%{version}
# tito build --tgz
Source: %{name}-%{version}.tar.gz
Source0: %{name}-%{version}.tar.gz
Source1: https://raw.githubusercontent.com/rpm-software-management/mock/main/mock-core-configs/mock.conf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the way to address the file in %pre when macros are evaluated (before the tarball is extrected).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are confusing %pre and %prep.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No no ... the %sysusers_create_compat macro is evaluated at the time when the tarball isn't extracted, yet. See how it is defined, it contains the shell-script macro %().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to my experiments, ... yes? But maybe I'm doing something wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I tried to install bodhi-server in fedora:rawhide container, and there's no bodhi user/group:

[root@db5467446600 /]# grep bodhi /etc/group /etc/passwd
[root@db5467446600 /]# 

The reason nobody noticed is that they (probably) install it on servers with systemd.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they also try to work around this by defining the file directly in spec
https://src.fedoraproject.org/rpms/bodhi-server/blob/rawhide/f/bodhi-server.spec#_84

it seems like hard task to push the .sysusers file from upstream to %pre...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is wrong approach, the -f file needs to contain shell script, not the sysusersdir content.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, as I realized in the other thread, the feature is now RPM built-in. But it still doesn't work for the bodhi-server :-) confusing as hell. I think the problem actually is that the dropin has .sysusers suffix instead of .conf. I'm trying to propose a fix here.

@nikromen nikromen force-pushed the provice-groups-via-sysusers branch 4 times, most recently from 55d852e to 1fd4da3 Compare July 25, 2024 15:41
@nikromen nikromen force-pushed the provice-groups-via-sysusers branch 2 times, most recently from 392a041 to 442074b Compare July 29, 2024 11:42
@nikromen nikromen force-pushed the provice-groups-via-sysusers branch 2 times, most recently from 29afaaa to 52dab30 Compare July 29, 2024 11:43
mock/mock.spec Outdated Show resolved Hide resolved
@nikromen nikromen force-pushed the provice-groups-via-sysusers branch from 52dab30 to 03f8f00 Compare July 29, 2024 12:14
@nikromen nikromen marked this pull request as ready for review July 29, 2024 13:39
mock/mock.spec Outdated Show resolved Hide resolved
@praiskup
Copy link
Member

The release notes snippet deserves update, otherwise LGTM

mock/mock.spec Outdated Show resolved Hide resolved
@nikromen nikromen force-pushed the provice-groups-via-sysusers branch from 03f8f00 to 0cfadda Compare July 29, 2024 16:22
@praiskup
Copy link
Member

Hmpf, now I recall I've seen this feature before, and guess what :)

[root@a7cd3c91a144 /]# rpm -q --provides mock-5.6.post1-1.git.3755.7d754b2.fc41.noarch | grep group
group(mock) = ZyBtb2NrIDEzNSAt

mock/mock.spec Outdated Show resolved Hide resolved
@@ -169,6 +169,9 @@ done

./precompile-bash-completion "mock.complete"

# this is what %%sysusers_create_compat will expand to
%{_rpmconfigdir}/sysusers.generate-pre.sh mock.conf > sysusers_script
Copy link
Member

@praiskup praiskup Jul 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I eventually think that we do not need this %pre dance at all for systems with RPM >= 4.19. I'd use something like:

%if 0%{!?fedora:1} && ( 0%{?rhel} <= 9 || ... others older distros may come ... )
%{_rpmconfigdir}/sysusers.generate-pre.sh mock.conf > sysusers_script

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this EL10+ condition, we risk a conflict of the "built-in" mechanism and the %pre code.

@nikromen nikromen force-pushed the provice-groups-via-sysusers branch 2 times, most recently from 3285a6b to 927d25f Compare August 5, 2024 10:58
@nikromen nikromen force-pushed the provice-groups-via-sysusers branch from 927d25f to 08966e2 Compare August 5, 2024 11:17
Copy link
Member

@xsuchy xsuchy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@xsuchy xsuchy merged commit 5071073 into rpm-software-management:main Aug 5, 2024
21 of 22 checks passed
praiskup added a commit to praiskup/mock that referenced this pull request Nov 1, 2024
The script for generating the %pre scriptlet is non-existing on new
distributions (like Mageia now, but Fedora will drop that sooner or
later, too).  The build of Mock on Mageia previously relied on
sysusers.generate-pre.sh, and failed.

The sysusers feature is an RPM built-in anyway (on modern systems), so
there's no reason to duplicate the logic in %pre.  Generate the %pre
scriptlet only for older distributions.

Complements: rpm-software-management#1367
Relates: rpm-software-management#289
Relates: fedora-copr/copr#2789
praiskup added a commit to praiskup/mock that referenced this pull request Nov 1, 2024
The script for generating the %pre scriptlet is non-existing on new
distributions (like Mageia now, but Fedora will drop that sooner or
later, too).  The build of Mock on Mageia previously relied on
sysusers.generate-pre.sh, and failed.

The sysusers feature is an RPM built-in anyway (on modern systems), so
there's no reason to duplicate the logic in %pre.  Generate the %pre
scriptlet only for older distributions.

Complements: rpm-software-management#1367
Relates: rpm-software-management#289
Relates: fedora-copr/copr#2789
praiskup added a commit to praiskup/mock that referenced this pull request Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide users/groups via /usr/lib/sysusers.d/
4 participants