Created attachment 1220 [details]
example .spec

Created attachment 1221 [details]
patch to resolve it

For now I only care about man pages... and it works. Tested!

Hi, thanks for reporting this bug.
Assigned to the package maintainer.

Keywords: (none) => Triaged
Assignee: bugsquad => boklm

Hi,

This bug was filed against cauldron, but we do not have cauldron at the moment.

Please report whether this bug is still valid for Mageia 2.

Thanks :)

Cheers,
marja

Keywords: (none) => NEEDINFO

@ Nicolas

If you agree with this request and are one day going to work on it: please put "OK" on the whiteboard or set status to ASSIGNED.

If you don't agree with the assignment (AFAICS it is nobody's package), please assign back to bugsquad

If you think it should be closed (wontfix, fixed), please close

Keywords: NEEDINFO => (none)
CC: (none) => marja11

Still valid

Should be "next if $withman && !$allname && $l =~ /%dir/;" instead

Anyway performing a somewhat costly regexp is uneeded since %dir is here only if $finallist{$f}{dir} is set.

So could be the following, done before computing $l:
"next if $withman && !$allname && $finallist{$f}{dir};"

(In reply to Thierry Vignaud from comment #7)
> 
> So could be the following, done before computing $l:
> "next if $withman && !$allname && $finallist{$f}{dir};"

Thierry, IIUC, you suggested the following patch.

--- find-lang.pl        (révision 7427)
+++ find-lang.pl        (copie de travail)
@@ -73,6 +73,7 @@
 
 foreach my $f (sort keys %finallist) {
     my ($lang, @otherlang) = keys %{ $finallist{$f}{lang} || {} };
+    next if $withman && !$allname && $finallist{$f}{dir};
     my $l = sprintf("%s%s%s",
         $finallist{$f}{dir} ? '%dir ' : '',
         @otherlang == 0 && $lang && $lang ne 'C'

I tested this patch (in order to fix bug #9055), and rebuilt locally 2 packages (amule & dcraw) from the affected packages, and it fixes this bug.

Can we push this fix?

CC: (none) => lmenut

Hi everyone, what is the situation now? This seems ok to push? This has been pushed? This bug is linked with 9055.

CC: (none) => pierre-malo.denielou

I applied the patch suggested by Thierry in comment 7 in rpm-mageia-setup 1.168.

Sadly, I've just discovered that it introduces a regression when --with-man is used in combination with another --with-xxxx; all the directories found by find-lang are skipped, so that other directories than man directories are now not owned by the packages.

eg. with dia that uses
  %{find_lang} %{name} --with-gnome --with-man

when rebuild with rpm-mageia-setup 1.168, the following help directories found by --with-gnome are now not owned in dia-0.97.2-7.mga3:
/usr/share/gnome/help/dia/C
/usr/share/gnome/help/dia/C/graphics
/usr/share/gnome/help/dia/eu
/usr/share/gnome/help/dia/eu/graphics
/usr/share/gnome/help/dia/fr
/usr/share/gnome/help/dia/fr/graphics
/usr/share/gnome/help/dia/pl
/usr/share/gnome/help/dia/pl/graphics

Thierry, this regression is quite annoying, and I don't have enough skills in perl and in all this part to help here and provide a better fix.

Is there a way to skip only man directories?

If we can fix this without regressions, I think that I will have to revert commit 7582 and restore the old behaviour.
http://svnweb.mageia.org/soft?view=revision&revision=7582

(In reply to Luc Menut from comment #10)
> If we can fix this without regressions, I think that I will have to revert
> commit 7582 and restore the old behaviour.
> http://svnweb.mageia.org/soft?view=revision&revision=7582

Absolutely not.  While it's unfortunate that a few orphan directories might be left around when something's uninstalled, that's a much smaller problem than causing rpm to choke when installing something due to directory "conflicts."  Pre-rpm 4.11 I would have agreed with you, but now this fix should not be reverted.

CC: (none) => luigiwalser

(In reply to David Walser from comment #11)
> 
> Absolutely not.  While it's unfortunate that a few orphan directories might
> be left around when something's uninstalled, that's a much smaller problem
> than causing rpm to choke when installing something due to directory
> "conflicts."  Pre-rpm 4.11 I would have agreed with you, but now this fix
> should not be reverted.

Now that you have fixed man-pages-%lang packages (man directories are owned by root), we shouldn't have perms conflicts if I revert commit 7582.
Orphan directories left around after uninstall isn't the only issue with installed directories unowned by packages; eg. more annoying, this prevents to detect directory<->file/link replacement on these unowned directories.

Wrong.  I've only fixed the packages in the distribution.  If there are any third-party packages that people want to use that also own those directories, there's still potential for conflicts.  We should not have packages owning directories that they shouldn't be owning.  This has always been the case ideally, but it's much more important now because of the rpm 4.11 behavior.  This fix should not be reverted.

Furthermore, these particular directories should never be symlinks, so that's not an issue.  Also, even if hypothetically that could happen, bash's test command doesn't care if something is owned by a package or not, so this in no way prevents detecting that anyway.

If we can replace the fix with a better one, we should.  I'm just saying we shouldn't revert this if we don't have another fix to replace it with.

It seems to me the better fix would involve a change around line 60 so that the regular expression matches only files and not directories.

Because of line 38, it should only be looking at files anyway.  I'm guessing the parent_to_own on line 64 makes it own the parent directories of the file as well.  Maybe that should just be changed to use own_file like on line 43.

(In reply to David Walser from comment #13)
> Wrong.  I've only fixed the packages in the distribution.  If there are any
> third-party packages that people want to use that also own those
> directories, there's still potential for conflicts.

nope, third-party packages will conflict with man-pages-%lang packages too if they own man directories with perms other than root:root. If they own man directories with root:root, they will not conflict with directories owned through find_lang.

> We should not have
> packages owning directories that they shouldn't be owning. 

and packages should own ALL directories that they install.
We have lot of directories owned by multiple packages elsewhere; there isn't problem if they have the same perms. It's the case now with man directories since the you change the group to root.
Look at the list https://bugs.mageia.org/attachment.cgi?id=3451, the only conflict was on group root <-> man.

(In reply to David Walser from comment #14)
> If we can replace the fix with a better one, we should. 

I've asked for a better fix, but no one provides a better patch or commit a fix.

> I'm just saying we
> shouldn't revert this if we don't have another fix to replace it with.

It should and I will revert it, the issue that it only partially fixes (not fixed for --with-man --all-name) is now fully fixed since man-pages-%lang packages own man directories with root:root.
So commit 7582 doesn't fix any real problem now, but introduces a real regression.
If I tested it carefully, I never committed this change, and now I don't want to let something that only introduces regression.

(In reply to David Walser from comment #15)
> Because of line 38, it should only be looking at files anyway.  I'm guessing
> the parent_to_own on line 64 makes it own the parent directories of the file
> as well.  Maybe that should just be changed to use own_file like on line 43.

Like I said in comment 10, I don't have enough skills in perl and in all this part to help here and provide a better and safe fix.

(In reply to Luc Menut from comment #16)
> nope, third-party packages will conflict with man-pages-%lang packages too
> if they own man directories with perms other than root:root. If they own man
> directories with root:root, they will not conflict with directories owned
> through find_lang.

And we can't assume they will all use root:root.

> and packages should own ALL directories that they install.

Wrong.  Ideally, each directory should be owned by one package canonically.

> We have lot of directories owned by multiple packages elsewhere;

Not ideal, but yes I know that.

> there isn't problem if they have the same perms.

and owners, but again we're tempting fate here.

> It's the case now with man directories
> since the you change the group to root.
> Look at the list https://bugs.mageia.org/attachment.cgi?id=3451, the only
> conflict was on group root <-> man.

Which was progress in the immediate sense, but it was still right for other packages to drop ownership of those directories.

> It should and I will revert it,

No!  Don't replace a very very minor problem with a potentially much bigger one.

> the issue that it only partially fixes (not
> fixed for --with-man --all-name) is now fully fixed since man-pages-%lang
> packages own man directories with root:root.

No it is not fully fixed, it's only fixed for our packages.

> So commit 7582 doesn't fix any real problem now, but introduces a real
> regression.

It still does fix a real problem, and the regression is extremely minor in comparison.

> Like I said in comment 10, I don't have enough skills in perl and in all
> this part to help here and provide a better and safe fix.

Well let's see if we can get a better fix, rather than taking a step backward.

I just tried what I suggested earlier, and it looks like it works:
--- find-lang.pl.orig   2013-03-31 12:33:56.649943812 -0400
+++ find-lang.pl        2013-03-31 12:33:59.385876269 -0400
@@ -61,7 +61,7 @@
             return if !$withman;
             my ($pkg, $lang, $langfile) = ($4, $3, $1);
             $file =~ s/\.[^\.]+$/.*/;
-            parent_to_own($langfile, $file, $lang) if pkg_match($pkg);
+            own_file($file, $lang) if pkg_match($pkg);
         } else {
             return;
         }
@@ -73,7 +73,6 @@
 
 foreach my $f (sort keys %finallist) {
     my ($lang, @otherlang) = keys %{ $finallist{$f}{lang} || {} };
-    next if $withman && !$allname && $finallist{$f}{dir};
     my $l = sprintf("%s%s%s",
         $finallist{$f}{dir} ? '%dir ' : '',
         @otherlang == 0 && $lang && $lang ne 'C'

(In reply to David Walser from comment #17)
> (In reply to Luc Menut from comment #16)
> > nope, third-party packages will conflict with man-pages-%lang packages too
> > if they own man directories with perms other than root:root. If they own man
> > directories with root:root, they will not conflict with directories owned
> > through find_lang.
> 
> And we can't assume they will all use root:root.

Like I explained previously, if they don't use root:root, they will conflict with man-pages-%lang packages. Don't you understand this?

> 
> > and packages should own ALL directories that they install.
> 
> Wrong.  Ideally, each directory should be owned by one package canonically.

Yes, of course, sorry. Packages should own ALL directories that they install, if they are not owned by another package. I thought about other directories like dia help directories in comment 10.

> 
> No!  Don't replace a very very minor problem with a potentially much bigger
> one.

Sorry, I disagree with you here, it's not a minor problem. It prevents automatic QA checks.
Currently, I'm working on scripts to detect dir <-> file changes, files moved between packages without proper conflicts, etc,.. between 2 sets of packages.
If we want to detect problem earlier, we absolutely need to package correctly all the files and directories when it's needed.

(In reply to Luc Menut from comment #18)
> Like I explained previously, if they don't use root:root, they will conflict
> with man-pages-%lang packages. Don't you understand this?

Exactly!  Any conflicts of this sort are BAD, and we can't avoid them all because we can't fix everybody's packages.  The only way to avoid it is to avoid packages redundantly owning directories they don't need to own.  Do you understand?

> Sorry, I disagree with you here, it's not a minor problem. It prevents
> automatic QA checks.
> Currently, I'm working on scripts to detect dir <-> file changes, files
> moved between packages without proper conflicts, etc,.. between 2 sets of
> packages.
> If we want to detect problem earlier, we absolutely need to package
> correctly all the files and directories when it's needed.

Unowned directories are still a minor problem compared to unpredictable conflicts, but I'm glad to hear what you are working on, that will be very helpful.  I'm not saying they're a total non-issue, just a very minor one.  It's unfortunate we're even having to have this discussion, due to the total bonehead change in rpm 4.11 that introduced this problem.

Anyway, I think the patch I posted above is a proper fix that will satisfy both of our goals here.

(In reply to David Walser from comment #19)
> 
> Unowned directories are still a minor problem compared to unpredictable
> conflicts

I still disagree, and maintain that reverting svn rev. 7582 doesn't add any conflicts, and fixes unowned directories problem shown in comment 10.

> 
> Anyway, I think the patch I posted above is a proper fix that will satisfy
> both of our goals here.

Your patch doesn't work; just on the list of packages from bug 9055, dpkg and po4a doesn't build when using it.

Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/iurt/rpmbuild/BUILDROOT/dpkg-1.16.8-5.mga3.x86_64
error: Installed (but unpackaged) file(s) found:
   /usr/share/man/de/man5/dpkg.cfg.5.xz
   /usr/share/man/es/man5/dpkg.cfg.5.xz
   /usr/share/man/fr/man5/dpkg.cfg.5.xz
   /usr/share/man/ja/man5/dpkg.cfg.5.xz
   /usr/share/man/pl/man5/dpkg.cfg.5.xz
   /usr/share/man/sv/man5/dpkg.cfg.5.xz

Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/iurt/rpmbuild/BUILDROOT/po4a-0.44-3.mga3.x86_64
error: Installed (but unpackaged) file(s) found:
   /usr/share/man/de/man5/po4a-build.conf.5.xz
   /usr/share/man/es/man5/po4a-build.conf.5.xz
   /usr/share/man/fr/man5/po4a-build.conf.5.xz
   /usr/share/man/ja/man5/po4a-build.conf.5.xz
   /usr/share/man/pl/man5/po4a-build.conf.5.xz
   /usr/share/man/pt/man5/po4a-build.conf.5.xz

Keywords: PATCH => (none)
CC: lmenut => (none)

(In reply to Luc Menut from comment #20)
> I still disagree, and maintain that reverting svn rev. 7582 doesn't add any
> conflicts, and fixes unowned directories problem shown in comment 10.

Your automatic checks are a great idea, but unowned directories is still a very minor problem.  It's certainly not anything that has any direct impact on users.  Yes we should fix this if possible now and definitely by mga4.

As for reverting 7582 not adding conflicts, you are incorrect for a couple of reasons.  First of all, you still can't guarantee that for anything but the packages in our repo.  Second of all, we don't have any way to ensure proper package ordering for upgrades with these changes, so if you revert that change and we still have all of these conflicting directory ownerships, it is still quite likely that this will cause a problem with upgrades.  This is a major problem, and we don't want to trade a minor one that will affect users for a minor one that won't.

> Your patch doesn't work; just on the list of packages from bug 9055, dpkg
> and po4a doesn't build when using it.

Actually my patch *does* work, but you just uncovered another bug in find-lang.pl.

This regular expression is bad:
$file =~ m!^((/+usr/share/man)/([^/@.]+)[^/]*)/man[^/]+/([^/.]+)\.\\
d[^/]*!

As it won't match files like the ones you listed above, because they have a dot in the man page name itself.  The last set of parentheses says at least one non-dot character.  After the parens is a dot and a digit.  So it won't match if the next thing after the first dot isn't a digit.

With parent_to_own this doesn't matter *if* there is at least one man page in that directory that doesn't have such a dot in its name, as the regular expression will match that one, the package will take ownership of the directory, which will then include all of those files.

If we switch to using own_file (which is the proper fix here), we need to make sure that regexp matches these files.  Removing the dot from the brackets in the last parens fixes it for me.  Here's the patch:

--- find-lang.pl.orig   2013-04-04 10:42:50.270829329 -0400
+++ find-lang.pl        2013-04-04 10:43:25.440864310 -0400
@@ -57,11 +57,11 @@
             return if !$withhtml;
             my ($pkg, $lang, $langfile) = ($4, $3, $1);
             parent_to_own($langfile, $file, $lang) if pkg_match($pkg);
-        } elsif ($file =~ m!^((/+usr/share/man)/([^/@.]+)[^/]*)/man[^/]+/([^/.]+)\.\d[^/]*!) {
+        } elsif ($file =~ m!^((/+usr/share/man)/([^/@.]+)[^/]*)/man[^/]+/([^/]+)\.\d[^/]*!) {
             return if !$withman;
             my ($pkg, $lang, $langfile) = ($4, $3, $1);
             $file =~ s/\.[^\.]+$/.*/;
-            parent_to_own($langfile, $file, $lang) if pkg_match($pkg);
+            own_file($file, $lang) if pkg_match($pkg);
         } else {
             return;
         }

Luc, you got dropped from CC.  Please see Comment 21.

Keywords: (none) => PATCH
CC: (none) => lmenut

Why do you ask me? You are always right.
I removed myself from CC because I don't have time to spend on this bug, and I don't want to continue to be involved on it.

CC: lmenut => (none)

(In reply to Luc Menut from comment #23)
> Why do you ask me? You are always right.
> I removed myself from CC because I don't have time to spend on this bug, and
> I don't want to continue to be involved on it.

Why are you being like that?

You're the one that raised the issue of the regression that was caused by the previous incorrect fix, and that was a good catch.  Aren't you still interested in fixing that?

I've just been trying to explain to you why simply reverting the previous change without having a better fix to replace it with was a bad idea.  You were being a little hasty in trying to push through that revert.  This bug needed a little more time and analysis to reach a proper fix.  It seems you and I are the only ones that have been looking into this recently, and we only have so much time to work on it.  I just needed you to be a little more patient so we could work through this to solve the problem.

You've raised some good issues along the way and caught things which have been helpful, and now I believe I have found the proper solution.  If you still want this fixed, please take a look and if it works for you, then we can get this pushed and both be happy.

CC: (none) => lmenut

rpm-mageia-setup with these fixes is pushed and the packages that actually weren't fixed in the previous attempt to fix Bug 9055 now are really fixed.

I also rebuilt dia to fix the regression found by Luc.  If any other packages are affected by this regression, we'll need a freeze push request to rebuild it.

Thanks everyone.

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