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: Bruce Korb
Subject: Re: Building sharutils 4.13.4 with MinGW
Date: Sun, 07 Apr 2013 13:46:00 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5

On 04/07/13 10:35, Eli Zaretskii wrote:
Here are the MinGW-specific problems with building and testing this
release (copy to gnulib list, because many issues are related to
gnulib modules).

1. Compilation fails in lib/:

    This is because idcache.h includes sys/types.h, but gnulib's
    sys/types.h does not define uid_t and gid_t, and also there are no
    pwd.h and grp.h in lib/.

    My solution was to add private pwd.h and grp.h to lib/, and modify
    idcache.h to include them.  But I guess a better solution would be
    to import more from gnulib, or perhaps enhance the corresponding
    gnulib modules.

Indeed.  "enhance the corresponding gnulib modules" because gnulib is
supposed to recursively include everything needed.

2. Compilation warning in lib/:

    This is because functions getpwuid and getgrgid are unavailable and
    there are no replacements for them in gnulib.

    To solve this, I added the missing functions to idcache.c:

That would require gnulib cooperation, too.
Otherwise, I have to glue it into sources I control.

3. Compilation warnings in src/shar.c:

      gcc -DLOCALEDIR=\"d:/usr/share/locale\" -DHAVE_CONFIG_H -I. -I..  
-I../libopts -I. -I.. -I../lib -I../intl -Id:/usr/include -Wno-format-contains-nul -g3 
-O2 -Wno-format-contains-nul -MT shar.o -MD -MP -MF .deps/shar.Tpo -c -o shar.o shar.c
      shar.c: In function 'walktree':
      shar.c:595:18: warning: assignment makes pointer from integer without a 
cast [enabled by default]
      shar.c: In function 'set_submitter':
      shar.c:1943:18: warning: initialization makes pointer from integer 
without a cast [enabled by default]
      make[2]: *** [shar.o] Error 1

    The problem here is with basename and dirname, whose declarations
    are absent.  My solution was to add "#include <libgen.h>" to
    lib/system.h (libgen.h in MinGW declares basename and dirname).
    However, I wonder if gnulib has a better solution.

I would hope.  Thanks.

4. Link errors for shar.exe:
    Solution: reorder in src/Makefile.in:

Pasted into my Makefile.am.

    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"
$


5. Link errors for unshar.exe:

    Solution: add mkstemp to unshar.c.  Again, I think gnulib has a
    module for that, so importing it will solve this.

Done.  (Added to gnulib modules)

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.

8. The configure script botches the popen-binary test: the test
    program does not include stdio.h, so the compilation fails for
    reasons that have no relation to the issue being tested.

gnulib

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.  I guess I should set it up to handle it -- only handle
it as if it were plain help.  Rats.

All four *-opts.c have the issue.  I've modified the source template:
#ifdef HAVE_WORKING_FORK
#define MORE_HELP_DESC  ([=<junk>=])
#define MORE_HELP_name  ([=<more junk>=])
#define MORE_HELP_FLAGS (OPTST_IMM | OPTST_NO_INIT)
#else
#define MORE_HELP_DESC  HELP_DESC
#define MORE_HELP_name  HELP_name
#define MORE_HELP_FLAGS (OPTST_OMITTED | OPTST_NO_INIT)
#endif

I am also certain that is not everything.  Basically, on platforms without 
fork(),
you'll just get --help with --more-help.

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.
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".

--- 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.
So the patch needed would be some boilerplate functions that behave differently
for Mingw and the rest of the world and it gets selected at unshar time.
This "wc -c" code gets replaced with that function call.

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?
"Portable" POSIX is hard enough without some system that
masquerades as almost, kind-of-like POSIXy.

There are a few other, less important issues, like with using binary
I/O in some places.  let me know if you want me to send diffs for
those.

HTH

Some of it most certainly is.  Now to the next (previous) one:

I've built this distribution with MinGW tools on MS-Windows.  Quite
unexpectedly for such a veteran package, it turned out to be an uphill
battle.

I had to change some options (add xz compression and handle a behavior
change mandated by our Austin Group friends), plus the docs were all
out of date and things had gotten crufty.  So I bit off too much.

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.)

2. There's a bug in text_mmap.c (in the part that falls back to
   reading a file if mmap is unavailable): the txt_zero_fd member of
   tmap_info_t structure was not initialized to -1, so stayed at zero
   and was closed (twice) by close_mmap_files, which caused an invalid
   argument debug message from the runtime library.

Thank you.

3. Using the -R option seems to bypass encoding and only records the
   options to the RC file -- is that intended?  If so, it seems to be
   undocumented (I initially thought it was a bug, and only by
   stepping through the code found out that uudecode -R simply exits
   after outputting the options to the RC file).

I've added the following sentence to the man page and info doc templates.
This will show up in ntp and tls documentation, too:
  The command will exit after updating the config file.
It basically turns the program into a config file editor.

4. When passed 2 or more base64-encoded files, uudecode barfs on the
   second one:

--- sharutils-4.13.4.orig/src/uudecode.c        2013-03-29 17:28:40.000000000 
+0300
+++ sharutils-4.13.4/src/uudecode.c     2013-04-07 09:38:10.818181900 +0300
@@ -497,6 +518,7 @@ multiple input files.\n"));
               error (0, errno, "%s", f);
               exit_status |= UUDECODE_EXIT_NO_INPUT;
             }
+          do_base64 = false;
         }
     }

Thanks. Fixed.


5. Question: can /dev/stdout or - appear on the encoded file's begin
   line?  If it can, then

   doesn't redirect stdout in that case.  So it looks like uudecode
   will continue writing to the previously decoded file, instead of
   resetting stdout to the original stream/descriptor.

That is an inherited feature. :)  I likely wordsmith-ed the following,
but it is an old caveat:

BUGS
       If more than one file is given to uudecode and the -o option  is  given
       or  more  than one name in the encoded files are the same the result is
       probably not what is expected.

6. shar fails to record the time and the submitter of the archive:

     # Made on  by <>.

   The time was missing because print_header_stamp assumed %Z format
   in strftime always takes at most 4 characters:
    static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
    /*
     * All fields are two characters, except %Y is four and
     * %Z may be up to 30 (?!?!)
     */
    char buffer[sizeof (ftime_fmt) + 64];

   My solution was to reallocate the buffer if strftime returns zero:

    {
      size_t l =
        strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
      if (l > 0)
        fprintf (fp, made_on_comment_z, buffer, OPT_ARG(SUBMITTER));
    }

   The problem with submitter was because set_submitter assigned a
   local buffer to the submitter option argument, without using
   xstrdup:

   Solution: use xstrdup inside the SET_OPT_SUBMITTER call.

Hmm.  I misremembered my own code.  Rehacked:

static void
set_submitter (void)
{
  char * buffer;
  char * uname = getuser (getuid ());
  size_t len   = strlen (uname);
  if (uname == NULL)
    fserr (SHAR_EXIT_FAILED, "getpwuid", "getuid()");
  buffer = xmalloc (len + 2 + HOST_NAME_MAX);
  memcpy (buffer, uname, len);
  buffer[len++] = '@';
  gethostname (buffer + len, HOST_NAME_MAX);
  SET_OPT_SUBMITTER(buffer);
}

P.P.S.  Thanks for maintaining sharutils!

It was supposed to be easy, and I went and made it harder.
Thank you.

http://autogen.sourceforge.net/data/

has both a "pre2" sharutils and the autogen for creating it.
(You do not need the latter, as the "libopts" gets rolled into sharutils.)
The issues that really need gnulib fixes are still in sharutils.
I'm also still trying to figure out how to select which
   ac_cv_header_xxx_h
values should be set to "yes".

Thank you again.  Regards, Bruce



reply via email to

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