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

Change embedding of runtime to platform agnostic way #470

Closed
TheAssassin opened this issue Sep 7, 2017 · 11 comments
Closed

Change embedding of runtime to platform agnostic way #470

TheAssassin opened this issue Sep 7, 2017 · 11 comments

Comments

@TheAssassin
Copy link
Member

Thanks @teras for bringing up the idea, and working out this proposal.

The way the runtime is embedded at the moment apparently works on Linux only. As appimagetool is prepared to run on other platforms as well (see #466), we need to change this to a more platform agnostic way.

The idea is to basically encode the freshly built runtime into C source code (using xxd -i for instance), and compile and link this file into the appimagetool binary. The effect is the same, the pre-built binary is inside the built application and can be written out. The big advantage compared to now is that this method does not rely on any ELF magic, and could even run on other platforms such as Windows.

Implementing this in the build workflow shouldn't be too hard either, at least on Linux. Problems could arise on other platforms, as we need to cross-compile runtime.c to a Linux target before embedding it (a binary blob in the repository so that this could be avoided is a no-go). Cross-compilation of the runtime itself is not problematic at all due to the presence of clang on macOS, but we will have to do this for all the dependencies as well.

This issue is by the way the perfect chance to switch over completely to CMake. As the compilation of the runtime now has to be changed anyway, we should avoid doing it in both places again.

@teras
Copy link
Contributor

teras commented Sep 7, 2017

Just to confirm that xxd -i works perfectly under osx.

@probonopd
Copy link
Member

As said before, I'd also support just using plain old files, and no embedding trickery.

@probonopd
Copy link
Member

Also see #304.

@TheAssassin
Copy link
Member Author

@probonopd nobody plans to remove the runtime parameter.

TheAssassin added a commit that referenced this issue Sep 7, 2017
TheAssassin added a commit that referenced this issue Sep 7, 2017
Not directly related to #470, but improves the repository structure.
@azubieta
Copy link
Contributor

@TheAssassin are you still on this? Can it be closed?

@TheAssassin
Copy link
Member Author

No. There was a PR for that (#471), but it was never merged. Closing until further notice.

@probonopd
Copy link
Member

  • Being worked on -> no
  • Resolved -> no
  • Good idea -> yes

So why close it? Closing things just because they are not resolved yet is never a good rationale...

@TheAssassin
Copy link
Member Author

We decided not to embed runtimes in binaries on macOS etc. We don't need that feature any more, but can use what we have now.

@probonopd
Copy link
Member

I think there are still people who would like to produce AppImages on macOS and Windows. I think electron-builder moved to writing their own tools because we did not have this feature...

@TheAssassin
Copy link
Member Author

The idea was to ship the runtime separately for those platforms.

@probonopd
Copy link
Member

Right, I think that is an OK solution.

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

4 participants