| Re: [eigen] is there a policy on committing bugfixes to a release branch? |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/eigen Archives
]
On 16.04.2014 16:43, Mark Borgerding wrote:
I'm not talking about adding new features on a release branch, but a
legitimate bugfix.
I have the privilege to commit, and I want to be responsible in its use.
I looked on the wiki, but could not find philosophical guidance as much
as mechanical advice. Perhaps I missed it in my brief search. In this
case, I apologize for wasting your time and the electrons required to
bring the rest of this email to you. Or maybe this policy doesn't exist
because good policy on this is hard as it requires good judgement.
I don't think we have anything written down for that -- I also have no
overview how many (and who) has write-access to the main repository.
Specifically, I found a bug that exists in both 3.2 and default
branches. I fixed it in default, but I have not yet patched the 3.2
branch since I don't want to display bad judgment and/or run afoul of
any best practices we may have. This email is not about this bugfix per
se, but it was a catalyst for it. More details at the end if you are
interested. It is a minor bug and one with an easy workaround. However
it is not easy to write a test case for it. Testing involves building
with multiple versions of Intel MKL. I've tested against 3.
The small question is: Should I apply this bugfix to 3.2?
I guess that specific bugfix is safe enough to be applied to 3.2.
I see that compability checks are very hard to unit-test.
The big question is: How far do we go to ensure a release branch is
always bugfree? This is complicated by the fact that a bugfix can
inadvertently introduce an ironic new bug -- not this one, of course,
but you know *other* peoples bugfixes ;) . Is there a policy or best
practice that makes the best use of developer resources to ensure the
quality of the release branches?
Before releasing, we call for extended unit-tests especially also by
users with non-common compilers/systems. I don't think we can guarantee
'bugfree'-ness of Eigen. Important is that we don't re-introduce bugs
which were once fixed -- that should be ensured by our unit-tests.
Path 1: Quality ensured by "THE PROCESS" (muffled screams)
It would be easy to dream up an elaborate system of checks and balances
that requires traceability to a confirmed bug report, automated test
cases, gatekeeper committers, etc, etc. But this could be a stiflingly
unrealistic barrier to bugfixes. As an aside: I consider cmake a big
barrier to entry for contributors to the project, unfortunately I have
no alternative to suggest.
At least on posix-based systems, cmake is very common and usually
installed by people who do any programming. Admittedly, that might not
be the case for Windows/MSVC developers.
I guess having the unit-tests run on bitbucket before accepting a commit
would end up very time-consuming (also I'm not sure if bitbucket
supports that feature). However, we could encourage more people to run
nightly tests on their servers (maybe write a small wiki-page how to set
that up).
Path 2: "Use your best judgement"
At the other end of the spectrum: a lack of review on the release branch
raises the possibility for ironic bugs. Should a user be able to assume
the tip of the release branch is absolutely the most stable version? Or
should they look to tagged versions? Assuming the latter allows us to
use the release branch for bugfixes with some confidence that any ironic
bugs will be found before a tag.
I would say the tip of 3.2 is mostly the latest tagged release with
added recent bugfixes, but no (entirely) new features. Usually, I would
consider this safe, but for extra-safety one should only update if
depending on a recently committed bugfix and stick to the last tagged
release otherwise.
To answer your actual question, I usually decide depending on the
severity and possible impact of the bug and the patch:
* Typo in the docu: Just commit
* Small obvious bug which never has been tested: Fix and add unit test
* "Medium" bug: As above, but additionally compile/run some tests which
might be affected by the fix
* "Big" bug, as above, but run entire test-suite before pushing upstream
* "Even bigger": Propose a patch, ask for review (i.e. only attach to
corresponding bugzilla-entry)
In the end, we will always depend on user feedback, if we accidentally
break something, especially if that shows only on exotic systems.
Bug Specifics:
(changeset f912c18f0267 : "Check IMKL version for compatibility with
Eigen" ).
The background on the specific bug: My project is built from source by
several customers who have various versions of tools. We defined
EIGEN_USE_MKL_ALL in our build, but then realized it bombed for
customers with older MKL versions (#include <mkl_lapacke.h> failed-- no
such file ). Rather than force our build system to detect the MKL
version and conditionally set this flag, I wanted to fix the upstream
Eigen source.
Maybe you could additionally produce a warning if MKL is too old.
Unfortunately, that requires another #if-construct to distinguish
between #warning and #pragma warning (or whatever the preferred MSVC
syntax is).
Christoph
--
----------------------------------------------
Dipl.-Inf., Dipl.-Math. Christoph Hertzberg
Cartesium 0.049
Universität Bremen
Enrique-Schmidt-Straße 5
28359 Bremen
Tel: +49 (421) 218-64252
----------------------------------------------