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

Add com.rustdesk.RustDesk #5233

Closed
wants to merge 13 commits into from
Closed

Add com.rustdesk.RustDesk #5233

wants to merge 13 commits into from

Conversation

rustdesk
Copy link

@rustdesk rustdesk commented May 11, 2024

RustDesk is an open-source remote desktop, and alternative to TeamViewer.

Please confirm your submission meets all the criteria

  • Please describe your application briefly.
  • I have read the App Requirements and App Maintenance pages.
  • My pull request follows the instructions at App Submission.
  • I have built and tested the submission locally.
  • I am using only the minimal set of permissions. (If not, please explain each non-standard permission.)
  • All assets referenced in the manifest are redistributable by any party. If not, the unredistributable parts are using an extra-data source type.
  • I am an author/developer/upstream contributor of the project. If not, I contacted upstream developers about submitting their software to Flathub. Link:
  • The domain used for the application ID is controlled by the application developers either directly or through the code hosting (e.g. GitHub, GitLab, SourceForge, etc.). The application id guidelines are followed.
  • Any additional patches or files have been submitted to the upstream projects concerned. (If not, explain why.)

com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
"--filesystem=home",
"--device=dri",
"--socket=pulseaudio",
"--talk-name=org.freedesktop.Flatpak"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not allowed by default. Why is this required?

Copy link
Author

Choose a reason for hiding this comment

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

This is remote desktop software, these are required to run it. Please check anydesk https://github.com/flathub/com.anydesk.Anydesk/blob/master/com.anydesk.Anydesk.json

Copy link
Contributor

@bbhtt bbhtt May 12, 2024

Choose a reason for hiding this comment

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

I'm going to need more than that. What specifically do you need to run on host that requires escaping the sandbox?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also this is not allowed by default, you are going to need an exception. Otherwise it won't build.

Choose a reason for hiding this comment

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

Maybe it is required to make this feature works?
Or if have any other methods that have smaller permission and won't interfere this feature?

rustdesk/rustdesk#7717

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

@rustdesk rustdesk May 12, 2024

Choose a reason for hiding this comment

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

AnyDesk does not need to use this because it support x11 only, but we support x11 and wayland both, we need this feature.

Copy link
Author

@rustdesk rustdesk May 12, 2024

Choose a reason for hiding this comment

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

We add these features not for playing, it is because we really need. Hope you can understand.

Copy link
Author

Choose a reason for hiding this comment

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

Maybe it is required to make this feature works?
Or if have any other methods that have smaller permission and won't interfere this feature?
rustdesk/rustdesk#7717

Why would a feature related to cursors require executing arbitrary commands on host?

Ignore this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok that makes sense. Thanks. However I see fallbacks everywhere in case loginctl does not work. Is that not enough?

Eg here https://github.com/rustdesk/rustdesk/blob/9827c7651442800f47428087903d52da8cb8c2c8/libs/hbb_common/src/platform/linux.rs#L115-L123

"--socket=fallback-x11",
"--socket=wayland",
"--share=network",
"--filesystem=home",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need home?

Choose a reason for hiding this comment

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

They are Remote Desktop software, transfer files is needed for end users

Copy link
Author

Choose a reason for hiding this comment

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

File transfer

Copy link
Contributor

@bbhtt bbhtt May 12, 2024

Choose a reason for hiding this comment

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

Does it support filechooser portals? Do you need permanent access to a specific directory?

Copy link
Author

Choose a reason for hiding this comment

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

Why does AnyDesk flathub have home? But we can not have

Copy link
Contributor

Choose a reason for hiding this comment

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

This does not answer anything... Each application is separate just because someone else uses it doesn't mean you have to as well.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I give up. Thanks for your time.

Copy link
Author

Choose a reason for hiding this comment

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

This does not answer anything... Each application is separate just because someone else uses it doesn't mean you have to as well.

I do not think so, you answer nothing either.

Copy link
Author

@rustdesk rustdesk May 12, 2024

Choose a reason for hiding this comment

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

It is ridiculous that the previous reviewer agreed with the home permission of remote desktop software, now a new reviewer does not agree with it. Though all of you are volunteers or students, you should do things like this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

what's ridiculous is that

  1. you think you know better
  2. it's not about what the app does but how it is written
  3. until recently filesystem permissions were not gated so they could have been changed
  4. talking about double standard, you are making one right now by thinking you know better while not even being able to justify the choices made when asked.

Locking this thread now since the PR is closed.

com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
com.rustdesk.RustDesk.metainfo.xml Outdated Show resolved Hide resolved
flathub.json Outdated Show resolved Hide resolved
@bbhtt bbhtt added the awaiting-changes Pull request waiting for changes from author label May 11, 2024
@hfiguiere
Copy link
Contributor

You have not:

Please describe your application briefly.

flathub.json Outdated Show resolved Hide resolved
@rustdesk
Copy link
Author

rustdesk commented May 12, 2024

@xlionjuan do you have interest to take over the job of merging RustDesk to flathub? I can add you as maintainer of rustdesk/flathub repo.

com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
com.rustdesk.RustDesk.json Outdated Show resolved Hide resolved
rustdesk and others added 2 commits May 12, 2024 13:42
Co-authored-by: bbhtt <bbhtt.zn0i8@slmail.me>
@xlionjuan
Copy link

@xlionjuan do you have interest to take over the job of merging RustDesk to flathub? I can add you as maintainer of rustdesk/flathub repo.

@rustdesk Thanks for your recognition, but sorry, I might not be able to maintain it because I'm not familiar with this either.

"bsdtar -zxvf rustdesk.deb",
"tar -xvf ./data.tar.xz",
"rm -rf usr/share/icons/hicolor/scalable/",
"cp -r usr/* /app/",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the metainfo info being installed. You removed it from here, but it is not present in the .deb

Copy link
Author

@rustdesk rustdesk May 12, 2024

Choose a reason for hiding this comment

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

How do you know not installed? I did see installed from log of flatpak-build.

Copy link
Contributor

Choose a reason for hiding this comment

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

How can it be installed when it is not in the deb and nowhere in the manifest as a source?

Copy link
Author

Choose a reason for hiding this comment

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

image

Copy link
Author

Choose a reason for hiding this comment

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

It is closed, you do not need to review any more. #5233 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Since you seem to know better, neither of the line above show the appstream file. So yeah the appstream file isn't installed.

@rustdesk rustdesk closed this May 12, 2024
@rustdesk
Copy link
Author

rustdesk commented May 12, 2024

Since we can not reach consensus with reviewers about the permissions, we decide to give up uploading RustDesk to FlatHub. Please use Flatpak from our github release downloads. Thanks for your understanding.

https://github.com/rustdesk/rustdesk/releases/latest

@flathub flathub locked as resolved and limited conversation to collaborators May 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting-changes Pull request waiting for changes from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants