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

Reports of build problems #21

Closed
alerque opened this issue Feb 27, 2020 · 11 comments
Closed

Reports of build problems #21

alerque opened this issue Feb 27, 2020 · 11 comments

Comments

@alerque
Copy link
Collaborator

alerque commented Feb 27, 2020

I have a report of problems trying to build the Arch Linux package, the user says they had to globally install some NPM module before it would go. Given what I know about this project that should not be the case.

Does that comment mean anything to you? Is there something missing in this projects internal deps? Why would it manifest on some systems and not others?

I'm happy to fix the packaging if there is anything to do, but I don't know what that might be. It's basically just running electron-rebuild and electron-packager plus adding a wrapper script and desktop file.

@tobias-klein
Copy link
Member

The reports are about the following dependencies:

Issue fixed after executing these two commands:
$ sudo npm -g install fstream
added 16 packages from 4 contributors in 2.429s
$ sudo npm -g install block-stream
added 2 packages from 1 contributor in 0.538s
Then was able to successfully install ezra-project.

I never had to install those packages (fstream, blockstream) explicitly to make Ezra Project successfully build.

The necessary dependencies for building on Linux are documented here:
https://github.com/tobias-klein/ezra-project/blob/master/BUILD.md#install-dependencies

Something currently not in the build instructions (I'll add that) is that electron-packager has to be installed.

@alerque
Copy link
Collaborator Author

alerque commented Feb 27, 2020

Thanks. I kind of suspect installing those 'deps' only incidentally poked something else on the system, maybe even fixing a broken npm or something like that.

In the mean time, what makes you say electron-packager has to be installed? I don't have it installed and the build system I'm using works fine because it uses the vendored deps in this project to run $(npm bin)/electron-rebuild and such, and apparently you have the electron package specified as a dependency already. It doesn't have to be on the system I don't think.

Also, I looked at the link you sent with this line:

Install compiler/lib dependencies by running the following command: sudo apt-get install build-essential npm nodejs libsqlite3-0 libcurl4-gnutls-dev libicu-dev pkg-config git cmake subversion

Really? Subversion? Are you sure? Cmake?

I understand these are Ubuntu land dependencies and need translating to other distros, but I don't see any part of the build system that uses subversion. Is there some run time dependency on it?

@alerque
Copy link
Collaborator Author

alerque commented Feb 27, 2020

Wow, I just fell down a rabbit hole and did not land in wonderland. I started trying to debuild the build process and there is all sorts of crazy stuff going on. You're mucking around with stuff in node_modules after npm does it's thing in there? Ugg. Off with her¹ head!

¹ "Her" being Electron.

@tobias-klein
Copy link
Member

tobias-klein commented Feb 28, 2020

In the mean time, what makes you say electron-packager has to be installed? I don't have it installed and the build system I'm using works fine because it uses the vendored deps in this project to run $(npm bin)/electron-rebuild and such, and apparently you have the electron package specified as a dependency already. It doesn't have to be on the system I don't think.

I think you're right about that. electron-packager is specified in the dev-dependency list in package.js. I'll take it out of the build page.

Also, I looked at the link you sent with this line:

Install compiler/lib dependencies by running the following command: sudo apt-get install build-essential npm nodejs libsqlite3-0 libcurl4-gnutls-dev libicu-dev pkg-config git cmake subversion

Really? Subversion? Are you sure? Cmake?

I understand these are Ubuntu land dependencies and need translating to other distros, but I don't see any part of the build system that uses subversion. Is there some run time dependency on it?

node-sword-interface depends on Sword and used to directly fetch the sword sources from the Crosswire SVN repo. As of now, SVN is not needed any longer. For building node-sword-interface Sword is now fetched from the BibleTime Git mirror of the Sword repo. Cmake is also necessary in the context of building Sword.

Wow, I just fell down a rabbit hole and did not land in wonderland. I started trying to debuild the build process and there is all sorts of crazy stuff going on. You're mucking around with stuff in node_modules after npm does it's thing in there? Ugg. Off with her¹ head!

¹ "Her" being Electron.

There's two things done there in the context of my packaging/build scripts:

  1. npm run prune-node-modules => runs a special cleaning tool that gets rid of optional files under node_modules. This step is optional, but reduces the (massive) size of the node_modules directory quite significantly.
  2. npm run cleanup-gyp-shebang. At the time when I integrated this step, node-gyp still depended on Python 2 and used old-style script shebangs (first line in script) referencing /usr/bin/python instead of /usr/bin/python2. This caused problems in some modern Linux distributions, so I'm patching the node-gyp files dynamically with that script.

@alerque
Copy link
Collaborator Author

alerque commented Feb 28, 2020

For building node-sword-interface Sword is now fetched from the BibleTime Git mirror of the Sword repo.

Fetched how? Is git a build time dependency? Or are you downloading an archive using curl? Could this download be injected by the builder instead? Is there a build time option to not download sword at all but link to a system installed one? Should this be an issue on node-sword-interface?

At the time when I integrated this step, node-gyp still depended on Python 2 and used old-style script shebangs [...]

Is this still the case or are those scripts now Python 3 compatible? I only have the system default Python (3.8 right now) document as a dependency, if it also requires Python-2 to build that's something else I should change.

@tobias-klein
Copy link
Member

tobias-klein commented Feb 28, 2020

Fetched how? Is git a build time dependency? Or are you downloading an archive using curl? Could this download be injected by the builder instead? Is there a build time option to not download sword at all but link to a system installed one? Should this be an issue on node-sword-interface?

Yeah, the actual root-cause is there, but of course it also impacts Ezra Project indirectly.
I would have to research a bit more on the gyp documentation regarding how to make this optional / supply a possibility to also build against the system installed sword library. Feel free to create an issue on node-sword-interface. Not sure whether we're able to fix this, though. Feel free to contribute the fix :)

The script used for building on Linux is this one: node-sword-interface/scripts/build_sword.sh

The script is linked in node-sword-interface/binding.gyp.

This script currently uses git and cmake directly.

On your other comment:

At the time when I integrated this step, node-gyp still depended on Python 2 and used old-style script shebangs [...]

Is this still the case or are those scripts now Python 3 compatible? I only have the system default Python (3.8 right now) document as a dependency, if it also requires Python-2 to build that's something else I should change.

The step npm run cleanup-gyp-shebang is technically only relevant when generating RPM packages (using electron-installer-redhat) based on a Ezra Project "distribution" (created with electron-packager / npm run package-linux). You find a comment in the actual script for this:

The package node-gyp (Node.js native addon build tool) still depends on Python 2. However, this causes problems with rpm-build.
The following snippet basically changes all plain Python shebangs of the node-gyp Python scripts to a specific Python 2 shebang

The corresponding issue on node-gyp is still open.

@tobias-klein
Copy link
Member

@alerque Any additional feedback on this? Since there are no reproducible build issues at the moment I think I'd rather close this issue.

@alerque
Copy link
Collaborator Author

alerque commented Mar 7, 2020

The corresponding issue on node-gyp is still open.

The issue is still open, but reading the comments I have no idea why. It is long since fixed. Also I've been experimenting with builds and am able to get gyp stuff to build just fine with Python 3, including the three packages Ezra depends on.

Your issue here is not that gyp doesn't support Python 3, it's that some distro you are running on only supports Python 2 — or defaults to it or something. I think that hack should be removed from the default build rules and only applied on the specific distro that is a problem.

@tobias-klein
Copy link
Member

The corresponding issue on node-gyp is still open.

The issue is still open, but reading the comments I have no idea why. It is long since fixed. Also I've been experimenting with builds and am able to get gyp stuff to build just fine with Python 3, including the three packages Ezra depends on.

Your issue here is not that gyp doesn't support Python 3, it's that some distro you are running on only supports Python 2 — or defaults to it or something. I think that hack should be removed from the default build rules and only applied on the specific distro that is a problem.

Good point! Thanks for paying attention to the quality of the build system here. I'll have another look!

@tobias-klein
Copy link
Member

These are the error messages on CentOS 8 when using rpm tools for packaging:

<snip>
*** ERROR: ambiguous python shebang in /usr/lib/ezra-project/resources/app/node_modules/node-gyp/gyp/samples/samples: #!/usr/bin/python. Change it to python3 (or python2) explicitly.
*** ERROR: ambiguous python shebang in /usr/lib/ezra-project/resources/app/node_modules/node-gyp/gyp/gyp_main.py: #!/usr/bin/env python. Change it to python3 (or python2) explicitly.
*** ERROR: ambiguous python shebang in /usr/lib/ezra-project/resources/app/node_modules/node-gyp/gyp/pylib/gyp/easy_xml_test.py: #!/usr/bin/env python. Change it to python3 (or python2) explicitly.
</snip>

node-gyp may support Python 3, but as the error messages indicate - it has "ambiguous Python shebangs". And that's what the script cleanup_gyp_shebangs.sh fixes. This looks like it could also become an issue on other (rpm-based) distributions eventually.

@tobias-klein
Copy link
Member

I've now adjusted the build.sh script, so that it invokes cleanup-gyp-shebang only on rpm-based distributions.

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

No branches or pull requests

2 participants