This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: upload: diffstat-1.40-1, tar-1.15.1-1


> On Aug 17 10:56, Christopher Faylor wrote:
> > On Wed, Aug 17, 2005 at 06:24:50AM -0600, Eric Blake wrote:
> > >tar: new maintainer, new upstream release.  Instead of linking against
> > >binmode.o and forcing text mode on the exceptions even on binary mounts,
> > 
> > I'm not sure what the above is implying but I didn't link with binmode.o
> > when I packaged tar.

Diffing the contents of /usr/src/tar-1.13.25-7 against a pristine
tar-1.13.25 tarball shows that you indeed patched configure.ac
to use binmode.o:
-test "$ac_cv_func_strstr" = yes || LIBOBJS="$LIBOBJS strstr.o"
+test "$ac_cv_func_strstr" = yes || AC_LIBOBJ(strstr)
+
+if test "$GCC" = "yes"; then
+    binmode=`$CC -print-file-name=binmode.o < /dev/null`
+    case "$binmode" in
+      /*) LDEXTRA="$LDEXTRA $binmode" ;;
+    esac
+fi
+AC_SUBST(LDEXTRA)

Then you went through the sources, and for all files that manipulate
human-readable files (such as file name lists, as opposed to actual
tars), you added FOPEN_TEXT_READ, defined as "rt", to fopen calls,
and O_TEXT to open calls.  All file manipulations that were on binary
files you left alone.  This means that in some cases, your patch to
1.13.25 actually created text files (\r\n endings) on a binary mount
point.

I did the opposite.  Instead of forcing binmode.o then specifying
exceptions that should be text mode, I fixed all remaining opens
to specify "rb" or O_BINARY as appropriate, leaving only the
human-readable files to track the underlying mount point at
creation, but to read in text mode (in other words, a file with \r\n
shouldn't break when moved to a binary mount point).  I verified
that everything checked in the testsuite still works with this approach,
on both binary and text mounts, and it also has the benefit of being
a patch more likely to be accepted upstream.

> > 
> > >I fixed the source code to always request bin mode on the files where
> > >it matters, so that text files are created according to the mount
> > >point's mode.
> > 
> > This is not a fix.  It is a change in philosophy.  If you changed things
> > so that reading a file via -T uses the mount mode or if you changed it
> > so that reading the incremental information uses the mount mode, it
> > is actually a bug.

I agree that reading the incremental information must be robust to
accept either line ending (whether that is accomplished by using
a forced text read, or by making sure that the line parser ignores
'\r', the effect is the same).  If I missed a case in my patch, I agree that
it is a bug in my packaging.  But I think I got everything.  Should we
just release this package in test status first, as one last chance to
review for any potential issues?

I agree that the philosophy should not change; file reads should be
tolerant, and file writes should prefer \n line endings.

> > 
> > The philosophy that Corinna and I have decided upon for cygwin tools is
> > basically what is available with automode.o.  This makes the default
> > such that you open for read in text mode and open for write in binary
> > mode.  This produces files which do not have the CRLF line endings which
> > is appropriate since cygwin is a linux emulation.
> 
> Am I missing something or shouldn't tar be one of those applications
> which should recreate *exactly* what they got?  In other words, shouldn't
> using binmode under all circumstances, regardless of mount mode, file
> type, or moon phase, be the way to go for tar?

Yes, the bulk of files created by tar are not human-readable, and
must be done in binary mode.  The testsuites fail rather dramatically
if that premise if violated!

--
Eric Blake



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]