Lionel Elie Mamane
lionel at mamane.lu
Fri Aug 15 16:24:29 UTC 2008
(Switching that thread to
debian-science-maintainers at lists.alioth.debian.org)
On Thu, Aug 14, 2008 at 10:25:29PM +0000, Achim D. Brucker wrote:
> I prepared a first package for Poly/ML
I had a look yesterday. If you feel strongly about any issue I raise,
feel free to argue with me; being your sponsor does not automatically
make me right :)
- I saw that you took the pain to exterminate compiler warnings;
in general, if you want to do that, it is not wrong per se, but in
my opinion, it is a bit of a time waste. So don't feel like you
Also consider that every divergence from upstream has a non-zero
cost; it makes it more work to package a new upstream version (need
to port the patch to the new upstream version, conflict with other
patches we may want to apply, ...). So it should be done only if
there is an advantage to the patch. Frankly, the advantage to
removing compiler warnings... Do you see any?
Note that these problems just fade away if upstream takes the patch
and applies it. In a nutshell, in general, exterminating compiler
warnings is an upstream work, and not a packaging work. There is
nothing wrong in doing upstream work from within Debian (we should
not shy away from making the software we ship better), but
1) don't feel forced to
2) upstream work can be done within Debian, but should generally be
sent back upstream!
Another side of the issue is that if upstream work is necessary to
get the package into good enough shape for Debian, then, well, you
have to do it :)
- Now, about the specific compiler warnings in compiling polyml:
* "deprecated conversion from string constant to 'char*"' (as
opposed to "const char*") and "passing argument N of function
drops constant qualifier from XXXX"
This kind of warnings, about things that are deprecated, but
still work, are a perfect example of warnings that are not useful
to fix within Debian only - it is useful to do it upstream, but
doing it within Debian, the cost is not balanced by any actual
advantage - yet; when it stops working, you need to do it. OK,
some kind of weak future warning.
These kind of warnings are also only worth fixing at all if you
fix the root of the problem, not only put a drape in front of the
problem. In my understanding, that is what you did with all those
(char*) casts. All they do is shut gcc up. The root of the
problem is that libtif2 does not use const qualifiers in its API
for strings it does not change. The problem is there. Trying to
"fix" it in every caller is just, in my opinion, insanity.
(Adding const qualifiers in the struct type definitions is
another story; that change looks fine, but send it upstream,
else we have to maintain that change, for no gain. You can put
it in Debian in the meantime - actually that's a good way to
test it - but if upstream doesn't take the patch, drop it.)
IMO, the best way to shut gcc up on this point is compiling with
-Wno-write-strings, not adding casts everywhere.
* warning: dereferencing type-punned pointer will break strict-aliasing rules
That's actually a warning that bothers me quite a bit
more. Warning about a real problem. To me, it suggests that it
should compile with -fno-strict-aliasing. It seems someone tried
to do that, because -fno-strict-aliasing is in CFLAGS, but not in
- Your changes also do some pure whitespace-damage (spaces that
become tabs or vice-versa, removing or adding spaces at the end of
a line, ...). That's usually a bad idea, because it creates
artificial conflicts in merges later, pollutes diffs, etc.
- The "arm" Debian port is dying, and being replaced by "armel". Put
"armel" instead of "arm" in the Architecture field of the packages.
- You commented and uncommented dh_* calls so many times I got
confused. I need to look at that again after lunch.
That's all for now.
More information about the debian-science-maintainers