Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Linuxisms in fsearch 0.1 #297

Closed
Schamschula opened this issue Sep 28, 2021 · 21 comments
Closed

[BUG] Linuxisms in fsearch 0.1 #297

Schamschula opened this issue Sep 28, 2021 · 21 comments
Labels

Comments

@Schamschula
Copy link

Describe the bug
When attempting to build fsearch 0.1 under MacPorts I get several errors in fsearch_database.c.

  1. There is no such thing as #include <malloc.h> under macOS, I use #include <stdlib.h> instead.
  2. There is no such thing as 'strverscmp' on *NIX platforms other than linux.
  3. There is no such thing as 'AT_NO_AUTOMOUNT' on *NIX platforms other than linux.

To Reproduce
Steps to reproduce the behavior:
When building

Expected behavior
Successful build (as for previous tag 0.1beta4)

Screenshots

  1. w/o malloc patch
    fsearch_database.c:28:10: fatal error: 'malloc.h' file not found
    #include <malloc.h>
    ^~~~~~~~~~
    CC fsearch-fsearch_database_view.o

  2. with malloc and AT_NO_AUTOMOUNT patch

--- src/fsearch_database.c.orig	2021-09-24 11:41:49.000000000 -0500
+++ src/fsearch_database.c	2021-09-28 07:30:47.000000000 -0500
@@ -25,7 +25,7 @@
 #include <fcntl.h>
 #include <fnmatch.h>
 #include <glib/gi18n.h>
-#include <malloc.h>
+#include <stdlib.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <string.h>
@@ -1262,7 +1262,7 @@
         g_string_append(path, dent->d_name);
 
         struct stat st;
-        if (fstatat(dir_fd, dent->d_name, &st, AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT)) {
+        if (fstatat(dir_fd, dent->d_name, &st, AT_SYMLINK_NOFOLLOW)) {
             g_debug("[db_scan] can't stat: %s", path->str);
             continue;
         }

Undefined symbols for architecture x86_64:
"_malloc_trim", referenced from:
_db_free in fsearch-fsearch_database.o
"_strverscmp", referenced from:
_db_entry_compare_entries_by_path in fsearch-fsearch_database_entry.o
_sort_entry_by_path_recursive in fsearch-fsearch_database_entry.o
_db_entry_compare_entries_by_name in fsearch-fsearch_database_entry.o
_db_entry_compare_entries_by_extension in fsearch-fsearch_database_entry.o

Desktop (please complete the following information):

Additional context
Notes:

  1. _malloc_trim is not portable, see http://www.hep.by/gnu/gnulib/malloc_005ftrim.html
  2. strverscmp is not portable, see https://www.gnu.org/software/gnulib/manual/html_node/strverscmp.html

PS: There is an issue with configure.ac

AX_CHECK_COMPILE_FLAG([-std=c11], [CFLAGS+=" -std=c11" ],
                      [echo "C compiler cannot compile C11 code. Abort"
                       exit -1
                      ])

even though I have configured the Portfile to use compiler.cxx_standard 2011

Schamschula added a commit to macports/macports-ports that referenced this issue Sep 28, 2021
@cboxdoerfer
Copy link
Owner

cboxdoerfer commented Sep 28, 2021

Thanks for letting me know! I'll fix those, but I'm going to add some compile time checks to detect the current platform. Because I want to keep those features on Linux builds.

The fact that strverscmp is missing, is a little more tricky to solve than the others. Because technically changing this to a different sort implementation would break the current database format (files are supposed to be in a certain order). So I'll probably need to add my own sort implementation for that to work consistently across different platforms. I'll add that to the todo-list for the 0.2 release.

However in the meantime you're of course free to just replace all usages of strverscmp with strcmp in your local build.

There is an issue with configure.ac

Can you post the error message when you invoke ./autogen.sh && ./configure?

@Schamschula
Copy link
Author

Actually, I did deal with the strverscmp issue in the beta version. I'll fix that on my end.

Here is the output for ./autogen.sh

autopoint: using AM_GNU_GETTEXT_REQUIRE_VERSION instead of AM_GNU_GETTEXT_VERSION
Copying file ABOUT-NLS
Copying file config.rpath
Creating directory m4
Copying file m4/codeset.m4
Copying file m4/extern-inline.m4
Copying file m4/fcntl-o.m4
Copying file m4/gettext.m4
Copying file m4/glibc2.m4
Copying file m4/glibc21.m4
Copying file m4/iconv.m4
Copying file m4/intdiv0.m4
Copying file m4/intl.m4
Copying file m4/intldir.m4
Copying file m4/intlmacosx.m4
Copying file m4/intmax.m4
Copying file m4/inttypes-pri.m4
Copying file m4/inttypes_h.m4
Copying file m4/lcmessage.m4
Copying file m4/lib-ld.m4
Copying file m4/lib-link.m4
Copying file m4/lib-prefix.m4
Copying file m4/lock.m4
Copying file m4/longlong.m4
Copying file m4/nls.m4
Copying file m4/po.m4
Copying file m4/printf-posix.m4
Copying file m4/progtest.m4
Copying file m4/size_max.m4
Copying file m4/stdint_h.m4
Copying file m4/threadlib.m4
Copying file m4/uintmax_t.m4
Copying file m4/visibility.m4
Copying file m4/wchar_t.m4
Copying file m4/wint_t.m4
Copying file m4/xsize.m4
Copying file po/Makefile.in.in
Copying file po/Makevars.template
Copying file po/Rules-quot
Copying file po/boldquot.sed
Copying file po/en@boldquot.header
Copying file po/en@quot.header
Copying file po/insert-header.sin
Copying file po/quot.sed
Copying file po/remove-potcdate.sin
configure.ac:16: installing './compile'
configure.ac:16: installing './config.guess'
configure.ac:16: installing './config.sub'
configure.ac:6: installing './install-sh'
configure.ac:6: installing './missing'
src/Makefile.am:94: warning: shell $(GLIB_COMPILE_RESOURCES: non-POSIX variable name
src/Makefile.am:94: (probably a GNU make extension)
src/Makefile.am: installing './depcomp'

and ./configure

Executing:  cd "/opt/local/var/macports/build/_Users_marius_Development_MacPorts_ports_sysutils_fsearch/fsearch/work/fsearch-0.1" && ./configure --prefix=/opt/local 
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a race-free mkdir -p... ./install-sh -c -d
checking for gawk... no
checking for mawk... no
checking for nawk... no
checking for awk... awk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking for a sed that does not truncate output... /usr/bin/sed
checking whether NLS is requested... yes
checking for msgfmt... /opt/local/bin/msgfmt
checking for gmsgfmt... /opt/local/bin/msgfmt
checking for xgettext... /opt/local/bin/xgettext
checking for msgmerge... /opt/local/bin/msgmerge
checking whether make supports the include directive... yes (GNU style)
checking for gcc... /usr/bin/clang
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether the compiler supports GNU C... yes
checking whether /usr/bin/clang accepts -g... yes
checking for /usr/bin/clang option to enable C11 features... none needed
checking whether /usr/bin/clang understands -c and -o together... yes
checking dependency style of /usr/bin/clang... gcc3
checking build system type... x86_64-apple-darwin18.7.0
checking host system type... x86_64-apple-darwin18.7.0
checking for ld used by /usr/bin/clang... /Library/Developer/CommandLineTools/usr/bin/ld
checking if the linker (/Library/Developer/CommandLineTools/usr/bin/ld) is GNU ld... no
checking for shared library run path origin... done
checking how to run the C preprocessor... /usr/bin/clang -E
checking for grep that handles long lines and -e... /usr/bin/grep
checking for egrep... /usr/bin/grep -E
checking for CFPreferencesCopyAppValue... yes
checking for CFLocaleCopyCurrent... yes
checking for GNU gettext in libc... no
checking for iconv... yes
checking for working iconv... yes
checking how to link with libiconv... -liconv
checking for GNU gettext in libintl... yes
checking whether to use NLS... yes
checking where the gettext function comes from... external libintl
checking how to link with libintl... -lintl -Wl,-framework -Wl,CoreFoundation
checking for gcc... (cached) /usr/bin/clang
checking whether the compiler supports GNU C... (cached) yes
checking whether /usr/bin/clang accepts -g... (cached) yes
checking for /usr/bin/clang option to enable C11 features... (cached) none needed
checking whether /usr/bin/clang understands -c and -o together... (cached) yes
checking dependency style of /usr/bin/clang... (cached) gcc3
./configure: line 8225: syntax error near unexpected token `-std=c11,'
./configure: line 8225: `AX_CHECK_COMPILE_FLAG(-std=c11, CFLAGS+=" -std=c11" ,'
Command failed:  cd "/opt/local/var/macports/build/_Users_marius_Development_MacPorts_ports_sysutils_fsearch/fsearch/work/fsearch-0.1" && ./configure --prefix=/opt/local 
Exit code: 2

@Schamschula
Copy link
Author

Updating the patch from 0.1beta4 takes care of the strverscmp issue.

@cboxdoerfer
Copy link
Owner

Updating the patch from 0.1beta4 takes care of the strverscmp issue.

That's great!

Regarding the configure.ac issue, that's because the autoconf-archive build dependency is missing. Is this available on MacPorts? Or is the meson build system supported? Because then you could try this instead.

@Schamschula
Copy link
Author

Yes, the meson build system is supported!

I added the meson PortGroup. However, now I need to patch src/meson.build. Done.

This leaves the malloc issue:

FAILED: src/fsearch 
/usr/bin/clang  -o src/fsearch src/fsearch.p/meson-generated_.._resources.c.o src/fsearch.p/main.c.o -L/opt/local/lib -I/opt/local/include -Wl,-dead_strip_dylibs -Wl,-headerpad_max_install_names -Wl,-undefined,error -Wl,-headerpad_max_install_names -Wl,-syslibroot,/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk -arch x86_64 -pipe -Os -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk -arch x86_64 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk -Wl,-rpath,/opt/local/lib src/libfsearch.a -lm /opt/local/lib/libgio-2.0.dylib /opt/local/lib/libgobject-2.0.dylib /opt/local/lib/libglib-2.0.dylib /opt/local/lib/libintl.dylib /opt/local/lib/libgtk-3.dylib /opt/local/lib/libgdk-3.dylib /opt/local/lib/libpangocairo-1.0.dylib /opt/local/lib/libpango-1.0.dylib /opt/local/lib/libatk-1.0.dylib /opt/local/lib/libcairo-gobject.dylib /opt/local/lib/libcairo.dylib /opt/local/lib/libgdk_pixbuf-2.0.dylib /opt/local/lib/libpcre.dylib /opt/local/lib/libicuuc.dylib /opt/local/lib/libicudata.dylib
Undefined symbols for architecture x86_64:
  "_malloc_trim", referenced from:
      _db_free in libfsearch.a(fsearch_database.c.o)
ld: symbol(s) not found for architecture x86_64

@cboxdoerfer
Copy link
Owner

cboxdoerfer commented Sep 29, 2021

@Schamschula, both the malloc_trim and AT_NO_AUTOMOUNT issues should now be fixed on master.

I'm going to release 0.1.1 in the next days with those fixes and a couple others as well.

@danfe
Copy link

danfe commented Sep 30, 2021

  1. There is no such thing as strverscmp on *NIX platforms other than linux.

It's a glibc thing, actually. I've fixed this back in 2017 when I added fsearch to FreeBSD Ports Collection and filed #63 but it didn't get traction, unfortunately. The patches are still there.

@danfe
Copy link

danfe commented Sep 30, 2021

The fact that strverscmp is missing, is a little more tricky to solve than the others. Because technically changing this to a different sort implementation would break the current database format (files are supposed to be in a certain order).

Not necessarily. I haven't tried to prove this formally, but my quick tests showed that

    tmp1 = g_utf8_collate_key_for_filename(s1, -1);
    tmp2 = g_utf8_collate_key_for_filename(s2, -1);
    ret = strcmp(tmp1, tmp2);
    g_free(tmp1);
    g_free(tmp2);
    return ret;

should DTRT.

@cboxdoerfer
Copy link
Owner

cboxdoerfer commented Sep 30, 2021

@danfe: g_utf8_collate_key_for_filename is not an option, because it's way too slow. For comparison, sorting 3 million files by name takes less than a second (~850ms) with strverscmp but more than a minute (~95s) with g_utf8_collate_key_for_filename. So it's roughly a hundred times slower.

The problem with using a different sort algorithm is this: The entries in the database have a certain order, so when we change the sort algorithm and FSearch reads the old database, which was sorted with the old algorithm, you get inconsistent behavior. For example binary searching won't work anymore, because the file arrays are not sorted by our new compare function. Or using the column header to sort the results will lead to inconsistent sort order. By default all results are sorted according to their order in the database file, but when a user clicks on a column, FSearch might need to resort at runtime.

@danfe
Copy link

danfe commented Sep 30, 2021

g_utf8_collate_key_for_filename is not an option, because it's way too slow.

Oh, I see, yeah, that's a bummer. :-(

The problem with using a different sort algorithm is this: The entries in the database have a certain order, so when we change the sort algorithm and FSearch reads the old database, which was sorted with the old algorithm, you get inconsistent behavior.

Right, this part was quite clear. Not sure what's the elegant solution to this, maybe implementing some sort of versioning in the database?

@cboxdoerfer
Copy link
Owner

Right, this part was quite clear. Not sure what's the elegant solution to this, maybe implementing some sort of versioning in the database?

The database already has some basic versioning. I just want to avoid bumping the database version too often. But yeah, the next version bump will probably happen with the 0.2 release anyway, because there are also some other changes planned for the database format and then I can safely change the sort algorithm.

@ryandesign
Copy link

the autoconf-archive build dependency is missing. Is this available on MacPorts?

Yes: https://ports.macports.org/port/autoconf-archive

@DUOLabs333
Copy link
Contributor

I compiled a local build, but when I start it, it isn't connecting to my XQuartz server.

@DUOLabs333
Copy link
Contributor

UPDATE: I got it to work, but the magnifying glass is weird, and it only works through SSH.

@cboxdoerfer
Copy link
Owner

What magnifying glass do you mean?

@DUOLabs333
Copy link
Contributor

DUOLabs333 commented Oct 4, 2021 via email

@DUOLabs333
Copy link
Contributor

DUOLabs333 commented Dec 22, 2021

For some reason, noq it fails with trace trap -- I may have uninstalled something important when cleaning up my directories though.

@danfe
Copy link

danfe commented Dec 22, 2021

I compiled a local build, but when I start it, it isn't connecting to my XQuartz server.

How this and the following discussion are related to the subject of the issue, if I may ask? :-/

@DUOLabs333
Copy link
Contributor

It may be that fsearch is expecting something from Linux, but can't find it on a MacOS system.

@danfe
Copy link

danfe commented Feb 16, 2022

... there are also some other changes planned for the database format and then I can safely change the sort algorithm.

Yeah, it would be nice not to use any GNU or other system-specific extensions. Meanwhile, I've replaced the slow g_utf8_collate_key_for_filename()-based code in the FreeBSD port with the actual strverscmp() implementation from gnulib, it's actually quite short.

@Schamschula
Copy link
Author

I finally got back to getting search to build under MacPorts

It required a couple more tweaks: macports/macports-ports@1d6be4c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants