Re: cp preserves mode with --no-preserve=mode

[Ondrej Oprala]

On 09/22/2012 11:17 PM, Jim Meyering wrote:
> Ondrej Oprala wrote:
>
> > On 08/07/2012 05:37 PM, Bernhard Voelker wrote:
> > > On 08/07/2012 05:01 PM, Ondrej Oprala wrote:
> > > > Hi, I've renamed the variable to be more hinting of it's purpose
> > > > and --no-preserve=mode should now work properly with directories
> > > > as well.
> > > > Cheers,
> > > >    Ondrej
> > > Thanks, that looks good ... maybe the directory case deserves to
> > > be added to the test. Minor nit: the change in copy_internal() is
> > > missing in the commit log.
> > > I'm sure that Padraig/Jim will add both when committing.
> > >
> > > BTW: Interestingly, the TODO entry mentioned "--no-preserve=X"
> > > to be buggy:
> > >
> > > > -cp --no-preserve=X should not attempt to preserve attribute X
> > > > -  reported by Andreas Schwab
> > > but in fact, all the modes (t,o,l,c, and x) but "mode"
> > > have already been working. ;-)
> > >
> > > Have a nice day,
> > > Berny
> > Hi, I've amended the commit log and added a test for directories.
> Thanks again for your work on this.
> Sorry it took so long for me to get back to it.
>
> I've looked over the patch and noticed a problem: What happens
> when we use --no-preserve=mode and --preserve=all together?
> I would not expect --no-preserve=mode to silently override
> a following --preserve=all.
> Rather, the latter should supersede the former.
>
> Hmm... the right thing appears to be what happens,
> in spite of conflicting settings.
>
> It looks like while the two members .preserve_mode and
> .explicit_no_preserve_mode can both be set (a contradiction),
> that doesn't cause an actual problem because whenever it happens,
> .preserve_mode is both correct and the member that is tested first
> in an if-else-if... cascade.
>
> So maybe all you need to do it to turn off .explicit_no_preserve_mode
> in the PRESERVE_ALL case?
>
> A test case addition to cover that case would be most welcome,
> but since I've waited so long to give feedback, it seems unfair
> to require that.
>
> >  From d52f8990353dac04bff141ff31b6601ba5112a18 Mon Sep 17 00:00:00 2001
> > From: Ondrej Oprala <address@hidden>
> > Date: Tue, 7 Aug 2012 16:56:52 +0200
> > Subject: [PATCH] cp: Fix the --no-preserve=mode option
The .explicit_no_preserve_mode flag should now be turned off when 
--preserve=all
is run into.
I've also added a test for the mixed options next to other tests in 
preserve-mode.sh.

Cheers,
 Ondrej

DIFF
Description: Text document