Re: [PATCH] Add an implementation of gnulib's canonicalize_file_name for mingw.

[Bruno Haible]

Hello Jan,

> > In which respect did you find it useless for mingw?
> 
> In that, when given an absolute file name, such as
> 
>    c:/program files/lilypond/usr/share/guile/2.0/ice-9/boot-9.scm
> 
> it returns NULL.

This is indeed useless and needs to be improved.

> It also returns NULL when given the CWD or a 
> simple drive letter, such as C: or C:/.

Likewise.

> When given a local file 
> name "baz", it returns something like
> 
>     C:\foo\bar/baz
> 
> which is certainly not canonical.

It is true that this can be a problem if a programmer wants to test whether
two file names denote the same file by calling canonicalize_file_name on
each and comparing the result. The \ vs. / could bring in an undesired
difference.

Thanks for bringing up all these issues.

> Attached are patches

Let me comment on each.

>> 0001-Enable-cross-building-along-with-basic-HACKING-instr.patch

About the HACKING file: In gnulib this kind of documentation is found
in the doc/ directory and in the README file. It would be better to
submit enhancements for one or the other.

> ./gnulib-tools --dir=work --test canonicalize-lgpl --with-tests

Please put options before non-options, always. This is needed by
program that use getopt() on some platforms, therefore it is a bad
habit to spread examples where options come after non-options.

I mostly use
"./gnulib-tool --create-testdir --dir=... --with-tests $modules"
because often the target platform is different from the development
platform (for example: develop on a glibc system, test on mingw).

It is not needed to extend the --test option of gnulib-tool,
precisely because --test is just a shorthand for --create-testdir
followed by a normal configuration on the same platform. As soon
as your task is no longer a normal configuration on the same platform,
you better use --create-testdir, tar cvfhz, scp, ssh and similar
commands, from a script.

>> 0002-canonicalize-lgpl-Use-del-instead-of-rm.-Fixes-abort.patch

The recommended environment for mingw is to use cygwin as a host
environment and "cross" compile using a configuration like this:

   PATH=/usr/local/mingw/bin:$PATH
   export PATH
   ./configure --host=i586-pc-mingw32 --prefix=/usr/local/mingw \
     CC="gcc-3 -mno-cygwin" \
     CXX="g++-3 -mno-cygwin" \
     CPPFLAGS="-Wall -I/usr/local/mingw/include" \
     LDFLAGS="-L/usr/local/mingw/lib"

A less recommendable environment for mingw is MSYS. Here you
don't even need to specify --host.

In both cases, the environment is assumed to contain a bash and
the coreutils. Therefore an 'rm' command will be found in PATH.

Given that there are these two setups which don't require patching
the unit tests, what's the point in using an environment which has
no 'rm' command?

>> 0003-canonicalize-lgpl-Skip-symlink-tests.-Fixes-aborting.patch
>> 0004-canonicalize-lgpl-Skip-cleanup-using-remove.-Fixes-a.patch

On Windows Vista and Windows 7, there are native symlinks in NTFS.
gnulib's readlink() does not yet, but ought to understand these symlinks.
Therefore #ifdefing out the code that handles symlinks, on the premise
that "there are no symlinks on native Windows" is wrong.

Btw, the preprocessor test for native Windows is
  #if (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__

Whereas
  #if defined __MINGW32__
is useful only to test specifics of the mingw environment that are
not present in the MSVC environment and not tied to GCC. Such as. maybe
some bugs in the mingw include files or in its <dirent.h> implementation.

>> 0005-canonicalize-lgpl-Add-basic-sanity-checks-for-mingw.patch

During development, it's obviously necessary to have fprintf statements.
When a test is committed, however, it should be silent when it succeeds.
In other words, use fprintf only to help debugging a problem that is
known to be a problem. Otherwise the volume of debugging output overshadows
the final result of the test.

>> 0006-canonicalize-lgpl-Add-an-implementation-for-canonica.patch

By creating a completely new implementation for mingw, we would make it
very hard later to have the same support for symlinks in the mingw specific
code as in the Unix code. I would find it better if you could enhance the
code (with GetFullPathName etc.) while at the same time reusing a maximum
of the Unix implementation.

Bruno
-- 
In memoriam Ilan Ramon <http://en.wikipedia.org/wiki/Ilan_Ramon>