[MPlayer-dev-eng] [rfc] committing rules and other policies

Rich Felker dalias at aerifal.cx
Sun Feb 25 21:30:57 CET 2007


On Sun, Feb 25, 2007 at 11:37:53AM +0100, Luca Barbato wrote:
> Recent complaints about Uoti behaviour lead to a long thread, here I'm
> asking some questions that were posed there:
> 
> - rules, which are the rules:
>   - coding style?
> 	Is it useful (yes)? Which one(ffmpeg one)?

My view is same as always: whatever style (or lack thereof) the
maintainer of the file likes, as long as it has a maintainer. Even if
it doesn't have a maintainer, if the file has a consistent style from
the original author it would be nice for that to be preserved.

>   - commit rules?
> 	do the history worth much (as in svn cp)?

Absolutely.

> 	cosmetics vs functional changes?

Should always be separated. This makes it possible to review commits.
In principle it's even possible to machine-test that a commit is
purely cosmetic. Perhaps we should have a pre-commit hook that
looks for "**cosmetic**" in the commit message:
- if it finds it, try compiling before and after and reject if the .o
files no not match
- if it doesn't find it, reject any changes in indention/etc.

Before getting pissed off keep in mind the above is half a joke...
I'll leave it to our evil dictator Diego to implement... ;)

> 	one change per commit?
> 	What about large changes?

Not entirely sure. IMO MPlayer should be in a working, compilable
state at any revision number, unless someone makes a mistake and
forgets to commit other dependent changes. But it's still nice to
split changes up as much as possible within this framework.

>   - code compatibility (gcc version?)
> 	do we really support gcc-2.95 (ppc does not, amd64 does not,
> 	sparc does?, mips does?)?

Your mention of specific archs shows that the issue is not trivial to
understand. These archs DO support gcc-2.95 because mplayer and ffmpeg
are portable C code (C89 + minor C99 features that have been in gcc
for over a decade)...aside from amd64 which is newer than gcc-2.95. :)
The question here is whether the vector ops are supported.

The relevant principle here is principle of least dependency.
Regardless of whether you want to explicitly support gcc-2.95 or not,
there is no reason to introduce unnecessary dependency. That's the C89
versus C99 issue. As far as I know, there's no remotely-working C
compiler that does not support the minor C99 features MPlayer and
ffmpeg are using now. There are plenty of compilers that don't support
full C99, _including_ latest gcc! So arguing that we should be free to
use whatever C99 constructs we want is obviously nonsensical.

There are plenty of projects I've had some involvement with, such as
ELinks, which insist on using pure C89 and seem to plan on doing so
for the forseeable future. Any claim of "backwardsness" is just
trolling. Well-managed projects will always follow a principle of
least dependency.

The second question here is forced upgrades. That's where the
asm/arch-specific stuff comes in. There is an asm syntax that's been
supported in gcc ever since gcc had has inline asm. It goes back at
least to 2.7 and probably 2.6 or 2.5. Do the research if you care.

Then, there's a second inline asm syntax that keeps changing with
every "modern" release of gcc. Following this new, moving target is
akin to using the C++ language or writing Linux-kernel software. The
rules keep changing under you.

Here the answer is much less clear. There's no standard to go by. What
MPlayer has been doing for x86, and what I believe is the correct
solution according to the principle that forced upgrades are bad, is
to follow the interface that's been stable for 10+ years, and restrict
using constructs that worked in that old interface but that have been
broken by recent instability.

FYI to the flamers^H: This is my last word on the issue in this
thread. I don't intend to reopen old arguments and waste everybody's
time. You know who you are and keep in mind that people are already
mad enough at you right now...

Rich



More information about the MPlayer-dev-eng mailing list