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

Add asar support in Atom #5382

Merged
merged 18 commits into from Apr 3, 2015

Conversation

Projects
None yet
8 participants
@zcbenz
Copy link
Member

zcbenz commented Feb 3, 2015

By using asar to package the app folder into a single file, we should be able to speed up the install time and startup time on computers that run on HDD significantly.

Known issues:

  • index.js not loading on Windows.
  • Can not interpret Less stylesheets because of graceful-fs not compatible with asar.
  • Program not quitting on Windows.
  • Tasks not working.
  • apm and atom commands can not be installed.
  • apm can not show built-in packages.

@zcbenz zcbenz force-pushed the asar branch from a2cc46c to 963d50d Feb 3, 2015

@benogle

This comment has been minimized.

Copy link
Contributor

benogle commented Feb 3, 2015

WIN. 💥

@paulcbetts

This comment has been minimized.

Copy link
Contributor

paulcbetts commented Feb 4, 2015

This PR is much shorter than I thought it would be, maybe we'll move to asar sooner rather than later

grunt.registerTask 'generate-asar', 'Generate asar archive for the app', ->
done = @async()

appDir = grunt.config.get('atom.appDir')

This comment has been minimized.

@paulcbetts

paulcbetts Feb 4, 2015

Contributor

Check to make sure this dir exists, otherwise if you run script/grunt generate-asar explicitly, you regenerate an empty asar file

@paulcbetts

This comment has been minimized.

Copy link
Contributor

paulcbetts commented Feb 4, 2015

This PR will forever and ever kill all of the long path problems on Windows too,

@Zireael07

This comment has been minimized.

Copy link

Zireael07 commented Mar 2, 2015

Any progress on the last bullet point?

@paulcbetts paulcbetts referenced this pull request Mar 11, 2015

Closed

Update.exe chewing CPU #5530

@zcbenz zcbenz changed the title WIP: Add asar support in Atom Add asar support in Atom Mar 25, 2015

@zcbenz zcbenz force-pushed the asar branch 4 times, most recently from 7f6c983 to 8a3c333 Mar 25, 2015

@zcbenz

This comment has been minimized.

Copy link
Member Author

zcbenz commented Mar 25, 2015

Everything is working now in this branch.

@thomasjo

This comment has been minimized.

Copy link
Member

thomasjo commented Mar 25, 2015

Everything is working now in this branch.

Awesome! :shipit: 🚀

@Zireael07

This comment has been minimized.

Copy link

Zireael07 commented Mar 25, 2015

Great!

@@ -100,6 +100,11 @@ class Task
@childProcess.removeAllListeners()
@childProcess.on 'message', ({event, args}) =>
@emit(event, args...) if @childProcess?
# Catch the errors that happened before task-bootstrap.
@childProcess.stdout.on 'data', (data) ->
console.log data.toString()

This comment has been minimized.

@kevinsawicki

kevinsawicki Mar 25, 2015

Member

Should these be reported somewhere else? Or just ignored?

This comment has been minimized.

@zcbenz

zcbenz Mar 26, 2015

Author Member

This only prints when there is output before the task-bootstrap script starts to run, e.g. when there is regression in atom-shell. So as long as atom-shell works as expected, there will be nothing printed.

@@ -145,9 +145,9 @@ module.exports = (grunt) ->
cp 'src', path.join(appDir, 'src'), filter: /.+\.(cson|coffee)$/
cp 'static', path.join(appDir, 'static')

cp path.join('apm', 'node_modules', 'atom-package-manager'), path.join(appDir, 'apm'), filter: filterNodeModule
cp path.join('apm', 'node_modules', 'atom-package-manager'), path.resolve(appDir, '..', 'apm'), filter: filterNodeModule

This comment has been minimized.

@kevinsawicki

kevinsawicki Mar 25, 2015

Member

Since the apm path is changing, does this need to be updated?

getApmPath: ->

@paulcbetts

This comment has been minimized.

Copy link
Contributor

paulcbetts commented Mar 25, 2015

@zcbenz

This comment has been minimized.

Copy link
Member Author

zcbenz commented Mar 26, 2015

I just found that apm can not list built-in packages in asar archive, I'll try to fix it in apm.

@Zireael07

This comment has been minimized.

Copy link

Zireael07 commented Mar 28, 2015

There was an apm update in the meantime, I think...

Can't wait for this, should significantly speed up install/update.

zcbenz added some commits Jan 29, 2015

Do not set "cwd" for tasks
The task can be inside an asar archive, which is not allowed to be cwd.

@zcbenz zcbenz force-pushed the asar branch from dbcd65c to 685e4eb Mar 30, 2015

zcbenz added some commits Mar 30, 2015

@kevinsawicki

This comment has been minimized.

Copy link
Member

kevinsawicki commented Apr 1, 2015

So changing the atom.sh and apm paths means that after upgrading, the existing symlinks will be broken right?

So if the person previously ran Atom > Install Shell Commands and auth-ed the operation, they will be out of luck until they run it again?

@zcbenz

This comment has been minimized.

Copy link
Member Author

zcbenz commented Apr 2, 2015

So if the person previously ran Atom > Install Shell Commands and auth-ed the operation, they will be out of luck until they run it again?

Yeah I believe so. However if they can install the shell commands without privilege I think the commands will be updated silently.

@zcbenz zcbenz force-pushed the asar branch from df0d831 to 6910c81 Apr 3, 2015

@zcbenz zcbenz force-pushed the asar branch from 7a94b6d to 15ec7a4 Apr 3, 2015

@zcbenz

This comment has been minimized.

Copy link
Member Author

zcbenz commented Apr 3, 2015

There are some random TextEditorComponent specs failing.

zcbenz added a commit that referenced this pull request Apr 3, 2015

Merge pull request #5382 from atom/asar
Add asar support in Atom

@zcbenz zcbenz merged commit ffa6d4a into master Apr 3, 2015

4 checks passed

atom Build #1977314 succeeded in 664s
Details
atom-linux Build #1977275 succeeded in 290s
Details
atom-rpm Build #1977276 succeeded in 477s
Details
atom-win Build #1977277 succeeded in 634s
Details

@zcbenz zcbenz deleted the asar branch Apr 3, 2015

@Zireael07

This comment has been minimized.

Copy link

Zireael07 commented Apr 3, 2015

Woot, merged?!

@kevinsawicki

This comment has been minimized.

Copy link
Member

kevinsawicki commented Apr 3, 2015

Looks like this broke the symbols view so I'm going to revert it for now so it isn't broken on master, atom/symbols-view#89

@m1ga

This comment has been minimized.

Copy link

m1ga commented Apr 3, 2015

asar will create a tar like file archive, right? How many files would be packed together? So instead of the 11000 files in the windows /app folder you would have only one (136 MB) file?

@kevinsawicki

This comment has been minimized.

Copy link
Member

kevinsawicki commented Apr 3, 2015

asar will create a tar like file archive, right? How many files would be packed together? So instead of the 11000 files in the windows /app folder you would have only one (136 MB) file?

Yup, exactly this.

@mark-hahn

This comment has been minimized.

Copy link
Contributor

mark-hahn commented Apr 3, 2015

Does this mean we wouldn't be able to go in and modify code?

On Fri, Apr 3, 2015 at 11:40 AM, Kevin Sawicki notifications@github.com
wrote:

asar will create a tar like file archive, right? How many files would be
packed together? So instead of the 11000 files in the windows /app folder
you would have only one (136 MB) file?

Yup, exactly this.


Reply to this email directly or view it on GitHub
#5382 (comment).

@kevinsawicki

This comment has been minimized.

Copy link
Member

kevinsawicki commented Apr 3, 2015

Does this mean we wouldn't be able to go in and modify code?

Yes, dev mode will still load from source though.

@mark-hahn

This comment has been minimized.

Copy link
Contributor

mark-hahn commented Apr 3, 2015

Luckily I'm always in dev mode. I'm one of those hackers who can't leave
any code untouched. It's a disease.

On Fri, Apr 3, 2015 at 11:57 AM, Kevin Sawicki notifications@github.com
wrote:

Does this mean we wouldn't be able to go in and modify code?

Yes, dev mode will still load from source though.


Reply to this email directly or view it on GitHub
#5382 (comment).

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