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

com.visualstudio.code #150

Closed
wants to merge 7 commits into from
Closed

Conversation

nedrichards
Copy link
Member

@nedrichards nedrichards commented Nov 13, 2017

Extra-data based flatpak for Visual Studio Code (since the source code based edition has both bitrotted and depends on us getting network free electron builds). Furthermore, there is value in providing the official, closed source packages with MS keys etc. If we wanted to migrate to the free software version, or offer it then it'd be with the com.visualstudio.code.oss App ID.

This doesn't use the new syntax for extra-data since Endless hasn't updated it's version of flatpak beyond 0.9.8 yet so I can't test it.

@nedrichards
Copy link
Member Author

"type": "archive",
"url": "https://www.x.org/releases/individual/lib/libxkbfile-1.0.9.tar.bz2",
"sha256": "51817e0530961975d9513b773960b4edd275f7d5c72293d5a151ed4f42aeb16a"
}
Copy link
Member

Choose a reason for hiding this comment

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

whitespace.

"cp share/pixmaps/code.png export/share/icons/hicolor/512x512/apps/com.visualstudio.code.png",
"mkdir -p export/share/applications",
"cp share/applications/code.desktop export/share/applications/com.visualstudio.code.desktop",
"desktop-file-edit --set-key=Exec --set-value='code %F' export/share/applications/com.visualstudio.code.desktop",
Copy link
Member

@TingPing TingPing Nov 13, 2017

Choose a reason for hiding this comment

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

desktop-file-edit isn't in the runtime. Nevermind this runs off the Sdk...

"mv usr/* .",
"rmdir usr",
"mkdir -p export/share/icons/hicolor/512x512/apps",
"cp share/pixmaps/code.png export/share/icons/hicolor/512x512/apps/com.visualstudio.code.png",
Copy link
Member

Choose a reason for hiding this comment

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

Can be combined into a install -Dm644 ... command.

Copy link
Member

@TingPing TingPing Nov 13, 2017

Choose a reason for hiding this comment

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

Also since you are copying this it leaves a duplicate around you can remove, same applies for desktop and appdata.

]
},
{
"name": "nodejs",
Copy link
Member

Choose a reason for hiding this comment

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

Probably reasonable to clean up: /include /share /lib/node_modules/npm/changelogs /lib/node_modules/npm/doc /lib/node_modules/npm/html /lib/node_modules/npm/man /lib/node_modules/npm/scripts, etc

]
},
{
"name": "git",
Copy link
Member

Choose a reason for hiding this comment

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

Can probably cleanup: /share/git-gui

"commands": [
"ar x code.deb",
"rm -f code.deb",
"tar -xf data.tar.xz",
Copy link
Member

Choose a reason for hiding this comment

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

Forgot to delete this.

"desktop-file-edit --set-key=Exec --set-value='code %F' export/share/applications/com.visualstudio.code.desktop",
"desktop-file-edit --set-key=Icon --set-value='com.visualstudio.code' export/share/applications/com.visualstudio.code.desktop",
"mkdir -p export/share/appdata",
"sed s/code.desktop/com.visualstudio.code.desktop/ share/appdata/code.appdata.xml > export/share/appdata/com.visualstudio.code.appdata.xml"
Copy link
Member

@TingPing TingPing Nov 13, 2017

Choose a reason for hiding this comment

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

The license of this is incorrect:

	<metadata_license>Multiple, see https://code.visualstudio.com/license</metadata_license>
	<project_license>Multiple, see https://code.visualstudio.com/license</project_license>

It should be LicenseRef-proprietary=https://code.visualstudio.com/license

Copy link
Member

Choose a reason for hiding this comment

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

Also its valuable for us to add <releases>.

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean, this is upstream appdata. I'm always reluctant to override that where not actually necessary - but fair enough on the proprietary thing.

I'll open an issue upstream on this and the front, they've seemed pretty responsive to suggestions and patches thus far and I don't want to patch that downstream if possible since it's a bit fragile with a frequently released app..

Copy link
Member

Choose a reason for hiding this comment

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

Yea, the license fix is required but for releases reasonable to get upstream to do it.

@TingPing TingPing added the awaiting-changes Pull request waiting for inputs or changes from author label Nov 13, 2017
@TingPing
Copy link
Member

TingPing commented Nov 13, 2017

This still writes to ~/.vscode, it would be nice to override that if possible.

EDIT: Having the script run /app/extra/share/code/bin/code --extensions-dir=$XDG_DATA_HOME/vscode/extensions is a start.

Also shipping npm bundled has its own nightmare of paths to override:

@nedrichards
Copy link
Member Author

We should think about adding some of this dotfile stuff to the Review Guidelines because we basically don't talk about sandboxing and permissions at all there.

@nedrichards nedrichards removed the awaiting-changes Pull request waiting for inputs or changes from author label Nov 16, 2017
@nedrichards
Copy link
Member Author

OK, that should cover the first review. ;-)

@nedrichards
Copy link
Member Author

"desktop-file-edit --set-key=Exec --set-value='code %F' export/share/applications/com.visualstudio.code.desktop",
"desktop-file-edit --set-key=Icon --set-value='com.visualstudio.code' export/share/applications/com.visualstudio.code.desktop",
"mkdir -p export/share/appdata",
"sed -e s/code.desktop/com.visualstudio.code.desktop/ -e 's/Multiple, see /LicenseRef-proprietary=/' share/appdata/code.appdata.xml > export/share/appdata/com.visualstudio.code.appdata.xml"
Copy link
Member

Choose a reason for hiding this comment

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

's/Multiple, see /LicenseRef-proprietary=/'

s/.../.../g since it has multiple matches

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks!

@nedrichards
Copy link
Member Author

Ugh, so I don't think using the appdata from inside the package can work - since it's extra-data that means that the package fails the flathub validation. So I'm going to fork the open source version's appdata, modify the Licence of the binary and add versions tags as suggested.

Such is life.

@nedrichards
Copy link
Member Author

nedrichards commented Nov 16, 2017

Newer build, now with appdata, hopefully https://flathub.org/builds/#/builders/1/builds/1003

<?xml version="1.0" encoding="UTF-8"?>
<component type="desktop">
<id>com.visualstudio.code.desktop</id>
<metadata_license>Multiple, see https://code.visualstudio.com/license</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.

This should be the same syntax as 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.

One problem though is this license would then not be distributable, but.. the same appdata file is under a different license if the foss version is built...

upstream really needs to fix this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I'm going to update the metadata to be under the MIT, which is where I've 'forked' it from for these purposes.

@amtlib-dot-dll
Copy link

@nedrichards Well I am using my cloud-built version of code-oss now https://github.com/amtlib-dot-dll/vscode-build-travis/blob/master/.travis.yml https://github.com/amtlib-dot-dll/vscode-build-travis/tree/out but yeah you are right we need to do some work to achieve offline and reproducible builds.

@nedrichards
Copy link
Member Author

Another (hopefully final) test build: https://flathub.org/builds/#/builders/1/builds/1028

@TingPing
Copy link
Member

Repository has been created: https://github.com/flathub/com.visualstudio.code

@TingPing TingPing closed this Nov 21, 2017
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.

None yet

3 participants