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

Bump zbus to 2.0.0-beta.7 #37

Merged
merged 4 commits into from
Jan 18, 2022
Merged

Bump zbus to 2.0.0-beta.7 #37

merged 4 commits into from
Jan 18, 2022

Conversation

Cogitri
Copy link
Contributor

@Cogitri Cogitri commented Jun 30, 2021

Fixes #36

@complexspaces
Copy link
Collaborator

Thanks for the PR to see what this would look like. Is there any ETA for when Zbus plans to do a make a stable release of 2.0? I was not able to find any tracking issue on the GitLab repository.

@Cogitri
Copy link
Contributor Author

Cogitri commented Jul 1, 2021

Not sure if there’s an ETA but I think it’ll get released soon since there have been 5 beta releases already

@zeenix
Copy link

zeenix commented Nov 8, 2021

Hi,

zbus maintainer here.

Thanks for the PR to see what this would look like. Is there any ETA for when Zbus plans to do a make a stable release of 2.0?

Given that nobody works on zbus as their day job, it's hard to give a good estimate about 2.0 delivery day. What I can tell you is that we're very close to the API freeze. There may be API breakages before 2.0 but apart from dropping the proxy signal/property callback API (which this PR doesn't seem to be using directly at least), there are unlikely to be any major changes after beta.7.

@Cogitri Cogitri changed the title WIP: Bump zbus to 2.0.0-beta.5 Bump zbus to 2.0.0-beta.7 Dec 24, 2021
@Cogitri
Copy link
Contributor Author

Cogitri commented Dec 24, 2021

Ah sorry, somehow I completely forgot to reply, @zeenix . Thanks for the estimate. I think at this point it"d be best to merge this into master and make a new release once zbus 2 is released.

@zeenix
Copy link

zeenix commented Dec 24, 2021

Ah sorry, somehow I completely forgot to reply, @zeenix

NP. Actually, I'm hoping to release a API-freeze release today or tomorrow as there are no other API breakages planned. Then comes 2.0 in some days/weeks after people get a chance to test the API a bit more. There might still be API breakages before 2.0 but the chances are extremely low and will only happen if absolutely necessary.

@Cogitri
Copy link
Contributor Author

Cogitri commented Dec 28, 2021

Thanks, updated this PR to beta8

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/collection.rs Outdated Show resolved Hide resolved
src/collection.rs Outdated Show resolved Hide resolved
src/item.rs Outdated Show resolved Hide resolved
src/util.rs Show resolved Hide resolved
src/util.rs Outdated Show resolved Hide resolved
src/util.rs Show resolved Hide resolved
@complexspaces
Copy link
Collaborator

It looks like version 2.0.0 has been released! @zeenix thanks for the work that went into the release 🎉

@Cogitri Would you mind please updating this branch to use the stable version? I'll run CI again once you do. Please rebase onto master as well, which should fix the CI failures.

@Cogitri Cogitri force-pushed the zbus-2 branch 2 times, most recently from d62e126 to 3baaf9c Compare January 7, 2022 09:58
@Cogitri
Copy link
Contributor Author

Cogitri commented Jan 7, 2022

Seems like tests currently fail though, I'll look into that in a bit.

@zeenix
Copy link

zeenix commented Jan 7, 2022

It looks like version 2.0.0 has been released! @zeenix thanks for the work that went into the release tada

You're most welcome! It was indeed a lot of hard work to get the async API sorted. I hope you're happy with the results. :)

src/util.rs Outdated Show resolved Hide resolved
@Cogitri
Copy link
Contributor Author

Cogitri commented Jan 9, 2022

3 Tests still fail for me - but maybe that's just due to my system? AFAICS secret-service-rs doesn't mock the libsecret DBus-API so I think this might be my libsecret implementation acting up (?). Maybe someone else can try running the tests too. The output for me is:

failures:

---- item::test::should_create_and_delete_item stdout ----
thread 'main' panicked at 'item still existed', src/item.rs:174:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- item::test::should_get_and_set_item_attributes stdout ----
Attributes: {}
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `{}`,
 right: `{"test_attributes_in_item_get": "test"}`', src/item.rs:299:9

---- item::test::should_get_and_set_item_label stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"Test"`,
 right: `"Tester"`', src/item.rs:243:9


failures:
    item::test::should_create_and_delete_item
    item::test::should_get_and_set_item_attributes
    item::test::should_get_and_set_item_label

test result: FAILED. 23 passed; 3 failed; 5 ignored; 0 measured; 0 filtered out; finished in 1.42s

error: test failed, to rerun pass '--lib'

@complexspaces
Copy link
Collaborator

I took the test suite through a run in a fresh Ubuntu 20.04 instance and got similarly interesting results. With this branch, I got a total of 4 test failures:

---- item::test::should_create_and_delete_item stdout ----
thread 'item::test::should_create_and_delete_item' panicked at 'item still existed', src/item.rs:174:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- collection::test::should_search_items stdout ----
thread 'collection::test::should_search_items' panicked at 'called `Result::unwrap()` on an `Err` value: Zbus(MethodError(OwnedInterfaceName(InterfaceName(Str(Owned("org.freedesktop.DBus.Error.UnknownMethod")))), Some("No such interface “org.freedesktop.DBus.Properties” on object at path /org/freedesktop/secrets/collection/login/4"), Msg { type: Error, sender: UniqueName(Str(Borrowed(":1.45"))), reply-serial: 20, body: Signature: [
        s (115),
] }))', src/collection.rs:274:49

---- item::test::should_get_and_set_item_label stdout ----
thread 'item::test::should_get_and_set_item_label' panicked at 'assertion failed: `(left == right)`
  left: `"Test"`,
 right: `"Tester"`', src/item.rs:243:9

---- item::test::should_get_and_set_item_attributes stdout ----
Attributes: {}
thread 'item::test::should_get_and_set_item_attributes' panicked at 'assertion failed: `(left == right)`
  left: `{}`,
 right: `{"test_attributes_in_item_get": "test"}`', src/item.rs:299:9


failures:
    collection::test::should_search_items
    item::test::should_create_and_delete_item
    item::test::should_get_and_set_item_attributes
    item::test::should_get_and_set_item_label

On master, I get this failure instead:

---- item::test::should_get_and_set_item_attributes stdout ----
Attributes: {"xdg:schema": "org.freedesktop.Secret.Generic", "test_attributes_in_item_get": "test"}
thread 'item::test::should_get_and_set_item_attributes' panicked at 'assertion failed: `(left == right)`
  left: `{"xdg:schema": "org.freedesktop.Secret.Generic", "test_attributes_in_item_get": "test"}`,
 right: `{"test_attributes_in_item_get": "test"}`', src/item.rs:283:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    item::test::should_get_and_set_item_attributes

So, in total, there's a net gain of 3 failures. I think that some of these, especially the unknown method error, should be looked at before this is merged as it appears to be a regression (even in the face of these tests being shaky).

@Cogitri
Copy link
Contributor Author

Cogitri commented Jan 17, 2022

---- collection::test::should_search_items stdout ----
thread 'collection::test::should_search_items' panicked at 'called `Result::unwrap()` on an `Err` value: Zbus(MethodError(OwnedInterfaceName(InterfaceName(Str(Owned("org.freedesktop.DBus.Error.UnknownMethod")))), Some("No such interface “org.freedesktop.DBus.Properties” on object at path /org/freedesktop/secrets/collection/login/4"), Msg { type: Error, sender: UniqueName(Str(Borrowed(":1.45"))), reply-serial: 20, body: Signature: [
        s (115),
] }))', src/collection.rs:274:49

That one is especially weird - are you sure that the keyring is actual functional on that Ubuntu install? Seems like others only get the same DBus error when SSH'ing into the machine, see e.g. jaraco/keyring#514 (comment)

@Cogitri
Copy link
Contributor Author

Cogitri commented Jan 17, 2022

Oh also - are you running with multiple test thread? That seems to result in similar errors at random for me.

@Cogitri
Copy link
Contributor Author

Cogitri commented Jan 17, 2022

With the latest commit all tests pass for me.

zbus 2.x has enabled the caching of properties upon first reading them.
This messes with some of our functionality, like reading the correct
name of an Item after changing it. We should probably look into
cache invalidation down the line.
src/item.rs Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@complexspaces complexspaces left a comment

Choose a reason for hiding this comment

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

All the tests now pass for me, even in multithreaded mode, so I think this is good to go. Thanks again for doing this.

@complexspaces
Copy link
Collaborator

The CI failure is unrelated. Its happening because the zbus crate family has bumped its unofficial MSRV to 1.54 with including the README in the Rustdoc documentation. Given that this crate is heavily reliant on zbus anyway, I'll bump CI's MSRV test to 1.54 in a followup to this.

@complexspaces complexspaces merged commit 17c425c into hwchen:master Jan 18, 2022
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.

ZBus 2.0
5 participants