lynx-dev
[Top][All Lists]
Advanced

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

lynx-dev overuse of system-specific conditionals (was: cygwin patch)


From: Klaus Weide
Subject: lynx-dev overuse of system-specific conditionals (was: cygwin patch)
Date: Mon, 1 May 2000 14:11:35 -0500 (CDT)

On Sun, 30 Apr 2000, Doug Kaufman wrote:
> On Sun, 30 Apr 2000, Klaus Weide wrote:
> 
> > On Sun, 30 Apr 2000, Doug Kaufman wrote:
> > > There are other uses of DOSPATH that could affect cygwin, but
> > > they don't all seem appropriate for cygwin. In addition, I
> > > believe that we agreed that DOSPATH is deprecated, since it
> > > had been used for things other than the path itself. 

If it's deprecated, it should have long been gone...  I'd prefer to
say, using it to mean all-kinds-of-doslike-behavior is deprecated (and
should be eliminated), but not using it to mean "use DOS-like filepath
syntax for local files".

> > > I think
> > > we are now trying to use individual defines (e.g. __DJGPP__
> > > || _WINDOWS || __EMX__ || __CYGWIN__), 
> > 
> > I find that quite horrible.  And it seems there are constantly
> > problems with keeping track of this kind of stuff, judging from
> > the number of patches that fiddle around just with ifdef-conditions.
> 
> I agree that this is horrible. I am just not sure what the best
> solution is. One of the problems is that I don't know who of the
> developers is familiar enough with the different platforms to know
> which changes are fine for multiple platforms and which are specific
> to individual platforms. 

Probably nobody is familiar with all the different platforms - even if
Tom did test compilation on all of them (I don't know which ones he
covers), he probably wouldn't discover platform-specific problems that
occur only at runtime.

But at leat the person who added an '#ifdef __CYGWIN__' (or whatever)
must know what he meant; he should know why he added that, whether it's
required or just "better", and what kind of behavior the patch changes.

> I haven't had any trouble with the cygwin
> port and cp on my own computer, when compiled with full pathnames.
> When I tried to compile without pathnames to create a binary that
> might be a candidate for distribution, it picked up my DJGPP cp which
> comes earlier in the path, and failed. I guess the question for
> distributable binaries is whom they are aimed at. If aimed at (e.g.,
> for cygwin) the usual Windows user who just wants to run lynx, it
> needs to work with as little special knowledge and setup as possible.
> I also thought that different binaries for the same platform (i.e.
> Borland or cygwin compiled) should have compatible user interfaces, as
> much as possible. This does conflict with keeping cygwin as close to
> possible as unix.

I don't mean that the behavior of distributed binaries should change;
that's really up to you (and others who package lynx binaries), I trust
that you know better than I what makes more sense for typical Windows
situations (from your personal use, as well as feedback you get from
others).

But these choices shouldn't be hardwired into the source code in such
a way that it gets near-impossible to disentagle them, for someone who
looks at the source code (and perhaps doesn't even know what DJGPP or
CYGWIN is).

> > > so that quirks of each system can be addressed individually.
> > 
> > Many of them don't seem to address quirks of the "system" identified
> > by the preprocessor macro (in this case, the cygwin environment), but
> > some preference that someone (using that environment) thinks is
> > more appropriate.

I mean, "don't use cp in a cygwin environment" is a preference, a choice
you made, not something that is required to run in a cygwin environment.
You think it's more appropriate as a default, and I have no problem with
that (but some questions, see below).

> Indeed, the SH_EX changes are Senshu Hiroyuki's preferences, which
> have not yet been accepted for general use. 

As well as WIN_EX?

> Clearly, anyone who compiles their own binary can change the code as
> they like.

My point is, it shouldn't be unnecessarily difficult to do so.

> The
> question is what should be the default, and how easily should it
> be configurable. At the other end of the spectrum we have multiple
> defines in lynx.cfg. It seems too much to put all the choices in
> lynx.cfg. Some must be made in the code itself. 

perhaps YM userdefs.h?

Anyway, I'd say don't be afraid to add a new preprocessor symbol
where it makes sense.  They don't all need to be exposed in userdefs.h
or in the makefile(s).  For example, for the "use cp or not" question,
something like
   /*
    * Copy a file
    */
   PUBLIC int LYCopyFile ARGS2(
           char *,         src,
           char *,         dst)
   {
   #ifdef USE_EXTERNAL_COPY_COMMAND
   ...

together with

   #if !(defined(DOSPATH) || defined(__CYGWIN__))
   /* We invoke an external 'cp' for copying files, except for
    * for DOS-like environments, because ... */
   #define USE_EXTERNAL_COPY_COMMAND
   #endif

would be better than the current situation.  That #define could be just
at the top of LYUtils.c, since in this case no other source files are
affected (needs checking).  (Consider also adding to README.defines.)
It's more obvious what the alternatives in LYCopyFile mean, easier
to adapt if some new platform should be added in the future, easier
to see what needs changing if someone wants to always avoid 'cp'
(even for UNIX).

> At this point, at
> least for platforms where unsophisticated users may be majority of
> users, I would favor uniformity within the platform.

(I'm not sure this includes cygwin.)

> Since most Windows users wouldn't have cp, this saves the need to
> distribute a compatible cp with the lynx binary.

Most Windows users wouldn't, but what about "most Windows users with
the cygwin library installed"?

> > I'd expect a "cygwin port" of lynx to use the
> > for-Unix code as much as possible (better tested, less bugs IMO),
> > and let cygwin do its job of emulating a Unix-like environment.
> 
> This works best when the user has all the other unix-like tools and
> programs installed. It is a potential problem for stand-alone programs
> for distribution.

(How "standalone" can a cygwin port be?  Users need to install the cygwin
stuff if they don't have it already; and if they do, won't they have
a working 'cp', too?)

> > Others may disagree (they apparently do), but that shouldn't be
> > forced on all users of cygwin.
> 
> We need a few more people to try compiling a cygwin version of lynx,
> so that we can get varied feedback as to where it works well and where
> problems need to be fixed.
>   
> > So if all the input to lynx is in Unix-style form, and output is
> > only fed to things that understand Unix-style output, where is the
> > problem?  I'd expect lynx to work in that mode as it does on a Unix-like
> 
> One of the problems is feeding output to programs that don't
> understand unix-style output, such as telnet. I have a telnet shell
> script that calls the Windows program, making appropriate conversions.

I don't understand why telnet is a problem.  The telnet process itself
doesn't get any output from lynx, unix-style or not; so the only question
that can arise is about the command invocation, but the #defines in
userdefs.h for TELNET_PATH (etc.) should handle that.

If userdefs.h has the wrong type of syntax for the platform, then lynx
is not configured correctly.

> > > I believe that there are
> > > cygwin functions to convert, but they would have to be inserted
> > > appropriately into the unix code (cygwin_conv_to_full_posix_path()
> > > and cygwin_conv_to_full_win32_path()). 
> > 
> > I disagree with the premise that all users of a cygwin-using lynx
> > would want or need such extra code.
> 
> I haven't recommended putting them in, and haven't in my personal
> compile. See, howver, the LYSystem code in LYUtils.c when DOSPATH and
> __CYGWIN__ are both defined.

That's a horrible and undocumented hack IMNSHO.  If I were using lynx
for a cygwin environment, I'd want to make damn sure that this code isn't
compiled in.  It looks like an attempt to fix some symptoms instead of
addressing the problem (which would be: LYSystem() gets called with
inappropriate arguments, for the platform).  It isn't clear at all
just what it does, without looking at every line.  It tries to fix
up the command name in one way, the arguments(s) in another, making
some unwarranted assumptions (that there is only one command line
argument and that the command line arguement is always a filename?). 
Not to mention missing checks for buffer overflow and quoting sanity.

No-one who compiles lynx should get this code without explicitly
requesting it.  It should depend on some '#ifdef SYSTEM_COMMAND_FOR_
DOS_HACKS', and 'defined(__CYGWIN__) && defined(DOSPATH)' should not
be taken to imply that one wants this.

> > I am using neither; but I like the idea that I *could* use a lynx
> > that's as-much-as-possible like the Unix lynx, if I went to use the
> > cygwin environment.  Making all kinds of DOS-ish behavior depend on
> > (just) __CYGWIN__ sabotages that, uhm, dream.
> 
> I think the cygwin port is still very much more like unix than like
> DOS or Windows.

Does that mean it falls short of the

   different binaries for the same platform (i.e. Borland or cygwin
   compiled) should have compatible user interfaces

goal?

But anyway, I didn't mean only "user interfaces" (in the easily
observable sense), but also just "using the same code as much as
possible".  For example, the DOS-ish branch of LYCopyFile doesn't
return any error indication if copying fails after the source and
destination files have been opened (e.g., disk full), while the UNIX
etc.  code does (if 'cp' and LYSystem() do the right thing).  I
suspect there are many of these "little things" - enough to make me
want UNIXish code rather than DOSish whenever there's a choice, even
if DOSish might be better for most users as default.

> > Where conditional compilation is necessary, there should be sensible
> > HAVE_* or NEED_* or WANT_* macros that allow to make a choice, not
> > something like
> >    (__DJGPP__ || _WINDOWS || __EMX__ || __CYGWIN__)
> > that forces one kind of behavior on all users on some set of platforms
> > (and is typically without any documenting comments in the code).
> 
> I am not sure how many of the defines are not required by the
> platforms for binary handling, pathname handling, or TCP handling.
> I suspect that very few represent personal choices between viable
> alternatives, but I haven't looked closely at them. 

Maybe it's time, now that 2.8.3 is out, to risk breaking something.
Let them complain (or help with the cleanup).

> Once again, we
> have a problem of few people on lynx-dev working on these different
> platforms.

Yes...

> > Something like good old "DOSPATH" isn't bad because it is less specific
> > than (some combination of) __DJGPP__, _WINDOWS, __EMX__, __CYGWIN__.
> > DOSPATH is bad because it has never been defined what it's supposed
> > to mean, and it had come to mean the kitchen sink of DOS-like behavior.
> 
> I know that FNAMES_8_3 came out of similar discussion. 

I made it to separate out one aspect of DOS-ish behavior (don't remember
the discussions around then), which isn't necessarily bound to actually
running under DOS or Windows.  (I remember I did some testing in linux
with FNAMES_8_3 defined; this would make sense if someone wants to share
files with DOS or run lynx with a purely msdos filesystem mounted in
linux.)

> Does anyone
> have any suggestions on clearing this up some before 2.8.4 gets
> released?

- For any new patches that would use conditional code with one of
  those symbols (__DJGPP__, _WINDOWS, __EMX__, __CYGWIN__, etc.),
  or change them around: question what is really meant, what the
  patch is trying to achieve.  Then express than in term of
  meaningful feature or behavior macros.
- (Somebody should) Clean up DOSPATH usage!
  Grep for DOSPATH in all source files.  Separate all tests where
  DOSPATH is used for something else than file path syntax from the
  rest.  Leave the latter as DOSPATH, change the none-path-related
  occurences to something more appropriate (as a first cut, just
  something like OTHER_DOSISH_BEHAVIOR if there's no obvious more
  specific description).  In most cases, the determination whether
  the conditional code deals with path syntax or other aspects is
  easy to make.
- In particular __EMX__ likes to occur in combination with DOSPATH -
  as in
              defined(DOSPATH) || defined (__EMX__)
  Why is that?  Either DOSPATH is always defined for OS/2, then
  get rid off all the unnecessary tests; or DOSPATH is never defined
  for DOSPATH, then maybe it should; or someone wants some peculiar
  behavior for OS/2, making it act like DOSPATH in some situations and
  like not-DOSPATH in others, then make up a new symbol for *that*.
  (HALF_A_DOSPATH? :) )
  If nobody actually using EMX can be bother to make the changes, we
  should make them anyway.  That should wake them up, if it breaks
  something.
- Repeat for __CYGWIN__, maybe others.

   Klaus




reply via email to

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