Julian Foad wrote on Wed, Apr 07, 2021 at 11:33:35 +0100:
> Daniel Shahaf wrote:
> > Julian Foad wrote:
> > > If we warn [...] it also has Bad properties:
> > > supporting misleading assumptions about how other variations of the
> > > scenario might behave.
> >
> > Could you be more concrete about those assumptions?
>
> It's not the warning itself but that continuing to behave illogically would
> support bad assumptions. One example: if the user is accustomed to the
> foreign-repo merge behaviour (despite same UUIDs) using a script that says (x
> = repository root URL path)
> source1=http://x source2=https://x target=https://x
> then they might assume that "correcting" source1 in their script to say
> source1=https://x source2=https://x target=https://x
> would keep the same behaviour; but it doesn't.
>
> Of course a warning could try to explain sufficiently but the current
> behaviour is not self-consistent enough to explain in one short sentence.
Not in one sentence, but we aren't limited to one sentence. I did
propose example text in my previous reply.
> > [...] we can phrase the
> > warning clearly enough that anyone who runs into it unintentionally will
> > abort the merge and re-do it "correctly", without us needing to enforce
> > it by issueing a fatal error.
> [...]
> > I don't disagree with classifying this as a bug, but I still don't
> > understand why issueing a warning wouldn't be satisfactory — or, at
> > least, have 1.15 issue a warning and 1.16 upgrade that to a fatal error,
> > just to be on the safe side. See the example output above, line 4.
>
> That's a good strategy for dealing with cases in general where a
> behaviour one "supported" in practice needs to be withdrawn. If this
> were an issue in a more main-stream merge case, I would agree.
> However, the proposed benefit that we offer by continuing the merge is
> targetting the wrong group: it is a supposed convenience to users
> whose UUIDs are misconfigured, while it would be a hindrance to users
> who run into it by accident while having their UUIDs correctly
> configured.
I don't see how getting a warning would be more of a hindrance than
getting a hard error. Run some command, get a warning, read it, thank
the tool for catching your error, correct your error. Happens every day
with other tools, and even with svn(1)'s use of svn_cl__similarity_check().
How would getting a warning be more of a hindrance than getting a hard
error? Is it because the user might do the merge before reading the
error message (using up bandwidth and CPU), or because the user would be
presented with a decision to make, or what?
> Also, this is so much an edge case that it isn't worth
> the extra complexity on our side; we don't have a great track record
> of getting around to doing things that we scheduled for a later
> release, for instance.
Actually, I thought we could implement this by committing the code up
front with a preprocessor-based time bomb:
#if (SVN_VER_MAJOR > 1 || SVN_VER_MINOR >= 16)
hard_error();
#else
warning();
#endif
> Handling it so gently for the affected users is a nicety that doesn't
> feel worth it.
Just erring on the side of caution. For affected users, the impact of
releasing a hard error would be more significant than the impact of
releasing a warning (with or without upgrading it later to a hard
error); and we don't know how many users would be affected.
The impact would be doubly notable if the change is made in a patch
release, and triply if the patch release will also happen to feature
a fix to a vulnerability (nowadays that we don't do separate releases
for such).
> I think the reasonable use cases for the case in question and the
> likely number of occurrences in practice are both near zero. I don't
> have numbers to back up this feeling, but as far as I am aware nobody
> has reported this issue and it has existed for many years.
I assume that by "occurrences" you refer to people who invoke this
syntax unintentionally.
> > By the way, I don't feel strongly about this matter; it simply happens
> > to be the one place in the write-up of the issue where I couldn't see
> > why an alternative design was ruled out.
>
> Ack, and thanks for picking up on this omission. Does that all stack up now?
Not entirely, I'm afraid. Anyway, I don't feel strongly about 1.15.0,
but I'm not too comfortable with converting the foreign merge to a hard
error in a patch release. Even if doing a foreign merge on equal UUIDs
is wrong, I don't think it's so wrong that we can tell people they
should have realized on their own that they shouldn't be relying on this
behaviour.
Cheers,
Daniel
