reprotest: inadvertent misconfiguration in salsa-ci config

[Vagrant Cascadian]

On 2024-02-27, Chris Lamb wrote:
>> * Update reprotest to handle a single-disabled-varations-value as a
>>   special case - treating it as vary and/or emitting a warning.

Well, I would broaden this to include an arbitrary number of negating
options:

  --variations=-time,-build_path

That seems just as invalid.

The one special case I could see is "--variations=-all" where you might
want to be normalizing as much as possible.


> On whether to magically/transparently fix this, needless to say, it's
> considered bad practice to change the behaviour of software that has
> already been released — I would, as a rule, subscribe to that idea.
> However, we should bear in mind that this idea revolves around what
> users are *expecting*, not necessarily what the software actually
> does.
>
> I say that because I hazard that all 400 usages are indeed expecting
> that `--variations=-foo` functions the same as `--variations=all,-foo`
> (or `--vary=-foo`), and so this proposed change would merely be
> modifying reprotest to reflect their existing expectations. It would
> not therefore be a violation of the "don't break existing
> functionality" dictum.
>
> (Saying that, the addition of a warning that we are doing so would
> definitely not go amiss.)

Hrm. Less inclined toward this approach; expectations can shift with
time and context and culture and whatnot. That said, I agree the current
behavior is confusing, and we should change something explicitly, rather
than implicitly...


>> * Treat removal of a variance factor from an already-empty-context
>> as an error.
>
> I'm also tempted by this as well. :)  How would this be experienced by
> most DDs? Would their new pushes to Salsa now suddenly fail in the
> reprotest job of the pipeline? If so, that's not too awful, given that
> the prominent error message would presumably let them know precisely
> how to fix it.

I would much prefer an error message if we can correctly identify this.

Some possible expected behaviors to consider treating as invalid, and
issue an error:

  --variations=-build_path

  --variations=-time,-build_path

This almost makes me want to entirely deprecate --variations, and switch
to recommending "--vary=-all,+whatever" or "--vary=-all
--vary=+whatever" instead of ever using --variations.

I'm not sure the variations syntax enables much that cannot be more
unambiguously expressed with --vary.

That said, the reprotest code is a bit hairy, and I am not sure what
sort of refactoring will be needed to make this possible. In particular,
how --auto-build is implemented, where it systematically tests each
variation one at a time. That said, Refactoring might be needed
regardless. :)


live well,
  vagrant
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <http://lists.reproducible-builds.org/pipermail/rb-general/attachments/20240227/40db9ae3/attachment.sig>