Bug 133453 - /etc/init.d/xfs buildfontlist(): "find -cnewer" bad interaction with chmod
Summary: /etc/init.d/xfs buildfontlist(): "find -cnewer" bad interaction with chmod
Keywords:
Status: CLOSED WORKSFORME
Alias: None
Product: Fedora
Classification: Fedora
Component: xorg-x11
Version: 3
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mike A. Harris
QA Contact: David Lawrence
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2004-09-24 02:31 UTC by Hans Ecke
Modified: 2007-11-30 22:10 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-04-08 12:18:14 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
patch to change "find -cnewer" to "find -newer" (505 bytes, patch)
2004-09-24 02:32 UTC, Hans Ecke
no flags Details | Diff

Description Hans Ecke 2004-09-24 02:31:25 UTC
Description of problem:

In the shell function buildfontlist(), one of the tests to see whether
a font directory needs to be regenerated, looks like this:

find . -type f -cnewer fonts.dir -not -name 'fonts.cache*'

There is nothing wrong with this, except for the use of "-cnewer". Why
is "-newer" not used? 

The problem here is a bad interaction with chmod. One of the
maintenance scripts we use does a "chmod -R ugoa+rX /usr/share/fonts"
every day. This has proven to be quite useful. Unfortunately, chmod
will update the ctime of _all_ files it encounters under
/usr/share/fonts, not only the ones the it changes. This means that
the lengthy directory regeneration will be run much to often.

However, chmod does not update the mtime of files. So I propose to
change the "-cnewer" test to a "-newer".

Version-Release number of selected component (if applicable):

All from RH9 to FC3test2

How reproducible:

Always

Steps to Reproduce:
1. Restart XFS
2. Do "chmod -Rc ugoa+rX /usr/share/fonts"
3. If it reports changes to permissions, restart XFS and again do a 
   "chmod -Rc ugoa+rX /usr/share/fonts"
4. Restart XFS
  
Actual results:

The last restart will regenerate directories unnecessarily, slowing
the XFS restart down.

Expected results:

No directories should be regenerated

Additional info:

See attached patch for the fix.

Comment 1 Hans Ecke 2004-09-24 02:32:28 UTC
Created attachment 104243 [details]
patch to change "find -cnewer" to "find -newer"

Comment 3 Mike A. Harris 2005-04-04 18:31:44 UTC
-cnewer is used, for reasons oulined in bug #53737



Comment 4 Mike A. Harris 2005-04-04 23:08:52 UTC
The logic in the initscript has now been simplified significantly, to
remove a lot of unnecessary complexity.  This basically removes all
of the REGEN_FONTS_DIR stuff by putting that logic directly in the
if statement which was the main use.

We now run fc-cache always, since it runs very fast anyway, and fonts
may have been added/removed/changed in fontconfig configured directories
which are not even configured in xfs.

A couple of other cleanups I made should speed up xfs initscript a slight
bit also in some cases.

The end result, should be that this bug is no longer present, fc-cache
is now ran to ensure all fontconfig configured font directories have their
cache files updated, wether or not mkfontdir et al. were ran, and this
will have more likelyhood of improving the performance of client side
fonts as well.

I have not committed the new initscript to CVS yet, as I'd like you to
test it with your setup first:  ftp://people.redhat.com/mharris/xfs.init

Please test that, and update the bug report to indicate if you see any
regressions or other problems.

Thanks in advance.


Setting status to "NEEDINFO", awaiting test results.

Comment 5 Mike A. Harris 2005-04-04 23:44:28 UTC
oops, the last comment was intended for bug #133451 rather
than this bug, however feel free to test the new initscript
as well if you like.


Comment 7 Hans Ecke 2005-04-05 20:23:27 UTC
 (In reply to comment #3) 
> -cnewer is used, for reasons oulined in bug #53737 
 
It seems bug #53737 is somehow private. When trying to open it I get "access 
denied". Can you elaborate what this bug is about? 
  
I understand that this can be fixed either on the initscripts or the chmod 
side. It would make sense for "chmod -c" to only change the ctime if it 
actually does change permissions. However, this is low on the priority list of 
the coreutils developers:  
 
http://lists.gnu.org/archive/html/bug-coreutils/2004-09/msg00145.html 
 
I'll go and check out your new scripts now... 
 

Comment 8 Hans Ecke 2005-04-05 20:24:49 UTC
Mike, I think I just overrode one of your bug annotations. Sorry! Please put it 
back on. 

Comment 9 Mike A. Harris 2005-04-06 11:03:19 UTC
>Mike, I think I just overrode one of your bug annotations. Sorry! Please put it 
>back on.

Not sure what you mean.



>It seems bug #53737 is somehow private. When trying to open it I get "access 
>denied". Can you elaborate what this bug is about? 

The bug was flagged private-beta from a long time ago, but I removed
the privacy flags now as there was nothing NDA in the report.  You should
be able to view it now.


> I understand that this can be fixed either on the initscripts or the chmod 
> side. It would make sense for "chmod -c" to only change the ctime if it 
> actually does change permissions. However, this is low on the priority list of 
> the coreutils developers:  
> 
> http://lists.gnu.org/archive/html/bug-coreutils/2004-09/msg00145.html 
 
Indeed, chmod should not be changing the ctime of files it does not actually
change.  If POSIX allows it to do that, it is unfortunate, as it inadvertently
causes problems such as this one.  In this case, the solution is to not
run chmod on all files.

Suggested workaround:
- Write a script or C application which checks the permissions of a file
  prior to changing it, and only changes them if necessary.  This will
  still trigger a fonts.dir rebuild however, as we're not checking file
  contents (a very expensive operation), but time only.  Using the right
  arguments to the "find" command, you can check wether a given file has
  the right permissions, and then cause chmod to be invoked on that file
  alone.  I suspect this would only add a line or two to your script
  to workaround the issue.

- Another possibility, is to change the manner in which fonts get installed,
  to something which ensures that the fonts are installed with the correct
  permissions always.  If permissions are set properly from the outset, the
  script wont be needed to reset the permissions.


I've brainstormed a bit about what other changes we might be able to do
in order to prevent this problem from occuring, however the only things
I can think of, involve detecting font addition/removal/modification by
checking the actual font file contents with md5sum or similar, and storing
a hash in a separate metadata file named fonts.hash or something.  Since
this would involve running md5sum on every single font in all font
directories in the system, every single invocation, it would be very
expensive an operation, probably more expensive than just running mkfontdir
et al. in every directory wether they were needed or not.


Hopefully these workarounds are helpful, as I can't see any other way
of solving this within the xfs initscript without breaking the script's
intended functionality.  Hopefully GNU maintainers will fix chmod in
the future though.

Let me know how you make out.

Comment 10 Hans Ecke 2005-04-06 22:23:06 UTC
(In reply to comment #9)   
> >Mike, I think I just overrode one of your bug annotations. Sorry! Please put   
it    
> >back on.   
>    
> Not sure what you mean.   
   
When I submitted the note bugzilla said something along the lines of "somebody   
else edited this bug while you were. Your note will override that person's   
changes." I hit "continue", and it sank in only later. I think it was some   
change in "depends on" or some other data field like that.   
   
> Indeed, chmod should not be changing the ctime of files it does not actually   
> change.  If POSIX allows it to do that, it is unfortunate, as it   
inadvertently   
> causes problems such as this one.  In this case, the solution is to not   
> run chmod on all files.   
>    
> Suggested workaround:   
> - Write a script or C application which checks the permissions of a file   
>   prior to changing it, and only changes them if necessary.  This will   
>   still trigger a fonts.dir rebuild however, as we're not checking file   
>   contents (a very expensive operation), but time only.  Using the right   
>   arguments to the "find" command, you can check wether a given file has   
>   the right permissions, and then cause chmod to be invoked on that file   
>   alone.  I suspect this would only add a line or two to your script   
>   to workaround the issue.   
   
That is what I did. However, to replace a "chmod -Rc ugoa+rX" takes a   
bit more than a line or two because you need to special-case files and   
directories. Anyway, that wrapper works just fine. It just is a non-general   
workaround, more complicated and so more error prone. Its also slower.   
   
For further reference for people who find this bug in google or so:   
   
accessible_dir () {   
   # all files readable   
   find "$1" -type f \! -perm -ugoa+r  -print0|xargs -r -0 chmod -c ugoa+r   
   # all dirs read- and executable   
   find "$1" -type d \! -perm -ugoa+rx -print0|xargs -r -0 chmod -c ugoa+rx   
   # all files that have one execute bit set - set it for everybody   
   find "$1" -type f -perm +ugoa+x \! -perm -ugoa+x -print0|xargs -r -0 chmod   
-c ugoa+x   
}   
   
I think you should set this bug as resolved "workaround" or "upstream" or  
something like that.  

Comment 11 Hans Ecke 2005-04-06 23:37:12 UTC
My last comment changed Status from ASSIGNED to NEEDINFO. I didn't mean for 
that. I'm sorry, just keep messing up here. 
 

Comment 13 Mike A. Harris 2005-04-08 12:18:14 UTC
Ok, it's good to see you were able to find a workaround for the chmod
bug.  I'll set the report status to "WORKSFORME" which I guess is the
closest thing to "WORKAROUND" bugzilla provides.  ;)

Thanks for the feedback.


Note You need to log in before you can comment on or make changes to this bug.