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

Create org.remmina.Remmina #307

Closed
wants to merge 7 commits into from

Conversation

amtlib-dot-dll
Copy link

@amtlib-dot-dll amtlib-dot-dll commented Mar 19, 2018

cc @larchunix

Copied from https://github.com/FreeRDP/Remmina/tree/next/flatpak. I am not the orginal author. I just want to use the flatpak version without compile it by myself 😆

},
"command": "remmina",
"rename-icon": "remmina",
"desktop-file-name-prefix": "(Flatpak) ",
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary

Copy link
Member

Choose a reason for hiding this comment

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

@amtlib-dot-dll This one =)

Copy link
Author

Choose a reason for hiding this comment

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

amtlib-dot-dll@aecd397 Thank you for notifying 😂

"/share/pkgconfig"
],
"finish-args": [
"--own-name=org.remmina.Remmina",
Copy link
Member

Choose a reason for hiding this comment

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

Not needed

/* Appindicator */
"--talk-name=org.kde.StatusNotifierWatcher",
/* Query GNOME Shell version (to display systray icon if supported) */
"--talk-name=org.gnome.Shell",
Copy link
Member

Choose a reason for hiding this comment

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

This is an upstream bug, but this is completely the wrong way to handle this.

Copy link
Member

Choose a reason for hiding this comment

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

I opened an upstream bug, please add it as a comment: https://github.com/FreeRDP/Remmina/issues/1536

"name": "nxproxy",
"buildsystem": "simple",
"build-commands": [
"make CONFIGURE='./configure --prefix=/app'",
Copy link
Member

Choose a reason for hiding this comment

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

I don't know why this is the simple build system.

},
{
"name": "freerdp",
"buildsystem": "cmake",
Copy link
Member

Choose a reason for hiding this comment

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

cmake-ninja

Copy link
Author

@amtlib-dot-dll amtlib-dot-dll Mar 19, 2018

Choose a reason for hiding this comment

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

Thank you for your review, by the way, what is the difference between cmake and cmake-ninja? The documentation manual is very unclear about that, and I am not familiar with C so reading the source code gives me no clue 🤔

Copy link
Member

Choose a reason for hiding this comment

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

cmake generates Makefiles, ninja is simply faster than make so when it works use it.

Copy link
Author

Choose a reason for hiding this comment

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

Alright 🤦‍♂️

"name": "lz4",
"buildsystem": "simple",
"build-commands": [
"make -C lib lib",
Copy link
Member

Choose a reason for hiding this comment

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

No need for simple build system:

{
  "subdir": "lib",
  "no-autogen": true,
  "make-args": ["PREFIX=/app", "lib"],
}

},
{
"name": "libssh",
"buildsystem": "cmake",
Copy link
Member

Choose a reason for hiding this comment

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

cmake-ninja

},
{
"name": "remmina",
"buildsystem": "cmake",
Copy link
Member

Choose a reason for hiding this comment

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

cmake-ninja

"name": "remmina",
"buildsystem": "cmake",
"config-opts": [
"-DCMAKE_INSTALL_LIBDIR:PATH=/app/lib",
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed.

{
"name": "remmina",
"buildsystem": "cmake",
"config-opts": [
Copy link
Member

Choose a reason for hiding this comment

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

-DCMAKE_BUILD_TYPE=Release

@nedrichards
Copy link
Member

bot, build org.remmina.Remmina

@nedrichards
Copy link
Member

Builds fail with appdata not available https://flathub.org/builds/#/builders/2/builds/221

@larchunix
Copy link

@amtlib-dot-dll Thanks for pushing my flatpak manifest here.

I opened a PR on Remmina to fix most of the issues raised by @TingPing (FreeRDP/Remmina#1526).

The error raised by @nedrichards has been fixed upstream by FreeRDP/Remmina@a860ca2.

@nedrichards
Copy link
Member

@larchunix that's awesome! do you know when the next upstream release that includes that will be?

@antenore
Copy link

You guys are amazing! Thanks a lot!!!!!!!!

I don't know when we will release a new version, I've just merged these modifications upstream in our main branch (named next)

If you need a release to go ahead, we can do it I think.

What do you think @giox069 ?

@amtlib-dot-dll
Copy link
Author

amtlib-dot-dll commented Mar 21, 2018

@larchunix @antenore @giox069

I have an idea. FreeRDP is well-known, and more famous than Remmina, so if we change the app-id from org.remmina.Remmina to org.freerdp.Remmina, people who use search engines to look for FreeRDP will know about Remmina, especially when they search for FreeRDP in the Flathub directory. What do you think?

I am not a developer of Remmina, so I should ask you first 😄

@antenore
Copy link

Personally I have nothing to objects, I think it's a good idea, moreover we belong to the FreeRDP organisation.

@antenore
Copy link

Finally I'm not sure anymore. The Remmina dbus id is org.remmina.Remmina and it's better to stay in-line with the dbus name.

As soon as we will endorse flatpak, I'm sure we won't need advertising.

So far flatpack and the flathub community looks amazing :-)

Thanks for supporting us!

@nedrichards
Copy link
Member

nedrichards commented Mar 21, 2018 via email

@antenore
Copy link

If it doesn't hurt we can change it

@amtlib-dot-dll
Copy link
Author

Searching may also be something like this flatpak remote-ls flathub | grep -i freerdp @nedrichards

@TingPing
Copy link
Member

TingPing commented Mar 21, 2018

The best app-id is a discussion to have with upstream as that is where these appdata files should go.

EDIT: I realize now @antenore is from upstream.

@TingPing
Copy link
Member

TingPing commented Mar 21, 2018

Searching may also be something like this flatpak remote-ls flathub | grep -i freerdp

Well that is just wrong: flatpak search freerdp.

@nedrichards
Copy link
Member

bot, build org.remmina.Remmina

@antenore
Copy link

@nedrichards
To resume..

  1. Do you need other changes from our side?
  2. Do you need an official release?

Thanks

@nedrichards
Copy link
Member

@antenore a release is always helpful, but so long as this is all merged upstream you can attach a patch here since it's not a functional change or user visible bug fix. I don't want to cause you extra work that's unnecessary.

@antenore
Copy link

Thanks @nedrichards we are preparing a new release, but I don't know exactly when will be available.
I'm waiting with impatience to see Remmina in Flathub, so whatever you need I'm available without any issues.

@amtlib-dot-dll
Copy link
Author

Any more news guys? 😀 :

@nedrichards
Copy link
Member

I thought we were waiting for an upstream release or for the changes accepted upstream to be added as a patch to this version (until the next release comes out)

@antenore
Copy link

antenore commented Apr 1, 2018

@nedrichards we prefer to don't release right now as we are still working hard for the next one.

It's not clear for me, sorry, what I'm supposed to do to accept the patches.
As soon as you'll tell me I'll do what is needed.
Thanks!

@nedrichards
Copy link
Member

Here's an example of what I mean by a patch. https://github.com/flathub/org.pitivi.Pitivi/blob/master/org.pitivi.Pitivi.json#L138

Flatpak-builder can apply patch files to the source code it uses for each module. You can take the upstream commits, save them as patch files and use this sort of syntax to apply and test them.

@antenore
Copy link

antenore commented Apr 2, 2018

@nedrichards I'd like to just propose our json file (from upstream) as the one to use for the flathub builds

https://github.com/FreeRDP/Remmina/blob/next/flatpak/org.remmina.Remmina.json

As I don't have rights to modify this PR I can only submit a new one or attach a patch file of the json file.

Regarding the commit to use (https://github.com/FreeRDP/Remmina/blob/next/flatpak/org.remmina.Remmina.json#L481), in our json file we use directly the main branch (named 'next').

The alternatives are to use the releases, or specific commits as @amtlib-dot-dll has done in this PR.

Our (as mainstream devs) preferred choice would be to use 'next'. We do a lo of testing before to push the code in there... Moreover @larchunix recently has fixed some small annoyances.

If you want I can just submit a new PR with the updated flatpak based on your input, but I need to know exactly what you prefer.

In the (near) future, we will submit PRs to improve it further also based on the @TingPing feedback.

Thanks!

@amtlib-dot-dll
Copy link
Author

@antenore Hi, I sent you an invitation so you can modify this PR via this branch, however I remembered that according to App Requirements · flathub/flathub Wiki

Manifests should therefore refer to tarballs or git tags and not the tip of a branch.

I am afraid that merely the main branch won't be accepted

@antenore
Copy link

antenore commented Apr 3, 2018

@amtlib-dot-dll thanks! We're discussing internally to do a new release

@barthalion
Copy link
Member

Some specific commit will also be fine as long as you are willing to support that in case of issues reported by upstream. Branch alone won't do as we want builds to be reproducible.

@nedrichards
Copy link
Member

bot, build org.remmina.Remmina

@antenore
Copy link

antenore commented Apr 4, 2018

We have found a couple of bugs that impact old distros, like Ubuntu 14.04.
We are working on these bugs ans we will re-tag a new commit or do a new release.

Sorry for the inconvenient.

@nedrichards
Copy link
Member

No worries, power of upstream!

@amtlib-dot-dll
Copy link
Author

New tag coming, thanks to @antenore and other upstream Remmina developers!

@nedrichards May you trigger another bot build? Thanks

@TingPing
Copy link
Member

bot, build org.remmina.Remmina

nedrichards pushed a commit to flathub/org.remmina.Remmina that referenced this pull request Apr 10, 2018
nedrichards pushed a commit to flathub/org.remmina.Remmina that referenced this pull request Apr 10, 2018
@nedrichards
Copy link
Member

Repository has been created: https://github.com/flathub/org.remmina.Remmina

@nedrichards
Copy link
Member

@antenore let us know the github usernames of any other upstream developers who we should add to the new repo.

@antenore
Copy link

@nedrichards Thanks!

Those in CC to this message

cc @giox069 @larchunix @amtlib-dot-dll

@TingPing
Copy link
Member

@antenore Added.

@antenore
Copy link

antenore commented Dec 2, 2022

@nedrichards @TingPing sorry to comment on a closed PR

How to ask for membership changes?

I'd like to remove larchunix and add @myheroyuki

Thanks!

@TingPing
Copy link
Member

TingPing commented Dec 2, 2022

@antenore Done. An invitation was sent.

@antenore
Copy link

antenore commented Dec 2, 2022

Thank you @TingPing !!!

@myheroyuki
Copy link

@TingPing Thank you, just accepted

jbruechert pushed a commit to jbruechert/flathub that referenced this pull request Jul 12, 2024
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.

7 participants