bug-gnulib
[Top][All Lists]
Advanced

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

Re: Building sharutils 4.13.4 with MinGW


From: Eli Zaretskii
Subject: Re: Building sharutils 4.13.4 with MinGW
Date: Mon, 08 Apr 2013 18:35:45 +0300

> Date: Sun, 07 Apr 2013 13:46:00 -0700
> From: Bruce Korb <address@hidden>
> CC: address@hidden, address@hidden, 
>  Andreas Schwab <address@hidden>
> 
> >     For the other errors: I wrote emulations for fork (always fails),
> >     wait (always fails) and pipe (calls _pipe), and also add getuid,
> >     getpwnam and getgrnam to idcache.c.  However, I think gnulib has
> >     replacements for at least some of these functions.
> 
> Not that I can find.
> > $ list=$(find * -type f)
> > $ fgrep getuid $list
> > $ fgrep getpwnam $list
> > $ fgrep getgrnam $list
> > $ echo "$list"|egrep "getuid|getpwnam|getgrnam"
> > $

I meant fork, wait (waitpid in gnulib), and pipe.

> > 6. uutest-1 test fails because the begin line doesn't match.  This is
> >     because the mode bits on Windows are not real, beyond the user-read
> >     and user-write bits.  So it would be nice if on Windows Diff would
> >     be invoked like this:
> >
> >       DIFF="diff -I^begin"
> >
> > 7. shar-4 fails because it uses cmp to compare results with expected
> >     ones, which fails due to line endings.  Suggest to use Diff
> >     instead.
> 
> Could you suggest a patch for these two, too? :)  Maybe a script test
> so that I know that I have to:
>    sed 's/^begin [0-7][0-7][0-7] /begin 664/'
> the result file.

The easiest way to test for this is to say "chmod a+w foo" and then
see that "ls -l foo" still shows it as "-rw-r--r--" (or anyway
something other than world-writable).

> > 9. Using -! or --more-help triggers an error message:
> >
> >       d:\usr\eli\utils\sharutils-4.13.4>uuencode -!
> >       uuencode: illegal option -- !
> >       uuencode (GNU sharutils) - encode a file into email friendly text
> >       Usage:  uuencode [ -<flag> | --<name> ]... [<in-file>] <output-name>
> >       Try 'uuencode --help' for more information.
> 
> >     This is confusing because --help does show these options as available.
> >
> >     The reason is that uuencode-options.c does this:
> >
> >       #if HAVE_WORKING_FORK
> >       #define MORE_HELP_DESC  (uuencode_opt_strs+1045)
> >       #define MORE_HELP_name  (uuencode_opt_strs+1090)
> >       #define MORE_HELP_FLAGS (OPTST_IMM | OPTST_NO_INIT)
> >       #else
> >       #define MORE_HELP_DESC  NULL  <<<<<<<<<<<<<<<<<<<<<<<<<
> >       #define MORE_HELP_name  NULL  <<<<<<<<<<<<<<<<<<<<<<<<<
> >       #define MORE_HELP_FLAGS (OPTST_OMITTED | OPTST_NO_INIT)
> >       #endif
> >
> >     which is unnecessary, since the relevant libopts code already knows
> >     how to handle the case of !HAVE_WORKING_FORK.
> >
> >     => Fixed by enabling the !HAVE_WORKING_FORK block above.
> >
> >     Same in shar-opts.c and uudecode-opts.c.
> 
> Lovely.  This is a dilemma.  In order to facilitate i18n, I cannot compute 
> help
> text on the fly.  If I don't compute it on the fly, then it has to be the same
> text for all platforms.

My suggestion was unrelated to i18n, I think.  All I was saying was
that if the help text cannot be piped through a pager, it should be
dumped to stdout instead.  That is better than an error message, IMO.

> Basically, on platforms without fork(), you'll just get --help with
> --more-help.

That's exactly what I think should be done here, no more.

> > 10.shar fails while processing files:
> >
> >       d:\usr\eli\utils\sharutils-4.13.4>src\shar src\encode.c > foo.shar
> >       shar: Saving src/encode.c (text)
> >       The system cannot find the path specified.
> >
> >     This is because emit_char_ct_validation invokes 'wc' with bad
> >     redirection:
> >
> >       set LC_ALL=C & wc -c < 'src/encode.c'
> >
> >     The file from which stdin is redirected uses forward slashes and is
> >     quoted with '..'.
> >
> >     Solution: use a slightly different command for MinGW to run 'wc'
> >     locally:
> >
> >       set LC_ALL=C & wc -c "src/encode.c"
> 
> Hmmm.  That routine is *called* with quoting it doesn't handle well.
> The ``quoted_name'' argument is the string "'src/encode.c'".
> It is the same argument as is passed to finish_sharing_file.

Yes, but the quoting caters to a Posix shell, not to what Windows
expects.

> Anyway, if in the end MinGW requires double quotes for path names
> that might have directory characters, it is starting to look like
> it might be better to:
>        local_name=`localize 'src/encode.c'`
>        LC_ALL=C wc -c < $local_name
> with the "localize" function most commonly being "echo".

This has nothing to do with localization.  The Windows shell simply
does not support '..' kind of quoting.

> > --- sharutils-4.13.4.orig/src/shar.c        2013-03-23 00:23:48.000000000 
> > +0200
> > +++ sharutils-4.13.4/src/shar.c     2013-04-07 13:41:14.742531000 +0300
> > @@ -1065,14 +1097,27 @@ emit_char_ct_validation (
> >     /* Shell command able to count characters from its standard input.
> >        We have to take care for the locale setting because wc in multi-byte
> >        character environments gets different results.  */
> > +#ifdef __MINGW32__
> > +  /* Windows needs a slightly different format to run wc locally:
> > +     redirection will fail because the file uses Unix forward
> > +     slashes.  */
> > +  static char const local_cmd[] = "set LC_ALL=C & wc -c";
> > +#endif
> >     static char const cct_cmd[] = "LC_ALL=C wc -c <";
> >
> >     FILE *pfp;
> >     char wc[16 /* log MAX_INT + a few extra */];
> > -  char *command = alloca (sizeof(cct_cmd) + strlen (quoted_name) + 2);
> >
> > +#ifdef __MINGW32__
> > +  /* On Windows, disregard Unix-style shell quoting and quote by hand
> > +     using "..".  */
> > +  char *command = alloca (sizeof(local_cmd) + strlen (local_name) + 2);
> > +  sprintf (command, "%s \"%s\"", local_cmd, local_name);
> > +#else
> > +  char *command = alloca (sizeof(cct_cmd) + strlen (quoted_name) + 2);
> >     sprintf (command, "%s %s", cct_cmd, quoted_name);
> > -  pfp = popen (command, "r");
> > +#endif
> > +  pfp = popen (command, freadonly_mode);
> 
> I know it is difficult to keep straight which commands get executed where.
> What is being emitted in this function is shell code that gets executed
> on the receiving machine.  In other words, shell code that can be interpreted
> by shell-on-windows and shell-on-real-posix.  Selecting the form of the
> output based on __MINGW32__ at shar compile time is incorrect.

I think there's a misunderstanding here.  I'm aware that the wc
command gets written into the shar.  My suggested patch didn't touch
that part, for that very reason, and as result the shar file still
includes the correct Posix shell command.  Here's an example produced
by my build on Windows:

  test `LC_ALL=C wc -c < 'gnu/sharutils-4.13.4/src/shar.c'` -ne 60556 && \
    ${echo} "restoration warning:  size of 'gnu/sharutils-4.13.4/src/shar.c' is 
not 60556"

However, the function in question, emit_char_ct_validation, uses the
command constructed from cct_cmd[] also for running wc locally, via
popen, at shar creation time -- that's how it produces the
verification value (60556 in the example above).  My changes touched
only the local invocation, not the verification command that gets
recorded in the shar and is supposed to run in a Unixy shell.

This problem is exacerbated by the fact that all the file names need
to have their backslash directory separators converted to Unix-style
forward slashes before they get to shar-creation code.  (This is in a
part of my changes I didn't show.)  This is because file names
recorded in the shar _must_ be Posix compatible.  So even if the
quoting problem would somehow be solved (e.g., by adding a special
style to the quotearg module in gnulib), redirection will still not
work, because the Windows shell which is run by popen does not
understand forward slashes in file names.

So I think a Windows-specific local command for running wc at shar
creation time is the best compromise.  Since the quoted strings are
file names, and Windows doesn't allow quote characters in file names,
simply enclosing each name in quotes is enough.

> > 11.shar.c uses /dev/null in freopen, but does not use the gnulib
> >     freopen module, which makes /dev/null work on MS-Windows.
> 
> Isn't there a POSIX layer that can be added to windows?

Not with MinGW.  MinGW produces native Windows executable that depend
on the Windows runtime libraries.

In any case, gnulib has the freopen module which solves the /dev/null
problem.  It just needs to be added to the package.

> > 1. configure errors out:
> >
> >      configure: error: you must have sys_mman.h on your system
> 
> >    I don't understand why configure insists on having sys/mman.h and
> >    sys/wait.h, because the code has appropriate fallbacks for when
> >    they are not present.
> 
> I think the correct fix is to do the gnulib futzing and then
> set ac_cv_header_sys_wait_h and ac_cv_header_sys_mman_h to "yes".
> The libopts stuff should follow that.  (I don't think stuffing
> gnulib stuff into libopts would work so well.)

Sorry, I don't understand.  The libopts stuff already has fallback
code for when sys/mman.h and mmap are missing -- see text_mmap.c,
which can cope with HAVE_MMAP being undefined.  The only problem is
that libopts/m4/libopts.m4 explicitly insists on some headers being
present:

  for f in sys_types sys_mman sys_param sys_stat sys_wait \
           string errno stdlib memory setjmp
  do eval as_ac_var=\${ac_cv_header_${f}_h}
     test "X${as_ac_var}" = Xyes || {
       ]AC_MSG_ERROR([you must have ${f}.h on your system])[
     }                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  done

Simply removing sys_mman and sys_wait from this list solves the
problem.

> http://autogen.sourceforge.net/data/
> 
> has both a "pre2" sharutils and the autogen for creating it.

I'm not done yet ;-)  See my other message.



reply via email to

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