From mboxrd@z Thu Jan 1 00:00:00 1970 Path: main.gmane.org!not-for-mail From: "Eli Zaretskii" Newsgroups: gmane.emacs.devel Subject: Re: [PATCHES] patches for compiling GNU emacs 21.2 under Cygwin Date: Wed, 27 Nov 2002 20:43:05 +0300 Sender: emacs-devel-admin@gnu.org Message-ID: <7263-Wed27Nov2002204304+0200-eliz@is.elta.co.il> References: Reply-To: Eli Zaretskii NNTP-Posting-Host: main.gmane.org X-Trace: main.gmane.org 1038422679 26973 80.91.224.249 (27 Nov 2002 18:44:39 GMT) X-Complaints-To: usenet@main.gmane.org NNTP-Posting-Date: Wed, 27 Nov 2002 18:44:39 +0000 (UTC) Cc: emacs-devel@gnu.org Return-path: Original-Received: from quimby.gnus.org ([80.91.224.244]) by main.gmane.org with esmtp (Exim 3.35 #1 (Debian)) id 18H7Aq-00070l-00 for ; Wed, 27 Nov 2002 19:44:36 +0100 Original-Received: from monty-python.gnu.org ([199.232.76.173]) by quimby.gnus.org with esmtp (Exim 3.12 #1 (Debian)) id 18H7HY-0000Lc-00 for ; Wed, 27 Nov 2002 19:51:32 +0100 Original-Received: from localhost ([127.0.0.1] helo=monty-python.gnu.org) by monty-python.gnu.org with esmtp (Exim 4.10) id 18H7AZ-0002Kv-00; Wed, 27 Nov 2002 13:44:19 -0500 Original-Received: from list by monty-python.gnu.org with tmda-scanned (Exim 4.10) id 18H79w-0002A7-00 for emacs-devel@gnu.org; Wed, 27 Nov 2002 13:43:40 -0500 Original-Received: from mail by monty-python.gnu.org with spam-scanned (Exim 4.10) id 18H79u-00029p-00 for emacs-devel@gnu.org; Wed, 27 Nov 2002 13:43:39 -0500 Original-Received: from thor.inter.net.il ([192.114.186.11]) by monty-python.gnu.org with esmtp (Exim 4.10) id 18H79t-00029O-00 for emacs-devel@gnu.org; Wed, 27 Nov 2002 13:43:37 -0500 Original-Received: from zaretsky (adsl-ayalon-pc-129-228.inter.net.il [213.8.129.228]) by thor.inter.net.il (Mirapoint Messaging Server MOS 3.2.1-GA) with ESMTP id APL42012; Wed, 27 Nov 2002 20:43:29 +0200 (IST) Original-To: jbuehler@hekimian.com X-Mailer: emacs 21.3.50 (via feedmail 8 I) and Blat ver 1.8.9 In-reply-to: (message from Joe Buehler on Thu, 26 Sep 2002 15:10:30 -0400) Errors-To: emacs-devel-admin@gnu.org X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.0.11 Precedence: bulk List-Help: List-Post: List-Subscribe: , List-Id: Emacs development discussions. List-Unsubscribe: , List-Archive: Xref: main.gmane.org gmane.emacs.devel:9711 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:9711 > From: Joe Buehler > Date: Thu, 26 Sep 2002 15:10:30 -0400 > > Attached are patches required to get emacs 21.2 up and running under Cygwin. > > Some of the patches have already been submitted, and so are split into a separate > attachment. I include them here for completeness, in case they never made it > into CVS yet. > > Others are submitted here for the first time -- I have been waiting for my VP > to sign a copyright assignment for FSF. I just got it and will get it in > the mail tomorrow. First, thank you for your contribution. And sorry for such a long delay in reviewing these patches. Below are some comments based on reading the patches. (I cannot actually apply the patches and try using them, since I don't have the Cygwin development enviroment installed.) Please provide ChangeLog entries for each change. Please also explain the changes the reason for which is not self-evident. For example, in this change to Makefile.in: - (cd $(docdir); chmod a+r DOC*; rm DOC); \ + (cd ${docdir}; chmod a+r DOC*; if test "`echo DOC-*`" != "DOC-*"; \ + then rm DOC; fi); \ why is it necessary to add this test? Is there something special in the way the Cygwin port of Bash expands wildcards? The purpose of this change is also not clear enough: +#ifdef CANNOT_DUMP + FRAME_FOREGROUND_PIXEL(f) = FACE_TTY_DEFAULT_FG_COLOR; + FRAME_BACKGROUND_PIXEL(f) = FACE_TTY_DEFAULT_BG_COLOR; +#endif What is special about systems that cannot dump that requires this fragment? - (if (memq system-type '(hpux dgux usg-unix-v irix linux gnu/linux)) + (if (memq system-type '(hpux dgux usg-unix-v irix linux gnu/linux cygwin)) Since we are introducing a new value for system-type, it should be documented in the ELisp manual, I think. In this change: +#ifdef CYGWIN +#define _CYGWIN_EXE_SUFFIX .exe +#else +#define _CYGWIN_EXE_SUFFIX +#endif + install: ${archlibdir} @echo @echo "Installing utilities for users to run." for file in ${INSTALLABLES} ; do \ $(INSTALL_PROGRAM) $(INSTALL_STRIP) $${file} ${bindir}/$${file} ; \ - chmod a+rx ${bindir}/$${file}; \ + chmod a+rx ${bindir}/$${file}_CYGWIN_EXE_SUFFIX; \ done for file in ${INSTALLABLE_SCRIPTS} ; do \ $(INSTALL_PROGRAM) ${srcdir}/$${file} ${bindir}/$${file} ; \ chmod a+rx ${bindir}/$${file}; \ done I don't really understand why is it necessary to introduce _CYGWIN_EXE_SUFFIX. On Windows, there's no need to run "chmod a+rx" on *.exe programs. If the issue is that there's no etags, say, but etags.exe, and that chmod will print an error message if run on a non-existent file, why not test if $$(file) exists and only run chmod if it does? Isn't that a simpler and cleaner solution? --- src/Makefile.in 2001-12-17 09:09:32.000000000 -0500 +++ src/Makefile.in 2002-08-02 11:59:25.000000000 -0400 @@ -836,7 +836,11 @@ emacs: temacs ${etc}DOC ${lisp} #ifdef CANNOT_DUMP rm -f emacs +#ifdef CYGWIN + ln temacs.exe emacs +#else ln temacs emacs +#endif I think it's cleaner to introduce the EXE variable here, make it empty on all platforms except Cygwin, and then use it everywhere, instead of adding many #ifdef's. +/* let's not be adventurous */ +#define SYSTEM_MALLOC 1 Why is that a good idea? Isn't it better to use gmalloc and ralloc? That should make an Emacs whose memory footprint does not grow that much, I think. What is so ``adventurous'' in using that? Also, what about many "#ifdef WINDOWSNT" and "#ifdef DOS_NT" that we have in the sources--are none of them appropriate for the Cygwin build? For example, what about setting system_eol_type (on coding.c) to CODING_CR_LF? Or about the part of editfns.c that gets the user id from the enviroment variable USERNAME? Or support for drive letters in file names in fileio.c? Isn't it better not to lose these and other Windows-specific features? I think Someone(tm) should also go through all the *.el files, look for code that's specific to system-type windows-nt, and see which ones should also work for system-type cygwin.