Re: [PATCH setup] Add new option --chown-admin

[Christian Franke <Christian.Franke@t-online.de>]

Jon Turney wrote:
> On 23/08/2022 18:27, Christian Franke wrote:
>> Jon Turney wrote:
>>> On 12/07/2022 13:50, Jon Turney wrote:
>>>> [Replying to the right list this time...]
>>>> On 09/07/2022 13:21, Christian Franke wrote:
>>> [...]
>>>>>
>>>>> The UserSettings ctor has a somewhat hidden side effect which sets 
>>>>> root_scope correctly:
>>>>>
>>>>>   UserSettings::UserSettings(...);
>>>>>    open_settings("setup.rc", ...);
>>>>>     io_stream::open("cygfile:///etc/setup/setup.rc", ...);
>>>>> io_stream_cygfile::io_stream_cygfile("/etc/setup/setup.rc", ...);
>>>>>       get_root_dir_now();
>>>>>        read_mounts("");
>>>>>         read_mounts_nt("");
>>>>>          root_scope = isuser ? IDC_ROOT_USER : IDC_ROOT_SYSTEM;
>>>>>
>>>>> Conclusion: Regression introduced Feb 24, 2012 (befc9dd).
>>>>>
>>>>
>>>> Thanks for tracking this down.
>>>>
>>>> That just seems... fractally wrong.
>>>
>>> I kind of lost track of this.  Is there anything else needed to fix 
>>> the original problem here?  Or is it solved by the change to defer 
>>> setting the group until after root_scope is known?
>>
>> The group seems to be correctly set now.
>>
>> An old problem still remains: root_scope always ends up as 
>> IDC_ROOT_SYSTEM if setup is run elevated, regardless off GUI setting. 
>> Apply the temporary patch from here to see what happens:
>> https://sourceware.org/pipermail/cygwin-apps/2022-July/042151.html
>
> Ah, right, I remember now.
>
> I think having read_mounts() have a side effect of setting root_scope 
> is just nonsense now (it might have made some sense back in the day 
> when the mount table was also stored in the registry).
>
> So, how about the attached?
>
> (We should perhaps also set the installation root directory to 
> something other than C:\cygwin64 if a non-admin user, since they are 
> unlikely to be able to write there in current windows versions, but 
> that's difficult from a sequencing point of view in that dialog, and 
> for backwards compatibility)
>
>> Possibly difficult to fix, in particular in conjunction with later 
>> changes via [< Back] button. An easier approach: Remove the GUI 
>> setting and connect root_scope to -B option.
>
> Looking for that option is not correct, because we might be running 
> without -B, but from an already elevated shell.  I think checking 
> nt_sec.isRunAsAdmin() would be the correct test.

Did some quick tests with patch applied. LGTM.