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

Support building Flatpak application bundles #16169

Merged
merged 1 commit into from
Dec 14, 2016
Merged

Support building Flatpak application bundles #16169

merged 1 commit into from
Dec 14, 2016

Conversation

aperezdc
Copy link
Contributor

@aperezdc aperezdc commented Nov 28, 2016

This adds support for building Flatpak application bundles, which should be runnable on any Linux distribution with Flatpak support.

New Gulp tasks are added to prepare a directory with the needed files (vscode-linux-${arch}-prepare-flatpak), assembling the Flatpak bundles (vscode-linux-${arch}-flatpak), and for cleaning the build directories (clean-vscode-linux-${arch}-flatpak). This mimics how the Debian and RPM package creation works. Hence, building an application bundle is done with:

$ gulp vscode-linux-x64-flatpak
[...]
$ ls *.flatpak
com.visualstudio.code.oss-x86_64.flatpak

Installing and running the application is achieved with:

$ flatpak --user install --bundle com.visualstudio.code.oss-x86_64.flatpak
$ flatpak run com.visualstudio.code.oss

(Removing --user would install the application system-wide.)

The org.freedesktop.Platform runtime needs to be installed for running the application, and for making builds the org.freedesktop.Sdk runtime is needed as well. There are installation instructions in the Flatpak website; the short version is:

wget https://sdk.gnome.org/keys/gnome-sdk.gpg
flatpak --user remote-add --gpg-import=gnome-sdk.gpg gnome https://sdk.gnome.org/repo/
flatpak --user install org.freedesktop.Platform 1.4
flatpak --user install org.freedesktop.Sdk 1.4

The flatpak-bundler development dependency provides an asynchronous API to invoke flatpak-builder. The gulp-image-resize module is used to
incorporate scaling of the application icon to multiple sizes, and uses ImageMagick (or GraphicsMagick) under the hood, which is commonly available on most GNU/Linux distributions.

Instead of building the Electron dependencies, this reuses the io.atom.electron.BaseApp (source) bundle as base application. The contents of the base application are imported at build time, and then Electron and the prepared files added on top. The prebuilt base application will be fetched automatically using the URL specified in the baseFlatpakref attribute, and it is kindly hosted at Amazon S3 by Endless Computers.

Merging this would tackle issue #7112.

@msftclas
Copy link

Hi @aperezdc, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

],
finishArgs: [
'--share=ipc', '--socket=x11', // Allow showing X11 windows.
'--filesystem=home', // Allow access to home directory.

Choose a reason for hiding this comment

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

I think for a text editor, exposing as much of the filesystem as possible would be a good idea. So use --filesystem=host instead. That's what gnome-builder does, for example.

finishArgs: [
'--share=ipc', '--socket=x11', // Allow showing X11 windows.
'--filesystem=home', // Allow access to home directory.
'--device=dri', // Allow OpenGL rendering.

Choose a reason for hiding this comment

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

Other finish args to consider

  • --socket=pulseaudio if the app or any app extensions need audio output
  • --share=network to install extensions using the network connection
  • --talk-name=org.freedesktop.Notifications if this even uses libnotify for desktop notifications

Lastly a bit weird, but adding --filesystem=/tmp will make sure you get the system tmp directory, which chromium/electron is using for its single check. https://github.com/electron/electron/blob/master/chromium_src/chrome/browser/process_singleton_posix.cc#L12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can add all of these:

  • Indeed --share=network is a good idea, somehow I missed the fact that the application goes online to fetch extensions. Not to mention that it may want to push/pull from Git repositories!
  • As for notifications, VSCode itself does not use them, and checking the popular extensions page I do not see how any of them would like to use them, and searching for “notification” brings up zero results... but anyway it's a small allowance, but it doesn't hurt having it.
  • Funnily enough, there's a few extensions which use audio devices, so let's add that, too!

@Tyriar Tyriar self-assigned this Nov 28, 2016
@aperezdc
Copy link
Contributor Author

Pushed an updated version with @mattdangerw's suggestions applied.

@aperezdc
Copy link
Contributor Author

...and updated again to symlink /app/bin/code-oss to the launcher shell script at /app/share/code-oss/bin/code-oss instead of symlinking to the Electron binary. This enables passing command line arguments normally, and things like the following do work:

flatpak run com.microsoft.CodeOSS --wait --goto file.js:10:3

(Probably people using the Flatpak will want to do alias code='flatpak run com.microsoft.CodeOSS' as a shortcut 😉)

@Tyriar
Copy link
Member

Tyriar commented Nov 29, 2016

@aperezdc we can't automatically install a reliable bin command due to the lack of sudo right? Is this typical for flatpak usage, to have a long command to run?

@aperezdc
Copy link
Contributor Author

aperezdc commented Nov 30, 2016

@Tyriar Sorry, I do not understand what do you mean exactly... Are you wondering about the flatpak binary and how to get it into the CI? If that is what you are worried about, there is a documented way which doesn't require sudo:

addons:
  apt:
    packages:
      - imagemagick
      - flatpak

I'll try to add the needed bits to the .travis.yml file myself, and one more commit to PR — it seems like a good idea to have it done already as part of this set of changes.

Or do you mean something else?

@aperezdc
Copy link
Contributor Author

aperezdc commented Nov 30, 2016

...but I see that .travis.yml does not call any of the tasks that build Linux packages, my previous comment was quite dumb.

@Tyriar I think now I get what you meant. And no, we cannot install things in the host machine when using Flatpak, because Flatpak bundles are self-contained and the applications run in a sandbox. Not even the libraries from the host are used: the ones from the runtime/version specified at build time are used, which ensures the application will keep running regardless of what changes in the host. This is by design. On the upside, notice how the .desktop file, icons, and a few other things around are renamed to contain com.microsoft.CodeOSS: Flatpak will export those files out of the bundle when installing, and place them in a location where the desktop shell will notice them. Even if we don't install a launcher script under /usr/bin the user will see the launcher being added to their applications list in any desktop environment which follows the XDG base applications spec (that includes GNOME, XFCE, Cinnamon, Mate, Budgie, Enlightenment, LXDE, and many others).

@Tyriar
Copy link
Member

Tyriar commented Nov 30, 2016

@aperezdc I was mainly just wondering is that's typical for using applications installed via flatpak, I have my answer now 😄

I'll look at merging this after the November/December end game happens as things are a bit busy currently. Could you sign the CLA in #16169 (comment) in the meantime? Thanks again!

@Tyriar Tyriar added this to the January 2017 milestone Nov 30, 2016
@aperezdc
Copy link
Contributor Author

I have signed the CLA a few minutes ago. Does @msftclas notice automatically or do we have to poke it somehow?

@Tyriar
Copy link
Member

Tyriar commented Dec 1, 2016

@aperezdc it normally happens automatically I think, I'll close and reopen the PR and see if that makes a difference.

@Tyriar Tyriar closed this Dec 1, 2016
@Tyriar Tyriar reopened this Dec 1, 2016
@msftclas
Copy link

msftclas commented Dec 1, 2016

Hi @aperezdc, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@Tyriar
Copy link
Member

Tyriar commented Dec 1, 2016

There we go 😄

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Most of the 1.8 release stuff is out of the way so I can look at this again 😄

const linuxPackageRevision = Math.floor(new Date().getTime() / 1000);

const flatpakManifest = {
appId: 'com.microsoft.CodeOSS',
Copy link
Member

Choose a reason for hiding this comment

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

Our application id in the other packages is code[-insiders].desktop, should this be the same or is this a new ID we would be introducing? This should be sourced from https://github.com/Microsoft/vscode/blob/master/product.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flatpak application identifiers need to be reverse-domain style. Now that I come back to review this, I see that we could reuse com.visualstudio.code.oss (it is the value for darwinBundleIdentifier).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah reusing that should be fine for now.

appId: 'com.microsoft.CodeOSS',
sdk: 'org.freedesktop.Sdk',
runtime: 'org.freedesktop.Sdk',
runtimeVersion: '1.4',
Copy link
Member

Choose a reason for hiding this comment

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

What version is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the version of the runtime bundle that the application needs. It is used both for running (org.freedesktop.Platform) and building (org.freedesktop.Sdk). The current version is 1.4. Setting this ensures that the application will be always ran using an userland which conforms exactly to the ABI exposed by the 1.4 version of the runtime.

runtimeVersion: '1.4',
base: 'io.atom.electron.BaseApp',
baseFlatpakref: 'https://s3-us-west-2.amazonaws.com/electron-flatpak.endlessm.com/electron-base-app-master.flatpakref',
command: "code-oss",
Copy link
Member

Choose a reason for hiding this comment

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

command: product.applicationName

baseFlatpakref: 'https://s3-us-west-2.amazonaws.com/electron-flatpak.endlessm.com/electron-base-app-master.flatpakref',
command: "code-oss",
symlinks: [
['/share/code-oss/bin/code-oss', '/bin/code-oss'],
Copy link
Member

Choose a reason for hiding this comment

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

This should use product.applicationName instead of code-oss

const destination = '.build/linux/flatpak/' + flatpakArch;

return function () {
const all = [16, 24, 32, 48, 64, 128, 192, 256, 512].map(function (size) {
Copy link
Member

Choose a reason for hiding this comment

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

Are all these sizes necessary? The smaller ones in particular would probably look pretty crappy being scaled like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fortunately the icon has clean, flat shapes and it scales down quite well. This is how the 16x16 px version looks:

com visualstudio code oss

24x24 px:

com visualstudio code oss

and 32x32 px:

com visualstudio code oss

<image>https://code.visualstudio.com/home/home-screenshot-linux-lg.png</image>
<caption>Editing a TypeScript file and searching for extensions.</caption>
</screenshot>
<screenshot>
Copy link
Member

Choose a reason for hiding this comment

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

Let's just keep the default screenshot for now, I've created #17131 to follow this up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to have only the default screenshot.

<summary>Code editor for developers supporting integration with existing tools</summary>
<description>
<p>
Visual Studio Code is a lightweight but powerful source code editor which
Copy link
Member

Choose a reason for hiding this comment

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

Let's go with the standard description now as defined in #17130, also could you put it all one line? <p>Visual Studio...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the description from #17130 results in:

vscode % appstream-util validate resources/linux/code.appdata.xml
resources/linux/code.appdata.xml: FAILED:
• tag-missing           : <translation> not specified
• tag-missing           : <update_contact> is not present
• style-invalid         : <p> cannot contain a hyperlink [Visual Studio Code is a new choice of tool that combines the simplicity of a code editor with what developers need for the core edit-build-debug cycle. See https://code.visualstudio.com/docs/setup/linux for installation instructions and FAQ.]

Using validate-relax still passes. Should we keep the URL in the description, or can I just remove the last sentence?

Copy link
Member

Choose a reason for hiding this comment

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

Removing the last line would be great. I'll fix up <translation> later and can you add this:

<update_contact>vscode-linux@microsoft.com</update_contact>

<?xml version="1.0" encoding="UTF-8"?>
<component type="desktop">
<id>@@NAME@@.desktop</id>
<metadata_license>CC-BY-SA-3.0</metadata_license>
Copy link
Member

Choose a reason for hiding this comment

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

@chrisdias do we have a preferred license for metadata files like these?

<project_license>MIT</project_license>
<name>@@NAME_LONG@@</name>
<url type="homepage">https://code.visualstudio.com</url>
<summary>Code editor for developers supporting integration with existing tools</summary>
Copy link
Member

Choose a reason for hiding this comment

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

As in #17130, let's make this "Code editing. Redefined." for now

<component type="desktop">
<id>@@NAME@@.desktop</id>
<metadata_license>CC-BY-SA-3.0</metadata_license>
<project_license>MIT</project_license>
Copy link
Member

Choose a reason for hiding this comment

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

This will need to change to be based on product.json but I just realized we don't do it for the rpm package so this is work I'll need to follow up 😄 #17133

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Picking the value from product.json once that PR is landed will be just perfect 👍

Copy link
Member

Choose a reason for hiding this comment

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

Merged

@@ -0,0 +1,46 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

Could you check that this validates against:

appstream-util validate code.appdata.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checked, and fixed a couple of issues. Right now it says:

vscode % appstream-util validate resources/linux/code.appdata.xml
resources/linux/code.appdata.xml: FAILED:
• tag-missing           : <translation> not specified
• tag-missing           : <update_contact> is not present
Validation of files failed

These are optional tags, and validate-relax succeds:

vscode % appstream-util validate-relax resources/linux/code.appdata.xml
resources/linux/code.appdata.xml: OK
vscode % 

This adds support for building Flatpak application bundles, which should be
runnable on any Linux distribution with Flatpak support. See the page at
http://flatpak.org/getting.html

New Gulp tasks are added to prepare a directory with the needed files
(vscode-linux-${arch}-prepare-flatpak), assembling the Flatpak bundles
(vscode-linux-${arch}-flatpak), and for cleaning the build directories
(clean-vscode-linux-${arch}-flatpak). This mimics how the Debian and RPM
package creation works. Hence, building an application bundle is done with:

  $ gulp vscode-linux-x64-flatpak
  [...]
  $ ls *.flatpak
  com.visualstudio.code.oss-x86_64.flatpak

Installing and running the application is achieved with:

  $ flatpak --user install --bundle com.visualstudio.code.oss-x86_64.flatpak
  $ flatpak run com.visualstudio.code.oss

(Removing "--user" would install the application system-wide. Also note that
the "org.freedesktop.Platform" runtime needs to be installeed, check the
Flatpak website for instructions.)

The "flatpak-bundler" development dependency provides an asynchronous API to
invoke "flatpak-builder". The "gulp-image-resize" module is used to
incorporate scaling of the application icon to multiple sizes, and uses
ImageMagick (or GraphicsMagick) under the hood, which is commonly available on
most GNU/Linux distributions.

Instead of building the Electron dependencies ourselves, this uses the
io.atom.electron.BaseApp bundle as base application.

Instead of building the Electron dependencies, this reuses the
"io.atom.electron.BaseApp" bundle as base application. The contents of the
base application are imported at build time, and then Electron and the
prepared files added on top. The prebuilt base application will be fetched
automatically using the URL specified in the "baseFlatpakref" attribute of the
manifest, and it is kindly hosted at Amazon S3 by Endless Computers
(https://endlessm.com). The sources for the base application can be found at:

  https://github.com/endlessm/electron-flatpak-base-app/
@aperezdc
Copy link
Contributor Author

@Tyriar: I have pushed again, this time only with the commit that adds the initial support (the rest of the commits were stashed to a flatpak-follow-up branch, to be submitted later). All the issues mentioned in the review (thanks!) were addressed.

@Tyriar
Copy link
Member

Tyriar commented Dec 14, 2016

@aperezdc thanks for the great work 😄 we probably won't distribute flatpaks just yet, but they should now be available for OSS builds! Looking forward to getting appdata.xml in as well.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants