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

build: fallback to autogen.sh if configure fails, and refactor config and build scripts for easier troubleshooting #125

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

beorn
Copy link

@beorn beorn commented Jan 7, 2022

Note to @addaleax - I am not very familiar with the node-gyp build system, and these changes do come with risks, so unless after your review you feel very confident in them, then I suggest we don't do these. It's perhaps better to tackle refactorings as part of a larger improvement effort on the build system for both windows and non-windows if/when that is done. Let me know if I should just close this PR.


This PR includes refactorings and some extra robustness if ./configure fails. It is arguably an improvement, but I'm not 100% sure removing the <! hack won't cause any problems (why was it there to begin with?).

For inspiration we could also look at node-ffi, which previously used a config and build script like we do, and:

  1. include the unpacked source files, perhaps after re-running ./autogen.sh, so we don't need the config.sub patch
  2. set up separate .gyp target for the libzma library

Changes:

  • liblzma-config.sh: if configure fails, run autogen.sh / autoconf (*)
  • Refactor build action in bindings.gyp so that config and build scripts output is shown, making troubleshooting easier:
    • It doesn't seem necessary to run configure using <! early variable expansion of 'input' key , so just run it as a regular build action, and make build dependent on it (in fact, they could be combined into one)
    • This means scripts can just output to stdout/stderr directly (instead of log files) for easier troubleshooting
    • Script output is made less verbose (but hopefully more clear)
  • For reference, added link to actual XZ library used to README.md
(*) To run autogen.sh you need a full autoconf build environment to build, such as this shell.nix file defines if using Nix
{ pkgs ? import <nixpkgs> {} }:

pkgs.mkShell {
  buildInputs = [
    pkgs.darwin.cctools
    pkgs.python3
    pkgs.gettext
    pkgs.autoconf
    pkgs.automake
    pkgs.libtool
    pkgs.nodejs-17_x
    pkgs.p7zip # for yarn prepare-win
  ];
}
If encountering: gyp: name 'openssl_fips' is not defined while evaluating condition 'openssl_fips != ""' in binding.gyp while trying to load binding.gyp Either wait for nodejs/node-gyp#2497 or update us node 17.

@beorn beorn changed the title Fix build for darwin-arm64 by running autogen.sh build: fix build for darwin-arm64 by running autogen.sh Jan 7, 2022
@beorn beorn force-pushed the fix/darwin-arm64 branch 9 times, most recently from ce62636 to c7dacdf Compare January 10, 2022 04:07
@beorn beorn changed the title build: fix build for darwin-arm64 by running autogen.sh build: fix build for darwin-arm64 through config.sub patch and fallback to autogen.sh, and refactor config and build scripts for easier troubleshooting Jan 10, 2022
- `liblzma-config.sh`: If configure fails, run autogen.sh / autoconf (*)
- refactor `build` action in `bindings.gyp` so that config and build scripts output is shown, making troubleshooting easier
  - `build` action refactored into `build` and `configure` actions with dependencies between them
  - since `configure` is a separate action instead of a command that runs as part of the 'input' key, it is free to output anything
  - script output is made less verbose and more clear, so it's not necessary to send to log file
  - this essentially means that configure & build could be the same script (configure is not run as part of configure stage of gyp)
- for reference, add link to XZ library used to README.md

(*) The drawback of this is that the fallback to autoconf requires a full autoconf type build environment, but it is a last resort fallback.
@beorn beorn changed the title build: fix build for darwin-arm64 through config.sub patch and fallback to autogen.sh, and refactor config and build scripts for easier troubleshooting build: fallback to autogen.sh if configure fails, and refactor config and build scripts for easier troubleshooting Jan 11, 2022
@beorn beorn requested a review from addaleax January 11, 2022 23:55
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

2 participants