Page MenuHomeFreeBSD
Differential D59150

bsd.init.mk: always define _gid when MK_INSTALL_AS_USER is set
ClosedPublic
Actions

Authored by fuz on Mon, Aug 24, 6:17 PM.
Tags
None
Referenced Files
F171442256: D59150.id184918.diff
Fri, Sep 11, 3:55 AM
Unknown Object (File)
Thu, Sep 10, 10:55 PM
Unknown Object (File)
Wed, Sep 9, 7:54 PM
Unknown Object (File)
Tue, Sep 8, 6:46 AM
Unknown Object (File)
Tue, Sep 8, 1:42 AM
Unknown Object (File)
Mon, Sep 7, 11:53 PM
Unknown Object (File)
Mon, Sep 7, 9:12 PM
Unknown Object (File)
Mon, Sep 7, 6:13 PM
View All 64 Files
Subscribers
emaste

Details

Reviewers
imp
vangyzen
bdrewery
Commits
rG9496457f230f: bsd.init.mk: always define _gid when MK_INSTALL_AS_USER is set
Summary

A recent commit started to use _uid and _gid in <bsd.dirs.mk> to
mangle the user and group for newly installed directories when
MK_INSTALL_AS_USER is set. However, _gid was previously only set
when _uid was not 0, causing the group to be set to the empty
string.

Set _uid and _gid together to avoid this problem.

Fixes: 541e6e2d516b6c9d3681b24464e9ef53c1f2579a
PR: 297841
Reported by: Ralph Zitz <ralph@zitz.dk>
MFC after: 1 week

Test Plan

port security/unix-selfauth-helper builds fine as root following this change.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 76055
Build 72938: arc lint + arc unit

Event Timeline

fuz created this revision.Mon, Aug 24, 6:17 PM
Herald added a subscriber: emaste. · View Herald TranscriptMon, Aug 24, 6:17 PM
Harbormaster completed remote builds in B76055: Diff 184918.Mon, Aug 24, 6:17 PM
fuz requested review of this revision.Mon, Aug 24, 6:17 PM
emaste added a comment.Mon, Aug 24, 6:22 PM

Looks fine, but shouldn't this and bsd.dirs.mk have the same (${_uid} != 0) logic?

fuz added a comment.Mon, Aug 24, 6:31 PM

@emaste Sounds like a reasonable idea. I'll wait for @imp to decide as he came up with the logic in the first place.

imp accepted this revision.Mon, Aug 24, 10:26 PM

I have this in my tree too, but I've not had time to test it out. If you do, then I think it's the right thing :)

This revision is now accepted and ready to land.Mon, Aug 24, 10:26 PM
Closed by commit rG9496457f230f: bsd.init.mk: always define _gid when MK_INSTALL_AS_USER is set (authored by fuz). · Explain WhyMon, Aug 24, 11:01 PM
This revision was automatically updated to reflect the committed changes.
fuz added a commit: rG9496457f230f: bsd.init.mk: always define _gid when MK_INSTALL_AS_USER is set.
emaste added a comment.Mon, Aug 24, 11:40 PM

Any comment on the other question @imp? I.e., why don't we use .if ${_uid} != 0 in bsd.dirs.mk also?

imp added a comment.Tue, Aug 25, 12:28 AM
In D59150#1356945, @emaste wrote:

Any comment on the other question @imp? I.e., why don't we use .if ${_uid} != 0 in bsd.dirs.mk also?

Both should always be defined. I didn't see that one had better behavior (or different) than the other.

fuz added a comment.Tue, Aug 25, 8:43 AM
In D59150#1356946, @imp wrote:
In D59150#1356945, @emaste wrote:

Any comment on the other question @imp? I.e., why don't we use .if ${_uid} != 0 in bsd.dirs.mk also?

Both should always be defined. I didn't see that one had better behavior (or different) than the other.

I think @emaste's question is more about whether bsd.dirs.mk should also special case ${_uid} == 0 and then not change the uid and gid of installed directories when you are root but supply MK_INSTALL_AS_USER as we do in bsd.init.mk.

imp added a comment.Tue, Aug 25, 6:53 PM
In D59150#1357225, @fuz wrote:
In D59150#1356946, @imp wrote:
In D59150#1356945, @emaste wrote:

Any comment on the other question @imp? I.e., why don't we use .if ${_uid} != 0 in bsd.dirs.mk also?

Both should always be defined. I didn't see that one had better behavior (or different) than the other.

I think @emaste's question is more about whether bsd.dirs.mk should also special case ${_uid} == 0 and then not change the uid and gid of installed directories when you are root but supply MK_INSTALL_AS_USER as we do in bsd.init.mk.

It's a more orthogonal design, but I do see the point, despite it not being name MK_INSTALL_AS_USER_UNLESS_ROOT... I'm honestly not sure why we have this check here, honestly, and don't think it should be both places.

Revision Contents
Changeset List

PathSize
share/
mk/
2 lines

Diff 184918

View Options

share/mk/bsd.init.mk

Loading...