Bug 24636

CC'ing tv and the gnome and mageia tools maintainers, I don't know whom to assign to (nor to which package).

CC: (none) => gnome, mageiatools, marja11, thierry.vignaud

What is the bug about? I understand the default umask differs between GNOME+X vs GNOME+Wayland session. However, if MCC requires specific permissions then MCC needs to set those permissions instead of relying on a specific umask. So is a fix wanted from MCC or is the intention that umask is same across GNOME sessions?

CC: (none) => olav

The bug is about making the Mageia tools work for our users, if they choose (or are forced) to use GNOME on Xorg, or choose a multi-DE install that includes GNOME. I don't really care how it's fixed, but I would tend to agree that it's rpmdrake's responsibility to set the correct permissions.

What I don't understand is why removing and then immediately re-adding the media sets the permissions correctly. If the umask is wrong, I'd expect the permissions to be wrong every time.

(In reply to Martin Whitaker from comment #3)
> The bug is about making the Mageia tools work for our users, if they choose
> (or are forced) to use GNOME on Xorg, or choose a multi-DE install that
> includes GNOME. I don't really care how it's fixed, but I would tend to
> agree that it's rpmdrake's responsibility to set the correct permissions.

Assinging to the Mageia tools maintainers, then :-)
> 
> What I don't understand is why removing and then immediately re-adding the
> media sets the permissions correctly. If the umask is wrong, I'd expect the
> permissions to be wrong every time.

Assignee: bugsquad => mageiatools
Source RPM: (none) => rpmdrake

*** Bug 24933 has been marked as a duplicate of this bug. ***

24933 is different, and it seems to be hardware related, in that 2 of my test laptops (hp 8510w and compaq c700) exhibit the issue, my toshiba and chromebooks do not.
the issue only seems to occur when adding on-line media from Mageia Welcome.
right clicking the mgaapplet to bring up the on-line media list and then closing it again is enough to fix the issue.

i am incorrect, 
adding from MCC as well also exhibits issue

what is the likelihood of creating the "final" .isos without GDM?

would still be available from repos as normal if required by usres

(In reply to ben mcmonagle from comment #8)
> what is the likelihood of creating the "final" .isos without GDM?

Can't do that - it is required by GNOME. Anyway, we would need it on the ISOs to support upgrades.

@Thierry, would you consider changing the umask when adding media an acceptable fix for this bug? I see that is already done when writing the urpmi.cfg file (which is why removing the media and re-adding them works (answering my comment #3).

(In reply to Martin Whitaker from comment #10)
> @Thierry, would you consider changing the umask when adding media an
> acceptable fix for this bug? I see that is already done when writing the
> urpmi.cfg file (which is why removing the media and re-adding them works
> (answering my comment #3).

I think we should fix this one before release...

@Martin, can you fix it up with umask... 

if tv wants it another way, we can do a follow-up fix later

CC: (none) => tmb
Priority: Normal => release_blocker

(In reply to Thomas Backlund from comment #11)
> I think we should fix this one before release...
> 
> @Martin, can you fix it up with umask... 

Will see what I can do.

OK, fixed in urpmi-8.117-1. I don't think this is a perfect fix, but it's good enough to allow mgaapplet to work.

*** Bug 24960 has been marked as a duplicate of this bug. ***

CC: (none) => tablackwell

(In reply to Martin Whitaker from comment #13)
> OK, fixed in urpmi-8.117-1. I don't think this is a perfect fix, but it's
> good enough to allow mgaapplet to work.

Isnt there a thinko here:
http://gitweb.mageia.org/software/rpm/urpmi/commit/?id=f8c0d00fa23a094173dbe17ef4644f860f871f20

first you do: my $old_umask = umask 0022;

then you call: umask $old_umask;

which if I read it correctly will translate to: "umask umask 0022;"

Was the intention to do:
diff --git a/urpm/media.pm b/urpm/media.pm
index 52575ff0..75267ac8 100644
--- a/urpm/media.pm
+++ b/urpm/media.pm
@@ -555,7 +555,8 @@ sub write_urpmi_cfg {
     #- a normal user won't be able to read it. We enforce umask here in the case
     #- where the msec security level is set to 'secure' (which means umask 077)
     #- or where we are run from a gdm-x-session (mga#24636)
-    my $old_umask = umask 0022;
+    my $old_umask = umask;
+    umask 0022;
     urpm::cfg::dump_config($urpm->{config}, $config)
        or $urpm->{fatal}(6, N("unable to write config file [%s]", $urpm->{config}));
     umask $old_umask;
@@ -978,7 +979,8 @@ sub add_medium {
     #- as a normal user won't be able to read them. We enforce umask here in the case
     #- where the msec security level is set to 'secure' (which means umask 077) or
     #- where we are run from a gdm-x-session (mga#24636)
-    my $old_umask = umask 0022;
+    my $old_umask = umask;
+    umask 0022;
     #- those files must not be there (cf mdvbz#36267)
     _clean_statedir_medium_files($urpm, $medium);
     if (!($options{virtual} && _local_file($medium))

(In reply to Thomas Backlund from comment #15)
> Isnt there a thinko here:
> http://gitweb.mageia.org/software/rpm/urpmi/commit/
> ?id=f8c0d00fa23a094173dbe17ef4644f860f871f20
> 
> first you do: my $old_umask = umask 0022;
> 
> then you call: umask $old_umask;
> 
> which if I read it correctly will translate to: "umask umask 0022;"

No, "umask" is a perl function that sets the new umask and returns the previous value.

If I'd done this from scratch, I would have written it as

  my $old_umask = umask(0022);

but I preserved the style of what was already there.

But unfortunately this still isn't quite enough to fix it. It works the first time you add media, but if you remove and re-add them, the downloaded synthesis files get the wrong permissions. That probably means that it would break when updating the media as well.

The simplest fix would be to set the umask when starting the various rpmdrake tools. Otherwise it could take me a while to fix this. WDYT?

Found a better way. Should be fixed in urpmi-8.118-1.

(In reply to Martin Whitaker from comment #17)
> (In reply to Thomas Backlund from comment #15)
> > Isnt there a thinko here:
> > http://gitweb.mageia.org/software/rpm/urpmi/commit/
> > ?id=f8c0d00fa23a094173dbe17ef4644f860f871f20
> > 
> > first you do: my $old_umask = umask 0022;
> > 
> > then you call: umask $old_umask;
> > 
> > which if I read it correctly will translate to: "umask umask 0022;"
> 
> No, "umask" is a perl function that sets the new umask and returns the
> previous value.
> 
> If I'd done this from scratch, I would have written it as
> 
>   my $old_umask = umask(0022);
> 
> but I preserved the style of what was already there.

Ah, ok.
Thanks for the explanation

Was fixed.

Resolution: (none) => FIXED
Status: NEW => RESOLVED