Use PutVarInt instead of PutFixed while encoding stream entry value

[torwig]

As @ShooterIT recommended here: #745
I changed PutFixed32 to PutVarint32 and GetFixed32 to GetVarint32.
Encoding/decoding functions were 'borrowed' from RocksDB codebase.
Obviously, this change makes it impossible to decode entries if entries were encoded with PutFixed32 (an error will be returned).
Is it too late to introduce such a change?

[ShooterIT]

Is it too late to introduce such a change?

I don't think it is late, since now it is only on unstable branch which we may make changes on it.

one problem we need to concern is about open source license about these codes copied from leveldb/rocksdb.
should we put its license header. And is xxFixed32 copied from leveldb/rocksdb, should we describe this? @git-hulk

[git-hulk]

@torwig Thanks for your contribution. As @ShooterIT mentioned that we don't promise stability and compatibility before releasing, so feel free to do that.

@ShooterIT Those Put|GetFixedXXX are NOT copied from rocksdb/leveldb indeed, we rewritten them into BIGENDIAN while the rocksdb is little endian, so I think it's good to keep Apache 2.0 statement only. https://github.com/facebook/rocksdb/blob/0050a73a4fb1ba52d78655b271a0f09ba1fbf7fc/util/coding_lean.h#L25

[PragmaTwice]

Other code looks good to me. Thanks for your contribution!

And you could add some unit test for *Varint functions.

[git-hulk]

LGTM, thanks

[PragmaTwice]

LGTM

[torwig]

@git-hulk BTW, after reverting snappy version I have Cmake error on Fedora 35

CMake Error at /usr/lib64/cmake/GTest/GTestTargets.cmake:37 (message):
  Some (but not all) targets in this export set were already defined.

  Targets Defined: GTest::gtest;GTest::gtest_main

  Targets not yet defined: GTest::gmock;GTest::gmock_main

Call Stack (most recent call first):
  /usr/lib64/cmake/GTest/GTestConfig.cmake:42 (include)
  /usr/share/cmake/Modules/FindGTest.cmake:187 (find_package)
  build/_deps/snappy-src/CMakeLists.txt:40 (find_package)

However, I see that all builds you have in your CI are successful.

[PragmaTwice]

@git-hulk BTW, after reverting snappy version I have Cmake error on Fedora 35

CMake Error at /usr/lib64/cmake/GTest/GTestTargets.cmake:37 (message):
  Some (but not all) targets in this export set were already defined.

  Targets Defined: GTest::gtest;GTest::gtest_main

  Targets not yet defined: GTest::gmock;GTest::gmock_main

Call Stack (most recent call first):
  /usr/lib64/cmake/GTest/GTestConfig.cmake:42 (include)
  /usr/share/cmake/Modules/FindGTest.cmake:187 (find_package)
  build/_deps/snappy-src/CMakeLists.txt:40 (find_package)

However, I see that all builds you have in your CI are successful.

There are some cmake changes from Snappy 1.1.7 to 1.1.9, find_package(gtest) always exists in 1.1.7 regardless of SNAPPY_BUILD_TESTS being set to OFF, which is fixed in 1.1.9.

@torwig Could you open an issue for this? I will fix it.

[PragmaTwice]

And if you want to workaround it now, you can just remove find_package(GTest QUIET) in <build dir>/_deps/snappy-src/CMakeLists.txt.

[torwig]

@PragmaTwice I've created an issue: #792

[git-hulk]

@ShooterIT Can have a look again and merge if have no comment.