Use mini_portile2 to build libmagic

[stanhu]

This pull request introduces significant changes to the build process:

• Use mini_portile2 gem to build the C extension.
• Clean up unneeded files after build.
• Switch the download archive to pull from the S3 endpoint.
• By default, build a static library of libmagic and link it.
• Support disabling of static library generation via the --disable-static argument.
• Support for using the system library via the --use-system-libraries argument.
• Update Travis CI to use Ubuntu 18.04 (bionic) instead of 16.04 (xenial).

This pull request lifts much of the extconf.rb code from Nokogiri.

[kwilczynski]

Hi @stanhu, thank you for this! This is amazing!

Before you move deeper into the proverbial "rabbit hole" (but I hope it won't be that bad, though) of trying to get this to work, I need to ask - are you sure you want to spent time on this?

An odd question to ask, but... I believe you were working on solving GitLab incident, and it also seems that the Ruby on Rails team is about to select the mini_mime Ruby Gem as their preferred solution (see rails/marcel@5040f48).

To put it differently - are you OK with helping me? I would really hate to waste your time, especially after when Ruby on Rails project fixes their problem, but in the same time I appreciate your help!

Krzysztof

[stanhu]

To put it differently - are you OK with helping me? I would really hate to waste your time, especially after when Ruby on Rails project fixes their problem, but in the same time I appreciate your help!

@kwilczynski Of course. This gem has value, even if Rails decides to use mini_mime. It looks like the transition mini_mime is not quite done yet, and the last release was in 2019. In the meantime, I'll do what I can to push this forward.

[stanhu]

I'm going to abandon this pull request because I'm having a hard time making mini_portile link against the compiled .so instead of the libmagic.

I figured out that statically compiling the binary is possible, but you need to use pkg-config --libs --static libmagic to get the linker flags needed for your system. For macOS, this adds -lmagic -llzma -lbz2 -lz, while on Linux it's only -l magic -lz. I couldn't find an easy way to tell mini_portile where to look for the .pc file other than to set PKG_CONFIG_PATH.

[kwilczynski]

Hi @stanhu, thank you for taking a look!

I wonder whether mini_portile is doing something awkward behind the scenes, as the "vanilla", so to speak, approach of downloading the libmagic and building a shared object, setting the environment, building ruby-magic ensuring that everything is properly linked, would be (one would assume) almost the same as the steps you captured in the extconf.rb at the moment.

Krzysztof

[stanhu]

Ok, finally got this to work. To do:

1. Document and test --disable-static flag
2. Consider adding --enable-system-libraries flag.
3. Add clean step

[kwilczynski]

Hi @stanhu, thank you for all the work here thus far! This is amazing!

We have some failures when using system libraries as the libmagic version is quite old (version 5.25), and it does not support the MAGIC_PARAM_BYTES_MAX that was added in version 5.27.

I used to support older versions of libmagic. I will bring the code I removed back.

Sorry about this!

Krzysztof

[stanhu]

@kwilczynski No problem! We could also just use your existing script and test this that way. I don't have a strong opinion whether backwards compatibility needs to be preserved, especially since I think we might want to vendor the libmagic source directly in this repo.

[stanhu]

@kwilczynski It looks like https://github.com/file/file/blob/7dea9d7eede587e2b37571d9388a81d49020cd86/src/apprentice.c#L1379 is responsible for setting the flags to 64. I think we can skip this test for the flags because it seems to be an internal state: https://github.com/file/file/blob/7dea9d7eede587e2b37571d9388a81d49020cd86/src/magic.h.in#L39.

When only a single file is provided to magic_load(), apprentice_map() in https://github.com/file/file/blob/7dea9d7eede587e2b37571d9388a81d49020cd86/src/apprentice.c#L461 appears to return a non-null value. When a search path is provided (e.g. /etc/magic:/usr/share/misc/magic, which can be provided via the MAGIC env variable), then this apprentice_load() gets called: (https://github.com/file/file/blob/7dea9d7eede587e2b37571d9388a81d49020cd86/src/apprentice.c#L465).

Alternatively, we could workaround the issue by running the tests within a known environment:

MAGIC=/usr/share/misc/magic.mgc bundle exec rake test

[stanhu]

@kwilczynski I updated the test, but I'm not sure why Travis CI stopped working. Does something need to be done to migrate to travis-ci.com?

[kwilczynski]

Hi @stanhu,

@kwilczynski I updated the test, but I'm not sure why Travis CI stopped working.

Looks like it's still running things on push to this branch, but why it's not showing here on GitHub anymore I am not sure.

Does something need to be done to migrate to travis-ci.com?

I honestly don't know. I haven't looked into it, as I considered Travis CI somewhat broken after the change of management. Having said that, it kept (the free version) working fine even with some occasional timeouts and issues here and there.

I also wonder, what would be the real difference between free plan on travis-ci.com and the one here on travis-ci.org - if the experience is the same, then I would be disappointed. Otherwise, I can't afford $70 per month to use the commercial plan :-(

But, let me have a look into switching over.

Krzysztof

[kwilczynski]

Hi @stanhu, sorry for late reply!

Thank you for all the detective work here! I haven't looked at the libmagic source code at the time when I did my initial investigation. It got late for me and I called it a day. Apologies!

@kwilczynski It looks like file/file@7dea9d7/src/apprentice.c#L1379 is responsible for setting the flags to 64.

Good to know!

I would consider this potentially an issue to handle on your side (or generally a bug, if this is not an intended behaviour). Why? Primarily because an internal state - as you also pointed out - is leaking to the user of libmagic in a case of an error. This is a problem since libmagic does not use a copy of current flags that the magic_t object holds while doing some of its internal operations, but rather muate the current value adding 0x40 to it - which in a case of an error is never cleared. This would be a bit of a surprise to the user since the bitmask holding flags would have been updated outside of user's knowledge.

I think we can skip this test for the flags because it seems to be an internal state: file/file@7dea9d7/src/magic.h.in#L39.

This particular flag is also a part of what I consider to be a public interface of libmagic since these are available in magic.h header file. Thus, if possible, I would like to support it - even though, there is magic_check(), setting MAGIC_CHECK could potentially reveal issues during the time of loading and parsing of the Magic library. Having said that, most users will probably never use this flag, unless they are developing their own database or want to troubleshooting something.

When only a single file is provided to magic_load(), apprentice_map() in file/file@7dea9d7/src/apprentice.c#L461 appears to return a non-null value. When a search path is provided (e.g. /etc/magic:/usr/share/misc/magic, which can be provided via the MAGIC env variable), then this apprentice_load() gets called: (file/file@7dea9d7/src/apprentice.c#L465).

Good find!

I need have a look into this a bit closer. A potential test cases would be:

• magic_load() using NULL to force it to load Magic database from a default location
• A single valid database passed to magic_load() directly and via the MAGIC environment variable
• An invalid database provided (the exact opposite of the above case)
• Multiple valid database files passed to magic_load()
• A set of magic databases was provided - one valid and one invalid

I think, this unexpected (let's call it like that) behaviour we've seen when using libmagic provided by the distribution would be due to the "/etc/magic" file (usually an empty or with some comments) being included in the search path for libmagic to use. Since it's empty, it probably fails to parse and sets an error (to be verified), even though the second location provided contains a path to a valid compiled Magic database. I suppose on the "happy path" through the code (when the second valid database has been loaded successfully) a previous/existing error isn't cleared, etc.

And, of course, I found a patch from Debian that changes loading of Magic database files, as per:

local.support-local-definitions-in-etc-magic.patch

I wonder if this is what is causing this interesting behaviour. Need to check.

Alternatively, we could workaround the issue by running the tests within a known environment:

MAGIC=/usr/share/misc/magic.mgc bundle exec rake test

Sure. To unblock us I see why not (I saw you already did that, thank you!).

Krzysztof

[kwilczynski]

Hi @stanhu, all of this looks amazing!

I did some testing.

Building with --use-system-libraries works fine on both macOS and Linux. I then run into problems when building vendored libmagic - not unexpected.

For example, on macOS libmagic failed to build as per:

   kwilczynski@rocinante  Ruby/ruby-magic   pr/5 ✔  $ ruby ext/magic/extconf.rb
Building ruby-magic using packaged libraries.
Static linking is enabled.
Cross build is disabled.
Using mini_portile version 2.5.0
Downloading file-5.39.tar.gz (100%)
Extracting file-5.39.tar.gz into tmp/x86_64-apple-darwin19.6.0/ports/libmagic/5.39... OK
Running 'configure' for libmagic 5.39... OK
Running 'compile' for libmagic 5.39... ERROR, review '/Users/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/tmp/x86_64-apple-darwin19.6.0/ports/libmagic/5.39/compile.log' to see what happened. Last lines are:
========================================================================
cd . && /bin/sh ./config.status config.h
config.status: creating config.h
config.status: config.h is unchanged
/Library/Developer/CommandLineTools/usr/bin/make  all-recursive
Making all in src
sed -e "s/X.YY/$(echo 5.39 | tr -d .)/" < ../src/magic.h.in > magic.h
/Library/Developer/CommandLineTools/usr/bin/make  all-am
gcc -DHAVE_CONFIG_H -I. -I..  -DMAGIC='"/Users/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/ports/x86_64-apple-darwin19.6.0/libmagic/5.39/share/misc/magic"'  -fvisibility=hidden -Wall -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith        -Wmissing-declarations -Wredundant-decls -Wnested-externs        -Wsign-compare -Wreturn-type -Wswitch -Wshadow        -Wcast-qual -Wwrite-strings -Wextra -Wunused-parameter -Wformat=2 -fPIC -c -o file.o file.c
In file included from file.c:32:
./file.h:37:10: fatal error: 'config.h' file not found
#include <config.h>
         ^~~~~~~~~~
1 error generated.
make[3]: *** [file.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
========================================================================

Does it build fine for you? I wonder if this is my runtime environment that is causing some issues here. I am going to build vanilla libmagic to see what is going on.

On Linux, default build (linking libmagic statically) is working fine, as per:

   kwilczynski@workstation  Ruby/ruby-magic   pr/5 ✘  $ file ./magic.so
./magic.so: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), statically linked, BuildID[sha1]=42f37d7651c2a48f3c4aee23a8ead25dd7459e89, with debug_info, not stripped
   kwilczynski@workstation  Ruby/ruby-magic   pr/5 ✘  $ patchelf --print-rpath magic.so
/home/kwilczynski/.rbenv/versions/2.7.2/lib:/home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/ports/x86_64-pc-linux-gnu/libmagic/5.39/lib
   kwilczynski@workstation  Ruby/ruby-magic   pr/5 ✘  $ patchelf --print-needed magic.so
libruby.so.2.7
libc.so.6
libz.so.1

But it seems that mkmf is having some troubles while checking for symbols when building with --disable-static, as per:

   kwilczynski@workstation  Ruby/ruby-magic   pr/5 ✘  $ ruby ext/magic/extconf.rb --disable-static
Building ruby-magic using packaged libraries.
Static linking is disabled.
Cross build is disabled.
Using mini_portile version 2.5.0
Extracting file-5.39.tar.gz into tmp/x86_64-pc-linux-gnu/ports/libmagic/5.39... OK
Running 'configure' for libmagic 5.39... OK
Running 'compile' for libmagic 5.39... OK
Running 'install' for libmagic 5.39... OK
Activating libmagic 5.39 (from /home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/ports/x86_64-pc-linux-gnu/libmagic/5.39)...
checking for ruby.h... yes
checking for rb_thread_call_without_gvl()... yes
checking for rb_thread_blocking_region()... no
checking for magic.h... yes
checking for -lmagic... yes
checking for magic_getpath()... no

 Your version of libmagic(3) appears to be too old.

 Please, consider upgrading to at least version 5.29 or newer,
 if possible. For more information about file(1) command and
 libmagic(3) please visit the following web site:

   https://www.darwinsys.com/file

Interesting. I wonder what is mkmf doing here not to be able to detect his function.

Krzysztof

[kwilczynski]

I am going to build vanilla libmagic to see what is going on.

 ✘    kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ ./configure --disable-silent-rules --disable-dependency-tracking --enable-fsect-man5 1>/dev/null
   kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ make
cd . && /bin/sh ./config.status config.h
config.status: creating config.h
config.status: config.h is unchanged
/Library/Developer/CommandLineTools/usr/bin/make  all-recursive
Making all in src
/Library/Developer/CommandLineTools/usr/bin/make  all-am
gcc -DHAVE_CONFIG_H -I. -I..  -DMAGIC='"/usr/local/share/misc/magic"'  -fvisibility=hidden -Wall -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith        -Wmissing-declarations -Wredundant-decls -Wnested-externs        -Wsign-compare -Wreturn-type -Wswitch -Wshadow        -Wcast-qual -Wwrite-strings -Wextra -Wunused-parameter -Wformat=2 -g -O2 -c -o file.o file.c
In file included from file.c:32:
./file.h:37:10: fatal error: 'config.h' file not found
#include <config.h>
         ^~~~~~~~~~
1 error generated.
make[3]: *** [file.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2

That is a bit odd.

 ✘    kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ ./config.status config.h
config.status: creating config.h
config.status: config.h is unchanged
   kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ ls -l config*
-rwxr-xr-x  1 kwilczynski  staff   44977 Jun 15  2020 config.guess
-rw-r--r--  1 kwilczynski  staff   11526 Jun 15  2020 config.h.in
-rw-r--r--  1 kwilczynski  staff   90412 Mar 29 14:05 config.log
-rwxr-xr-x  1 kwilczynski  staff   61723 Mar 29 14:05 config.status
-rwxr-xr-x  1 kwilczynski  staff   31560 Jun 15  2020 config.sub
-rwxr-xr-x  1 kwilczynski  staff  510342 Jun 15  2020 configure
-rw-r--r--  1 kwilczynski  staff    6623 Jun 15  2020 configure.ac
   kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ ./config.status
config.status: creating Makefile
config.status: creating src/Makefile
config.status: creating magic/Makefile
config.status: creating tests/Makefile
config.status: creating doc/Makefile
config.status: creating python/Makefile
config.status: creating libmagic.pc
config.status: creating config.h
config.status: config.h is unchanged
config.status: executing depfiles commands
config.status: executing libtool commands
   kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ ls -l config.h
ls: config.h: No such file or directory

Seems like autoconf script is not copying config.h for some reason.

   kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ ./config.status config.h --debug
config.status: creating config.h
config.status: config.h is unchanged
   kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ ls -l confAXId7R/config.h
-rw-r--r--  1 kwilczynski  staff  12128 Mar 29 14:21 confAXId7R/config.h
   kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ ln -sf confAXId7R/config.h .
   kwilczynski@rocinante  archives/file-5.39   pr/5 ✘  $ make
/Library/Developer/CommandLineTools/usr/bin/make  all-recursive
Making all in src
sed -e "s/X.YY/$(echo 5.39 | tr -d .)/" < ../src/magic.h.in > magic.h
/Library/Developer/CommandLineTools/usr/bin/make  all-am
(...)
Making all in magic
../src/file -C -m magic
Making all in tests
make[2]: Nothing to be done for `all'.

Interesting.

Krzysztof

[stanhu]

@kwilczynski I've found that I often have to clean up the ports/, tmp/x86_64-apple-darwin19.6.0/ports, and .o files when switching between different modes. We might need to investigate that or just run the clean step for even development environments.

[kwilczynski]

Hi @stanhu,

@kwilczynski I've found that I often have to clean up the ports/, tmp/x86_64-apple-darwin19.6.0/ports, and .o files when switching between different modes. We might need to investigate that or just run the clean step for even development environments.

Ah, good to know! I've been running rake clobber just in case. This reminds me, we probably need to update .gitignore at some point too. :)

Krzysztof

[stanhu]

I added some files to the rake clobber list. I tried building with macOS with --use-system-libraries and without it, interspersed with rake clobber, and it worked for me. I'm curious if it solves your problem.

[kwilczynski]

Hi @stanhu,

@kwilczynski I updated the test, but I'm not sure why Travis CI stopped working.

Looks like it's still running things on push to this branch, but why it's not showing here on GitHub anymore I am not sure.

Does something need to be done to migrate to travis-ci.com?

I honestly don't know. I haven't looked into it, as I considered Travis CI somewhat broken after the change of management. Having said that, it kept (the free version) working fine even with some occasional timeouts and issues here and there.

I also wonder, what would be the real difference between free plan on travis-ci.com and the one here on travis-ci.org - if the experience is the same, then I would be disappointed. Otherwise, I can't afford $70 per month to use the commercial plan :-(

But, let me have a look into switching over.

We are now on travis-ci.com!

Initially, the migration from travis-ci.org to travis-ci.com didn't work. It took awhile for things to synchronise or whatever they've been doing behind the scenes.

Some things seem broken e.g., it keep asking me for authentication against GitHub every time, even though it first time everything worked. Perhaps an issue with their integration or some caching issue, not sure.

Sadly, for free plan the number of credits is quite low. I've done a build of the project, and we are down 200 credits from available 10000 already. It seems, this will run out very quickly.

Krzysztof

[stanhu]

@kwilczynski Ok, thanks for migrating. Anything else in this pull request you would like to review?

[kwilczynski]

Hi @stanhu, looks amazing!

Thank you for working on this! Let's ship!

[kwilczynski]

Hi @stanhu,

@kwilczynski Ok, thanks for migrating.

No problem! Sorry for the delay.

Anything else in this pull request you would like to review?

Not that I can think of. I tested all the changes, and it works flawlessly on Linux. I still can't build it on macOS, but I believe that something is up with my runtime environment, so nothing that would stop this from being merged.

Having said that - once you merge, don't release new version just yet, as I have some local changes pending. :)

Krzysztof

[stanhu]

Ok, thanks!

[kwilczynski]

Hi @stanhu,

[...]

But it seems that mkmf is having some troubles while checking for symbols when building with --disable-static, as per:

   kwilczynski@workstation  Ruby/ruby-magic   pr/5 ✘  $ ruby ext/magic/extconf.rb --disable-static
Building ruby-magic using packaged libraries.
Static linking is disabled.
Cross build is disabled.
Using mini_portile version 2.5.0
Extracting file-5.39.tar.gz into tmp/x86_64-pc-linux-gnu/ports/libmagic/5.39... OK
Running 'configure' for libmagic 5.39... OK
Running 'compile' for libmagic 5.39... OK
Running 'install' for libmagic 5.39... OK
Activating libmagic 5.39 (from /home/kwilczynski/Development/Projects/Personal/Ruby/ruby-magic/ports/x86_64-pc-linux-gnu/libmagic/5.39)...
checking for ruby.h... yes
checking for rb_thread_call_without_gvl()... yes
checking for rb_thread_blocking_region()... no
checking for magic.h... yes
checking for -lmagic... yes
checking for magic_getpath()... no

 Your version of libmagic(3) appears to be too old.

 Please, consider upgrading to at least version 5.29 or newer,
 if possible. For more information about file(1) command and
 libmagic(3) please visit the following web site:

   https://www.darwinsys.com/file

I am happy to report that this is no longer an issue with the recent changes! Thank you!

Krzysztof

[kwilczynski]

Hi @stanhu,

Ok, thanks!

No. Thank YOU!

Krzysztof