bug-grep
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: MS-Windows build of Grep [2/4]


From: Jim Meyering
Subject: Re: MS-Windows build of Grep [2/4]
Date: Fri, 30 Dec 2011 11:37:59 +0100

Paul Eggert wrote:
> On 12/28/11 19:53, Eli Zaretskii wrote:
>> So for this to work, the configure script should add the necessary -I switch
>
> OK, thanks, here's a patch to do that it.
> It also moves the ms-specific stuff to src/ms, as you suggested.

Thanks for the clean up.

> I'd push this, except that I don't have commit rights
> on the 'grep' project.  Perhaps I should?

Ok.  I've added you to the list.
Please post before pushing, and preferably wait for an ACK.

> One other thing: the SAME_INODE change looks like it might be something
> we could merge back into gnulib.  I.e., it returns -1
> for "I don't know" on platforms that don't do inodes.
> Callers will need to be changed in gnulib and elsewhere, no doubt.

I find the new semantics undesirable.
SAME_INODE as a boolean is easy to read.
When it becomes ternary, you have to handle the new possibility
of non-POSIX file systems.  That is such a fundamental difference,
that it would require significant change to sensitive parts of
coreutils and gnulib.  I really do not want to invest in supporting
such a platform when doing so will (at the very least) clutter up
already involved code with new bits to handle this unusual case.

More importantly, we should not let an effort to accommodate such a
hamstrung and fringe system influence the semantics of functions/macros
that should remain relevant for all systems in the long run.  Eventually
no-inode systems will go away (in a way, they're already gone), and when
they are no longer worth supporting in grep I would like to be able to
remove any old support easily.  From that perspective, I like your new
use of SAME_INODE, but would prefer to keep the heuristic guard (easily
removable) rather than change the semantics of the macro.  Would you
please put that in a separate commit?

>>From a32384b0d025a9d86787622686849599ec3416ca Mon Sep 17 00:00:00 2001
> From: Paul Eggert <address@hidden>
> Date: Thu, 29 Dec 2011 11:08:29 -0800
> Subject: [PATCH] ms: move Microsoft-specific stuff to src/ms
>
> * configure.ac (GREP_SRC_INCLUDES): New macro.
> * src/Makefile.am (DEFAULT_INCLUDES): New macro.
> * src/main.c (hstdout, norm_attr, w32_console_init, w32_sgr2attr)
> (pr_sgr_start, w32_clreol, rr_sgr_end) [__MINGW32__]:
> Move to new file src/ms/colorize.h.
> (colorize_init): Rename from w32_console_init; caller changed.
> (grepdir): Use (new) SAME_INODE instead of a heuristic.
> (should_colorize): Move to colorize.h.
> * src/colorize.h, src/ms/colorize.h: New files, taken from src/main.c.
> The usual one is src/colorize.h; src/ms/colorize.h overrides it
> on Microsoft platforms.
> * src/ms/same-inode.h: New file, to override lib/same-inode.h on
> Microsoft platforms.

Please repost without the SAME_INODE changes, so as not to mix
code motion and another type of change in the same change set.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]