Build failure on M$ (using MSVC): Patch enclosed

Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

Hi,
 The current HEAD does not build with MSVC compiler on M$ (XP). There
is still an issue with the emacs.ico file that this patch does not fix
(I just copy the emacs22.ico as emacs.ico to get my build). I will try
getting a working emacs using MSVC on M$ as IMO, debugging is much
easier using the M$ debugger than GDB on M$ platforms.

diff --git a/src/font.c b/src/font.c
index 4f75615..85eb615 100644
--- a/src/font.c
+++ b/src/font.c
@@ -22,7 +22,14 @@ along with GNU Emacs.  If not, see
<http://www.gnu.org/licenses/>.  */
 #include <config.h>
 #include <stdio.h>
 #include <stdlib.h>
+#ifdef WINDOWSNT
+#include <string.h>
+#ifndef strcasecmp
+#define strcasecmp(a,b) stricmp(a,b)
+#endif
+#else
 #include <strings.h>
+#endif
 #include <ctype.h>
 #ifdef HAVE_M17N_FLT
 #include <m17n-flt.h>
diff --git a/src/fontset.c b/src/fontset.c
index 82e5b4e..f3d17ea 100644
--- a/src/fontset.c
+++ b/src/fontset.c
@@ -66,6 +66,12 @@ along with GNU Emacs.  If not, see
<http://www.gnu.org/licenses/>.  */
 #define xassert(X)	(void) 0
 #endif	/* not FONTSET_DEBUG */

+#ifdef WINDOWSNT
+#ifndef strcasecmp
+#define strcasecmp(a,b) stricmp(a,b)
+#endif
+#endif
+
 EXFUN (Fclear_face_cache, 1);

 /* FONTSET

-dhruva

-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

Hi,
 I tried doing a 'nmake bootstrap' and notice and infinite recursion
(or loop) in garbage collection. The call to 'mark_object' never comes
out. The following command (for emacs built with MSVC on WXP)
reproduces the problem consistently. I will try getting a proper back
trace and mail it.

"C:\users\dhruva\stub\repo\git\emacs\lisp/../bin/emacs.exe" -batch
--no-init-file --no-site-file --multibyte  -l autoload  --eval "(setq
generate-autoload-cookie \";;;###mh-autoload\")"  --eval "(setq
find-file-suppress-same-file-warnings t)"  --eval "(setq
make-backup-files nil)"  -f w32-batch-update-autoloads
"C:/users/dhruva/stub/repo/git/emacs/lisp/mh-e/mh-loaddefs.el" ./mh-e

-dhruva

-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

Hi,
 I figured out the problem. A call to find-file calls
hack-project-variables which calls project-find-settings-file which
tries to search for the file from the root (C: for me). It keeps
looping on "C:\.dir-settings.el". I saw that in a call to stat. I just
created an empty .dir-settings.el under the "C:" and the build is
going on fine (with my earlier path to fix the compilation error -
that needs to be made MSVC specific as it works with GCC/MinGW). I
will try to find the real cause for this looping once I am done with
the build.

-dhruva

On Tue, May 20, 2008 at 11:40 AM, dhruva <dhruvakm@gmail.com> wrote:
> Hi,
>  I tried doing a 'nmake bootstrap' and notice and infinite recursion
> (or loop) in garbage collection. The call to 'mark_object' never comes
> out. The following command (for emacs built with MSVC on WXP)
> reproduces the problem consistently. I will try getting a proper back
> trace and mail it.
>
> "C:\users\dhruva\stub\repo\git\emacs\lisp/../bin/emacs.exe" -batch
> --no-init-file --no-site-file --multibyte  -l autoload  --eval "(setq
> generate-autoload-cookie \";;;###mh-autoload\")"  --eval "(setq
> find-file-suppress-same-file-warnings t)"  --eval "(setq
> make-backup-files nil)"  -f w32-batch-update-autoloads
> "C:/users/dhruva/stub/repo/git/emacs/lisp/mh-e/mh-loaddefs.el" ./mh-e
>
> -dhruva
>
> --
> Contents reflect my personal views only!
>



-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

The below patch fixes the bug of infinite looping.

diff --git a/lisp/files.el b/lisp/files.el
index ee5efe4..c680d32 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3120,8 +3120,9 @@ If the file is in a registered project, a cons from
 `project-directory-alist' is returned.
 Otherwise this returns nil."
   (let ((dir (file-name-directory file))
+       (root (expand-file-name "/"))
        (result nil))
-    (while (and (not (string= dir "/"))
+    (while (and (not (string= dir root))
                (not result))
       (cond
        ((setq result (assoc dir project-directory-alist))

-dk

On Tue, May 20, 2008 at 1:56 PM, dhruva <dhruvakm@gmail.com> wrote:
> Hi,
>  I figured out the problem. A call to find-file calls
> hack-project-variables which calls project-find-settings-file which
> tries to search for the file from the root (C: for me). It keeps
> looping on "C:\.dir-settings.el". I saw that in a call to stat. I just
> created an empty .dir-settings.el under the "C:" and the build is
> going on fine (with my earlier path to fix the compilation error -
> that needs to be made MSVC specific as it works with GCC/MinGW). I
> will try to find the real cause for this looping once I am done with
> the build.
>
> -dhruva
>
> On Tue, May 20, 2008 at 11:40 AM, dhruva <dhruvakm@gmail.com> wrote:
>> Hi,
>>  I tried doing a 'nmake bootstrap' and notice and infinite recursion
>> (or loop) in garbage collection. The call to 'mark_object' never comes
>> out. The following command (for emacs built with MSVC on WXP)
>> reproduces the problem consistently. I will try getting a proper back
>> trace and mail it.
>>
>> "C:\users\dhruva\stub\repo\git\emacs\lisp/../bin/emacs.exe" -batch
>> --no-init-file --no-site-file --multibyte  -l autoload  --eval "(setq
>> generate-autoload-cookie \";;;###mh-autoload\")"  --eval "(setq
>> find-file-suppress-same-file-warnings t)"  --eval "(setq
>> make-backup-files nil)"  -f w32-batch-update-autoloads
>> "C:/users/dhruva/stub/repo/git/emacs/lisp/mh-e/mh-loaddefs.el" ./mh-e
>>
>> -dhruva
>>
>> --
>> Contents reflect my personal views only!
>>
>
>
>
> --
> Contents reflect my personal views only!
>



-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[Jason Rumney]

dhruva wrote:
> The below patch fixes the bug of infinite looping.
>
> diff --git a/lisp/files.el b/lisp/files.el
> index ee5efe4..c680d32 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -3120,8 +3120,9 @@ If the file is in a registered project, a cons from
>  `project-directory-alist' is returned.
>  Otherwise this returns nil."
>    (let ((dir (file-name-directory file))
> +       (root (expand-file-name "/"))
>         (result nil))
> -    (while (and (not (string= dir "/"))
> +    (while (and (not (string= dir root))
>                 (not result))
>        (cond
>         ((setq result (assoc dir project-directory-alist))
>   

I don't think this is sufficient for the case when the current directory 
is say "C:/some_dir/" and file is "D:/file.txt"

Should this new project code be enabled by default? It seems to add a 
lot of directory searching overhead for simple file visiting, especially 
in batch mode.

Re: Build failure on M$ (using MSVC): Patch enclosed

[David Kastrup]

Jason Rumney <jasonr@gnu.org> writes:

> dhruva wrote:
>> The below patch fixes the bug of infinite looping.
>>
>> diff --git a/lisp/files.el b/lisp/files.el
>> index ee5efe4..c680d32 100644
>> --- a/lisp/files.el
>> +++ b/lisp/files.el
>> @@ -3120,8 +3120,9 @@ If the file is in a registered project, a cons from
>>  `project-directory-alist' is returned.
>>  Otherwise this returns nil."
>>    (let ((dir (file-name-directory file))
>> +       (root (expand-file-name "/"))
>>         (result nil))
>> -    (while (and (not (string= dir "/"))
>> +    (while (and (not (string= dir root))
>>                 (not result))
>>        (cond
>>         ((setq result (assoc dir project-directory-alist))
>>   
>
> I don't think this is sufficient for the case when the current
> directory is say "C:/some_dir/" and file is "D:/file.txt"

Probably, yes.  But if the problem is that the loop gets stuck in an
upward traversal, then it should be sufficient to remember the last loop
value and quit when the next one happens to be the same.

> Should this new project code be enabled by default? It seems to add a
> lot of directory searching overhead for simple file visiting,
> especially in batch mode.

Probably would make sense to make this work only in interactive
sessions.

Anyway, the problems are so very similar to that of vc-find-root that it
should use the same function.  It could make sense giving it a more
general name and moving it together with vc-ignore-dir-regexp to
files.el.

But there is no sense in solving the associated problem more than once.

-- 
David Kastrup

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

diff --git a/lisp/files.el b/lisp/files.el
index ee5efe4..48b992b 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3120,8 +3120,9 @@ If the file is in a registered project, a cons from
 `project-directory-alist' is returned.
 Otherwise this returns nil."
   (let ((dir (file-name-directory file))
+       (prev "")
        (result nil))
-    (while (and (not (string= dir "/"))
+    (while (and (not (string= dir prev))
                (not result))
       (cond
        ((setq result (assoc dir project-directory-alist))
@@ -3130,6 +3131,7 @@ Otherwise this returns nil."
        ((file-exists-p (expand-file-name ".dir-settings.el" dir))
        (setq result (expand-file-name ".dir-settings.el" dir)))
        (t
+       (setq prev dir)
        (setq dir (file-name-directory (directory-file-name dir))))))
     result))


On Tue, May 20, 2008 at 3:51 PM, David Kastrup <dak@gnu.org> wrote:
> Jason Rumney <jasonr@gnu.org> writes:
>
>> dhruva wrote:
>>> The below patch fixes the bug of infinite looping.
>>>
>>> diff --git a/lisp/files.el b/lisp/files.el
>>> index ee5efe4..c680d32 100644
>>> --- a/lisp/files.el
>>> +++ b/lisp/files.el
>>> @@ -3120,8 +3120,9 @@ If the file is in a registered project, a cons from
>>>  `project-directory-alist' is returned.
>>>  Otherwise this returns nil."
>>>    (let ((dir (file-name-directory file))
>>> +       (root (expand-file-name "/"))
>>>         (result nil))
>>> -    (while (and (not (string= dir "/"))
>>> +    (while (and (not (string= dir root))
>>>                 (not result))
>>>        (cond
>>>         ((setq result (assoc dir project-directory-alist))
>>>
>>
>> I don't think this is sufficient for the case when the current
>> directory is say "C:/some_dir/" and file is "D:/file.txt"


The above patch stores the previous value as sugg by DAK.

-dk

-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

The complete patch for fixing the build on MSVC and the infinite loop
in files.el:

diff --git a/lisp/files.el b/lisp/files.el
index ee5efe4..48b992b 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3120,8 +3120,9 @@ If the file is in a registered project, a cons from
 `project-directory-alist' is returned.
 Otherwise this returns nil."
   (let ((dir (file-name-directory file))
+	(prev "")
 	(result nil))
-    (while (and (not (string= dir "/"))
+    (while (and (not (string= dir prev))
 		(not result))
       (cond
        ((setq result (assoc dir project-directory-alist))
@@ -3130,6 +3131,7 @@ Otherwise this returns nil."
        ((file-exists-p (expand-file-name ".dir-settings.el" dir))
 	(setq result (expand-file-name ".dir-settings.el" dir)))
        (t
+	(setq prev dir)
 	(setq dir (file-name-directory (directory-file-name dir))))))
     result))

diff --git a/src/font.c b/src/font.c
index 4f75615..4081ef8 100644
--- a/src/font.c
+++ b/src/font.c
@@ -22,7 +22,14 @@ along with GNU Emacs.  If not, see
<http://www.gnu.org/licenses/>.  */
 #include <config.h>
 #include <stdio.h>
 #include <stdlib.h>
+#if defined(WINDOWSNT) && !defined(__GNUC__)
+#include <string.h>
+#ifndef strcasecmp
+#define strcasecmp(a,b) stricmp(a,b)
+#endif
+#else
 #include <strings.h>
+#endif
 #include <ctype.h>
 #ifdef HAVE_M17N_FLT
 #include <m17n-flt.h>
diff --git a/src/fontset.c b/src/fontset.c
index 82e5b4e..60e14e0 100644
--- a/src/fontset.c
+++ b/src/fontset.c
@@ -66,6 +66,12 @@ along with GNU Emacs.  If not, see
<http://www.gnu.org/licenses/>.  */
 #define xassert(X)	(void) 0
 #endif	/* not FONTSET_DEBUG */

+#if defined(WINDOWSNT) && !defined(__GNUC__)
+#ifndef strcasecmp
+#define strcasecmp(a,b) stricmp(a,b)
+#endif
+#endif
+
 EXFUN (Fclear_face_cache, 1);

 /* FONTSET


-dk

-- 
Contents reflect my personal views only!

RE: Build failure on M$ (using MSVC): Patch enclosed

[Herbert Euler]

Probably off topic.  I am used to search in ChangeLog entries for
finding code history.  I encountered the same problem when building
emacs on w32 today.  After reading dhruva's post, I opened ChangeLog
as usual, and saw those `project-*' functions are newly added.  Then
spontaneously I explored the change logs with CVS to understand the
situation, but I saw the changes between 1.976 and 1.977 were not
recorded in ChangeLog, and those changes are not trivial.

I think it is better to record those changes in ChangeLog.

Regards,
Guanpeng Xu
_________________________________________________________________
Connect to the next generation of MSN Messenger 
http://imagine-msn.com/messenger/launch80/default.aspx?locale=en-us&source=wlmailtagline

Re: Build failure on M$ (using MSVC): Patch enclosed

[Eli Zaretskii]

> Date: Tue, 20 May 2008 10:28:58 +0530
> From: dhruva <dhruvakm@gmail.com>
> 
> +#ifdef WINDOWSNT
> +#include <string.h>
> +#ifndef strcasecmp
> +#define strcasecmp(a,b) stricmp(a,b)
> +#endif

Thanks, but please put the strcasecmp define in src/s/ms-w32.h.
There's no need to infect general files with OS-specific stuff.
As a bonus, you will need only one such #define that will take care of
all the uses of strcasecmp.

Re: Build failure on M$ (using MSVC): Patch enclosed

[Eli Zaretskii]

> Date: Tue, 20 May 2008 14:28:04 +0530
> From: dhruva <dhruvakm@gmail.com>
> 
> -    (while (and (not (string= dir "/"))

Such unportable code should have never been allowed into the CVS, IMO.

Use of a literal "/" is almost always a telltale sign that the code
will only work on Posix filesystems.  Please avoid that.

Re: Build failure on M$ (using MSVC): Patch enclosed

[Eli Zaretskii]

> From: Herbert Euler <herberteuler@hotmail.com>
> Date: Tue, 20 May 2008 20:11:15 +0800
> 
> 
> Probably off topic.

No, it isn't.

> I am used to search in ChangeLog entries for
> finding code history.  I encountered the same problem when building
> emacs on w32 today.  After reading dhruva's post, I opened ChangeLog
> as usual, and saw those `project-*' functions are newly added.  Then
> spontaneously I explored the change logs with CVS to understand the
> situation, but I saw the changes between 1.976 and 1.977 were not
> recorded in ChangeLog, and those changes are not trivial.
> 
> I think it is better to record those changes in ChangeLog.

I think _all_ changes are supposed to be recorded in ChangeLog's as a
matter of policy.

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

diff --git a/lisp/files.el b/lisp/files.el
index ee5efe4..bb8a021 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3120,8 +3120,9 @@ If the file is in a registered project, a cons from
 `project-directory-alist' is returned.
 Otherwise this returns nil."
   (let ((dir (file-name-directory file))
-	(result nil))
-    (while (and (not (string= dir "/"))
+        (result nil)
+        (prevdir nil))
+    (while (and (not (string= dir prevdir))
 		(not result))
       (cond
        ((setq result (assoc dir project-directory-alist))
@@ -3130,6 +3131,7 @@ Otherwise this returns nil."
        ((file-exists-p (expand-file-name ".dir-settings.el" dir))
 	(setq result (expand-file-name ".dir-settings.el" dir)))
        (t
+        (setq prevdir dir)
 	(setq dir (file-name-directory (directory-file-name dir))))))
     result))

diff --git a/src/font.c b/src/font.c
index 69d1906..f7f9e33 100644
--- a/src/font.c
+++ b/src/font.c
@@ -22,7 +22,11 @@ along with GNU Emacs.  If not, see
<http://www.gnu.org/licenses/>.  */
 #include <config.h>
 #include <stdio.h>
 #include <stdlib.h>
+#ifdef _MSC_VER
+#include <string.h>
+#else
 #include <strings.h>
+#endif
 #include <ctype.h>
 #ifdef HAVE_M17N_FLT
 #include <m17n-flt.h>
diff --git a/src/s/ms-w32.h b/src/s/ms-w32.h
index 3e043c8..54311d1 100644
--- a/src/s/ms-w32.h
+++ b/src/s/ms-w32.h
@@ -510,6 +510,11 @@ extern void _DebPrint (const char *fmt, ...);
 #define DebPrint(stuff)
 #endif

+#ifdef _MSC_VER
+#ifndef strcasecmp
+#define strcasecmp(a,b) stricmp(a,b)
+#endif
+#endif

 /* ============================================================ */



On Tue, May 20, 2008 at 11:42 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 20 May 2008 10:28:58 +0530
>> From: dhruva <dhruvakm@gmail.com>
>>
>> +#ifdef WINDOWSNT
>> +#include <string.h>
>> +#ifndef strcasecmp
>> +#define strcasecmp(a,b) stricmp(a,b)
>> +#endif
>
> Thanks, but please put the strcasecmp define in src/s/ms-w32.h.
> There's no need to infect general files with OS-specific stuff.
> As a bonus, you will need only one such #define that will take care of
> all the uses of strcasecmp.
>

done.

-dky

-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[Stefan Monnier]

> diff --git a/lisp/files.el b/lisp/files.el
> index ee5efe4..bb8a021 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -3120,8 +3120,9 @@ If the file is in a registered project, a cons from
>  `project-directory-alist' is returned.
>  Otherwise this returns nil."
>    (let ((dir (file-name-directory file))
> -	(result nil))
> -    (while (and (not (string= dir "/"))
> +        (result nil)
> +        (prevdir nil))
> +    (while (and (not (string= dir prevdir))
>  		(not result))
>        (cond
>         ((setq result (assoc dir project-directory-alist))
> @@ -3130,6 +3131,7 @@ Otherwise this returns nil."
>         ((file-exists-p (expand-file-name ".dir-settings.el" dir))
>  	(setq result (expand-file-name ".dir-settings.el" dir)))
>         (t
> +        (setq prevdir dir)
>  	(setq dir (file-name-directory (directory-file-name dir))))))
>      result))

Actually, why doesn't this code use locate-dominating-file?


        Stefan

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

Hi,

On 5/21/08, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > diff --git a/lisp/files.el b/lisp/files.el
>  > index ee5efe4..bb8a021 100644
>  > --- a/lisp/files.el
>  > +++ b/lisp/files.el
>  > @@ -3120,8 +3120,9 @@ If the file is in a registered project, a cons from
>  >  `project-directory-alist' is returned.
>  >  Otherwise this returns nil."
>  >    (let ((dir (file-name-directory file))
>  > -     (result nil))
>  > -    (while (and (not (string= dir "/"))
>  > +        (result nil)
>  > +        (prevdir nil))
>  > +    (while (and (not (string= dir prevdir))
>  >               (not result))
>  >        (cond
>  >         ((setq result (assoc dir project-directory-alist))
>  > @@ -3130,6 +3131,7 @@ Otherwise this returns nil."
>  >         ((file-exists-p (expand-file-name ".dir-settings.el" dir))
>  >       (setq result (expand-file-name ".dir-settings.el" dir)))
>  >         (t
>  > +        (setq prevdir dir)
>  >       (setq dir (file-name-directory (directory-file-name dir))))))
>  >      result))
>
>
> Actually, why doesn't this code use locate-dominating-file?

Well, I was not aware of that function. Looking a little deeper into
it, it does not traverse above the current user $HOME (~/ or %HOME%).
This code needs to go all the way till the top. Since it does a
username comparison, it apprears a bit more expensive (based on gut
feel though).


my HOME is set to: c:/users/dhruva
My lisp expresssion: (locate-dominating-file
"c:/users/dhruva/stub/git/emacs/" ".dir-settings.el")
1. I place the .dir-settings.el under "C:/" and I get a nil
2. I move the file under HOME (c:/users/dhruva/.dir-settings.el) and get it.
=> It does not traverse beyond the HOME directory.

-dhruva

-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[Stefan Monnier]

> Well, I was not aware of that function. Looking a little deeper into
> it, it does not traverse above the current user $HOME (~/ or %HOME%).
> This code needs to go all the way till the top. Since it does a
> username comparison, it apprears a bit more expensive (based on gut
> feel though).

Indeed, it doesn't go up past the home directory and it doesn't lookup
past a directory with a different owner.  It's all done on purpose to
try and avoid pathological cases.
I don't see that as a problem at all.


        Stefan

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

On 5/22/08, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > Well, I was not aware of that function. Looking a little deeper into
>  > it, it does not traverse above the current user $HOME (~/ or %HOME%).
>  > This code needs to go all the way till the top. Since it does a
>  > username comparison, it apprears a bit more expensive (based on gut
>  > feel though).
>
>
> Indeed, it doesn't go up past the home directory and it doesn't lookup
>  past a directory with a different owner.  It's all done on purpose to
>  try and avoid pathological cases.
>  I don't see that as a problem at all.

Though I do not see a practical use case, here is a case where it can fail:

1. As a system wide policy, I put some settings file in the root
folder or the parent folder of every user folder which has a bunch of
defaults
2. I allow the use to over ride it by adding their own in which case
your changes will find the user modified file (as it resides under the
user HOME folder on UNIX because users will not have permission to
create anywhere above that. But not true on M$ as I have local admin
privilege and hence I can keep it anywhere in the tree).
3. Not all users might want to over ride the default settings and
would like to rely on the system wide policy where the current code
fails.

-dhruva

-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[David Kastrup]

dhruva <dhruvakm@gmail.com> writes:

> On 5/22/08, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> > Well, I was not aware of that function. Looking a little deeper into
>>  > it, it does not traverse above the current user $HOME (~/ or %HOME%).
>>  > This code needs to go all the way till the top. Since it does a
>>  > username comparison, it apprears a bit more expensive (based on gut
>>  > feel though).
>>
>>
>> Indeed, it doesn't go up past the home directory and it doesn't lookup
>>  past a directory with a different owner.  It's all done on purpose to
>>  try and avoid pathological cases.
>>  I don't see that as a problem at all.
>
> Though I do not see a practical use case, here is a case where it can fail:
>
> 1. As a system wide policy, I put some settings file in the root
> folder or the parent folder of every user folder which has a bunch of
> defaults

There is no necessity whatsoever to use the settings file mechanism for
global defaults.  Just set the appropriate variables in site-start.el.

On a different tack: why can't vc-find-root be made to use
locate-dominating-file?  One less bunch of heuristics to maintain.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

Hi,
 For fixing the build failure on M$ in src/font.c, there is a more
elegant alternative approach.

1. Remove the "#include <strings.h>" from that file
2. Define HAVE_STRINGS_H if using GCC or define HAVE_STRING_H if using
MSVC. src/config.h and nt/config.nt both include string/strings header
based on the above define.
3. Retain the strcasecmp to stricmp in src/s/ms-w32.h

-dhruva

-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

Hi,
On 5/22/08, David Kastrup <dak@gnu.org> wrote:
> There is no necessity whatsoever to use the settings file mechanism for
>  global defaults.  Just set the appropriate variables in site-start.el.

The locate-dominating-file restricts me to see in folders that are in
the hierarchy of my HOME folder only. At work, I have my home folder
at a different mount point and development tree in a different mount
point. They do not share the same tree apart from the root '/'. If I
(and other users) need to have default settings for the build tree, I
would put it in the mount folder of the build tree. With the recent
changes, it will never see is as the tree does not share my HOME.

Ex:
Build dir:
/mnt/dev/build/users/dhruva/branch/...
Home:
/u/users/dhruva

Using "site-start.el" will affect all users, this will not provide me
the fine grained control that I will get using the folder structure.
 I use a similar thing for my BASH shell. Every directory I need to
change some values (like CVSROOT for different projects) will have a
local.bashrc which gets sources once I visit it. I use this traversal
back to find the most appropriate local.bashrc and find it quite
useful.

IMHO, it must go beyond the
>  On a different tack: why can't vc-find-root be made to use
>  locate-dominating-file?  One less bunch of heuristics to maintain.

Either one will do as long as we have one function so that it is easy
to fix and improve.

-dhruva

-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[David Kastrup]

dhruva <dhruvakm@gmail.com> writes:

> Hi,
> On 5/22/08, David Kastrup <dak@gnu.org> wrote:
>> There is no necessity whatsoever to use the settings file mechanism for
>>  global defaults.  Just set the appropriate variables in site-start.el.
>
> The locate-dominating-file restricts me to see in folders that are in
> the hierarchy of my HOME folder only.

No, it doesn't.

> At work, I have my home folder at a different mount point and
> development tree in a different mount point. They do not share the
> same tree apart from the root '/'. If I (and other users) need to have
> default settings for the build tree, I would put it in the mount
> folder of the build tree. With the recent changes, it will never see
> is as the tree does not share my HOME.

Have you actually _tried_ it?

> Ex:
> Build dir:
> /mnt/dev/build/users/dhruva/branch/...
> Home:
> /u/users/dhruva
>
> Using "site-start.el" will affect all users, this will not provide me
> the fine grained control that I will get using the folder structure.

Your previous example was about affecting all users, and
locate-dominating-file works perfectly well with the current example.

locate-dominating-file stops the traversal when it would pass from
within $HOME to outside, or when the traversal would change the owner of
the directory.  But it does not require the traversal to start within
$HOME or elsewhere.

> I use a similar thing for my BASH shell. Every directory I need to
> change some values (like CVSROOT for different projects) will have a
> local.bashrc which gets sources once I visit it. I use this traversal
> back to find the most appropriate local.bashrc and find it quite
> useful.

Could you _please_ provide a test case which you actually tried out and
which does not work as desired before complaining?

I can't see this to be the case right now.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Build failure on M$ (using MSVC): Patch enclosed

[Jason Rumney]

dhruva wrote:

> Well, I was not aware of that function. Looking a little deeper into
> it, it does not traverse above the current user $HOME (~/ or %HOME%).
> This code needs to go all the way till the top. Since it does a
> username comparison, it apprears a bit more expensive (based on gut
> feel though).
>
>
> my HOME is set to: c:/users/dhruva
> My lisp expresssion: (locate-dominating-file
> "c:/users/dhruva/stub/git/emacs/" ".dir-settings.el")
> 1. I place the .dir-settings.el under "C:/" and I get a nil
> 2. I move the file under HOME (c:/users/dhruva/.dir-settings.el) and get it.
> => It does not traverse beyond the HOME directory.
>   

If you run (locate-dominating-file "c:/some/other/path" 
".dir-settings.el"), does it go all the way to the top, or does it not 
search at all?

I think it is OK to stop at ~/ when inside the users home directory, as 
that will be what most users want.

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

Hello,

On 5/22/08, Jason Rumney <jasonr@gnu.org> wrote:
>  If you run (locate-dominating-file "c:/some/other/path"
> ".dir-settings.el"), does it go all the way to the top, or does it not
> search at all?

It works fine, apologies for all the noise.

>  I think it is OK to stop at ~/ when inside the users home directory, as
> that will be what most users want.

I agree. Most, if not all user customizations reside under the home folder.

-dhruva

-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[dhruva]

Hi,
 The below patch fixes the string header file issue. Since there is
support to either include the strings.h or string.h based on some
defines, I have added them in the compiler specific files for M$. If
the same is added to other platforms, we will no longer have to
include string/strings header directly, which I guess was the
intention in having it in src/config.h or nt/config.nt.

-dhruva

diff --git a/nt/gmake.defs b/nt/gmake.defs
index 00ae686..5a7b28b 100644
--- a/nt/gmake.defs
+++ b/nt/gmake.defs
@@ -204,7 +204,7 @@ else
 DEBUG_CFLAGS	=
 endif
 CFLAGS          = -I. -DWIN32_LEAN_AND_MEAN -D_WIN32_WINNT=0x0400 \
-		  $(ARCH_CFLAGS) -D$(ARCH) -D_CRTAPI1=_cdecl \
+		  $(ARCH_CFLAGS) -D$(ARCH) -D_CRTAPI1=_cdecl -DHAVE_STRINGS_H \
 		  $(DEBUG_CFLAGS) $(USER_CFLAGS) $(LOCAL_FLAGS)
 EMACS_EXTRA_C_FLAGS = -DUSE_CRT_DLL=1 $(FONT_CFLAGS)

diff --git a/nt/nmake.defs b/nt/nmake.defs
index 7c6a518..8cf9f9e 100644
--- a/nt/nmake.defs
+++ b/nt/nmake.defs
@@ -142,7 +142,7 @@ DEBUG_CFLAGS	= -DEMACSDEBUG
 DEBUG_CFLAGS	=

 !endif

 CFLAGS          = -I. -DWIN32_LEAN_AND_MEAN -D_WIN32_WINNT=0x0400 \

-		  $(ARCH_CFLAGS) -D$(ARCH) -D_CRTAPI1=_cdecl \

+		  $(ARCH_CFLAGS) -D$(ARCH) -D_CRTAPI1=_cdecl -DHAVE_STRING_H \

 		  $(DEBUG_CFLAGS) $(USER_CFLAGS) $(LOCAL_FLAGS)

 EMACS_EXTRA_C_FLAGS = $(FONT_CFLAGS)



diff --git a/src/font.c b/src/font.c
index 84f2a2a..42e62d5 100644
--- a/src/font.c
+++ b/src/font.c
@@ -22,7 +22,6 @@ along with GNU Emacs.  If not, see
<http://www.gnu.org/licenses/>.  */
 #include <config.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <strings.h>
 #include <ctype.h>
 #ifdef HAVE_M17N_FLT
 #include <m17n-flt.h>
diff --git a/src/s/ms-w32.h b/src/s/ms-w32.h
index 3e043c8..b748b3b 100644
--- a/src/s/ms-w32.h
+++ b/src/s/ms-w32.h
@@ -510,6 +510,11 @@ extern void _DebPrint (const char *fmt, ...);
 #define DebPrint(stuff)
 #endif

+#ifndef __GNUC__
+#ifndef strcasecmp
+#define strcasecmp(a,b) stricmp(a,b)
+#endif
+#endif

 /* ============================================================ */


-- 
Contents reflect my personal views only!

Re: Build failure on M$ (using MSVC): Patch enclosed

[Jason Rumney]

dhruva wrote:
> Hi,
>  The below patch fixes the string header file issue. Since there is
> support to either include the strings.h or string.h based on some
> defines, I have added them in the compiler specific files for M$.

I think the proper fix is to consistently use either strcasecmp or 
stricmp (or xstricmp, which we define in xfaces.c, though the other two 
are widely enough used that it seems unnecessary to define yet another 
function). Historically we seem to have settled on stricmp except in 
dosfns.c and unexcw.c, which are only compiled on DOS and Cygwin 
respectively. Even those should be changed for consistency.

Re: Build failure on M$ (using MSVC): Patch enclosed

[Jason Rumney]

Jason Rumney wrote:
> dhruva wrote:
>> Hi,
>>  The below patch fixes the string header file issue. Since there is
>> support to either include the strings.h or string.h based on some
>> defines, I have added them in the compiler specific files for M$.
>
> I think the proper fix is to consistently use either strcasecmp or 
> stricmp (or xstricmp, which we define in xfaces.c, though the other 
> two are widely enough used that it seems unnecessary to define yet 
> another function). Historically we seem to have settled on stricmp 
> except in dosfns.c and unexcw.c, which are only compiled on DOS and 
> Cygwin respectively. Even those should be changed for consistency.

Actually, only Windows code seems to use stricmp in Emacs 22.2, so we 
should probably replace uses of both strcasecmp and stricmp with 
xstricmp for consistency.

>
>
>

Re: Build failure on M$ (using MSVC): Patch enclosed

[Miles Bader]

Jason Rumney <jasonr@gnu.org> writes:
>> I think the proper fix is to consistently use either strcasecmp or
>> stricmp (or xstricmp, which we define in xfaces.c, though the other
>> two are widely enough used that it seems unnecessary to define yet
>> another function). Historically we seem to have settled on stricmp
>> except in dosfns.c and unexcw.c, which are only compiled on DOS and
>> Cygwin respectively. Even those should be changed for consistency.
>
> Actually, only Windows code seems to use stricmp in Emacs 22.2, so we
> should probably replace uses of both strcasecmp and stricmp with
> xstricmp for consistency.

But let's change the name to xstrcasecmp, as strcasecmp is the posix/gnu
name.

-miles

-- 
Kilt, n. A costume sometimes worn by Scotchmen [sic] in America and Americans
in Scotland.

Re: Build failure on M$ (using MSVC): Patch enclosed

[Stefan Monnier]

> On a different tack: why can't vc-find-root be made to use
> locate-dominating-file?  One less bunch of heuristics to maintain.

Yes, this duplication hurts.  After all, I wrote locate-dominating-file
based on vc-find-root.  But currently locate-dominating-file can only be
used if the file we're looking for is directoly in the higher directory,
not in a subdirectory.  E.g. I can look for ".dir-settings.el" or "CVS"
but not for "CVS/Entries", as does vc-find-root.


        Stefan

Re: Build failure on M$ (using MSVC): Patch enclosed

[Stefan Monnier]

> The locate-dominating-file restricts me to see in folders that are in
> the hierarchy of my HOME folder only.

No, you can put them anywhere you want.

> at a different mount point and development tree in a different mount
> point. They do not share the same tree apart from the root '/'. If I
> (and other users) need to have default settings for the build tree, I
> would put it in the mount folder of the build tree. With the recent
> changes, it will never see is as the tree does not share my HOME.

Huh, what makes you think so?

> Ex:
> Build dir:
> /mnt/dev/build/users/dhruva/branch/...
> Home:
> /u/users/dhruva

You can plce your .dir-settings.el anywhere along
/mnt/dev/build/users/dhruva/branch/...
It probably will be ignored if it's higher than
/mnt/dev/build/users/dhruva since I guess the owner is different, but
/mnt/dev/build/users/dhruva/.dir-settings.el should work just fine.

> change some values (like CVSROOT for different projects) will have a

[ You normally don't need to set CVSROOT for each project. ]


        Stefan

Re: Build failure on M$ (using MSVC): Patch enclosed

[Eli Zaretskii]

> Date: Thu, 22 May 2008 12:26:11 +0530
> From: dhruva <dhruvakm@gmail.com>
> Cc: "Eli Zaretskii" <eliz@gnu.org>, emacs-devel@gnu.org
> 
>  For fixing the build failure on M$ in src/font.c, there is a more
> elegant alternative approach.
> 
> 1. Remove the "#include <strings.h>" from that file
> 2. Define HAVE_STRINGS_H if using GCC or define HAVE_STRING_H if using
> MSVC. src/config.h and nt/config.nt both include string/strings header
> based on the above define.
> 3. Retain the strcasecmp to stricmp in src/s/ms-w32.h

Actually, we shouldn't be using strings.h at all, since it's a
non-standard header.

Re: Build failure on M$ (using MSVC): Patch enclosed

[Jason Rumney]

Eli Zaretskii wrote:
> Actually, we shouldn't be using strings.h at all, since it's a
> non-standard header.
>   

It's conditionally included from config.h, so there is no need to 
explicitly include it in font.c, but there must be some reason why it is 
included there.
I've fixed the rest of the problem by standardizing on using xstrcasecmp 
(renamed from xstricmp per Miles's suggestion) in all places where 
case-insesitive string comparison is done. The only exception is 
unexcw.c, which did not include dispextern.h directly, and since I don't 
have a Cygwin build environment set up, I could not test if it is 
included indirectly as was the case with w32.c.