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

use -N instead of -n for useradd #1052

Merged
merged 2 commits into from
May 2, 2023
Merged

Conversation

xsuchy
Copy link
Member

@xsuchy xsuchy commented Apr 6, 2023

-n is downstream patch in Fedora and RHEL now. It was used in past. Since RHEL6 the option -N is available.
Let use it.

But that means if you want to build for RHEL5 you have to alter config_opts['useradd'] and return back -n option.

Resolves: #1050

@Conan-Kudo
Copy link
Member

Does this allow us to drop the useradd overrides we've been using for all the non-RH/Fedora chroots now?

@Conan-Kudo
Copy link
Member

And in that case, we should put the override in the RHEL 5 configs as part of this PR and drop the overrides everywhere else.

@xsuchy
Copy link
Member Author

xsuchy commented Apr 6, 2023

I really hope that no one uses rhel5 config today.
I know of Brew people.

@praiskup
Copy link
Member

praiskup commented Apr 6, 2023

Does this allow us to drop the useradd overrides we've been using for all the non-RH/Fedora chroots now?

I'm not sure. The difference for those chroots would be:

- '/usr/sbin/useradd -o -m -u {{chrootuid}} -g {{chrootgid}} -d {{chroothome}} {{chrootuser}}'
+ '/usr/sbin/useradd -o -m -u {{chrootuid}} -g {{chrootgid}} -d {{chroothome}} -N {{chrootuser}}

Is it safe to use -N (capital) in all of those chroots that currently
override? In the new RH distributions, -n is just an equivalent to -N (so
the change Mirek is doing is no-op).

I know of Brew people.

They maintain their own configs, and typically also run an older Mock version
anyway.

@Conan-Kudo
Copy link
Member

Well, if -N has been the upstream one since RHEL 6 (2010), then everyone should have that option already. I didn't know what -n did when I wrote those stanzas and just dropped it. The correct thing to do is to put -N in and use it properly.

@praiskup
Copy link
Member

@Conan-Kudo we discussed with @xsuchy today - and it seems that removing the useradd overrides from config would make the mock-core-configs code (released asynchronously, e.g. we plan to release ASAP because of the new Amazon config fiules) incompatible with the old mock releases. So when removing the overrides, we should at the same time bump the mock version requirement.

Therefore we should either merge as-is (and do the config change later on) or wait. We are not yet ready to wrap a new mock.rpm release.

@praiskup
Copy link
Member

@Conan-Kudo what do you think?

@Aemn567
Copy link

Aemn567 commented Apr 13, 2023

n

@Conan-Kudo
Copy link
Member

We should wait on this, then. It's not urgent to land, and I'd rather this be a complete change.

@praiskup
Copy link
Member

I have no objections, @xsuchy would you mind updating this PR then so we have it prepared?

-n is downstream patch in Fedora and RHEL now. It was used in past.
Since RHEL6 the option -N is available.
Let use it.

But that means if you want to build for RHEL5 you have to alter
config_opts['useradd'] and return back -n option.

Resolves: rpm-software-management#1050
@xsuchy
Copy link
Member Author

xsuchy commented Apr 19, 2023

Updated.
I did not modified the eol/* configs. They will work even without modification. I hesitate to touch anything in eol/* (unless it is deleting operation).

@Conan-Kudo
Copy link
Member

Conan-Kudo commented Apr 19, 2023

I would say you should probably fix the EL5 configs and add an override there, but I don't care that much.

@praiskup praiskup merged commit 039145c into rpm-software-management:main May 2, 2023
14 checks passed
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.

/usr/sbin/useradd: invalid option -- 'n'
4 participants