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

Flathub Distribution #323

Closed
NNowakowski opened this issue Aug 19, 2020 · 30 comments
Closed

Flathub Distribution #323

NNowakowski opened this issue Aug 19, 2020 · 30 comments

Comments

@NNowakowski
Copy link

Describe the feature or problem you’d like to solve

While having solutions for multiple linux distributions is great, and AppImages also being offered, I'd like to have a Flatpak version offered via Flathub aswell. They integrate more seamlessly into the system and are easy to update out-of-the-box.

Proposed solution

Making the software available on Flathub for easy access, you don't even need to know about this repository at all, to find it. Without digging through the internet I wouldn't even have found this repository since it's nowhere mentioned on the official Github Desktop download page.

Additional context

-/-

@charitarthchugh
Copy link

I would agree. Also, distributing via packagecloud is expensive as @shiftkey has already stated.

@dnohales
Copy link

dnohales commented Nov 8, 2020

I'm working actively on that here: https://github.com/dnohales/com.github.Desktop

Right now it builds but it doesn't run, it's also missing a desktop entry, AppData, icon, which I plan to get it done once I get it ran stably and see what has to be added in the repo itself.

Once that's done we can request an App submission to Flathub. Of course if @shiftkey and GitHub team agree with that.

@Lunarequest
Copy link

is there a update on this, I would like to work on it if there has been no progress?

@dnohales
Copy link

dnohales commented May 5, 2021

Hello @AdvaithM, sorry, sadly I stopped working on this, I had a successful build but I wasn't able to run it, I was getting inspired by the VSCode Flatpak code, feel free to test and fork my repo, if you do more progress I'll remove mine to avoid confusion.

@Lunarequest
Copy link

Sure I'll take a shot and see if I can get further

@Lunarequest
Copy link

I've got a working build and the app to runs, oauth is never received mostly due to permissions. I'm not to sure how to fix it but I'll keep looking into it

@TommyTran732
Copy link

I've got a working build and the app to runs, oauth is never received mostly due to permissions. I'm not to sure how to fix it but I'll keep looking into it

Do you have any updates on this? It would be great if we can have a flatpak package :)

@Lunarequest
Copy link

not really sure how to fix the sign in stuff. it would be pretty helpful if someone could explain how the sign in stuff worked and how I could debug this

@Lunarequest
Copy link

also the latest commits break offline building, someone with a lot more knowledge in js should probably take a shot at this.

@Lunarequest
Copy link

@shiftkey is node-detect-arm64-translation a hard dep on linux or would it be possible to remove the dep via a patch? asking since currently its very problematic with offline builds

@shiftkey
Copy link
Owner

shiftkey commented Jul 6, 2021

@AdvaithM I think it's being used for macOS and Windows arm64 detection, but given those are not supported currently on Linux (see #251) then patching them away for now is 👍🏻 with me.

@Lunarequest
Copy link

so looking through the package.json it isn't listed as an explicit dependency, I'm not really sure which package pulls it in

@shiftkey
Copy link
Owner

shiftkey commented Jul 7, 2021

@AdvaithM I think it's in app/package.json as a runtime dependency:

"detect-arm64-translation": "https://github.com/desktop/node-detect-arm64-translation#v1.0.4",

Patching that out might also require patching this function:

import { App } from 'electron'
import { isRunningUnderARM64Translation } from 'detect-arm64-translation'
export type Architecture = 'x64' | 'arm64' | 'x64-emulated'
/**
* Returns the architecture of the build currently running, which could be
* either x64 or arm64. Additionally, it could also be x64-emulated in those
* arm64 devices with the ability to emulate x64 binaries (like macOS using
* Rosetta).
*/
export function getArchitecture(app: App): Architecture {
if (
app.runningUnderRosettaTranslation === true ||
isRunningUnderARM64Translation() === true
) {
return 'x64-emulated'
}
return process.arch === 'arm64' ? 'arm64' : 'x64'
}

@Lunarequest
Copy link

I believe for now we can just check the other arches and assume we aren't running under a translation layer

@Lunarequest
Copy link

Lunarequest commented Jul 7, 2021

new issue @shiftkey dugit doesn't seem to respect the cache dir env var

error /run/build/github-desktop/app/node_modules/@shiftkey/dugite: Command failed.
Exit code: 1
Command: node ./script/download-git.js
Arguments: 
Directory: /run/build/github-desktop/app/node_modules/@shiftkey/dugite
Output:
Downloading Git from: https://github.com/desktop/dugite-native/releases/download/v2.29.3-2/dugite-native-v2.29.3-3d467be-ubuntu.tar.gz
Error raised while downloading https://github.com/desktop/dugite-native/releases/download/v2.29.3-2/dugite-native-v2.29.3-3d467be-ubuntu.tar.gz GotError [RequestError]: getaddrinfo EAI_AGAIN github.com
    at ClientRequest.<anonymous> (/run/build/github-desktop/app/node_modules/got/source/request-as-event-emitter.js:178:14)
    at Object.onceWrapper (events.js:482:26)
    at ClientRequest.emit (events.js:387:35)
    at ClientRequest.origin.emit (/run/build/github-desktop/app/node_modules/@szmarczak/http-timer/source/index.js:37:11)
    at TLSSocket.socketErrorListener (_http_client.js:475:9)
    at TLSSocket.emit (events.js:375:28)
    at emitErrorNT (internal/streams/destroy.js:106:8)
    at emitErrorCloseNT (internal/streams/destroy.js:74:3)
    at processTicksAndRejections (internal/process/task_queues.js:82:21) {
  code: 'EAI_AGAIN',
  host: 'github.com',
  hostname: 'github.com',
  method: 'GET',
  path: '/desktop/dugite-native/releases/download/v2.29.3-2/dugite-native-v2.29.3-3d467be-ubuntu.tar.gz',
  socketPath: undefined,
  protocol: 'https:',
  url: 'https://github.com/desktop/dugite-native/releases/download/v2.29.3-2/dugite-native-v2.29.3-3d467be-ubuntu.tar.gz',
  gotOptions: {
    path: '/desktop/dugite-native/releases/download/v2.29.3-2/dugite-native-v2.29.3-3d467be-ubuntu.tar.gz',
    protocol: 'https:',
    slashes: true,
    auth: null,
    host: 'github.com',
    port: null,
    hostname: 'github.com',
    hash: null,
    search: null,
    query: null,
    pathname: '/desktop/dugite-native/releases/download/v2.29.3-2/dugite-native-v2.29.3-3d467be-ubuntu.tar.gz',
    href: 'https://github.com/desktop/dugite-native/releases/download/v2.29.3-2/dugite-native-v2.29.3-3d467be-ubuntu.tar.gz',
    retry: {
      retries: [Function (anonymous)],
      methods: [Set],
      statusCodes: [Set],
      errorCodes: [Set]
    },
    headers: {
      'user-agent': 'dugite',
      accept: 'application/octet-stream',
      'accept-encoding': 'gzip, deflate'
    },
    hooks: {
      beforeRequest: [],
      beforeRedirect: [],
      beforeRetry: [],
      afterResponse: [],
      beforeError: [],
      init: []
    },
    decompress: true,
    throwHttpErrors: true,
    followRedirect: true,
    stream: true,
    form: false,
    json: false,
    cache: false,
    useElectronNet: false,
    secureProtocol: 'TLSv1_2_method',




verbose 114.778105996 Error: Command failed with exit code 1.
    at /run/build/github-desktop/vendor/yarn-1.21.1.js:36439:32
    at Generator.throw (<anonymous>)
    at step (/run/build/github-desktop/vendor/yarn-1.21.1.js:310:30)
    at /run/build/github-desktop/vendor/yarn-1.21.1.js:323:13
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
Error: module github-desktop: Child process exited with code 1

no idea why but it seems to work now.

@shiftkey
Copy link
Owner

shiftkey commented Jul 7, 2021

no idea why but it seems to work now.

I couldn't spot anything obviously wrong with what's currently there. Here's the place where we validate the temp file, and either unpack the valid archive, or remove and try to download it again:

https://github.com/desktop/dugite/blob/85a76d38ce85a3e9fffa1fabd4cfc6a995695b4c/script/download-git.js#L139-L148

And here's where we derive config.tempFile which leverages DUGITE_CACHE_DIR:

https://github.com/desktop/dugite/blob/85a76d38ce85a3e9fffa1fabd4cfc6a995695b4c/script/config.js#L54-L64

My guess is that the cached file didn't match what was expected, so it proceeded to try and download the latest version.

@Lunarequest
Copy link

currently I have a hack patch that removes detect-arm64-translation, Ideally we should get upstream to make a var like Dugite

@Lunarequest
Copy link

@shiftkey, I have a working build now, but when you try and login in it just infinitely loads. console gives me this out put
image
no idea whats wrong, or how to fix it.

@shiftkey
Copy link
Owner

shiftkey commented Jul 8, 2021

@AdvaithM

Ideally we should get upstream to make a var like Dugite

Let's move this discussion to desktop/node-detect-arm64-translation#1 as there's some context with how the package is being used that might make this different to dugite.

no idea whats wrong, or how to fix it.

At this stage it seems like the app is trying to start the OAuth flow and open the browser - not sure if flatpack has restrictions on programs launching the browser but that button is initiating this flow (based on the [SignInStore] initializing OAuth flow message):

public async authenticateWithBrowser(): Promise<void> {
const currentState = this.state
if (!currentState || currentState.kind !== SignInStep.Authentication) {
const stepText = currentState ? currentState.kind : 'null'
return fatalError(
`Sign in step '${stepText}' not compatible with browser authentication`
)
}
this.setState({ ...currentState, loading: true })
let account: Account
try {
log.info('[SignInStore] initializing OAuth flow')
account = await askUserToOAuth(currentState.endpoint)
log.info('[SignInStore] account resolved')
} catch (e) {
log.info('[SignInStore] error with OAuth flow', e)
this.setState({ ...currentState, error: e, loading: false })
return
}
if (!this.state || this.state.kind !== SignInStep.Authentication) {
// Looks like the sign in flow has been aborted
return
}
this.emitAuthenticate(account, SignInMethod.Web)
this.setState({ kind: SignInStep.Success })
}

The next step from that is to invoke the browser, which is this bit:

export function askUserToOAuth(endpoint: string) {
// Disable the lint warning since we're storing the `resolve` and `reject`
// functions.
// tslint:disable-next-line:promise-must-complete
return new Promise<Account>((resolve, reject) => {
oauthState = { state: uuid(), endpoint, resolve, reject }
const oauthURL = getOAuthAuthorizationURL(endpoint, oauthState.state)
shell.openExternal(oauthURL)
})
}

I'm curious if Flatpak has restrictions on apps launching http or https URLs - we're using Electron's shell.openExternal here to start the OAuth flow but perhaps that's where we're stuck.

@Lunarequest
Copy link

I'm not to sure why this doesn't work. I'll need to look into it

@Croydon
Copy link

Croydon commented Jul 8, 2021

As a quick note, I gave it a short try too a while ago: https://github.com/Croydon/com.github.Desktop

For me, it worked that GitHub Desktop is opening the authorization page on GitHub in my browser, but after authorizing it did not successfully redirect me back for some reason that I don't know (I also don't have any logs for potential errors unfortunately).

The mime types are in the .desktop file, not sure if this is supposed to be enough https://github.com/Croydon/com.github.Desktop/blob/7166b4091f0a09dac60c45fd848ff0246b6c2d1f/com.github.Desktop.desktop#L9

The end result is that the app ultimately can't login.

@Lunarequest
Copy link

I fixed the issue with call back. I'm now getting user interaction errors. any ideas?
image

@Lunarequest
Copy link

running this on a system without github-desktop installed before had it works, I'm not sure why. maybe uninstalling github-desktop doesn't remove all of the files?

@Lunarequest
Copy link

opening a new issue to track what works, and possible fixes.

@Croydon
Copy link

Croydon commented Jul 12, 2021

@AdvaithM @shiftkey I think it is better to add it as com.github.Desktop from the official repository and add the Linux changes as a patch. On Flathub there are already several GitHub/Microsoft applications without official support. Same for other applications from other vendors.

If it is getting added as io.github.shiftey.Desktop migration later will be harder and probably not without friction/confusion from users.

What do you think?

@shiftkey
Copy link
Owner

@Croydon @AdvaithM I have no strong opinions on the package name

@Lunarequest
Copy link

Lunarequest commented Jul 13, 2021

Flathub wanted the name to be changed to show it's a fork flathub/flathub#2417 (comment)

@Croydon
Copy link

Croydon commented Jul 13, 2021

Flathub wanted the name to be changed to show it's a fork flathub/flathub#2417 (comment)

Yes, because you have downloaded the source directly from this fork. If you would first download the official source code and then apply the changes in this fork as a patch we can work around that and still fulfill all conventions and rules I guess.

@Lunarequest
Copy link

Lunarequest commented Jul 13, 2021

Flathub wanted the name to be changed to show it's a fork flathub/flathub#2417 (comment)

Yes, because you have downloaded the source directly from this fork. If you would first download the official source code and then apply the changes in this fork as a patch we can work around that and still fulfill all conventions and rules I guess.

it would create a massive unmaintainable patch not worth the cost of maintenance in the slightest. flatpak does have ways to e-o-l one app and point to a replacement. we could also just change the name during an update if upstream does deiced to support linux. Another issue is that this repo contains many files not present upstream which would make it even more difficult to maintain the patch. Every single file not present upstream would have to be created before the patch is applied which is would be very difficult to do constantly.

It is hard to maintain patches/patch sets which is why as maintainers of downstream distributions we try and upstream as much as possible. I personally do not see a patch set based on this repo being without missing releases by weeks or even months.

@Lunarequest
Copy link

This can be closed now, github desktop in now on flathub https://github.com/flathub/io.github.shiftey.Desktop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants