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

GCC build errors reported on Windows #155

Closed
snej opened this issue Jan 19, 2021 · 17 comments
Closed

GCC build errors reported on Windows #155

snej opened this issue Jan 19, 2021 · 17 comments
Assignees
Labels
enhancement not-a-bug external issue and/or not a libmdbx's bug

Comments

@snej
Copy link
Contributor

snej commented Jan 19, 2021

I have a bug report against my Nimdbx wrapper, that mdbx.c (the amalgamation prebuilt by me with make dist) fails to compile with GCC on Windows. I do know Nimdbx builds on macOS and Ubuntu. The first two errors are:

mdbx.c:16:10: error: #include expects "FILENAME" or <FILENAME>
 #include MDBX_CONFIG_H
          ^~~~~~~~~~~~~
mdbx.c:1371:29: error: unknown type name 'FILE_INFO_BY_HANDLE_CLASS'
     _In_ HANDLE hFile, _In_ FILE_INFO_BY_HANDLE_CLASS FileInformationClass,
                             ^~~~~~~~~~~~~~~~~~~~~~~~~

I've never seen a preprocessor symbol used as the parameter of #include before; is it a GCC extension? But it seems to work for me with Clang, and on Github Actions CI with GCC on Ubuntu.

I grepped the libmdbx "dist" sources, and there is no definition of FILE_INFO_BY_HANDLE_CLASS anywhere. The code that uses it is conditionalized for Windows only. Is this a libmdbx bug? Or is that symbol supposed to come from a Windows platform header?

Unrelated, but I also noticed a missing line break on line 1 of mdbx.c:

#define MDBX_ALLOY 1n#define MDBX_BUILD_SOURCERY 9854b516cd42bd97a2447d2f4adfda8cd99464906b15a3646a1409c129fbb1bc_v0_9_2_21_g1c70814

It looks like the "n" before the second #define was supposed to be a "\\n"...

(This is using libmdbx commit 3a441d6 from 4 Jan.)

@erthink
Copy link
Owner

erthink commented Jan 20, 2021

Ok, I'll see what's going on.

@erthink erthink self-assigned this Jan 20, 2021
@erthink
Copy link
Owner

erthink commented Jan 20, 2021

Briefly:

  • Seems the problem (incompatibility) is in the windows version of the utilities (sed, bash, echo, etc) used for make dist and building libmdbx by make.
  • I am leaning towards the decision to close this issue as "wontfix", while retaining the recommendations to use CMake for building on Windows and create amalgamated source code (i.e. running make dist) only on modern Unix.

@erthink
Copy link
Owner

erthink commented Jan 20, 2021

More details:

  1. libmdbx supposes the two build methods: by CMake with provided CMakeLists.txt (a portable way), or by GNU Make with provided GNUmakefile (requires GNU Linux or modern Unix environment).
  2. GNUmakefile uses commands like
    $(CC) $(CFLAGS) $(MDBX_OPTIONS) '-DMDBX_CONFIG_H="config.h"' -DLIBMDBX_EXPORTS=1 -c mdbx.c -o $@ to build, which expended to cc -O2 -g -Wall -Werror -Wextra -Wpedantic -ffunction-sections -fPIC -fvisibility=hidden -std=gnu11 -pthread -Wno-error=attributes -DNDEBUG=1 '-DMDBX_CONFIG_H="config.h"' -ULIBMDBX_EXPORTS -c mdbx.c -o mdbx-static.o, etc.
    This is correct, but requires properly handling command line options in Unix style, and this work perfect when mdbx.c (and other files) has proper header like this:
#define MDBX_ALLOY 1
#define MDBX_BUILD_SOURCERY 0a3f81552735496c153bc690bc12de...
#ifdef MDBX_CONFIG_H
#include MDBX_CONFIG_H
#endif
  1. You noted that header is incorrect, and the #define MDBX_ALLOY 1n#define MDBX_BUILD_SOURCERY 9854b5... means that at least sed tool don't proceed as expected (i.e. failed to handle extended syntax properly).

So it is not a libmdbx's error, but sed and/or other tools (including wrong command line options processing by windows version of GNU make and bash).

Nothing can be fixed inside libmdbx to avoid this.
I can only recommend using CMake to build libmdbx on Windows (either don't use Windows at all).

@erthink
Copy link
Owner

erthink commented Jan 20, 2021

Additionally, it is worth noting that the macro MDBX_CONFIG_H should be defined as "filename.ext" but not as filename.ext.
I.e. the macro value should be a valid C-string within double quotes in order for the #include MDBX_CONFIG_H construct to be valid for the C-preprocessor.

@erthink erthink removed their assignment Jan 20, 2021
@snej
Copy link
Contributor Author

snej commented Jan 21, 2021

You noted that header is incorrect ... at least sed tool don't proceed as expected (i.e. failed to handle extended syntax properly). So it is not a libmdbx's error, but sed and/or other tools (including wrong command line options processing by windows version of GNU make and bash).

Actually that glitch occurred when I ran make dist on macOS 10.15. (Those sources are checked into my repo.) I don't know the exact version of sed, but it seems to be based on the FreeBSD version.

use CMake for building on Windows and create amalgamated source code (i.e. running make dist) only on modern Unix

The CMake build fails unless the sources be in a git repo. This was causing problems for me with the way Nim installs packages ... basically by the time CMake runs, my package's source tree (and submodules) has been copied and isn't a git repo any more. Obviously not your problem; I'll try to find a workaround.

Thanks for your help!

@snej snej closed this as completed Jan 21, 2021
@erthink
Copy link
Owner

erthink commented Jan 21, 2021

Perhaps I'm confusing something, but AFAIR, I checked the work of make dist on Freebsd.
Nonetheless, at least, I will add a check of the make dist result to detect problem immediately.

@erthink
Copy link
Owner

erthink commented Jan 21, 2021

And, of course, all suggestions on how to make the sed script more portable are welcome.

@erthink erthink added the not-a-bug external issue and/or not a libmdbx's bug label Jan 21, 2021
erthink added a commit that referenced this issue Jan 22, 2021
Related to #155

Change-Id: I45a4f6d3d62052d091c18eae634bbe418829761e
@erthink
Copy link
Owner

erthink commented Jan 22, 2021

@snej, I just added an amalgamated source code check during make dist, and these changes have passed CI-check on Travis-CI, including for MacOS and FreeBSD.

This means that my amalgamation script do not have any troubles, it work fine on Linux, FreeBSD and MacOS.

erthink added a commit that referenced this issue Jan 23, 2021
Rework 0166071 for #155

Change-Id: I40697203ff351d10709b3094d50b894c663c67c3
erthink added a commit that referenced this issue Jan 24, 2021
Rework 0166071 for #155

Change-Id: I40697203ff351d10709b3094d50b894c663c67c3
erthink added a commit that referenced this issue Jan 24, 2021
Resolves #156
Related to #155

Change-Id: I68da3f40b055da08a905525a4a31b44018d419b0
erthink added a commit that referenced this issue Jan 24, 2021
Rework 0166071 for #155
Related to #156

Change-Id: I40697203ff351d10709b3094d50b894c663c67c3
@AskAlexSharov
Copy link
Contributor

AskAlexSharov commented May 8, 2021

Looks like Go doesn't support linking to MSVC files ( golang/go#20982 ). So, i'm trying to build with mingw.
But build with mingw failed with error from this ticket unknown type name 'FILE_INFO_BY_HANDLE_CLASS'
https://github.com/torquem-ch/mdbx-go/runs/2533036546?check_suite_focus=true#step:6:63

But I didn't use amalgamated code, I run next on "libmdbx" git submodule: cmake -G "MinGW Makefiles" . then cmake --build .

erthink added a commit that referenced this issue May 8, 2021
Related to #155

Change-Id: I1ec9e43c4aedc82c6d4fe4b43a078b72da5a00ae
erthink added a commit that referenced this issue May 8, 2021
Related to #155

Change-Id: I2dcee3601dfb1ac5787fda3e0222cbcb7c6446f2
@erthink
Copy link
Owner

erthink commented May 8, 2021

The first problem was that when building with MinGW, the target version of Windows is selected too old, in which this enum is not yet defined. I made a couple of commits to fix this.

However, the second problem error: unknown conversion type character 'z' in format still remains.
Suggested solutions worth trying:

  1. Add somithink like this to here:
    #ifdef __MINGW32__
    #define __USE_MINGW_ANSI_STDIO 1
    #endif 
    
  2. Use the latest CMake which (may be) known how to enable C11 for MinGW.
  3. Modifiy CMakeLists.txt to force add -std=c11 for MinGW.

@AskAlexSharov, could you try these solutions (since I haven't MinGW for now) ?

@erthink erthink reopened this May 8, 2021
erthink added a commit that referenced this issue May 8, 2021
More for #155

Change-Id: I98b41ac89ae07ed12f342a080b4921c0cd03b973
@erthink
Copy link
Owner

erthink commented May 8, 2021

@AskAlexSharov, another option: you may prefer to build libmdbx by MSVC as a shared library (i.e. dll) with MDBX_WITHOUT_MSVC_CRT option for avoid using any additional runtime/CRT(s).
Sure GoLand should be able to use a dll with the C-header (i.e. mdbx.h).

erthink added a commit that referenced this issue May 8, 2021
More for #155

Change-Id: Id2a7c55aee44a651dff40f6d04192693c5cd5453
@AskAlexSharov
Copy link
Contributor

  1. cmake version 3.20.1
  2. Applied patch erigontech/mdbx-go@17e87d1 - result https://github.com/torquem-ch/mdbx-go/runs/2533764261?check_suite_focus=true#step:6:176
  3. "Modifiy CMakeLists.txt to force add -std=c11 for MinGW." - sorry, not sure how to
  4. "since I haven't MinGW for now" - I using next github actions config: https://github.com/torquem-ch/mdbx-go/blob/win/.github/workflows/test.yml
  5. "as a shared library" - I will try today.

@erthink
Copy link
Owner

erthink commented May 8, 2021

I've added a few more fixes, but I am more and more inclined to close this issue as "won't fix" since MinGW don't support:

  • the __try and __except statements for native/structural exception handling.
  • the z printf's format flag for size_t.
  • the /* fall through */ comment for explicit marking a case statements which are fall through.

@erthink
Copy link
Owner

erthink commented May 8, 2021

Now it (likely) should be possible to build libmdbx by MinGW with (a lot of) warnings, but I still suggest you to use a dll built by MSVC.

@AskAlexSharov
Copy link
Contributor

AskAlexSharov commented May 8, 2021

cmake -DBUILD_SHARED:BOOL=ON -DBUILD_SHARED_LIBS:BOOL=ON -DMDBX_WITHOUT_MSVC_CRT:BOOL=ON .
cmake --build .

Some warnings of MSVC build: https://github.com/torquem-ch/mdbx-go/runs/2533871563?check_suite_focus=true#step:5:108

erthink added a commit that referenced this issue May 8, 2021
Related to #155

Change-Id: I2dcee3601dfb1ac5787fda3e0222cbcb7c6446f2
erthink added a commit that referenced this issue May 8, 2021
More for #155

Change-Id: Id2a7c55aee44a651dff40f6d04192693c5cd5453
@erthink erthink self-assigned this May 8, 2021
erthink added a commit that referenced this issue May 8, 2021
Related to #155

Change-Id: Ibd795817e05b6da39ef270ce7b55b31d963d07b0
erthink added a commit that referenced this issue May 8, 2021
Related to #155

Change-Id: I49c294bd0fa055026b742a12a6f6ea9cd805cf02
erthink added a commit that referenced this issue May 8, 2021
More for #155

Change-Id: I7de6122ff160372b2dcfd2a0a26e332cb52d0560
erthink added a commit that referenced this issue May 8, 2021
Related to #155

Change-Id: I378d9c47b51c55e1e61bc7f6269cbc8182265870
erthink added a commit that referenced this issue May 8, 2021
Related to #155

Change-Id: Ib9976ccb4bd03e52b4c8cdbbf9f6b45f367c6b3d
erthink added a commit that referenced this issue May 8, 2021
…nd add `MDBX_MANUAL_MODULE_HANDLER` macro.

Briefly:
 - Now constructor/destructor of "Thread Local Storage" handled automatically when possible.
 - Otherwise the MDBX_CONFIG_MANUAL_TLS_CALLBACK macro defined to 1 to indicate that mdbx_module_handle() should be called manually.
 - Corresponding build option MDBX_CONFIG_MANUAL_TLS_CALLBACK was removed.

Related to #155

Change-Id: Ic4e6a34b44f874676f0ab212ff473460e3d80559
erthink added a commit that referenced this issue May 8, 2021
Resolves Related to #155

Change-Id: I4419c4e494139e644ff0ed755ce4560000099d82
@erthink
Copy link
Owner

erthink commented May 8, 2021

Windows, MSVC and MinGW must die.
All together.

@erthink erthink closed this as completed May 8, 2021
@AskAlexSharov
Copy link
Contributor

I got a bit same conclusion today. Will see if I can over-sleep it.

erthink added a commit that referenced this issue May 9, 2021
Acknowledgements:
-----------------
 - [Mahlon E. Smith](https://github.com/mahlonsmith) for [Ruby bindings](https://rubygems.org/gems/mdbx/).
 - [Alex Sharov](https://github.com/AskAlexSharov) for [mdbx-go](https://github.com/torquem-ch/mdbx-go), bug reporting and testing.
 - [Artem Vorotnikov](https://github.com/vorot93) for bug reporting and PR.
 - [Paolo Rebuffo](https://www.linkedin.com/in/paolo-rebuffo-8255766/), [Alexey Akhunov](https://github.com/AlexeyAkhunov) and Mark Grosberg for donations.
 - [Noel Kuntze](https://github.com/Thermi) for preliminary [Python bindings](https://github.com/Thermi/libmdbx/tree/python-bindings)

New features:
-------------
 - Added `mdbx_env_set_option()` and `mdbx_env_get_option()` for controls
   various runtime options for an environment (announce of this feature  was missed in a previous news).
 - Added `MDBX_DISABLE_PAGECHECKS` build option to disable some checks to reduce an overhead
   and detection probability of database corruption to a values closer to the LMDB.
   The `MDBX_DISABLE_PAGECHECKS=1` provides a performance boost of about 10% in CRUD scenarios,
   and conjointly with the `MDBX_ENV_CHECKPID=0` and `MDBX_TXN_CHECKOWNER=0` options can yield
   up to 30% more performance compared to LMDB.
 - Using float point (exponential quantized) representation for internal 16-bit values
   of grow step and shrink threshold when huge ones (#166).
   To minimize the impact on compatibility, only the odd values inside the upper half
   of the range (i.e. 32769..65533) are used for the new representation.
 - Added the `mdbx_drop` similar to LMDB command-line tool to purge or delete (sub)database(s).
 - [Ruby bindings](https://rubygems.org/gems/mdbx/) is available now by [Mahlon E. Smith](https://github.com/mahlonsmith).
 - Added `MDBX_ENABLE_MADVISE` build option which controls the use of POSIX `madvise()` hints and friends.
 - The internal node sizes were refined, resulting in a reduction in large/overflow pages in some use cases
   and a slight increase in limits for a keys size to ≈½ of page size.
 - Added to `mdbx_chk` output number of keys/items on pages.
 - Added explicit `install-strip` and `install-no-strip` targets to the `Makefile` (#180).
 - Major rework page splitting (af9b7b5) for
     - An "auto-appending" feature upon insertion for both ascending and
       descending key sequences. As a result, the optimality of page filling
       increases significantly (more densely, less slackness) while
       inserting ordered sequences of keys,
     - A "splitting at middle" to make page tree more balanced on average.
 - Added `mdbx_get_sysraminfo()` to the API.
 - Added guessing a reasonable maximum DB size for the default upper limit of geometry (#183).
 - Major rework internal labeling of a dirty pages (958fd5b) for
   a "transparent spilling" feature with the gist to make a dirty pages
   be ready to spilling (writing to a disk) without further altering ones.
   Thus in the `MDBX_WRITEMAP` mode the OS kernel able to oust dirty pages
   to DB file without further penalty during transaction commit.
   As a result, page swapping and I/O could be significantly reduced during extra large transactions and/or lack of memory.
 - Minimized reading leaf-pages during dropping subDB(s) and nested trees.
 - Major rework a spilling of dirty pages to support [LRU](https://en.wikipedia.org/wiki/Cache_replacement_policies#Least_recently_used_(LRU))
   policy and prioritization for a large/overflow pages.
 - Statistics of page operations (split, merge, copy, spill, etc) now available through `mdbx_env_info_ex()`.
 - Auto-setup limit for length of dirty pages list (`MDBX_opt_txn_dp_limit` option).
 - Support `make options` to list available build options.
 - Support `make help` to list available make targets.
 - Silently `make`'s build by default.
 - Preliminary [Python bindings](https://github.com/Thermi/libmdbx/tree/python-bindings) is available now
   by [Noel Kuntze](https://github.com/Thermi) (#147).

Backward compatibility break:
-----------------------------
 - The `MDBX_AVOID_CRT` build option was renamed to `MDBX_WITHOUT_MSVC_CRT`.
   This option is only relevant when building for Windows.
 - The `mdbx_env_stat()` always, and `mdbx_env_stat_ex()` when called with the zeroed transaction parameter,
   now internally start temporary read transaction and thus may returns `MDBX_BAD_RSLOT` error.
   So, just never use deprecated `mdbx_env_stat()' and call `mdbx_env_stat_ex()` with transaction parameter.
 - The build option `MDBX_CONFIG_MANUAL_TLS_CALLBACK` was removed and now just a non-zero value of
   the `MDBX_MANUAL_MODULE_HANDLER` macro indicates the requirement to manually call `mdbx_module_handler()`
   when loading libraries and applications uses statically linked libmdbx on an obsolete Windows versions.

Fixes:
------
 - Fixed performance regression due non-optimal C11 atomics usage (#160).
 - Fixed "reincarnation" of subDB after it deletion (#168).
 - Fixed (disallowing) implicit subDB deletion via operations on `@MAIN`'s DBI-handle.
 - Fixed a crash of `mdbx_env_info_ex()` in case of a call for a non-open environment (#171).
 - Fixed the selecting/adjustment values inside `mdbx_env_set_geometry()` for implicit out-of-range cases (#170).
 - Fixed `mdbx_env_set_option()` for set initial and limit size of dirty page list ((#179).
 - Fixed an unreasonably huge default upper limit for DB geometry (#183).
 - Fixed `constexpr` specifier for the `slice::invalid()`.
 - Fixed (no)readahead auto-handling (#164).
 - Fixed non-alloy build for Windows.
 - Switched to using Heap-functions instead of LocalAlloc/LocalFree on Windows.
 - Fixed `mdbx_env_stat_ex()` to returning statistics of the whole environment instead of MainDB only (#190).
 - Fixed building by GCC 4.8.5 (added workaround for a preprocessor's bug).
 - Fixed building C++ part for iOS <= 13.0 (unavailability of  `std::filesystem::path`).
 - Fixed building for Windows target versions prior to Windows Vista (`WIN32_WINNT < 0x0600`).
 - Fixed building by MinGW for Windows (#155).

TODO for a next releases:
-------------------------
 - [Get rid of dirty-pages list in MDBX_WRITEMAP mode](#193).
 - [Large/Overflow pages accounting for dirty-room](#192).
 - [C++ Buffer issue](#191).
 - Finalize C++ API (few typos and trivia bugs are still likely for now).
 - [Support for RAW devices](#124).
 - [Test framework issue](#127).
 - [Support MessagePack for Keys & Values](#115).
 - [Engage new terminology](#137).
 - Packages for [Astra Linux](https://astralinux.ru/), [ALT Linux](https://www.altlinux.org/), [ROSA Linux](https://www.rosalinux.ru/), Fedora/RHEL, Debian/Ubuntu.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement not-a-bug external issue and/or not a libmdbx's bug
Projects
None yet
Development

No branches or pull requests

3 participants