From: Lennart B. <len...@st...> - 2005-10-12 12:45:27
|
Keith MARSHALL wrote: >I'll defer to Earnie on this; I know he is not keen to have the >autotools explicitly identify MSYS, (as opposed to MINGW), as a >publicly supported build system, but maybe config.guess should be >an exception. If not, the MSYS sources should include a locally >patched config.guess, (so that _they_ at least recognise their own >native build environment); you should be able to drop this into >the generic GNU source tree, to work around this problem. > > Thanks, it makes me curious, I wonder what Earnie have against it. The current solution makes it a bit difficult to compile newer versions of for example Diffutils. I looked in MSYS packages/diffutils but I could not find any configure.guess. What is the reason for this? Is it somewhere else? >Without having studied the original code, here are some initial >comments on your patch:-- > >1) It will never be accepted, if you don't accompany it with an > appropriately formatted ChangeLog entry. > > I keep that in my head, but this patch was just for comments. >2) All you appear to have done is unconditionally force diff to > use text mode for *all* I/O streams. This is diametrically > opposed the the general consensus of opinion that the default > behaviour of diff should remain unchanged. > > That is just a mistake, I did not want to force diff to use text mode for cases. In this patch I tried to follow the GNU diff manual (but maybe there is some bug in this version of Diffutils?). I know this is not what was seen by you and several others consensus. (I just looked up the word "consensus". Does not that mean that all participants should agree?) I think it makes sense still because we where probably not aware of that part of the GNU diff manual that I quoted in an earlier message before. >>+/* I believe this should be set -LB */ >>+#define HAVE_SETMODE 1 >> >> > >3) No, it shouldn't. This defeats the purpose of configuring the > package. What should happen is that configure should check for > setmode, using `AC_CHECK_FUNCS([setmode])', thus adding the > `#define HAVE_SETMODE 1' to config.h, but only when it is > pertinent. The purpose of this fragment in system.h is to > ensure that, on systems where setmode isn't available, that > `#if HAVE_SETMODE' works properly. > > Yes, thanks, of course. I just meant that the configure process should have set it. Some afterthought: Could the problem that this was left has it roots in that MSYS truly tries to be a "GNU environment" on w32 that looks very much like the environment on GNU/Linux? Could it just be that some fine tuning should be made because of the fact that the underlying system in fact is w32? > If configure isn't doing this, then you should either specify > `-DHAVE_SETMODE=1' within CFLAGS, (on your configure command > line), or better, patch configure.ac to add the appropriate > test, and rerun autoconf and autoheader before you start. > > I do not understand the organisation of things here. (And I do not know configure.ac but that is not the trouble at the moment ;-) I can not find any configure.ac in MSYS packages/diffutils. Is this some older version of the configure scripts? >5) The second `for' loop is redundant here; just write the first > as: > > for (i = 0; i <= 1; i++) > if (0 <= inf[i].desc) > setmode (inf[i].desc, binary_I_O ? O_BINARY : O_TEXT); > > Thanks, just my stupidity when I was in a hurry compiling. > > >>This version works for the default (reading data as text) but fails >>for --binary for some reason. >> >> > >Perhaps because your patch precludes the possibility of `binary_I_O' >ever being set to one; (just a guess, at present). > > Probably, I will fix that. Thanks, Lennart |