On 16. 3. 26 21:23, Timofei Zhakov wrote:
I'm currently revising the utf8-cmdline branch, and I had to make a
change to the function that was used to check a file for existence.
It's backed by a function that is called apr_stat in APR.

Subversion has a wrapper around it under svn_io_stat name. It adds
conversion from UTF8 to native encoding and svn_error error handling.

Here is everything it basically does:

[[[
svn_error_t *
svn_io_stat(apr_finfo_t *finfo, const char *fname,
             apr_int32_t wanted, apr_pool_t *pool)
{
   apr_status_t status;
   const char *fname_apr;

   /* APR doesn't like "" directories */
   if (fname[0] == '\0')
     fname = ".";

   SVN_ERR(cstring_from_utf8(&fname_apr, fname, pool));

   /* Quoting APR: On NT this request is incredibly expensive, but accurate. */
   wanted &= ~SVN__APR_FINFO_MASK_OUT;

   status = apr_stat(finfo, fname_apr, wanted, pool);
   if (status)
     return svn_error_wrap_apr(status, _("Can't stat '%s'"),
                               svn_dirent_local_style(fname, pool));

   return SVN_NO_ERROR;
}
]]]

And here is the change made in utf8-cmdline (taken from a diff after
running merge into trunk):

[[[
@@ -3179,23 +3151,33 @@ sub_main(int *exit_code,
        if (opt_state.message)
          {
            apr_finfo_t finfo;
-          if (apr_stat(&finfo, opt_state.message /* not converted to UTF-8 */,
-                       APR_FINFO_MIN, pool) == APR_SUCCESS)
+
+          /* We don't want to warn for '' */
+          if (opt_state.message[0] != '\0')
              {
-              if (subcommand->cmd_func != svn_cl__lock)
+              err = svn_io_stat(&finfo, opt_state.message,
+                                APR_FINFO_MIN, pool);
+
+              if (!err)
                  {
-                  return svn_error_create
-                    (SVN_ERR_CL_LOG_MESSAGE_IS_PATHNAME, NULL,
-                     _("The log message is a pathname "
-                       "(was -F intended?); use '--force-log' to override"));
+                  if (subcommand->cmd_func != svn_cl__lock)
+                    {
+                      return svn_error_create(
+                          SVN_ERR_CL_LOG_MESSAGE_IS_PATHNAME, NULL,
+                          _("The log message is a pathname "
+                            "(was -F intended?); use '--force-log' to "
+                            "override"));
+                    }
+                  else
+                    {
+                      return svn_error_create(
+                          SVN_ERR_CL_LOG_MESSAGE_IS_PATHNAME, NULL,
+                          _("The lock comment is a pathname "
+                            "(was -F intended?); use '--force-log' to "
+                            "override"));
+                    }
                  }
-              else
-                {
-                  return svn_error_create
-                    (SVN_ERR_CL_LOG_MESSAGE_IS_PATHNAME, NULL,
-                     _("The lock comment is a pathname "
-                       "(was -F intended?); use '--force-log' to override"));
-                }
+              svn_error_clear(err);
              }
          }
      }
]]]

Assuming there are no silly logical mistakes, it does not change any
existing behaviour.

I don't quite understand. Are you saying that opt_state.message is now encoded in UTF-8? Because if it isn't, that svn_io_stat() can't possibly work.

Also I don't see how this:

+          /* We don't want to warn for '' */
+          if (opt_state.message[0] != '\0')


could be considered not changing behaviour. It could be fixing wrong behaviour, but that's certainly changing it.


  I'm considering committing it into trunk
separately before the rest of changes made in that branch. The only
difference is that as at the current state of trunk, all arguments are
handled in a native encoding, hence should be converted using the
svn_path_cstring_from_utf8 function. This is not shown in the patch
above.

If anyone has any concerns about that, please speak up. I remember
there was a discussion that has not been entirely resolved.

Reply via email to