[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1131383 - in /subversion/trunk/subversion: include/private/svn_magic.h libsvn_client/add.c libsvn_client/client.h libsvn_client/commit.c libsvn_subr/io.c libsvn_subr/magic.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Sat, 4 Jun 2011 18:17:22 +0200

On Sat, Jun 04, 2011 at 07:11:05PM +0300, Daniel Shahaf wrote:
> stsp_at_apache.org wrote on Sat, Jun 04, 2011 at 12:32:28 -0000:
> > +/* Detect the mime-type of the file at LOCAL_ABSPATH using MAGIC_COOKIE.
> > + * If the mime-type is binary return the result in *MIMETYPE.
> > + * If the file is not a binary file or if its mime-type cannot be determined
> > + * set *MIMETYPE to NULL. Allocate *MIMETYPE in RESULT_POOL.
> > + * Use SCRATCH_POOL for temporary allocations. */
> > +svn_error_t *
> > +svn_magic__detect_binary_mimetype(const char **mimetype,
> > + const char *local_abspath,
> > + svn_magic__cookie_t *magic_cookie,
> > + apr_pool_t *result_pool,
> > + apr_pool_t *scratch_pool);
> > +
>
> As I said on IRC: I think this API is tailored for the caller and it
> would seem more natural to me to move the "Ignore the result if it's
> a binary type" to the caller.

Feel free to change that if it really bugs you.
But do we ever care about auto-detecting non-binary mimetypes in svn?
 
> > +svn_error_t *
> > +svn_magic__detect_binary_mimetype(const char **mimetype,
> > + const char *local_abspath,
> > + svn_magic__cookie_t *magic_cookie,
> > + apr_pool_t *result_pool,
> > + apr_pool_t *scratch_pool)
> > +{
> > + const char *magic_mimetype = NULL;
> > +#ifdef HAVE_LIBMAGIC
> > + apr_finfo_t finfo;
> > +
> > + /* Do not ask libmagic for the mime-types of empty files.
> > + * This prevents mime-types like "application/x-empty" from making
> > + * Subversion treat empty files as binary. */
> > + SVN_ERR(svn_io_stat(&finfo, local_abspath, APR_FINFO_SIZE, scratch_pool));
> > + if (finfo.size > 0)
> > + {
>
> I've suggested to simply call libmagic and look for the
> application/x-empty response, stsp claimed it wouldn't make
> a noticeable difference in the current users (add and import),
> I was convinced.

The problem here is also that we really don't know whether an empty
file will cause "application/x-empty" or something else being returned
from the magic DB. Databases out there can differ because of libmagic
versions, and they can be customized by distributors, users, sysadmins etc.

If an empty file gets a property that svn considers binary the tests
start failing all over the place.
Received on 2011-06-04 18:18:01 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.