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 file dialogs inside docker containers, improve file dialogs on web #6795

Closed

Conversation

Tuntenfisch
Copy link

@Tuntenfisch Tuntenfisch commented Jul 7, 2024

Fixes rfd file-dialogs not opening up inside Docker containers by bumping the rfd version to 0.14.1 which includes a zenity fallback that works inside Docker containers. See #6794 for more info.

What

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@Tuntenfisch Tuntenfisch marked this pull request as ready for review July 7, 2024 09:53
@Tuntenfisch Tuntenfisch changed the title Updated rfd version to 0.14.1. Update rfd version to 0.14.1. Jul 7, 2024
@Tuntenfisch Tuntenfisch marked this pull request as draft July 7, 2024 15:30
@Tuntenfisch Tuntenfisch marked this pull request as ready for review July 7, 2024 15:30
@emilk
Copy link
Member

emilk commented Jul 7, 2024

Unfortunately this leads to a lot of duplicated dependencies, as you can see by running cargo deny check. Usually this can be fixed by strategically updating other dependencies with cargo update -p $CRATE, where $CRATE can be deduced from the output of cargo deny.

EDIT: it seems difficult to avoid pulling in zbus twice without first updating winit and accesskit, i.e. wait for:

@Tuntenfisch
Copy link
Author

Tuntenfisch commented Jul 8, 2024

Unfortunately this leads to a lot of duplicated dependencies, as you can see by running cargo deny check. Usually this can be fixed by strategically updating other dependencies with cargo update -p $CRATE, where $CRATE can be deduced from the output of cargo deny

I think the main problem is that rfd bumped its ashpd version from 0.6 to 0.8 in PolyMeilex/rfd@a88718e which in turn now requires zbus version 4.0. Sadly I couldn't find a way to reconcile the duplicate zbus dependency. But I'm also not really familar with Rust as a whole, maybe I'm just too dumb.

EDIT: Nevermind, just saw your edit... Do you have an ETA on when emilk/egui#4437 will make it into egui and in turn into the Python rerun-sdk?

@emilk emilk added the blocked can't make progress right now label Jul 8, 2024
@emilk
Copy link
Member

emilk commented Jul 8, 2024

No ETA on the winit update unfortunately. It is likely another 2 months before it makes its way to Rerun.

We could consider living with the duplicated dependencies, but I'm not a big fan of the increased compilation times that will result in.

@Tuntenfisch
Copy link
Author

No ETA on the winit update unfortunately. It is likely another 2 months before it makes its way to Rerun.

We could consider living with the duplicated dependencies, but I'm not a big fan of the increased compilation times that will result in.

No worries, maybe I can figure out a way to get rfd 0.12 with xdg-portal backend working inside my docker container after all.

@emilk
Copy link
Member

emilk commented Aug 10, 2024

I wonder if this PR solves

@emilk
Copy link
Member

emilk commented Sep 6, 2024

an update of winit has now landed on main, so this is unblocked

@emilk emilk removed the blocked can't make progress right now label Sep 6, 2024
@emilk emilk marked this pull request as draft September 16, 2024 08:06
@Wumpf
Copy link
Member

Wumpf commented Sep 17, 2024

looks like accesskit didn't get updated on egui - the zbus dependency hasn't improved when merging, resetting cargo.lock to main and building

@Wumpf
Copy link
Member

Wumpf commented Oct 29, 2024

I wonder if this PR solves #7138

It does!
image
Still looks bad, but it's a looooot better!

@Wumpf
Copy link
Member

Wumpf commented Oct 29, 2024

The problem with access kit holding on to an old version unfortuantely persists

    = zvariant_derive v3.15.1
      └── zvariant v3.15.1
          ├── accesskit_atspi_common v0.9.0
          │   └── accesskit_unix v0.12.0
          │       └── accesskit_winit v0.22.0
          │           └── egui-winit v0.29.1
          │               └── eframe v0.29.1

Given that this update also makes our save dialogs a lot better on the web, we surely can do some effort to fix the situation in accesskit @emilk ?

fwiw, rfd is by now on version 0.15

@emilk
Copy link
Member

emilk commented Oct 29, 2024

The latest accesskit_atspi_common uses zvariant 0.4: https://crates.io/crates/accesskit_atspi_common/0.9.3/dependencies

A single duplicate of zvariant shouldn't block us though imho

Wumpf added a commit that referenced this pull request Oct 29, 2024
### What

While updating #6795, I noticed
that cargo deny warned about our version of `futures-utils` being
yanked. `cargo update -p futures-util` ran cleanly, no duplicated
dependencies as a result

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7925?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7925?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7925)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
@Wumpf
Copy link
Member

Wumpf commented Oct 29, 2024

tried for a while, but it's a bit of a mess. There's other packages that are on 3.x.y zbus whereas other have moved on to 4.x.y.
Added cargo deny exception for this and did overall cleanup of deny exceptions

@Wumpf Wumpf added the dependencies concerning crates, pip packages etc label Oct 29, 2024
@Wumpf Wumpf changed the title Update rfd version to 0.14.1. Fix fild dialogs inside docker containers, improve file dialogs on web Oct 29, 2024
@Wumpf Wumpf changed the title Fix fild dialogs inside docker containers, improve file dialogs on web Fix file dialogs inside docker containers, improve file dialogs on web Oct 29, 2024
@Wumpf
Copy link
Member

Wumpf commented Oct 29, 2024

Still looks bad, but it's a looooot better!

keeping the file dialog issue open since the confusing Ok button is still there

@emilk
Copy link
Member

emilk commented Oct 31, 2024

I'll tackle this in a new PR, together with a bunch of other cargo updates

@emilk emilk closed this Oct 31, 2024
@emilk
Copy link
Member

emilk commented Oct 31, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies concerning crates, pip packages etc include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants