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