Julian Foad wrote on Tue, 06 Apr 2021 09:12 +00:00: > Daniel Shahaf wrote: > > The write-up doesn't seem to consider the possibility of issuing > > a warning rather than a fatal error. This would negate the "does not > > introduce a silent behavioural change" argument and not constitute > > a flag day to anyone who may be "intentionally or unintentionally relying > > on the current behaviour". Thoughts? > > Good question. TL;DR: The behaviour is a bug not a feature. > > The case in question is where someone is running a merge with differing > source and target (repository-root) URLs, and getting the "foreign > repository" kind of merge result, but their "foreign repository" has > the same UUID. The UUIDs indicate these URLs point to logically the > same repository and so a "foreign repository" merge is the wrong > behaviour. If a user in this situation is getting results that "work" > for them it is only by accident. This is an exceptional case that we > reasonably assume to be rare. > > If we warn instead of error in this exceptional case, then in this case > it would continue to perform the "foreign repository" merge. While > that may be convenient for anyone currently using this scenario (they > would not have to change anything), 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? > But the convenience is only useful for users in this exceptional case, > whereas the negatives are there waiting for everybody who may some day > be caught out when they use a different variant of their URL. > The negatives that would be waiting in a hypothetical release that issues warnings as proposed aren't that bad, and certainly not as bad as those in the current release, are they? Imagine a user running into the warning: % svn merge https://svn-eu.apache.org/repos/asf/subversion/branches/foo svn: warning: W000042: Performing a foreign-repository merge, despite the merge source looking like a non-foreign repository svn: warning: W000042: To perform a non-foreign merge, revert the results and re-run the merge against the URL 'https://svn.apache.org/repos/asf/subversion/branches/foo'; see https://subversion.apache.org/faq#foo for details svn: warning: W000042: This warning will become a fatal error in Subversion 1.16.0 M CHANGES ⋮ % Never mind the specific wording; the point is that 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. > In other words, continuing to support the current behaviour is a bug. > > When filing the issue I couldn't quite decide whether it was a bug or > an enhancement (and started filing it as a bug at first, then finally > filed it as an enhancement), but after thinking through all this I > conclude it's a bug (and I've just changed the issue type to reflect > that). 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. 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. Cheers, Daniel
