Re: [Bug] setup regression #2
On 20/11/2022 17:16, Jon Turney wrote: On 13/11/2022 12:47, Achim Gratz wrote: The problem is actually a more knotty than you seem to think: prominently ca-certificates and man-db get their knickers in a twist when the group during post-install is different from the group of the installed files and I suspect some other packages will run into similar problems depending on how fussy they are with the group permissions. I take it seriously, otherwise I would have just released a setup with those changes. I've reverted them for the moment, since I want to do a setup release now for other reasons. I've reapplied these changes in the latest RC, since I'm assuming the problem(s) here are resolved as of [1], but it's very difficult to tell. [1] https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=a5bcfe616c7e8f78f464bf045595d8213244876a
Re: [Bug] setup regression #2
Christian Franke writes:
> Anything installed with "All Users" option should IMO be protected
> against modifications by any regular non-elevated user.
Yes.
> This is not the case if the RID=513 group ("HOST\None",
> "DOMAIN\Domain-Users") is used. Many upstream projects install
> directories and files with permissions like 0664, 0775, 0660 or
> 0770. This is safe when the group is "root". On current Cygwin, all
> users have R/W access regardless of the "other" permission bits.
Correct. That's why I was hoping I could use a dedicated group (either
local or domain depending the install) for "Cygwin Administrators".
> Using the administrators group as discussed here would solve this but
> apparently introduces interesting new permission problems with some
> packages. Could these possibly be solved by the maintainers of the
> affected packages?
The problem is not the Administrators group per se AFAICT, but the change
from a different group to another mid-flight. If the group could be
specified as alluded to above, I can keep the "wrong" group for existing
installs until I get around to fix their group ownership and ensure that
any new installs can be administered by whatever group of people will be
responsible for keeping things running smoothly.
Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+
SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
Re: [Bug] setup regression #2
Jon Turney wrote:
On 20/11/2022 19:05, Achim Gratz wrote:
Jon Turney writes:
I believe that the intent of the code in setup is that there should
only be two modes:
USER: install "for me", with the users primary group
As I understand it, the intention here was that the user can have a
"single user installation" in a place that they have access to (say,
their home directory) while they have no permission in one of the usual
places. In a setup where that place is a certain type of share the user
will not be able to change the group the files are owned by anyway
(standard NetApp CIFS shares are set up this way) and it may not be the
users primary group.
SYSTEM: install "for everyone", with the administrators primary group
(only permitted if you are an administrator)
I don't see why the fact the installation is meant to be used by
multiple users means that the install must be owned by group
Administrators. I'm not sure this is a good idea on Windows anyway, at
least when you don't put extra (inheritable) DACL on the install
folder.
Christian,
Maybe you can offer your opinion here, since you seem to have the
opposite, or at least a different, point of view.
Anything installed with "All Users" option should IMO be protected
against modifications by any regular non-elevated user.
This is not the case if the RID=513 group ("HOST\None",
"DOMAIN\Domain-Users") is used. Many upstream projects install
directories and files with permissions like 0664, 0775, 0660 or 0770.
This is safe when the group is "root". On current Cygwin, all users have
R/W access regardless of the "other" permission bits.
Try for example: find /usr/share ! -type l -perm -020 -ls (~4000 hits on
my installation)
Using the administrators group as discussed here would solve this but
apparently introduces interesting new permission problems with some
packages. Could these possibly be solved by the maintainers of the
affected packages?
An alternative may be: Keep the group as is, but prevent using above
permissions as far as possible. For new packages, this could possibly be
done with an enhancement of cygport. But I'm sure that there will also
be subtle cases where these modified permissions break things. Cygport
could allow to opt-out then.
Ideally the protection should also be effective for the non-elevated
user who runs setup elevated. This is automatically the case for typical
installers because Windows changes TokenOwner to administrators group if
run elevated. That's why I provided
https://sourceware.org/pipermail/cygwin-apps/2022-August/042221.html
Re: [Bug] setup regression #2
On 20/11/2022 19:05, Achim Gratz wrote: Jon Turney writes: I believe that the intent of the code in setup is that there should only be two modes: USER: install "for me", with the users primary group As I understand it, the intention here was that the user can have a "single user installation" in a place that they have access to (say, their home directory) while they have no permission in one of the usual places. In a setup where that place is a certain type of share the user will not be able to change the group the files are owned by anyway (standard NetApp CIFS shares are set up this way) and it may not be the users primary group. SYSTEM: install "for everyone", with the administrators primary group (only permitted if you are an administrator) I don't see why the fact the installation is meant to be used by multiple users means that the install must be owned by group Administrators. I'm not sure this is a good idea on Windows anyway, at least when you don't put extra (inheritable) DACL on the install folder. Christian, Maybe you can offer your opinion here, since you seem to have the opposite, or at least a different, point of view. I've never tried installing into the usual place (%ProgramFiles%) as that means that Windows enforces a number of rules that are different from Cygwin's and change non-domain vs. in-domain machines, applied GPO etc. So I'd really just introduce another parameter to specify what the group the installer uses should be and have the default depend on whether the user doing the install has administrative rights or not. A warning should be issued when that group is different from the existing root directory and of course the whole install should abort if the requested group can't be made primary.
Re: [Bug] setup regression #2
On Nov 21 13:39, ASSI wrote:
> Corinna Vinschen writes:
> > The idea is that the installation tree has POSIXy permissions and
> > administrative users have the right to change stuff. The administrators
> > group is part of the user's token if the process has been started
> > elevated, so, to me, this looks like a natural choice.
>
> As I said, I haven't thought through the implications of doing that. We
> certainly haven't done a security audit or anything like that
> w.r.t. group ownership of the Cygwin tree and permission of the
> installed files.
>
> > The other advantage is that the administrators group has a fixed SID on
> > all systems, while other groups depend on the environment. That goes
> > for the local group "None" just as well as for the "Domain Users"
> > group, etc.
>
> Yeah, a local non-domain installation currently installs as "None"
> ("Kein" in german Windows) and domain ones will have "Domain Users"
...both groups using the same RID is no accident @ MSFT :)
Corinna
Re: [Bug] setup regression #2
Corinna Vinschen writes:
> The idea is that the installation tree has POSIXy permissions and
> administrative users have the right to change stuff. The administrators
> group is part of the user's token if the process has been started
> elevated, so, to me, this looks like a natural choice.
As I said, I haven't thought through the implications of doing that. We
certainly haven't done a security audit or anything like that
w.r.t. group ownership of the Cygwin tree and permission of the
installed files.
> The other advantage is that the administrators group has a fixed SID on
> all systems, while other groups depend on the environment. That goes
> for the local group "None" just as well as for the "Domain Users"
> group, etc.
Yeah, a local non-domain installation currently installs as "None"
("Kein" in german Windows) and domain ones will have "Domain Users"
> I'm not adamant about this, it was just what was looking like being the
> right thing to do at the time. Especially I was not hot to make the
> permission set more complicated than necessary for a POSIX-like system.
Agreed.
Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+
Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs
Re: [Bug] setup regression #2
On Nov 20 20:05, Achim Gratz wrote: > Jon Turney writes: > > I believe that the intent of the code in setup is that there should > > only be two modes: > > > > USER: install "for me", with the users primary group > > As I understand it, the intention here was that the user can have a > "single user installation" in a place that they have access to (say, > their home directory) while they have no permission in one of the usual > places. In a setup where that place is a certain type of share the user > will not be able to change the group the files are owned by anyway > (standard NetApp CIFS shares are set up this way) and it may not be the > users primary group. > > > SYSTEM: install "for everyone", with the administrators primary group > > (only permitted if you are an administrator) > > I don't see why the fact the installation is meant to be used by > multiple users means that the install must be owned by group > Administrators. I'm not sure this is a good idea on Windows anyway, at > least when you don't put extra (inheritable) DACL on the install > folder. The idea is that the installation tree has POSIXy permissions and administrative users have the right to change stuff. The administrators group is part of the user's token if the process has been started elevated, so, to me, this looks like a natural choice. The other advantage is that the administrators group has a fixed SID on all systems, while other groups depend on the environment. That goes for the local group "None" just as well as for the "Domain Users" group, etc. I'm not adamant about this, it was just what was looking like being the right thing to do at the time. Especially I was not hot to make the permission set more complicated than necessary for a POSIX-like system. Corinna
Re: [Bug] setup regression #2
Jon Turney writes: > I believe that the intent of the code in setup is that there should > only be two modes: > > USER: install "for me", with the users primary group As I understand it, the intention here was that the user can have a "single user installation" in a place that they have access to (say, their home directory) while they have no permission in one of the usual places. In a setup where that place is a certain type of share the user will not be able to change the group the files are owned by anyway (standard NetApp CIFS shares are set up this way) and it may not be the users primary group. > SYSTEM: install "for everyone", with the administrators primary group > (only permitted if you are an administrator) I don't see why the fact the installation is meant to be used by multiple users means that the install must be owned by group Administrators. I'm not sure this is a good idea on Windows anyway, at least when you don't put extra (inheritable) DACL on the install folder. I've never tried installing into the usual place (%ProgramFiles%) as that means that Windows enforces a number of rules that are different from Cygwin's and change non-domain vs. in-domain machines, applied GPO etc. So I'd really just introduce another parameter to specify what the group the installer uses should be and have the default depend on whether the user doing the install has administrative rights or not. A warning should be issued when that group is different from the existing root directory and of course the whole install should abort if the requested group can't be made primary. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Factory and User Sound Singles for Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#WaldorfSounds
Re: [Bug] setup regression #2
On 13/11/2022 12:47, Achim Gratz wrote: The problem is actually a more knotty than you seem to think: prominently ca-certificates and man-db get their knickers in a twist when the group during post-install is different from the group of the installed files and I suspect some other packages will run into similar problems depending on how fussy they are with the group permissions. I take it seriously, otherwise I would have just released a setup with those changes. I've reverted them for the moment, since I want to do a setup release now for other reasons. Taking a step back: I believe that the intent of the code in setup is that there should only be two modes: USER: install "for me", with the users primary group SYSTEM: install "for everyone", with the administrators primary group (only permitted if you are an administrator) But in fact, this got broken this long ago, so we do something slightly different (Please take a look at the discussion around [1]). If the intention isn't adequate, can you describe what is needed? [1] https://cygwin.com/pipermail/cygwin-apps/2022-July/042144.html
Re: [Bug] setup regression #2
Jon Turney writes: > On 08/10/2022 17:56, Achim Gratz wrote: >> I think that setup was essentially treating the install as "for this >> user only" since it was created and maintained by a script that can't >> affect that option and the fact it was also in group Adminsitroators >> didn't actually register until now. > > Yeah, that seems possible, since some of these changes fix what are > arguably bugs in how that works (i.e. I suspect that previously, even > when elevated, if only the registry key > HKEY_CURRENT_USER\\Software\\Cygwin\\setup\rootdir exists (and not the > same key under HKLM), we're going to install for "Just Me", > irrespective of what the UI says) I've checked some old logs and even though the install was identified as "system", there was no line "Changing gid to Administrators" for the main install until setup version 2.921. > I wrote some code for this option (attached), but I have a hard time > seeing how it's functionally different from using '-B/'--no-admin'. This option does nothing to prevent the use of Administrator group when the install is identified as "system" and those rights are actually available (which they are as the scripting needs those rights in other places). > So, I guess a question is, does running with that option work as > expected in your problematic instance? No, it does not, see above. The problem is actually a more knotty than you seem to think: prominently ca-certificates and man-db get their knickers in a twist when the group during post-install is different from the group of the installed files and I suspect some other packages will run into similar problems depending on how fussy they are with the group permissions. The symptom is that you see failures from chmod (for whatever reason "Invalid argument") when these programs try to swap the existing with the newly gerenated (temporary) files. In the case of man-db that results in the /var/cache/man/index.db file getting removed (and depending on the version the PID temporaries getting left in place), for update-ca-trust the mkstemp temporaries will be left over and the original files left in place. So all installs from before the change to setup are affected if the installation wasn't done via the GUI at least. I think it would be best to have an option to directly specify a desired group for both the installed files and running the post-install (which already must be in the user token). The default should be the primary group of the user doing the installation. I don't think the installation should be group-owned by "Administrators" on Windows. If anything it makes it much more difficult to administer the installation from within Cygwin as there doesn't seem to be a way to change to a different than the primary group for domain accounts yet. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptations for KORG EX-800 and Poly-800MkII V0.9: http://Synth.Stromeko.net/Downloads.html#KorgSDada
Re: [Bug] setup regression #2
Jon Turney writes: > On 08/10/2022 17:56, Achim Gratz wrote: >> I think that setup was essentially treating the install as "for this >> user only" since it was created and maintained by a script that can't >> affect that option and the fact it was also in group Adminsitroators >> didn't actually register until now. > > Yeah, that seems possible, since some of these changes fix what are > arguably bugs in how that works (i.e. I suspect that previously, even > when elevated, if only the registry key > HKEY_CURRENT_USER\\Software\\Cygwin\\setup\rootdir exists (and not the > same key under HKLM), we're going to install for "Just Me", > irrespective of what the UI says) I haven't checked that. >> The DACL on the server install changed from conferring access to "Everyone" >> to >> just the install user and SYSTEM IIRC. It doesn't do that on the >> (non-domain) build machine at home that runs Win10 Pro. > > That makes less sense to me. We should always creating an entry in > the DACL for 'Everyone' to hold the POSIX permissions for 'other' > users. Well, it wasn't there and whichever way it ended up that way, it was an inconvenient enough fix that I don'*t want to try it again on a productive system. > I wrote some code for this option (attached), but I have a hard time > seeing how it's functionally different from using '-B/'--no-admin'. > > So, I guess a question is, does running with that option work as > expected in your problematic instance? I'm not having enough time for checking right now, but I might test this hypothesis later on. Regard, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptation for Waldorf Blofeld V1.15B11: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
Re: [Bug] setup regression #2
On 08/10/2022 17:56, Achim Gratz wrote:
I think that setup was essentially treating the install as "for this
user only" since it was created and maintained by a script that can't
affect that option and the fact it was also in group Adminsitroators
didn't actually register until now.
Yeah, that seems possible, since some of these changes fix what are
arguably bugs in how that works (i.e. I suspect that previously, even
when elevated, if only the registry key
HKEY_CURRENT_USER\\Software\\Cygwin\\setup\rootdir exists (and not the
same key under HKLM), we're going to install for "Just Me", irrespective
of what the UI says)
The DACL on the server install changed from conferring access to "Everyone" to
just the install user and SYSTEM IIRC. It doesn't do that on the
(non-domain) build machine at home that runs Win10 Pro.
That makes less sense to me. We should always creating an entry in the
DACL for 'Everyone' to hold the POSIX permissions for 'other' users.
(See win32.cc:NTSecurity::GetPosixPerms() which translates a file mode
to a SD)
>> As long as there's an option to force it to keep the former behaviour
>> things should be OK, but I haven't really checked if and how this is
>> possible.
>
> Unfortunately, there is no such option.
I wrote some code for this option (attached), but I have a hard time
seeing how it's functionally different from using '-B/'--no-admin'.
So, I guess a question is, does running with that option work as
expected in your problematic instance?
From ae547f5b4b4421bf9b7b9f204eb3d303cc6b2673 Mon Sep 17 00:00:00 2001
From: Jon Turney
Date: Wed, 2 Nov 2022 22:46:29 +
Subject: [PATCH setup] Add an option to not make files group owned by
Adminstrators
Add an option that, when elevated, do not make files group owned by
Adminstrators (i.e use the primary group of the user running setup
instead).
Fixes: 495b0148
---
res.pot | 8 ++--
res/en/res.rc | 1 +
resource.h| 1 +
root.cc | 7 ++-
4 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/res.pot b/res.pot
index 64079c8..e84c34c 100644
--- a/res.pot
+++ b/res.pot
@@ -3,7 +3,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2022-08-27 12:54+0100\n"
+"POT-Creation-Date: 2022-11-08 14:36+0100\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME \n"
"Language-Team: LANGUAGE \n"
@@ -11,7 +11,7 @@ msgstr ""
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"
"X-Accelerator-Marker: &\n"
-"X-Generator: Translate Toolkit 3.7.0\n"
+"X-Generator: Translate Toolkit 3.7.3\n"
"X-Merge-On: location\n"
#: DIALOG.IDD_SOURCE.CAPTION
@@ -1245,6 +1245,10 @@ msgstr ""
msgid "Disable creation of desktop shortcut"
msgstr ""
+#: STRINGTABLE.IDS_HELPTEXT_NO_GROUP_CHANGE
+msgid "When elevated, do not make files group owned by Adminstrators"
+msgstr ""
+
#: STRINGTABLE.IDS_HELPTEXT_NO_REPLACEONREBOOT
msgid "Disable replacing in-use files on next reboot"
msgstr ""
diff --git a/res/en/res.rc b/res/en/res.rc
index ef5e8b1..dad5c47 100644
--- a/res/en/res.rc
+++ b/res/en/res.rc
@@ -683,6 +683,7 @@ BEGIN
IDS_HELPTEXT_MIRROR_MODE "Skip package availability check when installing
from local directory (requires local directory to be clean mirror!)"
IDS_HELPTEXT_NO_ADMIN "Do not check for and enforce running as
Administrator"
IDS_HELPTEXT_NO_DESKTOP "Disable creation of desktop shortcut"
+IDS_HELPTEXT_NO_GROUP_CHANGE "When elevated, do not make files group owned
by Adminstrators"
IDS_HELPTEXT_NO_REPLACEONREBOOT "Disable replacing in-use files on next
reboot"
IDS_HELPTEXT_NO_SHORTCUTS "Disable creation of desktop and start menu
shortcuts"
IDS_HELPTEXT_NO_STARTMENU "Disable creation of start menu shortcut"
diff --git a/resource.h b/resource.h
index cfe860b..917534f 100644
--- a/resource.h
+++ b/resource.h
@@ -157,6 +157,7 @@
#define IDS_HELPTEXT_HEADER 1546
#define IDS_HELPTEXT_FOOTER 1547
#define IDS_HELPTEXT_NO_WRITE_REGISTRY 1548
+#define IDS_HELPTEXT_NO_GROUP_CHANGE 1549
// Dialogs
diff --git a/root.cc b/root.cc
index ccbd6ae..f81c5c9 100644
--- a/root.cc
+++ b/root.cc
@@ -37,8 +37,10 @@
#include "propsheet.h"
#include "getopt++/StringOption.h"
+#include "getopt++/BoolOption.h"
StringOption RootOption ("", 'R', "root", IDS_HELPTEXT_ROOT, false);
+static BoolOption NoGroupChangeOption (false, '\0', "no-group-change",
IDS_HELPTEXT_NO_GROUP_CHANGE);
static ControlAdjuster::ControlInfo RootControlsInfo[] = {
{ IDC_ROOTDIR_GRP, CP_STRETCH, CP_TOP },
@@ -310,7 +312,10 @@ RootPage::OnNext ()
<< (root_scope == IDC_ROOT_USER ? " user" : " system") << endLog;
if (root_scope == IDC_ROOT_SYSTEM)
-nt_sec.setAdminGroup ();
+{
+ if (!NoGroupChangeOption)
+nt_sec.setAdminGroup ();
+}
else
nt_sec.resetPrimaryGroup ();
--
2.38.1
Re: [Bug] setup regression #2
Jon Turney writes: > On 03/10/2022 20:23, Achim Gratz wrote: >> Jon Turney writes: >>> This problem is with files created by setup, or by post-install scripts? >> I think both, although the problematic symlinks were created through >> alternatives. > > That's pretty baffling. Even more baffling is that I have another installation that are completely fine even with their Group now switched to Administrators. The one syxstem where I've had the problems is a server version and might have some GPO that affect thing that an admin user does. > I don't see how any of those commits would change the ownership of > files created by setup itself. The ownership is still with the user that runs the install script, however the group has changed. > The only relevant change seems to be in "Defer setting group until > after All Users/Just For Me is chosen", I've made > nt_sec.resetPrimaryGroup() explicit, but that only happens for > non-admin installs... I think that setup was essentially treating the install as "for this user only" since it was created and maintained by a script that can't affect that option and the fact it was also in group Adminsitroators didn't actually register until now. The DACL on the server install changed from conferring access to "Everyone" to just the install user and SYSTEM IIRC. It doesn't do that on the (non-domain) build machine at home that runs Win10 Pro. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Wavetables for the Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables
Re: [Bug] setup regression #2
On 03/10/2022 20:23, Achim Gratz wrote: Jon Turney writes: This problem is with files created by setup, or by post-install scripts? I think both, although the problematic symlinks were created through alternatives. That's pretty baffling. I don't see how any of those commits would change the ownership of files created by setup itself. The only relevant change seems to be in "Defer setting group until after All Users/Just For Me is chosen", I've made nt_sec.resetPrimaryGroup() explicit, but that only happens for non-admin installs... (I'm not sure how these commits could have caused the former, if the latter then reverting 45d8e84e "Drop group change while running postinstall scripts" would be the thing to try...) As I said, I don't understand it either. It seems my installations were all using the primary group for the account that does the install (which does have administrative rights and is separate from my normal user account on most machines). The primary group is either "None" for the build machine that only uses local accounts and is not a member of any domain or "Domain Users" otherwise. The new code uses "Administrators", but that seems to have the side effect of denying "normal" users access to the installation and instead weaves in extra DACL for SYSTEM. As long as there's an option to force it to keep the former behaviour things should be OK, but I haven't really checked if and how this is possible. Unfortunately, there is no such option.
Re: [Bug] setup regression
Achim Gratz writes: >> Can you confirm if this fixes package selection for you? > > I'll have to setup a test installation to check this. I am currently > short on time, but I will try to wedge it in somehow. I wasn't able to do an exhaustive test, but the fix seems to be effective for the two scenarios I've tried (I've confirmed that both fail without the fix present). Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Wavetables for the Terratec KOMPLEXER: http://Synth.Stromeko.net/Downloads.html#KomplexerWaves
Re: [Bug] setup regression
Jon Turney wrote:
On 27/09/2022 14:51, Christian Franke wrote:
Christian Franke wrote:
...
I made the false assumption that default_version=empty in
set_action() always implies that the default version is not
accessible. This is not the case for packages selected for
installation before chooser is visible.
I'm working on a new fix for the "Ctrl+I pressed but current version
is not accessible" case. ...
See attached patch. It also fixes the same problem for the "Category"
view. Testing shows that the problem only affects the display of the
version number as the solver later silently removes such install
requests.
The correct logic is already in toggle_action(): Install the most
recent accessible non-test ('naively_preferred') version.
I dropped this idea and aligned Ctrl+I behavior with "Install" from
"Category" view instead. Toggle_action() behaves different in such
corner cases as it always installs something.
Thanks, I applied both patches.
Do you have a simple way of causing a non-accessible package for
testing with?
Unfortunately there is only a simple one if the current installation and
cache directory matches some conditions. The "Ignore install requests if
version is not accessible" case appears if:
- "Install from Local Directory" is selected.
- A previous version of a no longer installed package is still in the
cache directory. Otherwise the package would not appear in the chooser.
- The current version of this package is not in the cache directory. The
"Size" column shows "?" for such packages.
Without commit 63a2c90, Ctrl+I or "Install" from "Category" view shows
an empty version in "New" column. The "Install" from "Category" case
could also be reproduced with setup 2.919.
With this commit, the Install request does nothing.
Re: [Bug] setup regression #2
Jon Turney writes: > This problem is with files created by setup, or by post-install scripts? I think both, although the problematic symlinks were created through alternatives. > (I'm not sure how these commits could have caused the former, if the > latter then reverting 45d8e84e "Drop group change while running > postinstall scripts" would be the thing to try...) As I said, I don't understand it either. It seems my installations were all using the primary group for the account that does the install (which does have administrative rights and is separate from my normal user account on most machines). The primary group is either "None" for the build machine that only uses local accounts and is not a member of any domain or "Domain Users" otherwise. The new code uses "Administrators", but that seems to have the side effect of denying "normal" users access to the installation and instead weaves in extra DACL for SYSTEM. As long as there's an option to force it to keep the former behaviour things should be OK, but I haven't really checked if and how this is possible. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptations for KORG EX-800 and Poly-800MkII V0.9: http://Synth.Stromeko.net/Downloads.html#KorgSDada
Re: [Bug] setup regression
Jon Turney writes: > Achim, > > Can you confirm if this fixes package selection for you? I'll have to setup a test installation to check this. I am currently short on time, but I will try to wedge it in somehow. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptation for Waldorf rackAttack V1.04R1: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
Re: [Bug] setup regression #2
On 22/09/2022 18:14, Achim Gratz wrote: The release_2.91 comes with another regression that still puzzles me. In a nutshell, the three commits that deal with setting up the groups during / after installation 2022-08-27 Jon Turney Drop setting root_scope as a side-effect of read_mounts() 2022-08-16 Jon Turney Defer setting group until after All Users/Just For... 2022-08-16 Jon Turney Drop group change while running postinstall scripts break existing installs in certain circumstances that are not yet completely clear. The server installation @work (which uses exactly the same scripting around the installation that I use for my build system @home) changed from using the "Domain Users" group to "Administrators". Additionally the previous access for "Everyone" has been removed and instead SYSTEM is now part of the (Windows) ACL. In effect certain files have become inaccessible to the normal users (unless they are aso Administrators), in particular (this is the part that I still don't understand) newly created symlinks can't be read by a normal user even when the target is fully accessible. Even doing an ls on such a symlink gets a "permission denied". This problem is with files created by setup, or by post-install scripts? (I'm not sure how these commits could have caused the former, if the latter then reverting 45d8e84e "Drop group change while running postinstall scripts" would be the thing to try...)
Re: [Bug] setup regression
On 27/09/2022 14:51, Christian Franke wrote:
Christian Franke wrote:
...
I made the false assumption that default_version=empty in set_action()
always implies that the default version is not accessible. This is not
the case for packages selected for installation before chooser is
visible.
I'm working on a new fix for the "Ctrl+I pressed but current version
is not accessible" case. ...
See attached patch. It also fixes the same problem for the "Category"
view. Testing shows that the problem only affects the display of the
version number as the solver later silently removes such install requests.
The correct logic is already in toggle_action(): Install the most
recent accessible non-test ('naively_preferred') version.
I dropped this idea and aligned Ctrl+I behavior with "Install" from
"Category" view instead. Toggle_action() behaves different in such
corner cases as it always installs something.
Thanks, I applied both patches.
Do you have a simple way of causing a non-accessible package for testing
with?
Achim,
Can you confirm if this fixes package selection for you?
Re: [Bug] setup regression
Christian Franke wrote:
...
I made the false assumption that default_version=empty in set_action()
always implies that the default version is not accessible. This is not
the case for packages selected for installation before chooser is
visible.
I'm working on a new fix for the "Ctrl+I pressed but current version
is not accessible" case. ...
See attached patch. It also fixes the same problem for the "Category"
view. Testing shows that the problem only affects the display of the
version number as the solver later silently removes such install requests.
The correct logic is already in toggle_action(): Install the most
recent accessible non-test ('naively_preferred') version.
I dropped this idea and aligned Ctrl+I behavior with "Install" from
"Category" view instead. Toggle_action() behaves different in such
corner cases as it always installs something.
From 7e1efd346e35898e3486fb84cf25b3885a2a16ec Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Tue, 27 Sep 2022 15:41:06 +0200
Subject: [PATCH] Ignore install requests if version is not accessible
This avoids that an empty "New" version is shown when install is
requested via "Install" from "Category" view or Ctrl+I and the
version is not accessible.
---
package_meta.cc | 12
1 file changed, 12 insertions(+)
diff --git a/package_meta.cc b/package_meta.cc
index 05b8946..ebfc947 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -539,6 +539,17 @@ packagemeta::select_action (int id, trusts const deftrust)
{
if (id == packagemeta::NoChange_action)
set_action((packagemeta::_actions)id, installed);
+ else if (id == Install_action)
+ {
+ // Ignore install request if the default version is not accessible.
+ // This assumes that all available versions are already known.
+ // This is not always the case when set_action is called directly.
+ packageversion v = trustp (true, deftrust);
+ if (v.accessible ())
+ set_action(Install_action, v, true);
+ else
+ set_action(NoChange_action, installed);
+ }
else
set_action((packagemeta::_actions)id, trustp (true, deftrust), true);
}
@@ -627,6 +638,7 @@ packagemeta::set_action (_actions action, packageversion
const _version,
else if (action == Install_action)
{
desired = default_version;
+ // If desired is empty, it will be set to the solver's preferred version
later.
if (desired)
{
if (desired != installed)
--
2.37.2
Re: [Bug] setup regression
Jon Turney wrote:
On 22/09/2022 17:56, Achim Gratz wrote:
Achim Gratz writes:
Achim Gratz writes:
I had updated setup to 2.921 recently, so I rolled it back to 2.920
and
this version does the package selection correctly. I haven't yet
looked
what commit is responsible, but whatever the cause of the
regression is
still in 2.922 as well.
The most likely change responsible for this is the additions in
package_meta.cc in commit c99e4c14911181636892355a4f1855024051ea1d. I
might not be able to check this tomorrow, though I'll try to free up
some time for that.
That was indeed the culprit. I've reverted just these two hunks on top
of release_2.922 and things worked again.
Yes, looking again at that change, the first hunk in package_meta.cc,
changing Install_action doesn't look right.
Indeed and this should be removed ASAP - patch attached.
Thanks for catching and sorry for the regression.
If I remember correctly action=Install_action, desired=empty package
version (evaluating as a boolean is false) means "install the solver's
preferred version", so converting that to NoChange_action seems wrong.
I'm kind of confused how to reproduce this, or why it decided to
install only some things, rather than nothing.
Christian,
From your reply to
https://cygwin.com/pipermail/cygwin-apps/2022-August/042212.html, it
seems this change is meant to handle the case where 'I' is pressed but
the package isn't accessible? Although I don't seem quite how.
I made the false assumption that default_version=empty in set_action()
always implies that the default version is not accessible. This is not
the case for packages selected for installation before chooser is visible.
I'm working on a new fix for the "Ctrl+I pressed but current version is
not accessible" case. The correct logic is already in toggle_action():
Install the most recent accessible non-test ('naively_preferred') version.
From 54665f0596f8ca50eff99ec8ec35970dc5fd439d Mon Sep 17 00:00:00 2001
From: Christian Franke
Date: Mon, 26 Sep 2022 16:47:42 +0200
Subject: [PATCH] Fix ignored install requests added before run of solver
Regression was introduced by commit c99e4c1.
---
package_meta.cc | 7 ---
1 file changed, 7 deletions(-)
diff --git a/package_meta.cc b/package_meta.cc
index a5dc436..05b8946 100644
--- a/package_meta.cc
+++ b/package_meta.cc
@@ -651,13 +651,6 @@ packagemeta::set_action (_actions action, packageversion
const _version,
srcpick (false);
}
}
- else
- {
- action = NoChange_action;
- desired = installed;
- pick (false);
- srcpick (false);
- }
}
else if (action == Reinstall_action)
{
--
2.37.2
Re: [Bug] setup regression
On 22/09/2022 17:56, Achim Gratz wrote: Achim Gratz writes: Achim Gratz writes: I had updated setup to 2.921 recently, so I rolled it back to 2.920 and this version does the package selection correctly. I haven't yet looked what commit is responsible, but whatever the cause of the regression is still in 2.922 as well. The most likely change responsible for this is the additions in package_meta.cc in commit c99e4c14911181636892355a4f1855024051ea1d. I might not be able to check this tomorrow, though I'll try to free up some time for that. That was indeed the culprit. I've reverted just these two hunks on top of release_2.922 and things worked again. Yes, looking again at that change, the first hunk in package_meta.cc, changing Install_action doesn't look right. If I remember correctly action=Install_action, desired=empty package version (evaluating as a boolean is false) means "install the solver's preferred version", so converting that to NoChange_action seems wrong. I'm kind of confused how to reproduce this, or why it decided to install only some things, rather than nothing. Christian, From your reply to https://cygwin.com/pipermail/cygwin-apps/2022-August/042212.html, it seems this change is meant to handle the case where 'I' is pressed but the package isn't accessible? Although I don't seem quite how.
Re: [Bug] setup regression
Achim Gratz writes: > Achim Gratz writes: >> I had updated setup to 2.921 recently, so I rolled it back to 2.920 and >> this version does the package selection correctly. I haven't yet looked >> what commit is responsible, but whatever the cause of the regression is >> still in 2.922 as well. > > The most likely change responsible for this is the additions in > package_meta.cc in commit c99e4c14911181636892355a4f1855024051ea1d. I > might not be able to check this tomorrow, though I'll try to free up > some time for that. That was indeed the culprit. I've reverted just these two hunks on top of release_2.922 and things worked again. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptations for KORG EX-800 and Poly-800MkII V0.9: http://Synth.Stromeko.net/Downloads.html#KorgSDada
Re: [Bug] setup regression
Achim Gratz writes: > I had updated setup to 2.921 recently, so I rolled it back to 2.920 and > this version does the package selection correctly. I haven't yet looked > what commit is responsible, but whatever the cause of the regression is > still in 2.922 as well. The most likely change responsible for this is the additions in package_meta.cc in commit c99e4c14911181636892355a4f1855024051ea1d. I might not be able to check this tomorrow, though I'll try to free up some time for that. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptation for Waldorf rackAttack V1.04R1: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
