[PATCH] compat: expose canonicalize_file_name to C++

[PATCH] compat: expose canonicalize_file_name to C++

[Đoàn Trần Công Danh]

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.

Let's remove the language restriction to support those platforms don't
have canonicalize_file_name(3).
---
 compat/compat.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/compat/compat.h b/compat/compat.h
index 8f15e585..e418e62c 100644
--- a/compat/compat.h
+++ b/compat/compat.h
@@ -38,12 +38,9 @@ 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
-#endif
 
 #if ! HAVE_GETLINE
 #include <stdio.h>
-- 
2.31.1.192.g0881477623

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

[David Bremner]

Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:

> 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.
>
> Let's remove the language restriction to support those platforms don't
> have canonicalize_file_name(3).

Thanks for the patch, but it seems that we need to change some other
things to make this work with C++?

I set HAVE_CANONICALIZE_FILE_NAME = 0 in Makefile.config, and with g++
10.2 on Debian I get

CXX  -g -O2 lib/database.o
In file included from /usr/include/c++/10/cstdlib:75,
                 from /usr/include/c++/10/stdlib.h:36,
                 from ./util/xutil.h:24,
                 from lib/notmuch-private.h:34,
                 from lib/database-private.h:33,
                 from lib/database.cc:21:
/usr/include/stdlib.h:790:14: error: declaration of ‘char* canonicalize_file_name(const char*) throw ()’ has a different exception specifier
  790 | extern char *canonicalize_file_name (const char *__name)
      |              ^~~~~~~~~~~~~~~~~~~~~~
In file included from lib/notmuch-private.h:30,
                 from lib/database-private.h:33,
                 from lib/database.cc:21:
./compat/compat.h:42:1: note: from previous declaration ‘char* canonicalize_file_name(const char*)’
   42 | canonicalize_file_name (const char *path);
      | ^~~~~~~~~~~~~~~~~~~~~~
make: *** [Makefile.local:200: lib/database.o] Error 1

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

[Đoàn Trần Công Danh]

On 2021-04-17 09:13:19-0300, David Bremner <david@tethera.net> wrote:
> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
> 
> > 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.
> >
> > Let's remove the language restriction to support those platforms don't
> > have canonicalize_file_name(3).
> 
> Thanks for the patch, but it seems that we need to change some other
> things to make this work with C++?
> 
> I set HAVE_CANONICALIZE_FILE_NAME = 0 in Makefile.config, and with g++
> 10.2 on Debian I get
> 
> CXX  -g -O2 lib/database.o
> In file included from /usr/include/c++/10/cstdlib:75,
>                  from /usr/include/c++/10/stdlib.h:36,
>                  from ./util/xutil.h:24,
>                  from lib/notmuch-private.h:34,
>                  from lib/database-private.h:33,
>                  from lib/database.cc:21:
> /usr/include/stdlib.h:790:14: error: declaration of ‘char* canonicalize_file_name(const char*) throw ()’ has a different exception specifier
>   790 | extern char *canonicalize_file_name (const char *__name)
>       |              ^~~~~~~~~~~~~~~~~~~~~~
> In file included from lib/notmuch-private.h:30,
>                  from lib/database-private.h:33,
>                  from lib/database.cc:21:
> ./compat/compat.h:42:1: note: from previous declaration ‘char* canonicalize_file_name(const char*)’
>    42 | canonicalize_file_name (const char *path);
>       | ^~~~~~~~~~~~~~~~~~~~~~

Sorry, I forgot to test this change in a glibc system.

I'm not really sure how to workaround this since other systems may
have canonicalize_file_name(3) with different signature, let's say
with or without noexcept, throw().

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?

-- 
Danh

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

[David Bremner]

Đ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.

d

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

[Đoàn Trần Công Danh]

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

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

[Tomi Ollila]

On Sun, Apr 18 2021, Đoàn Trần Công Danh wrote:

> 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?

If that worked (I did not test), instead of macro "hackery", inline function
could be better(?) i.e. something like the following in notmuch_compat.h:

#if HAVE_CANONICALIZE_FILE_NAME
static inline char *
notmuch_canonicalize_file_name (const char *path)
{
        return canonicalize_file_name (path)
}
#else 
char *
notmuch_canonicalize_file_name (const char *path);
#endif


(OTOH I'd like to get upgraded to 2.57.1 but I still have 2.49 in the
system that builds this notmuch so...

    # 2.57.1 works w/o meson/ninja (and has ./configure) but requires libmount
    #glib_lnk=http://ftp.gnome.org/pub/gnome/sources/glib/2.57/glib-2.57.1.tar.xz
    #glib_cks=d029e7c4536835f1f103472f7510332c28d58b9b7d6cd0e9f45c2653e670d9b4

    glib_lnk=http://ftp.gnome.org/pub/gnome/sources/glib/2.49/glib-2.49.4.tar.xz
    glib_cks=9e914f9d7ebb88f99f234a7633368a7c1133ea21b5cac9db2a33bc25f7a0e0d1
)


> -/* 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


>
> 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)
w>  {
>  #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
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

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

[Đoàn Trần Công Danh]

On 2021-04-18 10:08:31+0300, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sun, Apr 18 2021, Đoàn Trần Công Danh wrote:
> 
> > 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?
> 
> If that worked (I did not test), instead of macro "hackery", inline function
> could be better(?) i.e. something like the following in notmuch_compat.h:
> 
> #if HAVE_CANONICALIZE_FILE_NAME
> static inline char *
> notmuch_canonicalize_file_name (const char *path)
> {
>         return canonicalize_file_name (path)
> }
> #else 
> char *
> notmuch_canonicalize_file_name (const char *path);
> #endif
> 

Yes, inline function are always better than macro.
I feel embarassed that I couldn't think about that earlier.

Here is a revised patch:
---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 an inline
function.

Helped-by: Tomi Ollila <tomi.ollila@iki.fi>
---
 compat/canonicalize_file_name.c |  2 +-
 compat/compat.h                 | 13 ++++++++-----
 lib/open.cc                     |  4 ++--
 notmuch-config.c                |  2 +-
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/compat/canonicalize_file_name.c b/compat/canonicalize_file_name.c
index 000f9e78..84f2a2c2 100644
--- a/compat/canonicalize_file_name.c
+++ b/compat/canonicalize_file_name.c
@@ -4,7 +4,7 @@
 #include <stdlib.h>
 
 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..c3ef4051 100644
--- a/compat/compat.h
+++ b/compat/compat.h
@@ -37,12 +37,15 @@ extern "C" {
 #define _POSIX_PTHREAD_SEMANTICS 1
 #endif
 
-#if ! HAVE_CANONICALIZE_FILE_NAME
-/* we only call this function from C, and this makes testing easier */
-#ifndef __cplusplus
+#if HAVE_CANONICALIZE_FILE_NAME
+static inline char *
+notmuch_canonicalize_file_name (const char *path)
+{
+	return canonicalize_file_name (path);
+}
+#else
 char *
-canonicalize_file_name (const char *path);
-#endif
+notmuch_canonicalize_file_name (const char *path);
 #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);
-- 

Danh

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

[David Bremner]

Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:

>
> Yes, inline function are always better than macro.
> I feel embarassed that I couldn't think about that earlier.
>
> Here is a revised patch:

This version still has some issues on a glibc system.

,----
| In file included from notmuch-client.h:31,
|                  from debugger.c:21:
| ./compat/compat.h: In function ‘notmuch_canonicalize_file_name’:
| ./compat/compat.h:44:9: warning: implicit declaration of function ‘canonicalize_file_name’; did you mean ‘notmuch_canonicalize_file_name’? [-Wimplicit-function-declaration]
|    44 |  return canonicalize_file_name (path);
|       |         ^~~~~~~~~~~~~~~~~~~~~~
|       |         notmuch_canonicalize_file_name
| ./compat/compat.h:44:9: warning: returning ‘int’ from a function with return type ‘char *’ makes pointer from integer without a cast [-Wint-conversion]
|    44 |  return canonicalize_file_name (path);
|       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
`----

I guess compat.h needs to define _GNU_SOURCE and include stdlib.h?

,----
| /usr/include/stdlib.h:790:14: error: conflicting types for ‘canonicalize_file_name’
|   790 | extern char *canonicalize_file_name (const char *__name)
|       |              ^~~~~~~~~~~~~~~~~~~~~~
| In file included from notmuch-client.h:31,
|                  from debugger.c:21:
| ./compat/compat.h:44:9: note: previous implicit declaration of ‘canonicalize_file_name’ was here
|    44 |  return canonicalize_file_name (path);
|       |         ^~~~~~~~~~~~~~~~~~~~~~
| make: *** [Makefile.local:204: debugger.o] Error 1
`----

I guess that's the same issue?

I was thinking if we are going to have provide
notmuch_canonicalize_file_name, it might make more sense to start e.g.
util/path-util.{c,h}, and put the relevant inline (or not) function
there.  I guess either the current approach (with appropriate fixes), or
putting things in util is fine.

d

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

[Đoàn Trần Công Danh]

On 2021-04-23 14:31:43-0300, David Bremner <david@tethera.net> wrote:
> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
> 
> >
> > Yes, inline function are always better than macro.
> > I feel embarassed that I couldn't think about that earlier.
> >
> > Here is a revised patch:
> 
> This version still has some issues on a glibc system.
> 
> ,----
> | In file included from notmuch-client.h:31,
> |                  from debugger.c:21:
> | ./compat/compat.h: In function ‘notmuch_canonicalize_file_name’:
> | ./compat/compat.h:44:9: warning: implicit declaration of function ‘canonicalize_file_name’; did you mean ‘notmuch_canonicalize_file_name’? [-Wimplicit-function-declaration]
> |    44 |  return canonicalize_file_name (path);
> |       |         ^~~~~~~~~~~~~~~~~~~~~~
> |       |         notmuch_canonicalize_file_name
> | ./compat/compat.h:44:9: warning: returning ‘int’ from a function with return type ‘char *’ makes pointer from integer without a cast [-Wint-conversion]
> |    44 |  return canonicalize_file_name (path);
> |       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> `----
> 
> I guess compat.h needs to define _GNU_SOURCE and include stdlib.h?
> 
> ,----
> | /usr/include/stdlib.h:790:14: error: conflicting types for ‘canonicalize_file_name’
> |   790 | extern char *canonicalize_file_name (const char *__name)
> |       |              ^~~~~~~~~~~~~~~~~~~~~~
> | In file included from notmuch-client.h:31,
> |                  from debugger.c:21:
> | ./compat/compat.h:44:9: note: previous implicit declaration of ‘canonicalize_file_name’ was here
> |    44 |  return canonicalize_file_name (path);
> |       |         ^~~~~~~~~~~~~~~~~~~~~~
> | make: *** [Makefile.local:204: debugger.o] Error 1
> `----
> 
> I guess that's the same issue?
> 
> I was thinking if we are going to have provide
> notmuch_canonicalize_file_name, it might make more sense to start e.g.
> util/path-util.{c,h}, and put the relevant inline (or not) function
> there.  I guess either the current approach (with appropriate fixes), or
> putting things in util is fine.

I've tried to put

	#define _GNU_SOURCE
	#include <stdlib.h>

in compat.h for HAVE_CANONICALIZE_FILE_NAME

However, other source files include stdlib.h w/o _GNU_SOURCE before
compat.h, thus declines to include stdlib.h again.

If we prefer to add above code into compat.h, I think it'd be an
invasive change (move all #include "compat.h") as first inclusion in
all files, for a long run, it sounds like a better move. I don't think
it's wise for me to go with that route, though.

Starting util/path-util.{c,h} sounds like a nice alternate to me.

-- 
Danh

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

[David Bremner]

Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:

>
> Anyway, I see some failure in the testsuite due to:
> - *My* hostname(1) (from coreutils) doesn't understand "-f"

ah, any suggestions for a portable replacement? I guess some perl one
liner might work.

> - 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.

Sure.

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

[Tomi Ollila]

On Sun, Apr 18 2021, David Bremner wrote:

> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
>
>>
>> Anyway, I see some failure in the testsuite due to:
>> - *My* hostname(1) (from coreutils) doesn't understand "-f"

Interesting (neither of these use coreutils hostname) 

    $ rpm -q -f =hostname
    hostname-3.23-3.fc33.x86_64 

    $ rpm -q -f =hostname
    'net-tools-1.60-109.el6.x86_64

Out of curiosity, about non-linux systems hostname...

>
> ah, any suggestions for a portable replacement? I guess some perl one
> liner might work.

not perl, but python3 -c 'import socket; print(socket.getfqdn())'

(dropped other comments I had for notmuch_passwd_sanitize() (this time ;) 

Tomi

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

[Tomi Ollila]

On Sun, Apr 18 2021, Tomi Ollila wrote:

>
> not perl, but python3 -c 'import socket; print(socket.getfqdn())'

notmuch_passwd_sanitize()
{
    python3 -c '
import os, sys, pwd, socket

pw = pwd.getpwuid(os.getuid())
user = pw.pw_name
name = pw.pw_gecos.partition(",")[0]
fqdn = socket.getfqdn()

for l in sys.stdin:
    l = l.replace(user, "USERNAME").replace(fqdn, "FQDN").replace(name, "USER_FULL_NAME")
   sys.stdout.write(l)
'
}

could work, could not remember whether it was PYTHON or NOTMUCH_PYTHON,
and tested only manually on command line...

>
> Tomi

Tomi

[PATCH v3] compat: rename {,notmuch_}canonicalize_file_name

[Đoàn Trần Công Danh]

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 move our implementation into a util library.

Helped-by: Tomi Ollila <tomi.ollila@iki.fi>
Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 compat/Makefile.local                         |  4 ----
 compat/compat.h                               |  8 --------
 lib/open.cc                                   |  5 +++--
 notmuch-config.c                              |  3 ++-
 util/Makefile.local                           |  2 +-
 .../path-util.c                               | 17 +++++++++++++----
 util/path-util.h                              | 19 +++++++++++++++++++
 7 files changed, 38 insertions(+), 20 deletions(-)
 rename compat/canonicalize_file_name.c => util/path-util.c (52%)
 create mode 100644 util/path-util.h

diff --git a/compat/Makefile.local b/compat/Makefile.local
index 2ee1b399..c58ca746 100644
--- a/compat/Makefile.local
+++ b/compat/Makefile.local
@@ -5,10 +5,6 @@ extra_cflags += -I$(srcdir)/$(dir)
 
 notmuch_compat_srcs :=
 
-ifneq ($(HAVE_CANONICALIZE_FILE_NAME),1)
-notmuch_compat_srcs += $(dir)/canonicalize_file_name.c
-endif
-
 ifneq ($(HAVE_GETLINE),1)
 notmuch_compat_srcs += $(dir)/getline.c $(dir)/getdelim.c
 endif
diff --git a/compat/compat.h b/compat/compat.h
index 8f15e585..59e91618 100644
--- a/compat/compat.h
+++ b/compat/compat.h
@@ -37,14 +37,6 @@ extern "C" {
 #define _POSIX_PTHREAD_SEMANTICS 1
 #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
-#endif
-
 #if ! HAVE_GETLINE
 #include <stdio.h>
 #include <unistd.h>
diff --git a/lib/open.cc b/lib/open.cc
index 5d80a884..bdb695fe 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -3,6 +3,7 @@
 
 #include "database-private.h"
 #include "parse-time-vrp.h"
+#include "path-util.h"
 
 #if HAVE_XAPIAN_DB_RETRY_LOCK
 #define DB_ACTION (Xapian::DB_CREATE_OR_OPEN | Xapian::DB_RETRY_LOCK)
@@ -612,9 +613,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..d9390c4d 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -24,6 +24,7 @@
 #include <netdb.h>
 #include <assert.h>
 
+#include "path-util.h"
 #include "unicode-util.h"
 
 static const char toplevel_config_comment[] =
@@ -327,7 +328,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);
diff --git a/util/Makefile.local b/util/Makefile.local
index 7ef029a5..8a0b9bc3 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -6,7 +6,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 libnotmuch_util_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
 		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c \
 		$(dir)/util.c $(dir)/gmime-extra.c $(dir)/crypto.c \
-		$(dir)/repair.c \
+		$(dir)/repair.c $(dir)/path-util.c \
 		$(dir)/unicode-util.c
 
 libnotmuch_util_modules := $(libnotmuch_util_c_srcs:.c=.o)
diff --git a/compat/canonicalize_file_name.c b/util/path-util.c
similarity index 52%
rename from compat/canonicalize_file_name.c
rename to util/path-util.c
index 000f9e78..4b352baf 100644
--- a/compat/canonicalize_file_name.c
+++ b/util/path-util.c
@@ -1,12 +1,21 @@
-#include "compat.h"
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+#define _GNU_SOURCE
+
+#include "path-util.h"
+
 #include <limits.h>
-#undef _GNU_SOURCE
 #include <stdlib.h>
 
+
 char *
-canonicalize_file_name (const char *path)
+notmuch_canonicalize_file_name (const char *path)
 {
-#ifdef PATH_MAX
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+    return canonicalize_file_name (path);
+#elif defined(PATH_MAX)
     char *resolved_path =  malloc (PATH_MAX + 1);
     if (resolved_path == NULL)
 	return NULL;
diff --git a/util/path-util.h b/util/path-util.h
new file mode 100644
index 00000000..ac85f696
--- /dev/null
+++ b/util/path-util.h
@@ -0,0 +1,19 @@
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+#ifndef NOTMUCH_UTIL_PATH_UTIL_H_
+#define NOTMUCH_UTIL_PATH_UTIL_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+char *
+notmuch_canonicalize_file_name (const char *path);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* NOTMUCH_UTIL_PATH_UTIL_H_ */
-- 
2.31.1.500.gbc6bbdd36b\r

[PATCH v3] compat: rename {,notmuch_}canonicalize_file_name

[Đoàn Trần Công Danh]

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 move our implementation into a util library.

Helped-by: Tomi Ollila <tomi.ollila@iki.fi>
Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
Interdiff against v2:
  diff --git a/util/path-util.c b/util/path-util.c
  index 4b352baf..3267a967 100644
  --- a/util/path-util.c
  +++ b/util/path-util.c
  @@ -13,7 +13,7 @@
   char *
   notmuch_canonicalize_file_name (const char *path)
   {
  -#ifdef HAVE_CANONICALIZE_FILE_NAME
  +#if HAVE_CANONICALIZE_FILE_NAME
       return canonicalize_file_name (path);
   #elif defined(PATH_MAX)
       char *resolved_path =  malloc (PATH_MAX + 1);

 compat/Makefile.local                         |  4 ----
 compat/compat.h                               |  8 --------
 lib/open.cc                                   |  5 +++--
 notmuch-config.c                              |  3 ++-
 util/Makefile.local                           |  2 +-
 .../path-util.c                               | 17 +++++++++++++----
 util/path-util.h                              | 19 +++++++++++++++++++
 7 files changed, 38 insertions(+), 20 deletions(-)
 rename compat/canonicalize_file_name.c => util/path-util.c (53%)
 create mode 100644 util/path-util.h

diff --git a/compat/Makefile.local b/compat/Makefile.local
index 2ee1b399..c58ca746 100644
--- a/compat/Makefile.local
+++ b/compat/Makefile.local
@@ -5,10 +5,6 @@ extra_cflags += -I$(srcdir)/$(dir)
 
 notmuch_compat_srcs :=
 
-ifneq ($(HAVE_CANONICALIZE_FILE_NAME),1)
-notmuch_compat_srcs += $(dir)/canonicalize_file_name.c
-endif
-
 ifneq ($(HAVE_GETLINE),1)
 notmuch_compat_srcs += $(dir)/getline.c $(dir)/getdelim.c
 endif
diff --git a/compat/compat.h b/compat/compat.h
index 8f15e585..59e91618 100644
--- a/compat/compat.h
+++ b/compat/compat.h
@@ -37,14 +37,6 @@ extern "C" {
 #define _POSIX_PTHREAD_SEMANTICS 1
 #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
-#endif
-
 #if ! HAVE_GETLINE
 #include <stdio.h>
 #include <unistd.h>
diff --git a/lib/open.cc b/lib/open.cc
index 5d80a884..bdb695fe 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -3,6 +3,7 @@
 
 #include "database-private.h"
 #include "parse-time-vrp.h"
+#include "path-util.h"
 
 #if HAVE_XAPIAN_DB_RETRY_LOCK
 #define DB_ACTION (Xapian::DB_CREATE_OR_OPEN | Xapian::DB_RETRY_LOCK)
@@ -612,9 +613,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..d9390c4d 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -24,6 +24,7 @@
 #include <netdb.h>
 #include <assert.h>
 
+#include "path-util.h"
 #include "unicode-util.h"
 
 static const char toplevel_config_comment[] =
@@ -327,7 +328,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);
diff --git a/util/Makefile.local b/util/Makefile.local
index 7ef029a5..8a0b9bc3 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -6,7 +6,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 libnotmuch_util_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
 		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c \
 		$(dir)/util.c $(dir)/gmime-extra.c $(dir)/crypto.c \
-		$(dir)/repair.c \
+		$(dir)/repair.c $(dir)/path-util.c \
 		$(dir)/unicode-util.c
 
 libnotmuch_util_modules := $(libnotmuch_util_c_srcs:.c=.o)
diff --git a/compat/canonicalize_file_name.c b/util/path-util.c
similarity index 53%
rename from compat/canonicalize_file_name.c
rename to util/path-util.c
index 000f9e78..3267a967 100644
--- a/compat/canonicalize_file_name.c
+++ b/util/path-util.c
@@ -1,12 +1,21 @@
-#include "compat.h"
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+#define _GNU_SOURCE
+
+#include "path-util.h"
+
 #include <limits.h>
-#undef _GNU_SOURCE
 #include <stdlib.h>
 
+
 char *
-canonicalize_file_name (const char *path)
+notmuch_canonicalize_file_name (const char *path)
 {
-#ifdef PATH_MAX
+#if HAVE_CANONICALIZE_FILE_NAME
+    return canonicalize_file_name (path);
+#elif defined(PATH_MAX)
     char *resolved_path =  malloc (PATH_MAX + 1);
     if (resolved_path == NULL)
 	return NULL;
diff --git a/util/path-util.h b/util/path-util.h
new file mode 100644
index 00000000..ac85f696
--- /dev/null
+++ b/util/path-util.h
@@ -0,0 +1,19 @@
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+#ifndef NOTMUCH_UTIL_PATH_UTIL_H_
+#define NOTMUCH_UTIL_PATH_UTIL_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+char *
+notmuch_canonicalize_file_name (const char *path);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* NOTMUCH_UTIL_PATH_UTIL_H_ */
-- 
2.31.1.500.gbc6bbdd36b\r

[PATCH v4] compat: rename {,notmuch_}canonicalize_file_name

[Đoàn Trần Công Danh]

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 move our implementation into a util library.

Helped-by: Tomi Ollila <tomi.ollila@iki.fi>
Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---

Please disregard all v3 PATCH, one of them is correct,
I messed up my tooling to send different v3.

Sorry for the noise.

Interdiff against v3:
  diff --git a/util/path-util.c b/util/path-util.c
  index 4b352baf..3267a967 100644
  --- a/util/path-util.c
  +++ b/util/path-util.c
  @@ -13,7 +13,7 @@
   char *
   notmuch_canonicalize_file_name (const char *path)
   {
  -#ifdef HAVE_CANONICALIZE_FILE_NAME
  +#if HAVE_CANONICALIZE_FILE_NAME
       return canonicalize_file_name (path);
   #elif defined(PATH_MAX)
       char *resolved_path =  malloc (PATH_MAX + 1);

 compat/Makefile.local                         |  4 ----
 compat/compat.h                               |  8 --------
 lib/open.cc                                   |  5 +++--
 notmuch-config.c                              |  3 ++-
 util/Makefile.local                           |  2 +-
 .../path-util.c                               | 17 +++++++++++++----
 util/path-util.h                              | 19 +++++++++++++++++++
 7 files changed, 38 insertions(+), 20 deletions(-)
 rename compat/canonicalize_file_name.c => util/path-util.c (53%)
 create mode 100644 util/path-util.h

diff --git a/compat/Makefile.local b/compat/Makefile.local
index 2ee1b399..c58ca746 100644
--- a/compat/Makefile.local
+++ b/compat/Makefile.local
@@ -5,10 +5,6 @@ extra_cflags += -I$(srcdir)/$(dir)
 
 notmuch_compat_srcs :=
 
-ifneq ($(HAVE_CANONICALIZE_FILE_NAME),1)
-notmuch_compat_srcs += $(dir)/canonicalize_file_name.c
-endif
-
 ifneq ($(HAVE_GETLINE),1)
 notmuch_compat_srcs += $(dir)/getline.c $(dir)/getdelim.c
 endif
diff --git a/compat/compat.h b/compat/compat.h
index 8f15e585..59e91618 100644
--- a/compat/compat.h
+++ b/compat/compat.h
@@ -37,14 +37,6 @@ extern "C" {
 #define _POSIX_PTHREAD_SEMANTICS 1
 #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
-#endif
-
 #if ! HAVE_GETLINE
 #include <stdio.h>
 #include <unistd.h>
diff --git a/lib/open.cc b/lib/open.cc
index 5d80a884..bdb695fe 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -3,6 +3,7 @@
 
 #include "database-private.h"
 #include "parse-time-vrp.h"
+#include "path-util.h"
 
 #if HAVE_XAPIAN_DB_RETRY_LOCK
 #define DB_ACTION (Xapian::DB_CREATE_OR_OPEN | Xapian::DB_RETRY_LOCK)
@@ -612,9 +613,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..d9390c4d 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -24,6 +24,7 @@
 #include <netdb.h>
 #include <assert.h>
 
+#include "path-util.h"
 #include "unicode-util.h"
 
 static const char toplevel_config_comment[] =
@@ -327,7 +328,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);
diff --git a/util/Makefile.local b/util/Makefile.local
index 7ef029a5..8a0b9bc3 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -6,7 +6,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 libnotmuch_util_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
 		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c \
 		$(dir)/util.c $(dir)/gmime-extra.c $(dir)/crypto.c \
-		$(dir)/repair.c \
+		$(dir)/repair.c $(dir)/path-util.c \
 		$(dir)/unicode-util.c
 
 libnotmuch_util_modules := $(libnotmuch_util_c_srcs:.c=.o)
diff --git a/compat/canonicalize_file_name.c b/util/path-util.c
similarity index 53%
rename from compat/canonicalize_file_name.c
rename to util/path-util.c
index 000f9e78..3267a967 100644
--- a/compat/canonicalize_file_name.c
+++ b/util/path-util.c
@@ -1,12 +1,21 @@
-#include "compat.h"
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+#define _GNU_SOURCE
+
+#include "path-util.h"
+
 #include <limits.h>
-#undef _GNU_SOURCE
 #include <stdlib.h>
 
+
 char *
-canonicalize_file_name (const char *path)
+notmuch_canonicalize_file_name (const char *path)
 {
-#ifdef PATH_MAX
+#if HAVE_CANONICALIZE_FILE_NAME
+    return canonicalize_file_name (path);
+#elif defined(PATH_MAX)
     char *resolved_path =  malloc (PATH_MAX + 1);
     if (resolved_path == NULL)
 	return NULL;
diff --git a/util/path-util.h b/util/path-util.h
new file mode 100644
index 00000000..ac85f696
--- /dev/null
+++ b/util/path-util.h
@@ -0,0 +1,19 @@
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+#ifndef NOTMUCH_UTIL_PATH_UTIL_H_
+#define NOTMUCH_UTIL_PATH_UTIL_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+char *
+notmuch_canonicalize_file_name (const char *path);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* NOTMUCH_UTIL_PATH_UTIL_H_ */
-- 
2.31.1.500.gbc6bbdd36b\r

Re: [PATCH v4] compat: rename {,notmuch_}canonicalize_file_name

[David Bremner]

Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:

>
> Let's move our implementation into a util library.
>
> Helped-by: Tomi Ollila <tomi.ollila@iki.fi>
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>

I have applied this version to master. It looks a bit odd to have
SPDX-License-Identifiers in just these two files, but I guess it is
harmless. I left them for now in case for some reason we want to add
such identifiers to all source files.

d