Hey everybody!
I have been working on some changes to different I/O related components
in the system, and have gotten to a point where I have changes I'd like
to get into shape for submission.
But first, my plan was to bring the pieces I'm planning to replace
under test, since none of them are, if only to document some expected
behavior and to catch potential regressions I introduce.
That lead me to the existing test suite[1], and I made it a goal to get
that into good shape before proceeding with my planned changes. More
specifically, my plan is to:
[x] Fix all regressions that crash the test suite (partial victory
because there are some remaining tests that were already disabled
because of crashes, such as this memory corruption bug exposed by a
test that uses clone_area()[9], and only fails with hoard2, not
rpmalloc).
[ ] Figure out why each failing test fails and decide whether to update
it or fix any regression that might cause it to fail.
[ ] Continue to fix regressions in the test suite until it becomes a
regular part of the workflow for other developers.
[ ] Figure out some way to run these in continuous integration, which
I think is possible but will not be easy, since most of these tests
are integration tests that require a running Haiku system to work.
I've been fixing these and a couple of those patches have been merged
already, but I've come to a point where I'm not totally sure how
everyone feels about how to proceed. Some of the tests are broken
because of legit regressions, and for those it's clear how to fix them.
But many of the tests are broken because behavior (documented or not),
is differnt from BeOS. The behavior differences are almost always due
to error code differences, but not always.
In cases where there is divergence, I've found that the tests were
designed to work in BeOS R5 and the BeOS behavior is defined in the
tests (although I'm double checking each case).
This leads me to my question, which is, in a case where behavior has
been stable in Haiku for a long time, but has diverged from BeOS, do we
prefer changing the Haiku behavior to match BeOS, or do we prefer to
not introduce changes into Haiku that might break userspace that has
come to depend on that behavior?
I don't think there is a clear answer and I think it depends on the
context, so I'll give some examples:
Case 1: Regression that causes tests to fail
--------------------------------------------
hrev53662[3] fixed a regression in BString that caused a test to fail.
This was a clear regression that caused the test to fail, so changing
BString, and not the tests, was the right thing to do.
Another more silly example is in this change[4] from one of my
branches, where this code path in BClipboard traps in the debugger,
without the call to debugger() the correct, BeOS R5 compatible error
code is returned.
Another example is here in BArchivable[8], where the BeOS R5 code
allows instantiate_object without providing a signature (mime-type)
value. In R5 it will just handle the missing signature field in the
BMessage and will search all loaded images for the specified type. This
seems like a clear regression where the implementation of
instantiate_object(BMessage*) should be changed and not the tests.
Case 2: Divergence from BeOS that is desired
--------------------------------------------
hrev53681[2] fixed the BMemoryIO tests. There was one regression, and
it was related to the fact that BMemoryIO::WriteAt(off_t, const void*,
size_t)
In this case, one of the BMemoryIO tests was passing in BeOS R5, but
not in Haiku, because the Haiku code was better and detects a potential
memory corruption bug. It Haiku's BMemoryIO prevents the use of some
invalid input that has some undefined side effect that is likely not
going to be desired.
In this case I think it's clear that the Haiku behavior is desired and
the test was updated appropriately.
Case 3: Haiku ABI has diverged from BeOS R5
-------------------------------------------
I haven't run into a case of this yet, but I haven't doen analysis on
all of the failed tests yet.
My assumption is that any ABI break should be resolved by updating the
Haiku ABI to match BeOS R5 under all circumstances.
Case 4: Divergence from BeOS that is of unclear value
-----------------------------------------------------
These are all over the place, and this is where I need some guidance.
I have branchs which fixe most of the tests for BArchivable and
BRoster, and while there are some regressions, most of the changes are
related to differences in error codes.
One example of an error-code mismatch is here in instantiate_object[5].
In Haiku, the B_LAUNCH_FAILED_APP_NOT_FOUND returned from
BRoster::FindApp() is bubbled up to the caller of instantiate_object(),
where in BeOS R5 the error returned is B_BAD_VALUE. It is not clear to
me whether it is best to attempt to match all error codes in BeOS, or
if it's best to leave this unchanged to avoid breaking Haiku userspace
that has come to expect this error.
This change[6] is a trickier example, where is given
instantiate_object() a valid signature matching an add-on, but once the
add-on is loaded, the type name is not found. In Haiku, the image_id
parameter is set to an error value, while in BeOS, the image_id
parameter is the ID of the add-on that was loaded by
instantiate_object(). This behavior difference may be unimportant, and
it isn't really documented, but it is different. One may argue that the
BeOS behavior is desired since you may want the option to unload the
add-on if the type you're trying to load was not found, but this
behavior is not documented[7].
In this case, I think the algorithm should be:
* If the behavior difference is purely a difference in error codes, and
those error codes are not documented in the Haiku API docs, then just
change Haiku to match BeOS.
* Else if the behavior difference *is* documented in the Haiku API
docs, then update the tests and the docs, noting that the behavior
has diverged from R5.
* Else, deside on a case-by-case basis which side to prefer, preferring
to have as little impact on Haiku userspace as possible.
Summary
-------
Specifically I'm wondering about Case4. What are your thoughts? Do you
favor Haiku API stabilty over compatibilty with BeOS in the case of
error handling differences?
Also please let me know if you think I'm classifying these changes
inappropriately.
[1] https://www.haiku-os.org/documents/dev/unit_testing
[2]
https://git.haiku-os.org/haiku/commit/?h=hrev53681&id=2c17ecf993238e82ed75c327a5442cd4b473f805
[3]
https://git.haiku-os.org/haiku/commit/?h=hrev53662&id=fbc30e914542cb3a452b01a0714b4e2dff2561a7
[4]
https://github.com/ambroff/haiku/commit/d06669217dd6cbe146e94cd90bc318ac989b4185
[5]
https://github.com/ambroff/haiku/commit/88e3d15ce6a00429cff1e8a0f657397b5258f0e2
[6]
https://github.com/ambroff/haiku/commit/3220c7d02d4cc541d223db10b969afc596b954d0
[7]
https://www.haiku-os.org/legacy-docs/bebook/TheSupportKit_Functions.html#instantiate_object
[8]
https://github.com/ambroff/haiku/commit/94fb2b509d64386d4a7df82e39c15e67bbcbc422
[9] https://dev.haiku-os.org/ticket/14376
Attachment:
signature.asc
Description: OpenPGP digital signature
