introduce abstractions to make Flatpak integration easier by shiftkey · Pull Request #555 · shiftkey/desktop

shiftkey · 2021-07-13T13:54:44Z

Related to #554 but enables us to better combine regular usage as well as integration with the Flatpak host

I've moved the pathExists and spawn usage into a standalone module, so we can keep this platform-specific code isolated from the rest of this app. Not sure how feasible this is, but I hope it makes things easier to reason about if they're funnelling through the same functions (which we can adapt based on the requirements).

cc @AdvaithM for thoughts

I'll add some notes in the PR to help guide the discussion about these changes

TODO

get feedback about changes
add new vscodium path
ensure CI passes
test regular usage of app in development mode to ensure no regressions

shiftkey · 2021-07-13T13:56:09Z

app/src/lib/editors/found-editor.ts

  readonly editor: T
  readonly path: string
+  /**
+   * Indicate to Desktop to launch the editor with the `shell: true` option included.


🎨 I plan to upstream this change as this context was helpful and perhaps overlooked at the time

shiftkey · 2021-07-13T13:56:48Z

app/src/lib/helpers/linux.ts

+} from 'child_process'
+
+export function isFlatpakBuild() {
+  return __LINUX__ && process.env.FLATPAK_HOST === '1'


I started with a helper method to detect whether we're in a flatpak build. This is now all internal, so I can drop the export above if we're happy with the other changes.

shiftkey · 2021-07-13T13:59:53Z

app/src/lib/helpers/linux.ts

+ */
+export async function pathExists(path: string): Promise<boolean> {
+  if (isFlatpakBuild()) {
+    path = convertToFlatpakPath(path)


🤔 as I understand it, we want to convert program paths before checking if we're running in a flatpak context as they exist in a special location, but we don't need to do this when we want to launch the program - flatpak-spawn will do that itself as long as we call it with --host.

Let me know if this logic is incorrect, or if I've misunderstood.

yup this is correct. the path outside the container should be real but detection for binaries in /usr should be pointed to /var/run/host

What about for other special directories we're using for editors:

/opt

/snap

/var/lib/flatpak (launching another flatpak program)

Should those also go through this conversion process before we try and open them with flatpak-spawn?

I think /var/lib/flatpak might be blocked by flatpak not sure(I'll check and update this), /snap is one that won't work since this dir won't be mounted

yeah /var/lib/flatpak is blocked

👍🏻 I'm assuming that the try-catch below will ensure these errors will be handled and swallowed by the app, so I'm going to try and avoid getting too complex with the logic in here for now for paths that we know are inaccessible in the flatpak context.

shiftkey · 2021-07-13T14:01:22Z

app/src/lib/helpers/linux.ts

+  options?: SpawnOptionsWithoutStdio
+): ChildProcess {
+  if (isFlatpakBuild()) {
+    return spawn('flatpak-spawn', ['--host', path, ...args], options)


🤔 I think this is the convention for mapping the original command (and parameters) to work with flatpak-spawn, but now that it's all centralized we can tweak this to suit.

shiftkey · 2021-07-13T14:02:54Z

app/src/lib/helpers/linux.ts

+ *
+ */
+function convertToFlatpakPath(path: string) {
+  return join('/var/run/host', path)


🤔 if there are cases where we shouldn't be applying this pattern let me know, ideally with the expected inputs and resulting paths. Having to adapt this behaviour based on the path type might make this a bit more complex, but I still think this approach is feasible...

/opt/ is the only path we don't need to apply this logic too

Oh cool, you answered the question I just posted here #555 (comment) - thanks!

Lunarequest · 2021-07-13T14:30:01Z

unfortunately this is not able to detect binaries properly, also please add /usr/share/vscodium-bin/bin/codium to the vscodium detection path, this gets around the fact /usr/bin/codium is a symlink and this symlink is broken in /var/run

Lunarequest · 2021-07-13T15:02:42Z

Konsole is detected but vscodium isn't due to the issue mentioned here #555 (comment), you have to patch the launch code here

desktop/app/src/lib/shells/shared.ts

Line 84 in 2846fd2

const exists = await pathExists(shell.path)

and over here

desktop/app/src/lib/editors/linux.ts

Line 37 in 2846fd2

paths: ['/usr/bin/codium', '/var/lib/flatpak/app/com.vscodium.codium'],

add /usr/share/vscodium-bin/bin/codium so vscodium detection works as intended

Lunarequest · 2021-07-13T15:34:27Z

#556 fixes both issued mentioned above

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

Lunarequest · 2021-07-14T01:41:57Z

Editor detection is still broken, shell's are detected and launched with zero issues

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added 6 commits July 13, 2021 10:18

add helper function for detecting we're in a flatpak build

4573cc2

add docs for the useShell parameter

b6f6e3a

add helpers for flatpak interactions with shells

da9bf48

update shells to use new API on Linux

b435a98

add helper method for spawning editor

85a28b8

update editor to use new abstractions on Linux

a3e2d0b

shiftkey commented Jul 13, 2021

View reviewed changes

shiftkey marked this pull request as ready for review July 13, 2021 14:15

shiftkey added 2 commits July 13, 2021 11:37

avoid conversion for /opt paths

f46a87b

add tests for specific cases with convertToFlatpakPath

2846fd2

shiftkey mentioned this pull request Jul 13, 2021

io.github.shiftey.Desktop flathub/flathub#2417

Closed

7 tasks

shiftkey and others added 3 commits July 13, 2021 17:08

move tests into platform-specific global

abc817f

add new path to vscodium

d9793be

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

ensure pathExists for shell is flatpak-aware

3bbef90

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey force-pushed the flatpak-abstractions branch from 722307d to 3bbef90 Compare July 13, 2021 20:21

shiftkey mentioned this pull request Jul 13, 2021

fixed shell launching and vscodium detection #556

Closed

add stub tests for mac/windows to get CI passing

bdbb31a

This comment has been minimized.

Sign in to view

Lunarequest added 2 commits July 14, 2021 10:39

fixed code editor detection

5095b70

code editor launched,but path appends " at the end

06c7481

shiftkey merged commit 478789f into linux Jul 14, 2021

shiftkey deleted the flatpak-abstractions branch July 14, 2021 14:53

shiftkey added a commit that referenced this pull request Jul 14, 2021

introduce abstractions to make Flatpak integration easier (#555)

88b7ad2

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Jul 19, 2021

introduce abstractions to make Flatpak integration easier (#555)

24f7c77

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Jul 22, 2021

introduce abstractions to make Flatpak integration easier (#555)

c0da2ff

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Feb 22, 2022

introduce abstractions to make Flatpak integration easier (#555)

707989c

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Feb 22, 2022

introduce abstractions to make Flatpak integration easier (#555)

221a700

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Feb 22, 2022

introduce abstractions to make Flatpak integration easier (#555)

8caa7ee

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Feb 22, 2022

introduce abstractions to make Flatpak integration easier (#555)

cf5581e

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Feb 22, 2022

introduce abstractions to make Flatpak integration easier (#555)

90c9a7c

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Feb 22, 2022

introduce abstractions to make Flatpak integration easier (#555)

40d97b5

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Feb 22, 2022

introduce abstractions to make Flatpak integration easier (#555)

aea3861

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Mar 12, 2022

introduce abstractions to make Flatpak integration easier (#555)

0c6a44c

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 3, 2022

introduce abstractions to make Flatpak integration easier (#555)

cda1a3a

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 3, 2022

introduce abstractions to make Flatpak integration easier (#555)

96022d4

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 3, 2022

introduce abstractions to make Flatpak integration easier (#555)

fb2676c

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 3, 2022

introduce abstractions to make Flatpak integration easier (#555)

6beff94

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 3, 2022

introduce abstractions to make Flatpak integration easier (#555)

dd10560

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 3, 2022

introduce abstractions to make Flatpak integration easier (#555)

96d6029

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 3, 2022

introduce abstractions to make Flatpak integration easier (#555)

1db1085

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 8, 2022

introduce abstractions to make Flatpak integration easier (#555)

1fba947

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 15, 2022

introduce abstractions to make Flatpak integration easier (#555)

f2f50ad

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 15, 2022

introduce abstractions to make Flatpak integration easier (#555)

8b86c25

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 15, 2022

introduce abstractions to make Flatpak integration easier (#555)

da04152

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 15, 2022

introduce abstractions to make Flatpak integration easier (#555)

9769302

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 15, 2022

introduce abstractions to make Flatpak integration easier (#555)

9524df0

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 21, 2022

introduce abstractions to make Flatpak integration easier (#555)

1057e87

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 21, 2022

introduce abstractions to make Flatpak integration easier (#555)

e082263

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 21, 2022

introduce abstractions to make Flatpak integration easier (#555)

ee11376

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

shiftkey added a commit that referenced this pull request Apr 27, 2022

introduce abstractions to make Flatpak integration easier (#555)

3077eb0

Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com>

Conversation

shiftkey commented Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lunarequest commented Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lunarequest commented Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lunarequest commented Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lunarequest commented Jul 14, 2021

Uh oh!

This comment has been minimized.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shiftkey commented Jul 13, 2021 •

edited

Loading

Lunarequest commented Jul 13, 2021 •

edited

Loading

Lunarequest commented Jul 13, 2021 •

edited

Loading

Lunarequest commented Jul 13, 2021 •

edited

Loading