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

Create a statically linked ELF/PE/Mach-O binary #259

Closed
ggrossetie opened this issue Apr 21, 2019 · 16 comments
Closed

Create a statically linked ELF/PE/Mach-O binary #259

ggrossetie opened this issue Apr 21, 2019 · 16 comments

Comments

@ggrossetie
Copy link
Member

I think it would be useful to add a binary in the npm package. If we do that, we would be able to execute the Reveal.js converter using npx:

$ npx -p @asciidoctor/reveal.js-converter asciidoctor-revealjs presentation.adoc

If we decide to publish a package with the same name as the command, then the command would be:

$ npx asciidoctor-revealjs presentation.adoc

Of course it would also be possible to install the package with npm:

$ echo {} > package.json        # eliminates warnings, use `npm init` if you prefer
$ npm i @asciidoctor/reveal.js-converter
$ ./node_modules/.bin/asciidoctor-revealjs presentation.adoc
@akosma
Copy link
Contributor

akosma commented Sep 20, 2019

I'm currently using this script (written in TypeScript) to generate the HTML slides (executed using ts-node):

// Drives the creation of the Reveal.js presentation using
// the asciidoctor-reveal.js JavaScript library.
import * as asciidoctorLib from 'asciidoctor.js';
import * as asciidoctorRevealjs from 'asciidoctor-reveal.js';
import * as fs from 'fs';
import * as path from 'path';

// Check that there is a parameter
if (process.argv.length < 3) {
    console.error('build.ts: Missing "filename" parameter. Exiting');
    process.exit(1);
}

// Check that the parameter points to a file that exists
const filename = process.argv[2];
const filepath : string = path.join(__dirname, '..', filename);
if (!fs.existsSync(filepath)) {
    console.error('build.ts: The path "%s" is invalid. Exiting.', filepath);
    process.exit(1);
}

// Drive the transformation from Asciidoc to HTML
const asciidoctor = asciidoctorLib();
asciidoctorRevealjs.register();
const options = {safe: 'safe', backend: 'revealjs'};
asciidoctor.convertFile(filename, options);

@obilodeau
Copy link
Member

I think this goal was achived through the merger of #254. The current master branch allows the use of a binary to build slides:

$ $(npm bin)/asciidoctor -r asciidoctor-reveal.js -b revealjs presentation.adoc

There's even a way to do so from PowerShell.

Can I close this?

@obilodeau obilodeau changed the title Create a binary for Node.js Create a statically linked ELF/PE/Mach-O binary Jan 8, 2020
@obilodeau
Copy link
Member

Can I close this?

After discussing with @Mogztter, I understand now what this aims to do. Updated the title of the ticket accordingly. Still aiming to achieve that shortly.

@obilodeau obilodeau added this to the 3.1.0 milestone Jan 8, 2020
@mojavelinux
Copy link
Member

mojavelinux commented Jan 8, 2020

Is the proposed solution different from nexe? If so, which one offers the better solution?

@ggrossetie
Copy link
Member Author

Is that an AsciiDoc link... 🤣
I've never used Nexe so I can't tell... But I found this quick comparison from a Nexe contributor: vercel/pkg#42 (comment)

To be honest, I don't quite understand the key differences... there is also mention of https://github.com/pmq20/node-packer

@mojavelinux
Copy link
Member

Is that an AsciiDoc link...

After 3 tries I finally got it right.

I've never used Nexe so I can't tell

It was the first tool I had come across that proposed to do something like this.

This seems to be the conclusion to that issue:

Pkg is without doubt my first choice for binaries and only fallback to Nexe for the specific reasons mentioned above

@mojavelinux
Copy link
Member

So to answer my question, basically the same. Hard to say exactly which one is better, but we can't go wrong it seems.

@obilodeau
Copy link
Member

pkg seems to generate ARM binaries also. Not that it is an important target architecture right now but one could build a slide deck on a Raspberry Pi 🤓

@mojavelinux
Copy link
Member

Plus!

@ggrossetie
Copy link
Member Author

@obilodeau Do you want to give it a try? I might work on it tomorrow unless you want to experiment with pkg and the Asciidoctor CLI 😀

Let me know if you have any questions.

@obilodeau
Copy link
Member

I started but I have to go. Should I name the binary asciidoctor-revealjs or asciidoctor-reveal.js I remember you having an opinion on that.

Also, bin/asciidoctor-revealjs already exists and is in Ruby so I'm working with npm/asciidoctor-revealjs now and using bin in package.json to link to it properly. I hope this is ok.

@obilodeau obilodeau modified the milestones: 3.1.0, 3.0.1 Jan 11, 2020
@ggrossetie
Copy link
Member Author

I started but I have to go. Should I name the binary asciidoctor-revealjs or asciidoctor-reveal.js I remember you having an opinion on that.

I prefer asciidoctor-revealjs.
In my opinion, a dot in binary name is somewhat unusual/unexpected.
Please note that you can define one or more names, see: https://github.com/asciidoctor/asciidoctor-cli.js/blob/87574b355a049a7320d79987b5c21d8e1f2495d4/package.json#L16-L17

Also, bin/asciidoctor-revealjs already exists and is in Ruby so I'm working with npm/asciidoctor-revealjs now and using bin in package.json to link to it properly. I hope this is ok.

That's completely fine since it's not mandatory to put this file in a bin directory.
But I think that you must add it to the files field: https://docs.npmjs.com/files/package.json#files

{
  "bin": {
    "asciidoctor-revealjs": "npm/asciidoctor-revealjs",
    "asciidoctor-reveal.js": "npm/asciidoctor-revealjs"
  },
  "files": [
    "npm/asciidoctor-revealjs"
  ],
}

@obilodeau
Copy link
Member

Trying to create binaries with pkg I run into a problem:

$ npx pkg -t "node12-linux-x64,node12-macos-x64,node12-win-x64" package.json --out-path dist/
> [email protected]
> Warning Cannot find module '@asciidoctor/cli' from '/home/olivier/src/asciidoc/asciidoctor-reveal.js/npm'
  /home/olivier/src/asciidoc/asciidoctor-reveal.js/npm/asciidoctor-revealjs
> Warning Cannot find module '@asciidoctor/reveal.js' from '/home/olivier/src/asciidoc/asciidoctor-reveal.js/npm'
  /home/olivier/src/asciidoc/asciidoctor-reveal.js/npm/asciidoctor-revealjs

Thinking about it, it kind of make sense since @asciidoctor/cli is not a dependency of this project and @asciidoctor/reveal.js is the project itself.

However, I tried specifying @asciidoctor/cli as a devDep and it didn't work. I also tried adding this to the package.json:

  "pkg": {
    "scripts": ["dist/main.js", "node_modules/@asciidoctor/**/*.js"]
  }

and it didn't work either. I'm starting to think that pkg doesn't support package names with @ in it.

@obilodeau
Copy link
Member

Ok, I got it to work. I needed to create a packages/asciidoctor-revealjs/ just like you have in asciidoctor.js repo so I could specify all deps (including @asciidoctor/reveal.js).

I think I have no other choice than to have a new package.json since the code requires itself. That said, there's a lot of copy and paste involved that I would like to get rid of.

I'll commit the ugly stuff but @Mogztter if you have any suggestion on cleaning it up I'm all ears.

@obilodeau
Copy link
Member

Ok, I think I made it better after all. I initially had a copy of the whole Opal generated code that I could remove with:

module.exports = require('@asciidoctor/reveal.js')

(inspired by your code again)

The only thing is the need to maintain two package.json and a slightly longer release process. Comments welcome anyhow if you think it can be simpler. I'll adjust the doc in this PR but later.

@obilodeau
Copy link
Member

The above 3 comments should have been on the PR #308. My mistake.

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