Re: svn commit: r1431279 - /subversion/trunk/subversion/libsvn_wc/wc_db_util.c

Philip Martin wrote:

> URL: http://svn.apache.org/viewvc?rev=1431279&view=rev

> Log:
> Ensure wc.db has the permissions allowed by umask.

... in order to be consistent with the other files we write,
and to fix sharing a WC?

If this bug was breaking sharing a WC, should we back-port this fix?

> * subversion/libsvn_wc/wc_db_util.c
>   (svn_wc__db_util_open_db): Ensure wc.db exists before it is opened by SQLite.

> +#ifndef WIN32
> +  else
> +    {
> +      apr_file_t *f;
> +
> +      /* A standard SQLite build creates a DB with mode 644 ^ !umask

Should the expression say "0644 & ~umask", using C syntax?

> +        which means the file doesn't have group/world write access
> +        even when umask allows it. By ensuring the file exists before
> +        SQLite gets involved we give it the permissions allowed by
> +        umask. */

It would be good to have a hint in the comment about *why*, even if just "for consistency".

> +      SVN_ERR(svn_io_file_open(&f, sdb_abspath,
> +                              (APR_READ | APR_WRITE | APR_CREATE),
> +                              APR_OS_DEFAULT, scratch_pool));
> +      SVN_ERR(svn_io_file_close(f, scratch_pool));
> +    }
> +#endif
>
>   SVN_ERR(svn_sqlite__open(sdb, sdb_abspath, smode,
>                             my_statements ? my_statements : statements,

If we consider this to be a fix for the way SQLite behaves, do we want the same throughout Subversion?  Move the fix into svn_sqlite__open()?  File a bug report with SQLite project?

- Julian
Received on 2013-01-10 14:23:59 CET