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

Fix clippy and fmt issues #1039

Conversation

iamnotacake
Copy link
Contributor

@iamnotacake iamnotacake commented Nov 24, 2023

New clippy, new issues that make working code fail CI checks. Again.

Fixed clippy issues with cargo clippy --fix --all-targets, the formatted with cargo fmt.

Only cosmetic changes, most of them are outside of wrapper, i.e. in tests and benchmarks.

Update MSRV to 1.60 since our dependency zeroize now requires it.

Freeze two more dev deps regex and memchr, dependencies of env_logger used in Rust Themis tests.

Checklist

  • Change is covered by automated tests
  • Benchmark results are attached (if applicable)
  • The coding guidelines are followed
  • Public API has proper documentation
  • Example projects and code samples are up-to-date (in case of API changes)
  • Changelog is updated (in case of notable or breaking changes)

@vixentael
Copy link
Contributor

Uh..

error: package `zeroize v1.7.0` cannot be built because it requires rustc 1.60 or newer, while the currently active rustc version is 1.58.1```

@iamnotacake
Copy link
Contributor Author

error: package zeroize v1.7.0 cannot be built because it requires rustc 1.60 or newer

Well, there are two-and-a-half possible solutions:

  1. Simply freeze zeroize version like I did with some other test deps that require newer compiler; basically, it's all about tests, and Themis itself does not require newer compiler
  2. Update MSRV to 1.60 (released on April 2022 btw) and solve this issue
    2.5) Update MSRV even more (something like 1.63?) to also remove other frozen deps, solve all issues until some deps will update again and will require even newer toolchain, but that's in future

@vixentael
Copy link
Contributor

i vote for (1), @Lagovas WDYT?

@Lagovas
Copy link
Collaborator

Lagovas commented Nov 24, 2023

I vote for both cases. Freeze and update, because I see, that a lot of dependencies upgraded their dependencies (it's 3th or 4th issue like that0

Due to themis dependency zeroize now requiring rustc 1.60
Update required Rust version in manifests of libthemis and libthemis-sys

Freeze some test deps so they compile on Rust 1.60, with no effect on
themis itself (does not use those frozen crates)
Copy link
Collaborator

@G1gg1L3s G1gg1L3s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy happy, me happy. Looks good!

Set exact version for all dev-dependencies of themis.
Also update few of them, explicitly set versions that were actually used
after running `cargo update` with build and tests passing.
@vixentael vixentael added the W-RustThemis 🦀 Wrapper: Rust-Themis, Rust API, Cargo crates label Nov 27, 2023
@iamnotacake iamnotacake merged commit 30578c8 into cossacklabs:master Nov 27, 2023
19 checks passed
@iamnotacake iamnotacake deleted the anatolii/fix-rust-lint-issues-yet-another-time branch November 27, 2023 15:53
G1gg1L3s added a commit to G1gg1L3s/themis that referenced this pull request Dec 4, 2023
commit 784033b
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Tue Nov 28 21:45:12 2023 +0200

    New methods of building/installing PyThemis (cossacklabs#1023)

    Refactor Makefile:
    * new target pythemis_make_wheel to create a .whl Python package,
      current modern format to be installed into virtual environmants
    * new target pythemis_install_wheel to install it in currently active virtualenv
    * new target deb_python that builds a .deb package for system-wide
      installation of PyThemis
    * new target pythemis_install_deb, alias for pythemis_deb + apt install of
      the created pkg
    * new target rpm_python, similar to deb_python
    * new target pythemis_install_rpm, pythemis_install_deb

    Update GitHub Actions workflow, test .whl and .deb installation

    See cossacklabs/product-docs/pull/317 for related docs update

commit 30578c8
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Mon Nov 27 17:53:02 2023 +0200

    Fix clippy and fmt issues, update MSRV (cossacklabs#1039)

    Fix clippy and fmt issues

    Update MSRV to 1.60

    Freeze test deps so they compile on Rust 1.60, with no effect on
    themis itself (does not use those frozen crates)

commit 6111766
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Tue Nov 21 17:08:51 2023 +0200

    Update emscripten requirements and WASM CI job (cossacklabs#1036)

    * Bump emsdk version to 3.1.47
      Produces module importable in Node v18
      With older (emsdk 3.0.0) version, generated `libthemis.js` that should
      load `libthemis.wasm` fails due to some internal autogenerated code
      working differently on v16 and v18

    * Add link flag for WASM builds
      Needed to make new emscripten produce working module

    * Update Node testing versions
      Add v18 that should now work, also add v20 that is in active development
      as of now, but if tests are green then why not?

    * Switched integration tests to use v16

    * Removed testing of quite old and deprecated v10

    * Updated version of BoringSSL submodule to a newer one (not the latest though)

commit 05cac26
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Thu Nov 9 22:24:35 2023 +0200

    CI fixes (cossacklabs#1034)

    Rix Rust CI builds (cossacklabs#1032)

    * Pin log dependency to 0.4.17
    * Pin byteorder dependency to 1.4.3

    Last versions that still work with current MSRV 1.58.

    Fix sanitizers CI job (cossacklabs#1033)

    * Switch to GCC 10
    * Install `libgcc-10-dev` that provides file `libtsan_preinit.o` needed
      for thread sanitizer

    Change priority of PREFIX in CFLAGS and LDFLAGS (cossacklabs#1031)

    * Change include dir priority in CFLAGS
    Move 'CFLAGS += -I/usr/local/include' below engine selection macros,
    so that if OS have OpenSSL header files installed in /usr/local/include,
    it won't mess with vendored BoringSSL or any other selected engine.

    * Put /usr/local/lib in separate LDFLAGS
    Introduce ADDITIONAL_LDFLAGS macro and put it after both LDFLAGS
    and CRYPTO_ENGINE_LDFLAGS during linking.

    * Add optional verbose logging to Makefile
    Add VERBOSE option to Makefile. If set, print executed command
    (compiler, linker etc) even for successful runs, and run ldd on created
    shared libraries.

    * Enable verbose builds on macOS jobs

commit 06d52f4
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Tue Sep 26 13:15:37 2023 +0300

    Fix make target rbthemis_uninstall (cossacklabs#1022)

    * Fix make target rbthemis_uninstall
    * Add RbThemis uninstall step to CI

commit 8609650
Author: Nazar Serhiichuk <[email protected]>
Date:   Mon Jul 10 22:57:46 2023 +0300

    Avoid specific versions in README (cossacklabs#1016)

    Just to avoid responsibility of updating it during release

    (•_•)
    ( •_•)>⌐■-■
    (⌐■_■)

commit 3219654
Author: Martin Arista <[email protected]>
Date:   Mon Jul 10 11:54:25 2023 -0400

    Update README.md (cossacklabs#1015)

    update links for maven and java/kotlin links
Lagovas pushed a commit that referenced this pull request Dec 5, 2023
* Pre-release Themis 0.15.0 (#1011)

* Fix rust issues (pin log, run bindgen) (#1005)

* rust: Pin log version to =0.4.18

The 0.4.19 requires rustc 1.60, but currently we support 1.58.
Pinning it is not a big deal since it's development dependecy for
tests and examples.

* rust: Regenerate and update lib.rs

bindgen was updated again and changed something which resulted in
new output (seems like some internal constants are removed).

* Pythemis: introduce `pyproject.toml` (#1006)

* pythemis: Add pyproject.toml

Since setup.py is deprecated, let's try moving to the pyproject.toml
and configuring it with the same data as in setup.py.

Use setuptools as a backend for no particular reasons ¯\_(ツ)_/¯,
just because the name is familiar and we have no reasons to not use
it or use something else.

Keep the old setup.py for backward compatibility so old systems can
try to build the package.

For now, keep 0.14.0, we will bump the version in another PR.

* makefile: Use pyproject.toml for installing pythemis

According to this [1] article, the correct command is

    pip install .

in the project's root. Let's try that. Also, the other option is

    python -m build --wheel

which builds the package but doesn't install it. We can provide
something like `pythemis_build` for it for example.

[1]: https://godatadriven.com/blog/a-practical-guide-to-setuptools-and-pyproject-toml/

* pythemis: Update classifiers to Python3.6+

With many hours and docker containers I tested that themis actually
works up to python 3.4. The other versions require some changes in
makefile so they are more like "grey area".

However, python3.5 is deprecated and it produces warning like
"DEPRECATION: Python 3.5 reached the end of its life on..." so many
libraries don't support it. Instead they start with 3.6 which will
do as well, I guess.

Though, actually python3.6 is also deprecated [1]. The same will be
true for python3.7 in a couple of days (Jun 27 2023), so the question
is, should we declare support of these versions?

[1]: https://devguide.python.org/versions/

* pythemis: Extend range of supported py versions

* Update changelog

* Run and pin bindgen (#1008)

* rust-themis: Update bindgen

It updated and broke something again 🤦

* rust-themis: Pin bindgen version

It is pretty unstable with its frequent releases, so let's pin it.

* Update changelog

* Bump wrapper versions to 0.15.0 (#1007)

* changelog: Add 0.15.0 summary

* themis-core: Update version

* pythemis: Update version

* pythemis: Fix 8-year old typo in AUTHORS :)

* rbthemis: Update version

* jsthemis: Update versions

* wasm-themis: Update versions

* android-themis: Update version

* rust-themis: Update versions

* react-native-themis: Update versions

* pythemis: https in AUTHORS

Co-authored-by: vixentael <[email protected]>

* rust-themis: Update bench versions

Somehow missed that.

* changelog: Forgot to mention rust 1.58

* changelog: Mention the new iteration count

---------

Co-authored-by: vixentael <[email protected]>

* Bump embedded BoringSSL (#1004)

* Bump BoringSSL and fix makefile

This is not the latest BoringSSL version yet, because there are
a couple of fixes. So, treat it as the first. Here we also fix
our makefile because the BoringSSL team fixed bug with the strange
behaviour of absolute path to symbols.txt [1].

[1]: https://boringssl.googlesource.com/boringssl/+/8c75ed046f799f1d8b805036b1dea9c5ec0a0fb5%5E%21/#F0

* Bump BoringSSL and fix opaque EVP

As OpenSSL, BoringSSL made many types opaque, so it will require
updating some of the code to not use fields.

* Bump BoringSSL again and fix RSA

The same issue - RSA type became opaque, so we need to use accessors
similar to what Openssl had.

* Bump BoringSSL once more

This is (hoperfully) the last bump. This time without issues but
we will see what CI says.

* Make bignum_to_bytes accept const bignum*

It will prevent some of the warnings. This function doesn't mutate
bignum anyway.

* Update changelog

* boringssl: Bump once again

* msys2: Update hashes temporarily

This are test values because we will move the tag. But for now,
let's just test it.

* phpthemis: Update version for the sake of testing

They will fail probably, but just out of curiosity let's try to run
the tests.

* Update date of the release

Solstice!

---------

Co-authored-by: vixentael <[email protected]>

* msys2: Update checksums (#1012)

It's the egg-chicken problem: we can update those hashes only
after the release. But then, the release tag will not point to
the updated hashes.

* Squashed commit of the following:

commit 784033b
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Tue Nov 28 21:45:12 2023 +0200

    New methods of building/installing PyThemis (#1023)

    Refactor Makefile:
    * new target pythemis_make_wheel to create a .whl Python package,
      current modern format to be installed into virtual environmants
    * new target pythemis_install_wheel to install it in currently active virtualenv
    * new target deb_python that builds a .deb package for system-wide
      installation of PyThemis
    * new target pythemis_install_deb, alias for pythemis_deb + apt install of
      the created pkg
    * new target rpm_python, similar to deb_python
    * new target pythemis_install_rpm, pythemis_install_deb

    Update GitHub Actions workflow, test .whl and .deb installation

    See cossacklabs/product-docs/pull/317 for related docs update

commit 30578c8
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Mon Nov 27 17:53:02 2023 +0200

    Fix clippy and fmt issues, update MSRV (#1039)

    Fix clippy and fmt issues

    Update MSRV to 1.60

    Freeze test deps so they compile on Rust 1.60, with no effect on
    themis itself (does not use those frozen crates)

commit 6111766
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Tue Nov 21 17:08:51 2023 +0200

    Update emscripten requirements and WASM CI job (#1036)

    * Bump emsdk version to 3.1.47
      Produces module importable in Node v18
      With older (emsdk 3.0.0) version, generated `libthemis.js` that should
      load `libthemis.wasm` fails due to some internal autogenerated code
      working differently on v16 and v18

    * Add link flag for WASM builds
      Needed to make new emscripten produce working module

    * Update Node testing versions
      Add v18 that should now work, also add v20 that is in active development
      as of now, but if tests are green then why not?

    * Switched integration tests to use v16

    * Removed testing of quite old and deprecated v10

    * Updated version of BoringSSL submodule to a newer one (not the latest though)

commit 05cac26
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Thu Nov 9 22:24:35 2023 +0200

    CI fixes (#1034)

    Rix Rust CI builds (#1032)

    * Pin log dependency to 0.4.17
    * Pin byteorder dependency to 1.4.3

    Last versions that still work with current MSRV 1.58.

    Fix sanitizers CI job (#1033)

    * Switch to GCC 10
    * Install `libgcc-10-dev` that provides file `libtsan_preinit.o` needed
      for thread sanitizer

    Change priority of PREFIX in CFLAGS and LDFLAGS (#1031)

    * Change include dir priority in CFLAGS
    Move 'CFLAGS += -I/usr/local/include' below engine selection macros,
    so that if OS have OpenSSL header files installed in /usr/local/include,
    it won't mess with vendored BoringSSL or any other selected engine.

    * Put /usr/local/lib in separate LDFLAGS
    Introduce ADDITIONAL_LDFLAGS macro and put it after both LDFLAGS
    and CRYPTO_ENGINE_LDFLAGS during linking.

    * Add optional verbose logging to Makefile
    Add VERBOSE option to Makefile. If set, print executed command
    (compiler, linker etc) even for successful runs, and run ldd on created
    shared libraries.

    * Enable verbose builds on macOS jobs

commit 06d52f4
Author: Anatolii Lishchynskyi <[email protected]>
Date:   Tue Sep 26 13:15:37 2023 +0300

    Fix make target rbthemis_uninstall (#1022)

    * Fix make target rbthemis_uninstall
    * Add RbThemis uninstall step to CI

commit 8609650
Author: Nazar Serhiichuk <[email protected]>
Date:   Mon Jul 10 22:57:46 2023 +0300

    Avoid specific versions in README (#1016)

    Just to avoid responsibility of updating it during release

    (•_•)
    ( •_•)>⌐■-■
    (⌐■_■)

commit 3219654
Author: Martin Arista <[email protected]>
Date:   Mon Jul 10 11:54:25 2023 -0400

    Update README.md (#1015)

    update links for maven and java/kotlin links

* Bump boringssl

* Revert "Bump boringssl"

This reverts commit dee8388.

Okay, this time I checked, the previous commit of boiringssl was
newer than one in the master.

* msys2: Fix hashes for 0.15.0 release (#1040)

Since we already have 0.15.0 assets on GitHub, we can specify those
and expect successful builds on stable.

---------

Co-authored-by: vixentael <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
W-RustThemis 🦀 Wrapper: Rust-Themis, Rust API, Cargo crates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants