bug#70887: In coreutils 9.5, "cp" command does not honor "--interactive" option

[Pádraig Brady]

On 12/05/2024 16:06, Paul Eggert wrote:
> On 2024-05-12 04:49, Pádraig Brady wrote:
>
> > @@ -1151,7 +1151,8 @@ main (int argc, char **argv)
> >                   {
> >                     /* Default cp operation.  */
> >                     x.update = false;
> > -                  x.interactive = I_UNSPECIFIED;
> > +                  if (x.interactive != I_ASK_USER)
> > +                    x.interactive = I_UNSPECIFIED;
> >                   }
> >                 else if (update_opt == UPDATE_NONE)
> >                   {
> > @@ -1166,7 +1167,8 @@ main (int argc, char **argv)
> >                 else if (update_opt == UPDATE_OLDER)
> >                   {
> >                     x.update = true;
> > -                  x.interactive = I_UNSPECIFIED;
> > +                  if (x.interactive != I_ASK_USER)
> > +                    x.interactive = I_UNSPECIFIED;
>
> Thanks for looking into this messy area. Here is a comment from another
> pair of eyes.
>
> Could you elaborate a bit more about why these two bits of code change
> x.interactive at all? That is, why doesn't update_opt simply affect
> x.update? Why does update_opt bother to override a previous setting of
> x.interactive to I_ALWAYS_YES, I_ALWAYS_NO, or I_ALWAYS_SKIP?
>
> Another way to put it: shouldn't x.update simply reflect the value of
> the --update option, whereas x.interactive reflects reflects whether -f,
> -i, -n are used? Although this would require changes to copy.c, it'd
> make the code easier to follow.

I agree that some refactoring would be good here.
At least x.update should be renamed to x.update_older.

As interactive selection, and file dates all relate
to selecting which files to update, it's tempting to conflate the settings.
However you're right that this introduces complexities when
trying to avoid all inconsistencies. Currently for example:
  $ cp -v -i --update=none new old  # Won't prompt as expected
  $ cp -v --update=none -i new old  # Unexpectedly ignores update option

So yes we should separate these things.

> Another way to put it: why should, for example, --update=all override a
> previous -f or (partly) -n but not a previous -i?

Right -f is significant for mv here (for completeness -f for cp is a separate 
thing).
I.e. we need to treat I_ALWAYS_YES specially in mv with the current scheme.

BTW -n is not overridden by any --update option currently,
and this change effectively applies the same logic to -i now.

thanks,
Pádraig