Re: [PATCH] conf: Accept duplicate and conflicting options, the last one wins

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 19 Jun 2024 11:06:37 +0200
Paul Holzinger <pholzing@redhat.com> wrote:

> On 18/06/2024 18:17, Stefano Brivio wrote:
> > In multiple occasions, especially when passt(1) and pasta(1) are used
> > in integrations such as the one with Podman, the ability to override
> > earlier options on the command line with later one would have been
> > convenient.  
> To give more context: Podman allows to specify pasta options in the 
> containers.conf file and on the cli. In podman we just append config 
> file + cli and pass all options to pasta. As such it would be very 
> difficult for users to overwrite a single option from the config file as 
> pasta errors without this patch. The alternative is for Podman to parse 
> the cli args and fix them before handing them to pasta but this is 
> clearly not maintainable on our end.

Right, I forgot to explain, I'll add this in v2 (recycling from your
paragraph).

> > Recently, to debug a number of issues happening with Podman, I would
> > have liked to ask users to share a debug log by passing --debug as
> > additional option, but pasta refuses --quiet (always passed by Podman)
> > and --debug at the same time.
> >
> > Finally, somebody took care of this on Podman side at:
> >    https://github.com/containers/common/pull/2052
> >
> > but still, we'll probably have similar cases, or older versions of
> > Podman around, etc.
> >
> > I think there's some value in telling users about duplicated or
> > conflicting options, because that might reveal issues in integrations
> > or accidental misconfigurations, but by now I'm fairly convinced that
> > the downsides outweigh this.
> >
> > Drop checks about duplicate options and mutually exclusive ones. In
> > some cases, we need to also undo a couple of initialisations caused
> > by earlier options, but this looks like a simplification, overall.
> >
> > Suggested-by: Paul Holzinger<pholzing@redhat.com>
> > Suggested-by: David Gibson<david@gibson.dropbear.id.au>
> > Signed-off-by: Stefano Brivio<sbrivio@redhat.com>  
>
> PS: The patch doesn't apply to current HEAD on https://passt.top/passt/ 
> (should I use another tree?) and git doesn't offer any merge conflict 
> resolution for applying patches either??? So I cannot test this locally.

My bad, I based this patch on other patches I had just posted, I'll
rebase this on HEAD for v2.

If you want to apply it anyway, git's three-way merge fixes this up,
given that there are no actual conflicts:

  $ curl https://archives.passt.top/passt-dev/20240618161723.1896519-1-sbrivio@redhat.com/raw | git am -3
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
  100 12963    0 12963    0     0  37441      0 --:--:-- --:--:-- --:--:-- 37465
  Applying: conf: Accept duplicate and conflicting options, the last one wins
  Using index info to reconstruct a base tree...
  M	conf.c
  M	passt.1
  Falling back to patching base and 3-way merge...
  Auto-merging passt.1
  Auto-merging conf.c

-- 
Stefano