[apparmor] [utils] proposed redesign for mergeprof

[Christian Boltz]

Hello,

Am Freitag, 5. September 2014 schrieb Kshitij Gupta:
> Version -1:
>
> Changes to facilitate 2-way merge (maybe also 3-way) of multiple
> profiles as discussed on IRC ( which someone had to summarize for ml
> ;-))

Now that you mention it... ;-)

I was busy with our yearly wine festival (last friday till monday, plus
various preparations, setup and teardown) and totally forgot to write 
this mail. Sorry!

> The proposal:
> - moves reset method to reset_aa function
> - modifies message displayed to user
> - allows processing of multiple files in 2-way merge
> - disables 3-way merge till new syntax has been decided
>
> The changes reflect the approach of providing arbitrary number of
> files using wildcards or explicitly. This makes it necessary to define
> a way to distinguish the files for base and other case to be used in
> 3-way merge.
> @cboltz Any ideas are welcome!

I'd use something like

  --oldupstream /home/foo/oldprofiles
  --newupstream /home/foo/newprofiles

(the parameter names are far from final - I don't really like
"oldupstream" and "newupstream", but don't have a better idea right now)

Then apply the changes to /etc/apparmor.d/ (or -d if specified)

So basically the code for 3-way merge has to do
    diff $oldupstream/$profile $newupstream/$profile |  \
    patch /etc/apparmor.d/$profile


The above (together with your text) is basically what I had to write ;-)

> The changes map the profiles in the given files to their respective
> files in the local directory specified using -d. Then the merges take
> place profile-wise. There are some cases with this approach with
> multiple profiles in a file where unnecessary questions are asked not
> relevant to the other profile.
>
> === modified file 'utils/aa-mergeprof'
> --- utils/aa-mergeprof    2014-09-03 23:49:47 +0000
> +++ utils/aa-mergeprof    2014-09-04 18:33:34 +0000
> @@ -26,19 +26,19 @@
>  from apparmor.translations import init_translation
>  _ = init_translation()
>
> -parser = argparse.ArgumentParser(description=_('Perform a 2-way or
> 3-way merge on the given profiles'),
> +parser = argparse.ArgumentParser(description=_('Perform a 2-way or
> 3-way merge on the given profiles'),

For now, I'd remove the "3-way" part from the description.

>      epilog='WARNING: the arguments will change in a future version!')

and also remove the warning (after applying your patch) ;-)
(adding some --options for 3-way-merge won't change the syntax 
existing users know, and with allowing any number of files as arguments, 
we _have to_ use --options for 3-way-merge)

> -parser.add_argument('mine', type=str, help=_('your profile'))
> -parser.add_argument('base', type=str, help=_('base profile'))
> -parser.add_argument('other', nargs='?', type=str, help=_('other profile'))
> +parser.add_argument('files', nargs='+', type=str, help=_('base profile'))
> +#parser.add_argument('other', nargs='?', type=str, help=_('other profile'))
>  parser.add_argument('-d', '--dir', type=str, help=_('path to profiles'))
>  #parser.add_argument('-a', '--auto', action='store_true',
> help=_('Automatically merge profiles, exits incase of *x conflicts'))
>  args = parser.parse_args()
>
> +args.other = None

Looks like a quick workaround, but I'd accept that to get your patch
into beta 2 ;-)  (however, a TODO comment would be a good idea)

The remaining part of your patch looks good, but I didn't test it.


Regards,

Christian Boltz
--
Tut mir leid. Tu hast dich dafür entschieden,  Computer zu benutzen. Aus
irgendeinem Grunde glaubst du,  das sei risikofrei.  Ich versichere dir,
daß es das nicht ist. Computer sind böse, rostige, alte Kettensägen, die
grundlos anspringen.   [Ratti in suse-linux]