Skip to content

Comments

update deps of libzauth#2327

Merged
jschaul merged 13 commits intodevelopfrom
update-libzauth-deps
May 3, 2022
Merged

update deps of libzauth#2327
jschaul merged 13 commits intodevelopfrom
update-libzauth-deps

Conversation

@jschaul
Copy link
Member

@jschaul jschaul commented Apr 27, 2022

Upgrade version of libzauth dependencies, notably sodiumoxide bindings to libsodium, and fix resulting errors and warnings.

libzauth is used as part of the nginz module for authorization.

This PR was triggered by dependabot warnings.

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@jschaul jschaul requested a review from mdimjasevic April 27, 2022 15:52
@jschaul jschaul temporarily deployed to cachix April 27, 2022 15:52 Inactive
- The result of running `cargo update` in the libzauth and libzauth-c
folders is reflected in Cargo.lock files. One of the two lock files
wasn't tracked by Git so this commit adds it.
@mdimjasevic mdimjasevic temporarily deployed to cachix April 28, 2022 08:47 Inactive
@mdimjasevic
Copy link
Contributor

@jschaul, did you say yesterday we might have to update sodiumoxide in some other place too?

@jschaul jschaul temporarily deployed to cachix April 28, 2022 12:37 Inactive
@jschaul
Copy link
Member Author

jschaul commented Apr 28, 2022

We're currently using libsodium 1.0.16 here, there's libsodium 1.0.18 out there, @comawill do we need to /should we update this also?

@jschaul jschaul requested a review from comawill April 28, 2022 12:43
@jschaul jschaul temporarily deployed to cachix April 28, 2022 18:00 Inactive
Copy link
Contributor

@comawill comawill left a comment

Choose a reason for hiding this comment

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

Good work, on updating this library and the dependencies.

Yes, we definitely should update the used libsodium, too.

Right now the build process does not execute the tests, we provide in the library, cargo test should be executed when building the debian files.

Co-authored-by: Sebastian Willenborg <sebastian.willenborg@wire.com>
@jschaul jschaul temporarily deployed to cachix May 2, 2022 10:47 Inactive
@jschaul jschaul temporarily deployed to cachix May 2, 2022 10:48 Inactive

#[repr(C)]
#[no_mangle]
#[derive(Clone, Copy, Debug)]
Copy link
Member Author

Choose a reason for hiding this comment

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

@comawill this is the place I'm least sure about my change: the no_mangle at the enum level led to warnings; but I'm not sure if removing them is okay to do here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine. I don't think enum names or enum value names are part of the C ABI.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think no_mangle on structs and enums doesn't do anything, so it is safe to remove.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for your reviews!

@jschaul jschaul temporarily deployed to cachix May 2, 2022 11:09 Inactive
@jschaul jschaul temporarily deployed to cachix May 2, 2022 11:14 Inactive
@jschaul jschaul requested a review from comawill May 2, 2022 11:33
Copy link
Contributor

@pcapriotti pcapriotti left a comment

Choose a reason for hiding this comment

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

LGTM


#[repr(C)]
#[no_mangle]
#[derive(Clone, Copy, Debug)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think no_mangle on structs and enums doesn't do anything, so it is safe to remove.

@jschaul jschaul temporarily deployed to cachix May 3, 2022 09:25 Inactive
@jschaul jschaul temporarily deployed to cachix May 3, 2022 09:25 Inactive
@jschaul jschaul merged commit 35399e6 into develop May 3, 2022
@jschaul jschaul deleted the update-libzauth-deps branch May 3, 2022 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants