Dmitry Ivanov authored 1 year ago and Yaroslav Dynnikov committed 1 year ago

This fixes failing builds on Arch Linux, Fedora and other Linux
distros which don't provide static libgomp or build it without -fPIE.

See https://git.picodata.io/picodata/tarantool/-/merge_requests/107

Some code in libldap which we don't need requires libresolv,
so we should relax linking rules to let linker drop it.

Closes #240

Feodor Alexandrov authored 1 year ago and Yaroslav Dynnikov committed 1 year ago

This commit reverts "build: fix libcrypto renaming" (9485e9c1). The fix
proposed in that commit actually broke the whole build instead of fixing
re-build. Since there are two `libcrypto.a` (tarantool one and symlinked
openssl), the linker sometimes loads the same library twice (`-lcrypto` and
`-lcrypto-ssl`).

This patch provides another solution - tarantool lib is renamed so now
`-lcrypto` uniquely corresponds to openssl and
`-ltcrypto` to the tarantool library.

Renaming libcrypto breaks the rebuid. Repro:

```bash
cargo build -vv
rm -rf target/debug/picodata-*
cargo build -vv
```

It results in

```
[build.rs:204] running ["cmake" "--build" "target/debug/build/tarantool-sys/tarantool-prefix/src/tarantool-build"]
gmake[2]: *** No rule to make target 'target/debug/build/tarantool-sys/openssl-prefix/lib/libcrypto.a', needed by 'src/tarantool'.  Stop.
```

Instead of renaming this patch makes a symlink.

Current commit introduces distributed SQL in picodata (we use
'sbroad' library). Now it is possible to execute cluster-wide
SQL with 'pico.sql([[query]], {params})' lua method.

Current limitations:
1. DDL is still not implemented in picodata, so space sharding
   is mocked by '_pico_space' space (we'll remove it later).
2. We still have duplicating symbols of the Tarantool module in
   sbroad and in picodata rlibs.
3. We need to reuse piocdata's sharding functions and handle the
   case when different spaces in the query have different sharding
   functions.

Co-authored-by: Georgy Moshkin <gmoshkin@picodata.io>

Denis Smirnov authored 2 years ago and Yaroslav Dynnikov committed 2 years ago

We don't want to strip unused symbols from the static libraries on
the linking stage. This leads to problems with tarantool's crypto
module. Some symbols ("crypto_EVP_get_digestbyname" for example)
were used only by the ffi calls inside `crypto.lua` file. So the
linker stripped these symbols as it was not able to find their
usage in the rest of the C code. This produced SIGABRT on the
picodata startup when tarantool_lua_init() initialized the crypto
module.

Denis Smirnov authored 2 years ago and Yaroslav Dynnikov committed 2 years ago

An attempt to start picodata on macos returns an error:
symbol not found in flat namespace '_res_9_getservers'

The reason is that macos executables require libresolv.dylib to
use resolv.h.

Yaroslav Dynnikov authored 2 years ago and Alexey Protsenko committed 2 years ago

The build process was refactored recently in b9aeab59 and it broke the
packaging in a tricky way.

Packpack wraps the whole build process in a Makefile which exports its
own `MAKEFLAGS` environment variable:

```
MAKEFLAGS="w -j -- BUILDDIR=/build"
```

The `cmake` crate used to override it with `CARGO_MAKEFLAGS` provided by
cargo. But after we've replaced it with `std::process::Command`, the
`BUILDDIR` setting has made its way to the `icu` build, which suddenly
clashed one of internal `make` variables and wrote there resource files.
As a result, the `icu` build started to fail with the error:

```
icupkg: unable to open input file
"/build/usr/src/debug/picodata-22.11.0.158/
   /target/debug/build/tarantool-sys/icu-prefix/src/icu-build/
   /data/out/build/icudt62l/af.res"
```

while all resources were copied directly to `/build/af.res`.

This patch repeats the behavior of the `cmake` crate.

It also makes logging more verbose and aligns the output. Replacing
`eprintln` with `println` is necessary since `make` and `cmake` both
write the logs to the `stdout`.

Yaroslav Dynnikov authored 2 years ago and Yaroslav Dynnikov committed 2 years ago

Cargo used to complain:

```
error: the linking modifiers `+bundle` and `+whole-archive` are not
compatible with each other when generating rlibs
```

Also remove unnecessary subdir

1. Encapsulate printing rustc flags in their own mod.
2. Arrange paths formatting.

1. Remove build dependency on cmake crate. It doesn't fulfill all our
   needs, so we run cmake directly via `std::process::command`. It also
   makes `build.rs` easier to understand.
2. Arrange paths variable names.

Provide a supplementary `Run` trait for the
`std::process::Command`.

We haven't used rustc < 1.61 for a long time.

Georgy Moshkin authored 2 years ago and Yaroslav Dynnikov committed 2 years ago

The commit c18fe6e6 is wrong. We've tried to disable building tests,
but:

1. A space in "CMAKE_TARANTOOL_ARGS" was parsed incorrectly.
2. `tarantool-sys/static-build/CMakeFiles.txt` did ignore
   `-DBUILD_TESTING` option.

It's impossible to build picodata with packpack - it removes the `.git`
directory from the build environment. The git is only necessary for
applying tarantool patches, but `patch -p1` can be used for that
instead.

Georgy Moshkin authored 2 years ago and Georgy Moshkin committed 2 years ago

since 1.61 rust has added a `whole-archive` modifier for link targets
which is unset by default.

Previously to this version this was probably undefined behavior, because
the linking would work fine when building an executable (like picodata)
but would fail with undefined symbols when building a static library.

But now with the advent of this option, we can probably refactor
picodata into a library which can be useful for testing, i.e. we can
exclude testing stuff from the binary but still have a test binary with
all the necessary runtime symbols.

Closes #81

- don't patch if tarantool-sys/patch-applied exists
- use a single build directory for `cargo build` & `cargo check`