You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/06/04 14:32:28 UTC
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
Author: stsp
Date: Sat Jun 4 12:32:27 2011
New Revision: 1131383
URL: http://svn.apache.org/viewvc?rev=1131383&view=rev
Log:
Wrap libmagic support in a private API and use this API from libsvn_client.
This moves all the #ifdef HAVE_LIBMAGIC spaghetti into a single file.
It also makes Subversion's use of libmagic more efficient for recursive
operations because the magic database is only loaded once per invocation
of svn_client_add() and svn_client_import(), not per every invocation of
svn_io_detect_mimetype().
Inspired by: hwright
* subversion/include/private/svn_magic.h: New.
* subversion/libsvn_subr/magic.c: New.
* subversion/libsvn_subr/io.c
(svn_io_detect_mimetype2): Drop libmagic support from this function.
* subversion/libsvn_client/add.c
(svn_client__get_auto_props): Add a MAGIC_COOKIE parameter for use with
the new svn_magic API. Let mime-types obtained from libmagic override
the mimetypes map if the map returned "application/octet-stream" (but
do not override "application/octet-stream" configured via auto-props).
(add_file, add_dir_recursive): Add MAGIC_COOKIE parameter.
(add): Initialise libmagic and pass the magic cookie on to other functions.
* subversion/libsvn_client/commit.c
(import_ctx): Add a MAGIC_COOKIE here, too.
(import_file): Pass CTX->MAGIC_COOKIE to svn_client__get_auto_props().
(import): Initialise libmagic and store the cookie in the import context.
* subversion/libsvn_client/client.h
(svn_client__get_auto_props): Update declaration.
Added:
subversion/trunk/subversion/include/private/svn_magic.h
subversion/trunk/subversion/libsvn_subr/magic.c
Modified:
subversion/trunk/subversion/libsvn_client/add.c
subversion/trunk/subversion/libsvn_client/client.h
subversion/trunk/subversion/libsvn_client/commit.c
subversion/trunk/subversion/libsvn_subr/io.c
Added: subversion/trunk/subversion/include/private/svn_magic.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_magic.h?rev=1131383&view=auto
==============================================================================
--- subversion/trunk/subversion/include/private/svn_magic.h (added)
+++ subversion/trunk/subversion/include/private/svn_magic.h Sat Jun 4 12:32:27 2011
@@ -0,0 +1,55 @@
+/**
+ * @copyright
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ * @endcopyright
+ *
+ * @file svn_magic.h
+ * @brief Subversion interface to libmagic.
+ */
+
+#ifndef SVN_MAGIC_H
+#define SVN_MAGIC_H
+
+/* An opaque struct that wraps a libmagic cookie. */
+typedef struct svn_magic__cookie_t svn_magic__cookie_t;
+
+/* This routine initialises libmagic.
+ * Upon success new *MAGIC_COOKIE is allocated in RESULT_POOL.
+ * On failure *MAGIC_COOKIE is set to NULL.
+ * All resources used by libmagic are freed by a cleanup handler
+ * installed on RESULT_POOL, i.e. *MAGIC_COOKIE becomes invalid when
+ * the pool is cleared! */
+void
+svn_magic__init(svn_magic__cookie_t **magic_cookie,
+ apr_pool_t *result_pool);
+
+/* 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);
+
+#endif /* SVN_MAGIC_H */
Modified: subversion/trunk/subversion/libsvn_client/add.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=1131383&r1=1131382&r2=1131383&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/add.c (original)
+++ subversion/trunk/subversion/libsvn_client/add.c Sat Jun 4 12:32:27 2011
@@ -47,6 +47,7 @@
#include "private/svn_client_private.h"
#include "private/svn_wc_private.h"
+#include "private/svn_magic.h"
#include "svn_private_config.h"
@@ -220,6 +221,7 @@ svn_error_t *
svn_client__get_auto_props(apr_hash_t **properties,
const char **mimetype,
const char *path,
+ svn_magic__cookie_t *magic_cookie,
svn_client_ctx_t *ctx,
apr_pool_t *pool)
{
@@ -254,38 +256,20 @@ svn_client__get_auto_props(apr_hash_t **
SVN_ERR(svn_io_detect_mimetype2(&autoprops.mimetype, path,
ctx->mimetypes_map, pool));
-#ifdef HAVE_LIBMAGIC
- /* We want to set an svn:mime-type property by default only on binary
- * files. So don't set an svn:mime-type property on text files unless
- * their mime-type appears in the map. This preserves behaviour
- * of Subversion releases that did not include libmagic support.
- * In those releases svn_io_detect_mimetype2() returned
- * "application/octet-stream" or NULL unless the type was in the map. */
- if (autoprops.mimetype && strncmp(autoprops.mimetype, "text/", 5) == 0)
- {
- if (ctx->mimetypes_map)
- {
- svn_boolean_t type_is_in_map = FALSE;
- apr_hash_index_t *hi;
-
- for (hi = apr_hash_first(pool, ctx->mimetypes_map);
- hi;
- hi = apr_hash_next(hi))
- {
- const char *type_from_map = svn__apr_hash_index_val(hi);
- if (strcmp(type_from_map, autoprops.mimetype) == 0)
- {
- type_is_in_map = TRUE;
- break;
- }
- }
- if (!type_is_in_map)
- autoprops.mimetype = NULL;
- }
- else
- autoprops.mimetype = NULL;
+ /* If we got no mime-type, or if it is "application/octet-stream",
+ * try to get the mime-type from libmagic. */
+ if (magic_cookie &&
+ (!autoprops.mimetype ||
+ strcmp(autoprops.mimetype, "application/octet-stream") == 0))
+ {
+ const char *magic_mimetype;
+
+ SVN_ERR(svn_magic__detect_binary_mimetype(&magic_mimetype,
+ path, magic_cookie,
+ pool, pool));
+ if (magic_mimetype)
+ autoprops.mimetype = magic_mimetype;
}
-#endif
if (autoprops.mimetype)
apr_hash_set(autoprops.properties, SVN_PROP_MIME_TYPE,
@@ -311,6 +295,7 @@ svn_client__get_auto_props(apr_hash_t **
/* Only call this if the on-disk node kind is a file. */
static svn_error_t *
add_file(const char *local_abspath,
+ svn_magic__cookie_t *magic_cookie,
svn_client_ctx_t *ctx,
apr_pool_t *pool)
{
@@ -331,7 +316,7 @@ add_file(const char *local_abspath,
we open them to estimate file type.
That's why we postpone the add until after this step. */
SVN_ERR(svn_client__get_auto_props(&properties, &mimetype, local_abspath,
- ctx, pool));
+ magic_cookie, ctx, pool));
/* Add the file */
SVN_ERR(svn_wc_add_from_disk(ctx->wc_ctx, local_abspath,
@@ -402,6 +387,9 @@ add_file(const char *local_abspath,
* Files and directories that match ignore patterns will not be added unless
* NO_IGNORE is TRUE.
*
+ * Use MAGIC_COOKIE (which may be NULL) to detect the mime-type of files
+ * if necessary.
+ *
* If CTX->CANCEL_FUNC is non-null, call it with CTX->CANCEL_BATON to allow
* the user to cancel the operation
*/
@@ -410,6 +398,7 @@ add_dir_recursive(const char *dir_abspat
svn_depth_t depth,
svn_boolean_t force,
svn_boolean_t no_ignore,
+ svn_magic__cookie_t *magic_cookie,
svn_client_ctx_t *ctx,
apr_pool_t *scratch_pool)
{
@@ -476,12 +465,13 @@ add_dir_recursive(const char *dir_abspat
depth_below_here = svn_depth_empty;
SVN_ERR(add_dir_recursive(abspath, depth_below_here,
- force, no_ignore, ctx, iterpool));
+ force, no_ignore, magic_cookie,
+ ctx, iterpool));
}
else if ((dirent->kind == svn_node_file || dirent->special)
&& depth >= svn_depth_files)
{
- err = add_file(abspath, ctx, iterpool);
+ err = add_file(abspath, magic_cookie, ctx, iterpool);
if (err && err->apr_err == SVN_ERR_ENTRY_EXISTS && force)
svn_error_clear(err);
else
@@ -516,6 +506,9 @@ add(void *baton, apr_pool_t *result_pool
svn_node_kind_t kind;
svn_error_t *err;
struct add_with_write_lock_baton *b = baton;
+ svn_magic__cookie_t *magic_cookie;
+
+ svn_magic__init(&magic_cookie, scratch_pool);
if (b->existing_parent_abspath)
{
@@ -565,11 +558,11 @@ add(void *baton, apr_pool_t *result_pool
and pass depth along no matter what it is, so that the
target's depth will be set correctly. */
err = add_dir_recursive(b->local_abspath, b->depth,
- b->force, b->no_ignore, b->ctx,
+ b->force, b->no_ignore, magic_cookie, b->ctx,
scratch_pool);
}
else if (kind == svn_node_file)
- err = add_file(b->local_abspath, b->ctx, scratch_pool);
+ err = add_file(b->local_abspath, magic_cookie, b->ctx, scratch_pool);
else if (kind == svn_node_none)
{
svn_boolean_t tree_conflicted;
Modified: subversion/trunk/subversion/libsvn_client/client.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/client.h?rev=1131383&r1=1131382&r2=1131383&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/client.h (original)
+++ subversion/trunk/subversion/libsvn_client/client.h Sat Jun 4 12:32:27 2011
@@ -35,6 +35,8 @@
#include "svn_ra.h"
#include "svn_client.h"
+#include "private/svn_magic.h"
+
#ifdef __cplusplus
extern "C" {
#endif /* __cplusplus */
@@ -348,10 +350,13 @@ svn_error_t * svn_client__can_delete(con
(const char * keys mapping to svn_string_t * values), or if
auto-props are disabled, set *PROPERTIES to NULL.
Set *MIMETYPE to the mimetype, if any, or to NULL.
+ If MAGIC_COOKIE is not NULL and no mime-type can be determined
+ via CTX->config try to detect the mime-type with libmagic.
Allocate the hash table, keys, values, and mimetype in POOL. */
svn_error_t *svn_client__get_auto_props(apr_hash_t **properties,
const char **mimetype,
const char *path,
+ svn_magic__cookie_t *magic_cookie,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
Modified: subversion/trunk/subversion/libsvn_client/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/commit.c?rev=1131383&r1=1131382&r2=1131383&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/commit.c (original)
+++ subversion/trunk/subversion/libsvn_client/commit.c Sat Jun 4 12:32:27 2011
@@ -49,6 +49,7 @@
#include "client.h"
#include "private/svn_wc_private.h"
+#include "private/svn_magic.h"
#include "svn_private_config.h"
@@ -70,6 +71,8 @@ typedef struct import_ctx_t
/* Whether any changes were made to the repository */
svn_boolean_t repos_changed;
+ /* A magic cookie for mime-type detection. */
+ svn_magic__cookie_t *magic_cookie;
} import_ctx_t;
@@ -211,8 +214,9 @@ import_file(const svn_delta_editor_t *ed
if (! is_special)
{
/* add automatic properties */
- SVN_ERR(svn_client__get_auto_props(&properties, &mimetype, path, ctx,
- pool));
+ SVN_ERR(svn_client__get_auto_props(&properties, &mimetype, path,
+ import_ctx->magic_cookie,
+ ctx, pool));
}
else
properties = apr_hash_make(pool);
@@ -495,6 +499,8 @@ import(const char *path,
const char *edit_path = "";
import_ctx_t *import_ctx = apr_pcalloc(pool, sizeof(*import_ctx));
+ svn_magic__init(&import_ctx->magic_cookie, pool);
+
/* Get a root dir baton. We pass an invalid revnum to open_root
to mean "base this on the youngest revision". Should we have an
SVN_YOUNGEST_REVNUM defined for these purposes? */
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1131383&r1=1131382&r2=1131383&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Sat Jun 4 12:32:27 2011
@@ -66,10 +66,6 @@
#include "private/svn_atomic.h"
#include "private/svn_io_private.h"
-#ifdef HAVE_LIBMAGIC
-#include <magic.h>
-#endif
-
#define SVN_SLEEP_ENV_VAR "SVN_I_LOVE_CORRUPTED_WORKING_COPIES_SO_DISABLE_SLEEP_FOR_TIMESTAMPS"
/*
@@ -2910,10 +2906,6 @@ svn_io_detect_mimetype2(const char **mim
svn_error_t *err;
unsigned char block[1024];
apr_size_t amt_read = sizeof(block);
- apr_finfo_t finfo;
-#ifdef HAVE_LIBMAGIC
- magic_t magic;
-#endif
/* Default return value is NULL. */
*mimetype = NULL;
@@ -2941,40 +2933,6 @@ svn_io_detect_mimetype2(const char **mim
_("Can't detect MIME type of non-file '%s'"),
svn_dirent_local_style(file, pool));
-
- /* Check if the file is empty and do not set a mime-type if it is.
- * This also avoids spurious mime-types like "application/x-empty"
- * from libmagic.
- * ### merge this with the kind check above to save a stat() call */
- SVN_ERR(svn_io_stat(&finfo, file, APR_FINFO_SIZE, pool));
- if (finfo.size == 0)
- return SVN_NO_ERROR;
-
-#ifdef HAVE_LIBMAGIC
- /* Try to determine the mime-type with libmagic. */
- magic = magic_open(MAGIC_MIME_TYPE | MAGIC_ERROR);
- if (magic)
- {
- /* This loads the default magic database.
- * Point the MAGIC environment variable at your favourite .mgc
- * file to load a non-default database. */
- if (magic_load(magic, NULL) != -1)
- {
- const char *magic_mime_type;
-
- magic_mime_type = magic_file(magic, file);
- if (magic_mime_type != NULL)
- *mimetype = apr_pstrdup(pool, magic_mime_type);
- }
-
- /* This deallocates memory pointed to by magic_mime_type. */
- magic_close(magic);
-
- if (*mimetype)
- return SVN_NO_ERROR;
- }
-#endif
-
SVN_ERR(svn_io_file_open(&fh, file, APR_READ, 0, pool));
/* Read a block of data from FILE. */
Added: subversion/trunk/subversion/libsvn_subr/magic.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/magic.c?rev=1131383&view=auto
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/magic.c (added)
+++ subversion/trunk/subversion/libsvn_subr/magic.c Sat Jun 4 12:32:27 2011
@@ -0,0 +1,128 @@
+/*
+ * magic.c: wrappers around libmagic
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+
+/* ==================================================================== */
+
+
+/*** Includes. ***/
+
+#include <apr_lib.h>
+#include <apr_file_info.h>
+
+#include "svn_io.h"
+#include "svn_pools.h"
+#include "svn_error.h"
+
+#include "svn_private_config.h"
+
+#include "private/svn_magic.h"
+
+#ifdef HAVE_LIBMAGIC
+#include <magic.h>
+#endif
+
+struct svn_magic__cookie_t {
+#ifdef HAVE_LIBMAGIC
+ magic_t magic;
+#endif
+ char dummy;
+};
+
+#ifdef HAVE_LIBMAGIC
+/* Close the magic database. */
+static apr_status_t
+close_magic_cookie(void *baton)
+{
+ svn_magic__cookie_t *mc = (svn_magic__cookie_t*)baton;
+ magic_close(mc->magic);
+ return APR_SUCCESS;
+}
+#endif
+
+void
+svn_magic__init(svn_magic__cookie_t **magic_cookie,
+ apr_pool_t *result_pool)
+{
+
+ svn_magic__cookie_t *mc = NULL;
+
+#ifdef HAVE_LIBMAGIC
+ mc = apr_palloc(result_pool, sizeof(*mc));
+
+ /* Initialise libmagic. */
+ mc->magic = magic_open(MAGIC_MIME_TYPE | MAGIC_ERROR);
+ if (mc->magic)
+ {
+ /* This loads the default magic database.
+ * Point the MAGIC environment variable at your favourite .mgc
+ * file to load a non-default database. */
+ if (magic_load(mc->magic, NULL) == -1)
+ {
+ magic_close(mc->magic);
+ mc = NULL;
+ }
+ else
+ apr_pool_cleanup_register(result_pool, mc, close_magic_cookie,
+ apr_pool_cleanup_null);
+ }
+#endif
+
+ *magic_cookie = mc;
+}
+
+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)
+ {
+ magic_mimetype = magic_file(magic_cookie->magic, local_abspath);
+ if (magic_mimetype)
+ {
+ /* Only return binary mime-types. */
+ if (strncmp(magic_mimetype, "text/", 5) == 0)
+ magic_mimetype = NULL;
+ else
+ {
+ /* The string is allocated from memory managed by libmagic so
+ * we must copy it to the result pool. */
+ magic_mimetype = apr_pstrdup(result_pool, magic_mimetype);
+ }
+ }
+ }
+#endif
+
+ *mimetype = magic_mimetype;
+ return SVN_NO_ERROR;
+}
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
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Jun 04, 2011 at 06:45:09PM +0300, Daniel Shahaf wrote:
> stsp@apache.org wrote on Sat, Jun 04, 2011 at 12:32:28 -0000:
> > @@ -516,6 +506,9 @@ add(void *baton, apr_pool_t *result_pool
> > svn_node_kind_t kind;
> > svn_error_t *err;
> > struct add_with_write_lock_baton *b = baton;
> > + svn_magic__cookie_t *magic_cookie;
> > +
>
> Need to initialize this to NULL, right?
That is not necessary because svn_magic__init() promises to always set it.
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
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stsp@apache.org wrote on Sat, Jun 04, 2011 at 12:32:28 -0000:
> Author: stsp
> Date: Sat Jun 4 12:32:27 2011
> New Revision: 1131383
>
> URL: http://svn.apache.org/viewvc?rev=1131383&view=rev
> Log:
> Wrap libmagic support in a private API and use this API from libsvn_client.
>
> This moves all the #ifdef HAVE_LIBMAGIC spaghetti into a single file.
> It also makes Subversion's use of libmagic more efficient for recursive
> operations because the magic database is only loaded once per invocation
> of svn_client_add() and svn_client_import(), not per every invocation of
> svn_io_detect_mimetype().
>
> Inspired by: hwright
>
> * subversion/include/private/svn_magic.h: New.
>
> * subversion/libsvn_subr/magic.c: New.
>
> * subversion/libsvn_subr/io.c
> (svn_io_detect_mimetype2): Drop libmagic support from this function.
>
> * subversion/libsvn_client/add.c
> (svn_client__get_auto_props): Add a MAGIC_COOKIE parameter for use with
> the new svn_magic API. Let mime-types obtained from libmagic override
> the mimetypes map if the map returned "application/octet-stream" (but
> do not override "application/octet-stream" configured via auto-props).
> (add_file, add_dir_recursive): Add MAGIC_COOKIE parameter.
> (add): Initialise libmagic and pass the magic cookie on to other functions.
>
> * subversion/libsvn_client/commit.c
> (import_ctx): Add a MAGIC_COOKIE here, too.
> (import_file): Pass CTX->MAGIC_COOKIE to svn_client__get_auto_props().
> (import): Initialise libmagic and store the cookie in the import context.
>
> * subversion/libsvn_client/client.h
> (svn_client__get_auto_props): Update declaration.
>
> Added:
> subversion/trunk/subversion/include/private/svn_magic.h
> subversion/trunk/subversion/libsvn_subr/magic.c
> Modified:
> subversion/trunk/subversion/libsvn_client/add.c
> subversion/trunk/subversion/libsvn_client/client.h
> subversion/trunk/subversion/libsvn_client/commit.c
> subversion/trunk/subversion/libsvn_subr/io.c
>
> Added: subversion/trunk/subversion/include/private/svn_magic.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_magic.h?rev=1131383&view=auto
> ==============================================================================
> --- subversion/trunk/subversion/include/private/svn_magic.h (added)
> +++ subversion/trunk/subversion/include/private/svn_magic.h Sat Jun 4 12:32:27 2011
> @@ -0,0 +1,55 @@
> +/**
> + * @copyright
> + * ====================================================================
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements. See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership. The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied. See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + * ====================================================================
> + * @endcopyright
> + *
> + * @file svn_magic.h
> + * @brief Subversion interface to libmagic.
> + */
> +
> +#ifndef SVN_MAGIC_H
> +#define SVN_MAGIC_H
> +
> +/* An opaque struct that wraps a libmagic cookie. */
> +typedef struct svn_magic__cookie_t svn_magic__cookie_t;
> +
> +/* This routine initialises libmagic.
> + * Upon success new *MAGIC_COOKIE is allocated in RESULT_POOL.
> + * On failure *MAGIC_COOKIE is set to NULL.
> + * All resources used by libmagic are freed by a cleanup handler
> + * installed on RESULT_POOL, i.e. *MAGIC_COOKIE becomes invalid when
> + * the pool is cleared! */
> +void
> +svn_magic__init(svn_magic__cookie_t **magic_cookie,
> + apr_pool_t *result_pool);
> +
> +/* 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);
> +
> +#endif /* SVN_MAGIC_H */
>
> Modified: subversion/trunk/subversion/libsvn_client/add.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=1131383&r1=1131382&r2=1131383&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/add.c (original)
> +++ subversion/trunk/subversion/libsvn_client/add.c Sat Jun 4 12:32:27 2011
> @@ -47,6 +47,7 @@
>
> #include "private/svn_client_private.h"
> #include "private/svn_wc_private.h"
> +#include "private/svn_magic.h"
>
> #include "svn_private_config.h"
>
> @@ -220,6 +221,7 @@ svn_error_t *
> svn_client__get_auto_props(apr_hash_t **properties,
> const char **mimetype,
> const char *path,
> + svn_magic__cookie_t *magic_cookie,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> @@ -254,38 +256,20 @@ svn_client__get_auto_props(apr_hash_t **
> SVN_ERR(svn_io_detect_mimetype2(&autoprops.mimetype, path,
> ctx->mimetypes_map, pool));
>
> -#ifdef HAVE_LIBMAGIC
> - /* We want to set an svn:mime-type property by default only on binary
> - * files. So don't set an svn:mime-type property on text files unless
> - * their mime-type appears in the map. This preserves behaviour
> - * of Subversion releases that did not include libmagic support.
> - * In those releases svn_io_detect_mimetype2() returned
> - * "application/octet-stream" or NULL unless the type was in the map. */
> - if (autoprops.mimetype && strncmp(autoprops.mimetype, "text/", 5) == 0)
> - {
> - if (ctx->mimetypes_map)
> - {
> - svn_boolean_t type_is_in_map = FALSE;
> - apr_hash_index_t *hi;
> -
> - for (hi = apr_hash_first(pool, ctx->mimetypes_map);
> - hi;
> - hi = apr_hash_next(hi))
> - {
> - const char *type_from_map = svn__apr_hash_index_val(hi);
> - if (strcmp(type_from_map, autoprops.mimetype) == 0)
> - {
> - type_is_in_map = TRUE;
> - break;
> - }
> - }
> - if (!type_is_in_map)
> - autoprops.mimetype = NULL;
> - }
> - else
> - autoprops.mimetype = NULL;
> + /* If we got no mime-type, or if it is "application/octet-stream",
> + * try to get the mime-type from libmagic. */
> + if (magic_cookie &&
> + (!autoprops.mimetype ||
> + strcmp(autoprops.mimetype, "application/octet-stream") == 0))
> + {
> + const char *magic_mimetype;
> +
> + SVN_ERR(svn_magic__detect_binary_mimetype(&magic_mimetype,
> + path, magic_cookie,
> + pool, pool));
> + if (magic_mimetype)
> + autoprops.mimetype = magic_mimetype;
> }
> -#endif
>
> if (autoprops.mimetype)
> apr_hash_set(autoprops.properties, SVN_PROP_MIME_TYPE,
> @@ -311,6 +295,7 @@ svn_client__get_auto_props(apr_hash_t **
> /* Only call this if the on-disk node kind is a file. */
> static svn_error_t *
> add_file(const char *local_abspath,
> + svn_magic__cookie_t *magic_cookie,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> @@ -331,7 +316,7 @@ add_file(const char *local_abspath,
> we open them to estimate file type.
> That's why we postpone the add until after this step. */
> SVN_ERR(svn_client__get_auto_props(&properties, &mimetype, local_abspath,
> - ctx, pool));
> + magic_cookie, ctx, pool));
>
> /* Add the file */
> SVN_ERR(svn_wc_add_from_disk(ctx->wc_ctx, local_abspath,
> @@ -402,6 +387,9 @@ add_file(const char *local_abspath,
> * Files and directories that match ignore patterns will not be added unless
> * NO_IGNORE is TRUE.
> *
> + * Use MAGIC_COOKIE (which may be NULL) to detect the mime-type of files
> + * if necessary.
> + *
> * If CTX->CANCEL_FUNC is non-null, call it with CTX->CANCEL_BATON to allow
> * the user to cancel the operation
> */
> @@ -410,6 +398,7 @@ add_dir_recursive(const char *dir_abspat
> svn_depth_t depth,
> svn_boolean_t force,
> svn_boolean_t no_ignore,
> + svn_magic__cookie_t *magic_cookie,
> svn_client_ctx_t *ctx,
> apr_pool_t *scratch_pool)
> {
> @@ -476,12 +465,13 @@ add_dir_recursive(const char *dir_abspat
> depth_below_here = svn_depth_empty;
>
> SVN_ERR(add_dir_recursive(abspath, depth_below_here,
> - force, no_ignore, ctx, iterpool));
> + force, no_ignore, magic_cookie,
> + ctx, iterpool));
> }
> else if ((dirent->kind == svn_node_file || dirent->special)
> && depth >= svn_depth_files)
> {
> - err = add_file(abspath, ctx, iterpool);
> + err = add_file(abspath, magic_cookie, ctx, iterpool);
> if (err && err->apr_err == SVN_ERR_ENTRY_EXISTS && force)
> svn_error_clear(err);
> else
> @@ -516,6 +506,9 @@ add(void *baton, apr_pool_t *result_pool
> svn_node_kind_t kind;
> svn_error_t *err;
> struct add_with_write_lock_baton *b = baton;
> + svn_magic__cookie_t *magic_cookie;
> +
Need to initialize this to NULL, right?
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
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stsp@apache.org wrote on Sat, Jun 04, 2011 at 12:32:28 -0000:
> Author: stsp
> Date: Sat Jun 4 12:32:27 2011
> New Revision: 1131383
>
> URL: http://svn.apache.org/viewvc?rev=1131383&view=rev
> Log:
> Wrap libmagic support in a private API and use this API from libsvn_client.
>
> This moves all the #ifdef HAVE_LIBMAGIC spaghetti into a single file.
> It also makes Subversion's use of libmagic more efficient for recursive
> operations because the magic database is only loaded once per invocation
> of svn_client_add() and svn_client_import(), not per every invocation of
> svn_io_detect_mimetype().
>
> Inspired by: hwright
>
> * subversion/include/private/svn_magic.h: New.
>
> * subversion/libsvn_subr/magic.c: New.
>
> * subversion/libsvn_subr/io.c
> (svn_io_detect_mimetype2): Drop libmagic support from this function.
>
> * subversion/libsvn_client/add.c
> (svn_client__get_auto_props): Add a MAGIC_COOKIE parameter for use with
> the new svn_magic API. Let mime-types obtained from libmagic override
> the mimetypes map if the map returned "application/octet-stream" (but
> do not override "application/octet-stream" configured via auto-props).
> (add_file, add_dir_recursive): Add MAGIC_COOKIE parameter.
> (add): Initialise libmagic and pass the magic cookie on to other functions.
>
> * subversion/libsvn_client/commit.c
> (import_ctx): Add a MAGIC_COOKIE here, too.
> (import_file): Pass CTX->MAGIC_COOKIE to svn_client__get_auto_props().
> (import): Initialise libmagic and store the cookie in the import context.
>
> * subversion/libsvn_client/client.h
> (svn_client__get_auto_props): Update declaration.
>
> Added:
> subversion/trunk/subversion/include/private/svn_magic.h
> subversion/trunk/subversion/libsvn_subr/magic.c
> Modified:
> subversion/trunk/subversion/libsvn_client/add.c
> subversion/trunk/subversion/libsvn_client/client.h
> subversion/trunk/subversion/libsvn_client/commit.c
> subversion/trunk/subversion/libsvn_subr/io.c
>
> Added: subversion/trunk/subversion/include/private/svn_magic.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_magic.h?rev=1131383&view=auto
> ==============================================================================
> --- subversion/trunk/subversion/include/private/svn_magic.h (added)
> +++ subversion/trunk/subversion/include/private/svn_magic.h Sat Jun 4 12:32:27 2011
> @@ -0,0 +1,55 @@
> +/**
> + * @copyright
> + * ====================================================================
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements. See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership. The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied. See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + * ====================================================================
> + * @endcopyright
> + *
> + * @file svn_magic.h
> + * @brief Subversion interface to libmagic.
> + */
> +
> +#ifndef SVN_MAGIC_H
> +#define SVN_MAGIC_H
> +
> +/* An opaque struct that wraps a libmagic cookie. */
> +typedef struct svn_magic__cookie_t svn_magic__cookie_t;
> +
> +/* This routine initialises libmagic.
> + * Upon success new *MAGIC_COOKIE is allocated in RESULT_POOL.
> + * On failure *MAGIC_COOKIE is set to NULL.
> + * All resources used by libmagic are freed by a cleanup handler
> + * installed on RESULT_POOL, i.e. *MAGIC_COOKIE becomes invalid when
> + * the pool is cleared! */
> +void
> +svn_magic__init(svn_magic__cookie_t **magic_cookie,
> + apr_pool_t *result_pool);
> +
> +/* 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);
> +
> +#endif /* SVN_MAGIC_H */
>
> Modified: subversion/trunk/subversion/libsvn_client/add.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=1131383&r1=1131382&r2=1131383&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/add.c (original)
> +++ subversion/trunk/subversion/libsvn_client/add.c Sat Jun 4 12:32:27 2011
> @@ -47,6 +47,7 @@
>
> #include "private/svn_client_private.h"
> #include "private/svn_wc_private.h"
> +#include "private/svn_magic.h"
>
> #include "svn_private_config.h"
>
> @@ -220,6 +221,7 @@ svn_error_t *
> svn_client__get_auto_props(apr_hash_t **properties,
> const char **mimetype,
> const char *path,
> + svn_magic__cookie_t *magic_cookie,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> @@ -254,38 +256,20 @@ svn_client__get_auto_props(apr_hash_t **
> SVN_ERR(svn_io_detect_mimetype2(&autoprops.mimetype, path,
> ctx->mimetypes_map, pool));
>
> -#ifdef HAVE_LIBMAGIC
> - /* We want to set an svn:mime-type property by default only on binary
> - * files. So don't set an svn:mime-type property on text files unless
> - * their mime-type appears in the map. This preserves behaviour
> - * of Subversion releases that did not include libmagic support.
> - * In those releases svn_io_detect_mimetype2() returned
> - * "application/octet-stream" or NULL unless the type was in the map. */
> - if (autoprops.mimetype && strncmp(autoprops.mimetype, "text/", 5) == 0)
> - {
> - if (ctx->mimetypes_map)
> - {
> - svn_boolean_t type_is_in_map = FALSE;
> - apr_hash_index_t *hi;
> -
> - for (hi = apr_hash_first(pool, ctx->mimetypes_map);
> - hi;
> - hi = apr_hash_next(hi))
> - {
> - const char *type_from_map = svn__apr_hash_index_val(hi);
> - if (strcmp(type_from_map, autoprops.mimetype) == 0)
> - {
> - type_is_in_map = TRUE;
> - break;
> - }
> - }
> - if (!type_is_in_map)
> - autoprops.mimetype = NULL;
> - }
> - else
> - autoprops.mimetype = NULL;
> + /* If we got no mime-type, or if it is "application/octet-stream",
> + * try to get the mime-type from libmagic. */
> + if (magic_cookie &&
> + (!autoprops.mimetype ||
> + strcmp(autoprops.mimetype, "application/octet-stream") == 0))
> + {
> + const char *magic_mimetype;
> +
> + SVN_ERR(svn_magic__detect_binary_mimetype(&magic_mimetype,
> + path, magic_cookie,
> + pool, pool));
> + if (magic_mimetype)
> + autoprops.mimetype = magic_mimetype;
> }
> -#endif
>
> if (autoprops.mimetype)
> apr_hash_set(autoprops.properties, SVN_PROP_MIME_TYPE,
> @@ -311,6 +295,7 @@ svn_client__get_auto_props(apr_hash_t **
> /* Only call this if the on-disk node kind is a file. */
> static svn_error_t *
> add_file(const char *local_abspath,
> + svn_magic__cookie_t *magic_cookie,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> @@ -331,7 +316,7 @@ add_file(const char *local_abspath,
> we open them to estimate file type.
> That's why we postpone the add until after this step. */
> SVN_ERR(svn_client__get_auto_props(&properties, &mimetype, local_abspath,
> - ctx, pool));
> + magic_cookie, ctx, pool));
>
> /* Add the file */
> SVN_ERR(svn_wc_add_from_disk(ctx->wc_ctx, local_abspath,
> @@ -402,6 +387,9 @@ add_file(const char *local_abspath,
> * Files and directories that match ignore patterns will not be added unless
> * NO_IGNORE is TRUE.
> *
> + * Use MAGIC_COOKIE (which may be NULL) to detect the mime-type of files
> + * if necessary.
> + *
> * If CTX->CANCEL_FUNC is non-null, call it with CTX->CANCEL_BATON to allow
> * the user to cancel the operation
> */
> @@ -410,6 +398,7 @@ add_dir_recursive(const char *dir_abspat
> svn_depth_t depth,
> svn_boolean_t force,
> svn_boolean_t no_ignore,
> + svn_magic__cookie_t *magic_cookie,
> svn_client_ctx_t *ctx,
> apr_pool_t *scratch_pool)
> {
> @@ -476,12 +465,13 @@ add_dir_recursive(const char *dir_abspat
> depth_below_here = svn_depth_empty;
>
> SVN_ERR(add_dir_recursive(abspath, depth_below_here,
> - force, no_ignore, ctx, iterpool));
> + force, no_ignore, magic_cookie,
> + ctx, iterpool));
> }
> else if ((dirent->kind == svn_node_file || dirent->special)
> && depth >= svn_depth_files)
> {
> - err = add_file(abspath, ctx, iterpool);
> + err = add_file(abspath, magic_cookie, ctx, iterpool);
> if (err && err->apr_err == SVN_ERR_ENTRY_EXISTS && force)
> svn_error_clear(err);
> else
> @@ -516,6 +506,9 @@ add(void *baton, apr_pool_t *result_pool
> svn_node_kind_t kind;
> svn_error_t *err;
> struct add_with_write_lock_baton *b = baton;
> + svn_magic__cookie_t *magic_cookie;
> +
Need to initialize this to NULL, right?
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
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Jun 06, 2011 at 10:47:36AM +0100, Philip Martin wrote:
> Stefan Sperling <st...@elego.de> writes:
> > So setting a binary mime-type on empty files by default is really not
> > a great idea. Even from the end-user perspective, not just in the test
> > suite. If people set it manually then they can deal with the consequences.
>
> The user can configure things like auto-props, external diff, etc. in
> such a way that the testsuite would fail. We allow these configurations
> because the user may have legitimate reasons for wanting Subversion to
> behave this way. The testsuite doesn't use the user or system values,
> it uses its own values that are known to work.
>
> Why is the magic database different? Perhaps there are users that want
> empty files to have a particular mime type automatically?
Fair enough.
For the tests I suppose we'd want a DB that returns either "text/plain"
or "application/octet-stream", but no other mime-types?
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
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> On Mon, Jun 06, 2011 at 09:08:11AM +0100, Philip Martin wrote:
>> Stefan Sperling <st...@elego.de> writes:
>>
>> > If an empty file gets a property that svn considers binary the tests
>> > start failing all over the place.
>>
>> Perhaps the testsuite should use its own magic database?
>
> Well, that would just cover up symptoms in the test suite.
>
> The problem is that some operations behave differently.
> For instance, running diffs against empty files stops working.
> If people use empty files to try things out in Subversion,
> they won't expect it to complain, by default(!), when certain
> operations are run on the these files.
>
> So setting a binary mime-type on empty files by default is really not
> a great idea. Even from the end-user perspective, not just in the test
> suite. If people set it manually then they can deal with the consequences.
The user can configure things like auto-props, external diff, etc. in
such a way that the testsuite would fail. We allow these configurations
because the user may have legitimate reasons for wanting Subversion to
behave this way. The testsuite doesn't use the user or system values,
it uses its own values that are known to work.
Why is the magic database different? Perhaps there are users that want
empty files to have a particular mime type automatically?
--
Philip
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
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Jun 06, 2011 at 09:08:11AM +0100, Philip Martin wrote:
> Stefan Sperling <st...@elego.de> writes:
>
> > If an empty file gets a property that svn considers binary the tests
> > start failing all over the place.
>
> Perhaps the testsuite should use its own magic database?
Well, that would just cover up symptoms in the test suite.
The problem is that some operations behave differently.
For instance, running diffs against empty files stops working.
If people use empty files to try things out in Subversion,
they won't expect it to complain, by default(!), when certain
operations are run on the these files.
So setting a binary mime-type on empty files by default is really not
a great idea. Even from the end-user perspective, not just in the test
suite. If people set it manually then they can deal with the consequences.
Also, if someone out there can find a magic DB that breaks our tests,
then I'd much rather fix our magic.c code. We must treat stuff coming
from libmagic as untrusted input, and not fail just because it returns
some junk.
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
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> If an empty file gets a property that svn considers binary the tests
> start failing all over the place.
Perhaps the testsuite should use its own magic database?
--
Philip
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
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Jun 04, 2011 at 06:17:22PM +0200, Stefan Sperling wrote:
> On Sat, Jun 04, 2011 at 07:11:05PM +0300, Daniel Shahaf wrote:
> > stsp@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?
If you still plan on doing this then please take into account the
comment added in r1133367. We should keep flagging UTF-16 files as
"application/octet-stream" (or some other binary type) for now.
But libmagic usually reports them as "text/plain", and the current
code happens to deal fine with that. (Maybe we should add a regression
test for this but I don't have enough time right now...)
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
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Jun 04, 2011 at 07:11:05PM +0300, Daniel Shahaf wrote:
> stsp@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.
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
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stsp@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.
> +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.
> + magic_mimetype = magic_file(magic_cookie->magic, local_abspath);
> + if (magic_mimetype)
> + {
> + /* Only return binary mime-types. */
> + if (strncmp(magic_mimetype, "text/", 5) == 0)
> + magic_mimetype = NULL;
> + else
> + {
> + /* The string is allocated from memory managed by libmagic so
> + * we must copy it to the result pool. */
> + magic_mimetype = apr_pstrdup(result_pool, magic_mimetype);
> + }
> + }
> + }
> +#endif
> +
> + *mimetype = magic_mimetype;
> + return SVN_NO_ERROR;
> +}
>
>
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
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stsp@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.
> +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.
> + magic_mimetype = magic_file(magic_cookie->magic, local_abspath);
> + if (magic_mimetype)
> + {
> + /* Only return binary mime-types. */
> + if (strncmp(magic_mimetype, "text/", 5) == 0)
> + magic_mimetype = NULL;
> + else
> + {
> + /* The string is allocated from memory managed by libmagic so
> + * we must copy it to the result pool. */
> + magic_mimetype = apr_pstrdup(result_pool, magic_mimetype);
> + }
> + }
> + }
> +#endif
> +
> + *mimetype = magic_mimetype;
> + return SVN_NO_ERROR;
> +}
>
>