Re: [PATCH] compat: expose canonicalize_file_name to C++

["Đoàn Trần Công Danh" <congdanhqx@gmail.com>]

On 2021-04-17 11:39:59-0300, David Bremner <david@tethera.net> wrote:
> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
> 
> >
> > However, I see that lib/open.cc uses g_key_file_get_value from GLib
> > already, we may switch to g_canonicalize_file_name then?
> >
> 
> Yes that could work. I think the treatment of NULL input might need some
> extra care with g_canonicalize_file_name; at least my 5 minute attempt
> to do a replacement causes many test failures. Do you want to make a
> patch that uses g_canonicalize_file_name? The one other place we use
> canonicalize_file_name (not totally coincidentally) also uses glib, so
> in principle we could completely eliminate the compat function.

Now, I'm thinking more about it and digging into GLib history,
I think using g_canonicalize_filename would be problem, since it's
available from 2.57.1/2.58 only. I think we're requiring glib-2.0 2.22
as of it's now. A big jump in dependencies isn't worth it.

I think below patch would be better?

Anyway, I see some failure in the testsuite due to:
- *My* hostname(1) (from coreutils) doesn't understand "-f"
- All emacs tests depend on dtach(1) but the
  test_require_external_prereq dtach is missing.  Would you want
  to see those patches for test_require_external_prereq dtach?
  It's pretty trivial.
-----8<-----
Subject: [PATCH] compat: rename {,notmuch_}canonicalize_file_name

When compat canonicalize_file_name was introduced, it was limited to
C code only because it was used by C code only during that time.

From 5ec6fd4d, (lib/open: check for split configuration when creating
database., 2021-02-16), lib/open.cc, which is C++, relies on the
existent of canonicalize_file_name.

However, we can't blindly enable canonicalize_file_name for C++ code,
because different implementation has different additional signature for
C++ and users can arbitrarily add -DHAVE_CANONICALIZE_FILE_NAME=0 to
{C,CXX}FLAGS.

Let's put our implementation of canonicalize_file_name to our namespace,
and prefer system's canonicalize_file_name if it's existed via a macro.
---
 compat/canonicalize_file_name.c | 6 +++++-
 compat/compat.h                 | 7 +++----
 lib/open.cc                     | 4 ++--
 notmuch-config.c                | 2 +-
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/compat/canonicalize_file_name.c b/compat/canonicalize_file_name.c
index 000f9e78..ba003268 100644
--- a/compat/canonicalize_file_name.c
+++ b/compat/canonicalize_file_name.c
@@ -3,8 +3,12 @@
 #undef _GNU_SOURCE
 #include <stdlib.h>
 
+#ifdef notmuch_canonicalize_file_name
+#undef notmuch_canonicalize_file_name
+#endif
+
 char *
-canonicalize_file_name (const char *path)
+notmuch_canonicalize_file_name (const char *path)
 {
 #ifdef PATH_MAX
     char *resolved_path =  malloc (PATH_MAX + 1);
diff --git a/compat/compat.h b/compat/compat.h
index 8f15e585..a38bc460 100644
--- a/compat/compat.h
+++ b/compat/compat.h
@@ -38,11 +38,10 @@ extern "C" {
 #endif
 
 #if ! HAVE_CANONICALIZE_FILE_NAME
-/* we only call this function from C, and this makes testing easier */
-#ifndef __cplusplus
 char *
-canonicalize_file_name (const char *path);
-#endif
+notmuch_canonicalize_file_name (const char *path);
+#else
+#define notmuch_canonicalize_file_name canonicalize_file_name
 #endif
 
 #if ! HAVE_GETLINE
diff --git a/lib/open.cc b/lib/open.cc
index 5d80a884..7454ffae 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -612,9 +612,9 @@ notmuch_database_create_with_config (const char *database_path,
     _set_database_path (notmuch, database_path);
 
     if (key_file && ! split) {
-	char *mail_root = canonicalize_file_name (
+	char *mail_root = notmuch_canonicalize_file_name (
 	    g_key_file_get_value (key_file, "database", "mail_root", NULL));
-	char *db_path = canonicalize_file_name (database_path);
+	char *db_path = notmuch_canonicalize_file_name (database_path);
 
 	split = (mail_root && (0 != strcmp (mail_root, db_path)));
 
diff --git a/notmuch-config.c b/notmuch-config.c
index 16e86916..d8d47768 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -327,7 +327,7 @@ notmuch_conffile_save (notmuch_conffile_t *config)
     }
 
     /* Try not to overwrite symlinks. */
-    filename = canonicalize_file_name (config->filename);
+    filename = notmuch_canonicalize_file_name (config->filename);
     if (! filename) {
 	if (errno == ENOENT) {
 	    filename = strdup (config->filename);
-- 
2.31.1



-- 
Danh